mirror of
https://github.com/Sea-Haven-Industries/afterhours-shift-manager.git
synced 2026-10-01 09:33:11 +00:00
Reuse weekly schedule post instead of reposting
The Monday rollover deleted last week's schedule message and posted a fresh one every week, which re-notified the channel, changed the permalink, and dropped any thread/reactions. Roll the two-week window forward by editing the stored message in place (the same chat_update path in-week shift changes already use), falling back to a fresh, pinned post on first run or when the edit fails. A native pin keeps it reachable from the channel header now that it no longer rises on the weekly repost. Records the evaluation behind this in docs/sticky-schedule-post.md. Refs: #135
This commit is contained in:
parent
b8ab4d6b77
commit
612572bde7
6 changed files with 235 additions and 27 deletions
12
CHANGELOG.md
12
CHANGELOG.md
|
|
@ -10,6 +10,18 @@ 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.)
|
||||
|
||||
## v1.12.0 — June 26, 2026
|
||||
|
||||
**Clearer shift drops and an easier-to-read schedule.** Two small quality-of-life
|
||||
|
|
|
|||
114
docs/sticky-schedule-post.md
Normal file
114
docs/sticky-schedule-post.md
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
# 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 (a) is implemented in the same PR._
|
||||
|
||||
## Background
|
||||
|
||||
`src/weekly-post/app.py` (the Monday 7am ET Lambda) currently, 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.
|
||||
|
||||
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**
|
||||
|
||||
On Monday, `chat_update` the existing message to roll the two-week window
|
||||
forward instead of delete + repost.
|
||||
|
||||
- **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.
|
||||
|
||||
### (b) True stickybot behavior — keep it pinned to the bottom
|
||||
|
||||
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.
|
||||
|
||||
### (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 **(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.
|
||||
|
||||
## What changes (option a)
|
||||
|
||||
In `src/weekly-post/app.py`, on the Monday rollover:
|
||||
|
||||
- **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`.
|
||||
|
||||
### Interaction with `_refresh_schedule_post`
|
||||
|
||||
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.
|
||||
|
||||
### The "ever-growing edited message" / retention question
|
||||
|
||||
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.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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).
|
||||
|
|
@ -5,11 +5,14 @@
|
|||
# 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 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 <API_URL> 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 <API_URL> with the SlackBotApiUrl stack output.
|
||||
display_information:
|
||||
name: After-Hours Shift Manager
|
||||
description: Manage after-hours on-call phone duty from Slack.
|
||||
|
|
@ -36,6 +39,10 @@ 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
|
||||
|
||||
settings:
|
||||
event_subscriptions:
|
||||
|
|
|
|||
|
|
@ -10,6 +10,18 @@ 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.)
|
||||
|
||||
## v1.12.0 — June 26, 2026
|
||||
|
||||
**Clearer shift drops and an easier-to-read schedule.** Two small quality-of-life
|
||||
|
|
|
|||
|
|
@ -247,6 +247,16 @@ 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)
|
||||
|
||||
|
|
@ -307,35 +317,57 @@ 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"]
|
||||
)
|
||||
# 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).
|
||||
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"]
|
||||
_pin_schedule_post(slack, channel_id, message_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"]),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -82,11 +82,42 @@ 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_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.
|
||||
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()
|
||||
assert result["message_ts"] == "999.000"
|
||||
assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000"
|
||||
|
||||
|
||||
@freeze_time(MON_0800)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue