From 488f0b9a9ba0897d8ea1fd7ddcbd363c9db78237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=27Kama=C5=9B=27=20Bruchal?= Date: Wed, 19 Aug 2026 14:44:26 +0200 Subject: [PATCH 1/2] feat: ignore unknown commands instead of replying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bot now answers only its predefined commands and stays silent on anything else, logging ignored commands at debug level. Tests no longer assert exact reply/reminder wording (which broke on copy tweaks like 37a81a2) — they check behavior instead: message counts, the names/handles substituted in, and that /sync persists its fetch and doesn't leak internal errors. --- README.md | 3 ++ scheduler_test.go | 123 ++++++++++++++-------------------------------- telegram.go | 14 +++--- telegram_test.go | 105 ++++++++++++--------------------------- 4 files changed, 80 insertions(+), 165 deletions(-) diff --git a/README.md b/README.md index 3ec7910..07dc7bf 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,9 @@ across restarts. - `/sync` — refresh the rota from Google Sheets now (throttled to once per minute across the chat to avoid spamming the Sheets API). +Any other command is ignored — the bot stays silent instead of replying +"unknown command". + ### Reminders If `GROUP_CHAT_ID` is set, the bot posts a weekly duty reminder to that chat. diff --git a/scheduler_test.go b/scheduler_test.go index 95db13f..3ead144 100644 --- a/scheduler_test.go +++ b/scheduler_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "math/rand" + "strings" "testing" "time" ) @@ -59,7 +60,6 @@ func TestFire(t *testing.T) { return DutyWeek{WeekStart: day(t, start), Person1: "X", Person2: "Y", Row: row} } thisWeekFilled := DutyWeek{WeekStart: day(t, "2026-06-15"), Person1: "Jan", Person2: "Anna", Row: 5} - wantThisReminder := "Przypomnienie: w tygodniu od 2026-06-15 dyżur mają Jan i Anna. Z góry dziękujemy!" t.Run("this week empty -> reminder only, never assigns", func(t *testing.T) { rota := Rota{ @@ -76,9 +76,8 @@ func TestFire(t *testing.T) { if len(h.assigns) != 0 { t.Errorf("assigns = %d, want 0 (this week is never auto-assigned)", len(h.assigns)) } - want := "Przypomnienie: w tygodniu od 2026-06-15 dyżur jest nieobsadzony." - if len(h.sends) != 1 || h.sends[0] != want { - t.Errorf("sends = %q, want [%q]", h.sends, want) + if len(h.sends) != 1 { + t.Errorf("sends = %q, want exactly the this-week reminder", h.sends) } }) @@ -101,9 +100,9 @@ func TestFire(t *testing.T) { if a.p1 != "Ala" || a.p2 != "Bartek" || a.week.Row != 6 { t.Errorf("assign = %+v, want Ala/Bartek on row 6", a) } - wantAnnounce := "Nikt nie zgłosił się na dyżur w tygodniu od 2026-06-22, więc został przydzielony losowo: Ala i Bartek. Z góry dziękujemy za sprzątanie!" - if len(h.sends) != 2 || h.sends[0] != wantThisReminder || h.sends[1] != wantAnnounce { - t.Errorf("sends = %q,\n want [%q, %q]", h.sends, wantThisReminder, wantAnnounce) + if len(h.sends) != 2 || + !strings.Contains(h.sends[1], "Ala") || !strings.Contains(h.sends[1], "Bartek") { + t.Errorf("sends = %q, want reminder + announcement naming Ala and Bartek", h.sends) } }) @@ -122,9 +121,8 @@ func TestFire(t *testing.T) { if len(h.assigns) != 0 { t.Errorf("assigns = %d, want 0 (warning only, no assign at +2)", len(h.assigns)) } - wantWarning := "Dyżur w tygodniu od 2026-06-29 jest jeszcze nieobsadzony. Proszę się zgłaszać — w przeciwnym razie zostanie przydzielony losowo." - if len(h.sends) != 2 || h.sends[0] != wantThisReminder || h.sends[1] != wantWarning { - t.Errorf("sends = %q,\n want [%q, %q]", h.sends, wantThisReminder, wantWarning) + if len(h.sends) != 2 { + t.Errorf("sends = %q, want reminder + warning", h.sends) } }) @@ -143,8 +141,8 @@ func TestFire(t *testing.T) { if len(h.assigns) != 0 { t.Errorf("assigns = %d, want 0", len(h.assigns)) } - if len(h.sends) != 1 || h.sends[0] != wantThisReminder { - t.Errorf("sends = %q, want [%q]", h.sends, wantThisReminder) + if len(h.sends) != 1 { + t.Errorf("sends = %q, want only the this-week reminder", h.sends) } }) @@ -163,8 +161,8 @@ func TestFire(t *testing.T) { if len(h.assigns) != 0 { t.Errorf("assigns = %d, want 0 (unwritable row)", len(h.assigns)) } - if len(h.sends) != 1 || h.sends[0] != wantThisReminder { - t.Errorf("sends = %q, want [%q]", h.sends, wantThisReminder) + if len(h.sends) != 1 { + t.Errorf("sends = %q, want only the this-week reminder", h.sends) } }) @@ -184,8 +182,8 @@ func TestFire(t *testing.T) { if len(h.assigns) != 1 { t.Errorf("assigns = %d, want 1 (attempted)", len(h.assigns)) } - if len(h.sends) != 1 || h.sends[0] != wantThisReminder { - t.Errorf("sends = %q, want [%q] (no announce on write failure)", h.sends, wantThisReminder) + if len(h.sends) != 1 { + t.Errorf("sends = %q, want only the this-week reminder (no announce on write failure)", h.sends) } }) @@ -204,8 +202,8 @@ func TestFire(t *testing.T) { if len(h.assigns) != 0 { t.Errorf("assigns = %d, want 0 (partial week is not empty)", len(h.assigns)) } - if len(h.sends) != 1 || h.sends[0] != wantThisReminder { - t.Errorf("sends = %q, want [%q]", h.sends, wantThisReminder) + if len(h.sends) != 1 { + t.Errorf("sends = %q, want only the this-week reminder", h.sends) } }) } @@ -354,54 +352,23 @@ func TestNextFireAcrossDST(t *testing.T) { } } +// TestReminderText checks only the match/skip behavior — a target inside a +// rota week yields a reminder, one outside skips. The wording itself is not +// asserted, so copy tweaks don't break the test. func TestReminderText(t *testing.T) { - // Week of 2026-06-29; target lands inside it. - target := day(t, "2026-06-30") - rota := func(p1, p2 string) Rota { - return Rota{Weeks: []DutyWeek{week(t, "2026-06-29", p1, p2)}} - } + rota := Rota{Weeks: []DutyWeek{week(t, "2026-06-29", "Ala", "Bartek")}} - tests := []struct { - name string - rota Rota - target time.Time - wantOK bool - want string - }{ - { - "both people", - rota("Ala", "Bartek"), target, true, - "Przypomnienie: w tygodniu od 2026-06-29 dyżur mają Ala i Bartek. Z góry dziękujemy!", - }, - { - "one person", - rota("Ala", ""), target, true, - "Przypomnienie: w tygodniu od 2026-06-29 dyżur ma Ala. Z góry dziękujemy!", - }, - { - "nobody assigned", - rota("", ""), target, true, - "Przypomnienie: w tygodniu od 2026-06-29 dyżur jest nieobsadzony.", - }, - { - "no matching week -> skip", - rota("Ala", "Bartek"), day(t, "2026-09-01"), false, - "", - }, + if _, ok := reminderText(rota, day(t, "2026-06-30")); !ok { + t.Error("target inside a rota week should yield a reminder") } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got, ok := reminderText(tc.rota, tc.target) - if ok != tc.wantOK { - t.Fatalf("ok = %v, want %v", ok, tc.wantOK) - } - if ok && got != tc.want { - t.Errorf("reminderText = %q, want %q", got, tc.want) - } - }) + if _, ok := reminderText(rota, day(t, "2026-09-01")); ok { + t.Error("target outside all rota weeks should skip") } } +// TestReminderTextMentions checks the handle substitution — names resolve to +// @handles when known (normalizing a missing leading @) and stay plain names +// otherwise. Only the mention tokens are asserted, not the full wording. func TestReminderTextMentions(t *testing.T) { target := day(t, "2026-06-30") people := []Person{ @@ -414,30 +381,14 @@ func TestReminderTextMentions(t *testing.T) { } tests := []struct { - name string - rota Rota - want string + name string + rota Rota + wantTokens []string // each must appear in the reminder text }{ - { - "both have handles", - rota("Ala", "Bartek"), - "Przypomnienie: w tygodniu od 2026-06-29 dyżur mają @ala i @bart. Z góry dziękujemy!", - }, - { - "one handle, one missing -> mixed", - rota("Ala", "Cela"), - "Przypomnienie: w tygodniu od 2026-06-29 dyżur mają @ala i Cela. Z góry dziękujemy!", - }, - { - "name not in people list -> plain name", - rota("Ala", "Zenon"), - "Przypomnienie: w tygodniu od 2026-06-29 dyżur mają @ala i Zenon. Z góry dziękujemy!", - }, - { - "single person with handle", - rota("Bartek", ""), - "Przypomnienie: w tygodniu od 2026-06-29 dyżur ma @bart. Z góry dziękujemy!", - }, + {"both have handles", rota("Ala", "Bartek"), []string{"@ala", "@bart"}}, + {"one handle, one missing -> mixed", rota("Ala", "Cela"), []string{"@ala", "Cela"}}, + {"name not in people list -> plain name", rota("Ala", "Zenon"), []string{"@ala", "Zenon"}}, + {"single person with handle", rota("Bartek", ""), []string{"@bart"}}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -445,8 +396,10 @@ func TestReminderTextMentions(t *testing.T) { if !ok { t.Fatalf("ok = false, want true") } - if got != tc.want { - t.Errorf("reminderText = %q, want %q", got, tc.want) + for _, token := range tc.wantTokens { + if !strings.Contains(got, token) { + t.Errorf("reminderText = %q, want it to contain %q", got, token) + } } }) } diff --git a/telegram.go b/telegram.go index cc30a7d..0e685c1 100644 --- a/telegram.go +++ b/telegram.go @@ -156,16 +156,16 @@ func (h *botHandlers) sync(ctx context.Context, b *bot.Bot, update *models.Updat "Zsynchronizowano: %d tygodni, %d osób.", len(rota.Weeks), len(rota.People))) } -// unknown is the fallback for any update the command matchers don't claim. It -// only answers messages that actually carry a bot_command entity (a mistyped or -// unsupported /command); plain chatter, service messages (joins/pins), media, -// and replies to the bot fall through here too but get no response, so the bot -// stays quiet in groups instead of "Nieznana komenda"-spamming every message. -func (h *botHandlers) unknown(ctx context.Context, b *bot.Bot, update *models.Update) { +// unknown is the fallback for any update the command matchers don't claim. +// It never replies: the bot answers only its predefined commands and ignores +// everything else — mistyped /commands, other bots' commands, plain chatter, +// service messages (joins/pins), and media. Unrecognized commands are logged +// at debug level so they stay visible without adding group noise. +func (h *botHandlers) unknown(_ context.Context, _ *bot.Bot, update *models.Update) { if update.Message == nil || !hasCommandEntity(update.Message.Entities) { return } - h.reply(ctx, b, update, "Nieznana komenda. Użyj /kto_sprzata lub /sync.") + slog.Debug("ignoring unknown command", "text", update.Message.Text) } // hasCommandEntity reports whether the message carries any bot_command entity, diff --git a/telegram_test.go b/telegram_test.go index 0d4dcbb..7ce65ec 100644 --- a/telegram_test.go +++ b/telegram_test.go @@ -214,7 +214,8 @@ func TestCurrentDutyLocalMidnight(t *testing.T) { } // TestKtoSprzataReply checks the /kto_sprzata handler reads the cached rota and -// replies with this week's duty rendered by formatDuty. +// replies with this week's duty. Only the data (the names) is asserted, not the +// exact wording, so copy tweaks don't break the test. func TestKtoSprzataReply(t *testing.T) { rota := Rota{Weeks: []DutyWeek{week(t, "2026-06-15", "Ala", "Bartek")}} svc := testService(t, rota, nil) @@ -224,14 +225,13 @@ func TestKtoSprzataReply(t *testing.T) { h.ktoSprzata(context.Background(), b, updateMsg(123, "/kto_sprzata")) got := sent() - want := "Dyżur w tym tygodniu (od 2026-06-15): Ala i Bartek" - if len(got) != 1 || got[0] != want { - t.Fatalf("reply = %v, want [%q]", got, want) + if len(got) != 1 || !strings.Contains(got[0], "Ala") || !strings.Contains(got[0], "Bartek") { + t.Fatalf("reply = %v, want one message naming Ala and Bartek", got) } } -// TestSyncReplySuccess checks /sync triggers a fetch and reports the resulting -// week/person counts. +// TestSyncReplySuccess checks /sync triggers a fetch, persists the result, and +// replies once. func TestSyncReplySuccess(t *testing.T) { fetched := Rota{ Weeks: []DutyWeek{week(t, "2026-06-15", "Ala", "Bartek")}, @@ -244,15 +244,17 @@ func TestSyncReplySuccess(t *testing.T) { h.sync(context.Background(), b, updateMsg(123, "/sync")) - got := sent() - want := "Zsynchronizowano: 1 tygodni, 3 osób." - if len(got) != 1 || got[0] != want { - t.Fatalf("reply = %v, want [%q]", got, want) + if got := sent(); len(got) != 1 { + t.Fatalf("replies = %v, want exactly one", got) + } + rota, _ := svc.Store.Snapshot() + if len(rota.Weeks) != 1 { + t.Errorf("store weeks = %d, want 1 (sync should persist the fetch)", len(rota.Weeks)) } } -// TestSyncReplyThrottled checks a /sync inside the cooldown window is rejected -// with the remaining-wait message and does not trigger a fetch. +// TestSyncReplyThrottled checks a /sync inside the cooldown window does not +// trigger a fetch but still answers (so the user isn't left hanging). func TestSyncReplyThrottled(t *testing.T) { now := fixedNow()() fetch := func(context.Context) (Rota, error) { @@ -266,16 +268,14 @@ func TestSyncReplyThrottled(t *testing.T) { h.sync(context.Background(), b, updateMsg(123, "/sync")) - got := sent() - want := "Synchronizowano niedawno. Spróbuj ponownie za 31s." - if len(got) != 1 || got[0] != want { - t.Fatalf("reply = %v, want [%q]", got, want) + if got := sent(); len(got) != 1 { + t.Fatalf("replies = %v, want exactly one", got) } } -// TestSyncReplyFailure checks a failed fetch yields the generic failure message -// (internal error details are logged, not shown). The slog.Error this emits is -// expected, not a harness fault. +// TestSyncReplyFailure checks a failed fetch still yields exactly one reply, +// and that internal error details are not leaked into it (they are logged +// instead — the slog.Error this emits is expected, not a harness fault). func TestSyncReplyFailure(t *testing.T) { fetch := func(context.Context) (Rota, error) { return Rota{}, context.DeadlineExceeded @@ -287,15 +287,18 @@ func TestSyncReplyFailure(t *testing.T) { h.sync(context.Background(), b, updateMsg(123, "/sync")) got := sent() - want := "Synchronizacja nie powiodła się. Spróbuj ponownie później." - if len(got) != 1 || got[0] != want { - t.Fatalf("reply = %v, want [%q]", got, want) + if len(got) != 1 { + t.Fatalf("replies = %v, want exactly one", got) + } + if strings.Contains(got[0], context.DeadlineExceeded.Error()) { + t.Errorf("reply %q leaks the internal error", got[0]) } } -// TestUnknownReply checks the fallback answers an unrecognized /command (a -// message carrying a bot_command entity that no matcher claimed). -func TestUnknownReply(t *testing.T) { +// TestUnknownCommandIgnored checks the fallback stays silent on an +// unrecognized /command (a bot_command entity no matcher claimed) — the bot +// answers only its own predefined commands and ignores everything else. +func TestUnknownCommandIgnored(t *testing.T) { svc := testService(t, Rota{}, nil) b, sent := newTestBot(t) h := &botHandlers{svc: svc, now: fixedNow()} @@ -307,16 +310,14 @@ func TestUnknownReply(t *testing.T) { }} h.unknown(context.Background(), b, cmd) - got := sent() - want := "Nieznana komenda. Użyj /kto_sprzata lub /sync." - if len(got) != 1 || got[0] != want { - t.Fatalf("reply = %v, want [%q]", got, want) + if got := sent(); len(got) != 0 { + t.Fatalf("unknown command should not reply, got %v", got) } } // TestUnknownPlainTextIgnored checks ordinary chatter (no bot_command entity) -// gets no reply, so the bot doesn't spam "Nieznana komenda" on every group -// message, service notice, or reply to its own messages. +// gets no reply — the bot stays silent on group messages, service notices, and +// replies to its own messages. func TestUnknownPlainTextIgnored(t *testing.T) { svc := testService(t, Rota{}, nil) b, sent := newTestBot(t) @@ -381,45 +382,3 @@ func TestPublishCommands(t *testing.T) { t.Fatalf("PublishCommands: %v", err) } } - -func TestFormatDuty(t *testing.T) { - tests := []struct { - name string - week DutyWeek - ok bool - want string - }{ - { - "both people", - week(t, "2026-06-15", "Ala", "Bartek"), true, - "Dyżur w tym tygodniu (od 2026-06-15): Ala i Bartek", - }, - { - "only person1", - week(t, "2026-06-15", "Ala", ""), true, - "Dyżur w tym tygodniu (od 2026-06-15): Ala", - }, - { - "only person2", - week(t, "2026-06-15", "", "Bartek"), true, - "Dyżur w tym tygodniu (od 2026-06-15): Bartek", - }, - { - "nobody assigned", - week(t, "2026-06-15", "", ""), true, - "Dyżur w tym tygodniu (od 2026-06-15): brak przypisania", - }, - { - "not found", - DutyWeek{}, false, - "Nie znalazłem dyżuru na ten tydzień.", - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if got := formatDuty(tc.week, tc.ok); got != tc.want { - t.Errorf("formatDuty = %q, want %q", got, tc.want) - } - }) - } -} From e17aefb9e325730dd6aabc317dde9723311f4461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=27Kama=C5=9B=27=20Bruchal?= Date: Wed, 19 Aug 2026 16:49:11 +0200 Subject: [PATCH 2/2] fix: assign duty weeks on their fetch-time tab across year rollover Around New Year the empty-rota guard keeps the old year's cached rows while resolveRange has already moved to the new year's tab, so an auto-assignment could write an old-tab row index onto the new tab. Stamp each DutyWeek with the tab it was fetched from and prefer it when building the assignment range (clock fallback for pre-upgrade caches). Also: widen the default fetch range to A1:G200 so a full year of rows plus roster can't be silently truncated, add -race/golangci-lint/ govulncheck gates to CI (container pinned to 1.26.4-alpine), and add .env.example documenting every configuration variable. --- .env.example | 27 +++++++++++++++++++++ .forgejo/workflows/ci.yml | 14 ++++++++--- README.md | 8 ++++--- service.go | 45 ++++++++++++++++++++++++++--------- service_test.go | 49 +++++++++++++++++++++++++++++++-------- sheet.go | 5 ++++ 6 files changed, 121 insertions(+), 27 deletions(-) create mode 100644 .env.example diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..310ebba --- /dev/null +++ b/.env.example @@ -0,0 +1,27 @@ +# Copy to .env and fill in the required values. Never commit .env or the +# service-account key file. + +# --- Required --- +# Telegram bot token from @BotFather. +BOT_TOKEN= +# Path to the Google service-account JSON key. The sheet must be shared with +# the service account's client_email (Editor for assignment writes). +GOOGLE_CREDENTIALS_FILE= +# Spreadsheet ID from the Google Sheets URL. +GOOGLE_SHEETS_ID= + +# --- Optional (defaults shown) --- +# Range to read; leave unset to follow the current year's tab automatically +# (e.g. 2026!A1:G200), including the New Year rollover. +#SHEET_RANGE= +# Path of the local JSON rota cache. +#ROTA_CACHE_PATH=rota.json +# Background sync frequency (Go duration). +#SYNC_INTERVAL=30m + +# --- Reminders (disabled unless GROUP_CHAT_ID is set) --- +# Telegram chat ID to post weekly reminders to. +#GROUP_CHAT_ID= +# Day and hour (0-23, Europe/Warsaw time) the weekly reminder fires. +#REMINDER_WEEKDAY=Monday +#REMINDER_HOUR=9 diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index c6dca71..c752515 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -8,16 +8,24 @@ on: jobs: build: runs-on: docker - container: golang:1.26-alpine + container: golang:1.26.4-alpine steps: - name: Install tools - run: apk add --no-cache git docker-cli + # build-base provides the C toolchain the race detector needs (-race + # implies CGO). + run: apk add --no-cache git docker-cli build-base - name: Checkout run: git clone ${{ github.server_url }}/${{ github.repository }} . && git checkout ${{ github.sha }} - name: Run tests - run: go test ./... + run: go test -race ./... + + - name: Run linter + run: go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest run + + - name: Check for known vulnerabilities + run: go run golang.org/x/vuln/cmd/govulncheck@latest ./... - name: Log in to container registry run: echo "${{ secrets.REGISTRY_TOKEN }}" | docker login git.kambr.pl -u "${{ secrets.REGISTRY_USER }}" --password-stdin diff --git a/README.md b/README.md index 07dc7bf..613e9e6 100644 --- a/README.md +++ b/README.md @@ -8,8 +8,10 @@ Backend read the schedule from a Google sheet, store it in local JSON file. Requires `GOOGLE_CREDENTIALS_FILE` (path to a Google service-account key, shared as an editor on the sheet), `GOOGLE_SHEETS_ID`, and `BOT_TOKEN` in the -environment or a `.env` file (see `service.go`/`sheet.go` for optional -`SHEET_RANGE`, `ROTA_CACHE_PATH`, `SYNC_INTERVAL`). Then: +environment or a `.env` file — copy `.env.example` to `.env` as a starting +point; it lists every variable (including the optional `SHEET_RANGE`, +`ROTA_CACHE_PATH`, `SYNC_INTERVAL`, and the reminder settings) with defaults. +Then: ```sh go run ./cmd/dyzur-bot @@ -20,7 +22,7 @@ The service loads the cached rota, syncs from Google Sheets in the background on interrupted (Ctrl-C / SIGTERM). When `SHEET_RANGE` is unset, the bot reads the current year's tab (e.g. -`2026!A1:G100`), resolved on every sync — so it follows the New Year rollover +`2026!A1:G200`), resolved on every sync — so it follows the New Year rollover automatically without a restart. A sync that fails, or that returns no duty weeks (e.g. a next-year tab that exists but hasn't been filled in yet), keeps the last-good cache instead of clearing it; if no sync succeeds for 6h the bot diff --git a/service.go b/service.go index 3df4b5c..f1a2b9c 100644 --- a/service.go +++ b/service.go @@ -218,14 +218,27 @@ func SheetFetcher(spreadsheetID, readRange string) FetchFunc { return func(ctx context.Context) (Rota, error) { ctx, cancel := context.WithTimeout(ctx, fetchTimeout) defer cancel() - rows, err := ReadSheet(ctx, spreadsheetID, resolveRange(readRange, time.Now())) + rng := resolveRange(readRange, time.Now()) + rows, err := ReadSheet(ctx, spreadsheetID, rng) if err != nil { return Rota{}, err } - return ParseRota(rows), nil + return stampTabs(ParseRota(rows), rng), nil } } +// stampTabs records on each week the tab its rows were fetched from, so an +// assignment written later (possibly after the clock-resolved tab has rolled to +// the next year while the cache still holds this fetch) targets the tab the +// week's Row is valid on. +func stampTabs(rota Rota, resolvedRange string) Rota { + tab, _, _ := strings.Cut(resolvedRange, "!") + for i := range rota.Weeks { + rota.Weeks[i].Tab = tab + } + return rota +} + // SheetAssigner returns an AssignFunc that writes a week's two names into its // B:C cells on the same tab the rota is read from. sheetRange mirrors the // fetcher's range so the tab (year or explicit) matches. @@ -233,28 +246,38 @@ func SheetAssigner(spreadsheetID, sheetRange string) AssignFunc { return func(ctx context.Context, week DutyWeek, p1, p2 string) error { ctx, cancel := context.WithTimeout(ctx, fetchTimeout) defer cancel() - rng := assignRange(sheetRange, time.Now(), week.Row) + rng := assignRangeFor(week, sheetRange, time.Now()) return WriteSheet(ctx, spreadsheetID, rng, [][]interface{}{{p1, p2}}) } } -// assignRange builds the A1 range for a week's assignment cells (Osoba 1/2 are -// columns B and C). It reuses the tab from resolveRange so the year rollover and -// any explicit range are honored identically to reads. -func assignRange(sheetRange string, now time.Time, row int) string { - tab, _, _ := strings.Cut(resolveRange(sheetRange, now), "!") - return fmt.Sprintf("%s!B%d:C%d", tab, row, row) +// assignRangeFor builds the A1 range for a week's assignment cells (Osoba 1/2 +// are columns B and C). An explicit sheetRange keeps its tab verbatim, matching +// reads. Otherwise the week's fetch-time tab is preferred over clock resolution: +// around New Year the cache can hold the old tab's rows (the empty-rota guard +// keeps them while the next year's tab is unfilled) after resolveRange has moved +// to the new tab, and writing an old-tab Row onto the new tab would corrupt it. +// A week without a recorded tab (cache from an older version) falls back to the +// clock, preserving prior behavior. +func assignRangeFor(week DutyWeek, sheetRange string, now time.Time) string { + tab := week.Tab + if sheetRange != "" || tab == "" { + tab, _, _ = strings.Cut(resolveRange(sheetRange, now), "!") + } + return fmt.Sprintf("%s!B%d:C%d", tab, week.Row, week.Row) } // resolveRange returns the explicit range when set, otherwise the default that // targets the current year's tab (sheets are named per year, e.g. "2026"). // Resolving against now (not a startup-captured value) is what lets the range -// advance to the next year's tab automatically. +// advance to the next year's tab automatically. The 200-row cap bounds the +// fetch; rows past it are silently dropped, so it must stay comfortably above +// one year of weekly rows (~53) plus roster entries. func resolveRange(explicit string, now time.Time) string { if explicit != "" { return explicit } - return fmt.Sprintf("%d!A1:G100", now.Year()) + return fmt.Sprintf("%d!A1:G200", now.Year()) } // Service ties the Store and Syncer together for the bot to consume. diff --git a/service_test.go b/service_test.go index 1462c00..cb3a9de 100644 --- a/service_test.go +++ b/service_test.go @@ -181,16 +181,43 @@ func TestSyncSuccessReplacesCache(t *testing.T) { } } -func TestAssignRange(t *testing.T) { - now := time.Date(2026, 6, 19, 9, 0, 0, 0, time.UTC) +// Regression test for the year-boundary tab/row divergence: a week cached from +// the old year's tab (e.g. kept by the empty-rota guard across New Year) must be +// assigned on the tab it was fetched from, not the tab the wall clock resolves +// to — its Row is only meaningful on the fetch tab. +func TestAssignRangeForUsesFetchTab(t *testing.T) { + jan2027 := time.Date(2027, 1, 4, 9, 0, 0, 0, time.UTC) - // Empty range -> current-year tab; the assignment targets cols B:C of the row. - if got, want := assignRange("", now, 5), "2026!B5:C5"; got != want { - t.Errorf("assignRange(\"\"): got %q, want %q", got, want) + // Week fetched from the 2026 tab, assigned after the clock rolled to 2027. + week := DutyWeek{Row: 54, Tab: "2026"} + if got, want := assignRangeFor(week, "", jan2027), "2026!B54:C54"; got != want { + t.Errorf("assignRangeFor(fetch tab): got %q, want %q", got, want) } - // Explicit range -> its tab is reused verbatim. - if got, want := assignRange("Dyżury!A1:C30", now, 7), "Dyżury!B7:C7"; got != want { - t.Errorf("assignRange(explicit): got %q, want %q", got, want) + + // A week without a recorded tab (cache written by an older version) falls + // back to clock-resolved tab, matching the previous behavior. + legacy := DutyWeek{Row: 54} + if got, want := assignRangeFor(legacy, "", jan2027), "2027!B54:C54"; got != want { + t.Errorf("assignRangeFor(legacy fallback): got %q, want %q", got, want) + } + + // An explicit range keeps its tab verbatim over the week's recorded tab. + explicit := DutyWeek{Row: 7, Tab: "2026"} + if got, want := assignRangeFor(explicit, "Dyżury!A1:C30", jan2027), "Dyżury!B7:C7"; got != want { + t.Errorf("assignRangeFor(explicit): got %q, want %q", got, want) + } +} + +func TestStampTabs(t *testing.T) { + rota := ParseRota(sampleRows()) + stamped := stampTabs(rota, "2026!A1:G200") + if len(stamped.Weeks) == 0 { + t.Fatal("sample rota has no weeks") + } + for _, w := range stamped.Weeks { + if w.Tab != "2026" { + t.Errorf("week %s: Tab = %q, want %q", w.WeekStart.Format("2006-01-02"), w.Tab, "2026") + } } } @@ -198,8 +225,10 @@ func TestResolveRange(t *testing.T) { now := time.Date(2027, 1, 1, 0, 0, 0, 0, time.UTC) // Empty range resolves to the current year's tab, evaluated against now — - // this is what advances the range after a New Year rollover. - if got, want := resolveRange("", now), "2027!A1:G100"; got != want { + // this is what advances the range after a New Year rollover. The row cap + // (200) is a deliberate constant: rows past it are silently dropped by the + // fetch, so keep it comfortably above one year of weekly rows. + if got, want := resolveRange("", now), "2027!A1:G200"; got != want { t.Errorf("resolveRange(\"\"): got %q, want %q", got, want) } // An explicit range is honored verbatim regardless of the year. diff --git a/sheet.go b/sheet.go index 1b2d066..d24e3fa 100644 --- a/sheet.go +++ b/sheet.go @@ -32,6 +32,11 @@ type DutyWeek struct { // cells when writing an assignment back. Valid only when the read range // starts at A1 (as resolveRange always produces); 0 means unknown. Row int + // Tab is the sheet tab Row was fetched from (e.g. "2026"). Row is only + // meaningful on this tab: around New Year the cache may still hold the old + // year's rows while the clock-resolved tab has moved on, so an assignment + // must target Tab, not the current year. "" means unknown (older cache). + Tab string } // Person is a single entry from the people list (columns E–G).