fix(campaigns): null-safe test_id serialization, correct stale comments
serialize_modification_request crashed toward a string 'None' instead of JSON null for requests whose test_id was nulled by the SET NULL cascade. Also corrects two comments that still described the debunked ordering theory instead of the actual SET NULL fix, and fixes a test that asserted post-cascade state without committing first (the ORM only picks up out-of-band DB-side SET NULL on already-loaded objects after expire_on_commit forces a re-read, matching real router behavior).
This commit is contained in:
@@ -177,6 +177,7 @@ from app.services.campaign_crud_service import (
|
||||
approve_modification_request,
|
||||
reject_modification_request,
|
||||
list_modification_requests,
|
||||
serialize_modification_request,
|
||||
)
|
||||
|
||||
|
||||
@@ -278,14 +279,18 @@ class TestModificationRequests:
|
||||
request_id = request.id
|
||||
|
||||
approve_modification_request(db, str(request_id), reviewer_id=admin_user.id)
|
||||
db.commit()
|
||||
|
||||
cts = db.query(CampaignTest).filter(CampaignTest.campaign_id == active_campaign_with_test.id).all()
|
||||
assert len(cts) == 0
|
||||
|
||||
# The request row must survive the cascade-delete of the underlying Test
|
||||
# (ondelete="CASCADE" on CampaignModificationRequest.test_id) — this locks
|
||||
# in the fix where the request's status is persisted BEFORE the test is
|
||||
# removed, not after.
|
||||
# The request row must survive deletion of the underlying Test — this
|
||||
# locks in test_id's ondelete="SET NULL" (not CASCADE), which is what
|
||||
# keeps this audit record intact after the test it referenced is gone.
|
||||
# (Commit above first: ON DELETE SET NULL is applied DB-side by the
|
||||
# cascade, and the ORM only picks that up on already-loaded objects
|
||||
# after expire_on_commit forces a fresh read — exactly what happens
|
||||
# for real in the router, which commits before serializing.)
|
||||
reloaded = (
|
||||
db.query(CampaignModificationRequest)
|
||||
.filter(CampaignModificationRequest.id == request_id)
|
||||
@@ -296,6 +301,13 @@ class TestModificationRequests:
|
||||
assert reloaded.reviewed_by == admin_user.id
|
||||
assert reloaded.reviewed_at is not None
|
||||
|
||||
# Serialization must not crash and must emit a real null, not the
|
||||
# string "None", for a request whose test_id has been nulled out.
|
||||
assert reloaded.test_id is None
|
||||
serialized = serialize_modification_request(db, reloaded)
|
||||
assert serialized["test_id"] is None
|
||||
assert serialized["test_name"] is None
|
||||
|
||||
def test_approve_already_decided_request_raises(self, db, active_campaign_with_test, admin_user):
|
||||
ct = db.query(CampaignTest).filter(CampaignTest.campaign_id == active_campaign_with_test.id).first()
|
||||
request = create_modification_request(
|
||||
|
||||
Reference in New Issue
Block a user