Skip to content
Closed
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
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,24 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad
the link-only fallback that needs no Zoom app. Answers
[#35](https://github.com/Calnode/calnode/issues/35).

- **Booking emails name the host a booking was assigned to, not the owner of its event
type.** On a round-robin or multi-host event type those are different people, and the
owner may not attend at all, but the host was looked up through `event_types.user_id`.
Fixes [#48](https://github.com/Calnode/calnode/issues/48), reported with the fix by
[@MinosChatzidakis](https://github.com/MinosChatzidakis).

- The **reminder** told the attendee they were meeting the owner, and was sent or
skipped by the owner's `notify_reminder` preference rather than the host's.
- The host's **reschedule** notice went to the owner, so the host who was actually
attending was never told the meeting had moved. The attendee's reschedule email and
its `.ics` organizer named the owner too. This applies to every reschedule path:
admin, manage link and MCP.
- The **cancellation** and **reassign** emails loaded the owner first and replaced
them before sending, so neither was visibly wrong. Both now start from the booking's
host, and reassign no longer needs its own second lookup.

An event type whose owner is also its only host is unaffected.

## [0.9.0] - 2026-09-10

### Added
Expand Down
6 changes: 6 additions & 0 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,12 @@ as the desired state:
itself is `multipart/alternative` (text+HTML) or single-part text.
- Multi-host fan-out: each assigned host gets their host-notification; the attendee
gets one. (See §9.)
- **The host in a booking email is the booking's, never the event type's owner.**
Name, address and notification prefs come from `bookings.host_id` (the primary host)
or `booking_hosts`, not `event_types.user_id`: on round-robin and multi-host event
types the owner is often not attending. The reminder worker joins on
`bookings.host_id`; `loadCancellationData` (cancel, reschedule, reassign) takes the
host from the `HostID` of the booking it is given.
- **Per-event-type customisation:** custom note bodies (`msg_*`) and custom subject
lines (`subj_*`, migration 00026) for the four attendee emails; a blank subject
falls back to the built-in default (`BookingData.SubjectOverride` / `subjectOr`).
Expand Down
21 changes: 10 additions & 11 deletions internal/handler/booking_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -1786,14 +1786,11 @@ func primaryHost(hosts []assignedHost) assignedHost {
return hosts[0]
}

// loadHostIntoData fills HostName and HostEmail in d from the users table.
func (h *Handler) loadHostIntoData(ctx context.Context, hostID string, d *mailer.BookingData) error {
return h.db.QueryRowContext(ctx,
`SELECT name, email FROM users WHERE id = ?`, hostID).
Scan(&d.HostName, &d.HostEmail)
}

// loadCancellationData assembles all fields needed for cancellation emails.
// loadCancellationData assembles the fields the cancellation, reschedule and
// reassign emails share. HostName and HostEmail are those of b.HostID, the
// booking's primary host, so the caller decides which host they describe by the
// booking copy it passes. Paths that notify several hosts still override them
// per host (hostBookingData).
func (h *Handler) loadCancellationData(ctx context.Context, b *booking.Booking) (mailer.BookingData, error) {
var d mailer.BookingData
d.BookingID = b.ID
Expand All @@ -1802,11 +1799,13 @@ func (h *Handler) loadCancellationData(ctx context.Context, b *booking.Booking)
d.LocationValue = b.LocationValue
d.CancellationReason = b.CancellationReason

// Event type name + slug and host name + email in one join.
// Event type name + slug, and the booking's host. Not event_types.user_id: the
// owner of a round-robin or multi-host event type is often not the person the
// booking was assigned to, and may not be attending at all.
err := h.db.QueryRowContext(ctx, `
SELECT et.name, et.slug, u.name, u.email
FROM event_types et JOIN users u ON u.id = et.user_id
WHERE et.id = ?`, b.EventTypeID).
FROM event_types et JOIN users u ON u.id = ?
WHERE et.id = ?`, b.HostID, b.EventTypeID).
Scan(&d.EventTypeName, &d.EventTypeSlug, &d.HostName, &d.HostEmail)
if err != nil {
return d, fmt.Errorf("load event/host: %w", err)
Expand Down
237 changes: 237 additions & 0 deletions internal/handler/booking_host_mail_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
package handler

import (
"context"
"encoding/json"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/calnode/calnode/internal/booking"
"github.com/calnode/calnode/internal/db"
"github.com/calnode/calnode/internal/mailer"
)

// Booking emails must name and notify the host a booking was assigned to
// (bookings.host_id), not the owner of its event type (event_types.user_id). On a
// round-robin or hosts-tab event type those are different people, and the owner
// may not be attending at all. Issue #48 reported the reminder; the same owner
// join fed the cancellation, reschedule and reassign emails through
// loadCancellationData.
//
// Every test here uses one shape: the event type is owned by the admin who ran
// setup (Test Host), and the booking is assigned to a second member (Hana Host).

const (
etOwnerEmail = "host@example.com" // the setup admin, who owns the event type
etOwnerName = "Test Host"
bookingHostEmail = "hana@example.com"
bookingHostName = "Hana Host"
bookerEmail = "dana@example.com"
)

type hostMailFixture struct {
h *Handler
cap *captureMailer
apiKey string
b *booking.Booking
}

func newHostMailFixture(t *testing.T) hostMailFixture {
t.Helper()
database, err := db.Open("sqlite://:memory:")
if err != nil {
t.Fatalf("db.Open: %v", err)
}
t.Cleanup(func() { database.Close() })
if err := db.Migrate(database); err != nil {
t.Fatalf("db.Migrate: %v", err)
}

h := New(database, slog.Default())
cap := &captureMailer{}
h.SetMailer(cap, "https://calnode.example.com")

rec := httptest.NewRecorder()
h.Setup(rec, httptest.NewRequest(http.MethodPost, "/v1/setup",
strings.NewReader(`{"name":"`+etOwnerName+`","email":"`+etOwnerEmail+`","timezone":"UTC"}`)))
if rec.Code != http.StatusCreated {
t.Fatalf("setup: %d — %s", rec.Code, rec.Body.String())
}
var setup struct {
APIKey string `json:"api_key"`
UserID string `json:"user_id"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &setup); err != nil {
t.Fatalf("decode setup: %v", err)
}

start := time.Now().UTC().Add(72 * time.Hour).Truncate(time.Hour)
end := start.Add(30 * time.Minute)
for _, q := range []struct {
sql string
args []any
}{
{`INSERT INTO users (id, email, name, iana_timezone, is_admin) VALUES ('u-hana', ?, ?, 'UTC', 0)`,
[]any{bookingHostEmail, bookingHostName}},
{`INSERT INTO users (id, email, name, iana_timezone, is_admin) VALUES ('u-nico', 'nico@example.com', 'Nico New', 'UTC', 0)`, nil},
{`INSERT INTO event_types (id, user_id, slug, name, duration_minutes) VALUES ('et-team', ?, 'team-intro', 'Team Intro', 30)`,
[]any{setup.UserID}},
{`INSERT INTO bookings (id, event_type_id, host_id, start_at, end_at, status) VALUES ('b-1', 'et-team', 'u-hana', ?, ?, 'confirmed')`,
[]any{start.Format(time.RFC3339Nano), end.Format(time.RFC3339Nano)}},
{`INSERT INTO booking_hosts (id, booking_id, user_id, is_primary) VALUES ('bh-1', 'b-1', 'u-hana', 1)`, nil},
{`INSERT INTO booking_attendees (id, booking_id, name, email, iana_timezone, is_organizer) VALUES ('a-1', 'b-1', 'Dana', ?, 'UTC', 1)`,
[]any{bookerEmail}},
} {
if _, err := database.Exec(q.sql, q.args...); err != nil {
t.Fatalf("seed: %v\n%s", err, q.sql)
}
}

b, err := h.bookingSvc.Get(context.Background(), "b-1")
if err != nil {
t.Fatalf("load booking: %v", err)
}
return hostMailFixture{h: h, cap: cap, apiKey: setup.APIKey, b: b}
}

// assertNamesOnly fails unless every body of msg names want and none names the
// event type's owner.
func assertNamesOnly(t *testing.T, label string, msg *mailer.Message, want string) {
t.Helper()
for part, body := range map[string]string{"text": msg.Text, "html": msg.HTML} {
if !strings.Contains(body, want) {
t.Errorf("%s (%s) does not name %q:\n%s", label, part, want, body)
}
if strings.Contains(body, etOwnerName) {
t.Errorf("%s (%s) names the event type's owner %q, who is not this booking's host:\n%s",
label, part, etOwnerName, body)
}
}
}

func TestLoadCancellationData_namesTheBookingsHostNotTheEventTypeOwner(t *testing.T) {
f := newHostMailFixture(t)

d, err := f.h.loadCancellationData(context.Background(), f.b)
if err != nil {
t.Fatalf("loadCancellationData: %v", err)
}
if d.HostName != bookingHostName || d.HostEmail != bookingHostEmail {
t.Errorf("host = %q <%s>; want the booking's host %q <%s>, not the event type's owner",
d.HostName, d.HostEmail, bookingHostName, bookingHostEmail)
}
if d.EventTypeName != "Team Intro" || d.EventTypeSlug != "team-intro" {
t.Errorf("event type = %q/%q; want Team Intro/team-intro", d.EventTypeName, d.EventTypeSlug)
}
}

// The reschedule notice went to the event type's owner, and the host the booking
// was assigned to heard nothing, while the preference consulted was the assigned
// host's. This is the path where the wrong join changed who received mail.
func TestRescheduleSideEffects_notifiesTheBookingsHostNotTheEventTypeOwner(t *testing.T) {
f := newHostMailFixture(t)

f.h.rescheduleSideEffects(*f.b, f.b.EventTypeID, f.b.StartAt.Add(-24*time.Hour), f.b.EndAt.Add(-24*time.Hour))

if msg := f.cap.find(etOwnerEmail); msg != nil {
t.Errorf("the event type's owner was sent %q; they are not this booking's host", msg.Subject)
}
host := f.cap.find(bookingHostEmail)
if host == nil {
t.Fatalf("the booking's host was not notified of the reschedule; sent to: %v", f.cap.recipients())
}
assertNamesOnly(t, "host reschedule notice", host, bookingHostName)

attendee := f.cap.find(bookerEmail)
if attendee == nil {
t.Fatalf("no reschedule email to the attendee; sent to: %v", f.cap.recipients())
}
assertNamesOnly(t, "attendee reschedule email", attendee, bookingHostName)
// The invite's ORGANIZER is the host too, or the attendee's calendar shows the
// owner as the person running the meeting.
if len(attendee.Attachments) == 0 {
t.Fatal("attendee reschedule email carries no .ics (no calendar is connected in this test, so it should)")
}
if ics := string(attendee.Attachments[0].Content); !strings.Contains(ics, "mailto:"+bookingHostEmail) ||
strings.Contains(ics, etOwnerEmail) {
t.Errorf("attendee .ics ORGANIZER is not the booking's host:\n%s", ics)
}
}

// Cancellation already fanned out over booking_hosts and overwrote the owner's
// details per host, so this held before the fix. It is pinned here so that the
// two paths sharing loadCancellationData stay in step.
func TestCancelSideEffects_notifiesTheBookingsHostNotTheEventTypeOwner(t *testing.T) {
f := newHostMailFixture(t)
ctx := context.Background()
if err := f.h.bookingSvc.CancelByID(ctx, f.b.ID, "conflict"); err != nil {
t.Fatalf("cancel: %v", err)
}
b, err := f.h.bookingSvc.Get(ctx, f.b.ID)
if err != nil {
t.Fatalf("reload: %v", err)
}

f.h.cancelSideEffects(*b)

if msg := f.cap.find(etOwnerEmail); msg != nil {
t.Errorf("the event type's owner was sent %q; they are not this booking's host", msg.Subject)
}
host := f.cap.find(bookingHostEmail)
if host == nil {
t.Fatalf("the booking's host was not notified of the cancellation; sent to: %v", f.cap.recipients())
}
assertNamesOnly(t, "host cancellation notice", host, bookingHostName)
attendee := f.cap.find(bookerEmail)
if attendee == nil {
t.Fatalf("no cancellation email to the attendee; sent to: %v", f.cap.recipients())
}
assertNamesOnly(t, "attendee cancellation email", attendee, bookingHostName)
}

// Reassign emails the attendee and the NEW host. They must name the new host: not
// the old one, and not the event type's owner. ReassignBooking sends them from its
// own goroutine, so this polls by recipient.
func TestReassignBooking_emailsNameTheNewHost(t *testing.T) {
f := newHostMailFixture(t)

req := httptest.NewRequest(http.MethodPost, "/v1/bookings/b-1/reassign", strings.NewReader(`{"host_id":"u-nico"}`))
req.Header.Set("X-API-Key", f.apiKey)
req.SetPathValue("id", "b-1")
rec := httptest.NewRecorder()
f.h.RequireAuth(f.h.ReassignBooking)(rec, req)
if rec.Code != http.StatusOK {
t.Fatalf("reassign: %d — %s", rec.Code, rec.Body.String())
}

// The attendee is emailed before the new host, so once the host's email has
// arrived both sends have happened.
var newHost *mailer.Message
for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(5 * time.Millisecond) {
if newHost = f.cap.find("nico@example.com"); newHost != nil {
break
}
}
if newHost == nil {
t.Fatalf("the new host was not notified; sent to: %v", f.cap.recipients())
}
assertNamesOnly(t, "new host notice", newHost, "Nico New")

attendee := f.cap.find(bookerEmail)
if attendee == nil {
t.Fatalf("no email to the attendee; sent to: %v", f.cap.recipients())
}
assertNamesOnly(t, "attendee reassign email", attendee, "Nico New")
if strings.Contains(attendee.Text, bookingHostName) {
t.Errorf("attendee reassign email still names the previous host %q:\n%s", bookingHostName, attendee.Text)
}
for _, to := range []string{etOwnerEmail, bookingHostEmail} {
if msg := f.cap.find(to); msg != nil {
t.Errorf("%s was sent %q; only the attendee and the new host should be emailed", to, msg.Subject)
}
}
}
7 changes: 3 additions & 4 deletions internal/handler/reassign.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,16 @@ func (h *Handler) ReassignBooking(w http.ResponseWriter, r *http.Request) {
}

// Notify the attendee (their host changed) and the new host, reusing the
// confirmation templates. Host details are overridden to the new host.
// confirmation templates. The emails name the NEW host: ReassignHost returns
// the booking with HostID already set to newHostID, and loadCancellationData
// reads the host from the booking copy it is given.
d, err := h.loadCancellationData(ctx, &bCopy)
if err != nil {
h.logger.Error("reassign: load email data", "error", err, "booking_id", bCopy.ID)
return
}
d.BaseURL = h.publicURL()
h.applyBranding(ctx, &d)
if err := h.loadHostIntoData(ctx, newHostID, &d); err != nil {
h.logger.Error("reassign: load new host", "error", err, "booking_id", bCopy.ID)
}

prefs := h.hostPrefsOrDefault(ctx, bCopy.ID, newHostID)
if prefs.NotifyConfirmation {
Expand Down
5 changes: 3 additions & 2 deletions internal/handler/reschedule_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ func (h *Handler) RescheduleBooking(w http.ResponseWriter, r *http.Request) {

// Side effects (calendar move, Zoom update, emails, webhook, reminders) are shared
// with the manage-token reschedule flow — see rescheduleSideEffects in
// manage_handler.go. etSlug is unused here now; it's re-derived inside the helper
// via loadCancellationData's own join, which is equivalent.
// manage_handler.go. etSlug is unused here now; the helper re-reads the slug from
// the booking's event type, and takes the host it names and notifies from the
// booking itself (updated.HostID), not from the event type's owner.
go h.rescheduleSideEffects(*updated, etID, previousStart, previousEnd) // #nosec G118 -- deliberately its own context.Background(); see rescheduleSideEffects' doc comment
}
Loading
Loading