fix(poll): harden lifecycle policy boundaries
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user