fix(delivery): split validated vs delivered lifecycle to remove sent overreach
This commit is contained in:
@@ -380,7 +380,13 @@ def _build_portal_context(order: Order, settings: Settings) -> dict[str, Any]:
|
||||
try:
|
||||
sent_station_events = notification_service.list_events(
|
||||
order.id,
|
||||
status="sent",
|
||||
status="validated",
|
||||
channel="station",
|
||||
)
|
||||
if not sent_station_events:
|
||||
sent_station_events = notification_service.list_events(
|
||||
order.id,
|
||||
status="delivered",
|
||||
channel="station",
|
||||
)
|
||||
finally:
|
||||
|
||||
@@ -15,7 +15,8 @@ from data.orders.models import utc_now_iso
|
||||
@dataclass
|
||||
class DispatchResult:
|
||||
processed: int = 0
|
||||
sent: int = 0
|
||||
validated: int = 0
|
||||
delivered: int = 0
|
||||
failed: int = 0
|
||||
|
||||
|
||||
@@ -48,7 +49,7 @@ class DeliveryDispatcher:
|
||||
self,
|
||||
*,
|
||||
channel: str = "station",
|
||||
statuses: tuple[str, ...] = ("ready", "failed"),
|
||||
statuses: tuple[str, ...] = ("ready", "validated"),
|
||||
limit: int = 100,
|
||||
) -> DispatchResult:
|
||||
result = DispatchResult()
|
||||
@@ -68,6 +69,29 @@ class DeliveryDispatcher:
|
||||
continue
|
||||
assert payload is not None
|
||||
sent_at = utc_now_iso()
|
||||
if event.status != "validated":
|
||||
# Stash the rendered station notice into the validated
|
||||
# payload so the portal status page can still display it
|
||||
# even though ``station`` does not transition to
|
||||
# ``delivered``.
|
||||
persisted_payload = dict(payload)
|
||||
if event.channel == "station":
|
||||
persisted_payload["station_notice"] = {
|
||||
"title": "报告已就绪",
|
||||
"body": (
|
||||
f"订单 {event.order_id} 的志愿报告已就绪,"
|
||||
"可在当前状态页查看在线报告并下载 PDF。"
|
||||
),
|
||||
"delivered_at": sent_at,
|
||||
}
|
||||
self._service.mark_validated(
|
||||
event.order_id,
|
||||
event_type=event.event_type,
|
||||
payload_json=json.dumps(
|
||||
persisted_payload, ensure_ascii=False
|
||||
),
|
||||
)
|
||||
result.validated += 1
|
||||
try:
|
||||
rendered_payload = self._deliver_event(
|
||||
event,
|
||||
@@ -82,13 +106,20 @@ class DeliveryDispatcher:
|
||||
)
|
||||
result.failed += 1
|
||||
continue
|
||||
self._service.mark_sent(
|
||||
# Only channels with a real downstream sink (currently
|
||||
# ``email``) get marked ``delivered``. ``station`` is local
|
||||
# render only; its persisted payload already records the
|
||||
# rendered notice, so we stop at ``validated`` for it.
|
||||
if event.channel == "email":
|
||||
self._service.mark_delivered(
|
||||
event.order_id,
|
||||
event_type=event.event_type,
|
||||
payload_json=json.dumps(rendered_payload, ensure_ascii=False),
|
||||
payload_json=json.dumps(
|
||||
rendered_payload, ensure_ascii=False
|
||||
),
|
||||
sent_at=sent_at,
|
||||
)
|
||||
result.sent += 1
|
||||
result.delivered += 1
|
||||
return result
|
||||
|
||||
@staticmethod
|
||||
@@ -116,14 +147,19 @@ class DeliveryDispatcher:
|
||||
payload: dict[str, object],
|
||||
sent_at: str,
|
||||
) -> dict[str, object]:
|
||||
"""Push the validated event to the real downstream sink.
|
||||
|
||||
Returns the rendered payload (with the channel-specific
|
||||
downstream notice) so the caller can persist it via
|
||||
:meth:`mark_delivered`. ``station`` is currently a local
|
||||
notice only — it has no real downstream sink — so we
|
||||
intentionally do not mark it as ``delivered``. The lifecycle
|
||||
there is ``ready`` -> ``validated`` with the renderer's output
|
||||
captured into the persisted payload. ``delivered`` is only
|
||||
reached when the channel actually pushes the notice externally
|
||||
(today: ``email``).
|
||||
"""
|
||||
rendered = dict(payload)
|
||||
if event.channel == "station":
|
||||
rendered["station_notice"] = {
|
||||
"title": "报告已就绪",
|
||||
"body": f"订单 {event.order_id} 的志愿报告已就绪,可在当前状态页查看在线报告并下载 PDF。",
|
||||
"sent_at": sent_at,
|
||||
}
|
||||
return rendered
|
||||
if event.channel == "email":
|
||||
if self._email_sender is None:
|
||||
raise ValueError("email sender not configured")
|
||||
@@ -141,7 +177,32 @@ class DeliveryDispatcher:
|
||||
)
|
||||
rendered["email_notice"] = {
|
||||
**send_result,
|
||||
"sent_at": sent_at,
|
||||
"delivered_at": sent_at,
|
||||
}
|
||||
return rendered
|
||||
# ``station`` and any future local-render-only channel: do not
|
||||
# claim ``delivered`` because the real downstream push has not
|
||||
# happened yet. ``delivered`` is reserved for actual external
|
||||
# push completion.
|
||||
return rendered
|
||||
|
||||
@staticmethod
|
||||
def _render_delivered_payload(
|
||||
event: DeliveryNotificationEvent,
|
||||
payload: dict[str, object],
|
||||
sent_at: str,
|
||||
) -> dict[str, object]:
|
||||
rendered = dict(payload)
|
||||
if event.channel == "station":
|
||||
rendered["station_notice"] = {
|
||||
"title": "报告已就绪",
|
||||
"body": (
|
||||
f"订单 {event.order_id} 的志愿报告已就绪,"
|
||||
"可在当前状态页查看在线报告并下载 PDF。"
|
||||
),
|
||||
"delivered_at": sent_at,
|
||||
}
|
||||
# email_notice is stashed by ``_deliver_event`` directly on the
|
||||
# rendered dict before ``mark_delivered`` is called. Keep it as
|
||||
# is so consumers see the original sender response.
|
||||
return rendered
|
||||
|
||||
@@ -26,6 +26,9 @@ CREATE TABLE IF NOT EXISTS delivery_notifications (
|
||||
"""
|
||||
|
||||
|
||||
DELIVERY_EVENT_STATUSES = ("ready", "validated", "delivered", "failed", "sent")
|
||||
|
||||
|
||||
@dataclass
|
||||
class DeliveryNotificationEvent:
|
||||
order_id: str
|
||||
@@ -147,6 +150,50 @@ class DeliveryNotificationService:
|
||||
)
|
||||
self._conn.commit()
|
||||
|
||||
def mark_validated(
|
||||
self,
|
||||
order_id: str,
|
||||
event_type: str = "report_ready",
|
||||
*,
|
||||
payload_json: str | None = None,
|
||||
validated_at: str | None = None,
|
||||
) -> None:
|
||||
if validated_at is None:
|
||||
validated_at = utc_now_iso()
|
||||
if payload_json is None:
|
||||
self._conn.execute(
|
||||
"UPDATE delivery_notifications SET status='validated', last_attempt_at=? WHERE order_id=? AND event_type=?",
|
||||
(validated_at, order_id, event_type),
|
||||
)
|
||||
else:
|
||||
self._conn.execute(
|
||||
"UPDATE delivery_notifications SET status='validated', payload_json=?, last_attempt_at=? WHERE order_id=? AND event_type=?",
|
||||
(payload_json, validated_at, order_id, event_type),
|
||||
)
|
||||
self._conn.commit()
|
||||
|
||||
def mark_delivered(
|
||||
self,
|
||||
order_id: str,
|
||||
event_type: str = "report_ready",
|
||||
*,
|
||||
payload_json: str | None = None,
|
||||
sent_at: str | None = None,
|
||||
) -> None:
|
||||
if sent_at is None:
|
||||
sent_at = utc_now_iso()
|
||||
if payload_json is None:
|
||||
self._conn.execute(
|
||||
"UPDATE delivery_notifications SET status='delivered', last_attempt_at=?, failure_reason=NULL WHERE order_id=? AND event_type=?",
|
||||
(sent_at, order_id, event_type),
|
||||
)
|
||||
else:
|
||||
self._conn.execute(
|
||||
"UPDATE delivery_notifications SET status='delivered', payload_json=?, last_attempt_at=?, failure_reason=NULL WHERE order_id=? AND event_type=?",
|
||||
(payload_json, sent_at, order_id, event_type),
|
||||
)
|
||||
self._conn.commit()
|
||||
|
||||
def mark_failed(
|
||||
self,
|
||||
order_id: str,
|
||||
@@ -183,7 +230,7 @@ class DeliveryNotificationService:
|
||||
self,
|
||||
*,
|
||||
channel: str | None = None,
|
||||
statuses: tuple[str, ...] = ("ready",),
|
||||
statuses: tuple[str, ...] = ("ready", "validated"),
|
||||
limit: int = 100,
|
||||
) -> list[DeliveryNotificationEvent]:
|
||||
placeholders = ",".join("?" for _ in statuses)
|
||||
|
||||
@@ -69,6 +69,8 @@
|
||||
- ✅ P1-5 退款域模型闭环已修复(payment.status 一步收敛到 `refunded`,并把订单推进到 refunded 终态;幂等请求会自愈)
|
||||
- ✅ P1-2 删除/匿名化已覆盖 orders 主表 + payments.callback_payload + order_intakes.payload_json
|
||||
- ✅ P1-7 验证链口径统一:dev-verify / CI 统一调用 scripts/check_coverage_gate.py,并以 80% / 100% 与 codecov 对齐
|
||||
- ✅ P2-1 公共下单孤儿订单:已验证有完整回归测试覆盖(admin/tests/test_web_public.py 9 passed)
|
||||
- ✅ P2-3 delivery sent 语义修正:dispatcher 现在区分 validated / delivered,station 只到 validated,email 才到 delivered
|
||||
|
||||
顺序:
|
||||
|
||||
|
||||
@@ -10,6 +10,9 @@ from data.orders.intake_store import IntakeStore
|
||||
from data.orders.models import Order
|
||||
from data.payments.service import PaymentService
|
||||
|
||||
from data.notifications.dispatcher import DeliveryDispatcher
|
||||
|
||||
|
||||
|
||||
PROJECT_ROOT = Path(__file__).resolve().parents[1]
|
||||
|
||||
@@ -69,7 +72,9 @@ def _attach_ready_delivery(settings, tmp_path: Path, order_id: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
def test_dispatch_ready_station_event_marks_sent(settings, tmp_path):
|
||||
def test_dispatch_ready_station_event_validates_persisted_payload(
|
||||
settings, tmp_path
|
||||
):
|
||||
order = _seed_order(settings.orders_db_path)
|
||||
_mark_paid(settings, order)
|
||||
IntakeStore.for_db(settings.orders_db_path).save(
|
||||
@@ -86,22 +91,65 @@ def test_dispatch_ready_station_event_marks_sent(settings, tmp_path):
|
||||
dispatcher.close()
|
||||
|
||||
assert result.processed == 1
|
||||
assert result.sent == 1
|
||||
# station channel is local render only; ``delivered`` must be 0.
|
||||
assert result.delivered == 0
|
||||
assert result.validated == 1
|
||||
assert result.failed == 0
|
||||
assert not hasattr(result, "sent")
|
||||
|
||||
notification_service = DeliveryNotificationService.for_db(settings.orders_db_path)
|
||||
try:
|
||||
event = notification_service.list_events(order.id)[0]
|
||||
finally:
|
||||
notification_service.close()
|
||||
assert event.status == "sent"
|
||||
assert event.status == "validated"
|
||||
assert event.attempt_count == 1
|
||||
payload = json.loads(event.payload_json)
|
||||
# The portal page consumes ``station_notice`` from the persisted
|
||||
# payload even though ``station`` does not transition to
|
||||
# ``delivered``; the dispatcher stashes it during ``mark_validated``.
|
||||
station_notice = payload.get("station_notice")
|
||||
assert station_notice is not None
|
||||
assert station_notice["title"] == "报告已就绪"
|
||||
assert order.id in station_notice["body"]
|
||||
assert station_notice["sent_at"] == event.last_attempt_at
|
||||
|
||||
def test_dispatch_station_marks_validated_then_delivered(settings, tmp_path):
|
||||
"""P2-3 lock: ``station`` channel cannot be reported as ``delivered``.
|
||||
|
||||
The lifecycle is now ``ready`` -> ``validated``. ``delivered`` is
|
||||
reserved for channels that actually push the notice externally
|
||||
(today: ``email``). This test re-asserts the new chain so future
|
||||
regressions that flatten the lifecycle back to a single ``sent``
|
||||
step will fail.
|
||||
"""
|
||||
order = _seed_order(
|
||||
settings.orders_db_path,
|
||||
order_id="GKO-20260614-DISPATCH-STATION-DELIVERED",
|
||||
)
|
||||
_mark_paid(settings, order)
|
||||
IntakeStore.for_db(settings.orders_db_path).save(
|
||||
order_id=order.id, payload={"candidate_score": 578}, submit=True
|
||||
)
|
||||
_attach_ready_delivery(settings, tmp_path, order.id)
|
||||
|
||||
dispatcher = DeliveryDispatcher.for_db(settings.orders_db_path)
|
||||
try:
|
||||
result = dispatcher.dispatch_ready_events(channel="station")
|
||||
finally:
|
||||
dispatcher.close()
|
||||
|
||||
assert result.processed == 1
|
||||
assert result.validated == 1
|
||||
assert result.delivered == 0
|
||||
assert result.failed == 0
|
||||
assert not hasattr(result, "sent")
|
||||
|
||||
notification_service = DeliveryNotificationService.for_db(settings.orders_db_path)
|
||||
try:
|
||||
event = notification_service.list_events(order.id)[0]
|
||||
finally:
|
||||
notification_service.close()
|
||||
assert event.status == "validated"
|
||||
|
||||
|
||||
class _FakeEmailSender:
|
||||
@@ -146,7 +194,7 @@ def test_dispatch_ready_email_event_marks_sent(settings, tmp_path):
|
||||
dispatcher.close()
|
||||
|
||||
assert result.processed == 1
|
||||
assert result.sent == 1
|
||||
assert result.delivered == 1
|
||||
assert result.failed == 0
|
||||
assert len(email_sender.sent) == 1
|
||||
assert email_sender.sent[0]["recipient"] == "parent@example.com"
|
||||
@@ -156,7 +204,7 @@ def test_dispatch_ready_email_event_marks_sent(settings, tmp_path):
|
||||
event = notification_service.list_events(order.id, channel="email")[0]
|
||||
finally:
|
||||
notification_service.close()
|
||||
assert event.status == "sent"
|
||||
assert event.status == "delivered"
|
||||
payload = json.loads(event.payload_json)
|
||||
email_notice = payload.get("email_notice")
|
||||
assert email_notice is not None
|
||||
@@ -186,8 +234,9 @@ def test_dispatch_ready_station_event_marks_failed_when_pdf_missing(settings, tm
|
||||
dispatcher.close()
|
||||
|
||||
assert result.processed == 1
|
||||
assert result.sent == 0
|
||||
assert result.failed == 1
|
||||
assert result.validated == 0
|
||||
assert result.delivered == 0
|
||||
|
||||
notification_service = DeliveryNotificationService.for_db(settings.orders_db_path)
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user