fix(campaign): mark excluded delivery as skipped

This commit is contained in:
2026-07-22 08:48:46 +02:00
parent 3487ec7048
commit 7229fb8e3d
19 changed files with 249 additions and 9 deletions

View File

@@ -107,6 +107,8 @@ bed where possible:
## Reporting Checks
- Partial delivery must show accepted, failed, and unknown counts separately.
- Excluded messages must show SMTP and IMAP as `skipped`, with skipped counts
and filters separate from unattempted or failed delivery.
- Accepted and unknown jobs must not appear in retry selections.
- Reconciled accepted jobs must remain protected from resend.
- Reconciled not-sent jobs must appear only as explicit retry candidates.

View File

@@ -261,6 +261,13 @@ The delivery record should be able to identify:
reconciliation note; and
- actor/system trigger, timestamps, policy context, and corrections.
An excluded recipient/message is a completed validation decision, not a
pending delivery. Its SMTP and IMAP states are both `skipped`; it is counted and
filterable separately from unattempted, failed, accepted, and append outcomes.
No SMTP or IMAP attempt exists for such a row. If historical data contains
actual transport evidence despite an exclusion marker, that evidence is
preserved for audit and reconciliation rather than relabelled.
## Administration and policy
### Roles and permissions

View File

@@ -89,6 +89,7 @@ class BuildStatus(StrEnum):
class SendStatus(StrEnum):
DRAFT = "draft"
QUEUED = "queued"
SKIPPED = "skipped"
class CampaignMeta(StrictModel):

View File

@@ -78,6 +78,7 @@ class JobQueueStatus(StrEnum):
class JobSendStatus(StrEnum):
NOT_QUEUED = "not_queued"
SKIPPED = "skipped"
QUEUED = "queued"
CLAIMED = "claimed"
SENDING = "sending"

View File

@@ -513,6 +513,12 @@ def _message_draft(
eml_path: str | None = None,
eml_size: int | None = None,
) -> MessageDraft:
if validation_status == MessageValidationStatus.EXCLUDED:
# Exclusion is a completed validation decision, not a pending delivery.
# Keep both transport projections explicit so reports never imply that
# SMTP or IMAP work is still expected for this row.
send_status = SendStatus.SKIPPED
imap_status = ImapStatus.SKIPPED
if imap_status is None:
imap_status = _imap_initial_status(config) if build_status == BuildStatus.BUILT else ImapStatus.SKIPPED
return MessageDraft(

View File

@@ -0,0 +1,42 @@
"""mark untouched excluded jobs as skipped delivery
Revision ID: d8b3e2c1f4a5
Revises: c7a2f91e4b60
Create Date: 2026-07-22 11:00:00.000000
"""
from __future__ import annotations
from alembic import op
import sqlalchemy as sa
revision = "d8b3e2c1f4a5"
down_revision = "c7a2f91e4b60"
branch_labels = None
depends_on = None
def upgrade() -> None:
# Only normalize rows with no recorded transport effect. Unexpected
# historical delivery evidence must remain intact for audit/reconciliation.
op.get_bind().execute(
sa.text(
"UPDATE campaign_jobs "
"SET send_status = 'skipped', imap_status = 'skipped' "
"WHERE validation_status = 'excluded' "
"AND send_status = 'not_queued' "
"AND imap_status IN ('not_requested', 'pending', 'skipped')"
)
)
def downgrade() -> None:
op.get_bind().execute(
sa.text(
"UPDATE campaign_jobs "
"SET send_status = 'not_queued', imap_status = 'not_requested' "
"WHERE validation_status = 'excluded' "
"AND send_status = 'skipped' "
"AND imap_status = 'skipped'"
)
)

View File

@@ -0,0 +1,42 @@
"""mark untouched excluded jobs as skipped delivery
Revision ID: d8b3e2c1f4a5
Revises: c7a2f91e4b60
Create Date: 2026-07-22 11:00:00.000000
"""
from __future__ import annotations
from alembic import op
import sqlalchemy as sa
revision = "d8b3e2c1f4a5"
down_revision = "c7a2f91e4b60"
branch_labels = None
depends_on = None
def upgrade() -> None:
# Only normalize rows with no recorded transport effect. Unexpected
# historical delivery evidence must remain intact for audit/reconciliation.
op.get_bind().execute(
sa.text(
"UPDATE campaign_jobs "
"SET send_status = 'skipped', imap_status = 'skipped' "
"WHERE validation_status = 'excluded' "
"AND send_status = 'not_queued' "
"AND imap_status IN ('not_requested', 'pending', 'skipped')"
)
)
def downgrade() -> None:
op.get_bind().execute(
sa.text(
"UPDATE campaign_jobs "
"SET send_status = 'not_queued', imap_status = 'not_requested' "
"WHERE validation_status = 'excluded' "
"AND send_status = 'skipped' "
"AND imap_status = 'skipped'"
)
)

View File

@@ -35,7 +35,7 @@ from govoplan_campaign.backend.campaign.validation import validate_campaign_conf
from govoplan_campaign.backend.messages.builder import build_campaign_messages
from govoplan_campaign.backend.messages.models import MessageDraft
from govoplan_campaign.backend.sending.execution import create_execution_snapshot, profile_transport_revisions
from govoplan_campaign.backend.campaign.models import CampaignConfig
from govoplan_campaign.backend.campaign.models import CampaignConfig, SendStatus
from govoplan_campaign.backend.integrations import files_integration, mail_integration
from govoplan_campaign.backend.path_security import assert_server_safe_campaign_paths
@@ -406,7 +406,11 @@ def _job_from_message(
build_status=message.build_status.value if hasattr(message.build_status, "value") else str(message.build_status),
validation_status=_job_validation_status(message.validation_status.value),
queue_status=JobQueueStatus.DRAFT.value,
send_status=JobSendStatus.NOT_QUEUED.value,
send_status=(
JobSendStatus.SKIPPED.value
if message.send_status == SendStatus.SKIPPED
else JobSendStatus.NOT_QUEUED.value
),
imap_status=message.imap_status.value if hasattr(message.imap_status, "value") else JobImapStatus.NOT_REQUESTED.value,
resolved_recipients={
"from": message.from_.model_dump(mode="json") if message.from_ else None,

View File

@@ -114,10 +114,10 @@ def _load_delivery_info(
"queueable_job_count": 0,
"estimated_remaining_send_seconds": None,
"estimated_remaining_send_human": None,
"delivery_mode": version.delivery_mode if version else None,
"delivery_mode": getattr(version, "delivery_mode", None) if version else None,
"delivery_mode_selected_at": (
version.delivery_mode_selected_at.isoformat()
if version and version.delivery_mode_selected_at
getattr(version, "delivery_mode_selected_at", None).isoformat()
if version and getattr(version, "delivery_mode_selected_at", None)
else None
),
}
@@ -577,7 +577,7 @@ def _campaign_report_cards(version: CampaignVersion | None, jobs: list[CampaignJ
1
for job in jobs
if job.send_status
not in {"smtp_accepted", "sent", "outcome_unknown", "claimed", "sending", "cancelled"}
not in {"skipped", "smtp_accepted", "sent", "outcome_unknown", "claimed", "sending", "cancelled"}
)
needs_attention = sum(
1
@@ -590,6 +590,7 @@ def _campaign_report_cards(version: CampaignVersion | None, jobs: list[CampaignJ
failed = send_counts.get("failed_temporary", 0) + send_counts.get("failed_permanent", 0)
outcome_unknown = send_counts.get("outcome_unknown", 0)
not_attempted = send_counts.get("not_queued", 0)
skipped = send_counts.get("skipped", 0)
queued = send_counts.get("queued", 0) + send_counts.get("claimed", 0) + send_counts.get("sending", 0)
cancelled = send_counts.get("cancelled", 0)
inactive_entries = _inactive_entry_count(version)
@@ -606,11 +607,13 @@ def _campaign_report_cards(version: CampaignVersion | None, jobs: list[CampaignJ
"failed": failed,
"outcome_unknown": outcome_unknown,
"not_attempted": not_attempted,
"skipped": skipped,
"queued_or_active": queued,
"cancelled": cancelled,
"partially_completed": bool(sent and (failed or outcome_unknown or not_attempted or cancelled)),
"imap_appended": imap_counts.get("appended", 0),
"imap_failed": imap_counts.get("failed", 0),
"imap_skipped": imap_counts.get("skipped", 0),
}

View File

@@ -88,8 +88,10 @@ def _text_summary(report: dict[str, Any]) -> str:
f"- Needs attention: {cards['needs_attention']}",
f"- Sent: {cards['sent']}",
f"- Failed: {cards['failed']}",
f"- SMTP skipped (excluded): {cards.get('skipped', status.get('send', {}).get('skipped', 0))}",
f"- IMAP appended: {cards['imap_appended']}",
f"- IMAP failed: {cards['imap_failed']}",
f"- IMAP skipped: {cards.get('imap_skipped', status.get('imap', {}).get('skipped', 0))}",
"",
f"Build status: {status.get('build', {})}",
f"Validation status: {status.get('validation', {})}",

View File

@@ -212,6 +212,7 @@ DELIVERY_MODES = {
AUTOMATICALLY_SENDABLE_STATUSES = {JobSendStatus.QUEUED.value}
EXPLICIT_RETRY_STATUSES = {JobSendStatus.FAILED_TEMPORARY.value, JobSendStatus.FAILED_PERMANENT.value}
INITIAL_QUEUE_SKIPPED_SEND_STATUSES = SMTP_ACCEPTED_STATUSES | {
JobSendStatus.SKIPPED.value,
JobSendStatus.CLAIMED.value,
JobSendStatus.SENDING.value,
JobSendStatus.OUTCOME_UNKNOWN.value,
@@ -1001,9 +1002,13 @@ def cancel_campaign_jobs(session: Session, *, tenant_id: str, campaign_id: str)
)
cancelled_count = 0
protected_count = 0
skipped_count = 0
version_ids: set[str] = set()
for job in jobs:
version_ids.add(job.campaign_version_id)
if job.send_status == JobSendStatus.SKIPPED.value:
skipped_count += 1
continue
if job.send_status in SMTP_ACCEPTED_STATUSES | {
JobSendStatus.OUTCOME_UNKNOWN.value,
JobSendStatus.CLAIMED.value,
@@ -1024,6 +1029,7 @@ def cancel_campaign_jobs(session: Session, *, tenant_id: str, campaign_id: str)
"campaign_id": campaign.id,
"cancelled_count": cancelled_count,
"protected_count": protected_count,
"skipped_count": skipped_count,
"campaign_status": campaign.status,
}

View File

@@ -123,6 +123,9 @@ class CampaignAttachmentBuildTests(unittest.TestCase):
message = result.report.messages[0]
self.assertEqual(message.build_status.value, build_status)
self.assertEqual(message.validation_status.value, validation_status)
if validation_status == "excluded":
self.assertEqual(message.send_status.value, "skipped")
self.assertEqual(message.imap_status.value, "skipped")
coverage_issues = [issue for issue in message.issues if issue.code == "missing_attachment_coverage"]
if issue_behavior is None:
self.assertEqual(coverage_issues, [])

View File

@@ -0,0 +1,67 @@
from __future__ import annotations
from importlib import import_module
from unittest.mock import patch
from sqlalchemy import create_engine, text
from govoplan_campaign.backend.campaign.models import BuildStatus, SendStatus
from govoplan_campaign.backend.messages.models import ImapStatus, MessageDraft, MessageValidationStatus
from govoplan_campaign.backend.persistence.campaigns import _job_from_message
def test_excluded_message_persists_explicit_skipped_transport_states() -> None:
message = MessageDraft(
entry_index=0,
entry_id="excluded-entry",
active=True,
build_status=BuildStatus.BUILT,
validation_status=MessageValidationStatus.EXCLUDED,
send_status=SendStatus.SKIPPED,
imap_status=ImapStatus.SKIPPED,
)
job = _job_from_message(
tenant_id="tenant-1",
campaign_id="campaign-1",
version_id="version-1",
message=message,
)
assert job.send_status == "skipped"
assert job.imap_status == "skipped"
def test_status_migration_only_normalizes_excluded_rows_without_transport_evidence() -> None:
migration = import_module(
"govoplan_campaign.backend.migrations.versions."
"d8b3e2c1f4a5_v0110_excluded_delivery_skipped"
)
engine = create_engine("sqlite+pysqlite:///:memory:")
with engine.begin() as connection:
connection.execute(text(
"CREATE TABLE campaign_jobs ("
"id TEXT PRIMARY KEY, validation_status TEXT NOT NULL, "
"send_status TEXT NOT NULL, imap_status TEXT NOT NULL)"
))
connection.execute(
text(
"INSERT INTO campaign_jobs (id, validation_status, send_status, imap_status) VALUES "
"('untouched', 'excluded', 'not_queued', 'pending'), "
"('evidence', 'excluded', 'smtp_accepted', 'appended'), "
"('queueable', 'ready', 'not_queued', 'pending')"
)
)
with patch.object(migration.op, "get_bind", return_value=connection):
migration.upgrade()
rows = {
row.id: (row.send_status, row.imap_status)
for row in connection.execute(
text("SELECT id, send_status, imap_status FROM campaign_jobs ORDER BY id")
)
}
assert rows["untouched"] == ("skipped", "skipped")
assert rows["evidence"] == ("smtp_accepted", "appended")
assert rows["queueable"] == ("not_queued", "pending")

View File

@@ -40,6 +40,7 @@ class CampaignJobListQueryTests(unittest.TestCase):
(3, "ordinary-3@example.test", "General notice", "blocked", "draft", "failed_permanent", "failed", 3),
(4, "target-b@example.test", "Target notice B", "ready", "draft", "failed_temporary", "not_requested", 4),
(5, "target-a@example.test", "Target notice A", "ready", "draft", "outcome_unknown", "outcome_unknown", 5),
(6, "excluded@example.test", "Excluded notice", "excluded", "draft", "skipped", "skipped", 0),
]
for entry_index, recipient, subject, validation, queue, send, imap, attempts in rows:
self.session.add(CampaignJob(
@@ -89,7 +90,7 @@ class CampaignJobListQueryTests(unittest.TestCase):
)
self.assertEqual(page.total, 3)
self.assertEqual(page.total_unfiltered, 6)
self.assertEqual(page.total_unfiltered, 7)
self.assertEqual(page.pages, 2)
self.assertEqual(
[row["recipient_email"] for row in page.jobs],
@@ -118,6 +119,31 @@ class CampaignJobListQueryTests(unittest.TestCase):
self.assertEqual(raised.exception.status_code, 422)
def test_skipped_transport_filters_and_counts_remain_separate(self) -> None:
base_filters = [CampaignJob.tenant_id == "tenant-1", CampaignJob.campaign_id == "campaign-1"]
grid_filters = {"send": 'list:["skipped"]', "imap": 'list:["skipped"]'}
filtered = [*base_filters, *_campaign_jobs_grid_filter_expressions(grid_filters)]
page = _campaign_jobs_page_response(
self.session,
campaign_id="campaign-1",
version_id="version-1",
base_filters=base_filters,
filtered=filtered,
reviewed_keys=set(),
review_metadata={},
page=1,
page_size=20,
grid_filters=grid_filters,
)
self.assertEqual([row["id"] for row in page.jobs], ["job-6"])
self.assertEqual(page.counts["send"]["skipped"], 1)
self.assertEqual(page.counts["send"]["not_queued"], 2)
self.assertEqual(page.counts["imap"]["skipped"], 1)
self.assertEqual(page.filtered_counts["send"], {"skipped": 1})
self.assertEqual(page.filtered_counts["imap"], {"skipped": 1})
if __name__ == "__main__":
unittest.main()

View File

@@ -53,6 +53,13 @@ class CampaignQueueControlTests(unittest.TestCase):
self._add_job("unknown", queue="draft", send="outcome_unknown")
self._add_job("accepted", queue="draft", send="smtp_accepted")
self._add_job("claimed", queue="sending", send="claimed")
self._add_job(
"excluded",
queue="draft",
send="skipped",
validation="excluded",
imap="skipped",
)
self._add_job(
"other-tenant",
tenant_id="tenant-2",
@@ -111,8 +118,10 @@ class CampaignQueueControlTests(unittest.TestCase):
self.assertEqual(first["cancelled_count"], 3)
self.assertEqual(first["protected_count"], 3)
self.assertEqual(first["skipped_count"], 1)
self.assertEqual(second["cancelled_count"], 0)
self.assertEqual(second["protected_count"], 3)
self.assertEqual(second["skipped_count"], 1)
self.session.expire_all()
for job_id in ("queued", "paused", "failed"):
job = self.session.get(CampaignJob, job_id)
@@ -121,6 +130,8 @@ class CampaignQueueControlTests(unittest.TestCase):
self.assertEqual(self.session.get(CampaignJob, "unknown").send_status, JobSendStatus.OUTCOME_UNKNOWN.value)
self.assertEqual(self.session.get(CampaignJob, "accepted").send_status, JobSendStatus.SMTP_ACCEPTED.value)
self.assertEqual(self.session.get(CampaignJob, "claimed").send_status, JobSendStatus.CLAIMED.value)
self.assertEqual(self.session.get(CampaignJob, "excluded").send_status, JobSendStatus.SKIPPED.value)
self.assertEqual(self.session.get(CampaignJob, "excluded").queue_status, JobQueueStatus.DRAFT.value)
self.assertEqual(self.session.get(CampaignJob, "other-tenant").send_status, JobSendStatus.QUEUED.value)
def test_controls_fail_closed_for_a_campaign_owned_by_another_tenant(self) -> None:
@@ -148,6 +159,8 @@ class CampaignQueueControlTests(unittest.TestCase):
self.assertEqual(cards["retryable"], 1)
self.assertEqual(cards["queueable_unattempted"], 0)
self.assertEqual(cards["cancellable"], 3)
self.assertEqual(cards["skipped"], 1)
self.assertEqual(cards["imap_skipped"], 1)
self.assertEqual(projected_version["delivery_mode"], "worker_queue")
self.assertIn("delivery_mode_selected_at", projected_version)
self.assertNotIn("execution_snapshot", projected_version)
@@ -179,6 +192,8 @@ class CampaignQueueControlTests(unittest.TestCase):
tenant_id: str = "tenant-1",
campaign_id: str = "campaign-1",
version_id: str = "version-1",
validation: str = JobValidationStatus.READY.value,
imap: str = "not_requested",
) -> None:
self.session.add(CampaignJob(
id=job_id,
@@ -188,9 +203,10 @@ class CampaignQueueControlTests(unittest.TestCase):
entry_index=len(self.session.new),
entry_id=f"entry-{job_id}",
build_status=JobBuildStatus.BUILT.value,
validation_status=JobValidationStatus.READY.value,
validation_status=validation,
queue_status=queue,
send_status=send,
imap_status=imap,
resolved_attachments=[],
issues_snapshot=[],
))

View File

@@ -258,11 +258,13 @@ export type CampaignSummary = {
failed?: number;
outcome_unknown?: number;
not_attempted?: number;
skipped?: number;
queued_or_active?: number;
cancelled?: number;
partially_completed?: boolean;
imap_appended?: number;
imap_failed?: number;
imap_skipped?: number;
};
status_counts?: Record<string, Record<string, number>>;
issues?: Record<string, unknown>;

View File

@@ -31,6 +31,7 @@ import type { CampaignJobSortColumn } from "./utils/jobListQuery";
const SEND_STATUS_OPTIONS: DataGridListOption[] = [
"not_queued",
"skipped",
"queued",
"claimed",
"sending",
@@ -387,6 +388,7 @@ export default function CampaignReportPage({ settings, campaignId }: {settings:
<div><dt>i18n:govoplan-campaign.failed.09fef5d8</dt><dd>{cards?.failed ?? 0}</dd></div>
<div><dt>i18n:govoplan-campaign.outcome_unknown.6e929fca</dt><dd>{cards?.outcome_unknown ?? 0}</dd></div>
<div><dt>i18n:govoplan-campaign.not_attempted.e1be3c69</dt><dd>{cards?.not_attempted ?? 0}</dd></div>
<div><dt>SMTP skipped (excluded)</dt><dd>{cards?.skipped ?? jobs.counts.send?.skipped ?? 0}</dd></div>
<div><dt>i18n:govoplan-campaign.cancelled.a1bf92ef</dt><dd>{cards?.cancelled ?? 0}</dd></div>
</dl>
</Card>
@@ -394,6 +396,7 @@ export default function CampaignReportPage({ settings, campaignId }: {settings:
<dl className="detail-list">
<div><dt>i18n:govoplan-campaign.imap_appended.56017ea3</dt><dd>{cards?.imap_appended ?? 0}</dd></div>
<div><dt>i18n:govoplan-campaign.imap_failed.50dbca55</dt><dd>{cards?.imap_failed ?? 0}</dd></div>
<div><dt>IMAP skipped</dt><dd>{cards?.imap_skipped ?? jobs.counts.imap?.skipped ?? 0}</dd></div>
<div><dt>i18n:govoplan-campaign.append_policy.f195cb05</dt><dd>{imapPolicy.enabled === true ? i18nMessage("i18n:govoplan-campaign.enabled_value.e395e48f", { value0: String(imapPolicy.folder ?? "i18n:govoplan-campaign.auto.0d612c12") }) : "i18n:govoplan-campaign.disabled.f4f4473d"}</dd></div>
<div><dt>i18n:govoplan-campaign.rate_limit.d08e55f5</dt><dd>{rateLimit.messages_per_minute ? i18nMessage("i18n:govoplan-campaign.value_minute.aeb1a9ea", { value0: String(rateLimit.messages_per_minute) }) : "—"}</dd></div>
<div><dt>i18n:govoplan-campaign.minimum_remaining_duration.639b792c</dt><dd>{String(delivery.estimated_remaining_send_human ?? "—")}</dd></div>
@@ -413,6 +416,9 @@ export default function CampaignReportPage({ settings, campaignId }: {settings:
</div>
<Card title="i18n:govoplan-campaign.recipient_delivery_jobs.52492608">
<p className="muted small-note">
Excluded rows are intentionally omitted from delivery. Their SMTP and IMAP states are shown as Skipped because no transport effect is attempted; use the separate status filters to isolate them.
</p>
<div className="page-heading split">
<div className="button-row compact-actions">
<FormField label="i18n:govoplan-campaign.search_recipient_subject_or_entry_id.6d6544f5">

View File

@@ -902,7 +902,7 @@ export default function ReviewSendPage({ settings, auth, campaignId }: {settings
setMessage(
action === "pause" ? `Paused ${String(result.paused_count ?? 0)} queued message(s).` :
action === "resume" ? `Resumed ${String(result.resumed_count ?? 0)} message(s); ${String(result.enqueued_count ?? 0)} worker task(s) published.` :
`Cancelled ${String(result.cancelled_count ?? 0)} unsent message(s); ${String(result.protected_count ?? 0)} protected outcome(s) were retained.`
`Cancelled ${String(result.cancelled_count ?? 0)} unsent message(s); ${String(result.protected_count ?? 0)} protected outcome(s) and ${String(result.skipped_count ?? 0)} excluded/skipped message(s) were retained.`
);
if (action === "cancel") setCancelDeliveryConfirmOpen(false);
await reload();

View File

@@ -40,4 +40,8 @@ assert(reportSource.includes("totalRows: jobs.total"), "the shared pagination co
assert(reportSource.includes("onQueryChange={handleJobGridQuery}"), "header sort and filter changes drive the backend query");
assert(reportSource.includes("initialReportGridFilters()"), "status deep links initialize the DataGrid filters");
assert(reportSource.includes("initialReportQuery()"), "q deep links initialize the report search");
assert(reportSource.includes('"skipped",\n"queued"'), "SMTP skipped is a first-class report filter option");
assert(reportSource.includes("Excluded rows are intentionally omitted from delivery"), "the report explains excluded transport semantics");
assert(reportSource.includes("cards?.skipped ?? jobs.counts.send?.skipped"), "SMTP skipped has a separate report count");
assert(reportSource.includes("cards?.imap_skipped ?? jobs.counts.imap?.skipped"), "IMAP skipped has a separate report count");
assert(!reportSource.includes("setPage((value) => Math.max(1, value - 1))"), "the one-off report pager is removed in favor of the central DataGrid pager");