Repository navigation
feat(event-types): transfer ownership without changing the booking URL - #68
Conversation
|
All contributors have signed the CLA. ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
This repo's model isn't fully set up yet. OPENAI_COMPATIBLE_MODEL env var is required when the model is set to "openai-compatible/byok". set it to the model ID served by your OpenAI-compatible endpoint (e.g. a Cloudflare AI Gateway or DashScope model). see https://docs.pullfrog.com/openai-compatible for setup.
|
|
recheck |
|
Warning Your repo is set to an OpenAI‑compatible model but lacks the required model ID, so every run fails before starting. Every Pullfrog run on Calnode has failed since September 18 (6 runs, no successes), so this review did not happen. The repo is configured to use "openai‑compatible/byok", which requires you to specify which model your endpoint serves. Without OPENAI_COMPATIBLE_MODEL set, the agent cannot launch. To fix it:
|
|
@pullfrog please review this PR |
There was a problem hiding this comment.
Important
The transfer control is inert for fixed and round_robin event types — the two most common host configurations — because the picker only offers non-owner required hosts, and the server rejects everything else. The PR only works end-to-end for collective events. See the inline comment and the member-owned-event question below.
Reviewed changes
- New transfer endpoint —
POST /v1/event-types/{slug}/transfer: admin-and-current-owner gated, target must be an activerequiredhost, blocked while upcoming bookings exist; movesevent_types.user_idplus the old owner's event-scopedavailability_rulesin one transaction, preserving id, slug and booking history. - Hosts-tab control — server-rendered transfer panel in the event editor;
loadHostsnow runs on mount and again after save so the eligible-target list stays fresh (commitd4e6aa9). - Tests + docs — handler test for valid transfer, stale owner, ineligible host and upcoming bookings; ARCHITECTURE §7 note.
ℹ️ Member-owned events have no transfer path
The endpoint requires the caller to be an admin and the event's owner, and the list/get queries only ever return events the caller owns or hosts. An event owned by a non-admin member therefore cannot be transferred by anyone — the usual offboarding case still needs promote → transfer → demote.
Technical details
# transfer cannot reach member-owned events
## Affected sites
- `internal/handler/event_type_transfer.go:9,18` — `requireAdmin`, then `body.ExpectedOwnerID != user.ID`; the ownership-scoped `SELECT` at `:29` means only the event's current owner can ever pass.
- `internal/handler/event_type.go:164-179` — `listEventTypesQuery` / `getEventTypeQuery` scope the view to events the caller owns or hosts, so a non-owner admin cannot even see the event.
## Open questions for the human
- Should a workspace owner/admin be able to transfer an event they do not own?
- If not, is the promote → transfer → demote workaround acceptable, and should it be documented?ℹ️ Nitpicks
expected_owner_idmust always equal the caller's own id (event_type_transfer.go:18), so it can never express "I believed X owned this"; the stale-request guard already falls out of the ownership-scopedSELECT. Either drop it or make it meaningful.- The new test covers the happy path, stale owner, ineligible host and upcoming bookings, but not an archived target, a non-admin caller, or the rule-collision path.
opencode-go/deepseek-v4.1-flash | 𝕏
| try { | ||
| const res = await api.get<{ items: EventTypeHost[] }>(`/v1/event-types/${slug}/hosts`); | ||
| const items = res.items ?? []; | ||
| transferHosts = items.filter(h => h.role === 'required' && h.user_id !== $currentUser?.id); |
There was a problem hiding this comment.
Because this filter excludes the current user and demands required, the dropdown is empty for the two most common configurations: a fixed event's only required host is its owner (written that way by saveET at line 386), and a round_robin event's hosts are all rotation (line 376). The panel then renders with just the placeholder and a permanently disabled button, and the server gate at event_type_transfer.go:35 would reject those targets anyway — so transfer only works for collective events with a non-owner required co-host.
Technical details
# transfer is unreachable for fixed and round_robin events
## Affected sites
- `frontend/src/routes/event-types/[slug]/+page.svelte:140` — filters to `role === 'required' && user_id !== self`.
- `frontend/src/routes/event-types/[slug]/+page.svelte:374-388` — save writes rotation-only for round_robin, required/optional for collective, and exactly `[{owner, required}]` for fixed.
- `internal/handler/event_type_transfer.go:35` — server requires `h.role='required'`.
## Required outcome
- Align the eligibility rule with the host model: a `rotation` host is a valid booking taker (`booking_handler.go:243-264`), so requiring `required` is stricter than "can host".
- Do not render an inert transfer panel; either list eligible hosts or explain why none qualify.
## Open questions for the human
- Is `required` deliberate, or should any active host qualify?
- For fixed events, should the target be added as a required host as part of the transfer?| _, err = tx.ExecContext(r.Context(), `UPDATE availability_rules SET user_id=? WHERE event_type_id=? AND user_id=?`, body.NewOwnerID, id, body.ExpectedOwnerID) | ||
| if err != nil { | ||
| h.writeError(w, http.StatusConflict, "event availability conflicts with new owner rules") | ||
| return |
There was a problem hiding this comment.
This rewrite can violate availability_rules' UNIQUE (user_id, event_type_id, day_of_week, start_time, end_time) when the target already holds a byte-identical event-scoped rule, aborting the whole transfer; nothing in the shipped UI creates such rules, but POST /v1/availability-rules lets any authenticated user attach one to any event, so it is reachable via the API. The error branch also reports every failure as a 409 "conflict", masking a transient/DB error as user-fixable. Classify with db.IsUniqueViolation (as availability.go:50 does) and decide whether duplicate rows should be skipped or the old owner's rows cleared instead.
| <div class="mt-6 space-y-3 border-t pt-5"> | ||
| <Label for="transfer-owner">Transfer ownership</Label> | ||
| <p class="text-sm text-muted-foreground">Choose a saved required host. The booking URL stays the same. Transfer is available only when there are no upcoming bookings. Calendar connections and global availability stay with each account.</p> | ||
| <select id="transfer-owner" bind:value={transferOwner} class="h-9 w-full rounded-md border bg-background px-3 text-sm"> |
There was a problem hiding this comment.
This hand-rolls a native <select> while the rest of the page (and the admin app generally) builds from the shadcn-svelte Select — CLAUDE.md asks that such deviations be flagged rather than silent. Consider a ConfirmDialog too, since transferring ownership is a destructive action. If native is intentional here, please note why.

I wanted to move an event to another account without breaking its public link. At the moment that means recreating the event or editing the database.
This adds a transfer control in the Hosts tab and an admin-only endpoint. The current owner must be an admin, and the new owner must already be an active required host. Transfers are rejected while the event has upcoming bookings. A current-owner check also rejects stale requests.
The event ID, slug and booking history stay in place. Event-specific availability moves with the event. Calendar connections and global availability stay with each account, so the new owner needs their own calendar setup. The host list is kept as configured.
Validation:
make build,go test ./...,pnpm checkandpnpm test:visualpass. The handler tests cover a valid transfer, stale ownership, an ineligible host and upcoming bookings. Screenshots below use local demo accounts.I also checked a transfer through the local admin UI. The old owner keeps host access, while the event leaves their owned-event controls.
Before
After