diff --git a/docs/phase-6b7b-notes.md b/docs/phase-6b7b-notes.md index cbb64be..0262c32 100644 --- a/docs/phase-6b7b-notes.md +++ b/docs/phase-6b7b-notes.md @@ -36,18 +36,24 @@ hour moves with the day's first event (07:00 on one page, 06:00 on another). ## 2. The decode map (FR-17) `codes` (migration `005_codes.sql`) is the editable map: `subject`, `room`, -`teacher` and `period`. It is seeded on first read and never overwrites a -correction. An unknown code renders as the **raw code**, never blank. +`teacher` and `period`. It is seeded **once**, while the table is empty, and never +overwrites or restores a row after that, so a correction or a delete sticks. An +unknown code renders as the **raw code**, never blank. + +The same map decodes the timetable **events**, not only the lesson periods, so +an ensemble reads in plain English too: ``` -=== FINN === -PERIODS TODAY -- Period 1 — Year 7 Science — Room H14LAB — Mr KNI (08:55) -... -- Period 4 — Year 7 Physical Education — Gym 4 — Mr LTA (13:26) +EVENTS TODAY +- JAZZ ENSEMBLE — Ensemble Room — Mr KWO (07:45) +- JUNIOR GUITAR ENSEMBLE — Theatre — Mr HHL / Mr OFG (07:45) ``` -Sam edits it without a rebuild: +Sam edits it without a rebuild, **in the dashboard** or by CLI: + +- dashboard: `/codes` (linked as **Codes** in the top bar) lists every mapping by + kind with an Add form per kind, an inline Edit + Save per row, and Delete. The + page sits behind the same session login as the dashboard. ```bash docker exec compass-teams-nhs /app/school codes list @@ -56,9 +62,9 @@ docker exec compass-teams-nhs /app/school codes delete --kind room --code Z9 ``` Teacher rows are placeholders (`KNI` -> `Mr KNI`) because Compass gives only -initials; Sam replaces them with surnames. Real unknown-code fallback from the -live brief: Harry's period 2 teacher field is `GT AGE_13`, and `AGE_13` is not -mapped, so it renders raw (`Mr GT / AGE_13`). +initials; Sam replaces them with surnames in the dashboard. Real unknown-code +fallback from the live brief: Harry's period 2 teacher field is `GT AGE_13`, and +`AGE_13` is not mapped, so it renders raw (`Mr GT / AGE_13`). Seed size: 65 rows — 23 subjects, 22 teachers, 18 rooms, 2 period labels. @@ -223,6 +229,8 @@ internal/parse/daywidget.go the per-child "Today" widget internal/parse/parse.go, calendar.go, importance.go per-child hook, week-calendar filter internal/dashboard/dashboard.go brief buckets + the required 4-section renderers internal/brief/brief.go Finn-first order, codes decoder, new renderers +internal/web/server.go /codes page + save/delete handlers +internal/web/templates/codes.html the dashboard decode-map editor internal/calendar/sync.go periods + due + overdue + events, decoded, once internal/calendar/ha.go Event JSON exposes start/end for evidence cmd/school/{main,parse,calendar,codes,items,notify}.go wiring, codes CLI, purge by page @@ -240,4 +248,5 @@ docker-compose.yml, Makefile image 0.6.2-phase6b7b - The **week calendar** is still not fetched by the daily scan, so a *new* major event is only written if a run fetches that page. The two existing school-wide events remain in the calendar. -- **Teacher names** are placeholders; only Sam can supply the surnames. +- **Teacher names** are placeholders; Sam supplies the surnames in the new + `/codes` dashboard page. Until then they render as initials. diff --git a/internal/brief/brief.go b/internal/brief/brief.go index ad25bb1..cc7f9d7 100644 --- a/internal/brief/brief.go +++ b/internal/brief/brief.go @@ -138,14 +138,15 @@ func (s *Service) ChildrenText(ctx context.Context, child store.Child) (string, return notice + "\n" + text, false, nil } -// decoder loads the editable codes map and returns the lesson-title decoder the -// brief uses (FR-17). An unknown code renders raw, never blank. +// decoder loads the editable codes map and returns the label decoder the brief +// uses (FR-17). It decodes lesson periods and timetable events; an unknown code +// renders raw, never blank. func (s *Service) decoder(ctx context.Context) (func(string) string, error) { codes, err := s.Store.Codes(ctx) if err != nil { return nil, fmt.Errorf("decode map: %w", err) } - return func(title string) string { return decode.Period(title, codes) }, nil + return func(title string) string { return decode.Label(title, codes) }, nil } // CombinedText returns the single brief for Jo and Sam — both boys in one diff --git a/internal/dashboard/dashboard.go b/internal/dashboard/dashboard.go index 5816e11..4c5add1 100644 --- a/internal/dashboard/dashboard.go +++ b/internal/dashboard/dashboard.go @@ -419,7 +419,7 @@ func RenderBrief(child string, b Buckets, decode func(string) string, loc *time. fmt.Fprintf(&sb, "=== %s ===\n", strings.ToUpper(strings.TrimSpace(child))) writeBriefPeriods(&sb, b.PeriodsToday, decode, loc) - writeBriefEvents(&sb, b.EventsToday, loc) + writeBriefEvents(&sb, b.EventsToday, decode, loc) writeBriefWork(&sb, "OVERDUE", b.Overdue, loc) writeBriefFuture(&sb, b, loc) return sb.String() @@ -455,19 +455,25 @@ func writeBriefPeriods(sb *strings.Builder, items []store.Item, decode func(stri } } -// writeBriefEvents lists today's major school events (FR-9b). -func writeBriefEvents(sb *strings.Builder, items []store.Item, loc *time.Location) { +// writeBriefEvents lists today's major school events (FR-9b), decoded through +// the same map as periods so an ensemble or a concert reads as plain English +// (FR-17). Unknown codes stay raw. +func writeBriefEvents(sb *strings.Builder, items []store.Item, decode func(string) string, loc *time.Location) { sb.WriteString("\nEVENTS TODAY\n") if len(items) == 0 { sb.WriteString("- none\n") return } for _, it := range items { + title := strings.TrimSpace(it.Title) + if decode != nil { + title = decode(title) + } when := "" if it.DueAt != nil { when = " (" + it.DueAt.In(loc).Format("15:04") + ")" } - fmt.Fprintf(sb, "- %s%s\n", strings.TrimSpace(it.Title), when) + fmt.Fprintf(sb, "- %s%s\n", title, when) } } diff --git a/internal/decode/decode.go b/internal/decode/decode.go index 6a93267..e1ff78e 100644 --- a/internal/decode/decode.go +++ b/internal/decode/decode.go @@ -38,11 +38,12 @@ func Period(title string, m store.CodeMap) string { } // teacherName decodes a teacher field. Compass sometimes carries several codes -// in the one field ("GT SMH"), so every token is decoded and joined. +// in the one field, separated by spaces ("GT SMH") or commas ("HHL, OFG"), so +// every token is decoded and joined. An unmapped token stays raw (FR-17). func teacherName(m store.CodeMap, field string) string { - tokens := strings.Fields(field) - if len(tokens) <= 1 { - return codeName(m, store.CodeTeacher, field) + tokens := teacherTokens(field) + if len(tokens) == 0 { + return strings.TrimSpace(field) } out := make([]string, 0, len(tokens)) for _, tok := range tokens { @@ -51,6 +52,43 @@ func teacherName(m store.CodeMap, field string) string { return strings.Join(out, " / ") } +// teacherTokens splits a teacher field on commas and whitespace. +func teacherTokens(field string) []string { + return strings.Fields(strings.ReplaceAll(field, ",", " ")) +} + +// Label renders any Compass timetable label as plain English: a lesson label +// through Period, anything else (an ensemble, a concert, an excursion) through +// Event. It is the decoder the briefs use. An unmapped code stays raw (FR-17). +func Label(title string, m store.CodeMap) string { + if _, _, _, _, ok := period.Split(strings.TrimSpace(title)); ok { + return Period(title, m) + } + return Event(title, m) +} + +// Event renders a timetable event label as plain English: +// +// "JUNIOR GUITAR ENSEMBLE - THEATRE - HHL, OFG" -> "JUNIOR GUITAR ENSEMBLE — Theatre — Mr HHL / Mr OFG" +// +// The event name is kept as the school wrote it; the room and teacher codes are +// decoded. A label with no room or teacher is returned with just its name, and +// an unmapped code stays raw (FR-17). +func Event(title string, m store.CodeMap) string { + parts := period.SplitDash(strings.TrimSpace(title)) + if len(parts) <= 1 { + return strings.TrimSpace(title) + } + out := []string{parts[0]} + if parts[1] != "" { + out = append(out, codeName(m, store.CodeRoom, parts[1])) + } + if len(parts) >= 3 && parts[2] != "" { + out = append(out, teacherName(m, parts[2])) + } + return strings.Join(out, " — ") +} + // periodName renders the period slot: a word label such as "C" uses the map // ("Connect"), a number becomes "Period 3", and anything unknown stays raw. func periodName(slot string, m store.CodeMap) string { diff --git a/internal/decode/decode_test.go b/internal/decode/decode_test.go index 6aac647..a56efe0 100644 --- a/internal/decode/decode_test.go +++ b/internal/decode/decode_test.go @@ -50,3 +50,31 @@ func TestPeriodLeavesEventsAlone(t *testing.T) { t.Errorf("Period = %q, want the event unchanged", got) } } + +// TestLabelDecodesEventsAndLessons proves FR-17 covers timetable events too: an +// ensemble or a concert reads as plain English, its room and teachers decoded, +// while the event name is kept as the school wrote it. A single-part label is +// returned unchanged and an unmapped code stays raw. +func TestLabelDecodesEventsAndLessons(t *testing.T) { + codes := testCodes() + codes.Room["THEATRE"] = "Theatre" + codes.Room["ENS"] = "Ensemble Room" + codes.Room["IM3"] = "Room IM3" + codes.Teacher["HHL"] = "Mr HHL" + codes.Teacher["OFG"] = "Mr OFG" + codes.Teacher["KWO"] = "Mr KWO" + + cases := map[string]string{ + "1 - 7SCF - H14LAB - KNI": "Period 1 — Year 7 Science — Room H14LAB — Mr KNI", + "JUNIOR GUITAR ENSEMBLE - THEATRE - HHL, OFG": "JUNIOR GUITAR ENSEMBLE — Theatre — Mr HHL / Mr OFG", + "JAZZ ENSEMBLE - ENS - KWO": "JAZZ ENSEMBLE — Ensemble Room — Mr KWO", + "HHL GUITAR E - IM3 - HHL": "HHL GUITAR E — Room IM3 — Mr HHL", + "Curriculum Day": "Curriculum Day", + "Concert - HALL": "Concert — HALL", + } + for raw, want := range cases { + if got := Label(raw, codes); got != want { + t.Errorf("Label(%q) = %q, want %q", raw, got, want) + } + } +} diff --git a/internal/store/codes.go b/internal/store/codes.go index 123775c..7451787 100644 --- a/internal/store/codes.go +++ b/internal/store/codes.go @@ -134,10 +134,18 @@ var seedCodes = []struct { {CodePeriod, "RLE GROUP D", "RLE Group D"}, } -// SeedCodes inserts the shipped mappings that are not present yet. It is -// idempotent and never overwrites a value, so Sam's corrections survive every -// start and every scan. +// SeedCodes inserts the shipped mappings once, when the map is still empty. It +// never overwrites a value, so Sam's corrections survive every start and every +// scan — and because it is a one-time bootstrap, a mapping Sam deletes in the +// dashboard stays deleted instead of reappearing on the next read. func (s *Store) SeedCodes(ctx context.Context) error { + var n int + if err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM codes`).Scan(&n); err != nil { + return fmt.Errorf("seed codes: count: %w", err) + } + if n > 0 { + return nil + } tx, err := s.db.BeginTx(ctx, nil) if err != nil { return fmt.Errorf("seed codes: begin: %w", err) @@ -157,8 +165,9 @@ func (s *Store) SeedCodes(ctx context.Context) error { return nil } -// Codes returns the whole decode map. It seeds the shipped defaults first, so a -// fresh database renders the known captures and a corrected row is kept. +// Codes returns the whole decode map. It seeds the shipped defaults first (once) +// so a fresh database renders the known captures, and a corrected or deleted row +// is kept. func (s *Store) Codes(ctx context.Context) (CodeMap, error) { if err := s.SeedCodes(ctx); err != nil { return CodeMap{}, err diff --git a/internal/web/server.go b/internal/web/server.go index dcfba29..36b9d28 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -79,6 +79,9 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("GET /items/{id}", s.requireAuth(s.handleItemDetail)) mux.HandleFunc("POST /items/{id}/delete", s.requireAuth(s.handleItemDelete)) mux.HandleFunc("POST /items/{id}/calendar", s.requireAuth(s.handleItemCalendarPush)) + mux.HandleFunc("GET /codes", s.requireAuth(s.handleCodesPage)) + mux.HandleFunc("POST /codes", s.requireAuth(s.handleCodeSave)) + mux.HandleFunc("POST /codes/delete", s.requireAuth(s.handleCodeDelete)) mux.HandleFunc("POST /summaries/build", s.requireAuth(s.handleSummaryBuild)) mux.HandleFunc("POST /summaries/send", s.requireAuth(s.handleSendBrief)) mux.HandleFunc("POST /summaries/{id}/send", s.requireAuth(s.handleSummarySend)) @@ -1421,6 +1424,139 @@ func pathID(r *http.Request, name string) (int64, bool) { return v, true } +// ---------- codes (the editable decode map, FR-17) ---------- + +// codeKinds is the page order and the plain-English label for the four decode +// groups. +var codeKinds = []struct { + Kind string + Label string + Hint string +}{ + {store.CodeSubject, "Subjects", "e.g. 7SCF → Year 7 Science. New subject codes appear each term."}, + {store.CodeTeacher, "Teachers", "Compass gives initials only. Put the surname here, e.g. KNI → Mr Knight. Unknown initials stay raw."}, + {store.CodeRoom, "Rooms", "e.g. B11 → Room B11, GYM4 → Gym 4."}, + {store.CodePeriod, "Period labels", "Word slots only, e.g. C → Connect. A number is always “Period N”."}, +} + +// codesData renders the editable decode map (AGENTS.md §4 FR-17). The map is +// data, not code: a correction here changes the very next brief. +type codesData struct { + Title string + Username string + CSRF string + Version string + AppEnv string + Groups []codeGroup + ErrMsg string + Flash string + FlashLevel string +} + +type codeGroup struct { + Kind string + Label string + Hint string + Codes []codeRow +} + +type codeRow struct { + Kind string + Code string + Name string +} + +// handleCodesPage lists every mapping so Sam can correct it in the dashboard. +func (s *Server) handleCodesPage(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) + defer cancel() + + data := codesData{ + Title: "Codes", + Username: s.cfg.AdminUsername, + CSRF: s.auth.EnsureCSRF(w, r), + Version: s.cfg.Version, + AppEnv: s.cfg.AppEnv, + } + // Same flash contract as the dashboard: ?msg=… is the text, ?level=error + // makes it red. + if msg := strings.TrimSpace(r.URL.Query().Get("msg")); msg != "" { + data.Flash = msg + data.FlashLevel = "ok" + } + if strings.TrimSpace(r.URL.Query().Get("level")) == "error" { + data.FlashLevel = "error" + } + + rows, err := s.store.ListCodes(ctx) + if err != nil { + s.log.Error("list codes", "err", err) + data.ErrMsg = "Could not read the codes: " + err.Error() + } + byKind := map[string][]codeRow{} + for _, c := range rows { + byKind[c.Kind] = append(byKind[c.Kind], codeRow{Kind: c.Kind, Code: c.Code, Name: c.Name}) + } + for _, k := range codeKinds { + data.Groups = append(data.Groups, codeGroup{Kind: k.Kind, Label: k.Label, Hint: k.Hint, Codes: byKind[k.Kind]}) + } + s.render(w, r, "codes.html", data) +} + +// handleCodeSave creates or corrects one mapping (FR-17). +func (s *Server) handleCodeSave(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) + defer cancel() + + if err := r.ParseForm(); err != nil { + http.Error(w, "bad request", http.StatusBadRequest) + return + } + if !s.auth.CheckCSRF(r) { + s.log.Warn("code save rejected: bad csrf token") + http.Error(w, "session expired", http.StatusForbidden) + return + } + row, err := s.store.SetCode(ctx, + strings.TrimSpace(r.PostFormValue("kind")), + strings.TrimSpace(r.PostFormValue("code")), + strings.TrimSpace(r.PostFormValue("name"))) + if err != nil { + s.redirectAfterWrite(w, r, "/codes?level=error&msg="+url.QueryEscape("Not saved: "+err.Error())) + return + } + s.log.Info("code saved", "kind", row.Kind, "code", row.Code, "name", row.Name) + s.redirectAfterWrite(w, r, "/codes?msg="+url.QueryEscape("Saved "+row.Kind+" "+row.Code+" → "+row.Name)) +} + +// handleCodeDelete removes one mapping, so the code falls back to raw. +func (s *Server) handleCodeDelete(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) + defer cancel() + + if err := r.ParseForm(); err != nil { + http.Error(w, "bad request", http.StatusBadRequest) + return + } + if !s.auth.CheckCSRF(r) { + s.log.Warn("code delete rejected: bad csrf token") + http.Error(w, "session expired", http.StatusForbidden) + return + } + kind := strings.TrimSpace(r.PostFormValue("kind")) + code := strings.TrimSpace(r.PostFormValue("code")) + ok, err := s.store.DeleteCode(ctx, kind, code) + switch { + case err != nil: + s.redirectAfterWrite(w, r, "/codes?level=error&msg="+url.QueryEscape("Not removed: "+err.Error())) + case !ok: + s.redirectAfterWrite(w, r, "/codes?level=error&msg="+url.QueryEscape("No such code: "+kind+" "+code)) + default: + s.log.Info("code deleted", "kind", kind, "code", code) + s.redirectAfterWrite(w, r, "/codes?msg="+url.QueryEscape("Removed "+kind+" "+code+"; it renders raw again.")) + } +} + // ---------- partials ---------- func (s *Server) handleScanStatusPartial(w http.ResponseWriter, r *http.Request) { diff --git a/internal/web/server_test.go b/internal/web/server_test.go index 87bd932..50dd0b2 100644 --- a/internal/web/server_test.go +++ b/internal/web/server_test.go @@ -875,3 +875,91 @@ func TestMorningEndpointDryRun(t *testing.T) { t.Errorf("a dry run must not record deliveries: %+v", rows) } } + +// TestCodesPageEditsDecodeMap proves FR-17 in the dashboard: Sam can see the +// decode map, correct a teacher initial to a surname, and remove a mapping. +// A corrected or deleted mapping must stick, not reappear from the seed. +func TestCodesPageEditsDecodeMap(t *testing.T) { + ts, st := newTestServer(t) + client := login(t, ts) + + page := getBody(t, client, ts.URL+"/codes") + if !strings.Contains(page, "KNI") || !strings.Contains(page, "Teachers") { + t.Fatalf("codes page does not list the seeded teacher codes") + } + token := csrfToken(t, page) + + // Correct an initial to a surname. + resp, err := client.PostForm(ts.URL+"/codes", url.Values{ + "csrf": {token}, "kind": {"teacher"}, "code": {"KNI"}, "name": {"Mr Knight"}, + }) + if err != nil { + t.Fatalf("save: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("save status = %d, want 303", resp.StatusCode) + } + codes, err := st.Codes(context.Background()) + if err != nil { + t.Fatalf("codes: %v", err) + } + if codes.Teacher["KNI"] != "Mr Knight" { + t.Errorf("teacher KNI = %q, want Mr Knight", codes.Teacher["KNI"]) + } + + // An empty name is refused and the mapping is unchanged. + resp, err = client.PostForm(ts.URL+"/codes", url.Values{ + "csrf": {token}, "kind": {"teacher"}, "code": {"KNI"}, "name": {""}, + }) + if err != nil { + t.Fatalf("bad save: %v", err) + } + resp.Body.Close() + if loc := resp.Header.Get("Location"); !strings.Contains(loc, "level=error") { + t.Errorf("bad save Location = %q, want an error flash", loc) + } + if codes, _ := st.Codes(context.Background()); codes.Teacher["KNI"] != "Mr Knight" { + t.Errorf("the refused save changed the mapping to %q", codes.Teacher["KNI"]) + } + + // A bad CSRF token is refused. + resp, err = client.PostForm(ts.URL+"/codes", url.Values{ + "csrf": {"wrong"}, "kind": {"teacher"}, "code": {"KNI"}, "name": {"Mr Nope"}, + }) + if err != nil { + t.Fatalf("csrf save: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("bad csrf status = %d, want 403", resp.StatusCode) + } + + // Delete returns the code to raw, and it stays deleted. + resp, err = client.PostForm(ts.URL+"/codes/delete", url.Values{ + "csrf": {token}, "kind": {"teacher"}, "code": {"KNI"}, + }) + if err != nil { + t.Fatalf("delete: %v", err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusSeeOther { + t.Fatalf("delete status = %d, want 303", resp.StatusCode) + } + if codes, _ := st.Codes(context.Background()); codes.Teacher["KNI"] != "" { + t.Errorf("teacher KNI still mapped after delete: %q", codes.Teacher["KNI"]) + } + + // The page is behind the session login. + anon := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }} + anonResp, err := anon.Get(ts.URL + "/codes") + if err != nil { + t.Fatalf("anon get: %v", err) + } + anonResp.Body.Close() + if anonResp.StatusCode != http.StatusSeeOther { + t.Errorf("anonymous /codes = %d, want a redirect to login", anonResp.StatusCode) + } +} diff --git a/internal/web/templates/codes.html b/internal/web/templates/codes.html new file mode 100644 index 0000000..7178130 --- /dev/null +++ b/internal/web/templates/codes.html @@ -0,0 +1,88 @@ +{{define "codes.html"}} + + + + + + {{.Title}} — Compass / Teams + + + + + + + +
+
+

Codes

+

Plain-English names for the Compass codes in the briefs. + A code that is not here renders as the raw code — never blank.

+
+ + {{if .ErrMsg}}{{end}} + {{if .Flash}}

{{.Flash}}

{{end}} + + {{range .Groups}} +
+
+

{{.Label}}

+ {{len .Codes}} +
+

{{.Hint}}

+ +
+ + + + + +
+ + {{if .Codes}} + + {{else}} +

No codes yet. Add one above.

+ {{end}} +
+ {{end}} +
+ + + +{{end}} diff --git a/internal/web/templates/dashboard.html b/internal/web/templates/dashboard.html index de4fddc..1c3aa21 100644 --- a/internal/web/templates/dashboard.html +++ b/internal/web/templates/dashboard.html @@ -17,6 +17,7 @@ Compass / Teams
+ Codes {{.Username}}
diff --git a/internal/web/templates/item.html b/internal/web/templates/item.html index 0a566a8..0becbde 100644 --- a/internal/web/templates/item.html +++ b/internal/web/templates/item.html @@ -17,6 +17,7 @@ Compass / Teams
+ Codes {{.Username}}