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.
This commit is contained in:
Adam Moussa 2026-05-01 15:16:11 -04:00 • committed by GitHub
parent a85eb0afd9
commit 61074dc7cc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 23 additions and 10 deletions

View file

@ -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

View file

@ -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