mirror of
https://github.com/Sea-Haven-Industries/afterhours-shift-manager.git
synced 2026-10-03 13:53:21 +00:00
fix: delete+repost schedule on weekly rollover for bottom placement
The Monday rollover was chat_update-ing in place, which only refreshes content without moving the message to the bottom. Now it chat_deletes the old post and chat_postMessages a fresh one so the schedule lands at the bottom every Monday, independent of in-week activity. Also added info-level logging to the bump handler silent return paths so skipped bumps are observable at runtime.
This commit is contained in:
parent
7a8133fea3
commit
0fa5d702be
4 changed files with 57 additions and 47 deletions
|
|
@ -80,13 +80,12 @@ is therefore not a substitute for bottom-stickiness.
|
||||||
|
|
||||||
### Weekly rollover — `src/weekly-post/app.py`
|
### Weekly rollover — `src/weekly-post/app.py`
|
||||||
|
|
||||||
- **Drop the unconditional `chat_delete` + always-repost.** On Monday,
|
- **Delete + repost on Monday** — the previous week's stored post is
|
||||||
`chat_update` the stored post to roll the two-week window forward, keeping the
|
`chat_delete`d and a fresh schedule is `chat_postMessage`d so the message
|
||||||
same `ts` (the activity bump is what moves it to the bottom; the rollover just
|
lands at the bottom of the channel every Monday regardless of in-week
|
||||||
refreshes content).
|
activity. The delete failure is non-fatal; the handler always reposts.
|
||||||
- **First-run / recovery fallback:** when there is no stored post or the
|
- **First-run:** when there is no stored post to delete, the handler
|
||||||
`chat_update` fails (e.g. the message was deleted manually), `chat_postMessage`
|
`chat_postMessage`s a fresh message and stores its `ts`.
|
||||||
a fresh message and store its `ts`.
|
|
||||||
- The `pins:write` scope and the `_pin_schedule_post` helper are removed — the
|
- The `pins:write` scope and the `_pin_schedule_post` helper are removed — the
|
||||||
pin is fully replaced by bottom-stickiness.
|
pin is fully replaced by bottom-stickiness.
|
||||||
|
|
||||||
|
|
@ -123,16 +122,18 @@ request URL. On a `message.channels` event in the schedule channel
|
||||||
|
|
||||||
All three paths converge on the **single stored `ts`** and never fight:
|
All three paths converge on the **single stored `ts`** and never fight:
|
||||||
|
|
||||||
- **Monday rollover** — `chat_update` the stored `ts` (content refresh; same
|
- **Monday rollover** — `chat_delete`s the old stored `ts`, `chat_postMessage`s
|
||||||
message), clearing `last_bump_ts` so the next activity is free to bump.
|
a fresh post at the bottom, and saves the new `ts` (clearing `last_bump_ts` so
|
||||||
- **In-week shift edits** — `_refresh_schedule_post` `chat_update`s the same
|
the next activity is free to bump).
|
||||||
|
- **In-week shift edits** — `_refresh_schedule_post` `chat_update`s the current
|
||||||
stored `ts`; it does not change the `ts` or touch `last_bump_ts`.
|
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
|
- **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
|
bottom, and saves the new `ts`. Subsequent rollovers and edits then operate on
|
||||||
that new `ts`.
|
that new `ts`.
|
||||||
|
|
||||||
Because only the bump ever mints a new `ts` (and it always re-saves it
|
Both the rollover and the bump mint a new `ts` on repost and re-save it
|
||||||
immediately), the other two paths always read the current `ts` from the record.
|
immediately, so `_refresh_schedule_post` always reads the current `ts` from the
|
||||||
|
record.
|
||||||
|
|
||||||
## Scope / manifest impact
|
## Scope / manifest impact
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -461,15 +461,25 @@ def handle_channel_message(event, client, schedule, schedule_channel, retry_num=
|
||||||
logger.info("Ignoring retried message event (retry %s)", retry_num)
|
logger.info("Ignoring retried message event (retry %s)", retry_num)
|
||||||
return
|
return
|
||||||
if not schedule_channel or event.get("channel") != schedule_channel:
|
if not schedule_channel or event.get("channel") != schedule_channel:
|
||||||
|
logger.info(
|
||||||
|
"Skipping bump — channel mismatch (expected %s, got %s)",
|
||||||
|
schedule_channel,
|
||||||
|
event.get("channel"),
|
||||||
|
)
|
||||||
return
|
return
|
||||||
# Ignore the bot's own posts and non-user message events (edits, deletes,
|
# Ignore the bot's own posts and non-user message events (edits, deletes,
|
||||||
# joins, …), plus thread replies — a threaded reply doesn't push the
|
# joins, …), plus thread replies — a threaded reply doesn't push the
|
||||||
# schedule down the main timeline, so it isn't worth a delete+repost.
|
# 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"):
|
if event.get("bot_id") or event.get("subtype") or event.get("thread_ts"):
|
||||||
|
logger.info(
|
||||||
|
"Skipping bump — bot message, subtype %s, or thread reply",
|
||||||
|
event.get("subtype"),
|
||||||
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
post = schedule.get_schedule_post(schedule_channel)
|
post = schedule.get_schedule_post(schedule_channel)
|
||||||
if not post or not post.get("message_ts"):
|
if not post or not post.get("message_ts"):
|
||||||
|
logger.info("Skipping bump — no stored schedule post for %s", schedule_channel)
|
||||||
return
|
return
|
||||||
|
|
||||||
now = time.time()
|
now = time.time()
|
||||||
|
|
@ -478,6 +488,11 @@ def handle_channel_message(event, client, schedule, schedule_channel, retry_num=
|
||||||
last_bump is not None
|
last_bump is not None
|
||||||
and now - float(last_bump) < SCHEDULE_BUMP_DEBOUNCE_SECONDS
|
and now - float(last_bump) < SCHEDULE_BUMP_DEBOUNCE_SECONDS
|
||||||
):
|
):
|
||||||
|
logger.info(
|
||||||
|
"Skipping bump — debounced (last bump %.0fs ago, window %ds)",
|
||||||
|
now - float(last_bump),
|
||||||
|
SCHEDULE_BUMP_DEBOUNCE_SECONDS,
|
||||||
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
# Optimistically stamp the debounce window *before* the delete/repost, so a
|
# Optimistically stamp the debounce window *before* the delete/repost, so a
|
||||||
|
|
|
||||||
|
|
@ -318,39 +318,31 @@ def handler(event, context):
|
||||||
f"to {end_date.strftime('%b %-d')}"
|
f"to {end_date.strftime('%b %-d')}"
|
||||||
)
|
)
|
||||||
|
|
||||||
# Roll the two-week window forward by editing the stored message in place
|
# Delete the previous week's schedule post + repost a fresh one so the
|
||||||
# (same ts) so the Monday rollover, in-week shift edits, and the activity
|
# message lands at the bottom of the channel every Monday. The activity
|
||||||
# bump all converge on a single stored ts. Fall back to a fresh post when
|
# bump (handle_channel_message) handles in-week bottom-stickiness; this
|
||||||
# there is no stored message or the edit fails (e.g. it was deleted).
|
# ensures the rollover itself re-places the post at the bottom.
|
||||||
old_post = schedule.get_schedule_post(channel_id)
|
old_post = schedule.get_schedule_post(channel_id)
|
||||||
message_ts = None
|
|
||||||
if old_post and old_post.get("message_ts"):
|
if old_post and old_post.get("message_ts"):
|
||||||
try:
|
try:
|
||||||
slack.chat_update(
|
slack.chat_delete(channel=channel_id, ts=old_post["message_ts"])
|
||||||
channel=channel_id,
|
|
||||||
ts=old_post["message_ts"],
|
|
||||||
blocks=blocks,
|
|
||||||
text=fallback_text,
|
|
||||||
)
|
|
||||||
message_ts = old_post["message_ts"]
|
|
||||||
logger.info(
|
logger.info(
|
||||||
"Rolled schedule post %s forward to week of %s",
|
"Deleted previous schedule post %s for weekly rollover",
|
||||||
message_ts,
|
old_post["message_ts"],
|
||||||
week_start,
|
|
||||||
)
|
)
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Could not update existing schedule post; reposting", exc_info=True
|
"Could not delete old schedule post for rollover; continuing",
|
||||||
|
exc_info=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
if message_ts is None:
|
result = slack.chat_postMessage(
|
||||||
result = slack.chat_postMessage(
|
channel=channel_id, blocks=blocks, text=fallback_text
|
||||||
channel=channel_id, blocks=blocks, text=fallback_text
|
)
|
||||||
)
|
message_ts = result["ts"]
|
||||||
message_ts = result["ts"]
|
logger.info(
|
||||||
logger.info(
|
"Posted new weekly schedule to channel %s (ts=%s)", channel_id, message_ts
|
||||||
"Posted new weekly schedule to channel %s (ts=%s)", channel_id, message_ts
|
)
|
||||||
)
|
|
||||||
|
|
||||||
schedule.save_schedule_post(channel_id, message_ts, week_start)
|
schedule.save_schedule_post(channel_id, message_ts, week_start)
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
|
|
@ -83,24 +83,26 @@ def test_pay_email_failure_does_not_block_schedule_post(
|
||||||
|
|
||||||
@freeze_time(MON_0800)
|
@freeze_time(MON_0800)
|
||||||
def test_rolls_existing_post_forward(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.
|
# An existing post is deleted + reposted so it lands at the bottom every
|
||||||
|
# Monday. The old ts is cleaned up and a new one is stored.
|
||||||
seed.schedule_post("C_TEST", "111.111")
|
seed.schedule_post("C_TEST", "111.111")
|
||||||
result = weeklypost_app.handler({"force": True}, None)
|
result = weeklypost_app.handler({"force": True}, None)
|
||||||
|
|
||||||
slack.chat_delete.assert_not_called()
|
slack.chat_delete.assert_called_once()
|
||||||
slack.chat_update.assert_called_once()
|
assert slack.chat_delete.call_args.kwargs["ts"] == "111.111"
|
||||||
assert slack.chat_update.call_args.kwargs["ts"] == "111.111"
|
slack.chat_update.assert_not_called()
|
||||||
# The stored ts is unchanged, so the permalink stays stable.
|
slack.chat_postMessage.assert_called_once()
|
||||||
assert result["message_ts"] == "111.111"
|
# New ts from the repost is stored.
|
||||||
assert schedule.get_schedule_post("C_TEST")["message_ts"] == "111.111"
|
assert result["message_ts"] == "999.000"
|
||||||
|
assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000"
|
||||||
|
|
||||||
|
|
||||||
@freeze_time(MON_0800)
|
@freeze_time(MON_0800)
|
||||||
def test_reposts_when_update_fails(weeklypost_app, schedule, seed, slack, env):
|
def test_reposts_when_delete_fails(weeklypost_app, schedule, seed, slack, env):
|
||||||
# A stored post that can no longer be edited (e.g. deleted manually) falls
|
# A stored post that can no longer be deleted (e.g. was already removed
|
||||||
# back to a fresh post and re-saves the new ts.
|
# manually) still reposts — the delete failure is non-fatal.
|
||||||
seed.schedule_post("C_TEST", "111.111")
|
seed.schedule_post("C_TEST", "111.111")
|
||||||
slack.chat_update.side_effect = Exception("message_not_found")
|
slack.chat_delete.side_effect = Exception("message_not_found")
|
||||||
|
|
||||||
result = weeklypost_app.handler({"force": True}, None)
|
result = weeklypost_app.handler({"force": True}, None)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue