diff --git a/server/app/api/v1/campaigns.py b/server/app/api/v1/campaigns.py index 6749ccd..880bd5a 100644 --- a/server/app/api/v1/campaigns.py +++ b/server/app/api/v1/campaigns.py @@ -288,11 +288,11 @@ def fork_version_for_edit( session: Session = Depends(get_session), principal: ApiPrincipal = Depends(require_scope("campaign:write")), ): - """Create a new editable campaign version from a locked/validated/sent version. + """Create the campaign's next and only editable working version. - Versions that were validated, built, queued or sent are immutable audit - snapshots. This endpoint makes an explicit editable copy and makes that - new copy the campaign's current version. + A new working copy may be created only after the current version is + permanently user-locked or delivery-final. Validation and temporary user + locks must be removed in place instead of creating parallel drafts. """ payload = payload or CampaignVersionUpdateRequest() @@ -324,6 +324,8 @@ def fork_version_for_edit( campaign=CampaignResponse.model_validate(campaign), version=CampaignVersionResponse.model_validate(version), ) + except LockedCampaignVersionError as exc: + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc except CampaignPersistenceError as exc: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)) from exc diff --git a/server/app/mailer/persistence/campaigns.py b/server/app/mailer/persistence/campaigns.py index 95d54ee..73aa829 100644 --- a/server/app/mailer/persistence/campaigns.py +++ b/server/app/mailer/persistence/campaigns.py @@ -135,6 +135,12 @@ def create_campaign_version_from_json( session.add(campaign) session.flush() else: + current = session.get(CampaignVersion, campaign.current_version_id) if campaign.current_version_id else None + if current and not _version_is_audit_safe_snapshot(current): + raise CampaignPersistenceError( + f"Campaign already has active working version #{current.version_number}. " + "Continue editing or unlock that version instead of importing a parallel draft." + ) campaign.name = config.campaign.name campaign.description = config.campaign.description @@ -168,6 +174,24 @@ def _version_is_user_locked(version: CampaignVersion) -> bool: return _version_user_lock_state(version) is not None +def _version_is_audit_safe_snapshot(version: CampaignVersion) -> bool: + return _version_user_lock_state(version) == "permanent" or version.workflow_state in { + CampaignVersionWorkflowState.QUEUED.value, + CampaignVersionWorkflowState.SENDING.value, + CampaignVersionWorkflowState.COMPLETED.value, + CampaignVersionWorkflowState.CANCELLED.value, + CampaignVersionWorkflowState.ARCHIVED.value, + } + + +def _ensure_current_campaign_version(campaign: Campaign, version: CampaignVersion, *, action: str) -> None: + if campaign.current_version_id != version.id: + raise CampaignPersistenceError( + f"Historical campaign versions are read-only and cannot be used to {action}. " + "Open the current working version instead." + ) + + def _version_is_validated_and_locked(version: CampaignVersion) -> bool: validation_summary = version.validation_summary if isinstance(version.validation_summary, dict) else {} return bool(version.locked_at and validation_summary.get("ok") is True and not _version_is_user_locked(version)) @@ -203,6 +227,7 @@ def validate_campaign_version( campaign = session.get(Campaign, version.campaign_id) if not campaign or campaign.tenant_id != tenant_id: raise CampaignPersistenceError("Campaign version is not accessible for this tenant") + _ensure_current_campaign_version(campaign, version, action="validate") if _version_is_user_locked(version) or version.workflow_state in { CampaignVersionWorkflowState.QUEUED.value, CampaignVersionWorkflowState.SENDING.value, @@ -320,6 +345,7 @@ def build_campaign_version( campaign = session.get(Campaign, version.campaign_id) if not campaign or campaign.tenant_id != tenant_id: raise CampaignPersistenceError("Campaign version is not accessible for this tenant") + _ensure_current_campaign_version(campaign, version, action="build") if version.workflow_state == CampaignVersionWorkflowState.COMPLETED.value: raise CampaignPersistenceError("Sent campaign versions cannot be rebuilt") validation_summary = version.validation_summary if isinstance(version.validation_summary, dict) else {} diff --git a/server/app/mailer/persistence/versions.py b/server/app/mailer/persistence/versions.py index 0672ec0..1b97616 100644 --- a/server/app/mailer/persistence/versions.py +++ b/server/app/mailer/persistence/versions.py @@ -257,6 +257,36 @@ def is_version_locked(version: CampaignVersion) -> bool: ) +def ensure_current_working_version(campaign: Campaign, version: CampaignVersion, *, action: str = "modify") -> None: + """Require the campaign's single active working version. + + Historical versions remain reviewable, but they never become writable in + place. Continuing from immutable history must create a new working copy, + and that copy becomes the campaign's sole current version. + """ + + if campaign.current_version_id != version.id: + raise LockedCampaignVersionError( + f"Historical campaign versions are read-only and cannot be used to {action}. " + "Open the current working version instead." + ) + + +def campaign_has_active_working_version(session: Session, campaign: Campaign) -> bool: + """Return True while the campaign already has a non-final working version. + + Validation locks and temporary user locks are still the same working + version; they must be unlocked rather than forked into parallel drafts. + """ + + if not campaign.current_version_id: + return False + current = session.get(CampaignVersion, campaign.current_version_id) + if not current or current.campaign_id != campaign.id: + return False + return not is_audit_safe_version(current) + + def _apply_campaign_metadata(campaign: Campaign, raw_json: dict[str, Any]) -> None: campaign_meta = raw_json.get("campaign") if isinstance(raw_json.get("campaign"), dict) else {} if campaign_meta: @@ -279,17 +309,30 @@ def fork_campaign_version_for_edit( source_base_path: str | None = None, autosave: bool = True, ) -> CampaignVersion: - """Create a new editable working version from a locked/validated version. + """Create the next sole working version from immutable campaign history. - This preserves the audit value of the validated/sent version while allowing - users to continue editing a campaign. New content starts with the supplied - raw_json when provided, otherwise with a clone of the source version. + Validation and temporary user locks are still the active working version + and must be unlocked in place. A copy is allowed only once the current + version is permanently user-locked or delivery-final. """ source = get_campaign_version_for_tenant(session, tenant_id=tenant_id, campaign_id=campaign_id, version_id=version_id) campaign = session.get(Campaign, campaign_id) assert campaign is not None + if campaign_has_active_working_version(session, campaign): + current = session.get(CampaignVersion, campaign.current_version_id) + current_number = current.version_number if current else "current" + raise LockedCampaignVersionError( + f"Campaign already has active working version #{current_number}. " + "Unlock or continue editing that version instead of creating a parallel draft." + ) + if campaign.current_version_id and source.id != campaign.current_version_id: + raise LockedCampaignVersionError( + "Historical versions remain review-only and cannot become a new branch. " + "Create the next working copy from the campaign's current immutable version." + ) + base_json = raw_json if raw_json is not None else copy.deepcopy(source.raw_json) runtime_json = normalize_campaign_paths(base_json, source_base_path) if source_base_path else copy.deepcopy(base_json) @@ -381,6 +424,7 @@ def unlock_validated_campaign_version( version = get_campaign_version_for_tenant(session, tenant_id=tenant_id, campaign_id=campaign_id, version_id=version_id) campaign = session.get(Campaign, campaign_id) assert campaign is not None + ensure_current_working_version(campaign, version, action="unlock") if is_temporary_user_locked_version(version): raise LockedCampaignVersionError("This version has a temporary user lock. Remove that lock before unlocking validation.") @@ -440,6 +484,7 @@ def update_campaign_version( version = get_campaign_version_for_tenant(session, tenant_id=tenant_id, campaign_id=campaign_id, version_id=version_id) campaign = session.get(Campaign, campaign_id) assert campaign is not None + ensure_current_working_version(campaign, version, action="edit") if is_version_locked(version): raise LockedCampaignVersionError( @@ -511,6 +556,9 @@ def update_campaign_review_state( campaign_id=campaign_id, version_id=version_id, ) + campaign = session.get(Campaign, campaign_id) + assert campaign is not None + ensure_current_working_version(campaign, version, action="record review state for") if is_version_final_locked(version): raise LockedCampaignVersionError("Delivery has started; message review state can no longer be changed.") build_summary = version.build_summary if isinstance(version.build_summary, dict) else {} @@ -555,6 +603,9 @@ def lock_campaign_version_temporarily( campaign_id=campaign_id, version_id=version_id, ) + campaign = session.get(Campaign, campaign_id) + assert campaign is not None + ensure_current_working_version(campaign, version, action="lock") if is_version_final_locked(version): raise LockedCampaignVersionError("Delivery/final versions are permanently locked and cannot receive a temporary user lock.") if is_permanent_user_locked_version(version): @@ -587,6 +638,9 @@ def unlock_user_locked_campaign_version( campaign_id=campaign_id, version_id=version_id, ) + campaign = session.get(Campaign, campaign_id) + assert campaign is not None + ensure_current_working_version(campaign, version, action="unlock") state = campaign_version_user_lock_state(version) if state == USER_LOCK_PERMANENT: raise LockedCampaignVersionError("Permanently locked versions cannot be unlocked. Create an editable copy instead.") @@ -623,6 +677,9 @@ def permanently_lock_campaign_version( campaign_id=campaign_id, version_id=version_id, ) + campaign = session.get(Campaign, campaign_id) + assert campaign is not None + ensure_current_working_version(campaign, version, action="lock permanently") if is_version_final_locked(version): raise LockedCampaignVersionError("This version is already permanently locked by its delivery/final state.") if is_permanent_user_locked_version(version): diff --git a/server/app/mailer/sending/jobs.py b/server/app/mailer/sending/jobs.py index 0da7458..dc9111b 100644 --- a/server/app/mailer/sending/jobs.py +++ b/server/app/mailer/sending/jobs.py @@ -175,6 +175,10 @@ def _get_current_version(session: Session, campaign: Campaign, version_id: str | version = session.get(CampaignVersion, wanted) if not version or version.campaign_id != campaign.id: raise QueueingError(f"Campaign version not found or not part of campaign: {wanted}") + if campaign.current_version_id != version.id: + raise QueueingError( + "Historical campaign versions are review-only. Open the current working version before queueing or sending." + ) return version diff --git a/server/multimailer-dev.db b/server/multimailer-dev.db index 0370f63..f003e55 100644 Binary files a/server/multimailer-dev.db and b/server/multimailer-dev.db differ diff --git a/server/tests/test_api_smoke.py b/server/tests/test_api_smoke.py index de63eea..fa52e3e 100644 --- a/server/tests/test_api_smoke.py +++ b/server/tests/test_api_smoke.py @@ -221,6 +221,86 @@ class ApiSmokeTests(unittest.TestCase): ) self.assertEqual(refused_unlock.status_code, 409, refused_unlock.text) + + def test_only_one_campaign_version_is_writable(self) -> None: + headers, _ = self._login() + created = self.client.post( + "/api/v1/campaigns/new", + headers=headers, + json={"external_id": "single-working-version", "name": "Single working version"}, + ) + self.assertEqual(created.status_code, 200, created.text) + campaign_id = created.json()["campaign"]["id"] + first_version_id = created.json()["version"]["id"] + + parallel = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{first_version_id}/fork", + headers=headers, + json={}, + ) + self.assertEqual(parallel.status_code, 409, parallel.text) + self.assertIn("active working version", parallel.text) + + permanent = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{first_version_id}/lock-permanently", + headers=headers, + ) + self.assertEqual(permanent.status_code, 200, permanent.text) + + copied = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{first_version_id}/fork", + headers=headers, + json={}, + ) + self.assertEqual(copied.status_code, 200, copied.text) + second_version_id = copied.json()["version"]["id"] + self.assertNotEqual(second_version_id, first_version_id) + self.assertEqual(copied.json()["campaign"]["current_version_id"], second_version_id) + + historical_update = self.client.put( + f"/api/v1/campaigns/{campaign_id}/versions/{first_version_id}", + headers=headers, + json={"current_step": "fields"}, + ) + self.assertEqual(historical_update.status_code, 409, historical_update.text) + self.assertIn("Historical campaign versions are read-only", historical_update.text) + + second_update = self.client.put( + f"/api/v1/campaigns/{campaign_id}/versions/{second_version_id}", + headers=headers, + json={"current_step": "fields"}, + ) + self.assertEqual(second_update.status_code, 200, second_update.text) + + second_parallel = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{first_version_id}/fork", + headers=headers, + json={}, + ) + self.assertEqual(second_parallel.status_code, 409, second_parallel.text) + + second_permanent = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{second_version_id}/lock-permanently", + headers=headers, + ) + self.assertEqual(second_permanent.status_code, 200, second_permanent.text) + + historical_branch = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{first_version_id}/fork", + headers=headers, + json={}, + ) + self.assertEqual(historical_branch.status_code, 409, historical_branch.text) + self.assertIn("cannot become a new branch", historical_branch.text) + + third = self.client.post( + f"/api/v1/campaigns/{campaign_id}/versions/{second_version_id}/fork", + headers=headers, + json={}, + ) + self.assertEqual(third.status_code, 200, third.text) + self.assertEqual(third.json()["version"]["version_number"], 3) + def test_campaign_create_validate_build_and_mock_send(self) -> None: headers, _ = self._login() campaign_json = {