From 08753d59aba9d37bed945f4bb5b7b7e010fb413d Mon Sep 17 00:00:00 2001 From: Jonathon Leight Date: Thu, 2 Jul 2026 12:47:41 -0400 Subject: [PATCH] Fix maintenance log author name --- internal/store/maintenance.go | 22 ++++-- internal/store/maintenance_test.go | 109 +++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 internal/store/maintenance_test.go diff --git a/internal/store/maintenance.go b/internal/store/maintenance.go index 8f0d93a..a4af879 100644 --- a/internal/store/maintenance.go +++ b/internal/store/maintenance.go @@ -12,8 +12,10 @@ import ( // records of physical service work (antenna swap, battery replacement, site // visit) that would otherwise live only in the builder's head. -// MaintenanceEntry is one logged maintenance record. AuthorName is denormalized -// so the record stays readable after the author leaves (author_id goes NULL). +// MaintenanceEntry is one logged maintenance record. AuthorName is resolved live +// at read time (ListMaintenance) from the author's current display name/username, +// falling back to the write-time snapshot only once the author is gone +// (author_id goes NULL) — the snapshot is that deleted-author tombstone. type MaintenanceEntry struct { ID int64 AuthorID *int64 @@ -38,11 +40,19 @@ func (s *Store) AddMaintenanceEntry(ctx context.Context, repeaterID, authorID in // ListMaintenance returns a repeater's maintenance history, most recent first. func (s *Store) ListMaintenance(ctx context.Context, repeaterID int64) ([]MaintenanceEntry, error) { + // Resolve the author's *current* name (display name, else username) via the + // live users row; fall back to the denormalized snapshot only when the author + // has been deleted (author_id NULL → the join yields no user). LEFT JOIN + // because author_id is nullable. Matches the name expression used across the + // codebase (see repeaterSelect's corroborators, orgs.go member listings). rows, err := s.pool.Query(ctx, ` - SELECT id, author_id, author_name, note, performed_at, created_at - FROM repeater_maintenance - WHERE repeater_id = $1 - ORDER BY performed_at DESC, id DESC`, repeaterID) + SELECT m.id, m.author_id, + COALESCE(NULLIF(u.display_name, ''), u.username, NULLIF(m.author_name, '')) AS author_name, + m.note, m.performed_at, m.created_at + FROM repeater_maintenance m + LEFT JOIN users u ON u.id = m.author_id + WHERE m.repeater_id = $1 + ORDER BY m.performed_at DESC, m.id DESC`, repeaterID) if err != nil { return nil, fmt.Errorf("list maintenance: %w", err) } diff --git a/internal/store/maintenance_test.go b/internal/store/maintenance_test.go new file mode 100644 index 0000000..0dc9a84 --- /dev/null +++ b/internal/store/maintenance_test.go @@ -0,0 +1,109 @@ +package store + +import ( + "strings" + "testing" + "time" +) + +// TestListMaintenanceUsesLiveAuthorName is the regression for the maintenance +// page showing a *stale* author name: ListMaintenance must resolve the author's +// current display name, not the name snapshotted into author_name at write +// time. It also covers the username fallback when the display name is cleared. +func TestListMaintenanceUsesLiveAuthorName(t *testing.T) { + t.Parallel() + st, ctx := orgTestStore(t) + + author, err := st.CreateUser(ctx, "alice", "Alice One") + if err != nil { + t.Fatal(err) + } + rep, err := st.CreateRepeater(ctx, &Repeater{ + OwnerID: author.ID, Name: "R", PublicKeyHex: strings.Repeat("a", 64), + RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5, + }) + if err != nil { + t.Fatal(err) + } + + // Snapshot the name at write time, exactly as handleAddMaintenance does. + if err := st.AddMaintenanceEntry(ctx, rep.ID, author.ID, "Alice One", "swapped antenna", time.Now()); err != nil { + t.Fatal(err) + } + + nameNow := func() string { + t.Helper() + es, err := st.ListMaintenance(ctx, rep.ID) + if err != nil { + t.Fatal(err) + } + if len(es) != 1 { + t.Fatalf("want 1 entry, got %d", len(es)) + } + return es[0].AuthorName + } + + if got := nameNow(); got != "Alice One" { + t.Fatalf("initial: got %q, want %q", got, "Alice One") + } + + // Rename the author. The entry must now reflect the NEW display name — this + // is the bug: before the fix ListMaintenance returns the stale "Alice One". + if err := st.SetDisplayName(ctx, author.ID, "Alice Renamed"); err != nil { + t.Fatal(err) + } + if got := nameNow(); got != "Alice Renamed" { + t.Fatalf("after rename: got %q, want %q (stale snapshot returned)", got, "Alice Renamed") + } + + // Clearing the display name falls back to the username, matching User.Name(). + if err := st.SetDisplayName(ctx, author.ID, ""); err != nil { + t.Fatal(err) + } + if got := nameNow(); got != "alice" { + t.Fatalf("after clearing display name: got %q, want username %q", got, "alice") + } +} + +// TestListMaintenanceDeletedAuthorFallsBackToSnapshot verifies the denormalized +// author_name still shows once the author row is gone (author_id NULL) — the +// tombstone the column exists for. There is no DeleteUser store method, so the +// NULL-author row is inserted directly. +func TestListMaintenanceDeletedAuthorFallsBackToSnapshot(t *testing.T) { + t.Parallel() + st, ctx := orgTestStore(t) + + owner, err := st.CreateUser(ctx, "owner", "") + if err != nil { + t.Fatal(err) + } + rep, err := st.CreateRepeater(ctx, &Repeater{ + OwnerID: owner.ID, Name: "R", PublicKeyHex: strings.Repeat("b", 64), + RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5, + }) + if err != nil { + t.Fatal(err) + } + + // A maintenance row whose author has since been deleted: author_id NULL, + // author_name retained as the readable tombstone. + if _, err := st.pool.Exec(ctx, ` + INSERT INTO repeater_maintenance (repeater_id, author_id, author_name, note) + VALUES ($1, NULL, $2, $3)`, rep.ID, "Departed Builder", "site visit"); err != nil { + t.Fatal(err) + } + + es, err := st.ListMaintenance(ctx, rep.ID) + if err != nil { + t.Fatal(err) + } + if len(es) != 1 { + t.Fatalf("want 1 entry, got %d", len(es)) + } + if es[0].AuthorID != nil { + t.Fatalf("author_id: got %d, want nil", *es[0].AuthorID) + } + if es[0].AuthorName != "Departed Builder" { + t.Fatalf("deleted-author fallback: got %q, want %q", es[0].AuthorName, "Departed Builder") + } +}