feat: ignore unknown commands instead of replying
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.
This commit is contained in:
Kamil 'Kamaś' Bruchal
parent
37a81a2710
commit
488f0b9a9b
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
|
|
|||
14
telegram.go
14
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,
|
||||
|
|
|
|||
105
telegram_test.go
105
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue