From 8292c9d70941f7e22741b08bae859680c350262c Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Wed, 22 Jul 2026 03:22:04 +0200 Subject: [PATCH] feat(poll): reorder options without invalidating answers --- src/govoplan_poll/backend/capabilities.py | 35 ++++++++++++- src/govoplan_poll/backend/manifest.py | 1 + src/govoplan_poll/backend/service.py | 55 ++++++++++++++++++++ tests/test_response_editing.py | 62 ++++++++++++++++++++++- 4 files changed, 151 insertions(+), 2 deletions(-) diff --git a/src/govoplan_poll/backend/capabilities.py b/src/govoplan_poll/backend/capabilities.py index fd39e8d..d7e1bc1 100644 --- a/src/govoplan_poll/backend/capabilities.py +++ b/src/govoplan_poll/backend/capabilities.py @@ -11,6 +11,7 @@ from govoplan_core.core.poll import ( PollCreateCommand, PollInvitationCommand, PollInvitationRef, + PollOptionOrderCommand, PollOptionRequest, PollOptionRef, PollOptionUpdateCommand, @@ -70,6 +71,7 @@ from govoplan_poll.backend.service import ( poll_result_summary_by_id, response_datetime, remove_poll_option, + reorder_poll_options, revoke_poll_invitation_with_replay, set_poll_workflow_context, submit_poll_response, @@ -82,7 +84,13 @@ def _poll_ref(poll: object) -> PollRef: return PollRef( id=poll.id, status=poll.status, - options=tuple(PollOptionRef(id=option.id, position=option.position) for option in poll.options), + options=tuple( + PollOptionRef(id=option.id, position=option.position) + for option in sorted( + (option for option in poll.options if option.deleted_at is None), + key=lambda item: (item.position, item.id), + ) + ), ) @@ -610,6 +618,31 @@ class SqlPollSchedulingProvider(PollSchedulingProvider): raise PollCapabilityError(str(exc)) from exc return PollOptionRef(id=option.id, position=option.position) + def reorder_options( + self, + session: object, + *, + tenant_id: str, + poll_id: str, + command: PollOptionOrderCommand, + ) -> PollRef: + try: + mutation_owner = _stored_owner( + session, + tenant_id=tenant_id, + poll_id=poll_id, + ) + poll = reorder_poll_options( + session, + tenant_id=tenant_id, + poll_id=poll_id, + option_ids=command.option_ids, + mutation_owner=mutation_owner, + ) + except PollError as exc: + raise PollCapabilityError(str(exc)) from exc + return _poll_ref(poll) + def open_poll(self, session: object, *, tenant_id: str, poll_id: str) -> PollRef: return self._transition(open_poll, session, tenant_id=tenant_id, poll_id=poll_id) diff --git a/src/govoplan_poll/backend/manifest.py b/src/govoplan_poll/backend/manifest.py index ed6dad1..d98d25d 100644 --- a/src/govoplan_poll/backend/manifest.py +++ b/src/govoplan_poll/backend/manifest.py @@ -121,6 +121,7 @@ manifest = ModuleManifest( optional_capabilities=(CAPABILITY_AUTH_PRINCIPAL_RESOLVER, CAPABILITY_AUTH_PERMISSION_EVALUATOR), provides_interfaces=( ModuleInterfaceProvider(name="poll.option_selection", version=MODULE_VERSION), + ModuleInterfaceProvider(name="poll.option_ordering", version=MODULE_VERSION), ModuleInterfaceProvider(name="poll.availability_matrix", version=MODULE_VERSION), ModuleInterfaceProvider(name="poll.response_collection", version=MODULE_VERSION), ModuleInterfaceProvider(name="poll.workflow_context", version=MODULE_VERSION), diff --git a/src/govoplan_poll/backend/service.py b/src/govoplan_poll/backend/service.py index 8358fd3..3a738fd 100644 --- a/src/govoplan_poll/backend/service.py +++ b/src/govoplan_poll/backend/service.py @@ -1382,6 +1382,61 @@ def update_poll_option( return option +def reorder_poll_options( + session: Session, + *, + tenant_id: str, + poll_id: str, + option_ids: tuple[str, ...], + mutation_owner: PollMutationOwner | None = None, +) -> Poll: + """Atomically replace the complete active option order. + + Position is presentation state, so existing answers remain attached to + their stable option identities. The Poll row is locked first to keep the + lock order consistent with response and option mutations. + """ + + poll = _lock_poll_for_response( + session, + tenant_id=tenant_id, + poll_id=poll_id, + ) + _assert_poll_mutation_owner(poll, mutation_owner=mutation_owner) + requested_ids = tuple(option_ids) + if len(requested_ids) != len(set(requested_ids)): + raise PollError("Poll option order cannot contain duplicate options") + options = ( + session.query(PollOption) + .filter( + PollOption.tenant_id == tenant_id, + PollOption.poll_id == poll.id, + PollOption.deleted_at.is_(None), + ) + .order_by(PollOption.id.asc()) + .populate_existing() + .with_for_update() + .all() + ) + by_id = {option.id: option for option in options} + if set(requested_ids) != set(by_id): + raise PollError( + "Poll option order must contain every active option exactly once" + ) + current_ids = tuple( + option.id + for option in sorted(options, key=lambda item: (item.position, item.id)) + ) + if requested_ids == current_ids: + return poll + if poll.status not in {"draft", "open"}: + raise PollError("Only draft or open poll options can be reordered") + for position, option_id in enumerate(requested_ids): + by_id[option_id].position = position + session.flush() + return poll + + def _locked_active_poll_responses(session: Session, *, poll: Poll) -> list[PollResponse]: return ( session.query(PollResponse) diff --git a/tests/test_response_editing.py b/tests/test_response_editing.py index 2352491..3ea0c39 100644 --- a/tests/test_response_editing.py +++ b/tests/test_response_editing.py @@ -5,7 +5,12 @@ import unittest from sqlalchemy import create_engine from sqlalchemy.orm import Session, sessionmaker -from govoplan_core.core.poll import PollCapabilityError, PollOptionUpdateCommand, PollSchedulingProvider +from govoplan_core.core.poll import ( + PollCapabilityError, + PollOptionOrderCommand, + PollOptionUpdateCommand, + PollSchedulingProvider, +) from govoplan_core.db.base import Base from govoplan_poll.backend.capabilities import SqlPollSchedulingProvider from govoplan_poll.backend.db.models import Poll, PollOption, PollResponse @@ -174,6 +179,61 @@ class PollResponseEditingTests(unittest.TestCase): self.assertEqual(poll.options[0].label, "Monday") self.assertEqual(len(response.answers), 2) + def test_reorder_preserves_option_identity_and_existing_answers(self) -> None: + poll = self._poll(allow_response_update=False) + response = self._submit_both(poll, "person-1") + first_id, second_id = (option.id for option in poll.options) + original_answers = [dict(answer) for answer in response.answers] + provider = SqlPollSchedulingProvider() + + reordered = provider.reorder_options( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + command=PollOptionOrderCommand(option_ids=(second_id, first_id)), + ) + + self.assertEqual( + [(option.id, option.position) for option in reordered.options], + [(second_id, 0), (first_id, 1)], + ) + self.assertEqual(response.answers, original_answers) + self.assertIsNone(response.deleted_at) + + replayed = provider.reorder_options( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + command=PollOptionOrderCommand(option_ids=(second_id, first_id)), + ) + self.assertEqual(replayed.options, reordered.options) + self.assertEqual(response.answers, original_answers) + + def test_reorder_rejects_partial_or_duplicate_active_option_order(self) -> None: + poll = self._poll() + first_id, second_id = (option.id for option in poll.options) + provider = SqlPollSchedulingProvider() + + with self.assertRaisesRegex(PollCapabilityError, "every active option"): + provider.reorder_options( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + command=PollOptionOrderCommand(option_ids=(first_id,)), + ) + with self.assertRaisesRegex(PollCapabilityError, "duplicate"): + provider.reorder_options( + self.session, + tenant_id="tenant-1", + poll_id=poll.id, + command=PollOptionOrderCommand(option_ids=(first_id, first_id)), + ) + + self.assertEqual( + [(option.id, option.position) for option in poll.options], + [(first_id, 0), (second_id, 1)], + ) + if __name__ == "__main__": unittest.main()