diff --git a/src/govoplan_scheduling/backend/manifest.py b/src/govoplan_scheduling/backend/manifest.py index e0bcaa0..681a597 100644 --- a/src/govoplan_scheduling/backend/manifest.py +++ b/src/govoplan_scheduling/backend/manifest.py @@ -154,6 +154,7 @@ manifest = ModuleManifest( ModuleInterfaceProvider(name="scheduling.decision_handoff", version=MODULE_VERSION), ), requires_interfaces=( + ModuleInterfaceRequirement(name="poll.option_ordering", version_min="0.1.10", version_max_exclusive="0.2.0"), ModuleInterfaceRequirement(name="poll.availability_matrix", version_min="0.1.10", version_max_exclusive="0.2.0"), ModuleInterfaceRequirement(name="poll.response_collection", version_min="0.1.10", version_max_exclusive="0.2.0"), ModuleInterfaceRequirement(name="poll.workflow_context", version_min="0.1.10", version_max_exclusive="0.2.0"), diff --git a/src/govoplan_scheduling/backend/service.py b/src/govoplan_scheduling/backend/service.py index a6873ac..623895f 100644 --- a/src/govoplan_scheduling/backend/service.py +++ b/src/govoplan_scheduling/backend/service.py @@ -20,6 +20,7 @@ from govoplan_core.core.poll import ( PollAnswerRequest, PollCapabilityError, PollCreateCommand, + PollOptionOrderCommand, PollOptionUpdateCommand, PollOptionRequest, PollResponseRef, @@ -207,7 +208,10 @@ def _poll_status_for_request(status: str) -> str: def _active_slots(request: SchedulingRequest) -> list[SchedulingCandidateSlot]: - return [slot for slot in request.slots if slot.deleted_at is None] + return sorted( + (slot for slot in request.slots if slot.deleted_at is None), + key=lambda slot: (slot.position, slot.id), + ) def _active_participants(request: SchedulingRequest) -> list[SchedulingParticipant]: @@ -2397,35 +2401,6 @@ def _candidate_slot_changes_from_reconcile( ) -def _validate_slot_reconciliation_order( - current_slots: list[SchedulingCandidateSlot], - supplied_slots: list[SchedulingCandidateSlotReconcileInput], -) -> None: - supplied_existing_ids = [item.id for item in supplied_slots if item.id is not None] - retained_ids = { - item.id - for item in supplied_slots - if item.id is not None - } - expected_existing_ids = [ - slot.id - for slot in current_slots - if slot.id in retained_ids - ] - if supplied_existing_ids != expected_existing_ids: - raise SchedulingError( - "Existing candidate slots cannot be reordered; retain their order and append new slots" - ) - encountered_new = False - for item in supplied_slots: - if item.id is None: - encountered_new = True - elif encountered_new: - raise SchedulingError( - "New candidate slots must be appended after existing slots" - ) - - def _reconcile_scheduling_slots( session: Session, *, @@ -2449,7 +2424,6 @@ def _reconcile_scheduling_slots( ) if unknown_ids: raise SchedulingError("Unknown scheduling candidate slot") - _validate_slot_reconciliation_order(current_slots, supplied_slots) supplied_ids = { item.id @@ -2514,6 +2488,7 @@ def _reconcile_scheduling_slots( ) participation_provider = _poll_participation_provider() + created_slots: list[SchedulingCandidateSlot] = [] for item in new_inputs: changes = _candidate_slot_changes_from_reconcile(request, item) slot = SchedulingCandidateSlot( @@ -2557,6 +2532,7 @@ def _reconcile_scheduling_slots( raise SchedulingError(str(exc)) from exc slot.poll_option_id = created.id slot.position = created.position + created_slots.append(slot) for slot in removed_slots: if participation_provider is None: # Guarded above; keeps type narrowing explicit. @@ -2574,7 +2550,40 @@ def _reconcile_scheduling_slots( raise SchedulingError(str(exc)) from exc slot.deleted_at = _now() - return bool(planned_updates or new_inputs or removed_slots) + created_iter = iter(created_slots) + ordered_slots = [ + current_by_id[item.id] if item.id is not None else next(created_iter) + for item in supplied_slots + ] + if any(slot.poll_option_id is None for slot in ordered_slots): + raise SchedulingError("Scheduling slot has no backing poll option") + position_changed = any( + slot.position != position + for position, slot in enumerate(ordered_slots) + ) + try: + _poll_provider().reorder_options( + session, + tenant_id=request.tenant_id, + poll_id=request.poll_id, + command=PollOptionOrderCommand( + option_ids=tuple( + cast(str, slot.poll_option_id) + for slot in ordered_slots + ), + ), + ) + except PollCapabilityError as exc: + raise SchedulingError(str(exc)) from exc + for position, slot in enumerate(ordered_slots): + slot.position = position + + return bool( + planned_updates + or new_inputs + or removed_slots + or position_changed + ) def _normalized_participant_identity(value: str | None) -> str | None: diff --git a/tests/test_response_editing.py b/tests/test_response_editing.py index b4fb6c5..002f618 100644 --- a/tests/test_response_editing.py +++ b/tests/test_response_editing.py @@ -1180,6 +1180,103 @@ class SchedulingResponseEditingTests(unittest.TestCase): self.assertEqual(raised.exception.status_code, 409) + def test_full_edit_reorders_slots_and_poll_options_without_losing_answers(self) -> None: + request = self._request(create_participant_invitations=False) + self._submit_both(request) + first_slot, second_slot = request.slots + first_slot.tentative_hold_event_id = "calendar-hold-1" + self.session.flush() + + def reconcile_input( + slot: SchedulingCandidateSlot, + ) -> SchedulingCandidateSlotReconcileInput: + return SchedulingCandidateSlotReconcileInput( + id=slot.id, + revision=scheduling_slot_revision(slot), + label=slot.label, + description=slot.description, + start_at=slot.start_at.replace(tzinfo=timezone.utc), + end_at=slot.end_at.replace(tzinfo=timezone.utc), + timezone=slot.timezone, + location=slot.location, + metadata=slot.metadata_ or {}, + ) + + response = api_update_scheduling_request( + request.id, + SchedulingRequestUpdateRequest( + slots=[reconcile_input(second_slot), reconcile_input(first_slot)] + ), + session=self.session, + principal=self._principal( + "organizer-1", + email=None, + scopes={WRITE_SCOPE}, + ), + ) + + self.assertEqual( + [(slot.id, slot.position) for slot in response.slots], + [(second_slot.id, 0), (first_slot.id, 1)], + ) + poll_options = ( + self.session.query(PollOption) + .filter( + PollOption.poll_id == request.poll_id, + PollOption.deleted_at.is_(None), + ) + .order_by(PollOption.position.asc()) + .all() + ) + self.assertEqual( + [option.id for option in poll_options], + [second_slot.poll_option_id, first_slot.poll_option_id], + ) + current = api_get_my_scheduling_availability( + request.id, + session=self.session, + principal=self._principal( + "alice-account", + email="alice@example.test", + scopes={RESPOND_SCOPE}, + ), + ) + self.assertEqual( + {answer.slot_id: answer.value for answer in current.answers}, + {first_slot.id: "available", second_slot.id: "maybe"}, + ) + self.assertEqual(first_slot.tentative_hold_event_id, "calendar-hold-1") + + replayed = api_update_scheduling_request( + request.id, + SchedulingRequestUpdateRequest( + slots=[reconcile_input(second_slot), reconcile_input(first_slot)] + ), + session=self.session, + principal=self._principal( + "organizer-1", + email=None, + scopes={WRITE_SCOPE}, + ), + ) + self.assertEqual( + [(slot.id, slot.position) for slot in replayed.slots], + [(second_slot.id, 0), (first_slot.id, 1)], + ) + current = api_get_my_scheduling_availability( + request.id, + session=self.session, + principal=self._principal( + "alice-account", + email="alice@example.test", + scopes={RESPOND_SCOPE}, + ), + ) + self.assertEqual( + {answer.slot_id: answer.value for answer in current.answers}, + {first_slot.id: "available", second_slot.id: "maybe"}, + ) + def test_draft_edit_does_not_issue_link_for_added_participant(self) -> None: request = self._request( status="draft", diff --git a/webui/scripts/test-scheduling-page-structure.mjs b/webui/scripts/test-scheduling-page-structure.mjs index 3e30679..a0b61be 100644 --- a/webui/scripts/test-scheduling-page-structure.mjs +++ b/webui/scripts/test-scheduling-page-structure.mjs @@ -74,6 +74,8 @@ assert.match(page, /id="scheduling-participant-picker"/); assert.match(page, /id="scheduling-candidate-slots-grid"/); assert.match(page, /id="scheduling-participants-grid"/); assert.match(page, / { setSlots((items) => editorMode === "edit" ? [...items, newSlot(items.length + 1, translateText)]