Phase 6 follow-up: today-and-future events, one write per school-wide event

Two corrections found while preparing the first live run: - A scan that runs late in the day still records today's school events; the candidate window now starts at the local start of day instead of "now". - The Compass week calendar is school-wide, so one event is stored against each child. Write it once, with no child prefix in the title, and record the same Home Assistant event id on every child's item. A per-child event still keeps the child's name. Also report pending events as events, not rows, in `school calendar status`. Tests cover the window and the de-duplication. Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-10-15 18:05:33 +11:00
parent f6be17fe4f
commit f56dbb46ac
5 changed files with 244 additions and 80 deletions
--- a/internal/calendar/sync.go
+++ b/internal/calendar/sync.go
@@ -38,10 +38,20 @@ type Options struct {
 	DryRun bool
 	// Now is the clock, so tests are deterministic.
 	Now time.Time
+	// From overrides the start of the window. It defaults to the start of the
+	// local day, so a scan that runs late in the day still records today's
+	// school events instead of dropping them as "past".
+	From time.Time
 	// Duration overrides DefaultDuration.
 	Duration time.Duration
 }

+// StartOfDay is midnight at the start of t's local day.
+func StartOfDay(t time.Time) time.Time {
+	y, m, d := t.Date()
+	return time.Date(y, m, d, 0, 0, 0, 0, t.Location())
+}
+
 // Created is one event that the run wrote (or would write on a dry run).
 type Created struct {
 	ItemID  int64  `json:"item_id"`
@@ -91,77 +101,75 @@ func Sync(ctx context.Context, st *store.Store, c *Client, opts Options, log *sl
 	if duration <= 0 {
 		duration = DefaultDuration
 	}
+	from := opts.From
+	if from.IsZero() {
+		from = StartOfDay(now)
+	}

 	res := Result{Entity: c.Entity(), DryRun: opts.DryRun}

-	candidates, err := st.CalendarCandidates(ctx, opts.ChildID, opts.ItemID, now, opts.Limit)
+	candidates, err := st.CalendarCandidates(ctx, opts.ChildID, opts.ItemID, from, opts.Limit)
 	if err != nil {
 		return res, err
 	}
-	res.Candidates = len(candidates)
-	if len(candidates) == 0 {
+	// The Compass week calendar is school-wide, so the same event is stored
+	// against each child. Write it once, and keep every child's item in step.
+	groups := GroupDuplicates(candidates)
+	res.Candidates = len(groups)
+	if len(groups) == 0 {
 		return res, nil
 	}

 	// One window covers every candidate, so the marker scan is a single call.
 	windowEnd := now.Add(24 * time.Hour)
-	for _, it := range candidates {
-		if it.DueAt != nil && it.DueAt.After(windowEnd) {
-			windowEnd = it.DueAt.Add(24 * time.Hour)
+	for _, g := range groups {
+		if due := g[0].DueAt; due != nil && due.After(windowEnd) {
+			windowEnd = due.Add(24 * time.Hour)
 		}
 	}
-	existing, err := c.Events(ctx, now.Add(-24*time.Hour), windowEnd)
+	existing, err := c.Events(ctx, from.Add(-24*time.Hour), windowEnd)
 	if err != nil {
 		// A failed read is not a licence to write blind: it would risk
 		// duplicates, so record one loud failure and stop.
 		msg := fmt.Sprintf("calendar: reading existing events failed: %v", err)
-		res.Failed = len(candidates)
+		res.Failed = len(groups)
 		res.Errors = append(res.Errors, msg)
-		for _, it := range candidates {
-			if _, rerr := st.RecordCalendarPush(ctx, it.ID, c.Entity(), "", false, msg); rerr != nil {
-				log.Error("record calendar push", "item", it.ID, "err", rerr)
-			}
+		for _, g := range groups {
+			recordPush(ctx, st, c.Entity(), g, "", false, msg, log)
 		}
 		log.Error("calendar sync aborted", "entity", c.Entity(), "err", err)
 		return res, nil
 	}
 	known := indexByMarker(existing)

-	for _, it := range candidates {
-		if it.DueAt == nil {
+	for _, g := range groups {
+		first := g[0]
+		if first.DueAt == nil {
 			continue
 		}
-		marker := Marker(it.ID)
+		children := strings.Join(groupChildren(g), ", ")
+		start := first.DueAt.In(time.Local).Format(time.RFC3339)

-		if ev, ok := known[marker]; ok {
-			// Adopt an event this run (or an earlier one) already wrote.
+		if uid, ok := adopt(known, g); ok {
 			res.Skipped++
-			uid := ev.UID
-			if uid == "" {
-				// No id from Home Assistant: keep the item out of the next
-				// candidate set by adopting the marker as its reference.
-				uid = marker
-			}
 			if !opts.DryRun {
-				if err := st.MarkItemCalendarSynced(ctx, it.ID, uid, now); err != nil {
-					log.Error("mark calendar item", "item", it.ID, "err", err)
-				}
-				if _, err := st.RecordCalendarPush(ctx, it.ID, c.Entity(), uid, true, ""); err != nil {
-					log.Error("record calendar push", "item", it.ID, "err", err)
+				for _, it := range g {
+					if err := st.MarkItemCalendarSynced(ctx, it.ID, uid, now); err != nil {
+						log.Error("mark calendar item", "item", it.ID, "err", err)
+					}
 				}
+				recordPush(ctx, st, c.Entity(), g, uid, true, "", log)
 			}
 			res.Events = append(res.Events, Created{
-				ItemID: it.ID, Child: it.Child, Title: it.Title,
-				Start: it.DueAt.In(time.Local).Format(time.RFC3339), UID: uid,
-				Summary: "already in the calendar",
+				ItemID: first.ID, Child: children, Title: first.Title,
+				Start: start, UID: uid, Summary: "already in the calendar",
 			})
 			continue
 		}

 		created := Created{
-			ItemID: it.ID, Child: it.Child, Title: it.Title,
-			Start:  it.DueAt.In(time.Local).Format(time.RFC3339),
-			DryRun: opts.DryRun,
+			ItemID: first.ID, Child: children, Title: first.Title,
+			Start: start, DryRun: opts.DryRun,
 		}
 		if opts.DryRun {
 			res.Created++
@@ -170,33 +178,31 @@ func Sync(ctx context.Context, st *store.Store, c *Client, opts Options, log *sl
 		}

 		uid, err := c.CreateEvent(ctx, Event{
-			Summary:     eventSummary(it),
-			Description: eventDescription(it),
-			Start:       it.DueAt,
-			End:         ptrTime(it.DueAt.Add(duration)),
+			Summary:     eventSummary(g),
+			Description: eventDescription(g),
+			Start:       first.DueAt,
+			End:         ptrTime(first.DueAt.Add(duration)),
 		})
 		if err != nil {
 			res.Failed++
-			msg := fmt.Sprintf("item %d (%s %q): %v", it.ID, it.Child, it.Title, err)
+			msg := fmt.Sprintf("item %d (%s %q): %v", first.ID, children, first.Title, err)
 			res.Errors = append(res.Errors, msg)
-			log.Error("calendar write failed", "item", it.ID, "child", it.Child, "title", it.Title, "entity", c.Entity(), "err", err)
-			if _, rerr := st.RecordCalendarPush(ctx, it.ID, c.Entity(), "", false, err.Error()); rerr != nil {
-				log.Error("record calendar push", "item", it.ID, "err", rerr)
-			}
+			log.Error("calendar write failed", "item", first.ID, "child", children, "title", first.Title, "entity", c.Entity(), "err", err)
+			recordPush(ctx, st, c.Entity(), g, "", false, err.Error(), log)
 			continue
 		}
 		if uid == "" {
 			// Home Assistant did not report an event id. The marker in the
 			// description still makes the next run idempotent, but say so.
 			log.Warn("home assistant returned no event id; relying on the description marker",
-				"item", it.ID, "child", it.Child, "title", it.Title, "entity", c.Entity())
+				"item", first.ID, "child", children, "title", first.Title, "entity", c.Entity())
 		}
-		if err := st.MarkItemCalendarSynced(ctx, it.ID, uid, now); err != nil {
-			log.Error("mark calendar item", "item", it.ID, "err", err)
-		}
-		if _, err := st.RecordCalendarPush(ctx, it.ID, c.Entity(), uid, true, ""); err != nil {
-			log.Error("record calendar push", "item", it.ID, "err", err)
+		for _, it := range g {
+			if err := st.MarkItemCalendarSynced(ctx, it.ID, uid, now); err != nil {
+				log.Error("mark calendar item", "item", it.ID, "err", err)
+			}
 		}
+		recordPush(ctx, st, c.Entity(), g, uid, true, "", log)
 		res.Created++
 		created.UID = uid
 		res.Events = append(res.Events, created)
@@ -206,47 +212,130 @@ func Sync(ctx context.Context, st *store.Store, c *Client, opts Options, log *sl
 	return res, nil
 }

-// eventSummary is the calendar entry title. The child's name is prefixed, so a
-// shared family calendar says who the event belongs to.
-func eventSummary(it store.CalendarItem) string {
-	title := strings.TrimSpace(it.Title)
-	if it.Child != "" {
-		return it.Child + " — " + title
+// recordPush appends one push-log row per item in a group, so the log always
+// names every child whose event was written (FR-12).
+func recordPush(ctx context.Context, st *store.Store, entity string, g []store.CalendarItem, uid string, ok bool, errMsg string, log *slog.Logger) {
+	for _, it := range g {
+		if _, err := st.RecordCalendarPush(ctx, it.ID, entity, uid, ok, errMsg); err != nil {
+			log.Error("record calendar push", "item", it.ID, "err", err)
+		}
+	}
+}
+
+// GroupDuplicates collapses the same school event stored against more than one
+// child into one calendar write. The key is the source, the type and the
+// stable external reference.
+func GroupDuplicates(items []store.CalendarItem) [][]store.CalendarItem {
+	index := make(map[string]int, len(items))
+	var out [][]store.CalendarItem
+	for _, it := range items {
+		key := it.Source + "\x00" + it.Type + "\x00" + it.ExternalRef
+		if i, ok := index[key]; ok {
+			out[i] = append(out[i], it)
+			continue
+		}
+		index[key] = len(out)
+		out = append(out, []store.CalendarItem{it})
+	}
+	return out
+}
+
+// adopt reports the Home Assistant event id when any item in the group already
+// has its event in the calendar, so a re-run never duplicates it.
+func adopt(known map[string]Event, g []store.CalendarItem) (string, bool) {
+	for _, it := range g {
+		ev, ok := known[Marker(it.ID)]
+		if !ok {
+			continue
+		}
+		if ev.UID != "" {
+			return ev.UID, true
+		}
+		// No id from Home Assistant: keep the item out of the next candidate
+		// set by adopting the marker as its reference.
+		return Marker(it.ID), true
+	}
+	return "", false
+}
+
+// groupChildren lists the children that share one event, sorted.
+func groupChildren(g []store.CalendarItem) []string {
+	seen := make(map[string]bool, len(g))
+	var out []string
+	for _, it := range g {
+		if it.Child == "" || seen[it.Child] {
+			continue
+		}
+		seen[it.Child] = true
+		out = append(out, it.Child)
+	}
+	sort.Strings(out)
+	return out
+}
+
+// eventSummary is the calendar entry title. A per-child event carries the
+// child's name; a school-wide event (stored against more than one child)
+// carries just its title, because it is one event for the whole family.
+func eventSummary(g []store.CalendarItem) string {
+	title := strings.TrimSpace(g[0].Title)
+	if children := groupChildren(g); len(children) == 1 {
+		return children[0] + " — " + title
 	}
 	return title
 }

 // eventDescription carries the school detail, the Compass link and the
-// idempotency marker. The marker is what makes a re-run safe.
-func eventDescription(it store.CalendarItem) string {
+// idempotency markers. The markers are what make a re-run safe.
+func eventDescription(g []store.CalendarItem) string {
+	first := g[0]
 	var b strings.Builder
-	body := strings.TrimSpace(it.Body)
-	if body != "" && body != strings.TrimSpace(it.Title) {
+	body := strings.TrimSpace(first.Body)
+	if body != "" && body != strings.TrimSpace(first.Title) {
 		b.WriteString(body)
 		b.WriteString("\n")
 	}
-	if it.URL != "" {
-		b.WriteString(it.URL)
+	if first.URL != "" {
+		b.WriteString(first.URL)
 		b.WriteString("\n")
 	}
-	if it.Subject != "" {
-		b.WriteString("Subject: " + it.Subject + "\n")
+	if first.Subject != "" {
+		b.WriteString("Subject: " + first.Subject + "\n")
+	}
+	if children := groupChildren(g); len(children) > 1 {
+		b.WriteString("Children: " + strings.Join(children, ", ") + "\n")
+	}
+	b.WriteString("\n")
+	for _, it := range g {
+		b.WriteString(ItemMarker(it.ID))
+		b.WriteString("\n")
 	}
-	b.WriteString("\n" + ItemMarker(it.ID))
 	return b.String()
 }

-// indexByMarker maps a marker to the event that carries it.
+// indexByMarker maps every marker in a description to the event that carries it.
 func indexByMarker(events []Event) map[string]Event {
 	out := make(map[string]Event, len(events))
 	for _, ev := range events {
-		if m := findMarker(ev.Description); m != "" {
+		for _, m := range findMarkers(ev.Description) {
 			out[m] = ev
 		}
 	}
 	return out
 }

+// findMarkers returns every compass-nhs marker in a text, in order.
+func findMarkers(text string) []string {
+	var out []string
+	for {
+		m := findMarker(text)
+		if m == "" {
+			return out
+		}
+		out = append(out, m)
+		text = text[strings.Index(text, m)+len(m):]
+	}
+}
+
 // ptrTime returns a pointer to t, so an event end can hold a computed time.
 func ptrTime(t time.Time) *time.Time { return &t }

--- a/internal/calendar/sync_test.go
+++ b/internal/calendar/sync_test.go
@@ -188,31 +188,35 @@ func TestSyncDryRunWritesNothing(t *testing.T) {
 	}
 }

-func TestSyncIgnoresRoutineLessonsAndPastEvents(t *testing.T) {
+func TestSyncWindowIsTodayAndFuture(t *testing.T) {
 	ctx := context.Background()
 	st, childID := testStore(t)
 	now := time.Now()
+	day := StartOfDay(now)
+
 	seedEvent(t, st, childID, "5 - 11HLEGB5 - D8 - LHT", now.Add(3*time.Hour), "normal", "lesson")
-	seedEvent(t, st, childID, "Year 11 Assembly - HALL", now.Add(-2*time.Hour), "high", "past")
+	seedEvent(t, st, childID, "Yesterday excursion - CITY", day.Add(-time.Hour), "high", "yesterday")
+	seedEvent(t, st, childID, "Year 11 Assembly - HALL", day.Add(time.Minute), "high", "today")
+
 	ha := newFakeHA(t, "calendar.school")
 	c := testClient(t, ha.URL, "calendar.school")

-	candidates, err := st.CalendarCandidates(ctx, nil, nil, now, 0)
+	candidates, err := st.CalendarCandidates(ctx, nil, nil, day, 0)
 	if err != nil {
 		t.Fatalf("candidates: %v", err)
 	}
-	// The store selects on importance; the lesson and the past event are
-	// excluded because the parser never marks a lesson high and the past event
-	// is behind `now`.
-	if len(candidates) != 0 {
+	// A routine lesson is never high, and yesterday is behind the window. The
+	// event earlier today stays a candidate: a scan that runs late in the day
+	// must still record today's school events.
+	if len(candidates) != 1 || candidates[0].Title != "Year 11 Assembly - HALL" {
 		t.Fatalf("candidates = %+v", candidates)
 	}
 	res, err := Sync(ctx, st, c, Options{Now: now}, silentLogger())
 	if err != nil {
 		t.Fatalf("Sync: %v", err)
 	}
-	if res.Created != 0 || ha.created() != 0 {
-		t.Fatalf("result = %+v", res)
+	if res.Candidates != 1 || res.Created != 1 || ha.created() != 1 {
+		t.Fatalf("result = %+v (create calls %d)", res, ha.created())
 	}
 }

@@ -236,3 +240,53 @@ func TestSyncReportsAReadFailureInsteadOfWritingBlind(t *testing.T) {
 		t.Errorf("error = %q", res.Errors[0])
 	}
 }
+
+// TestSyncWritesASchoolWideEventOnce proves the shared Compass week calendar is
+// not duplicated: one event stored against both children is written once, and
+// both children's items record the same Home Assistant event id.
+func TestSyncWritesASchoolWideEventOnce(t *testing.T) {
+	ctx := context.Background()
+	st, harryID := testStore(t)
+	if err := st.SeedChildren(ctx, "Finn"); err != nil {
+		t.Fatalf("seed finn: %v", err)
+	}
+	finn, err := st.ChildByName(ctx, "Finn")
+	if err != nil || finn == nil {
+		t.Fatalf("finn: %v %v", finn, err)
+	}
+
+	due := time.Now().Add(4 * time.Hour).UTC().Truncate(time.Minute)
+	harryItem := seedEvent(t, st, harryID, "Year 11 Assembly - HALL - CMA", due, "high", "assembly")
+	finnItem := seedEvent(t, st, finn.ID, "Year 11 Assembly - HALL - CMA", due, "high", "assembly")
+
+	ha := newFakeHA(t, "calendar.school")
+	c := testClient(t, ha.URL, "calendar.school")
+
+	res, err := Sync(ctx, st, c, Options{Now: time.Now()}, silentLogger())
+	if err != nil {
+		t.Fatalf("Sync: %v", err)
+	}
+	if res.Candidates != 1 || res.Created != 1 {
+		t.Fatalf("result = %+v", res)
+	}
+	if ha.created() != 1 {
+		t.Fatalf("school-wide event was written %d times", ha.created())
+	}
+	// A school-wide event carries just its title, not a child's name.
+	if got := str(ha.lastCreate["summary"]); got != "Year 11 Assembly - HALL - CMA" {
+		t.Errorf("summary = %q", got)
+	}
+	uid := res.Events[0].UID
+	if uid == "" {
+		t.Fatal("event written without a Home Assistant id")
+	}
+	for _, id := range []int64{harryItem.ID, finnItem.ID} {
+		ci, err := st.CalendarItemByID(ctx, id)
+		if err != nil || ci == nil {
+			t.Fatalf("item %d: %v %v", id, ci, err)
+		}
+		if ci.HAEventUID != uid {
+			t.Errorf("item %d uid = %q, want %q", id, ci.HAEventUID, uid)
+		}
+	}
+}
--- a/internal/web/server.go
+++ b/internal/web/server.go
@@ -605,7 +605,7 @@ func (s *Server) itemCalendarView(it store.CalendarItem, now time.Time) calendar
 		UID:        it.HAEventUID,
 	}
 	v.Eligible = it.Type == "event" && it.Source == "compass" && it.Importance == "high" &&
-		it.DueAt != nil && it.DueAt.After(now)
+		it.DueAt != nil && !it.DueAt.Before(calendar.StartOfDay(now))
 	if it.HAPushedAt != nil {
 		v.PushedAt = it.HAPushedAt.In(s.cfg.Location).Format("Mon 02 Jan 2006 15:04")
 	}