Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ the platform/recovery secret doesn't expose secrets.

## 7. Routing — the host-roles model

An administrator who owns an event can transfer it to an active required host with `POST /v1/event-types/{slug}/transfer`. The request supplies `expected_owner_id` and `new_owner_id`. Upcoming bookings prevent transfer. The event ID, slug, and historical booking hosts are preserved. Event-specific availability rules move to the new owner; global availability and calendar connections do not.

An event type owns a **host list** (`event_type_hosts`): each row = (user, role,
priority), role ∈ **required | rotation | optional**. The editor authors these
roles through **two plain questions** rather than a mode picker — *who can host?*
Expand Down
33 changes: 32 additions & 1 deletion frontend/src/routes/event-types/[slug]/+page.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,23 @@
let rotationHosts = $state<Host[]>([]);
let togetherHosts = $state<TogetherHost[]>([]);
let hostsLoaded = $state(false);
let transferOwner = $state('');
let transferring = $state(false);
let transferHosts = $state<EventTypeHost[]>([]);
async function transferEvent() {
if (!transferOwner || !$currentUser) return;
transferring = true;
try {
await api.post(`/v1/event-types/${slug}/transfer`, { expected_owner_id: $currentUser.id, new_owner_id: transferOwner });
toast.success('Event transferred');
await goto(`${base}/event-types`);
} catch (error) {
toast.error(error instanceof Error ? error.message : 'Could not transfer event');
} finally {
transferring = false;
}
}

let members = $state<TeamMember[]>([]);
let teams = $state<Team[]>([]);

Expand All @@ -120,6 +137,7 @@
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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

const toHost = (h: EventTypeHost): Host => ({ user_id: h.user_id, name: h.name, email: h.email });
rotationHosts = items.filter((h) => h.role === 'rotation').map(toHost);
togetherHosts = items
Expand Down Expand Up @@ -377,6 +395,7 @@
return;
}
await loadET();
await loadHosts();
} catch (e: any) {
toast.error(e.message || 'Could not save changes');
} finally {
Expand Down Expand Up @@ -433,8 +452,8 @@
// Connected calendar — best-effort; drives the meeting-link hint only.
api.get<CalendarStatus>('/v1/calendar/status').then((s) => (calStatus = s)).catch(() => {});
api.get<ZoomStatus>('/v1/zoom/status').then((s) => (zoomStatus = s)).catch(() => {});
await loadHosts();
if (hostScope === 'people') {
await loadHosts();
loadMembers();
loadTeams();
}
Expand Down Expand Up @@ -1057,6 +1076,18 @@
</div>
{/if}

{#if activeTab === 'hosts' && $currentUser?.is_admin}
<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">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<option value="">Select new owner</option>
{#each transferHosts as host}<option value={host.user_id}>{host.name || host.email}</option>{/each}
</select>
<Button variant="outline" disabled={!transferOwner || transferring} onclick={transferEvent}>{transferring ? 'Transferring…' : 'Transfer ownership'}</Button>
</div>
{/if}

{#if activeTab === 'questions'}
<QuestionsPanel slug={slug ?? ''} />
{/if}
Expand Down
70 changes: 70 additions & 0 deletions internal/handler/event_type_transfer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package handler

import (
"encoding/json"
"net/http"
)

func (h *Handler) TransferEventType(w http.ResponseWriter, r *http.Request) {
user, ok := h.requireAdmin(w, r)
if !ok {
return
}
r.Body = http.MaxBytesReader(w, r.Body, 4096)
var body struct {
ExpectedOwnerID string `json:"expected_owner_id"`
NewOwnerID string `json:"new_owner_id"`
}
if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body.ExpectedOwnerID != user.ID || body.NewOwnerID == "" || body.NewOwnerID == user.ID {
h.writeError(w, http.StatusBadRequest, "expected current owner and a different new owner are required")
return
}
tx, err := h.db.BeginTx(r.Context(), nil)
if err != nil {
h.writeError(w, http.StatusInternalServerError, "internal error")
return
}
defer tx.Rollback()
var id string
err = tx.QueryRowContext(r.Context(), `SELECT id FROM event_types WHERE slug = ? AND user_id = ? AND archived_at IS NULL`, r.PathValue("slug"), body.ExpectedOwnerID).Scan(&id)
if err != nil {
h.writeError(w, http.StatusConflict, "event ownership changed or event not found")
return
}
var eligible int
err = tx.QueryRowContext(r.Context(), `SELECT COUNT(*) FROM event_type_hosts h JOIN users u ON u.id=h.user_id WHERE h.event_type_id=? AND h.user_id=? AND h.role='required' AND u.archived_at IS NULL`, id, body.NewOwnerID).Scan(&eligible)
if err != nil {
h.writeError(w, http.StatusInternalServerError, "internal error")
return
}
if eligible != 1 {
h.writeError(w, http.StatusConflict, "new owner must be an active required host")
return
}
var active int
err = tx.QueryRowContext(r.Context(), `SELECT COUNT(*) FROM bookings WHERE event_type_id=? AND status!='cancelled' AND end_at > strftime('%Y-%m-%dT%H:%M:%SZ','now')`, id).Scan(&active)
if err != nil {
h.writeError(w, http.StatusInternalServerError, "internal error")
return
}
if active > 0 {
h.writeError(w, http.StatusConflict, "transfer requires no upcoming bookings")
return
}
_, 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
Comment on lines +54 to +57

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
_, err = tx.ExecContext(r.Context(), `UPDATE event_types SET user_id=? WHERE id=? AND user_id=?`, body.NewOwnerID, id, body.ExpectedOwnerID)
if err != nil {
h.writeError(w, http.StatusInternalServerError, "internal error")
return
}
if err = tx.Commit(); err != nil {
h.writeError(w, http.StatusInternalServerError, "internal error")
return
}
h.logger.InfoContext(r.Context(), "event ownership transferred", "event_type_id", id, "previous_owner", user.ID, "new_owner", body.NewOwnerID)
h.writeJSON(w, http.StatusOK, map[string]any{"id": id, "slug": r.PathValue("slug"), "owner_id": body.NewOwnerID})
}
63 changes: 63 additions & 0 deletions internal/handler/event_type_transfer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package handler_test

import (
"fmt"
"net/http"
"net/http/httptest"
"testing"
)

func TestTransferEventTypePreservesIdentityAndRequiresCurrentOwner(t *testing.T) {
h, database, key, owner := setupWorkspaceWithDB(t)
slug, id := seedEventTypeHTTP(t, h, key)
_, err := database.Exec(`INSERT INTO users (id,email,name,iana_timezone,is_admin) VALUES ('new-owner','new@example.com','New','Europe/Amsterdam',0)`)
if err != nil {
t.Fatal(err)
}
transfer := func() *httptest.ResponseRecorder {
req := authReq(http.MethodPost, "/v1/event-types/"+slug+"/transfer", fmt.Sprintf(`{"expected_owner_id":%q,"new_owner_id":"new-owner"}`, owner), key)
req.SetPathValue("slug", slug)
rec := httptest.NewRecorder()
h.RequireAuth(h.TransferEventType)(rec, req)
return rec
}
if rec := transfer(); rec.Code != http.StatusConflict {
t.Fatalf("unassigned host transfer: %d %s", rec.Code, rec.Body)
}
if rec := putHosts(t, h, slug, key, `{"hosts":[{"user_id":"new-owner","role":"required","priority":0}]}`); rec.Code != http.StatusOK {
t.Fatalf("assign: %d", rec.Code)
}
_, err = database.Exec(`INSERT INTO availability_rules (id,user_id,event_type_id,day_of_week,start_time,end_time) VALUES ('rule',?,?,1,'09:00','20:00')`, owner, id)
if err != nil {
t.Fatal(err)
}
_, err = database.Exec(`INSERT INTO bookings (id,event_type_id,host_id,start_at,end_at,status) VALUES ('upcoming',?,?,'2099-01-01T09:00:00Z','2099-01-01T09:15:00Z','confirmed')`, id, owner)
if err != nil {
t.Fatal(err)
}
if rec := transfer(); rec.Code != http.StatusConflict {
t.Fatalf("upcoming booking: %d", rec.Code)
}
if _, err = database.Exec(`UPDATE bookings SET status='cancelled' WHERE id='upcoming'`); err != nil {
t.Fatal(err)
}
if rec := transfer(); rec.Code != http.StatusOK {
t.Fatalf("transfer: %d %s", rec.Code, rec.Body)
}
var gotID, gotOwner, ruleOwner string
if err := database.QueryRow(`SELECT id,user_id FROM event_types WHERE slug=?`, slug).Scan(&gotID, &gotOwner); err != nil {
t.Fatal(err)
}
if gotID != id || gotOwner != "new-owner" {
t.Fatalf("identity: %s %s", gotID, gotOwner)
}
if err := database.QueryRow(`SELECT user_id FROM availability_rules WHERE id='rule'`).Scan(&ruleOwner); err != nil {
t.Fatal(err)
}
if ruleOwner != "new-owner" {
t.Fatalf("rule owner: %s", ruleOwner)
}
if rec := transfer(); rec.Code != http.StatusConflict {
t.Fatalf("stale transfer: %d", rec.Code)
}
}
1 change: 1 addition & 0 deletions internal/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ func New(ctx context.Context, cfg *config.Config, db *sql.DB, logger *slog.Logge
mux.HandleFunc("PATCH /v1/event-types/{slug}", h.RequireAuth(h.PatchEventType))
mux.HandleFunc("DELETE /v1/event-types/{slug}", h.RequireAuth(h.DeleteEventType))
mux.HandleFunc("POST /v1/event-types/{slug}/duplicate", h.RequireAuth(h.DuplicateEventType))
mux.HandleFunc("POST /v1/event-types/{slug}/transfer", h.RequireAuth(h.TransferEventType))
mux.HandleFunc("GET /v1/event-types/{slug}/hosts", h.RequireAuth(h.ListEventTypeHosts))
mux.HandleFunc("PUT /v1/event-types/{slug}/hosts", h.RequireAuth(h.SetEventTypeHosts))
testEmailRL := RateLimit(10, time.Minute)
Expand Down
Loading