Skip to content

Fix/Cancellation double publish #407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Adam-C-Mitchell merged 3 commits into from
Sep 14, 2026
+22 −50
Merged

Fix/Cancellation double publish #407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
094a09c
Test with just ignoring cancelled alerts
Adam-C-Mitchell Sep 11, 2026
ad60361
fix: Don't republish for after alert activities at all if one is curr…
Adam-C-Mitchell Sep 12, 2026
80f04aa
Revert "fix: Don't republish for after alert activities at all if one…
Adam-C-Mitchell Sep 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions app/dao/broadcast_message_dao.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ def dao_get_all_pre_broadcast_messages():

def dao_get_all_finished_broadcast_messages_with_outstanding_actions() -> list[BroadcastMessage]:
"""
Find all BroadcastMessages that have finished (either expired or been cancelled)
and have one or more flags indicating actions due.
Find all BroadcastMessages that have finished by natural expiry and still have an
action due (i.e., GOV.UK/Alerts has not been asked to republish for them yet).
"""

now = datetime.now(timezone.utc)
Expand All @@ -274,15 +274,10 @@ def dao_get_all_finished_broadcast_messages_with_outstanding_actions() -> list[B
.filter(
and_(
or_(
# Pick up cancelled alerts after a grace period of 10 minutes,
# to prevent double-republishes when queue_after_alert_activities runs.
and_(
BroadcastMessage.status == BroadcastStatusType.CANCELLED,
BroadcastMessage.cancelled_at < now - timedelta(minutes=10),
),
# Completed or naturally-finished BROADCASTING alerts have no other
# publish trigger, so are picked up immediately.
# Naturally-completed alerts have no other publish trigger.
BroadcastMessage.status == BroadcastStatusType.COMPLETED,
# BROADCASTING alerts whose end time has passed but that have not yet
# been swept into COMPLETED.
and_(
BroadcastMessage.finishes_at < now,
BroadcastMessage.status == BroadcastStatusType.BROADCASTING,
Expand Down
57 changes: 17 additions & 40 deletions tests/app/dao/test_broadcast_message_dao.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,25 +270,18 @@ def test_dao_get_all_finished_broadcast_messages_with_outstanding_actions(sample
finished_govuk_acknowledged=True,
)
create_broadcast_message(
# Cancelled, but already actioned
# Cancelled: never picked up by this job, as cancelling already triggers its own
# GOV.UK publish. Included here to prove it is excluded even when unacknowledged
# and cancelled long ago.
t,
created_at=datetime(2023, 10, 12, 12, 0, 0),
cancelled_at=datetime(2023, 10, 12, 12, 30, 0),
starts_at=datetime.now(),
status=BroadcastStatusType.CANCELLED,
finished_govuk_acknowledged=True,
)

# --- Should be picked up (all in 2024) ---

create_broadcast_message(
# Cancelled more than 10 minutes ago, should be picked up
t,
created_at=datetime(2024, 10, 12, 12, 0, 0),
cancelled_at=datetime(2024, 10, 12, 12, 0, 0),
starts_at=datetime.now(),
status=BroadcastStatusType.CANCELLED,
)
create_broadcast_message(
# Completed, should be picked up
t,
Expand All @@ -307,49 +300,41 @@ def test_dao_get_all_finished_broadcast_messages_with_outstanding_actions(sample

outstanding_action_broadcast_messages = dao_get_all_finished_broadcast_messages_with_outstanding_actions()

assert len(outstanding_action_broadcast_messages) == 3
assert len(outstanding_action_broadcast_messages) == 2

# All ones to *not* pickup are in 2023
assert not any([x.created_at < datetime(2024, 1, 1, 0, 0) for x in outstanding_action_broadcast_messages])
assert not any([x.finished_govuk_acknowledged for x in outstanding_action_broadcast_messages])
# Cancelled alerts are never picked up by this job
assert not any([x.status == BroadcastStatusType.CANCELLED for x in outstanding_action_broadcast_messages])


@freeze_time("2024-12-12 12:12:12")
def test_dao_applies_grace_period_to_recently_cancelled_broadcast_messages(sample_broadcast_service):
def test_dao_never_picks_up_cancelled_broadcast_messages(sample_broadcast_service):
t = create_template(sample_broadcast_service, BROADCAST_TYPE)

now = datetime(2024, 12, 12, 12, 12, 12)

create_broadcast_message(
# Cancelled just now - within the grace period, shouldn't be picked up yet
# Cancelled just now
t,
created_at=now - timedelta(hours=1),
cancelled_at=now - timedelta(minutes=2),
starts_at=now - timedelta(hours=1),
status=BroadcastStatusType.CANCELLED,
)
create_broadcast_message(
# Cancelled just inside the grace period boundary, shouldn't be picked up
t,
created_at=now - timedelta(hours=1),
cancelled_at=now - timedelta(minutes=9, seconds=59),
starts_at=now - timedelta(hours=1),
status=BroadcastStatusType.CANCELLED,
)
create_broadcast_message(
# Cancelled more than 10 minutes ago, should be picked up
# Cancelled a long time ago, still unacknowledged
t,
created_at=now - timedelta(hours=1),
cancelled_at=now - timedelta(minutes=11),
cancelled_at=now - timedelta(hours=1),
starts_at=now - timedelta(hours=1),
status=BroadcastStatusType.CANCELLED,
)

outstanding_action_broadcast_messages = dao_get_all_finished_broadcast_messages_with_outstanding_actions()

# Only the one cancelled more than 10 minutes ago should be returned
assert len(outstanding_action_broadcast_messages) == 1
assert outstanding_action_broadcast_messages[0].cancelled_at == now - timedelta(minutes=11)
assert len(outstanding_action_broadcast_messages) == 0


@freeze_time("2024-12-12 12:12:12")
Expand All @@ -376,15 +361,7 @@ def test_dao_get_only_relevant_broadcast_messages_with_outstanding_actions(sampl
status=BroadcastStatusType.REJECTED,
)

# --- Should be picked up (all have been published) ---
create_broadcast_message(
# Cancelled more than 10 minutes ago, should be picked up
t,
created_at=datetime(2024, 10, 12, 12, 0, 0),
cancelled_at=datetime(2024, 10, 12, 12, 0, 0),
starts_at=datetime.now(),
status=BroadcastStatusType.CANCELLED,
)
# --- Should be picked up (naturally finished, no other publish trigger) ---
create_broadcast_message(
# Completed, should be picked up
t,
Expand All @@ -403,7 +380,7 @@ def test_dao_get_only_relevant_broadcast_messages_with_outstanding_actions(sampl

outstanding_action_broadcast_messages = dao_get_all_finished_broadcast_messages_with_outstanding_actions()

assert len(outstanding_action_broadcast_messages) == 3
assert len(outstanding_action_broadcast_messages) == 2

# All ones to *not* pickup have pre-broadcast status
assert not any(
Expand Down Expand Up @@ -450,12 +427,12 @@ def test_dao_get_only_broadcast_messages_with_outstanding_actions_from_live_serv

# --- Should be picked up ---
create_broadcast_message(
# Cancelled more than 10 minutes ago, should be picked up
# Broadcasting and expired, should be picked up
t,
created_at=datetime(2024, 10, 12, 12, 0, 0),
cancelled_at=datetime(2024, 10, 12, 12, 0, 0),
created_at=datetime(2024, 10, 11, 12, 0, 0),
starts_at=datetime.now(),
status=BroadcastStatusType.CANCELLED,
finishes_at=datetime(2024, 10, 11, 13, 0, 0),
status=BroadcastStatusType.BROADCASTING,
)
create_broadcast_message(
# Completed, should be picked up
Expand Down
Loading