Fix maintenance log author name

This commit is contained in:
Jonathon Leight
2026-07-02 12:47:41 -04:00
parent 0b54870fa1
commit 08753d59ab
2 changed files with 125 additions and 6 deletions
+16 -6
View File
@@ -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)
}
+109
View File
@@ -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")
}
}