From 61074dc7ccb78c93a14dec0d7ca767d3ecc70111 Mon Sep 17 00:00:00 2001
From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com>
Date: Fri, 1 May 2026 15:16:11 -0400
Subject: [PATCH] Fix shift pickup showing error despite succeeding (#31) (#32)

Two root causes:
- claim_open_shift ConditionExpression failed when no OVERRIDE record
  existed (shift available via weekly fallback). Added attribute_not_exists
  check so claims succeed for both missing and OPEN overrides.
- Slack ack timeout: chat_postMessage took too long before respond() was
  called, causing Slack to show an error. Moved respond() first and made
  channel notifications best-effort with try/except.
---
 src/app.py      | 26 +++++++++++++++++++-------
 src/schedule.py |  7 ++++---
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/app.py b/src/app.py
index e5c7e45..2114608 100644
--- a/src/app.py
+++ b/src/app.py
@@ -121,7 +121,7 @@ def create_app(bot_token: str, signing_secret: str, schedule_channel: str | None
     # ── Interactive button: pick up open shift ──────────────────────────
 
     @app.action(re.compile(r"^pickup_"))
-    def handle_pickup_button(ack, body, client):
+    def handle_pickup_button(ack, body, client, respond):
         ack()
         action_id = body["actions"][0]["action_id"]
         remainder = action_id.replace("pickup_", "")
@@ -159,7 +159,7 @@ def create_app(bot_token: str, signing_secret: str, schedule_channel: str | None
         blocks = build_shift_change_message(
             user_id, date_str, "picked_up", employee["extension"], employee["name"]
         )
-        client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift picked up for {date_str}")
+        respond(response_type="in_channel", replace_original=False, blocks=blocks, text=f"Shift picked up for {date_str}")
 
     # ── Subcommand handlers ─────────────────────────────────────────────
 
@@ -288,11 +288,15 @@ def create_app(bot_token: str, signing_secret: str, schedule_channel: str | None
         if is_today(date_str):
             invoke_3cx_scheduler(employee["extension"])
 
+        respond(text=f"You picked up the shift for *{date.strftime('%A, %b %-d')}*.")
+
         blocks = build_shift_change_message(
             user_id, date_str, "picked_up", employee["extension"], employee["name"]
         )
-        client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift picked up for {date_str}")
-        respond(text=f"You picked up the shift for *{date.strftime('%A, %b %-d')}*.")
+        try:
+            client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift picked up for {date_str}")
+        except Exception:
+            logger.exception("Failed to post pickup notification to channel")
 
     def _handle_drop(respond, schedule, user_id, text, channel_id, client):
         parts = text.split(maxsplit=1)
@@ -323,10 +327,14 @@ def create_app(bot_token: str, signing_secret: str, schedule_channel: str | None
         if is_today(date_str):
             invoke_3cx_scheduler(FALLBACK_EXTENSION)
 
-        blocks = build_shift_change_message(user_id, date_str, "dropped", ext, name)
-        client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift dropped for {date_str}")
         respond(text=f"You dropped the shift for *{date.strftime('%A, %b %-d')}*. It's now open for pickup.")
 
+        blocks = build_shift_change_message(user_id, date_str, "dropped", ext, name)
+        try:
+            client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift dropped for {date_str}")
+        except Exception:
+            logger.exception("Failed to post drop notification to channel")
+
     def _handle_swap(respond, schedule, user_id, text, channel_id, client):
         # Expected format: swap <date> @user  OR  swap <date> <extension>
         parts = text.split(maxsplit=2)
@@ -379,7 +387,11 @@ def create_app(bot_token: str, signing_secret: str, schedule_channel: str | None
             target["extension"],
             target["name"],
         )
-        client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift swapped for {date_str}")
         respond(text=f"Swapped *{date.strftime('%A, %b %-d')}* to {target['name']} (Ext {target['extension']}).")
 
+        try:
+            client.chat_postMessage(channel=channel_id, blocks=blocks, text=f"Shift swapped for {date_str}")
+        except Exception:
+            logger.exception("Failed to post swap notification to channel")
+
     return app
diff --git a/src/schedule.py b/src/schedule.py
index c93dfa7..dd88326 100644
--- a/src/schedule.py
+++ b/src/schedule.py
@@ -74,9 +74,10 @@ class ShiftSchedule:
         )
 
     def claim_open_shift(self, date_str: str, extension: str, name: str, shift_type: str = "night") -> bool:
-        """Atomically claim a shift only if it is currently OPEN.
+        """Atomically claim a shift only if it is currently open.
 
-        Returns True if the claim succeeded, False if someone else already took it.
+        Succeeds when no override exists (weekly fallback) or the override
+        is explicitly OPEN. Fails if someone else already claimed it.
         """
         sk = f"{date_str}-DAY" if shift_type == "day" else date_str
         try:
@@ -87,7 +88,7 @@ class ShiftSchedule:
                     "extension": extension,
                     "name": name,
                 },
-                ConditionExpression="extension = :open",
+                ConditionExpression="attribute_not_exists(PK) OR extension = :open",
                 ExpressionAttributeValues={":open": "OPEN"},
             )
             return True