diff --git a/README.md b/README.md index 7687fab..f2caa94 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ The weekly schedule post is updated live when shifts change, and the previous we | `/oncall` | Show this week's schedule | | `/oncall next` | Show next week's schedule | | `/oncall pick ` | Pick up an available shift | -| `/oncall drop ` | Drop your shift (marks it available) | +| `/oncall drop ` | Drop your shift (marks it available) — blocked within 24h of shift start; swap or ask an admin instead | | `/oncall swap @person` | Request a swap — the other person gets an Accept/Decline DM and the shift only moves once they accept | | `/oncall register ` | Link your Slack account to your phone extension | | `/oncall roster` | Show all employees and their link status | diff --git a/src/shared/shared/blocks.py b/src/shared/shared/blocks.py index b974005..6edd5e8 100644 --- a/src/shared/shared/blocks.py +++ b/src/shared/shared/blocks.py @@ -204,7 +204,7 @@ def build_help_blocks(is_admin: bool = False) -> list[dict]: "`/oncall` — Show the two-week schedule\n" "`/oncall next` — Show the following two weeks\n" "`/oncall pick ` — Pick up a shift\n" - "`/oncall drop ` — Drop your shift (marks it open)\n" + "`/oncall drop ` — Drop your shift (marks it open; locked within 24h of start — swap instead)\n" "`/oncall swap @person` — Swap your shift with someone\n" "`/oncall register ` — Link your Slack account to your extension\n" "`/oncall pay` — Show last week's pay summary\n" diff --git a/src/slack-bot/app.py b/src/slack-bot/app.py index e69486f..5428995 100644 --- a/src/slack-bot/app.py +++ b/src/slack-bot/app.py @@ -115,6 +115,17 @@ def _shift_started(date_str: str, shift_type: str) -> bool: return datetime.now(EASTERN) >= _shift_start(date_str, shift_type) +def _within_drop_lock(date_str: str, shift_type: str) -> bool: + """True inside the 24h-before-start window where dropping a shift is locked. + + Within this window a shift can't be abandoned via drop — it must be handed + off through a verified swap (target accepts) or opened by an admin. + """ + return datetime.now(EASTERN) >= _shift_start(date_str, shift_type) - timedelta( + hours=24 + ) + + def _shift_type_label(day_name: str, shift_type: str) -> str: if day_name not in WEEKEND_DAYS: return "" @@ -511,10 +522,20 @@ def _handle_drop( return ext, name, source, shift_type = found - schedule.mark_open(date_str, shift_type) - if is_today(date_str) and _is_active_shift_type(shift_type): - _update_3cx_routing(FALLBACK_EXTENSION) + if _within_drop_lock(date_str, shift_type): + respond( + text=( + "This shift starts in under 24 hours — you can't drop it now. " + "Hand it off with `/oncall swap @person` (they'll need to accept), " + "or ask an admin to open it." + ) + ) + return + + # No 3CX repoint here: a same-day shift is always inside the 24h lock above, + # so a drop that reaches this point is never today's active shift. + schedule.mark_open(date_str, shift_type) date_label = date.strftime("%A, %b %-d") shift_label = _shift_type_label(day_name, shift_type) diff --git a/tests/slack_bot/test_handle_drop.py b/tests/slack_bot/test_handle_drop.py index e399097..6bf338f 100644 --- a/tests/slack_bot/test_handle_drop.py +++ b/tests/slack_bot/test_handle_drop.py @@ -1,72 +1,124 @@ -"""Tests for slack-bot _handle_drop.""" +"""Tests for slack-bot _handle_drop, including the 24h drop lock (#84).""" from freezegun import freeze_time -from shared.schedule import FALLBACK_EXTENSION - -# Monday 2026-06-01 08:00 ET — weekday, active shift is night. -MON = "2026-06-01 12:00:00" +# Monday 2026-06-01 08:00 ET. +MON_0800 = "2026-06-01 12:00:00" +# Monday 2026-06-01 18:00 ET — past 17:00 (so tomorrow's night shift is <24h away). +MON_1800 = "2026-06-01 22:00:00" +# Friday 2026-06-05. +FRI_0600 = "2026-06-05 10:00:00" # 06:00 ET +FRI_0900 = "2026-06-05 13:00:00" # 09:00 ET -def _register_on_monday(seed): - """Alice (114) is registered and assigned the Monday night shift.""" - seed.roster("114", "Alice", slack_user_id="U_ALICE") - seed.weekly("Monday", "114", "Alice") - - -@freeze_time(MON) -def test_drop_today_marks_open_and_repoints_3cx( +@freeze_time(MON_0800) +def test_drop_outside_24h_marks_open( slackbot_app, schedule, seed, respond, client, routing_spy, text_of ): - _register_on_monday(seed) - slackbot_app._handle_drop( - respond, schedule, "U_ALICE", "drop today", "C1", client, None - ) - - assert schedule.get_override("2026-06-01")["extension"] == "OPEN" - assert "dropped" in text_of(respond) - # Today + active night shift → queue falls back. - routing_spy.assert_called_once_with(FALLBACK_EXTENSION) - client.chat_postMessage.assert_called_once() - - -@freeze_time(MON) -def test_drop_future_does_not_repoint_3cx( - slackbot_app, schedule, seed, respond, client, routing_spy -): + # Tomorrow's night shift starts 06-02 17:00; now is 06-01 08:00 → ~33h away. seed.roster("114", "Alice", slack_user_id="U_ALICE") seed.weekly("Tuesday", "114", "Alice") slackbot_app._handle_drop( respond, schedule, "U_ALICE", "drop tomorrow", "C1", client, None ) - assert schedule.get_override("2026-06-02")["extension"] == "OPEN" + assert "dropped" in text_of(respond) + client.chat_postMessage.assert_called_once() routing_spy.assert_not_called() -@freeze_time(MON) -def test_drop_not_your_shift( +@freeze_time(MON_0800) +def test_drop_today_blocked( slackbot_app, schedule, seed, respond, client, routing_spy, text_of ): + # A same-day shift is always inside the 24h lock. + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.weekly("Monday", "114", "Alice") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop today", "C1", client, None + ) + assert "under 24 hours" in text_of(respond) + assert schedule.get_override("2026-06-01") is None # not opened + routing_spy.assert_not_called() + client.chat_postMessage.assert_not_called() + + +@freeze_time(MON_1800) +def test_drop_blocked_once_within_24h_of_start( + slackbot_app, schedule, seed, respond, client, text_of +): + # At 06-01 18:00, tomorrow's 06-02 17:00 shift is <24h away. + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.weekly("Tuesday", "114", "Alice") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop tomorrow", "C1", client, None + ) + assert "under 24 hours" in text_of(respond) + assert schedule.get_override("2026-06-02") is None + + +@freeze_time(FRI_0600) +def test_drop_weekend_day_shift_outside_24h( + slackbot_app, schedule, seed, respond, client, text_of +): + # Saturday day shift starts 06-06 08:00; now Fri 06:00 → >24h. + seed.roster("200", "Alice", slack_user_id="U_ALICE") + seed.weekly("Saturday", "200", "Alice", shift_type="day") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop saturday", "C1", client, None + ) + assert "dropped" in text_of(respond) + assert schedule.get_override("2026-06-06", "day")["extension"] == "OPEN" + + +@freeze_time(FRI_0900) +def test_drop_weekend_day_shift_within_24h( + slackbot_app, schedule, seed, respond, client, text_of +): + # Now Fri 09:00 → Saturday 08:00 day shift is <24h away. + seed.roster("200", "Alice", slack_user_id="U_ALICE") + seed.weekly("Saturday", "200", "Alice", shift_type="day") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop saturday", "C1", client, None + ) + assert "under 24 hours" in text_of(respond) + assert schedule.get_override("2026-06-06", "day") is None + + +@freeze_time(MON_0800) +def test_admin_open_bypasses_24h_lock( + slackbot_app, schedule, seed, respond, client, routing_spy, text_of +): + # Admins can still open a same-day shift — the lock only applies to user drop. + seed.weekly("Monday", "114", "Alice") + slackbot_app._handle_admin( + respond, schedule, "U_ADMIN", "admin open today", True, client, None + ) + assert schedule.get_override("2026-06-01")["extension"] == "OPEN" + assert "marked as open" in text_of(respond).lower() + + +@freeze_time(MON_0800) +def test_drop_not_your_shift(slackbot_app, schedule, seed, respond, client, text_of): seed.roster("114", "Alice", slack_user_id="U_ALICE") seed.weekly("Monday", "115", "Bob") # Bob is on shift, not Alice slackbot_app._handle_drop( respond, schedule, "U_ALICE", "drop today", "C1", client, None ) assert "not your shift" in text_of(respond).lower() - routing_spy.assert_not_called() -@freeze_time(MON) +@freeze_time(MON_0800) def test_drop_past_date(slackbot_app, schedule, seed, respond, client, text_of): - _register_on_monday(seed) + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.weekly("Monday", "114", "Alice") slackbot_app._handle_drop( respond, schedule, "U_ALICE", "drop 2026-05-01", "C1", client, None ) assert "past" in text_of(respond).lower() -@freeze_time(MON) +@freeze_time(MON_0800) def test_drop_unregistered(slackbot_app, schedule, respond, client, text_of): slackbot_app._handle_drop( respond, schedule, "U_NOBODY", "drop today", "C1", client, None @@ -74,16 +126,17 @@ def test_drop_unregistered(slackbot_app, schedule, respond, client, text_of): assert "not registered" in text_of(respond).lower() -@freeze_time(MON) +@freeze_time(MON_0800) def test_drop_bad_date(slackbot_app, schedule, seed, respond, client, text_of): - _register_on_monday(seed) + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.weekly("Monday", "114", "Alice") slackbot_app._handle_drop( respond, schedule, "U_ALICE", "drop notaday", "C1", client, None ) assert "couldn't parse" in text_of(respond).lower() -@freeze_time(MON) +@freeze_time(MON_0800) def test_drop_usage(slackbot_app, schedule, respond, client, text_of): slackbot_app._handle_drop(respond, schedule, "U_ALICE", "drop", "C1", client, None) assert "Usage" in text_of(respond) diff --git a/tests/slack_bot/test_helpers.py b/tests/slack_bot/test_helpers.py index c3f1113..7378edf 100644 --- a/tests/slack_bot/test_helpers.py +++ b/tests/slack_bot/test_helpers.py @@ -7,6 +7,34 @@ MON = "2026-06-01 12:00:00" SAT_DATE = "2026-06-06" +class TestShiftTiming: + def test_shift_start_weekday_night_is_5pm(self, slackbot_app): + assert slackbot_app._shift_start("2026-06-03", "night").hour == 17 + + def test_shift_start_weekend_day_is_8am(self, slackbot_app): + assert slackbot_app._shift_start("2026-06-06", "day").hour == 8 + + @freeze_time("2026-06-01 22:00:00") # 18:00 ET + def test_shift_started_true_after_start(self, slackbot_app): + assert slackbot_app._shift_started("2026-06-01", "night") is True + + @freeze_time(MON) # 08:00 ET + def test_shift_started_false_before_start(self, slackbot_app): + assert slackbot_app._shift_started("2026-06-01", "night") is False + + @freeze_time(MON) + def test_within_drop_lock_for_today(self, slackbot_app): + assert slackbot_app._within_drop_lock("2026-06-01", "night") is True + + @freeze_time(MON) # tomorrow 17:00 is ~33h away + def test_not_within_lock_tomorrow_morning(self, slackbot_app): + assert slackbot_app._within_drop_lock("2026-06-02", "night") is False + + @freeze_time("2026-06-01 22:00:00") # 18:00 ET, tomorrow 17:00 now <24h + def test_within_lock_once_under_24h(self, slackbot_app): + assert slackbot_app._within_drop_lock("2026-06-02", "night") is True + + class TestIsToday: @freeze_time(MON) def test_true_for_today(self, slackbot_app):