diff --git a/README.md b/README.md index 0461651..4c225ac 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,12 @@ and, when unavailable, the policy reason. Management clients can use the `GET /poll/polls/{poll_id}/transitions` endpoint to read the durable history instead of duplicating the matrix in their UI. +Polls created through the v0.1.9 contract start in `draft` or `open`; later +states are entered through the transition engine. Older archived rows may not +contain the status from which they were archived. Their prior state cannot be +inferred safely, so unarchive restores them to the least-permissive `draft` +state and marks that legacy fallback explicitly in the audit metadata. + ## Development Install ```bash diff --git a/src/govoplan_poll/backend/schemas.py b/src/govoplan_poll/backend/schemas.py index 04b8c9f..693b7e7 100644 --- a/src/govoplan_poll/backend/schemas.py +++ b/src/govoplan_poll/backend/schemas.py @@ -8,6 +8,7 @@ from pydantic import BaseModel, ConfigDict, Field, model_validator PollKind = Literal["single_choice", "multiple_choice", "yes_no", "yes_no_maybe", "ranked_choice", "availability"] PollStatus = Literal["draft", "open", "closed", "decided", "archived"] +PollInitialStatus = Literal["draft", "open"] PollTransitionAction = Literal["open", "draft", "close", "decide", "archive", "unarchive"] PollVisibility = Literal["private", "tenant", "public", "unlisted"] PollResultVisibility = Literal["organizer", "after_response", "after_close", "public"] @@ -31,7 +32,7 @@ class PollCreateRequest(BaseModel): slug: str | None = Field(default=None, max_length=120) description: str | None = None kind: PollKind = "single_choice" - status: PollStatus = "draft" + status: PollInitialStatus = "draft" visibility: PollVisibility = "private" result_visibility: PollResultVisibility = "after_close" context_module: str | None = Field(default=None, max_length=100) diff --git a/src/govoplan_poll/backend/service.py b/src/govoplan_poll/backend/service.py index a126434..fe12c66 100644 --- a/src/govoplan_poll/backend/service.py +++ b/src/govoplan_poll/backend/service.py @@ -19,7 +19,7 @@ from govoplan_poll.backend.schemas import ( PollSubmitResponseRequest, PollUpdateRequest, ) -from govoplan_poll.backend.transitions import DEFAULT_POLL_TRANSITION_ENGINE, POLL_STATUSES +from govoplan_poll.backend.transitions import DEFAULT_POLL_TRANSITION_ENGINE, PollTransitionEngine class PollError(ValueError): @@ -27,6 +27,7 @@ class PollError(ValueError): POLL_KINDS = {"single_choice", "multiple_choice", "yes_no", "yes_no_maybe", "ranked_choice", "availability"} +POLL_INITIAL_STATUSES = {"draft", "open"} CHOICE_POLL_KINDS = {"single_choice", "multiple_choice", "yes_no", "yes_no_maybe", "ranked_choice"} AVAILABILITY_VALUES = {"available", "maybe", "unavailable"} YES_NO_OPTIONS = ( @@ -149,8 +150,8 @@ def _validate_choice_bounds(kind: str, min_choices: int, max_choices: int | None def _ensure_valid_poll_payload(payload: PollCreateRequest) -> tuple[list[PollOptionInput], int, int | None]: if payload.kind not in POLL_KINDS: raise PollError(f"Unsupported poll kind: {payload.kind}") - if payload.status not in POLL_STATUSES: - raise PollError(f"Unsupported poll status: {payload.status}") + if payload.status not in POLL_INITIAL_STATUSES: + raise PollError("Poll initial status must be draft or open") if payload.opens_at is not None and payload.closes_at is not None and payload.closes_at <= payload.opens_at: raise PollError("closes_at must be after opens_at") options = _normalize_options(payload.kind, payload.options) @@ -305,7 +306,9 @@ def poll_results_are_visible( if poll.result_visibility == "public": return True if poll.result_visibility == "after_close": - if poll.status in {"closed", "decided", "archived"}: + if poll.status in {"closed", "decided"}: + return True + if poll.status == "archived" and poll.archived_from_status in {"closed", "decided"}: return True return poll.closes_at is not None and response_datetime(poll.closes_at) <= _now() if poll.result_visibility == "after_response" and ids: @@ -431,6 +434,7 @@ def transition_poll( idempotency_key: str | None = None, actor_user_id: str | None = None, actor_api_key_id: str | None = None, + transition_engine: PollTransitionEngine = DEFAULT_POLL_TRANSITION_ENGINE, ) -> PollTransitionResult: """Apply one policy-controlled transition and append its durable audit record.""" @@ -454,7 +458,7 @@ def transition_poll( return PollTransitionResult(poll=poll, transition=existing, replayed=True) try: - plan = DEFAULT_POLL_TRANSITION_ENGINE.plan( + plan = transition_engine.plan( current_status=poll.status, action=action, archived_from_status=poll.archived_from_status, @@ -464,6 +468,14 @@ def transition_poll( previous_decision_option_id = poll.decided_option_id now = _now() + used_legacy_unarchive_fallback = action == "unarchive" and poll.archived_from_status is None + cleared_expired_closes_at: datetime | None = None + if action == "open" and poll.closes_at is not None: + normalized_closes_at = response_datetime(poll.closes_at) + if normalized_closes_at is not None and normalized_closes_at <= now: + cleared_expired_closes_at = normalized_closes_at + poll.closes_at = None + if action == "archive": poll.archived_from_status = plan.from_status elif action == "unarchive": @@ -483,6 +495,13 @@ def transition_poll( transition_metadata["archived_from_status"] = plan.from_status elif action == "unarchive": transition_metadata["restored_status"] = plan.to_status + if used_legacy_unarchive_fallback: + transition_metadata["legacy_restore_fallback"] = { + "reason": "missing_archived_from_status", + "restored_status": plan.to_status, + } + if cleared_expired_closes_at is not None: + transition_metadata["cleared_expired_closes_at"] = cleared_expired_closes_at.isoformat() transition = PollLifecycleTransition( tenant_id=tenant_id, poll_id=poll.id, @@ -913,8 +932,12 @@ def poll_lifecycle_transition_response(transition: PollLifecycleTransition) -> d } -def poll_response(poll: Poll) -> dict[str, Any]: - lifecycle_actions = DEFAULT_POLL_TRANSITION_ENGINE.available_actions( +def poll_response( + poll: Poll, + *, + transition_engine: PollTransitionEngine = DEFAULT_POLL_TRANSITION_ENGINE, +) -> dict[str, Any]: + lifecycle_actions = transition_engine.available_actions( current_status=poll.status, archived_from_status=poll.archived_from_status, ) diff --git a/src/govoplan_poll/backend/transitions.py b/src/govoplan_poll/backend/transitions.py index 88d6002..f56c8f1 100644 --- a/src/govoplan_poll/backend/transitions.py +++ b/src/govoplan_poll/backend/transitions.py @@ -22,6 +22,7 @@ class PollTransitionRule: source_statuses: frozenset[str] target_status: str | None restore_archived_status: bool = False + missing_restore_target_status: str | None = None @dataclass(frozen=True) @@ -56,8 +57,12 @@ class PollTransitionPolicy: if rule.restore_archived_status: if rule.target_status is not None: raise ValueError(f"Restoring transition {action!r} cannot have a fixed target") + if rule.missing_restore_target_status not in (POLL_STATUSES - {"archived"}) | {None}: + raise ValueError(f"Restoring transition {action!r} has an invalid missing-origin target") elif rule.target_status not in POLL_STATUSES: raise ValueError(f"Poll transition rule {action!r} has an unknown target status") + elif rule.missing_restore_target_status is not None: + raise ValueError(f"Non-restoring transition {action!r} cannot have a missing-origin target") self._rules = MappingProxyType(normalized) @property @@ -97,6 +102,7 @@ DEFAULT_POLL_TRANSITION_POLICY = PollTransitionPolicy( source_statuses=frozenset({"archived"}), target_status=None, restore_archived_status=True, + missing_restore_target_status="draft", ), } ) @@ -125,9 +131,12 @@ class PollTransitionEngine: target_status = rule.target_status if rule.restore_archived_status: - if archived_from_status not in POLL_STATUSES - {"archived"}: + if archived_from_status is None and rule.missing_restore_target_status is not None: + target_status = rule.missing_restore_target_status + elif archived_from_status not in POLL_STATUSES - {"archived"}: raise ValueError("Archived poll has no valid status to restore") - target_status = archived_from_status + else: + target_status = archived_from_status assert target_status is not None return PollTransitionPlan(action=action, from_status=current_status, to_status=target_status) @@ -140,7 +149,11 @@ class PollTransitionEngine: actions: list[PollTransitionAvailability] = [] for action in POLL_TRANSITION_ACTIONS: rule = self.policy.rules[action] - target_status = archived_from_status if rule.restore_archived_status else rule.target_status + target_status = ( + archived_from_status or rule.missing_restore_target_status + if rule.restore_archived_status + else rule.target_status + ) try: self.plan( current_status=current_status, diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 4df739c..5149d1b 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -1,9 +1,11 @@ from __future__ import annotations import unittest +from datetime import datetime, timedelta, timezone from types import SimpleNamespace from fastapi import HTTPException +from pydantic import ValidationError from sqlalchemy import create_engine from sqlalchemy.orm import Session, sessionmaker @@ -29,6 +31,8 @@ from govoplan_poll.backend.service import ( PollError, create_poll, list_poll_lifecycle_transitions, + poll_response, + poll_results_are_visible, response_datetime, submit_poll_response, transition_poll, @@ -131,7 +135,7 @@ class PollLifecycleTests(unittest.TestCase): with self.assertRaisesRegex(ValueError, "not allowed"): engine.plan(current_status="draft", action="close") - def test_policy_is_injectable_without_changing_the_engine(self) -> None: + def test_injected_policy_governs_service_transition_and_projection(self) -> None: rules = dict(DEFAULT_POLL_TRANSITION_POLICY.rules) rules["close"] = PollTransitionRule( action="close", @@ -139,10 +143,38 @@ class PollLifecycleTests(unittest.TestCase): target_status="closed", ) custom_engine = PollTransitionEngine(PollTransitionPolicy(rules)) + poll = self._poll() - plan = custom_engine.plan(current_status="draft", action="close") + projected = poll_response(poll, transition_engine=custom_engine) + close_action = next(item for item in projected["lifecycle_actions"] if item["action"] == "close") + result = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="close", + transition_engine=custom_engine, + ) - self.assertEqual(plan.to_status, "closed") + self.assertTrue(close_action["available"]) + self.assertEqual(result.poll.status, "closed") + self.assertEqual(result.transition.from_status, "draft") + + def test_only_draft_or_open_are_valid_initial_states(self) -> None: + for initial_status in ("closed", "decided", "archived"): + with self.subTest(initial_status=initial_status): + with self.assertRaises(ValidationError): + PollCreateRequest(title="Invalid initial state", kind="yes_no", status=initial_status) + + bypassed_schema = PollCreateRequest(title="Invalid initial state", kind="yes_no").model_copy( + update={"status": "closed"} + ) + with self.assertRaisesRegex(PollError, "initial status must be draft or open"): + create_poll( + self.session, + tenant_id="tenant-1", + user_id="owner", + payload=bypassed_schema, + ) def test_reopening_preserves_responses_close_history_and_previous_decision(self) -> None: poll = self._poll() @@ -180,7 +212,9 @@ class PollLifecycleTests(unittest.TestCase): self.assertEqual([item.action for item in history], ["open", "close", "decide", "open"]) def test_direct_redecision_is_audited_and_exact_retry_is_idempotent(self) -> None: - poll = self._poll(status="closed") + poll = self._poll() + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="close") first = transition_poll( self.session, tenant_id="tenant-1", @@ -213,7 +247,12 @@ class PollLifecycleTests(unittest.TestCase): self.assertEqual(second.transition.previous_decision_option_id, first.transition.decision_option_id) self.assertEqual(poll.decided_option_id, second.transition.decision_option_id) self.assertEqual( - self.session.query(PollLifecycleTransition).filter(PollLifecycleTransition.poll_id == poll.id).count(), + self.session.query(PollLifecycleTransition) + .filter( + PollLifecycleTransition.poll_id == poll.id, + PollLifecycleTransition.action == "decide", + ) + .count(), 2, ) @@ -235,8 +274,9 @@ class PollLifecycleTests(unittest.TestCase): transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") def test_archive_and_unarchive_restore_status_and_append_audit_history(self) -> None: - poll = self._poll(status="closed") - poll.closed_at = poll.created_at + poll = self._poll() + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="close") response = PollResponse( tenant_id="tenant-1", poll_id=poll.id, @@ -267,13 +307,119 @@ class PollLifecycleTests(unittest.TestCase): self.assertEqual(poll.status, "closed") self.assertIsNone(poll.archived_from_status) self.assertIsNone(response.deleted_at) - self.assertEqual([item.action for item in poll.lifecycle_transitions], ["archive", "unarchive"]) + self.assertEqual( + [item.action for item in poll.lifecycle_transitions], + ["open", "close", "archive", "unarchive"], + ) - def test_legacy_archive_without_restore_status_is_not_silently_unarchived(self) -> None: - poll = self._poll(status="archived") + def test_legacy_archive_without_origin_restores_to_draft_with_audit_marker(self) -> None: + poll = Poll( + tenant_id="tenant-1", + slug="legacy-archive", + title="Legacy archive", + kind="yes_no", + status="archived", + visibility="tenant", + result_visibility="after_close", + workflow_steps=[], + allow_anonymous=False, + allow_response_update=True, + min_choices=1, + ) + self.session.add(poll) + self.session.flush() - with self.assertRaisesRegex(PollError, "no valid status to restore"): - transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="unarchive") + restored = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="unarchive", + ) + + self.assertEqual(restored.poll.status, "draft") + self.assertEqual(restored.transition.to_status, "draft") + self.assertEqual( + restored.transition.metadata_["legacy_restore_fallback"], + { + "reason": "missing_archived_from_status", + "restored_status": "draft", + }, + ) + + def test_reopening_clears_only_an_expired_deadline_and_preserves_close_history(self) -> None: + poll = self._poll() + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="open") + transition_poll(self.session, tenant_id="tenant-1", poll_id=poll.id, action="close") + closed_at = poll.closed_at + expired_deadline = datetime.now(timezone.utc) - timedelta(days=1) + poll.closes_at = expired_deadline + + reopened = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="open", + ) + + self.assertEqual(reopened.poll.status, "open") + self.assertIsNone(reopened.poll.closes_at) + self.assertEqual(response_datetime(reopened.poll.closed_at), response_datetime(closed_at)) + self.assertEqual( + reopened.transition.metadata_["cleared_expired_closes_at"], + expired_deadline.isoformat(), + ) + submit_poll_response( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + payload=PollSubmitResponseRequest( + respondent_id="participant-after-reopen", + answers=[PollAnswerInput(option_key="yes")], + ), + ) + + future_deadline = datetime.now(timezone.utc) + timedelta(days=1) + future_poll = self._poll(title="Future deadline") + future_poll.closes_at = future_deadline + opened = transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=future_poll.id, + action="open", + ) + self.assertEqual(response_datetime(opened.poll.closes_at), future_deadline) + self.assertNotIn("cleared_expired_closes_at", opened.transition.metadata_) + + def test_archiving_draft_or_open_poll_does_not_grant_after_close_visibility(self) -> None: + for initial_status in ("draft", "open"): + with self.subTest(initial_status=initial_status): + poll = self._poll(status=initial_status, title=f"Archive from {initial_status}") + transition_poll( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + action="archive", + ) + + self.assertFalse( + poll_results_are_visible( + self.session, + poll=poll, + actor_ids=("participant",), + ) + ) + + closed_poll = self._poll(title="Archive from closed") + transition_poll(self.session, tenant_id="tenant-1", poll_id=closed_poll.id, action="open") + transition_poll(self.session, tenant_id="tenant-1", poll_id=closed_poll.id, action="close") + transition_poll(self.session, tenant_id="tenant-1", poll_id=closed_poll.id, action="archive") + self.assertTrue( + poll_results_are_visible( + self.session, + poll=closed_poll, + actor_ids=("participant",), + ) + ) def test_generic_api_returns_action_availability_history_and_actor(self) -> None: poll = self._poll()