From 59011ad0c737a17b6ed09b3c3d66863c26b6bdd9 Mon Sep 17 00:00:00 2001 From: KitKat31337 <4126578+KitKat31337@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:57:53 -0700 Subject: [PATCH] feat: add explicit public directory display order --- docs/ARCHITECTURE.md | 8 + frontend/src/lib/api.ts | 2 + .../routes/event-types/[slug]/+page.svelte | 9 + internal/db/directory_order_test.go | 42 ++++ .../00070_event_type_display_order.sql | 7 + internal/handler/directory.go | 2 +- internal/handler/directory_order_test.go | 185 ++++++++++++++++++ internal/handler/event_type.go | 16 +- internal/handler/event_type_duplicate.go | 4 +- internal/handler/event_type_duplicate_test.go | 1 + 10 files changed, 268 insertions(+), 8 deletions(-) create mode 100644 internal/db/directory_order_test.go create mode 100644 internal/db/migrations/00070_event_type_display_order.sql create mode 100644 internal/handler/directory_order_test.go diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8baf33cc..899b14ef 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -69,6 +69,14 @@ app you must `pnpm build` in `frontend/` **and** rebuild/restart the Go binary ## 4. Persistence (SQLite) — and the single-connection rule +Public person (`/u/{handle}`) and team (`/team/{slug}`) directories list public, +active event types by `display_order` ascending, then name and unique slug for +deterministic ties. The integer defaults to 0, preserving alphabetical order for +existing types. Owners can set it through event-type create/PATCH or the event +editor's Directory order field; negative values put an event ahead of the default +group. It applies wherever the event is listed and is retained on duplication. +It does not affect the admin event-type list or team member roster ordering. + - `internal/db`: opens SQLite with **`SetMaxOpenConns(1)`** + `SetMaxIdleConns(1)`, **WAL** journal mode, `busy_timeout=5000`. One writer connection by design. - Migrations: **goose** SQL files in `internal/db/migrations/` (00001→00029). Run diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 4f2172c3..5a121d77 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -22,6 +22,8 @@ export type User = { }; export type EventType = { + /** Lower first on public person/team pages; ties sort by name, then slug. */ + display_order: number; allow_phone_call: boolean; id: string; slug: string; diff --git a/frontend/src/routes/event-types/[slug]/+page.svelte b/frontend/src/routes/event-types/[slug]/+page.svelte index 8243ff7c..234360dc 100644 --- a/frontend/src/routes/event-types/[slug]/+page.svelte +++ b/frontend/src/routes/event-types/[slug]/+page.svelte @@ -68,6 +68,7 @@ const isOnlineMeeting = (t: string) => t === 'google_meet' || t === 'teams'; let form = $state({ + display_order: 0, name: '', slug: '', description: '', duration_minutes: 30, slot_interval_minutes: 30, is_active: true, is_public: true, show_taken_slots: false, invite_delivery: 'calendar' as EventType['invite_delivery'], @@ -283,6 +284,7 @@ et = await api.get(`/v1/event-types/${slug}`); allowPhoneCall = et.allow_phone_call; form = { + display_order: et.display_order, name: et.name, slug: et.slug, description: et.description ?? '', @@ -326,6 +328,7 @@ } async function saveET() { + if (!Number.isSafeInteger(form.display_order)) { toast.error('Directory order must be a whole number.'); return; } if (!form.name.trim()) { toast.error('Name is required.'); return; } if (form.duration_minutes < 5) { toast.error('Duration must be at least 5 minutes.'); return; } // Matches the API, which only requires a positive value. A stricter floor here would @@ -342,6 +345,7 @@ etSaving = true; try { const updated = await api.patch(`/v1/event-types/${slug}`, { + display_order: form.display_order, slug: form.slug.trim(), name: form.name.trim(), description: form.description.trim() || null, @@ -588,6 +592,11 @@ +
+ + +

Lower numbers appear first on public person and team pages. Equal numbers sort by name. Applies wherever this event is listed.

+
diff --git a/internal/db/directory_order_test.go b/internal/db/directory_order_test.go new file mode 100644 index 00000000..fb8b7b0a --- /dev/null +++ b/internal/db/directory_order_test.go @@ -0,0 +1,42 @@ +package db_test + +import ( + "os" + "testing" + + "github.com/calnode/calnode/internal/db" + "github.com/pressly/goose/v3" +) + +func TestMigrate_displayOrderPreservesExistingEvents(t *testing.T) { + database, err := db.Open("sqlite://:memory:") + if err != nil { + t.Fatal(err) + } + defer database.Close() + // Start from the schema immediately before display_order was introduced. + goose.SetBaseFS(os.DirFS(".")) + if err := goose.SetDialect("sqlite3"); err != nil { + t.Fatal(err) + } + if err := goose.UpTo(database, "migrations", 69); err != nil { + t.Fatal(err) + } + if _, err := database.Exec(`INSERT INTO users (id,email,name) VALUES ('owner','owner@example.com','Owner')`); err != nil { + t.Fatal(err) + } + if _, err := database.Exec(`INSERT INTO event_types (id,user_id,slug,name,duration_minutes) VALUES ('event','owner','existing','Existing event',30)`); err != nil { + t.Fatal(err) + } + if err := db.Migrate(database); err != nil { + t.Fatal(err) + } + var name, slug string + var order int + if err := database.QueryRow(`SELECT name,slug,display_order FROM event_types WHERE id = 'event'`).Scan(&name, &slug, &order); err != nil { + t.Fatal(err) + } + if name != "Existing event" || slug != "existing" || order != 0 { + t.Fatalf("existing event changed: name=%q slug=%q order=%d", name, slug, order) + } +} diff --git a/internal/db/migrations/00070_event_type_display_order.sql b/internal/db/migrations/00070_event_type_display_order.sql new file mode 100644 index 00000000..cbd03dc3 --- /dev/null +++ b/internal/db/migrations/00070_event_type_display_order.sql @@ -0,0 +1,7 @@ +-- +goose Up +-- Public directory order is independent of names and shared booking URLs. +-- Equal values retain alphabetical ordering; existing event types all start at 0. +ALTER TABLE event_types ADD COLUMN display_order INTEGER NOT NULL DEFAULT 0; + +-- +goose Down +-- Leave the additive column in place, consistent with other column migrations. diff --git a/internal/handler/directory.go b/internal/handler/directory.go index cb76df94..df89ed17 100644 --- a/internal/handler/directory.go +++ b/internal/handler/directory.go @@ -58,7 +58,7 @@ func (h *Handler) directoryItems(ctx context.Context, loc *i18n.Locale, where st rows, err := h.db.QueryContext(ctx, ` SELECT slug, name, duration_minutes, location_type, COALESCE(location_value,''), price_cents, currency - FROM event_types et WHERE `+where+` ORDER BY name`, args...) // #nosec G202 -- where is one of two literals at the call sites; every value is bound via args... + FROM event_types et WHERE `+where+` ORDER BY et.display_order, et.name, et.slug`, args...) // #nosec G202 -- where is one of two literals at the call sites; every value is bound via args... if err != nil { return nil, err } diff --git a/internal/handler/directory_order_test.go b/internal/handler/directory_order_test.go new file mode 100644 index 00000000..d053c29b --- /dev/null +++ b/internal/handler/directory_order_test.go @@ -0,0 +1,185 @@ +package handler_test + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/calnode/calnode/internal/handler" +) + +func patchDirectoryOrder(t *testing.T, h *handler.Handler, key, slug, body string) *httptest.ResponseRecorder { + t.Helper() + req := authReq(http.MethodPatch, "/v1/event-types/"+slug, body, key) + req.SetPathValue("slug", slug) + rec := httptest.NewRecorder() + h.RequireAuth(h.PatchEventType)(rec, req) + return rec +} + +func assertDisplayOrder(t *testing.T, rec *httptest.ResponseRecorder, status, want int) { + t.Helper() + if rec.Code != status { + t.Fatalf("status = %d; want %d: %s", rec.Code, status, rec.Body.String()) + } + var resp struct { + DisplayOrder *int `json:"display_order"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatal(err) + } + if resp.DisplayOrder == nil || *resp.DisplayOrder != want { + t.Fatalf("display_order missing or wrong; want %d: %s", want, rec.Body.String()) + } +} + +func TestEventType_displayOrder(t *testing.T) { + h, database, key, _ := setupWorkspaceWithDB(t) + for i, value := range []string{"", `,"display_order":-10`, `,"display_order":20`} { + slug := fmt.Sprintf("ordered-%d", i) + req := authReq(http.MethodPost, "/v1/event-types", fmt.Sprintf(`{"slug":%q,"name":"Meeting","duration_minutes":30%s}`, slug, value), key) + rec := httptest.NewRecorder() + h.RequireAuth(h.CreateEventType)(rec, req) + assertDisplayOrder(t, rec, http.StatusCreated, []int{0, -10, 20}[i]) + } + + const slug = "ordered-1" + for _, tc := range []struct { + body string + want int + }{ + {`{"name":"Renamed"}`, -10}, // omitted fields survive partial updates + {`{"display_order":null}`, -10}, + {`{"display_order":7}`, 7}, + {`{"display_order":0}`, 0}, // explicit zero resets the order + {`{"display_order":-5}`, -5}, + } { + assertDisplayOrder(t, patchDirectoryOrder(t, h, key, slug, tc.body), http.StatusOK, tc.want) + } + for _, body := range []string{`{"display_order":1.5}`, `{"display_order":"1"}`, `{"display_order":9223372036854775808}`} { + t.Run(body, func(t *testing.T) { + if rec := patchDirectoryOrder(t, h, key, slug, body); rec.Code != http.StatusBadRequest { + t.Fatalf("invalid order: status = %d: %s", rec.Code, rec.Body.String()) + } + req := authReq(http.MethodPost, "/v1/event-types", strings.Replace(body, "{", `{"slug":"invalid-order","name":"Meeting","duration_minutes":30,`, 1), key) + rec := httptest.NewRecorder() + h.RequireAuth(h.CreateEventType)(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("invalid create order: status = %d: %s", rec.Code, rec.Body.String()) + } + }) + } + seedMember(t, database, "order-member", "order-member@example.com") + const memberKey = "directory-order-member-key" + mustExec(t, database, `INSERT INTO api_keys (id,user_id,name,key_hash) VALUES ('order-key','order-member','test',?)`, sha256HexForTest(memberKey)) + mustExec(t, database, `INSERT INTO event_type_hosts (id,event_type_id,user_id,role) SELECT 'order-host',id,'order-member','required' FROM event_types WHERE slug = ?`, slug) + if rec := patchDirectoryOrder(t, h, memberKey, slug, `{"display_order":100}`); rec.Code != http.StatusNotFound { + t.Fatalf("host can change order: status = %d", rec.Code) + } + // GET/list use the same scan contract as create/PATCH; check both explicitly. + req := authReq(http.MethodGet, "/v1/event-types/"+slug, "", key) + req.SetPathValue("slug", slug) + rec := httptest.NewRecorder() + h.RequireAuth(h.GetEventType)(rec, req) + assertDisplayOrder(t, rec, http.StatusOK, -5) + rec = httptest.NewRecorder() + h.RequireAuth(h.ListEventTypes)(rec, authReq(http.MethodGet, "/v1/event-types", "", key)) + var list struct { + Items []struct { + Slug string `json:"slug"` + DisplayOrder *int `json:"display_order"` + } `json:"items"` + } + if rec.Code != http.StatusOK { + t.Fatalf("list: %d: %s", rec.Code, rec.Body.String()) + } + if err := json.Unmarshal(rec.Body.Bytes(), &list); err != nil { + t.Fatal(err) + } + found := false + for _, item := range list.Items { + if item.Slug == slug { + found = true + if item.DisplayOrder == nil || *item.DisplayOrder != -5 { + t.Fatalf("list lost display_order: %s", rec.Body.String()) + } + } + } + if !found { + t.Fatal("list missing event") + } + assertDisplayOrder(t, duplicate(t, h, key, slug), http.StatusCreated, -5) +} + +func TestDirectories_displayOrder(t *testing.T) { + h, database, key, ownerID := setupWorkspaceWithDB(t) + mustExec(t, database, `UPDATE users SET handle = 'ordered-host' WHERE id = ?`, ownerID) + seedMember(t, database, "other-owner", "other-owner@example.com") + mustExec(t, database, `INSERT INTO teams (id,name,slug) VALUES ('order-team','Order team','order-team')`) + // Insert in a different order from the desired output. Slugs deliberately + // disagree with names, so sorting solely by slug cannot pass this test. + for _, item := range []struct { + slug, name string + order int + }{ + {"a-last", "Alpha", 10}, + {"z-first", "Zulu", -10}, + {"b-beta", "Beta", 0}, + {"d-alpha", "Alpha", 0}, + {"c-alpha", "Alpha", 0}, + } { + mustExec(t, database, `INSERT INTO event_types (id,user_id,team_id,slug,name,duration_minutes,display_order) VALUES (?,?,'order-team',?,?,30,?)`, item.slug, ownerID, item.slug, item.name, item.order) + } + // A hosted event participates in the same ordering as owned events. + mustExec(t, database, `UPDATE event_types SET user_id = 'other-owner' WHERE slug = 'b-beta'`) + mustExec(t, database, `INSERT INTO event_type_hosts (id,event_type_id,user_id,role) VALUES ('order-host','b-beta',?,'required')`, ownerID) + for _, slug := range []string{"hidden", "inactive", "unrelated"} { + mustExec(t, database, `INSERT INTO event_types (id,user_id,team_id,slug,name,duration_minutes,display_order) VALUES (?,?,'order-team',?, ?,30,-100)`, slug, ownerID, slug, slug) + } + mustExec(t, database, `UPDATE event_types SET is_public = 0 WHERE slug = 'hidden'`) + mustExec(t, database, `UPDATE event_types SET is_active = 0 WHERE slug = 'inactive'`) + mustExec(t, database, `UPDATE event_types SET user_id = 'other-owner', team_id = NULL WHERE slug = 'unrelated'`) + checkPages := func(want []string) { + t.Helper() + for _, page := range []string{"person", "team"} { + t.Run(page+strings.Join(want, ","), func(t *testing.T) { + var code int + var body string + if page == "person" { + code, body = personPage(t, h, "ordered-host") + } else { + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/team/order-team", nil) + req.SetPathValue("slug", "order-team") + h.TeamPage(rec, req) + code, body = rec.Code, rec.Body.String() + } + if code != http.StatusOK { + t.Fatalf("page: %d: %s", code, body) + } + previous := -1 + for _, slug := range want { + index := strings.Index(body, `href="/book/`+slug+`"`) + if index <= previous { + t.Fatalf("missing or out-of-order %s; want %v", slug, want) + } + previous = index + } + for _, slug := range []string{"hidden", "inactive", "unrelated"} { + if strings.Contains(body, `href="/book/`+slug+`"`) { + t.Errorf("page contains %s", slug) + } + } + }) + } + } + checkPages([]string{"z-first", "c-alpha", "d-alpha", "b-beta", "a-last"}) + assertDisplayOrder(t, patchDirectoryOrder(t, h, key, "a-last", `{"display_order":-20}`), http.StatusOK, -20) + checkPages([]string{"a-last", "z-first", "c-alpha", "d-alpha", "b-beta"}) + // All-zero defaults retain alphabetical ordering with deterministic slug ties. + mustExec(t, database, `UPDATE event_types SET display_order = 0`) + checkPages([]string{"a-last", "c-alpha", "d-alpha", "b-beta", "z-first"}) +} diff --git a/internal/handler/event_type.go b/internal/handler/event_type.go index c0bf93ba..3beae545 100644 --- a/internal/handler/event_type.go +++ b/internal/handler/event_type.go @@ -24,6 +24,7 @@ type eventTypeJSON struct { ID string `json:"id"` Slug string `json:"slug"` Name string `json:"name"` + DisplayOrder int `json:"display_order"` // lower first on public directories; ties sort by name, then slug Description *string `json:"description"` DurationMinutes int `json:"duration_minutes"` SlotIntervalMinutes int `json:"slot_interval_minutes"` @@ -105,7 +106,7 @@ func scanEventTypeRow(s rowScanner, trailing ...any) (*eventTypeJSON, error) { &isActive, &isPublic, &showTaken, &et.CreatedAt, &msgConf, &msgCancel, &msgResched, &msgRemind, &msgGreeting, &subjConf, &subjCancel, &subjResched, &subjRemind, - &et.PriceCents, &et.Currency, &et.InviteDelivery, + &et.PriceCents, &et.Currency, &et.InviteDelivery, &et.DisplayOrder, } dests = append(dests, trailing...) err := s.Scan(dests...) @@ -164,7 +165,7 @@ const etColumns = `id, slug, name, description, is_active, is_public, show_taken_slots, created_at, msg_confirmation, msg_cancellation, msg_reschedule, msg_reminder, msg_greeting, subj_confirmation, subj_cancellation, subj_reschedule, subj_reminder, - price_cents, currency, invite_delivery` + price_cents, currency, invite_delivery, display_order` // selectETCols fetches a single owner-scoped event type (no `owned` column). const selectETCols = "SELECT " + etColumns + " FROM event_types" @@ -218,6 +219,7 @@ func (h *Handler) CreateEventType(w http.ResponseWriter, r *http.Request) { var req struct { Slug string `json:"slug"` Name string `json:"name"` + DisplayOrder int `json:"display_order"` Description *string `json:"description"` DurationMinutes int `json:"duration_minutes"` SlotIntervalMinutes *int `json:"slot_interval_minutes"` @@ -335,12 +337,12 @@ func (h *Handler) CreateEventType(w http.ResponseWriter, r *http.Request) { slot_interval_minutes, location_type, location_value, allow_phone_call, routing_mode, buffer_before_minutes, buffer_after_minutes, min_notice_minutes, max_future_days, max_active_bookings, show_taken_slots, invite_delivery, - msg_confirmation, msg_cancellation, msg_reschedule, msg_reminder) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + msg_confirmation, msg_cancellation, msg_reschedule, msg_reminder, display_order) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, id, user.ID, req.Slug, req.Name, req.Description, req.DurationMinutes, slotInterval, locType, req.LocationValue, req.AllowPhoneCall != nil && *req.AllowPhoneCall, routingMode, bufBefore, bufAfter, minNotice, maxFuture, maxActive, showTaken, inviteDelivery, - defaultMsgConfirmation, defaultMsgCancellation, defaultMsgReschedule, defaultMsgReminder) + defaultMsgConfirmation, defaultMsgCancellation, defaultMsgReschedule, defaultMsgReminder, req.DisplayOrder) if err != nil { if db.IsUniqueViolation(err) { h.writeError(w, http.StatusConflict, "slug already in use") @@ -457,6 +459,7 @@ func (h *Handler) PatchEventType(w http.ResponseWriter, r *http.Request) { var req struct { Slug *string `json:"slug"` Name *string `json:"name"` + DisplayOrder *int `json:"display_order"` Description *string `json:"description"` DurationMinutes *int `json:"duration_minutes"` SlotIntervalMinutes *int `json:"slot_interval_minutes"` @@ -524,6 +527,9 @@ func (h *Handler) PatchEventType(w http.ResponseWriter, r *http.Request) { if req.Description != nil { set("description", *req.Description) } + if req.DisplayOrder != nil { + set("display_order", *req.DisplayOrder) + } if req.DurationMinutes != nil { if *req.DurationMinutes <= 0 { h.writeError(w, http.StatusBadRequest, "duration_minutes must be positive") diff --git a/internal/handler/event_type_duplicate.go b/internal/handler/event_type_duplicate.go index e56a5d97..d3e3b8fc 100644 --- a/internal/handler/event_type_duplicate.go +++ b/internal/handler/event_type_duplicate.go @@ -141,7 +141,7 @@ func (h *Handler) DuplicateEventType(w http.ResponseWriter, r *http.Request) { is_active, is_public, show_taken_slots, archived_at, msg_confirmation, msg_cancellation, msg_reschedule, msg_reminder, msg_greeting, subj_confirmation, subj_cancellation, subj_reschedule, subj_reminder, - price_cents, currency, invite_delivery) + price_cents, currency, invite_delivery, display_order) SELECT ?, user_id, team_id, ?, name, description, duration_minutes, slot_interval_minutes, @@ -152,7 +152,7 @@ func (h *Handler) DuplicateEventType(w http.ResponseWriter, r *http.Request) { 0, is_public, show_taken_slots, NULL, msg_confirmation, msg_cancellation, msg_reschedule, msg_reminder, msg_greeting, subj_confirmation, subj_cancellation, subj_reschedule, subj_reminder, - price_cents, currency, invite_delivery + price_cents, currency, invite_delivery, display_order FROM event_types WHERE id = ?`, newID, newSlug, srcID); err != nil { h.logger.ErrorContext(r.Context(), "duplicate event type: copy row", "error", err) h.writeError(w, http.StatusInternalServerError, "internal error") diff --git a/internal/handler/event_type_duplicate_test.go b/internal/handler/event_type_duplicate_test.go index 44ae4bbb..581aef3d 100644 --- a/internal/handler/event_type_duplicate_test.go +++ b/internal/handler/event_type_duplicate_test.go @@ -605,6 +605,7 @@ func TestDuplicateEventType_handlesEveryEventTypeColumn(t *testing.T) { "subj_reminder": true, "price_cents": true, "currency": true, "invite_delivery": true, + "display_order": true, } // Deliberately not inherited, with the reason. notInherited := map[string]string{