diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index 9b01610..c1cf19a 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -143,7 +143,9 @@ func main() { log.Printf("mavweb: ambient notification ingest enabled at POST /api/ambient") } - // The read surfaces. Every one of them 503s without -core. + // The data surfaces. Every one of them 503s without -core. /reminders also + // accepts an ID-bound cancellation POST. It is deliberately not step-up + // gated: like dismissing a proposed routine, it can only make Maven quieter. mux.HandleFunc("/dash", corePage(handleDash)) mux.HandleFunc("/history", corePage(handleHistory)) mux.HandleFunc("/trace", corePage(handleTrace)) diff --git a/cmd/mavweb/reminders.go b/cmd/mavweb/reminders.go index 6536680..c58981d 100644 --- a/cmd/mavweb/reminders.go +++ b/cmd/mavweb/reminders.go @@ -3,8 +3,10 @@ package main import ( _ "embed" "encoding/json" + "errors" "fmt" "net/http" + "strconv" "strings" "github.com/kami/maven/internal/ipc" @@ -23,11 +25,14 @@ var remindersTmpl = parsePage("reminders", remindersHTML, nil) // (Vikunja #469). Neither is a formatting nicety: the envelope is an internal // shape he never chose, and a time on a page he reads is the time on his wall. type reminderRow struct { - Created string - Fires string - Status string - Detail string - Text string + ID int64 + Created string + Fires string + Schedule string + Status string + Detail string + Text string + CanCancel bool } // reminderText unwraps the {"text":...} payload the router writes. @@ -51,8 +56,16 @@ func reminderText(payload string) string { func reminderRows(rs []ipc.Reminder) []reminderRow { out := make([]reminderRow, 0, len(rs)) for _, r := range rs { + fire := r.NextFireTs + if fire.IsZero() { + fire = r.FireTs + } status := r.Status detail := "" + schedule := "" + if r.Cron != "" { + schedule = "recurring · " + r.Cron + } if !r.DeliveryBlockedTs.IsZero() { status = "blocked" detail = r.DeliveryBlockedError @@ -60,25 +73,108 @@ func reminderRows(rs []ipc.Reminder) []reminderRow { detail = "retry " + r.NextAttemptTs.Local().Format("02 Jan 15:04") } out = append(out, reminderRow{ - Created: r.CreatedTs.Local().Format("02 Jan 15:04"), - Fires: r.FireTs.Local().Format("02 Jan 15:04"), - Status: status, - Detail: detail, - Text: reminderText(r.Payload), + ID: r.ID, + Created: r.CreatedTs.Local().Format("02 Jan 15:04"), + Fires: fire.Local().Format("02 Jan 15:04"), + Schedule: schedule, + Status: status, + Detail: detail, + Text: reminderText(r.Payload), + CanCancel: r.Status == "pending", }) } return out } +// remindersForPage keeps every pending row reachable while retaining the +// recent terminal history the page already showed. Pending rows come first in +// firing order (the CoreAPI contract); IDs present in the recent window are not +// duplicated below them. +func remindersForPage(pending, recent []ipc.Reminder) []ipc.Reminder { + out := make([]ipc.Reminder, 0, len(pending)+len(recent)) + seen := make(map[int64]bool, len(pending)) + for _, reminder := range pending { + out = append(out, reminder) + seen[reminder.ID] = true + } + for _, reminder := range recent { + if seen[reminder.ID] { + continue + } + out = append(out, reminder) + } + return out +} + func handleReminders(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { if !requireCore(w, r, core, "reminders") { return } - reminders, err := core.ListReminders(r.Context(), 50) - if err != nil { - writeProblem(w, r, http.StatusBadGateway, problemCoreReadFailed, - "reminders unavailable", fmt.Errorf("list reminders: %w", err)) + msg := "" + if r.Method == http.MethodGet && r.URL.Query().Get("cancelled") == "1" { + msg = "reminder cancelled" + } + switch r.Method { + case http.MethodGet: + case http.MethodPost: + if strings.TrimSpace(r.FormValue("action")) != "cancel" { + writeProblem(w, r, http.StatusBadRequest, problemInvalidRequest, + "unknown reminder action", nil) + return + } + id, err := strconv.ParseInt(strings.TrimSpace(r.FormValue("id")), 10, 64) + if err != nil || id <= 0 { + writeProblem(w, r, http.StatusBadRequest, problemInvalidRequest, + "invalid reminder id", err) + return + } + if err := core.CancelReminder(r.Context(), id); err != nil { + switch { + case errors.Is(err, ipc.ErrReminderNotFound): + writeProblem(w, r, http.StatusNotFound, problemResourceNotFound, + "reminder not found", err) + case errors.Is(err, ipc.ErrReminderInFlight): + writeProblem(w, r, http.StatusConflict, problemCoreChangeFailed, + "reminder delivery has already started", err) + case errors.Is(err, ipc.ErrReminderState): + writeProblem(w, r, http.StatusConflict, problemCoreChangeFailed, + "reminder is no longer pending", err) + default: + writeProblem(w, r, http.StatusBadGateway, problemCoreChangeFailed, + "reminder cancellation failed", fmt.Errorf("cancel reminder %d: %w", id, err)) + } + return + } + http.Redirect(w, r, "/reminders?cancelled=1", http.StatusSeeOther) + return + default: + writeProblem(w, r, http.StatusMethodNotAllowed, problemMethodNotAllowed, + "method not allowed", nil) return } - renderPage(w, remindersTmpl, map[string]any{"Reminders": reminderRows(reminders)}) + pending, err := core.ListPendingReminders(r.Context(), 0) + if err != nil { + public := "reminders unavailable" + if msg != "" { + public = "reminder cancelled; refreshed list unavailable" + } + writeProblem(w, r, http.StatusBadGateway, problemCoreReadFailed, + public, fmt.Errorf("list pending reminders: %w", err)) + return + } + recent, err := core.ListReminders(r.Context(), 50) + if err != nil { + public := "reminders unavailable" + if msg != "" { + public = "reminder cancelled; refreshed list unavailable" + } + writeProblem(w, r, http.StatusBadGateway, problemCoreReadFailed, + public, fmt.Errorf("list reminders: %w", err)) + return + } + reminders := remindersForPage(pending, recent) + renderPage(w, remindersTmpl, struct { + Msg string + Reminders []reminderRow + }{msg, reminderRows(reminders)}) } diff --git a/cmd/mavweb/reminders.html b/cmd/mavweb/reminders.html index 7bdbb20..fa76242 100644 --- a/cmd/mavweb/reminders.html +++ b/cmd/mavweb/reminders.html @@ -1,12 +1,17 @@ {{template "shellTop" "reminders"}}

Reminders

+{{if .Msg}}
{{.Msg}}
{{end}} {{if .Reminders}}
- + {{range .Reminders}} - + +{{end}}
createdfiresstatuswhat
createdfiresstatuswhataction
{{.Created}}{{.Fires}}{{.Fires}}{{if .Schedule}}
{{.Schedule}}
{{end}}
{{.Status}}{{if .Detail}}
{{.Detail}}
{{end}}
{{.Text}}{{if .CanCancel}}
+ + +
{{end}}
{{else}}
diff --git a/cmd/mavweb/reminders_test.go b/cmd/mavweb/reminders_test.go index 3e5e16b..7aefbe0 100644 --- a/cmd/mavweb/reminders_test.go +++ b/cmd/mavweb/reminders_test.go @@ -1,6 +1,11 @@ package main import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "net/url" "strings" "testing" "time" @@ -12,6 +17,7 @@ import ( func TestReminderRowsUnwrapAndLocalise(t *testing.T) { fire := time.Date(2026, 8, 4, 18, 30, 0, 0, time.UTC) rows := reminderRows([]ipc.Reminder{{ + ID: 17, CreatedTs: fire.Add(-time.Hour), FireTs: fire, Status: "pending", @@ -29,6 +35,9 @@ func TestReminderRowsUnwrapAndLocalise(t *testing.T) { if strings.Contains(rows[0].Text, "{") { t.Errorf("Text still carries JSON: %q", rows[0].Text) } + if rows[0].ID != 17 || !rows[0].CanCancel { + t.Errorf("pending reminder action binding = %+v, want id 17 cancellable", rows[0]) + } } func TestReminderRowsExposeBlockedDelivery(t *testing.T) { @@ -42,6 +51,33 @@ func TestReminderRowsExposeBlockedDelivery(t *testing.T) { if len(rows) != 1 || rows[0].Status != "blocked" || rows[0].Detail != "ntfy credentials rejected" { t.Fatalf("blocked reminder is not visible: %+v", rows) } + if !rows[0].CanCancel { + t.Fatal("a blocked but still-pending reminder must remain cancellable") + } +} + +func TestReminderRowsShowTheCurrentRecurringOccurrence(t *testing.T) { + original := time.Date(2026, 8, 1, 9, 0, 0, 0, time.UTC) + next := time.Date(2026, 8, 16, 9, 0, 0, 0, time.UTC) + rows := reminderRows([]ipc.Reminder{{ + ID: 42, FireTs: original, NextFireTs: next, Cron: "0 9 * * *", + Status: "pending", Payload: `{"text":"принять лекарство"}`, + }}) + if len(rows) != 1 || rows[0].Fires != next.Local().Format("02 Jan 15:04") { + t.Fatalf("recurring fire = %+v, want current occurrence %s", rows, next) + } + if rows[0].Schedule != "recurring · 0 9 * * *" || !rows[0].CanCancel { + t.Fatalf("recurring identity/action = %+v", rows[0]) + } +} + +func TestReminderRowsOnlyPendingCanCancel(t *testing.T) { + rows := reminderRows([]ipc.Reminder{{Status: "cancelled"}, {Status: "fired"}}) + for _, row := range rows { + if row.CanCancel { + t.Errorf("terminal row %+v exposed a cancel action", row) + } + } } // A payload that is not the envelope is his own words, so it is shown as it is. @@ -57,3 +93,169 @@ func TestReminderTextKeepsPlainPayload(t *testing.T) { } } } + +type reminderCore struct { + ipc.UnimplementedCoreAPI + + reminders []ipc.Reminder + pending []ipc.Reminder + listErr error + pendingErr error + cancelErr error + cancelID int64 +} + +func (c *reminderCore) ListReminders(context.Context, int) ([]ipc.Reminder, error) { + return c.reminders, c.listErr +} + +func (c *reminderCore) ListPendingReminders(context.Context, int) ([]ipc.Reminder, error) { + return c.pending, c.pendingErr +} + +func (c *reminderCore) CancelReminder(_ context.Context, id int64) error { + c.cancelID = id + if c.cancelErr != nil { + return c.cancelErr + } + for i := range c.reminders { + if c.reminders[i].ID == id { + c.reminders[i].Status = "cancelled" + } + } + return nil +} + +func reminderPost(action, id string) *http.Request { + form := url.Values{"action": {action}, "id": {id}} + req := httptest.NewRequest(http.MethodPost, "/reminders", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +func TestHandleRemindersCancel(t *testing.T) { + core := &reminderCore{reminders: []ipc.Reminder{{ + ID: 23, Status: "pending", Payload: `{"text":"позвонить врачу"}`, + }}} + rr := httptest.NewRecorder() + handleReminders(rr, reminderPost("cancel", "23"), core) + + if rr.Code != http.StatusSeeOther || rr.Header().Get("Location") != "/reminders?cancelled=1" { + t.Fatalf("status/location = %d %q, want 303 PRG", rr.Code, rr.Header().Get("Location")) + } + if core.cancelID != 23 { + t.Fatalf("cancel id = %d, want 23", core.cancelID) + } + + rr = httptest.NewRecorder() + handleReminders(rr, httptest.NewRequest(http.MethodGet, "/reminders?cancelled=1", nil), core) + if rr.Code != http.StatusOK || !strings.Contains(rr.Body.String(), "reminder cancelled") || strings.Contains(rr.Body.String(), "value=\"23\"") { + t.Fatalf("redirect outcome rendered incorrectly: status=%d body=%s", rr.Code, rr.Body.String()) + } +} + +func TestHandleRemindersSuccessfulMutationDoesNotDependOnRefresh(t *testing.T) { + core := &reminderCore{listErr: errors.New("offline"), pendingErr: errors.New("offline")} + rr := httptest.NewRecorder() + handleReminders(rr, reminderPost("cancel", "23"), core) + if rr.Code != http.StatusSeeOther || core.cancelID != 23 { + t.Fatalf("successful cancellation became refresh failure: status=%d id=%d body=%s", rr.Code, core.cancelID, rr.Body.String()) + } +} + +func TestHandleRemindersIncludesPendingRowsOutsideRecentWindow(t *testing.T) { + old := ipc.Reminder{ID: 1, Status: "pending", Payload: `{"text":"old but pending"}`} + recent := make([]ipc.Reminder, 50) + for i := range recent { + recent[i] = ipc.Reminder{ID: int64(i + 2), Status: "fired", Payload: `{"text":"history"}`} + } + core := &reminderCore{pending: []ipc.Reminder{old}, reminders: recent} + rr := httptest.NewRecorder() + handleReminders(rr, httptest.NewRequest(http.MethodGet, "/reminders", nil), core) + if rr.Code != http.StatusOK || !strings.Contains(rr.Body.String(), "old but pending") || + !strings.Contains(rr.Body.String(), `value="1"`) { + t.Fatalf("old pending reminder is not reachable: status=%d body=%s", rr.Code, rr.Body.String()) + } +} + +func TestHandleRemindersRejectsMalformedPosts(t *testing.T) { + for _, tc := range []struct { + name string + action string + id string + }{ + {"unknown action", "delete", "23"}, + {"missing id", "cancel", ""}, + {"non-numeric id", "cancel", "twenty-three"}, + {"non-positive id", "cancel", "0"}, + } { + t.Run(tc.name, func(t *testing.T) { + core := &reminderCore{} + rr := httptest.NewRecorder() + handleReminders(rr, reminderPost(tc.action, tc.id), core) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400; body=%s", rr.Code, rr.Body.String()) + } + if core.cancelID != 0 { + t.Fatalf("CancelReminder called with %d for malformed post", core.cancelID) + } + }) + } +} + +func TestHandleRemindersCancelErrors(t *testing.T) { + for _, tc := range []struct { + name string + err error + status int + public string + }{ + {"missing", ipc.ErrReminderNotFound, http.StatusNotFound, "reminder not found"}, + {"delivery started", ipc.ErrReminderInFlight, http.StatusConflict, "reminder delivery has already started"}, + {"terminal", ipc.ErrReminderState, http.StatusConflict, "reminder is no longer pending"}, + {"transport", errors.New("socket closed"), http.StatusBadGateway, "reminder cancellation failed"}, + } { + t.Run(tc.name, func(t *testing.T) { + core := &reminderCore{cancelErr: tc.err} + rr := httptest.NewRecorder() + handleReminders(rr, reminderPost("cancel", "23"), core) + if rr.Code != tc.status || !strings.Contains(rr.Body.String(), tc.public) { + t.Fatalf("status/body = %d %q, want %d containing %q", rr.Code, rr.Body.String(), tc.status, tc.public) + } + if got := rr.Header().Get("Content-Type"); !strings.HasPrefix(got, "application/problem+json") { + t.Fatalf("content type = %q, want problem JSON", got) + } + }) + } +} + +func TestHandleRemindersMethodAndListErrors(t *testing.T) { + t.Run("method", func(t *testing.T) { + rr := httptest.NewRecorder() + handleReminders(rr, httptest.NewRequest(http.MethodDelete, "/reminders", nil), &reminderCore{}) + if rr.Code != http.StatusMethodNotAllowed { + t.Fatalf("status = %d, want 405", rr.Code) + } + }) + t.Run("pending list", func(t *testing.T) { + rr := httptest.NewRecorder() + handleReminders(rr, httptest.NewRequest(http.MethodGet, "/reminders", nil), &reminderCore{pendingErr: errors.New("offline")}) + if rr.Code != http.StatusBadGateway || !strings.Contains(rr.Body.String(), "reminders unavailable") { + t.Fatalf("status/body = %d %q, want 502 problem", rr.Code, rr.Body.String()) + } + }) + t.Run("recent list", func(t *testing.T) { + rr := httptest.NewRecorder() + handleReminders(rr, httptest.NewRequest(http.MethodGet, "/reminders", nil), &reminderCore{listErr: errors.New("offline")}) + if rr.Code != http.StatusBadGateway || !strings.Contains(rr.Body.String(), "reminders unavailable") { + t.Fatalf("status/body = %d %q, want 502 problem", rr.Code, rr.Body.String()) + } + }) + t.Run("successful outcome remains explicit when redirected refresh fails", func(t *testing.T) { + rr := httptest.NewRecorder() + handleReminders(rr, httptest.NewRequest(http.MethodGet, "/reminders?cancelled=1", nil), &reminderCore{listErr: errors.New("offline")}) + if rr.Code != http.StatusBadGateway || !strings.Contains(rr.Body.String(), "reminder cancelled; refreshed list unavailable") { + t.Fatalf("status/body = %d %q, want truthful refresh problem", rr.Code, rr.Body.String()) + } + }) +}