diff --git a/README.md b/README.md index 4bf920e..9c61671 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,13 @@ Schedule section: | `d` | Remove the assignment | | `←` / `→` | Shift the week window | +One person holds a given day. Assigning over days somebody else already has +asks first — naming them and how many days are being taken — and moves the whole +selection at once when you accept, so reassigning a week is one confirmation +rather than seven deletions. Taking somebody's shift needs terdut-server +**v0.8.0 or later**; against an older server the assignment is refused with +`date already assigned`. + Users section: | Key | Action | diff --git a/internal/api/client.go b/internal/api/client.go index 4a3bbad..194d880 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -358,11 +358,17 @@ func (c *Client) GetCurrentOnCall() (*ScheduleEntry, error) { return &entry, nil } -func (c *Client) AssignSchedule(userID int64, dates []string) ([]ScheduleEntry, error) { +// AssignSchedule puts one user on call for the given dates. +// +// The server holds one person per day and refuses a date somebody already has, +// so replace is what takes a shift off its current holder. It is all-or-nothing +// either way: a week of free and taken days moves as a unit, or not at all. +func (c *Client) AssignSchedule(userID int64, dates []string, replace bool) ([]ScheduleEntry, error) { body := struct { - UserID int64 `json:"user_id"` - Dates []string `json:"dates"` - }{UserID: userID, Dates: dates} + UserID int64 `json:"user_id"` + Dates []string `json:"dates"` + Replace bool `json:"replace,omitempty"` + }{UserID: userID, Dates: dates, Replace: replace} req, err := c.newRequestWithBody(http.MethodPost, "/api/schedule", body) if err != nil { return nil, err diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 4675f85..b0ba422 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -166,6 +166,28 @@ func TestClient_RequestBodies(t *testing.T) { } }) + // replace is what takes a day off its current holder, so it has to reach the + // wire when asked for — and stay off it when not. + t.Run("assign schedule", func(t *testing.T) { + c, got := stub(t, http.StatusCreated, `[]`) + if _, err := c.AssignSchedule(3, []string{"2026-07-27"}, false); err != nil { + t.Fatalf("assign: %v", err) + } + if got.body != `{"user_id":3,"dates":["2026-07-27"]}` { + t.Errorf("unexpected body %q", got.body) + } + }) + + t.Run("assign schedule with replace", func(t *testing.T) { + c, got := stub(t, http.StatusCreated, `[]`) + if _, err := c.AssignSchedule(3, []string{"2026-07-27"}, true); err != nil { + t.Fatalf("assign: %v", err) + } + if got.body != `{"user_id":3,"dates":["2026-07-27"],"replace":true}` { + t.Errorf("unexpected body %q", got.body) + } + }) + t.Run("set notify target", func(t *testing.T) { c, got := stub(t, http.StatusOK, `{}`) if _, err := c.SetUserNotifyTarget(3, "terdut-niklas"); err != nil { diff --git a/internal/tui/model.go b/internal/tui/model.go index 29fed15..59acb67 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -54,6 +54,7 @@ const ( confirmResolveIncident confirmDeleteScheduleEntry confirmDeleteUser + confirmReassignSchedule ) // pickerTarget says what the user picker is choosing a person for. @@ -143,6 +144,18 @@ type scheduleDay struct { entry *api.ScheduleEntry } +// pendingAssign is an on-call assignment held back by the reassignment +// confirmation, because some of its dates belong to somebody else. +type pendingAssign struct { + userID int64 + username string + dates []string + // taken are the dates currently held by other people, and holders the + // distinct names holding them — both only for wording the prompt. + taken []string + holders []string +} + type Model struct { client *api.Client serverURL string @@ -194,6 +207,7 @@ type Model struct { confirmTarget confirmTarget pendingDeleteID int64 // note event ID pendingDeleteEntry *api.ScheduleEntry + pendingAssign *pendingAssign // Stats topAlerts []api.TopAlert @@ -866,9 +880,9 @@ func fetchScheduleCmd(client *api.Client, from, to time.Time) tea.Cmd { } } -func assignScheduleCmd(client *api.Client, userID int64, dates []string, from, to time.Time) tea.Cmd { +func assignScheduleCmd(client *api.Client, userID int64, dates []string, replace bool, from, to time.Time) tea.Cmd { return func() tea.Msg { - if _, err := client.AssignSchedule(userID, dates); err != nil { + if _, err := client.AssignSchedule(userID, dates, replace); err != nil { return scheduleActionErrMsg{err} } entries, err := client.GetSchedule(from.Format("2006-01-02"), to.Format("2006-01-02")) diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 0b8bad5..299ebe8 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -277,3 +277,95 @@ func TestBuildScheduleDays(t *testing.T) { t.Error("expected unassigned days to have no entry") } } + +// ── Schedule reassignment ───────────────────────────────────────────────── + +// scheduledWeek builds a model showing the week of 2026-07-27 with the given +// entries already on the rota. +func scheduledWeek(entries []api.ScheduleEntry) Model { + m := NewModel(nil, "http://test", time.Minute) + m.width, m.height = 120, 40 + m.connected = true + m.activeSection = sectionSchedule + m.scheduleWindow = time.Date(2026, 7, 27, 0, 0, 0, 0, time.UTC) + m.scheduleEntries = entries + m.scheduleDays = buildScheduleDays(m.scheduleWindow, entries) + m.rebuildScheduleTable() + return m +} + +func TestScheduleConflicts(t *testing.T) { + m := scheduledWeek([]api.ScheduleEntry{ + {ID: 1, Date: "2026-07-27", UserID: 1, Username: "niklas"}, + {ID: 2, Date: "2026-07-28", UserID: 3, Username: "sam"}, + {ID: 3, Date: "2026-07-29", UserID: 2, Username: "alex"}, + }) + week := []string{"2026-07-27", "2026-07-28", "2026-07-29", "2026-07-30"} + + // Assigning alex: the days niklas and sam hold are conflicts, the day alex + // already holds is not, and the free day is not. + taken, holders := m.scheduleConflicts(week, 2) + if len(taken) != 2 || taken[0] != "2026-07-27" || taken[1] != "2026-07-28" { + t.Errorf("expected the two other people's days, got %v", taken) + } + if len(holders) != 2 || holders[0] != "niklas" || holders[1] != "sam" { + t.Errorf("expected both holders named once, got %v", holders) + } +} + +// Reassigning somebody to a day they already hold takes nothing from anyone, so +// it must not raise a prompt — but it still needs replace, because the server +// rejects any date that already exists. +func TestScheduleConflicts_OwnDayIsNotAConflict(t *testing.T) { + m := scheduledWeek([]api.ScheduleEntry{ + {ID: 1, Date: "2026-07-27", UserID: 2, Username: "alex"}, + }) + dates := []string{"2026-07-27"} + + if taken, _ := m.scheduleConflicts(dates, 2); len(taken) != 0 { + t.Errorf("expected no conflict on the user's own day, got %v", taken) + } + if !m.scheduleOccupied(dates) { + t.Error("expected the day to still count as occupied, so replace is sent") + } +} + +func TestScheduleOccupied_FreeDays(t *testing.T) { + m := scheduledWeek(nil) + if m.scheduleOccupied([]string{"2026-07-27", "2026-07-28"}) { + t.Error("expected an empty rota to need no replace") + } +} + +func TestDayCount(t *testing.T) { + tests := []struct { + taken, total int + want string + }{ + {1, 1, "This day is"}, + {7, 7, "All 7 days are"}, + {3, 7, "3 of 7 days are"}, + } + for _, tt := range tests { + if got := dayCount(tt.taken, tt.total); got != tt.want { + t.Errorf("dayCount(%d, %d) = %q, want %q", tt.taken, tt.total, got, tt.want) + } + } +} + +func TestJoinNames(t *testing.T) { + tests := []struct { + names []string + want string + }{ + {nil, "somebody else"}, + {[]string{"niklas"}, "niklas"}, + {[]string{"niklas", "alex"}, "niklas and alex"}, + {[]string{"niklas", "alex", "sam"}, "niklas, alex and sam"}, + } + for _, tt := range tests { + if got := joinNames(tt.names); got != tt.want { + t.Errorf("joinNames(%v) = %q, want %q", tt.names, got, tt.want) + } + } +} diff --git a/internal/tui/update.go b/internal/tui/update.go index 8f75d69..5c7bd99 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -822,6 +822,7 @@ func (m Model) handleConfirmKey(msg tea.KeyMsg) (Model, tea.Cmd) { } m.pendingDeleteID = 0 m.pendingDeleteEntry = nil + m.pendingAssign = nil return m, nil } @@ -851,6 +852,17 @@ func (m Model) handleConfirmKey(msg tea.KeyMsg) (Model, tea.Cmd) { m.mode = modeDashboard m.usersLoading = true return m, deleteUserCmd(m.client, userID) + + case confirmReassignSchedule: + p := m.pendingAssign + m.mode = modeDashboard + m.pendingAssign = nil + if p == nil { + return m, nil + } + m.scheduleLoading = true + return m, assignScheduleCmd(m.client, p.userID, p.dates, true, + m.scheduleWindow, m.scheduleWindow.AddDate(0, 0, 6)) } return m, nil @@ -899,15 +911,84 @@ func (m Model) handleUserPickerKey(msg tea.KeyMsg) (Model, tea.Cmd) { } else { dates = []string{d.Format("2006-01-02")} } + + // The server refuses a date somebody else holds, so ask before taking + // it rather than letting the request come back 409. The answer is + // already on screen — no round trip is needed to work out who loses + // their shift. + taken, holders := m.scheduleConflicts(dates, user.ID) + if len(taken) > 0 { + m.pendingAssign = &pendingAssign{ + userID: user.ID, + username: user.Username, + dates: dates, + taken: taken, + holders: holders, + } + m.confirmTarget = confirmReassignSchedule + m.mode = modeConfirm + return m, nil + } + m.mode = modeDashboard m.scheduleLoading = true - return m, assignScheduleCmd(m.client, user.ID, dates, + // Nobody else loses anything, but the server rejects any date that + // already exists — including days this same person already holds, which + // is a no-op worth letting through silently. + return m, assignScheduleCmd(m.client, user.ID, dates, m.scheduleOccupied(dates), m.scheduleWindow, m.scheduleWindow.AddDate(0, 0, 6)) } return m, nil } +// scheduleConflicts reports which of dates are already held by somebody other +// than newUserID, and the distinct names holding them. +// +// Days the target already owns are not conflicts — reassigning somebody to +// their own shift takes nothing from anyone, and prompting for it would be +// noise. The server still needs replace for those, since it rejects any date +// that exists. +func (m Model) scheduleConflicts(dates []string, newUserID int64) (taken, holders []string) { + held := make(map[string]api.ScheduleEntry, len(m.scheduleDays)) + for _, d := range m.scheduleDays { + if d.entry != nil { + held[d.entry.Date] = *d.entry + } + } + seen := make(map[string]bool) + for _, date := range dates { + e, ok := held[date] + if !ok || e.UserID == newUserID { + continue + } + taken = append(taken, date) + if !seen[e.Username] { + seen[e.Username] = true + holders = append(holders, e.Username) + } + } + return taken, holders +} + +// scheduleOccupied reports whether any of dates already has an entry at all, +// including one belonging to the incoming user. That is what decides whether +// the request needs replace, as opposed to whether it needs confirming. +func (m Model) scheduleOccupied(dates []string) bool { + held := make(map[string]bool, len(m.scheduleDays)) + for _, d := range m.scheduleDays { + if d.entry != nil { + held[d.entry.Date] = true + } + } + for _, date := range dates { + if held[date] { + return true + } + } + return false +} + // ── User management ─────────────────────────────────────────────────────────── func (m Model) handleUserCreateKey(msg tea.KeyMsg) (Model, tea.Cmd) { diff --git a/internal/tui/update_test.go b/internal/tui/update_test.go index 3436fa8..453ed8a 100644 --- a/internal/tui/update_test.go +++ b/internal/tui/update_test.go @@ -309,6 +309,109 @@ func TestDeleteNote_ConfirmsThenActs(t *testing.T) { } } +// ── Schedule reassignment ───────────────────────────────────────────────── + +// pickingOnCall opens the user picker for the schedule day at dayIndex, which +// is where a reassignment actually starts. +func pickingOnCall(entries []api.ScheduleEntry, dayIndex int, week bool) Model { + m := scheduledWeek(entries) + m.users = []api.User{ + {ID: 1, Username: "niklas", Email: "n@example.com"}, + {ID: 2, Username: "alex", Email: "a@example.com"}, + } + m.rebuildUserPickerTable() + m.scheduleTable.SetCursor(dayIndex) + m.pickerAssignWeek = week + m.pickerTarget = pickerSchedule + m.mode = modeUserPicker + m.userPickerTable.SetCursor(1) // alex + return m +} + +// The bug: a day somebody already holds could not be handed to anybody else. +// The server refuses it, so the TUI has to ask first and then say so. +func TestSchedule_ReassigningATakenDayAsksFirst(t *testing.T) { + m := pickingOnCall([]api.ScheduleEntry{ + {ID: 1, Date: "2026-07-27", UserID: 1, Username: "niklas"}, + }, 0, false) + + m, cmd := press(t, m, "enter") + + if m.mode != modeConfirm || m.confirmTarget != confirmReassignSchedule { + t.Fatalf("expected a reassignment confirmation, got mode %v target %v", + m.mode, m.confirmTarget) + } + if cmd != nil { + t.Error("expected nothing sent to the server before confirming") + } + mustContain(t, m.confirmPrompt(), "This day is assigned to niklas", "Reassign to alex?") +} + +func TestSchedule_ReassignConfirmedSends(t *testing.T) { + m := pickingOnCall([]api.ScheduleEntry{ + {ID: 1, Date: "2026-07-27", UserID: 1, Username: "niklas"}, + }, 0, false) + m, _ = press(t, m, "enter") + + m, cmd := press(t, m, "y") + if cmd == nil { + t.Fatal("expected the confirmed reassignment to be sent") + } + if m.mode != modeDashboard { + t.Errorf("expected a return to the dashboard, got mode %v", m.mode) + } + if m.pendingAssign != nil { + t.Error("expected the pending assignment cleared") + } +} + +// Declining must leave the rota alone — that is the whole point of the guard. +func TestSchedule_ReassignDeclinedSendsNothing(t *testing.T) { + m := pickingOnCall([]api.ScheduleEntry{ + {ID: 1, Date: "2026-07-27", UserID: 1, Username: "niklas"}, + }, 0, false) + m, _ = press(t, m, "enter") + + m, cmd := press(t, m, "n") + if cmd != nil { + t.Error("expected nothing sent when the reassignment is declined") + } + if m.pendingAssign != nil { + t.Error("expected the pending assignment discarded") + } +} + +// A free day is the path that always worked, and must not grow a prompt. +func TestSchedule_AssigningAFreeDayDoesNotAsk(t *testing.T) { + m := pickingOnCall(nil, 0, false) + + m, cmd := press(t, m, "enter") + if m.mode != modeDashboard { + t.Errorf("expected no prompt for a free day, got mode %v", m.mode) + } + if cmd == nil { + t.Error("expected the assignment to be sent straight away") + } +} + +// The week case is the one that was worst: a single taken day rejected all +// seven. One prompt now covers the lot, and it says how much is being taken. +func TestSchedule_ReassigningAPartlyTakenWeekAsksOnce(t *testing.T) { + m := pickingOnCall([]api.ScheduleEntry{ + {ID: 1, Date: "2026-07-28", UserID: 1, Username: "niklas"}, + {ID: 2, Date: "2026-07-30", UserID: 3, Username: "sam"}, + }, 0, true) + + m, _ = press(t, m, "enter") + if m.confirmTarget != confirmReassignSchedule { + t.Fatalf("expected one confirmation for the week, got target %v", m.confirmTarget) + } + if got := len(m.pendingAssign.dates); got != 7 { + t.Errorf("expected all 7 days in the assignment, got %d", got) + } + mustContain(t, m.confirmPrompt(), "2 of 7 days are assigned to niklas and sam") +} + // ── Ntfy topic ──────────────────────────────────────────────────────────── // onUsers puts the model in the Users section with a loaded table. diff --git a/internal/tui/view.go b/internal/tui/view.go index 1f244a6..7043a66 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -178,10 +178,44 @@ func (m Model) confirmPrompt() string { return "Delete schedule entry? [y/N]" case confirmDeleteUser: return fmt.Sprintf("Delete user %s (cascades all API keys)? [y/N]", m.selectedUser.Username) + case confirmReassignSchedule: + if p := m.pendingAssign; p != nil { + return fmt.Sprintf("%s assigned to %s. Reassign to %s? [y/N]", + dayCount(len(p.taken), len(p.dates)), joinNames(p.holders), p.username) + } + return "Reassign these days? [y/N]" } return "Are you sure? [y/N]" } +// dayCount phrases how much of an assignment is being taken from somebody. A +// single day says so plainly; a partial week says which part, because "3 of 7" +// is the difference between taking a shift and taking somebody's whole week. +func dayCount(taken, total int) string { + switch { + case total == 1: + return "This day is" + case taken == total: + return fmt.Sprintf("All %d days are", total) + default: + return fmt.Sprintf("%d of %d days are", taken, total) + } +} + +// joinNames renders a list of people as prose. +func joinNames(names []string) string { + switch len(names) { + case 0: + return "somebody else" + case 1: + return names[0] + case 2: + return names[0] + " and " + names[1] + default: + return strings.Join(names[:len(names)-1], ", ") + " and " + names[len(names)-1] + } +} + // ── Dashboard ────────────────────────────────────────────────────────────── func (m Model) renderDashboard() string {