feat(poll): reorder options without invalidating answers
This commit is contained in:
@@ -11,6 +11,7 @@ from govoplan_core.core.poll import (
|
|||||||
PollCreateCommand,
|
PollCreateCommand,
|
||||||
PollInvitationCommand,
|
PollInvitationCommand,
|
||||||
PollInvitationRef,
|
PollInvitationRef,
|
||||||
|
PollOptionOrderCommand,
|
||||||
PollOptionRequest,
|
PollOptionRequest,
|
||||||
PollOptionRef,
|
PollOptionRef,
|
||||||
PollOptionUpdateCommand,
|
PollOptionUpdateCommand,
|
||||||
@@ -70,6 +71,7 @@ from govoplan_poll.backend.service import (
|
|||||||
poll_result_summary_by_id,
|
poll_result_summary_by_id,
|
||||||
response_datetime,
|
response_datetime,
|
||||||
remove_poll_option,
|
remove_poll_option,
|
||||||
|
reorder_poll_options,
|
||||||
revoke_poll_invitation_with_replay,
|
revoke_poll_invitation_with_replay,
|
||||||
set_poll_workflow_context,
|
set_poll_workflow_context,
|
||||||
submit_poll_response,
|
submit_poll_response,
|
||||||
@@ -82,7 +84,13 @@ def _poll_ref(poll: object) -> PollRef:
|
|||||||
return PollRef(
|
return PollRef(
|
||||||
id=poll.id,
|
id=poll.id,
|
||||||
status=poll.status,
|
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
|
raise PollCapabilityError(str(exc)) from exc
|
||||||
return PollOptionRef(id=option.id, position=option.position)
|
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:
|
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)
|
return self._transition(open_poll, session, tenant_id=tenant_id, poll_id=poll_id)
|
||||||
|
|
||||||
|
|||||||
@@ -121,6 +121,7 @@ manifest = ModuleManifest(
|
|||||||
optional_capabilities=(CAPABILITY_AUTH_PRINCIPAL_RESOLVER, CAPABILITY_AUTH_PERMISSION_EVALUATOR),
|
optional_capabilities=(CAPABILITY_AUTH_PRINCIPAL_RESOLVER, CAPABILITY_AUTH_PERMISSION_EVALUATOR),
|
||||||
provides_interfaces=(
|
provides_interfaces=(
|
||||||
ModuleInterfaceProvider(name="poll.option_selection", version=MODULE_VERSION),
|
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.availability_matrix", version=MODULE_VERSION),
|
||||||
ModuleInterfaceProvider(name="poll.response_collection", version=MODULE_VERSION),
|
ModuleInterfaceProvider(name="poll.response_collection", version=MODULE_VERSION),
|
||||||
ModuleInterfaceProvider(name="poll.workflow_context", version=MODULE_VERSION),
|
ModuleInterfaceProvider(name="poll.workflow_context", version=MODULE_VERSION),
|
||||||
|
|||||||
@@ -1382,6 +1382,61 @@ def update_poll_option(
|
|||||||
return 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]:
|
def _locked_active_poll_responses(session: Session, *, poll: Poll) -> list[PollResponse]:
|
||||||
return (
|
return (
|
||||||
session.query(PollResponse)
|
session.query(PollResponse)
|
||||||
|
|||||||
@@ -5,7 +5,12 @@ import unittest
|
|||||||
from sqlalchemy import create_engine
|
from sqlalchemy import create_engine
|
||||||
from sqlalchemy.orm import Session, sessionmaker
|
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_core.db.base import Base
|
||||||
from govoplan_poll.backend.capabilities import SqlPollSchedulingProvider
|
from govoplan_poll.backend.capabilities import SqlPollSchedulingProvider
|
||||||
from govoplan_poll.backend.db.models import Poll, PollOption, PollResponse
|
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(poll.options[0].label, "Monday")
|
||||||
self.assertEqual(len(response.answers), 2)
|
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__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|||||||
Reference in New Issue
Block a user