From bf359100a180dd7297f04738bf8826420893902b Mon Sep 17 00:00:00 2001 From: "seahaven-openswe[bot]" <296972425+seahaven-openswe[bot]@users.noreply.github.com> Date: Fri, 26 Jun 2026 20:01:28 +0000 Subject: [PATCH] Stick schedule post to bottom of channel Replace the native pin with true bottom-of-channel stickiness: a debounced message.channels handler in the slack-bot Lambda deletes and re-posts the tracked schedule message on new activity so it stays last, while the Monday rollover and in-week edits keep editing the same stored ts. The pin only gave header reachability, not bottom placement. Refs: #135 --- CHANGELOG.md | 18 ++-- docs/sticky-schedule-post.md | 147 ++++++++++++++++----------- slack-app-manifest.yaml | 20 ++-- src/shared/shared/schedule.py | 26 +++-- src/slack-bot/CHANGELOG.md | 18 ++-- src/slack-bot/app.py | 104 +++++++++++++++++-- src/weekly-post/app.py | 20 +--- tests/slack_bot/test_channel_bump.py | 130 +++++++++++++++++++++++ tests/weekly_post/test_handler.py | 15 +-- 9 files changed, 368 insertions(+), 130 deletions(-) create mode 100644 tests/slack_bot/test_channel_bump.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 02e5d95..1359743 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,15 +12,15 @@ fine and still supported. ## v1.13.0 — June 27, 2026 -**The two-week schedule post now stays put instead of being reposted every -week.** Each Monday the bot used to delete last week's schedule message and post -a brand-new one — which pinged the channel again, changed the message's link, -and dropped any replies or reactions. Now it simply edits the existing post to -roll the two-week window forward, so the link stays stable, there's no weekly -re-notification, and any thread stays attached. The post is also pinned, so you -can always reach it from the channel header even after a busy day of chat. (The -pin needs a one-time app reinstall to switch on; until then everything else still -works.) +**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 diff --git a/docs/sticky-schedule-post.md b/docs/sticky-schedule-post.md index 7f06f71..88ae188 100644 --- a/docs/sticky-schedule-post.md +++ b/docs/sticky-schedule-post.md @@ -2,52 +2,54 @@ _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 (a) is implemented in the same PR._ +recommended option (b) is implemented in the same PR._ ## Background -`src/weekly-post/app.py` (the Monday 7am ET Lambda) currently, every week: +`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 gets a new `ts` (new permalink), re-notifies the -channel, and loses any thread/reactions. During the week it steadily sinks as -people chat. +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`. -The weekly rollover is the *only* place that creates a fresh message. ## Options considered -### (a) Reuse the same post across weeks — **recommended** +### (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. +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. + 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 pinned to the bottom +### (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:** always visible at the bottom of the channel. -- **Cons:** directly conflicts with (a)'s stable permalink (every repost = new - `ts`); requires a new event subscription (`message.channels`) and the - `channels:history` scope (a reinstall); needs dedupe/debounce plus Slack - rate-limit handling; constant repost churn; many more moving parts and a new - always-on event path to operate. The benefit (bottom-of-channel visibility) - is marginal for a low-traffic on-call channel. +- **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 @@ -57,58 +59,85 @@ posts, so it is always last. ## Decision -Adopt **(a) reuse-in-place**, complemented by a **native Slack pin** so the post -stays reachable from the channel header even as the channel fills with chatter. -Reject **(b)** — its only advantage over a pin is keeping the post at the literal -bottom, which is not worth a new event subscription, an extra scope, and -rate-limit/debounce machinery for this channel. The pin recovers most of the -discoverability that the weekly bump used to provide, at the cost of one extra -API call and one extra scope (`pins:write`), with no new event path to operate. +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: -## What changes (option a) +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. -In `src/weekly-post/app.py`, on the Monday rollover: +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. -- **Drop the unconditional `chat_delete`.** There is no previous message to - remove when we reuse the same one. -- **`chat_update` the stored post** to roll the two-week window forward, keeping - the same `ts` (stable permalink, no re-notification). -- **First-run / recovery fallback:** when there is no stored post, or the - `chat_update` fails (e.g. the message was deleted manually), fall back to - `chat_postMessage` and **pin** the new message. -- **Keep `save_schedule_post` / `get_schedule_post`.** Still needed — they hold - the `ts` that both the weekly rollover and `_refresh_schedule_post` edit. On - reuse the stored `ts` is unchanged; on a fresh post it is updated to the new - `ts` and the new `week_start`. +## What changes (option b) -### Interaction with `_refresh_schedule_post` +### Weekly rollover — `src/weekly-post/app.py` -No change needed. Both paths now converge on a single long-lived message keyed -by the stored `ts`: the weekly Lambda rolls the window forward each Monday, and -in-week shift changes keep editing the same message. They never fight because -neither creates a new `ts` while one already exists. +- **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. -### The "ever-growing edited message" / retention question +### Activity bump — `src/slack-bot/app.py` -A `chat_update` does not grow channel history — it rewrites one message in place -rather than appending. Slack keeps the message's original post time, so an -edited-forever post sorts by where it was first posted (hence the pin). There is -no retention or history-size concern from editing the same message indefinitely. +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: + - 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 `chat_delete`s the stored post and `chat_postMessage`s the current + `build_week_schedule` blocks, then `save_schedule_post`s the new `ts` (with + `last_bump_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 -- `pins:write` is added to `slack-app-manifest.yaml` for the native pin. This is - a new OAuth scope, so the app must be **reinstalled** once for the pin to take - effect. The pin call is best-effort: until the reinstall happens (or if the - bot lacks the scope) it logs and is skipped, and the reuse-in-place behavior - still works without it. -- No new event subscriptions. Option (b)'s `message.channels` event and - `channels:history` scope are **not** added. +- `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 `pins:write` takes effect, then - confirm the first rolled-forward post gets pinned. -- Optional: a one-line threaded note on the rolled-forward post if we later - decide we still want a light weekly nudge (kept out here on purpose — the whole - point of (a) is to stop the weekly re-notification). +- 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 4d6d6f6..0087dcc 100644 --- a/slack-app-manifest.yaml +++ b/slack-app-manifest.yaml @@ -5,10 +5,11 @@ # reconcile it against the live app config (App settings → App Manifest) so no # existing scope or setting is dropped. # -# This release adds the bot `pins:write` scope so the Monday rollover can natively -# pin the reused two-week schedule post. A new OAuth scope means a one-time -# reinstall is required for the pin to take effect (the pin is best-effort and is -# skipped until then). Replace with the SlackBotApiUrl stack output. +# 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 @@ -39,16 +40,19 @@ oauth_config: - chat:write - im:write - users:read - # pins:write lets the Monday rollover natively pin the reused schedule post - # so it stays reachable from the channel header. Adding it requires a - # one-time reinstall; the pin is best-effort until then. - - pins:write + # 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 02e5d95..1359743 100644 --- a/src/slack-bot/CHANGELOG.md +++ b/src/slack-bot/CHANGELOG.md @@ -12,15 +12,15 @@ fine and still supported. ## v1.13.0 — June 27, 2026 -**The two-week schedule post now stays put instead of being reposted every -week.** Each Monday the bot used to delete last week's schedule message and post -a brand-new one — which pinged the channel again, changed the message's link, -and dropped any replies or reactions. Now it simply edits the existing post to -roll the two-week window forward, so the link stays stable, there's no weekly -re-notification, and any thread stays attached. The post is also pinned, so you -can always reach it from the channel header even after a busy day of chat. (The -pin needs a one-time app reinstall to switch on; until then everything else still -works.) +**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 diff --git a/src/slack-bot/app.py b/src/slack-bot/app.py index db6c28c..14cc4be 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,88 @@ 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): + """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 when there is no stored post, + when the triggering message is the bot's own / an edit-delete subtype, when + debounced inside the window, or when the bot isn't in the channel. + """ + 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, …) — only genuine new user messages should bump. + if event.get("bot_id") or event.get("subtype"): + 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 + + 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 +2198,8 @@ def create_app( return publish_home(client, event["user"], _changelog_text()) + @app.event("message") + def handle_message_event(event, client): + handle_channel_message(event, client, schedule, schedule_channel) + return app diff --git a/src/weekly-post/app.py b/src/weekly-post/app.py index d4a25fb..74b67c9 100644 --- a/src/weekly-post/app.py +++ b/src/weekly-post/app.py @@ -247,16 +247,6 @@ def _send_pay_email(week_label: str, pay_record: dict) -> None: logger.info("Sent pay email to %s", recipients) -def _pin_schedule_post(slack, channel_id, message_ts): - """Best-effort native pin so a freshly posted schedule stays reachable from - the channel header even as the channel fills with chatter.""" - try: - slack.pins_add(channel=channel_id, timestamp=message_ts) - logger.info("Pinned schedule post %s", message_ts) - except Exception: - logger.info("Could not pin schedule post %s", message_ts, exc_info=True) - - def handler(event, context): now = datetime.now(EASTERN) @@ -328,11 +318,10 @@ def handler(event, context): f"to {end_date.strftime('%b %-d')}" ) - # Reuse the same post across weeks: roll the two-week window forward by - # editing the existing message in place (stable permalink, no weekly - # re-notification, keeps any thread/reactions) instead of delete + repost. - # Fall back to a fresh, pinned post when there is no stored message or the - # edit fails (e.g. the message was deleted manually). + # 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"): @@ -359,7 +348,6 @@ def handler(event, context): channel=channel_id, blocks=blocks, text=fallback_text ) message_ts = result["ts"] - _pin_schedule_post(slack, channel_id, message_ts) logger.info( "Posted new weekly schedule to channel %s (ts=%s)", channel_id, message_ts ) diff --git a/tests/slack_bot/test_channel_bump.py b/tests/slack_bot/test_channel_bump.py new file mode 100644 index 0000000..3e282ea --- /dev/null +++ b/tests/slack_bot/test_channel_bump.py @@ -0,0 +1,130 @@ +"""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_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 418b762..b98c33a 100644 --- a/tests/weekly_post/test_handler.py +++ b/tests/weekly_post/test_handler.py @@ -95,27 +95,16 @@ def test_rolls_existing_post_forward(weeklypost_app, schedule, seed, slack, env) assert schedule.get_schedule_post("C_TEST")["message_ts"] == "111.111" -@freeze_time(MON_0800) -def test_pins_post_on_first_run(weeklypost_app, schedule, seed, slack, env): - # No existing post → post a fresh message and natively pin it. - result = weeklypost_app.handler({"force": True}, None) - - slack.chat_postMessage.assert_called() - slack.pins_add.assert_called_once() - assert slack.pins_add.call_args.kwargs["timestamp"] == "999.000" - assert result["message_ts"] == "999.000" - - @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, pinned post and re-saves the new ts. + # 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.pins_add.assert_called_once() + slack.chat_postMessage.assert_called() assert result["message_ts"] == "999.000" assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000"