feat(portal): store an optional note on swap requests (IP-132) (#283)

* feat(portal): store an optional note on swap requests (IP-132)

* fix(portal): address review feedback

* fix(portal): address review feedback

openapi.yaml

@@ -542,6 +542,9 @@ components:
         targetExtension:
           type: string
           minLength: 1
+        note:
+          type: string
+          description: Optional. Stored after trim, and the trimmed value must be 500 characters or fewer.
 
     AdminOverrideBody:
       type: object
@@ -698,6 +701,9 @@ components:
           type: string
         incoming:
           type: boolean
+        note:
+          type: string
+          maxLength: 500
 
     PendingPickup:
       type: object

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("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
+
+
 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,
             },
         },
         {

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"

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,

src/shared/shared/schedule.py

@@ -261,15 +261,16 @@ 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={
+        item = {
             "PK": "SWAP",
             "SK": sk,
             "shift_type": shift_type,
@@ -283,7 +284,9 @@ class ShiftSchedule:
             "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

src/shared/shared/side_effects.py

@@ -15,6 +15,7 @@ from shared.blocks import (
     build_holiday_added_blocks,
     build_pickup_request_blocks,
     build_shift_change_message,
+    _mrkdwn_text,
     build_swap_request_blocks,
     build_week_schedule,
 )
@@ -272,15 +273,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: {_mrkdwn_text(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,
     )
 
 

tests/shared/test_blocks.py

@@ -259,6 +259,37 @@ 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 <commitment> & more"
+        )
+        assert "Note: Family &lt;commitment&gt; &amp; more" in blocks[0]["text"]["text"]
+        ids = self._action_ids(blocks)
+        assert ids == ["swap_accept_2026-06-03", "swap_decline_2026-06-03"]
+
+
+def test_dm_swap_fallback_escapes_the_note(monkeypatch):
+    from shared import side_effects as effects
+
+    captured = {}
+
+    def fake_slack_call(_method, _token, **kwargs):
+        captured.update(kwargs)
+        return True
+
+    monkeypatch.setattr(effects, "slack_call", fake_slack_call)
+    assert effects.dm_swap_request(
+        "tok",
+        "U_REQ",
+        "U_TGT",
+        "2026-06-03",
+        "night",
+        "Alice",
+        note="ping <@U_ADMIN>",
+    )
+    assert "Note: ping &lt;@U_ADMIN&gt;" in captured["text"]
+    assert "<@U_ADMIN>" not in captured["text"]
+
 
 class TestBuildSwapResolvedBlocks:
     def test_renders_text_no_buttons(self):

tests/shared/test_portal_ops.py

@@ -75,6 +75,73 @@ 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_accepts_note_that_trims_to_500(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=f" {'a' * 500} ")
+    assert schedule.get_swap("2026-06-12", "night")["note"] == "a" * 500
+
+
+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"
+    assert schedule.get_swap("2026-06-12", "night") is None
 
 
 def test_late_pickup_creates_request(schedule, seed, quiet_slack):