From 17dcc0b2642c24884d3dd9aee01dd7ce8f86e6e1 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Fri, 25 Sep 2026 16:26:45 -0400 Subject: [PATCH] feat(portal): store an optional note on swap requests (IP-132) --- openapi.yaml | 6 ++++ src/shared/shared/blocks.py | 20 ++++++++--- src/shared/shared/portal_http.py | 1 + src/shared/shared/portal_ops.py | 29 ++++++++++++++-- src/shared/shared/schedule.py | 35 ++++++++++--------- src/shared/shared/side_effects.py | 8 +++-- tests/shared/test_blocks.py | 8 +++++ tests/shared/test_portal_ops.py | 58 +++++++++++++++++++++++++++++++ 8 files changed, 140 insertions(+), 25 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index aa4adac..a29be73 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -542,6 +542,9 @@ components: targetExtension: type: string minLength: 1 + note: + type: string + maxLength: 500 AdminOverrideBody: type: object @@ -698,6 +701,9 @@ components: type: string incoming: type: boolean + note: + type: string + maxLength: 500 PendingPickup: type: object diff --git a/src/shared/shared/blocks.py b/src/shared/shared/blocks.py index 8e77914..9005ac5 100644 --- a/src/shared/shared/blocks.py +++ b/src/shared/shared/blocks.py @@ -254,8 +254,15 @@ def build_shift_change_message( return [{"type": "section", "text": {"type": "mrkdwn", "text": text}}] +def _mrkdwn_text(value: str) -> str: + return value.replace("&", "&").replace("<", "<").replace(">", ">") + + def build_swap_request_blocks( - requester_slack: str, date_str: str, shift_type: str = "night" + requester_slack: str, + date_str: str, + shift_type: str = "night", + note: str | None = None, ) -> list[dict]: """Build the interactive Accept / Decline message DMed to a swap target.""" dt = datetime.strptime(date_str, "%Y-%m-%d") @@ -266,15 +273,18 @@ def build_swap_request_blocks( else "" ) action_suffix = "_day" if shift_type == "day" else "" + text = ( + f"<@{requester_slack}> wants you to cover the " + f"*{day_label}*{type_label} shift. Accept to take it on." + ) + if note: + text += f"\n\nNote: {_mrkdwn_text(note)}" return [ { "type": "section", "text": { "type": "mrkdwn", - "text": ( - f"<@{requester_slack}> wants you to cover the " - f"*{day_label}*{type_label} shift. Accept to take it on." - ), + "text": text, }, }, { diff --git a/src/shared/shared/portal_http.py b/src/shared/shared/portal_http.py index 1ee7108..c555141 100644 --- a/src/shared/shared/portal_http.py +++ b/src/shared/shared/portal_http.py @@ -100,6 +100,7 @@ def dispatch( body.get("date", ""), body.get("targetExtension", ""), body.get("shiftType"), + note=body.get("note"), ) if ( method == "POST" diff --git a/src/shared/shared/portal_ops.py b/src/shared/shared/portal_ops.py index cb19da5..b9e4882 100644 --- a/src/shared/shared/portal_ops.py +++ b/src/shared/shared/portal_ops.py @@ -160,9 +160,25 @@ def snapshot(schedule: ShiftSchedule, employee: dict, week: str = "this") -> dic return payload +NOTE_MAX = 500 + + +def _clean_swap_note(note: str | None) -> str | None: + if note is None: + return None + if not isinstance(note, str): + raise ActionError(400, "INVALID_NOTE", "Note must be text.") + cleaned = note.strip() + if not cleaned: + return None + if len(cleaned) > NOTE_MAX: + raise ActionError(400, "NOTE_TOO_LONG", "Note must be 500 characters or fewer.") + return cleaned + + def _swap_payload(item: dict, my_ext: str) -> dict: date_str, shift_type = _sk_to_date_shift(item.get("SK", "")) - return { + payload = { "date": date_str, "shiftType": item.get("shift_type") or shift_type, "requesterExt": item.get("requester_ext", ""), @@ -171,6 +187,10 @@ def _swap_payload(item: dict, my_ext: str) -> dict: "targetName": item.get("target_name", ""), "incoming": item.get("target_ext") == my_ext, } + note = item.get("note") + if isinstance(note, str) and note.strip(): + payload["note"] = note.strip() + return payload def _pickup_payload(item: dict) -> dict: @@ -419,6 +439,7 @@ def swap( date_str: str, target_extension: str, shift_type: str | None, + note: str | None = None, ) -> dict: date = _parse_date(date_str) date_str = date.strftime(DATE_FMT) @@ -454,8 +475,11 @@ def swap( raise ActionError(400, "UNKNOWN_TARGET", "That extension is not on the roster.") if target["extension"] == employee["extension"]: raise ActionError(400, "SELF_SWAP", "That shift is already yours.") + cleaned_note = _clean_swap_note(note) expires_at = int(shift_start(date_str, resolved).timestamp()) - schedule.create_pending_swap(date_str, resolved, employee, target, expires_at) + schedule.create_pending_swap( + date_str, resolved, employee, target, expires_at, note=cleaned_note + ) token = effects.slack_token() if target.get("slack_user_id"): effects.dm_swap_request( @@ -465,6 +489,7 @@ def swap( date_str, resolved, employee["name"], + note=cleaned_note, ) return { "ok": True, diff --git a/src/shared/shared/schedule.py b/src/shared/shared/schedule.py index 477b65c..ef08c39 100644 --- a/src/shared/shared/schedule.py +++ b/src/shared/shared/schedule.py @@ -261,29 +261,32 @@ class ShiftSchedule: requester: dict, target: dict, expires_at: int, + note: str | None = None, ) -> None: """Create (or supersede) a pending swap request for a shift. One swap per shift (unique SK), so a new request overwrites any prior pending one. ``expires_at`` is an epoch timestamp used for DynamoDB TTL. + ``note`` is omitted when empty so Slack ``/oncall swap`` stays unchanged. """ sk = f"{date_str}-DAY" if shift_type == "day" else date_str - self.table.put_item( - Item={ - "PK": "SWAP", - "SK": sk, - "shift_type": shift_type, - "status": "pending", - "requester_ext": requester["extension"], - "requester_name": requester["name"], - "requester_slack": requester.get("slack_user_id", ""), - "target_ext": target["extension"], - "target_name": target["name"], - "target_slack": target.get("slack_user_id", ""), - "created_at": datetime.now(EASTERN).isoformat(), - "expires_at": expires_at, - } - ) + item = { + "PK": "SWAP", + "SK": sk, + "shift_type": shift_type, + "status": "pending", + "requester_ext": requester["extension"], + "requester_name": requester["name"], + "requester_slack": requester.get("slack_user_id", ""), + "target_ext": target["extension"], + "target_name": target["name"], + "target_slack": target.get("slack_user_id", ""), + "created_at": datetime.now(EASTERN).isoformat(), + "expires_at": expires_at, + } + if note: + item["note"] = note + self.table.put_item(Item=item) def get_swap(self, date_str: str, shift_type: str = "night") -> dict | None: sk = f"{date_str}-DAY" if shift_type == "day" else date_str diff --git a/src/shared/shared/side_effects.py b/src/shared/shared/side_effects.py index b83f8a0..6576d3d 100644 --- a/src/shared/shared/side_effects.py +++ b/src/shared/shared/side_effects.py @@ -272,15 +272,19 @@ def dm_swap_request( date_str: str, shift_type: str, requester_name: str, + note: str | None = None, ) -> bool: if not token or not target_slack: return False + text = f"{requester_name} wants to swap you the {date_str} shift" + if note: + text += f"\nNote: {note}" return slack_call( "chat.postMessage", token, channel=target_slack, - blocks=build_swap_request_blocks(requester_slack, date_str, shift_type), - text=f"{requester_name} wants to swap you the {date_str} shift", + blocks=build_swap_request_blocks(requester_slack, date_str, shift_type, note), + text=text, ) diff --git a/tests/shared/test_blocks.py b/tests/shared/test_blocks.py index cd25d9b..01bc392 100644 --- a/tests/shared/test_blocks.py +++ b/tests/shared/test_blocks.py @@ -259,6 +259,14 @@ class TestBuildSwapRequestBlocks: assert elements[0]["style"] == "primary" # Accept assert elements[1]["style"] == "danger" # Decline + def test_note_is_appended_without_changing_actions(self): + blocks = build_swap_request_blocks( + "U_REQ", "2026-06-03", "night", note="Family & more" + ) + assert "Note: Family <commitment> & more" in blocks[0]["text"]["text"] + ids = self._action_ids(blocks) + assert ids == ["swap_accept_2026-06-03", "swap_decline_2026-06-03"] + class TestBuildSwapResolvedBlocks: def test_renders_text_no_buttons(self): diff --git a/tests/shared/test_portal_ops.py b/tests/shared/test_portal_ops.py index 9853318..6f3f803 100644 --- a/tests/shared/test_portal_ops.py +++ b/tests/shared/test_portal_ops.py @@ -75,6 +75,64 @@ def test_swap_creates_pending(schedule, seed, quiet_slack): assert snap["pendingSwaps"][0]["incoming"] is True mine = snapshot(schedule, employee) assert mine["pendingSwaps"][0]["incoming"] is False + assert "note" not in pending + assert "note" not in snap["pendingSwaps"][0] + + +def test_swap_stores_returns_and_dms_note(schedule, seed, quiet_slack, monkeypatch): + employee = _alice(schedule, seed) + seed.override("2026-06-12", "114", "Alice") + calls = [] + + def capture(*args, **kwargs): + calls.append((args, kwargs)) + return True + + monkeypatch.setattr(effects, "dm_swap_request", capture) + with freezegun.freeze_time("2026-06-08 12:00:00-04:00"): + swap( + schedule, + employee, + "2026-06-12", + "115", + "night", + note=" Family commitment ", + ) + pending = schedule.get_swap("2026-06-12", "night") + assert pending["note"] == "Family commitment" + snap = snapshot(schedule, employee) + assert snap["pendingSwaps"][0]["note"] == "Family commitment" + assert calls[0][1]["note"] == "Family commitment" + + +def test_swap_omits_blank_note(schedule, seed, quiet_slack): + employee = _alice(schedule, seed) + seed.override("2026-06-12", "114", "Alice") + with freezegun.freeze_time("2026-06-08 12:00:00-04:00"): + swap(schedule, employee, "2026-06-12", "115", "night", note=" ") + pending = schedule.get_swap("2026-06-12", "night") + assert "note" not in pending + snap = snapshot(schedule, employee) + assert "note" not in snap["pendingSwaps"][0] + + +def test_swap_rejects_long_note(schedule, seed, quiet_slack): + employee = _alice(schedule, seed) + seed.override("2026-06-12", "114", "Alice") + with freezegun.freeze_time("2026-06-08 12:00:00-04:00"): + with pytest.raises(ActionError) as err: + swap(schedule, employee, "2026-06-12", "115", "night", note="a" * 501) + assert err.value.code == "NOTE_TOO_LONG" + assert schedule.get_swap("2026-06-12", "night") is None + + +def test_swap_rejects_non_string_note(schedule, seed, quiet_slack): + employee = _alice(schedule, seed) + seed.override("2026-06-12", "114", "Alice") + with freezegun.freeze_time("2026-06-08 12:00:00-04:00"): + with pytest.raises(ActionError) as err: + swap(schedule, employee, "2026-06-12", "115", "night", note=12) + assert err.value.code == "INVALID_NOTE" def test_late_pickup_creates_request(schedule, seed, quiet_slack):