From 55ab07b864d524b82e8e36dd7a9fbc8af10ae98a Mon Sep 17 00:00:00 2001 From: amoussa1229 <166072409+amoussa1229@users.noreply.github.com> Date: Fri, 26 Jun 2026 18:39:42 +0000 Subject: [PATCH] Let drop pick a shift and flag night rows Drop now accepts an optional [day|night|holiday] qualifier and, when a date carries more than one shift the user holds, asks which to drop instead of silently releasing the holiday or weekend day shift. The static post also tags day/night rows with distinct glyphs and labels on weekdays, so the after-hours row is unmistakable. Refs: #134 --- src/shared/shared/blocks.py | 13 ++- src/slack-bot/app.py | 135 ++++++++++++++++++++++++---- tests/shared/test_blocks.py | 15 ++++ tests/slack_bot/test_handle_drop.py | 114 +++++++++++++++++++++++ 4 files changed, 256 insertions(+), 21 deletions(-) diff --git a/src/shared/shared/blocks.py b/src/shared/shared/blocks.py index 5760db3..dda207a 100644 --- a/src/shared/shared/blocks.py +++ b/src/shared/shared/blocks.py @@ -70,6 +70,13 @@ SHIFT_LABELS = { "night": "Night (5pm–8am)", } +# Per-shift glyphs so day vs. night rows are distinguishable at a glance — even +# on weekdays, where only the night (after-hours) shift exists. +SHIFT_GLYPHS = { + "day": ":sunny:", + "night": ":crescent_moon:", +} + HOLIDAY_BADGE = ":palm_tree: *Holiday*" @@ -88,8 +95,8 @@ def _format_shift_line( shift_type: str = "night", ) -> str: day_label = date.strftime("%a %b %-d") - if date.strftime("%A") in WEEKEND_DAYS and shift_type in SHIFT_LABELS: - day_label += f" {SHIFT_LABELS[shift_type]}" + if shift_type in SHIFT_LABELS: + day_label += f" {SHIFT_GLYPHS[shift_type]} {SHIFT_LABELS[shift_type]}" if is_today: day_label = f"*{day_label} (today)*" @@ -430,7 +437,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; locked within 24h of start — swap instead)\n" + "`/oncall drop [day|night|holiday]` — 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 a446891..2a537b5 100644 --- a/src/slack-bot/app.py +++ b/src/slack-bot/app.py @@ -363,6 +363,34 @@ def _find_employee_shift(schedule, date_str, day_name, employee_ext): return None +_DROP_SHIFT_LABELS = { + "holiday": "holiday", + "day": "day (8am–5pm)", + "night": "night (5pm–8am)", +} + + +def _droppable_shifts(schedule, date_str, day_name, employee_ext) -> list[str]: + """Return the shift types the employee holds on a date and could drop. + + Possible values, in display priority: ``holiday``, ``day``, ``night``. A + holiday day-slot supersedes a regular weekend day shift on its date. + """ + held = [] + holiday_ctx = schedule.get_shift_context(date_str, day_name, "day") + if holiday_ctx["kind"] == "holiday": + if any(a["extension"] == employee_ext for a in holiday_ctx["assignees"]): + held.append("holiday") + elif day_name in WEEKEND_DAYS: + ext, _name, _source = schedule.resolve_shift(date_str, day_name, "day") + if ext == employee_ext: + held.append("day") + ext, _name, _source = schedule.resolve_shift(date_str, day_name, "night") + if ext == employee_ext: + held.append("night") + return held + + def _refresh_schedule_post(schedule, schedule_channel, client): """Update the pinned schedule message in-place after a shift change.""" channel = schedule_channel @@ -919,9 +947,11 @@ def _pick_holiday( def _handle_drop( respond, schedule, user_id, text, channel_id, client, schedule_channel ): - parts = text.split(maxsplit=1) + parts = text.split() if len(parts) < 2: - respond(text="Usage: `/oncall drop ` (e.g. `/oncall drop friday`)") + respond( + text="Usage: `/oncall drop [day|night|holiday]` (e.g. `/oncall drop friday`)" + ) return employee = schedule.get_employee_by_slack_id(user_id) @@ -929,10 +959,24 @@ def _handle_drop( respond(text="You're not registered. Use `/oncall register ` first.") return - date = parse_date(parts[1]) + # An optional trailing qualifier picks the shift to drop; the date itself may + # be multiple words (e.g. `jul 3`), so strip the qualifier off the end first. + tokens = parts[1:] + explicit_shift = None + if tokens[-1] in ("day", "night", "holiday"): + explicit_shift = tokens[-1] + tokens = tokens[:-1] + if not tokens: + respond( + text="Usage: `/oncall drop [day|night|holiday]` (e.g. `/oncall drop friday`)" + ) + return + + date_text = " ".join(tokens) + date = parse_date(date_text) if not date: respond( - text=f"Couldn't parse date: `{parts[1]}`. Try: today, tomorrow, friday, 4/5, 7/3/26, jul 3, 2026-04-05" + text=f"Couldn't parse date: `{date_text}`. Try: today, tomorrow, friday, 4/5, 7/3/26, jul 3, 2026-04-05" ) return @@ -942,13 +986,43 @@ def _handle_drop( return day_name = date.strftime("%A") + held = _droppable_shifts(schedule, date_str, day_name, employee["extension"]) + date_label = date.strftime("%A, %b %-d") - # A holiday slot the employee holds is dropped (released) ahead of regular - # day/night shifts — holidays take priority on their date. - holiday_ctx = schedule.get_shift_context(date_str, day_name, "day") - if holiday_ctx["kind"] == "holiday" and any( - a["extension"] == employee["extension"] for a in holiday_ctx["assignees"] - ): + if explicit_shift: + if explicit_shift not in held: + if held: + options = ", ".join(_DROP_SHIFT_LABELS[s] for s in held) + respond( + text=( + f"You don't hold the {_DROP_SHIFT_LABELS[explicit_shift]} " + f"shift on *{date_label}*. You hold: {options}." + ) + ) + else: + ext, name, _source = schedule.resolve_shift(date_str, day_name) + respond( + text=f"That's not your shift — it belongs to {name} (Ext {ext})." + ) + return + target = explicit_shift + elif not held: + ext, name, _source = schedule.resolve_shift(date_str, day_name) + respond(text=f"That's not your shift — it belongs to {name} (Ext {ext}).") + return + elif len(held) > 1: + options = ", ".join(_DROP_SHIFT_LABELS[s] for s in held) + respond( + text=( + f"You hold more than one shift on *{date_label}*: {options}. " + f"Tell me which to drop: `/oncall drop {date_text} [{'|'.join(held)}]`." + ) + ) + return + else: + target = held[0] + + if target == "holiday": _drop_holiday( respond, schedule, @@ -961,15 +1035,33 @@ def _handle_drop( ) return - found = _find_employee_shift(schedule, date_str, day_name, employee["extension"]) + _drop_regular( + respond, + schedule, + employee, + date, + date_str, + day_name, + target, + channel_id, + client, + schedule_channel, + ) - if not found: - ext, name, _source = schedule.resolve_shift(date_str, day_name) - respond(text=f"That's not your shift — it belongs to {name} (Ext {ext}).") - return - - ext, name, source, shift_type = found +def _drop_regular( + respond, + schedule, + employee, + date, + date_str, + day_name, + shift_type, + channel_id, + client, + schedule_channel, +): + """Release a regular (weekend day or night) shift, honouring the 24h lock.""" if _within_drop_lock(date_str, shift_type): respond( text=( @@ -980,6 +1072,8 @@ def _handle_drop( ) return + ext, name, _source = schedule.resolve_shift(date_str, day_name, shift_type) + # 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) @@ -994,7 +1088,12 @@ def _handle_drop( ) blocks = build_shift_change_message( - user_id, date_str, "dropped", ext, name, shift_type=shift_type + employee.get("slack_user_id", ""), + date_str, + "dropped", + ext, + name, + shift_type=shift_type, ) try: client.chat_postMessage( diff --git a/tests/shared/test_blocks.py b/tests/shared/test_blocks.py index dc00bef..6e8a8b4 100644 --- a/tests/shared/test_blocks.py +++ b/tests/shared/test_blocks.py @@ -51,6 +51,21 @@ class TestBuildWeekSchedule: assert "pickup_2026-06-03" not in _all_action_ids(blocks) assert "Alice (Ext 114)" in blocks[1]["text"]["text"] + @freeze_time("2026-06-01 12:00:00") + def test_weekday_night_row_shows_night_glyph_and_label(self, schedule, seed): + # A weekday (after-hours) night row is now unmistakable: moon glyph + label. + seed.weekly("Wednesday", "114", "Alice") + text = build_week_schedule(schedule)[1]["text"]["text"] + assert ":crescent_moon:" in text + assert "Night (5pm" in text + + @freeze_time("2026-06-01 12:00:00") + def test_weekend_day_row_shows_day_glyph_and_label(self, schedule, seed): + seed.weekly("Saturday", "200", "Alice", shift_type="day") + text = build_week_schedule(schedule)[1]["text"]["text"] + assert ":sunny:" in text + assert "Day (8am" in text + @freeze_time("2026-06-01 12:00:00") def test_holiday_on_weekday_renders_badge_and_open_slots(self, schedule, seed): # A holiday can land on a weekday (here a Thursday) and shows the badge, diff --git a/tests/slack_bot/test_handle_drop.py b/tests/slack_bot/test_handle_drop.py index 6bf338f..382f1ab 100644 --- a/tests/slack_bot/test_handle_drop.py +++ b/tests/slack_bot/test_handle_drop.py @@ -140,3 +140,117 @@ def test_drop_bad_date(slackbot_app, schedule, seed, respond, client, text_of): 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) + + +# ── shift disambiguation (#134) ────────────────────────────────────────────── + + +@freeze_time(MON_0800) +def test_ambiguous_holiday_and_night_prompts( + slackbot_app, schedule, seed, respond, client, text_of +): + # Thursday 2026-06-04: Alice holds both a holiday slot and the night shift. + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.holiday( + "2026-06-04", + slots=2, + label="Test Holiday", + assignees={"114": {"name": "Alice", "claimed_at": "x"}}, + ) + seed.weekly("Thursday", "114", "Alice") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop 2026-06-04", "C1", client, None + ) + msg = text_of(respond) + assert "more than one" in msg.lower() + assert "holiday" in msg.lower() and "night" in msg.lower() + # Nothing was dropped. + assert "114" in schedule.get_holiday("2026-06-04")["assignees"] + assert schedule.get_override("2026-06-04") is None + client.chat_postMessage.assert_not_called() + + +@freeze_time(MON_0800) +def test_explicit_night_drops_night_not_holiday( + slackbot_app, schedule, seed, respond, client, text_of +): + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.holiday( + "2026-06-04", + slots=2, + label="Test Holiday", + assignees={"114": {"name": "Alice", "claimed_at": "x"}}, + ) + seed.weekly("Thursday", "114", "Alice") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop 2026-06-04 night", "C1", client, None + ) + assert "dropped" in text_of(respond) + assert schedule.get_override("2026-06-04")["extension"] == "OPEN" # night opened + assert "114" in schedule.get_holiday("2026-06-04")["assignees"] # holiday kept + + +@freeze_time(MON_0800) +def test_explicit_holiday_drops_holiday_not_night( + slackbot_app, schedule, seed, respond, client, text_of +): + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.holiday( + "2026-06-04", + slots=2, + label="Test Holiday", + assignees={"114": {"name": "Alice", "claimed_at": "x"}}, + ) + seed.weekly("Thursday", "114", "Alice") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop 2026-06-04 holiday", "C1", client, None + ) + assert "dropped your slot" in text_of(respond).lower() + assert "114" not in schedule.get_holiday("2026-06-04")["assignees"] + assert schedule.get_override("2026-06-04") is None # night untouched + + +@freeze_time(MON_0800) +def test_ambiguous_weekend_day_and_night_prompts( + slackbot_app, schedule, seed, respond, client, text_of +): + # Saturday 2026-06-06: Alice holds both the day and night shift. + seed.roster("200", "Alice", slack_user_id="U_ALICE") + seed.weekly("Saturday", "200", "Alice", shift_type="day") + seed.weekly("Saturday", "200", "Alice", shift_type="night") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop saturday", "C1", client, None + ) + assert "more than one" in text_of(respond).lower() + assert schedule.get_override("2026-06-06", "day") is None + assert schedule.get_override("2026-06-06") is None + + +@freeze_time(MON_0800) +def test_explicit_weekend_night_drops_night( + slackbot_app, schedule, seed, respond, client, text_of +): + seed.roster("200", "Alice", slack_user_id="U_ALICE") + seed.weekly("Saturday", "200", "Alice", shift_type="day") + seed.weekly("Saturday", "200", "Alice", shift_type="night") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop saturday night", "C1", client, None + ) + assert "dropped" in text_of(respond) + assert schedule.get_override("2026-06-06")["extension"] == "OPEN" # night opened + assert schedule.get_override("2026-06-06", "day") is None # day untouched + + +@freeze_time(MON_0800) +def test_explicit_qualifier_not_held_is_rejected( + slackbot_app, schedule, seed, respond, client, text_of +): + # Alice holds only the (weekday) night shift; asking to drop the day shift + # is rejected rather than silently dropping the night shift. + seed.roster("114", "Alice", slack_user_id="U_ALICE") + seed.weekly("Tuesday", "114", "Alice") + slackbot_app._handle_drop( + respond, schedule, "U_ALICE", "drop tomorrow day", "C1", client, None + ) + assert "don't hold" in text_of(respond).lower() + assert schedule.get_override("2026-06-02") is None