diff --git a/CHANGELOG.md b/CHANGELOG.md index 1445c11..cc199d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,23 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad ## [Unreleased] +### Added +- **Empty days and minimum-notice gaps now explain themselves** on all three booking + surfaces (booking page, manage/reschedule page, embed widget). Closes + [#20](https://github.com/Calnode/calnode/issues/20). + + A day with nothing on it names the day, and the host when the event type has exactly + one, instead of the bare "No available times." that never said whether another day would + help. And when `min_notice_minutes` is what removed the nearest starts, the surfaces say + so rather than leaving the visitor to guess - the most common "why can't I see those + times". + + The engine decides that, not the front ends: `GET /slots` gains + `min_notice: {minutes, dates}` listing the booker-local days the policy actually cost + something. A start that is simply in the past, one a booking took away, and one no host + pool could satisfy are all excluded, so the explanation never appears attached to the + wrong cause. Three new/changed keys in all eight locales. + ## [0.8.0] - 2026-09-03 ### Added diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e81e86c..f9659d1 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -319,13 +319,56 @@ in discussion #14, issue #19. - **`taken` is absent, not empty, when off** - a client must distinguish "does not show taken times" from "opted in, nothing booked today". - **Client side:** `mergeDaySlots` and `bookableDayKeys` in the shared - `assets/booking-logic.js`, so all three surfaces share one implementation. Free and + `assets/booking-logic.js`, inlined into book.html and manage.html. **The embed widget + does not load that module** - `EmbedJS` serves `embed.js` as standalone bytes, so + `BookingLogic` is undefined inside it and it carries its own copies of the helpers it + needs (a test asserts it never calls into `BookingLogic`). Free and taken stay separate on the wire and are combined only for display. `bookableDayKeys` exists for a specific trap: once taken slots are grouped by day too, a fully booked day still produces a key, and using those keys for the calendar would advertise it as having something available. Fully booked days *are* still openable, deliberately - a list of struck-through times explains itself better than a dead date. +### Explaining an empty day, and the minimum-notice gap + +Two silences on the booker-facing surfaces, issue #20. A day with nothing on it used to +render a bare "No available times.", which does not say whether another day would help; +and `min_notice_minutes` removes the nearest starts with nothing left behind to explain +them - the most common "why can't I see those times". + +- **The empty-day message names the day, and the host when there is one.** + `no_available_times` takes the date; `no_available_times_host` adds the host. The host + is named only when the event type (or, on manage, the booking) has exactly ONE - + `HostsLabel` can be "Alex, Sam & 2 others", which cannot be the subject of that + sentence in any shipped locale. `SoleHostName` on both page data structs is empty + otherwise, and the surfaces fall back to the date-only form. +- **The notice gap is computed server-side, in the engine that already knows it.** + `slots.GenerateDetailed(req, slots.Extras{NoticeGap: true})` reports the starts the + notice rule removed. Collected during the main walk (the cutoff is evaluated there + anyway), not by a second pass like `taken`. +- **Two exclusions keep the attribution honest.** A start already in the past is never + reported - it would have gone with no policy at all, and blaming the policy would put + the message on every event type by dinnertime. And busy intervals are applied on the + way in, so a start a booking took away is never reported either: `NoticeGap` and + `Taken` are therefore disjoint, and no start is ever explained two ways. Routing rules + are applied to the withheld starts too, so a start no host pool could satisfy is not + blamed on the policy. +- **The wire carries days, not times.** `GET /slots` returns + `min_notice: {minutes, dates}`, where `dates` are booker-local `YYYY-MM-DD` keys + matching what the surfaces group slots by. Sending the individual withheld times would + describe the host's working hours at a finer grain than the feature needs. Absent when + the event type sets no minimum notice, present-with-empty-`dates` when it set one that + cost this range nothing - the same distinction `taken` draws. +- **The label is server-rendered.** `MinNoticeLabel` on the book/manage page data and + `min_notice_label` on `GET /public` (for the widget), both via `durationLabel`, because + the `/slots` call the surfaces make carries no `?lang=` - a label derived from its + response would silently ignore a language override. `minutes` still travels for clients + with no label of their own. +- **Where it renders.** On the day itself when that day lost starts, whether or not times + remain (a day showing 2pm onwards but nothing this morning is exactly the case in the + issue), and *also* in the "pick a day" state, because a day the policy emptied + completely is greyed out in the calendar and cannot be clicked for an explanation. + --- ## 9. Booking lifecycle diff --git a/internal/handler/assets/booking-logic.js b/internal/handler/assets/booking-logic.js index 0358b30..a9b7bc8 100644 --- a/internal/handler/assets/booking-logic.js +++ b/internal/handler/assets/booking-logic.js @@ -1,8 +1,14 @@ -// booking-logic.js — the PURE date/slot/format logic shared by the THREE booking surfaces -// (book.html, manage.html, embed.js), so a change is made once instead of three times. No DOM. -// Served inlined into the book/manage Go templates and prepended to embed.js (so `BookingLogic` -// is a page global), and require()-able by the node tests (booking-logic.test.js). Same UMD -// pattern as room-logic.js — no build step, stays framework-free. +// booking-logic.js — the PURE date/slot/format logic shared by book.html and manage.html, so a +// change is made once instead of twice. No DOM. +// Served inlined into the book/manage Go templates, and require()-able by the node tests +// (booking-logic.test.js). Same UMD pattern as room-logic.js — no build step, stays +// framework-free. +// +// NOT loaded by embed.js. The widget is served as its own standalone file +// (internal/handler/embed_handler.go serves the embedded bytes unmodified), so `BookingLogic` +// is undefined inside it and it carries its own copies of the few helpers it needs — see the +// comments on its dowLabels and fmt. Anything added here that all three surfaces need has to be +// mirrored there deliberately. (function (root, factory) { if (typeof module === 'object' && module.exports) module.exports = factory(); else root.BookingLogic = factory(); @@ -113,6 +119,29 @@ function addMonths(d, n) { return new Date(d.getFullYear(), d.getMonth() + n, 1); } function daysInMonth(year, month) { return new Date(year, month + 1, 0).getDate(); } + // fmt — argument substitution for the translated strings the booking surfaces render + // themselves, so the three of them don't each grow their own. Supports exactly the two + // forms the locale files use for these keys: plain %s, taken in order, and the indexed + // %[n]s that lets a translation reorder its arguments ("%[1]s has no available times on + // %[2]s" is date-first in several languages). Server-side, Go's fmt does this job; this + // is the client half of the same contract. + // + // Deliberately not a printf. Accepting %d without implementing number formatting would + // be worse than not claiming to: the keys these surfaces substitute carry %s only, and + // internal/i18n's verb-parity test holds every locale to English's verbs. + // + // A missing argument renders as an empty string rather than leaving "%s" on screen — + // visibly wrong copy beats a literal format verb in front of a customer. + function fmt(template, args) { + var list = args || []; + var next = 0; + return String(template).replace(/%(?:\[(\d+)\])?s/g, function (_match, index) { + var pick = index ? Number(index) - 1 : next++; + var value = list[pick]; + return value === undefined || value === null ? '' : String(value); + }); + } + // NOTE: there is deliberately no host-label helper here. Each surface builds its own // (hostsLabel in book.go for the server-rendered page, in book.html's script for the // post-slot-pick rewrite, and in embed.js), because the label needs the resolved locale's @@ -128,6 +157,7 @@ groupSlotsByDay: groupSlotsByDay, mergeDaySlots: mergeDaySlots, bookableDayKeys: bookableDayKeys, + fmt: fmt, formatTime: formatTime, formatDay: formatDay, dowIndex: dowIndex, diff --git a/internal/handler/assets/booking-logic.test.js b/internal/handler/assets/booking-logic.test.js index 71ffda2..6710f4d 100644 --- a/internal/handler/assets/booking-logic.test.js +++ b/internal/handler/assets/booking-logic.test.js @@ -87,3 +87,30 @@ test('bookableDayKeys omits a day whose slots are all taken', () => { const freeByDay = { '2026-06-15': [{ start: 'x' }], '2026-06-16': [] }; assert.deepEqual(B.bookableDayKeys(freeByDay).sort(), ['2026-06-15']); }); + +test('fmt substitutes %s in order', () => { + assert.equal(B.fmt('No available times on %s.', ['Monday, 15 June']), 'No available times on Monday, 15 June.'); + assert.equal(B.fmt('%s has no available times on %s.', ['Alex', 'Monday']), 'Alex has no available times on Monday.'); + assert.equal(B.fmt('Bookings must be made at least %s in advance.', ['4 hours']), + 'Bookings must be made at least 4 hours in advance.'); +}); + +test('fmt honours indexed %[n]s, so a translation can reorder its arguments', () => { + // German and Swedish put the date before the verb; the locale files are allowed to + // reorder as long as the verbs match English (internal/i18n's parity test). + assert.equal(B.fmt('%[2]s: %[1]s hat keine Termine.', ['Alex', 'Montag']), 'Montag: Alex hat keine Termine.'); + // An index may repeat an argument, and mixing forms keeps the sequential counter + // independent of the indexed reads. + assert.equal(B.fmt('%[1]s / %[1]s / %s', ['a', 'b']), 'a / a / a'); +}); + +test('fmt leaves no format verb on screen when an argument is missing', () => { + assert.equal(B.fmt('No available times on %s.', []), 'No available times on .'); + assert.equal(B.fmt('No available times on %s.'), 'No available times on .'); + assert.equal(B.fmt('%[3]s missing', ['a']), ' missing'); +}); + +test('fmt leaves a string with no verbs untouched', () => { + assert.equal(B.fmt('No available times.', ['unused']), 'No available times.'); + assert.equal(B.fmt('Inga lediga tider.'), 'Inga lediga tider.'); +}); diff --git a/internal/handler/book.go b/internal/handler/book.go index ff64123..cb5ef2b 100644 --- a/internal/handler/book.go +++ b/internal/handler/book.go @@ -53,12 +53,22 @@ type bookPageData struct { AvatarURL string Hosts []hostDisplay // faces for the info panel (1 = single, >1 = group stack) HostsLabel string // "Alex, Sam & 2 others" for the group case - LocationLabel string - PriceLabel string // formatted price (e.g. "$50.00"); empty for free events - PriceCents int // raw price for the dataLayer conversion value (0 = free) - Currency string // ISO 4217, lowercase - MaxFutureDays int - Questions []bookQuestion + // SoleHostName is the host's name when this event type has exactly one, and "" when + // it has several. It is what lets an empty day read "Alex has no available times on + // …": a group label ("Alex, Sam & 2 others") in that sentence would need a plural + // verb no translation key can supply, so the message drops the name instead (#20). + SoleHostName string + // MinNoticeLabel is the translated minimum-notice duration ("4 hours"), or "" when the + // event type sets none. Rendered here rather than derived by the page's JS because the + // server already knows the resolved locale — the /slots call the page makes carries no + // ?lang=, so a label built from its response would silently ignore a language override. + MinNoticeLabel string + LocationLabel string + PriceLabel string // formatted price (e.g. "$50.00"); empty for free events + PriceCents int // raw price for the dataLayer conversion value (0 = free) + Currency string // ISO 4217, lowercase + MaxFutureDays int + Questions []bookQuestion // AssistantEnabled shows the conversational-booking chat panel when the LLM layer is on. AssistantEnabled bool // AssistantDisclosure is the persistent AI-disclosure notice on the chat panel (Art. 50(1)). @@ -197,6 +207,28 @@ func durationLabel(minutes int, loc *i18n.Locale) string { return fmt.Sprintf(loc.T("duration_hr_min"), h, m) } +// noticeLabel renders an event type's minimum notice as a translated duration ("4 +// hours"), or "" when there is no such policy and so nothing to explain. +// +// It reuses durationLabel rather than introducing notice-specific plural keys: the +// booking surfaces already label durations that way, and a second set of plural forms in +// every locale would be more strings to keep in step for no gain. +func noticeLabel(minNoticeMinutes int, loc *i18n.Locale) string { + if minNoticeMinutes <= 0 { + return "" + } + return durationLabel(minNoticeMinutes, loc) +} + +// soleHostName returns the host's name when the event type has exactly one, else "". +// See bookPageData.SoleHostName for why a group deliberately yields nothing. +func soleHostName(hosts []hostDisplay) string { + if len(hosts) != 1 { + return "" + } + return hosts[0].Name +} + var mdRenderer = goldmark.New( goldmark.WithExtensions(extension.Strikethrough), goldmark.WithRendererOptions(html.WithHardWraps()), @@ -285,18 +317,18 @@ func (h *Handler) PublicEventType(w http.ResponseWriter, r *http.Request) { var ( etID, name, description, locType, locValue string hostName, avatarURL, routingMode, currency string - durMins, maxDays, priceCents int + durMins, maxDays, minNotice, priceCents int msgGreeting sql.NullString ) err := h.db.QueryRowContext(r.Context(), ` SELECT et.id, et.name, COALESCE(et.description, ''), et.duration_minutes, et.location_type, COALESCE(et.location_value, ''), - et.max_future_days, et.routing_mode, u.name, COALESCE(u.avatar_url, ''), + et.max_future_days, et.min_notice_minutes, et.routing_mode, u.name, COALESCE(u.avatar_url, ''), et.price_cents, et.currency, et.msg_greeting FROM event_types et JOIN users u ON u.id = et.user_id WHERE et.slug = ? AND et.is_active = 1 AND et.is_public = 1`, - slug).Scan(&etID, &name, &description, &durMins, &locType, &locValue, &maxDays, &routingMode, &hostName, &avatarURL, &priceCents, ¤cy, &msgGreeting) + slug).Scan(&etID, &name, &description, &durMins, &locType, &locValue, &maxDays, &minNotice, &routingMode, &hostName, &avatarURL, &priceCents, ¤cy, &msgGreeting) if errors.Is(err, sql.ErrNoRows) { h.writeError(w, http.StatusNotFound, "event type not found") return @@ -355,10 +387,16 @@ func (h *Handler) PublicEventType(w http.ResponseWriter, r *http.Request) { // doesn't have to rebuild it from duration_minutes (it used to hardcode " min", // which both skipped translation and disagreed with the pages for >= 60 min). // duration_minutes stays for clients that want the raw number. - "duration_label": durationLabel(durMins, loc), - "location_type": locType, - "location_label": locationLabel(locType, locValue, loc), - "max_future_days": maxDays, + "duration_label": durationLabel(durMins, loc), + "location_type": locType, + "location_label": locationLabel(locType, locValue, loc), + "max_future_days": maxDays, + // min_notice_label is the translated minimum-notice duration ("4 hours"), empty + // when the event type sets none. The widget needs it here because the /slots call + // it makes later carries no language of its own, and min_notice_minutes alone + // would leave it rebuilding a plural-aware label the server already has (#20). + "min_notice_minutes": minNotice, + "min_notice_label": noticeLabel(minNotice, loc), "assistant_enabled": h.getLLM() != nil, "assistant_greeting": assistantGreeting(msgGreeting, loc), "price_cents": priceCents, @@ -384,6 +422,7 @@ func (h *Handler) BookPage(w http.ResponseWriter, r *http.Request) { locType string locValue string maxDays int + minNotice int hostName string avatarURL string routingMode string @@ -394,12 +433,12 @@ func (h *Handler) BookPage(w http.ResponseWriter, r *http.Request) { err := h.db.QueryRowContext(r.Context(), ` SELECT et.id, et.name, COALESCE(et.description, ''), et.duration_minutes, et.location_type, COALESCE(et.location_value, ''), - et.max_future_days, et.routing_mode, u.name, COALESCE(u.avatar_url, ''), + et.max_future_days, et.min_notice_minutes, et.routing_mode, u.name, COALESCE(u.avatar_url, ''), et.price_cents, et.currency, et.msg_greeting FROM event_types et JOIN users u ON u.id = et.user_id WHERE et.slug = ? AND et.is_active = 1 AND et.is_public = 1`, - slug).Scan(&etID, &name, &description, &durMins, &locType, &locValue, &maxDays, &routingMode, &hostName, &avatarURL, &priceCents, ¤cy, &msgGreeting) + slug).Scan(&etID, &name, &description, &durMins, &locType, &locValue, &maxDays, &minNotice, &routingMode, &hostName, &avatarURL, &priceCents, ¤cy, &msgGreeting) if errors.Is(err, sql.ErrNoRows) { http.Error(w, "Page not found", http.StatusNotFound) @@ -470,6 +509,8 @@ func (h *Handler) BookPage(w http.ResponseWriter, r *http.Request) { AvatarURL: hosts[0].AvatarURL, Hosts: hosts, HostsLabel: hostsLabel(hosts, loc), + SoleHostName: soleHostName(hosts), + MinNoticeLabel: noticeLabel(minNotice, loc), LocationLabel: locationLabel(locType, locValue, loc), PriceLabel: formatPrice(priceCents, currency), Locale: loc.Code, diff --git a/internal/handler/booking_surfaces_contract_test.go b/internal/handler/booking_surfaces_contract_test.go index 2dd38d7..ff5e79a 100644 --- a/internal/handler/booking_surfaces_contract_test.go +++ b/internal/handler/booking_surfaces_contract_test.go @@ -63,3 +63,67 @@ func TestBookingSurfacesShareStructuralHooks(t *testing.T) { } } } + +// TestBookingSurfacesExplainEmptyDaysAndMinNotice is the same safety net for the strings +// and payload field that answer "why can't I see those times" (#20). All three surfaces +// have to name the day on an empty one, name the host when there is exactly one, and +// surface the minimum-notice policy when that is what removed the nearest starts — and +// each does it in its own separately-authored code, so forgetting one is silent. +func TestBookingSurfacesExplainEmptyDaysAndMinNotice(t *testing.T) { + var bookBuf, manageBuf bytes.Buffer + if err := bookTmpl.Execute(&bookBuf, bookPageData{T: i18n.Default().T}); err != nil { + t.Fatalf("book render: %v", err) + } + if err := manageTmpl.Execute(&manageBuf, managePageData{T: i18n.Default().T}); err != nil { + t.Fatalf("manage render: %v", err) + } + surfaces := map[string]string{ + "book.html": bookBuf.String(), + "manage.html": manageBuf.String(), + "embed.js": string(embedJS), + } + + required := []string{ + "no_available_times", // the empty-day message, which now names the date + "no_available_times_host", // its " has no available times on " form + "min_notice_hint", // the minimum-notice explanation + "min_notice", // the GET /slots field saying which days it applied to + } + for _, key := range required { + for name, src := range surfaces { + if !strings.Contains(src, key) { + t.Errorf("%q missing from %s — an empty or thinned day there will not explain "+ + "itself; add it to all three surfaces (#20)", key, name) + } + } + } + + // Every locale must actually carry the keys the surfaces look up. i18n's own key-parity + // test compares locales against English; this checks English has them at all, so a + // renamed key can't leave three surfaces rendering their own key names at visitors. + en := i18n.Default() + for _, key := range []string{"no_available_times", "no_available_times_host", "min_notice_hint"} { + if en.T(key) == key { + t.Errorf("locale key %q is missing from en.json — Locale.T falls back to the key "+ + "itself, so the booking page would show %q to a visitor", key, key) + } + } +} + +// TestEmbedJSDoesNotDependOnBookingLogic pins the trap that the shared module's own header +// used to get wrong: embed.js is served as standalone bytes (EmbedJS writes the embedded +// file unmodified), so `BookingLogic` does not exist inside the widget. It carries its own +// copies of the few helpers it needs. A well-meant de-duplication onto BookingLogic would +// throw a ReferenceError on a customer's site, where nothing here would see it. +func TestEmbedJSDoesNotDependOnBookingLogic(t *testing.T) { + for i, line := range strings.Split(string(embedJS), "\n") { + code := strings.TrimSpace(line) + if strings.HasPrefix(code, "//") { // comments may reference it by name + continue + } + if strings.Contains(code, "BookingLogic") { + t.Errorf("embed.js:%d calls into BookingLogic, which is not loaded in the widget: %s", + i+1, code) + } + } +} diff --git a/internal/handler/embed.js b/internal/handler/embed.js index 4a3c1de..ce0556d 100644 --- a/internal/handler/embed.js +++ b/internal/handler/embed.js @@ -38,6 +38,20 @@ // t: pure lookup, not a method — mirrors internal/i18n.Locale.T (falls back to the // key itself if the string table hasn't loaded yet or the key is missing). function t(i18n, key) { return (i18n && i18n[key]) || key; } + // fmt: argument substitution for the translated strings that carry one — %s in order, + // plus the indexed %[n]s a translation may use to reorder. Mirrors BookingLogic.fmt in + // internal/handler/assets/booking-logic.js; this widget does NOT load that module (see + // dowLabels below), so the two must be kept in step. Deliberately not a printf: the keys + // substituted here carry %s only. + function fmt(template, args) { + var list = args || []; + var next = 0; + return String(template).replace(/%(?:\[(\d+)\])?s/g, function (_match, index) { + var pick = index ? Number(index) - 1 : next++; + var value = list[pick]; + return value === undefined || value === null ? '' : String(value); + }); + } // dowLabels: Monday-first weekday header labels via Intl, matching // BookingLogic.dowLabels in booking-logic.js (not literally shared code — this widget // doesn't import that module — but the same approach, replacing what used to be a @@ -176,7 +190,7 @@ this.root.appendChild(el('style', { text: STYLE })); this.wrap = el('div', { class: 'wrap' }); this.root.appendChild(this.wrap); - this.state = { month: startOfMonth(new Date()), slotsByDay: {}, day: null, view: 'pick', slot: null }; + this.state = { month: startOfMonth(new Date()), slotsByDay: {}, noticeDates: [], day: null, view: 'pick', slot: null }; this.narrow = false; this.cw = 9999; this.descExpanded = false; @@ -242,6 +256,10 @@ (by[dayKey(s.start)] = by[dayKey(s.start)] || []).push(s); }); this.state.slotsByDay = by; + // Days the minimum-notice policy took starts away from, so an empty or thin day + // can say why instead of leaving the visitor to guess (#20). Server-side these are + // already in TZ, so they match dayKey's output. + this.state.noticeDates = (r.min_notice && r.min_notice.dates) || []; // Capture the id→host map so the header can narrow to a slot's actual host once // one is picked. Avatar URLs come back relative; make them absolute (the widget // runs cross-origin to the Calnode instance). @@ -251,7 +269,7 @@ var m = r.hosts[id] || {}, av = m.avatar_url || ''; hm[id] = { name: m.name || '', avatar_url: av && av.charAt(0) === '/' ? BASE + av : av }; }); - } catch (e) { this.state.slotsByDay = {}; } + } catch (e) { this.state.slotsByDay = {}; this.state.noticeDates = []; } } infoPane() { @@ -444,9 +462,35 @@ return el('section', { class: 'cal-col' }, [nav, grid, el('p', { class: 'tz-label', text: t(this.i18n, 'times_shown_in') + TZ })]); } + // noticeHint — the minimum-notice explanation, or '' when the event type sets none. + // The label is translated server-side and arrives on /public: the /slots call carries + // no language of its own, and rebuilding a plural-aware duration here would duplicate + // what the server already knows (#20). + noticeHint() { + var label = this.info && this.info.min_notice_label; + return label ? fmt(t(this.i18n, 'min_notice_hint'), [label]) : ''; + } + + // emptyDayText — "Alex has no available times on Mon, 15 Jun", or the date-only form + // when the event type has several hosts (a group label cannot be the subject of that + // sentence in any of the shipped locales). + emptyDayText(dayKeyStr) { + var hosts = (this.info && this.info.hosts) || []; + // 'T00:00:00' (not the bare date) so the browser reads it as local midnight rather + // than UTC, which would name the previous day west of Greenwich. + var label = new Intl.DateTimeFormat(this.locale || [], { + weekday: 'long', month: 'long', day: 'numeric' + }).format(new Date(dayKeyStr + 'T00:00:00')); + if (hosts.length === 1 && hosts[0].name) { + return fmt(t(this.i18n, 'no_available_times_host'), [hosts[0].name, label]); + } + return fmt(t(this.i18n, 'no_available_times'), [label]); + } + rightPane() { var self = this, st = this.state; var inner; + var notice = this.noticeHint(); if (st.view === 'form') inner = this.formView(st.slot); else if (st.view === 'confirm') inner = this.confirmView(st.slot); else if (st.day) { @@ -469,16 +513,43 @@ b.addEventListener('click', function () { self.state.slot = s; self.state.view = 'form'; self.render(); }); listEl.appendChild(b); }); + if (!list.length) { + // Name the day, and the host when there is one: a bare "No available times." + // never said whether another day would help. + listEl.appendChild(el('p', { class: 'hint', text: this.emptyDayText(st.day) })); + } if (list.length && !list.some(function (s) { return !s.taken; })) { listEl.appendChild(el('p', { class: 'hint', text: t(self.i18n, 'all_times_taken') })); } - inner = el('div', {}, [el('p', { class: 'slots-header', text: list[0] ? shortDay(list[0].start, self.locale) : '' }), listEl]); + // Only on a day the policy actually thinned - including one it emptied, and one + // that still shows later times, which is the commonest "why can't I see those + // times" case. + if (notice && st.noticeDates && st.noticeDates.indexOf(st.day) !== -1) { + listEl.appendChild(el('p', { class: 'hint', text: notice })); + } + inner = el('div', {}, [el('p', { class: 'slots-header', text: list[0] ? shortDay(list[0].start, self.locale) : this.dayHeader(st.day) }), listEl]); } else { - inner = el('p', { class: 'hint', text: t(this.i18n, 'select_day_hint') }); + // Before a day is chosen. The notice line belongs here as well as in the list: a + // day the policy emptied completely is greyed out in the calendar, so this is the + // only place the explanation can be reached. + var kids = [el('p', { class: 'hint', text: t(this.i18n, 'select_day_hint') })]; + if (notice && st.noticeDates && st.noticeDates.length) { + kids.push(el('p', { class: 'hint', text: notice })); + } + inner = el('div', {}, kids); } return el('section', { class: 'right-col' }, [inner]); } + // dayHeader — the selected day's short label when no slot is available to derive it + // from, so an empty day still gets the same header as a full one. + dayHeader(dayKeyStr) { + if (!dayKeyStr) return ''; + return new Intl.DateTimeFormat(this.locale || [], { + weekday: 'short', month: 'short', day: 'numeric' + }).format(new Date(dayKeyStr + 'T00:00:00')); + } + formView(slot) { var self = this; var back = el('button', { class: 'back-btn', html: SVG_BACK + ' ' + t(this.i18n, 'back') }); diff --git a/internal/handler/manage_handler.go b/internal/handler/manage_handler.go index 401d1ff..412df06 100644 --- a/internal/handler/manage_handler.go +++ b/internal/handler/manage_handler.go @@ -26,13 +26,21 @@ var manageTmpl = template.Must(template.Must(template.New("manage").Funcs(templa }).Parse(sharedPartialsSrc)).Parse(manageTmplSrc)) type managePageData struct { - Token string - BookingID string - EventTypeName string - EventTypeSlug string - HostName string - HostInitial string - AvatarURL string + Token string + BookingID string + EventTypeName string + EventTypeSlug string + HostName string + HostInitial string + AvatarURL string + // SoleHostName is the host's name when this booking has exactly one, else "" — see + // bookPageData.SoleHostName. HostName can be a group label ("Alex, Sam & 2 others"), + // which no "%s has no available times" sentence can use grammatically. + SoleHostName string + // MinNoticeLabel is the translated minimum-notice duration ("4 hours") of the event + // type being rescheduled, or "" when it sets none. Reschedule goes through the same + // /slots endpoint as booking, so the same policy hides the same nearest times (#20). + MinNoticeLabel string DurationLabel string LocationLabel string PriceLabel string // empty on manage → the eventMeta partial omits the price row @@ -86,14 +94,14 @@ func (h *Handler) ManagePage(w http.ResponseWriter, r *http.Request) { } var etName, etSlug, locType, locValue string - var durMins, maxDays int + var durMins, maxDays, minNotice int var hostName string if err := h.db.QueryRowContext(r.Context(), ` - SELECT et.name, et.slug, et.duration_minutes, et.max_future_days, + SELECT et.name, et.slug, et.duration_minutes, et.max_future_days, et.min_notice_minutes, et.location_type, COALESCE(et.location_value,''), u.name FROM event_types et JOIN users u ON u.id = et.user_id WHERE et.id = ?`, b.EventTypeID). - Scan(&etName, &etSlug, &durMins, &maxDays, &locType, &locValue, &hostName); err != nil { + Scan(&etName, &etSlug, &durMins, &maxDays, &minNotice, &locType, &locValue, &hostName); err != nil { h.logger.ErrorContext(r.Context(), "manage page: load event type", "error", err) http.Error(w, "Internal server error", http.StatusInternalServerError) return @@ -103,13 +111,17 @@ func (h *Handler) ManagePage(w http.ResponseWriter, r *http.Request) { // (round-robin/Group route elsewhere). Falls back to the owner name above if // no booking_hosts rows exist. The avatar uses the primary host. loc := h.resolveLocale(r) - var hostInitial, avatarURL string + var hostInitial, avatarURL, soleHost string if hosts := h.displayHostsForBooking(r.Context(), b.ID); len(hosts) > 0 { hostName = hostsLabel(hosts, loc) hostInitial = hosts[0].Initial avatarURL = hosts[0].AvatarURL + if len(hosts) == 1 { + soleHost = hosts[0].Name + } } else { hostInitial = firstRune(hostName) + soleHost = hostName // the event-type owner: one person, so nameable } var orgTZ string @@ -128,6 +140,8 @@ func (h *Handler) ManagePage(w http.ResponseWriter, r *http.Request) { HostName: hostName, HostInitial: hostInitial, AvatarURL: avatarURL, + SoleHostName: soleHost, + MinNoticeLabel: noticeLabel(minNotice, loc), DurationLabel: durationLabel(durMins, loc), LocationLabel: locationLabel(locType, locValue, loc), MaxFutureDays: maxDays, diff --git a/internal/handler/slots_handler.go b/internal/handler/slots_handler.go index 45e8c99..f13636c 100644 --- a/internal/handler/slots_handler.go +++ b/internal/handler/slots_handler.go @@ -31,6 +31,14 @@ type slotsResult struct { // being nil does not work, because converting an empty result yields an empty // non-nil slice. ShowsTaken bool + // MinNoticeMinutes is the event type's minimum-notice policy; 0 = none. + MinNoticeMinutes int + // MinNoticeDates are the days (YYYY-MM-DD, in the requested timezone, so they match + // the keys the booking surfaces group slots by) on which that policy removed at + // least one start that was otherwise bookable. It is the only thing a surface needs + // to decide whether to explain a thin or empty day, and it says nothing about which + // times or which host - see slots.Result.NoticeGap. + MinNoticeDates []string } // Sentinel errors from computeSlots, so non-HTTP callers (the MCP tools) can map @@ -75,6 +83,21 @@ func (h *Handler) GetSlots(w http.ResponseWriter, r *http.Request) { if res.ShowsTaken { body["taken"] = res.Taken } + // Present whenever the event type sets a minimum notice, with an empty `dates` when + // the policy happened to remove nothing in this range. Same reasoning as `taken`: a + // client can tell "there is no such policy" from "the policy cost you nothing here", + // and only the second is worth explaining. `minutes` lets a client that has no + // localized label of its own still say something. + if res.MinNoticeMinutes > 0 { + dates := res.MinNoticeDates + if dates == nil { + dates = []string{} + } + body["min_notice"] = map[string]any{ + "minutes": res.MinNoticeMinutes, + "dates": dates, + } + } h.writeJSON(w, http.StatusOK, body) } @@ -126,8 +149,13 @@ func (h *Handler) computeSlots(ctx context.Context, slug, tzName, fromStr, toStr } if len(pool) == 0 { // No bookable hosts (e.g. all archived, or a round-robin with no rotation - // members) — offer nothing rather than erroring. - return slotsResult{Slots: []slotJSON{}, Hosts: map[string]map[string]string{}}, nil + // members) — offer nothing rather than erroring. The notice policy still travels, + // so the response shape doesn't depend on how the day turned out. + return slotsResult{ + Slots: []slotJSON{}, + Hosts: map[string]map[string]string{}, + MinNoticeMinutes: et.MinNoticeMinutes, + }, nil } // Load each host's availability concurrently. The slow part is the Google @@ -178,12 +206,7 @@ func (h *Handler) computeSlots(ctx context.Context, slug, tzName, fromStr, toStr // opted in. GenerateWithTaken walks the range a second time with busy ignored, so // it is not free, and it returns exactly the information the default must withhold. showsTaken := includeTaken && et.ShowTakenSlots - var result, takenSlots []slots.Slot - if showsTaken { - result, takenSlots, err = slots.GenerateWithTaken(req) - } else { - result, err = slots.Generate(req) - } + result, err := slots.GenerateDetailed(req, slots.Extras{Taken: showsTaken, NoticeGap: true}) if err != nil { return slotsResult{}, fmt.Errorf("slots generate: %w", err) } @@ -196,13 +219,40 @@ func (h *Handler) computeSlots(ctx context.Context, slug, tzName, fromStr, toStr poolIDs[i] = ph.id } return slotsResult{ - Slots: toSlotJSON(result), - Taken: toSlotJSON(takenSlots), - Hosts: h.hostDisplayMap(ctx, poolIDs), - ShowsTaken: showsTaken, + Slots: toSlotJSON(result.Free), + Taken: toSlotJSON(result.Taken), + Hosts: h.hostDisplayMap(ctx, poolIDs), + ShowsTaken: showsTaken, + MinNoticeMinutes: et.MinNoticeMinutes, + MinNoticeDates: noticeDates(result.NoticeGap), }, nil } +// noticeDates reduces the starts the minimum-notice rule withheld to the distinct days +// they fall on, ascending. The engine renders those starts in the booker's timezone, so +// formatting them here yields the same YYYY-MM-DD keys the booking surfaces group their +// slots by. +// +// A day is all a surface needs to explain the gap, and it is all that is sent: the +// individual withheld times would describe the host's working hours at a finer grain than +// the feature requires. +func noticeDates(gap []slots.Slot) []string { + if len(gap) == 0 { + return nil + } + out := make([]string, 0, 2) // the notice window spans a day or two at most + seen := make(map[string]bool, 2) + for _, s := range gap { + day := s.Start.Format("2006-01-02") + if seen[day] { + continue + } + seen[day] = true + out = append(out, day) + } + return out +} + // toSlotJSON renders engine slots for the wire. Taken slots carry no host ids, so // host_ids is simply absent for them rather than invented. func toSlotJSON(in []slots.Slot) []slotJSON { diff --git a/internal/handler/slots_notice_test.go b/internal/handler/slots_notice_test.go new file mode 100644 index 0000000..cf0365a --- /dev/null +++ b/internal/handler/slots_notice_test.go @@ -0,0 +1,162 @@ +package handler_test + +import ( + "database/sql" + "testing" + "time" + + "github.com/calnode/calnode/internal/uid" +) + +// The minimum-notice policy hides the nearest starts and leaves nothing behind to explain +// them (#20). GET /slots therefore reports the policy and the days it actually cost +// something, which is what lets the three booking surfaces say so without re-deriving it. +// +// These tests run against the real clock, because computeSlots calls time.Now itself. +// Every assertion is therefore phrased relative to now rather than against a fixture +// date — a test that only holds between 09:00 and 17:00 would be worse than none. + +type slotsPayload = slotsBody + +// seedNoticeEventType inserts a bookable event type whose single host is available around +// the clock every day, so the only thing that can remove a start is the notice policy. +func seedNoticeEventType(t *testing.T, database *sql.DB, ownerID, slug string, minNoticeMinutes int) { + t.Helper() + etID := uid.New() + if _, err := database.Exec(` + INSERT INTO event_types (id, user_id, slug, name, duration_minutes, slot_interval_minutes, + min_notice_minutes, max_future_days, is_active, is_public) + VALUES (?, ?, ?, 'Intro Call', 30, 30, ?, 0, 1, 1)`, + etID, ownerID, slug, minNoticeMinutes); err != nil { + t.Fatalf("seed event type: %v", err) + } + if _, err := database.Exec(` + INSERT INTO event_type_hosts (id, event_type_id, user_id, role, priority) + VALUES (?, ?, ?, 'required', 0)`, uid.New(), etID, ownerID); err != nil { + t.Fatalf("seed host: %v", err) + } + for day := 0; day < 7; day++ { + if _, err := database.Exec(` + INSERT INTO availability_rules (id, user_id, day_of_week, start_time, end_time) + VALUES (?, ?, ?, '00:00', '23:59')`, uid.New(), ownerID, day); err != nil { + t.Fatalf("seed availability day %d: %v", day, err) + } + } +} + +func TestGetSlots_reportsTheDaysMinimumNoticeEmptied(t *testing.T) { + h, database, _, ownerID := setupWorkspaceWithDB(t) + seedNoticeEventType(t, database, ownerID, "notice-call", 24*60) + + now := time.Now().UTC() + from := now.Format("2006-01-02") + to := now.AddDate(0, 0, 2).Format("2006-01-02") + got := getSlots(t, h, "notice-call", "?from="+from+"&to="+to+"&tz=UTC") + + if got.MinNotice == nil { + t.Fatal("min_notice is absent for an event type that sets one") + } + if got.MinNotice.Minutes != 24*60 { + t.Errorf("min_notice.minutes: got %d; want %d", got.MinNotice.Minutes, 24*60) + } + if len(got.MinNotice.Dates) == 0 { + t.Fatal("min_notice.dates is empty, but a 24-hour notice on an all-hours calendar must have hidden today's remaining starts") + } + // The notice window is 24 hours from now, so it can only touch today and tomorrow. + allowed := map[string]bool{ + from: true, + now.AddDate(0, 0, 1).Format("2006-01-02"): true, + } + for _, d := range got.MinNotice.Dates { + if !allowed[d] { + t.Errorf("min_notice.dates contains %q, which is outside the 24-hour notice window (today/tomorrow)", d) + } + } + // And the policy really is in force: nothing bookable inside the window. + cutoff := now.Add(24 * time.Hour) + if len(got.Slots) == 0 { + t.Fatal("no slots at all; the fixture is meant to leave later days bookable") + } + for _, s := range got.Slots { + start, err := time.Parse(time.RFC3339, s.Start) + if err != nil { + t.Fatalf("parse slot start %q: %v", s.Start, err) + } + if start.Before(cutoff) { + t.Errorf("slot at %s is inside the 24-hour notice window", s.Start) + } + } +} + +func TestGetSlots_omitsMinNoticeWhenThereIsNoPolicy(t *testing.T) { + // Absent rather than zeroed, so a client can tell "no such policy" from "the policy + // cost you nothing here" — the same distinction `taken` draws. + h, database, _, ownerID := setupWorkspaceWithDB(t) + seedNoticeEventType(t, database, ownerID, "open-call", 0) + + now := time.Now().UTC() + got := getSlots(t, h, "open-call", + "?from="+now.Format("2006-01-02")+"&to="+now.AddDate(0, 0, 1).Format("2006-01-02")+"&tz=UTC") + if got.MinNotice != nil { + t.Errorf("min_notice present for an event type with no minimum notice: %+v", *got.MinNotice) + } +} + +func TestGetSlots_minNoticeDatesEmptyWhenThePolicyCostThisRangeNothing(t *testing.T) { + // A one-minute notice, asked about days that start the day after tomorrow: the policy + // exists and is reported, but it took nothing away in this window, so nothing should + // be explained. + // + // ⚠️ The window deliberately starts at +2 days, not +1. seedNoticeEventType opens + // availability at 00:00, so tomorrow's first slot is midnight — and in the last minute + // of a UTC day a one-minute notice pushes the cutoff past it (now 23:59:01 → cutoff + // 00:00:01), withholding that slot and making dates non-empty. That is a real ~59 + // seconds a day where this test would fail for a reason unrelated to what it asserts. + // Starting a full day later puts every candidate slot at least 24 hours beyond any + // cutoff a one-minute policy can produce, whatever the clock says. + h, database, _, ownerID := setupWorkspaceWithDB(t) + seedNoticeEventType(t, database, ownerID, "tiny-notice", 1) + + now := time.Now().UTC() + from := now.AddDate(0, 0, 2).Format("2006-01-02") + to := now.AddDate(0, 0, 4).Format("2006-01-02") + got := getSlots(t, h, "tiny-notice", "?from="+from+"&to="+to+"&tz=UTC") + + if got.MinNotice == nil { + t.Fatal("min_notice is absent for an event type that sets one") + } + if len(got.MinNotice.Dates) != 0 { + t.Errorf("min_notice.dates: got %v; want empty (the window starts tomorrow)", got.MinNotice.Dates) + } +} + +func TestGetSlots_minNoticeDatesUseTheRequestedTimezone(t *testing.T) { + // The surfaces group slots by day in the tz they asked for, so these keys have to be + // in that tz too or the explanation lands on the wrong day. Auckland is up to 13 hours + // ahead of UTC, so its "today" differs from UTC's for half the clock. + h, database, _, ownerID := setupWorkspaceWithDB(t) + seedNoticeEventType(t, database, ownerID, "tz-call", 24*60) + + akl, err := time.LoadLocation("Pacific/Auckland") + if err != nil { + t.Skipf("Pacific/Auckland unavailable: %v", err) + } + now := time.Now().UTC() + from := now.AddDate(0, 0, -1).Format("2006-01-02") + to := now.AddDate(0, 0, 2).Format("2006-01-02") + got := getSlots(t, h, "tz-call", "?from="+from+"&to="+to+"&tz=Pacific%2FAuckland") + + if got.MinNotice == nil || len(got.MinNotice.Dates) == 0 { + t.Fatal("expected min_notice dates for a 24-hour notice on an all-hours calendar") + } + nowAKL := time.Now().In(akl) + allowed := map[string]bool{ + nowAKL.Format("2006-01-02"): true, + nowAKL.AddDate(0, 0, 1).Format("2006-01-02"): true, + } + for _, d := range got.MinNotice.Dates { + if !allowed[d] { + t.Errorf("min_notice.dates contains %q; want Auckland-local today/tomorrow (%v)", d, allowed) + } + } +} diff --git a/internal/handler/taken_slots_test.go b/internal/handler/taken_slots_test.go index 007d200..c7fa165 100644 --- a/internal/handler/taken_slots_test.go +++ b/internal/handler/taken_slots_test.go @@ -23,6 +23,13 @@ type slotsBody struct { Start string `json:"start"` HostIDs []string `json:"host_ids"` } `json:"taken"` + // MinNotice is a pointer for the same reason: absent means the event type sets no + // minimum notice, present-but-empty means it does and this range lost nothing to it + // (see slots_notice_test.go). + MinNotice *struct { + Minutes int `json:"minutes"` + Dates []string `json:"dates"` + } `json:"min_notice"` } func getSlots(t *testing.T, h interface { diff --git a/internal/handler/templates/book.html b/internal/handler/templates/book.html index 353a95b..63b21db 100644 --- a/internal/handler/templates/book.html +++ b/internal/handler/templates/book.html @@ -166,6 +166,11 @@

{{.Name}}

{{call .T "select_day_hint"}}

+ +