diff --git a/CHANGELOG.md b/CHANGELOG.md index df3339c..1359743 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ fine and still supported. --- +## v1.13.0 — June 27, 2026 + +**The two-week schedule post now sticks to the bottom of the channel.** It used +to drift up out of sight as people chatted through the day. Now, whenever there's +new activity in the channel, the bot quietly moves the schedule back down to the +bottom so it's always the last thing you see. To avoid spamming during a busy +back-and-forth, it only does this at most once every few minutes. The trade-off +for keeping it at the bottom is that the post's link changes each time it moves, +and moving it re-pings the channel. (This needs a one-time app reinstall to +switch on the new permission; until then the schedule still updates in place as +before.) + ## v1.12.0 — June 26, 2026 **Clearer shift drops and an easier-to-read schedule.** Two small quality-of-life diff --git a/docs/sticky-schedule-post.md b/docs/sticky-schedule-post.md new file mode 100644 index 0000000..5039b1f --- /dev/null +++ b/docs/sticky-schedule-post.md @@ -0,0 +1,151 @@ +# Evaluation: sticky / reused weekly schedule post + +_Spike for issue #135 — decide how the Monday two-week schedule post should +behave across weeks. This document records the decision and the rationale; the +recommended option (b) is implemented in the same PR._ + +## Background + +`src/weekly-post/app.py` (the Monday 7am ET Lambda) originally, every week: + +1. `chat_delete` the previous week's schedule post (stored `ts` from + `get_schedule_post`), +2. `chat_postMessage` a brand-new two-week schedule message, and +3. `save_schedule_post(channel_id, new_ts, …)`. + +So every Monday the post got a new `ts` (new permalink), re-notified the +channel, and lost any thread/reactions. During the week it steadily sank as +people chatted. + +The infrastructure to edit in place **already exists**: in-week shift changes +(`pick` / `drop` / `swap` / `admin`) call `_refresh_schedule_post()` +(`src/slack-bot/app.py`), which does a `chat_update` against the stored `ts`. + +## Options considered + +### (a) Reuse the same post across weeks — pin for reachability + +On Monday, `chat_update` the existing message to roll the two-week window +forward instead of delete + repost, and add a native Slack pin so the post is +reachable from the channel header. + +- **Pros:** stable permalink; no weekly re-notification; keeps any + thread/reactions; trivial change (reuses the `_refresh_schedule_post` + `chat_update` pattern already in the codebase). +- **Cons:** the message stays where it was first posted — it does **not** rise + to the bottom as the channel gets new activity. A native pin only surfaces the + post in the channel's pinned-items panel (the header); it does **not** hold the + message at the bottom of the timeline, which is the actual requirement from + #135. + +### (b) True stickybot behavior — keep it at the bottom — **recommended** + +Slack has no native "sticky" message. Stickybot-style bots subscribe to channel +message events and **delete + repost** the bot message whenever someone else +posts, so it is always last. + +- **Pros:** the schedule post is always at the literal bottom of the channel + timeline — exactly the behavior #135 asks for. +- **Cons:** every bump is a new `ts` (the permalink changes); the post + re-surfaces / re-notifies on activity; requires a new `message.channels` event + subscription, the `channels:history` scope (a one-time reinstall), and + debounce machinery to avoid thrashing on bursts. + +### (c) Status quo — delete + repost weekly + +- **Pros:** the weekly repost bumps the post to the bottom once a week. +- **Cons:** new permalink every week; a weekly re-notification; loses + thread/reactions; the post still sinks for the rest of the week. + +## Decision + +Adopt **(b) true bottom-of-channel stickiness**. Adam wants literal +bottom-of-channel placement of the schedule post, and explicitly accepts the +tradeoffs that come with it: + +1. **The permalink changes on each bump** — every delete + repost produces a new + `ts`, so any saved link to the post goes stale. +2. **The post re-surfaces / re-notifies on activity** — re-posting pings the + channel (subject to the debounce window below), rather than sitting quietly. +3. **New machinery** — a `message.channels` event subscription, the + `channels:history` scope (one-time reinstall), and a debounce to avoid + thrashing on bursts. + +The **native pin (option a's complement) was rejected**: it only gives header +reachability — the post shows up in the pinned-items panel — but it does **not** +hold the message at the bottom of the timeline, which is the requirement. A pin +is therefore not a substitute for bottom-stickiness. + +## What changes (option b) + +### Weekly rollover — `src/weekly-post/app.py` + +- **Drop the unconditional `chat_delete` + always-repost.** On Monday, + `chat_update` the stored post to roll the two-week window forward, keeping the + same `ts` (the activity bump is what moves it to the bottom; the rollover just + refreshes content). +- **First-run / recovery fallback:** when there is no stored post or the + `chat_update` fails (e.g. the message was deleted manually), `chat_postMessage` + a fresh message and store its `ts`. +- The `pins:write` scope and the `_pin_schedule_post` helper are removed — the + pin is fully replaced by bottom-stickiness. + +### Activity bump — `src/slack-bot/app.py` + +The bump lives in the **slack-bot** Lambda, which already owns the Slack events +request URL. On a `message.channels` event in the schedule channel +(`handle_channel_message`): + +- **Debounced** so a burst of chatter triggers at most one bump: + `SCHEDULE_BUMP_DEBOUNCE_SECONDS = 180` (3 minutes). The last bump time is + stored as `last_bump_ts` on the schedule-post record, so the debounce check + reads no channel history. +- **Idempotent / safe** — the bump is skipped when: + - the delivery is a Slack **retry** (`X-Slack-Retry-Num` set) — under + `process_before_response=True` the delete+repost runs before the 200 ack, so + a slow run can be retried; we never bump on a retry (the next real message + bumps anyway), + - the triggering message is a **thread reply** (`thread_ts`) — a threaded reply + doesn't push the schedule down the main timeline, + - there is no stored post, + - the triggering message is the bot's own (`bot_id`) or a non-user message + subtype (edits, deletes, joins, …), + - the debounce window has not elapsed, or + - the bot isn't in the channel (`not_in_channel`). +- Otherwise it **stamps `last_bump_ts` optimistically before** the delete/repost + (so a retry fired mid-bump — or after the run dies before the final save — is + debounced and can't orphan a duplicate), `chat_delete`s the stored post, + `chat_postMessage`s the current `build_week_schedule` blocks, then + `save_schedule_post`s the new `ts`. If the old message is already gone, it + still reposts so the channel always ends with the schedule. + +### Interaction with `_refresh_schedule_post` and the rollover + +All three paths converge on the **single stored `ts`** and never fight: + +- **Monday rollover** — `chat_update` the stored `ts` (content refresh; same + message), clearing `last_bump_ts` so the next activity is free to bump. +- **In-week shift edits** — `_refresh_schedule_post` `chat_update`s the same + stored `ts`; it does not change the `ts` or touch `last_bump_ts`. +- **Activity bump** — deletes the stored `ts`, reposts the same content at the + bottom, and saves the new `ts`. Subsequent rollovers and edits then operate on + that new `ts`. + +Because only the bump ever mints a new `ts` (and it always re-saves it +immediately), the other two paths always read the current `ts` from the record. + +## Scope / manifest impact + +- `channels:history` (bot scope) and `message.channels` (bot event) are added to + `slack-app-manifest.yaml` so the slack-bot Lambda receives channel messages and + can run the bump. This is a new scope + event, so the app must be + **reinstalled** once for them to take effect. +- `pins:write` is **removed** — the pin is no longer used. + +## Follow-ups (not in this PR) + +- After deploy, reinstall the Slack app so `channels:history` / + `message.channels` take effect, then confirm a new channel message bumps the + schedule post to the bottom (and that bursts are debounced). +- If the re-notification on each bump proves noisy, revisit the debounce window + or consider posting without a broadcast. diff --git a/slack-app-manifest.yaml b/slack-app-manifest.yaml index afd17d8..0087dcc 100644 --- a/slack-app-manifest.yaml +++ b/slack-app-manifest.yaml @@ -5,11 +5,15 @@ # reconcile it against the live app config (App settings → App Manifest) so no # existing scope or setting is dropped. # -# The ONLY delta this release introduces is the App Home tab: +# This release adds the `channels:history` bot scope and the `message.channels` +# bot event so the slack-bot Lambda can keep the two-week schedule post stuck to +# the bottom of the channel: on new activity it deletes and re-posts the tracked +# message (debounced). The new scope + event require a one-time reinstall to take +# effect. Replace with the SlackBotApiUrl stack output. +# +# A prior release added the App Home tab (no new scope): # - settings.event_subscriptions.bot_events: + app_home_opened # - features.app_home.home_tab_enabled: true -# Neither needs a new OAuth scope, so no reinstall is required (see README → -# "Releases & versioning"). Replace with the SlackBotApiUrl stack output. display_information: name: After-Hours Shift Manager description: Manage after-hours on-call phone duty from Slack. @@ -36,12 +40,19 @@ oauth_config: - chat:write - im:write - users:read + # channels:history lets the bot receive message.channels events for the + # schedule channel, which drive the activity bump that keeps the schedule + # post at the bottom of the channel. Requires a one-time reinstall. + - channels:history settings: event_subscriptions: request_url: bot_events: - app_home_opened + # message.channels drives the activity bump that keeps the schedule post + # at the bottom of the channel (debounced delete + repost). + - message.channels interactivity: is_enabled: true request_url: diff --git a/src/shared/shared/schedule.py b/src/shared/shared/schedule.py index 2d792a5..e260759 100644 --- a/src/shared/shared/schedule.py +++ b/src/shared/shared/schedule.py @@ -561,16 +561,24 @@ class ShiftSchedule: return resp.get("Item") def save_schedule_post( - self, channel_id: str, message_ts: str, week_start: str + self, + channel_id: str, + message_ts: str, + week_start: str, + last_bump_ts: float | None = None, ) -> None: - self.table.put_item( - Item={ - "PK": "SCHEDULE_POST", - "SK": channel_id, - "message_ts": message_ts, - "week_start": week_start, - } - ) + """Store the tracked schedule post. ``last_bump_ts`` (epoch seconds) is + the debounce marker for the activity bump; omit it on the weekly + rollover so the next channel message is free to bump to the bottom.""" + item = { + "PK": "SCHEDULE_POST", + "SK": channel_id, + "message_ts": message_ts, + "week_start": week_start, + } + if last_bump_ts is not None: + item["last_bump_ts"] = Decimal(str(last_bump_ts)) + self.table.put_item(Item=item) # ── Pay records ───────────────────────────────────────────────────── diff --git a/src/slack-bot/CHANGELOG.md b/src/slack-bot/CHANGELOG.md index df3339c..1359743 100644 --- a/src/slack-bot/CHANGELOG.md +++ b/src/slack-bot/CHANGELOG.md @@ -10,6 +10,18 @@ fine and still supported. --- +## v1.13.0 — June 27, 2026 + +**The two-week schedule post now sticks to the bottom of the channel.** It used +to drift up out of sight as people chatted through the day. Now, whenever there's +new activity in the channel, the bot quietly moves the schedule back down to the +bottom so it's always the last thing you see. To avoid spamming during a busy +back-and-forth, it only does this at most once every few minutes. The trade-off +for keeping it at the bottom is that the post's link changes each time it moves, +and moving it re-pings the channel. (This needs a one-time app reinstall to +switch on the new permission; until then the schedule still updates in place as +before.) + ## v1.12.0 — June 26, 2026 **Clearer shift drops and an easier-to-read schedule.** Two small quality-of-life diff --git a/src/slack-bot/app.py b/src/slack-bot/app.py index db6c28c..8d99b3c 100644 --- a/src/slack-bot/app.py +++ b/src/slack-bot/app.py @@ -11,6 +11,7 @@ import json import logging import os import re +import time from datetime import datetime, timedelta from zoneinfo import ZoneInfo @@ -46,6 +47,12 @@ from shared.three_cx_client import ThreeCXClient logger = logging.getLogger(__name__) EASTERN = ZoneInfo("America/New_York") +# Activity-bump debounce: don't re-post the schedule to the bottom of the +# channel more than once per this many seconds, so a burst of chatter triggers +# at most one delete+repost. The window is stored as ``last_bump_ts`` on the +# schedule-post record (avoids reading channel history). +SCHEDULE_BUMP_DEBOUNCE_SECONDS = 180 + DAY_NAMES = [ "monday", "tuesday", @@ -392,8 +399,19 @@ def _droppable_shifts(schedule, date_str, day_name, employee_ext) -> list[str]: return held +def _schedule_fallback_text() -> str: + """Notification fallback text for the two-week schedule post.""" + now = datetime.now(EASTERN) + this_monday = now - timedelta(days=now.weekday()) + end_date = this_monday + timedelta(days=13) + return ( + f"After-Hours Schedule — {this_monday.strftime('%b %-d')} " + f"to {end_date.strftime('%b %-d')}" + ) + + def _refresh_schedule_post(schedule, schedule_channel, client): - """Update the pinned schedule message in-place after a shift change.""" + """Update the tracked schedule message in-place after a shift change.""" channel = schedule_channel if not channel: return @@ -401,20 +419,106 @@ def _refresh_schedule_post(schedule, schedule_channel, client): if not post or not post.get("message_ts"): return try: - blocks = build_week_schedule(schedule) - now = datetime.now(EASTERN) - this_monday = now - timedelta(days=now.weekday()) - end_date = this_monday + timedelta(days=13) client.chat_update( channel=channel, ts=post["message_ts"], - blocks=blocks, - text=f"After-Hours Schedule — {this_monday.strftime('%b %-d')} to {end_date.strftime('%b %-d')}", + blocks=build_week_schedule(schedule), + text=_schedule_fallback_text(), ) except Exception: logger.warning("Could not update schedule post", exc_info=True) +def _slack_error_code(exc) -> str | None: + """Best-effort Slack API error code (e.g. ``not_in_channel``) from an exc.""" + response = getattr(exc, "response", None) + if response is None: + return None + try: + return response.get("error") + except Exception: + return None + + +def handle_channel_message(event, client, schedule, schedule_channel, retry_num=None): + """Bump the schedule post to the bottom of the channel on new activity. + + Debounced delete+repost of the tracked post so it stays at the bottom of + the channel timeline. Idempotent/safe: skips retried Slack deliveries + (``retry_num``), thread replies, the bot's own / edit-delete messages, when + there is no stored post, when debounced inside the window, or when the bot + isn't in the channel. + """ + # A retry means our first delivery already (likely) did the work but the + # 200 ack didn't reach Slack in time. Never bump again on a retry — the next + # real channel message bumps anyway — so a slow run can't double-post. + if retry_num: + logger.info("Ignoring retried message event (retry %s)", retry_num) + return + if not schedule_channel or event.get("channel") != schedule_channel: + return + # Ignore the bot's own posts and non-user message events (edits, deletes, + # joins, …), plus thread replies — a threaded reply doesn't push the + # schedule down the main timeline, so it isn't worth a delete+repost. + if event.get("bot_id") or event.get("subtype") or event.get("thread_ts"): + return + + post = schedule.get_schedule_post(schedule_channel) + if not post or not post.get("message_ts"): + return + + now = time.time() + last_bump = post.get("last_bump_ts") + if ( + last_bump is not None + and now - float(last_bump) < SCHEDULE_BUMP_DEBOUNCE_SECONDS + ): + return + + # Optimistically stamp the debounce window *before* the delete/repost, so a + # retry fired while this run is still in flight (or after it dies mid-bump) + # is suppressed and can't orphan a duplicate schedule message. + schedule.save_schedule_post( + schedule_channel, + post["message_ts"], + post.get("week_start", ""), + last_bump_ts=now, + ) + + blocks = build_week_schedule(schedule) + text = _schedule_fallback_text() + try: + client.chat_delete(channel=schedule_channel, ts=post["message_ts"]) + except Exception as exc: + if _slack_error_code(exc) == "not_in_channel": + logger.info("Bot not in schedule channel; skipping bump") + return + # The old message may already be gone — keep going and repost a fresh + # one so the channel still ends with the schedule. + logger.info("Could not delete old schedule post for bump", exc_info=True) + + try: + result = client.chat_postMessage( + channel=schedule_channel, blocks=blocks, text=text + ) + except Exception as exc: + if _slack_error_code(exc) == "not_in_channel": + logger.info("Bot not in schedule channel; skipping bump") + return + logger.warning("Could not repost schedule for bump", exc_info=True) + return + + schedule.save_schedule_post( + schedule_channel, + result["ts"], + post.get("week_start", ""), + last_bump_ts=now, + ) + logger.info( + "Bumped schedule post to bottom of %s (ts=%s)", schedule_channel, result["ts"] + ) + + # ── Command dispatch ──────────────────────────────────────────────────── @@ -2112,4 +2216,11 @@ def create_app( return publish_home(client, event["user"], _changelog_text()) + @app.event("message") + def handle_message_event(event, client, request): + retry_num = request.headers.get("x-slack-retry-num") if request else None + handle_channel_message( + event, client, schedule, schedule_channel, retry_num=retry_num + ) + return app diff --git a/src/weekly-post/app.py b/src/weekly-post/app.py index 524a4aa..74b67c9 100644 --- a/src/weekly-post/app.py +++ b/src/weekly-post/app.py @@ -307,35 +307,55 @@ def handler(event, context): week_key, ) - # --- Delete previous week's schedule post --- - old_post = schedule.get_schedule_post(channel_id) - if old_post and old_post.get("message_ts"): - try: - slack.chat_delete(channel=channel_id, ts=old_post["message_ts"]) - logger.info("Deleted previous schedule post %s", old_post["message_ts"]) - except Exception: - logger.warning("Could not delete old schedule post", exc_info=True) - # --- Two-week schedule (always starts on Monday of this week) --- this_monday = now - timedelta(days=now.weekday()) + week_start = this_monday.strftime("%Y-%m-%d") blocks = build_week_schedule(schedule, start_date=this_monday) end_date = this_monday + timedelta(days=13) - result = slack.chat_postMessage( - channel=channel_id, - blocks=blocks, - text=f"After-Hours Schedule — {this_monday.strftime('%b %-d')} to {end_date.strftime('%b %-d')}", + fallback_text = ( + f"After-Hours Schedule — {this_monday.strftime('%b %-d')} " + f"to {end_date.strftime('%b %-d')}" ) - schedule.save_schedule_post( - channel_id, result["ts"], this_monday.strftime("%Y-%m-%d") - ) - logger.info( - "Posted weekly schedule to channel %s (ts=%s)", channel_id, result["ts"] - ) + # Roll the two-week window forward by editing the stored message in place + # (same ts) so the Monday rollover, in-week shift edits, and the activity + # bump all converge on a single stored ts. Fall back to a fresh post when + # there is no stored message or the edit fails (e.g. it was deleted). + old_post = schedule.get_schedule_post(channel_id) + message_ts = None + if old_post and old_post.get("message_ts"): + try: + slack.chat_update( + channel=channel_id, + ts=old_post["message_ts"], + blocks=blocks, + text=fallback_text, + ) + message_ts = old_post["message_ts"] + logger.info( + "Rolled schedule post %s forward to week of %s", + message_ts, + week_start, + ) + except Exception: + logger.warning( + "Could not update existing schedule post; reposting", exc_info=True + ) + + if message_ts is None: + result = slack.chat_postMessage( + channel=channel_id, blocks=blocks, text=fallback_text + ) + message_ts = result["ts"] + logger.info( + "Posted new weekly schedule to channel %s (ts=%s)", channel_id, message_ts + ) + + schedule.save_schedule_post(channel_id, message_ts, week_start) return { "posted": True, "channel": channel_id, - "message_ts": result["ts"], + "message_ts": message_ts, "pay_calculated": bool(pay_record["breakdown"]), } diff --git a/tests/slack_bot/test_channel_bump.py b/tests/slack_bot/test_channel_bump.py new file mode 100644 index 0000000..fb44eb7 --- /dev/null +++ b/tests/slack_bot/test_channel_bump.py @@ -0,0 +1,180 @@ +"""Tests for the activity bump that keeps the schedule post at the bottom of +the channel (slack-bot handle_channel_message).""" + +import time + +from freezegun import freeze_time + +CHANNEL = "C_TEST" +MON = "2026-06-08 12:00:00" + + +class _SlackError(Exception): + """Stand-in for slack_sdk.errors.SlackApiError with a `.response` dict.""" + + def __init__(self, code): + super().__init__(code) + self.response = {"error": code} + + +def _msg(channel=CHANNEL, user="U_ALICE", ts="100.500", **extra): + return {"channel": channel, "user": user, "ts": ts, **extra} + + +@freeze_time(MON) +def test_bumps_on_new_message(slackbot_app, schedule, seed, client): + seed.schedule_post(CHANNEL, "111.111") + client.chat_postMessage.return_value = {"ts": "222.222"} + + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + client.chat_delete.assert_called_once() + assert client.chat_delete.call_args.kwargs["ts"] == "111.111" + client.chat_postMessage.assert_called_once() + # The stored ts now points at the freshly reposted message + debounce stamp. + post = schedule.get_schedule_post(CHANNEL) + assert post["message_ts"] == "222.222" + assert post.get("last_bump_ts") is not None + + +@freeze_time(MON) +def test_debounce_skips_within_window(slackbot_app, schedule, client): + schedule.save_schedule_post( + CHANNEL, "111.111", "2026-06-08", last_bump_ts=time.time() + ) + + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + # Stored post is untouched. + assert schedule.get_schedule_post(CHANNEL)["message_ts"] == "111.111" + + +@freeze_time(MON) +def test_bumps_after_debounce_window(slackbot_app, schedule, client): + old = time.time() - (slackbot_app.SCHEDULE_BUMP_DEBOUNCE_SECONDS + 60) + schedule.save_schedule_post(CHANNEL, "111.111", "2026-06-08", last_bump_ts=old) + client.chat_postMessage.return_value = {"ts": "222.222"} + + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + client.chat_postMessage.assert_called_once() + assert schedule.get_schedule_post(CHANNEL)["message_ts"] == "222.222" + + +@freeze_time(MON) +def test_skips_when_no_stored_post(slackbot_app, schedule, client): + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + + +@freeze_time(MON) +def test_skips_bots_own_message(slackbot_app, schedule, seed, client): + seed.schedule_post(CHANNEL, "111.111") + + slackbot_app.handle_channel_message(_msg(bot_id="B123"), client, schedule, CHANNEL) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + + +@freeze_time(MON) +def test_skips_message_edit_and_delete_subtypes(slackbot_app, schedule, seed, client): + seed.schedule_post(CHANNEL, "111.111") + + slackbot_app.handle_channel_message( + _msg(subtype="message_changed"), client, schedule, CHANNEL + ) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + + +@freeze_time(MON) +def test_skips_other_channel(slackbot_app, schedule, seed, client): + seed.schedule_post(CHANNEL, "111.111") + + slackbot_app.handle_channel_message( + _msg(channel="C_OTHER"), client, schedule, CHANNEL + ) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + + +@freeze_time(MON) +def test_retried_event_is_a_noop(slackbot_app, schedule, seed, client): + # A Slack retry (X-Slack-Retry-Num set) must never trigger a second bump. + seed.schedule_post(CHANNEL, "111.111") + + slackbot_app.handle_channel_message( + _msg(), client, schedule, CHANNEL, retry_num="1" + ) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + # The record is left untouched (no optimistic debounce stamp either). + post = schedule.get_schedule_post(CHANNEL) + assert post["message_ts"] == "111.111" + assert post.get("last_bump_ts") is None + + +@freeze_time(MON) +def test_thread_reply_is_a_noop(slackbot_app, schedule, seed, client): + # A threaded reply doesn't push the schedule down the main timeline. + seed.schedule_post(CHANNEL, "111.111") + + slackbot_app.handle_channel_message( + _msg(thread_ts="050.000"), client, schedule, CHANNEL + ) + + client.chat_delete.assert_not_called() + client.chat_postMessage.assert_not_called() + assert schedule.get_schedule_post(CHANNEL)["message_ts"] == "111.111" + + +@freeze_time(MON) +def test_stamps_debounce_before_repost(slackbot_app, schedule, seed, client): + # The debounce window is stamped optimistically before the delete/repost so + # a retry mid-bump is suppressed even if this run dies before the final save. + seed.schedule_post(CHANNEL, "111.111") + captured = {} + + def _delete(**kwargs): + # Mid-bump snapshot: the record already carries last_bump_ts. + captured["post"] = schedule.get_schedule_post(CHANNEL) + + client.chat_delete.side_effect = _delete + client.chat_postMessage.return_value = {"ts": "222.222"} + + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + assert captured["post"].get("last_bump_ts") is not None + + +@freeze_time(MON) +def test_skips_when_bot_not_in_channel(slackbot_app, schedule, seed, client): + seed.schedule_post(CHANNEL, "111.111") + client.chat_delete.side_effect = _SlackError("not_in_channel") + + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + # No repost, and the stored post is left intact. + client.chat_postMessage.assert_not_called() + assert schedule.get_schedule_post(CHANNEL)["message_ts"] == "111.111" + + +@freeze_time(MON) +def test_reposts_when_old_message_already_gone(slackbot_app, schedule, seed, client): + # A stale ts (deleted manually) → delete fails non-fatally; still reposts. + seed.schedule_post(CHANNEL, "111.111") + client.chat_delete.side_effect = _SlackError("message_not_found") + client.chat_postMessage.return_value = {"ts": "222.222"} + + slackbot_app.handle_channel_message(_msg(), client, schedule, CHANNEL) + + client.chat_postMessage.assert_called_once() + assert schedule.get_schedule_post(CHANNEL)["message_ts"] == "222.222" diff --git a/tests/weekly_post/test_handler.py b/tests/weekly_post/test_handler.py index 0100185..b98c33a 100644 --- a/tests/weekly_post/test_handler.py +++ b/tests/weekly_post/test_handler.py @@ -82,11 +82,31 @@ def test_pay_email_failure_does_not_block_schedule_post( @freeze_time(MON_0800) -def test_deletes_previous_schedule_post(weeklypost_app, schedule, seed, slack, env): +def test_rolls_existing_post_forward(weeklypost_app, schedule, seed, slack, env): + # An existing post is edited in place (stable permalink), never deleted. seed.schedule_post("C_TEST", "111.111") - weeklypost_app.handler({"force": True}, None) - slack.chat_delete.assert_called_once() - assert slack.chat_delete.call_args.kwargs["ts"] == "111.111" + result = weeklypost_app.handler({"force": True}, None) + + slack.chat_delete.assert_not_called() + slack.chat_update.assert_called_once() + assert slack.chat_update.call_args.kwargs["ts"] == "111.111" + # The stored ts is unchanged, so the permalink stays stable. + assert result["message_ts"] == "111.111" + assert schedule.get_schedule_post("C_TEST")["message_ts"] == "111.111" + + +@freeze_time(MON_0800) +def test_reposts_when_update_fails(weeklypost_app, schedule, seed, slack, env): + # A stored post that can no longer be edited (e.g. deleted manually) falls + # back to a fresh post and re-saves the new ts. + seed.schedule_post("C_TEST", "111.111") + slack.chat_update.side_effect = Exception("message_not_found") + + result = weeklypost_app.handler({"force": True}, None) + + slack.chat_postMessage.assert_called() + assert result["message_ts"] == "999.000" + assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000" @freeze_time(MON_0800)