Web: visual design pass (issue #26)
Addresses the screenshot-review feedback in #26. No framework or build step added — all of this stays within the existing plain HTML/CSS/ vanilla-JS + go:embed architecture. - Nav: re-enable the bottom tab bar that was already built and switched off (Queue/On-call/Alerts/Team + a "More" sheet for Stats/Admin/Account), replacing the hamburger on phone width. - Queue: chip counts, a scroll fade on the filter row, a "Triggered Xh ago" + severity label per row, a chevron on the team switcher so it reads as a dropdown. - On-call: collapse repeated same-person days into shift bars (week view and "your shifts" both), show the week as a date range with the ISO week number as secondary text, split "Current shift" out from "Next shifts" with "ends in Nd", a pill badge + row highlight for "you". - Incident detail: fix the actual bug behind the duplicate "acknowledged" timeline entries (acknowledgeIncident's UPDATE had no guard on the incident's current status, so acknowledging an already-acknowledged incident silently re-logged the event — now idempotent, with regression tests on both the authenticated route and the ntfy ack-button route). Relabel escalation re-pages so they don't look like the same page landing twice. Copy the primary action up near the top. Label the "···" button. Group the timeline by phase (triggered/acknowledged/resolved). Add an "at a glance" summary row (duration/severity/responsible) and collapse the group labels by default. - Team overview: reword the vague copy ("One owner." etc.) into plain labels. - Empty states: fill in missing icons/one-liners across queue, alerts, stats and the incident timeline. - CSS: fix card padding bugs, verify link contrast already passes AA, introduce a --fs-* type-scale token set and migrate the few genuinely isolated cases onto it (left sizes tied to a fixed shape, a deliberately prominent display, or a non-negotiable constraint like the iOS-zoom-prevention input size as documented exceptions rather than guess at a render this change can't see). Verified with the full fmt/lint/test/helm-lint gate, plus a live instance against the test DB with seeded incidents and schedule data to trace the on-call grouping and timeline phase-splitting logic against real API responses. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -261,14 +261,17 @@ func resolveIfSettled(ctx context.Context, q querier, incidentID int64) (bool, e
|
||||
}
|
||||
|
||||
// acknowledgeIncident records that userID has picked an incident up, and reports
|
||||
// whether it changed anything — an already-resolved incident is left alone.
|
||||
// Shared by the authenticated handler and the Acknowledge button in a push
|
||||
// notification, so both write the same state and the same timeline entry.
|
||||
// whether it changed anything — an already-resolved or already-acknowledged
|
||||
// incident is left alone, so a second acknowledge (a retried request, or a
|
||||
// stale push notification tapped after the web UI already acked it) is a
|
||||
// no-op rather than a second "acknowledged" timeline entry. Shared by the
|
||||
// authenticated handler and the Acknowledge button in a push notification,
|
||||
// so both write the same state and the same timeline entry.
|
||||
func acknowledgeIncident(ctx context.Context, q querier, incidentID, userID int64) (bool, error) {
|
||||
res, err := q.ExecContext(ctx, `
|
||||
UPDATE incidents
|
||||
SET status = 'acknowledged', acknowledged_by = $1, acknowledged_at = $2
|
||||
WHERE id = $3 AND resolved_at IS NULL`,
|
||||
WHERE id = $3 AND status = 'triggered'`,
|
||||
userID, time.Now().Unix(), incidentID)
|
||||
if err != nil {
|
||||
return false, err
|
||||
|
||||
@@ -197,10 +197,20 @@ func handleIncidentAcknowledge(db *sql.DB) http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
if !acked {
|
||||
if !incidentExists(w, r, db, id) {
|
||||
// incidentIDParam above already confirmed the incident exists, so this
|
||||
// is either resolved, or already acknowledged — the latter is now a
|
||||
// no-op rather than an error, since the caller's desired state
|
||||
// (acknowledged) already holds.
|
||||
inc, err := fetchIncident(r.Context(), db, id)
|
||||
if err != nil {
|
||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||
return
|
||||
}
|
||||
respond(w, http.StatusConflict, errResp("incident is resolved"))
|
||||
if inc.Status == "resolved" {
|
||||
respond(w, http.StatusConflict, errResp("incident is resolved"))
|
||||
return
|
||||
}
|
||||
respond(w, http.StatusOK, inc)
|
||||
return
|
||||
}
|
||||
respondIncident(w, r, db, id)
|
||||
|
||||
@@ -338,6 +338,48 @@ func TestIncident_Acknowledge(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A second acknowledge — a retried request, or a stale push notification
|
||||
// tapped after the web UI already acked it — must be a no-op: same state,
|
||||
// no second "acknowledged" timeline entry. Regression test for the bug
|
||||
// described in issue #26 ("two acknowledged entries look like a bug").
|
||||
func TestIncident_AcknowledgeTwiceIsIdempotent(t *testing.T) {
|
||||
s := newTS(t)
|
||||
postWebhook(t, s, []map[string]any{
|
||||
amAlert("fp-ack2", "Y", "firing", "2026-05-20T10:00:00Z", zeroTime, nil),
|
||||
})
|
||||
|
||||
resp := s.req(t, http.MethodPost, "/api/incidents/1/acknowledge", nil)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("first acknowledge returned %d", resp.StatusCode)
|
||||
}
|
||||
var first map[string]any
|
||||
decode(t, resp, &first)
|
||||
|
||||
resp = s.req(t, http.MethodPost, "/api/incidents/1/acknowledge", nil)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("second acknowledge returned %d, want 200 (idempotent)", resp.StatusCode)
|
||||
}
|
||||
var second map[string]any
|
||||
decode(t, resp, &second)
|
||||
if second["status"] != "acknowledged" {
|
||||
t.Errorf("expected status still acknowledged, got %v", second["status"])
|
||||
}
|
||||
if second["acknowledged_by"] != first["acknowledged_by"] {
|
||||
t.Errorf("expected the same acknowledged_by, got %v then %v", first["acknowledged_by"], second["acknowledged_by"])
|
||||
}
|
||||
|
||||
types := eventTypes(timeline(t, s, 1))
|
||||
n := 0
|
||||
for _, ty := range types {
|
||||
if ty == "acknowledged" {
|
||||
n++
|
||||
}
|
||||
}
|
||||
if n != 1 {
|
||||
t.Errorf("expected exactly one acknowledged event, got %d in %v", n, types)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIncident_ManualResolveIsTerminal(t *testing.T) {
|
||||
s := newTS(t)
|
||||
postWebhook(t, s, []map[string]any{
|
||||
|
||||
@@ -66,12 +66,19 @@ func handleNotifyAck(db *sql.DB) http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
if !acked {
|
||||
// The incident closed between the page and the tap. Nothing to do,
|
||||
// and nothing the responder did wrong — report the state, not an error,
|
||||
// so ntfy shows a success toast rather than a failure.
|
||||
// Either the incident closed between the page and the tap, or it was
|
||||
// already acknowledged (e.g. from the web UI, or an earlier tap of
|
||||
// the same button) — either way nothing the responder did wrong, so
|
||||
// report the actual state rather than assuming "resolved", and let
|
||||
// ntfy show a success toast rather than a failure.
|
||||
inc, err := fetchIncident(r.Context(), db, incidentID)
|
||||
if err != nil {
|
||||
respond(w, http.StatusInternalServerError, errResp("internal error"))
|
||||
return
|
||||
}
|
||||
respond(w, http.StatusOK, map[string]any{
|
||||
"incident_id": incidentID,
|
||||
"status": "resolved",
|
||||
"status": inc.Status,
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
@@ -408,6 +408,47 @@ func TestNotify_AckButtonAcknowledgesIncident(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The ack token isn't single-use (it stays valid for a day, in case the
|
||||
// first tap never reaches the server), so tapping the same notification's
|
||||
// Acknowledge button twice is a real scenario, not just a retried request.
|
||||
// It must report the incident's actual state, not assume "resolved" —
|
||||
// see handleNotifyAck's !acked branch — and must not log a second
|
||||
// "acknowledged" event.
|
||||
func TestNotify_AckButtonTwiceIsIdempotent(t *testing.T) {
|
||||
s, f := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
|
||||
|
||||
fireCritical(t, s)
|
||||
s.sweepNotify(t)
|
||||
|
||||
ackURL := f.messages()[0].Actions[0].URL
|
||||
path := ackURL[strings.Index(ackURL, "/api/notify/ack/"):]
|
||||
|
||||
for i := range 2 {
|
||||
resp, err := http.Post(s.URL+path, "application/json", nil)
|
||||
if err != nil {
|
||||
t.Fatalf("ack %d: %v", i+1, err)
|
||||
}
|
||||
var body map[string]any
|
||||
decode(t, resp, &body)
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("ack %d returned %d", i+1, resp.StatusCode)
|
||||
}
|
||||
if body["status"] != "acknowledged" {
|
||||
t.Errorf("ack %d: expected status acknowledged, got %v", i+1, body["status"])
|
||||
}
|
||||
}
|
||||
|
||||
n := 0
|
||||
for _, ty := range eventTypes(timeline(t, s, 1)) {
|
||||
if ty == "acknowledged" {
|
||||
n++
|
||||
}
|
||||
}
|
||||
if n != 1 {
|
||||
t.Errorf("expected exactly one acknowledged event after two taps, got %d", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNotify_AckRejectsUnknownToken(t *testing.T) {
|
||||
s, _ := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
|
||||
fireCritical(t, s)
|
||||
|
||||
Reference in New Issue
Block a user