Let a saved policy be repointed to a v2 policy (PATCH /v2/user-policies policy_id)

changelog.d/apiv2-1e-user-policy-repoint.added.md

@@ -0,0 +1 @@
+PATCH /v2/user-policies/{id} accepts policy_id, so a saved policy can be pointed at another v2 policy without changing its ID. Supplied alone, policy_id clears the v1 reference (legacy_policy_id); supplied with legacy_policy_id, both are stored, and policy_id must be the v2 copy of that v1 policy when the Stage 10 mirror made one. Unknown, other-country and conflicting policies are refused as on creation.

docs/migration/stage-12a-user-login.md

@@ -183,6 +183,11 @@ Concurrency on Postgres:
     `legacy_policy_mappings` / `legacy_household_mappings` when a mapping
     exists.
   - Changing the legacy ID re-resolves the v2 reference, or clears it.
+  - `PATCH /v2/user-policies` also accepts `policy_id`, which must name a v2
+    policy in the row's country. Supplied alone, it points the row at that
+    policy and clears `legacy_policy_id`, so an edited reform keeps its saved
+    policy ID. Supplied with `legacy_policy_id`, both are stored, and
+    `policy_id` must be the mirror copy of that v1 policy when one exists.
   - `user-simulations` and `user-reports` accept only legacy IDs in 12A.
 - **Stage 10 mirror rows.** A `user_policies` row with a
   `legacy_user_policy_mappings` entry returns 409 on PATCH and DELETE. The

policyengine_api/fastapi_routes/v2/user_policies/routes.py

@@ -159,8 +159,11 @@ def read() -> UserPolicyPageResponse:
         summary="Update or repoint one of the current user's saved policies",
         description=(
             "Changing legacy_policy_id repoints the row and re-resolves policy_id "
-            "from the Stage 10 mirror mapping, or clears it. Rows maintained by "
-            "the v1 saved-policy mirror return 409."
+            "from the Stage 10 mirror mapping, or clears it. Changing policy_id "
+            "alone points the row at that v2 policy and clears legacy_policy_id. "
+            "Supplied together, both are stored, and policy_id must be the mirror "
+            "copy of legacy_policy_id when one exists. Rows maintained by the v1 "
+            "saved-policy mirror return 409."
         ),
     )
     def patch_user_policy_route(

policyengine_api/services/v2/user_policies/database_connectors/updates.py

@@ -7,30 +7,36 @@
 from policyengine_api.data.v2.models.base import utc_now
 
 if TYPE_CHECKING:
-    from uuid import UUID
-
     from sqlmodel import Session
 
     from policyengine_api.data.v2.models import LegacyUserPolicyMapping, UserPolicy
-    from policyengine_api.services.v2.user_policies.types import UserPolicyUpdateInput
+    from policyengine_api.services.v2.user_policies.types import (
+        UserPolicyReferences,
+        UserPolicyUpdateInput,
+    )
+
+POLICY_REFERENCE_FIELDS = frozenset({"policy_id", "legacy_policy_id"})
 
 
 def update_user_policy(
     session: Session,
     association: UserPolicy,
     association_input: UserPolicyUpdateInput,
     *,
-    repointed_policy_id: UUID | None = None,
-    repoint: bool = False,
+    references: UserPolicyReferences | None = None,
 ) -> UserPolicy:
-    """Apply the supplied fields, bump ``updated_at`` and return the row.
+    """Apply the supplied presentation fields, bump ``updated_at`` and return the row.
 
-    With ``repoint``, also set ``policy_id`` to ``repointed_policy_id``.
+    With ``references``, also store both of its policy references; the service
+    resolves them, so the request's own reference fields are never copied.
     """
-    for field_name, value in association_input.model_dump(exclude_unset=True).items():
+    for field_name, value in association_input.model_dump(
+        exclude_unset=True, exclude=set(POLICY_REFERENCE_FIELDS)
+    ).items():
         setattr(association, field_name, value)
-    if repoint:
-        association.policy_id = repointed_policy_id
+    if references is not None:
+        association.policy_id = references.policy_id
+        association.legacy_policy_id = references.legacy_policy_id
     association.updated_at = utc_now()
     session.add(association)
     session.flush()

policyengine_api/services/v2/user_policies/services.py

@@ -61,6 +61,7 @@
     UserPolicyCreationInput,
     UserPolicyPage,
     UserPolicyRead,
+    UserPolicyReferences,
     UserPolicyUpdateInput,
 )
 from policyengine_api.services.v2.user_policies.validators import (
@@ -533,12 +534,16 @@ def patch_user_policy(
         """Update the supplied fields of the principal's association in this country.
 
         A new ``legacy_policy_id`` must exist in v1 and repoints ``policy_id`` to the
-        v2 copy of that policy, or to None when there is no copy. Raise
+        v2 copy of that policy, or to None when there is no copy. A new
+        ``policy_id`` must exist in this country (AssociationPolicyNotFoundError or
+        AssociationCountryConflictError). Without ``legacy_policy_id`` it clears the
+        v1 reference; with it, both are stored, and it must be the v2 copy of that
+        v1 policy when one exists (AssociationReferenceConflictError). Raise
         UserPolicyNotFoundError when the association is not the principal's, and
         MirrorOwnedAssociationError when the v1 saved-policy mirror maintains it.
         """
-        repoint = "legacy_policy_id" in association_input.model_fields_set
-        if repoint:
+        supplied = association_input.model_fields_set
+        if "legacy_policy_id" in supplied:
             if association_input.legacy_policy_id is None:
                 msg = "legacy_policy_id must not be null"
                 raise TypeError(msg)
@@ -560,22 +565,25 @@ def patch_user_policy(
                 read_user_policy_mirror_exists(session, association_id=association.id),
                 association_id=association.id,
             )
-            repointed_policy_id = (
-                read_mirrored_policy_ids(
-                    session,
-                    country_id=country_id,
-                    legacy_policy_ids=(association_input.legacy_policy_id,),
-                ).get(association_input.legacy_policy_id)
-                if repoint and association_input.legacy_policy_id is not None
+            references = (
+                UserPolicyReferences(
+                    policy_id=resolve_user_policy_reference(
+                        session,
+                        country_id=country_id,
+                        policy_id=association_input.policy_id,
+                        legacy_policy_id=association_input.legacy_policy_id,
+                    ),
+                    legacy_policy_id=association_input.legacy_policy_id,
+                )
+                if association_input.repoints
                 else None
             )
             return user_policy_read(
                 update_user_policy(
                     session,
                     association,
                     association_input,
-                    repointed_policy_id=repointed_policy_id,
-                    repoint=repoint,
+                    references=references,
                 )
             )
 

policyengine_api/services/v2/user_policies/types.py

@@ -85,10 +85,16 @@ def require_policy_reference(self) -> UserPolicyCreationInput:
 class UserPolicyUpdateInput(StrictAssociationInput):
     """A partial update of a saved policy; only supplied fields are changed.
 
-    At least one field must be supplied. ``legacy_policy_id`` may point the
-    association at another v1 policy but cannot be cleared.
+    At least one field must be supplied. ``policy_id`` points the association at
+    another v2 policy; supplied without ``legacy_policy_id``, it also clears the
+    v1 reference, which no longer describes the saved policy. ``legacy_policy_id``
+    alone points the association at another v1 policy and takes ``policy_id`` from
+    that policy's v2 copy. Supplied together, both are stored, and ``policy_id``
+    must be that v2 copy when the Stage 10 mirror made one. Neither reference can
+    be set to null.
     """
 
+    policy_id: ResourceId | None = None
     legacy_policy_id: LegacyRecordId | None = None
     name: AssociationName | None = None
     description: AssociationDescription | None = None
@@ -97,21 +103,44 @@ class UserPolicyUpdateInput(StrictAssociationInput):
 
     @model_validator(mode="after")
     def require_supplied_field(self) -> UserPolicyUpdateInput:
-        """Require a supplied field, and reject an explicit null legacy_policy_id."""
+        """Require a supplied field, and reject an explicit null policy reference."""
         supplied = self.model_fields_set.intersection(
-            {"legacy_policy_id", "name", "description", "baseline", "provenance"}
+            {
+                "policy_id",
+                "legacy_policy_id",
+                "name",
+                "description",
+                "baseline",
+                "provenance",
+            }
         )
         if not supplied:
             msg = (
-                "At least one of legacy_policy_id, name, description, baseline, "
-                "or provenance must be supplied"
+                "At least one of policy_id, legacy_policy_id, name, description, "
+                "baseline, or provenance must be supplied"
             )
             raise ValueError(msg)
-        if "legacy_policy_id" in supplied and self.legacy_policy_id is None:
-            msg = "legacy_policy_id must not be null"
-            raise ValueError(msg)
+        for reference in ("policy_id", "legacy_policy_id"):
+            if reference in supplied and getattr(self, reference) is None:
+                msg = f"{reference} must not be null"
+                raise ValueError(msg)
         return self
 
+    @property
+    def repoints(self) -> bool:
+        """Whether the update changes which policy the association references."""
+        return bool(
+            self.model_fields_set.intersection({"policy_id", "legacy_policy_id"})
+        )
+
+
+@dataclass(frozen=True)
+class UserPolicyReferences:
+    """The v2 and v1 policy references a repointed association stores."""
+
+    policy_id: UUID | None
+    legacy_policy_id: int | None
+
 
 class LegacyUserPolicySnapshot(StrictAssociationInput):
     """Detached complete committed v1 saved-policy row."""

tests/unit/v2/test_user_policy_routes.py

@@ -248,7 +248,7 @@ def test_patch_repoints_legacy_reference_and_rejects_invalid_changes() -> None:
             json=body,
         )
         for body in (
-            {"policy_id": str(POLICY_ID)},
+            {"policy_id": None},
             {"user_id": str(OWNER_ID)},
             {"client_ref": "rf-2"},
             {"legacy_policy_id": None},
@@ -265,6 +265,21 @@ def test_patch_repoints_legacy_reference_and_rejects_invalid_changes() -> None:
     assert flask_calls["count"] == 0
 
 
+def test_patch_repoints_to_a_v2_policy_reference() -> None:
+    client, service, flask_calls = _client()
+
+    repointed = client.patch(
+        f"/v2/user-policies/{ASSOCIATION_ID}?country_id=us",
+        json={"policy_id": str(POLICY_ID)},
+    )
+
+    assert repointed.status_code == 200
+    association_input = service.calls[0][1]["association_input"]
+    assert association_input.policy_id == POLICY_ID
+    assert association_input.model_fields_set == {"policy_id"}
+    assert flask_calls["count"] == 0
+
+
 def test_delete_returns_no_content_for_the_session_user() -> None:
     client, service, _flask_calls = _client()
 
@@ -381,6 +396,7 @@ def test_openapi_publishes_signed_in_association_contracts() -> None:
     patch_schema = schema["components"]["schemas"][patch_ref.rsplit("/", 1)[-1]]
     assert patch_schema["additionalProperties"] is False
     assert set(patch_schema["properties"]) == {
+        "policy_id",
         "legacy_policy_id",
         "name",
         "description",

tests/unit/v2/test_user_policy_service.py

@@ -249,6 +249,104 @@ def test_repointing_re_resolves_or_clears_the_v2_reference(store) -> None:
     assert legacy_references.calls[-1] == ("us", {"policy": [404]})
 
 
+def test_repointing_to_a_v2_policy_replaces_the_v1_reference(store) -> None:
+    service, selected, legacy_references = store
+    created = service.create_user_policy(principal=ALICE, association_input=_fields())
+    v1_checks_before_repoint = list(legacy_references.calls)
+
+    repointed = service.patch_user_policy(
+        principal=ALICE,
+        country_id="us",
+        association_id=created.id,
+        association_input=UserPolicyUpdateInput(policy_id=selected.policy_id),
+    )
+
+    assert (repointed.policy_id, repointed.legacy_policy_id) == (
+        selected.policy_id,
+        None,
+    )
+    assert repointed.name == "Saved reform"
+    assert legacy_references.calls == v1_checks_before_repoint
+    with selected.sessions() as session:
+        stored = session.get(UserPolicy, created.id)
+        assert stored is not None
+        assert (stored.policy_id, stored.legacy_policy_id) == (
+            selected.policy_id,
+            None,
+        )
+
+
+def test_repointing_to_both_references_keeps_both_when_they_agree(store) -> None:
+    service, selected, _legacy_references = store
+    created = service.create_user_policy(principal=ALICE, association_input=_fields())
+
+    repointed = service.patch_user_policy(
+        principal=ALICE,
+        country_id="us",
+        association_id=created.id,
+        association_input=UserPolicyUpdateInput(
+            policy_id=selected.policy_id,
+            legacy_policy_id=MAPPED_LEGACY_POLICY_ID,
+        ),
+    )
+
+    assert (repointed.policy_id, repointed.legacy_policy_id) == (
+        selected.policy_id,
+        MAPPED_LEGACY_POLICY_ID,
+    )
+
+
+def test_repointing_to_a_missing_foreign_or_conflicting_policy_changes_nothing(
+    store,
+) -> None:
+    service, selected, _legacy_references = store
+    created = service.create_user_policy(principal=ALICE, association_input=_fields())
+    missing_policy_id = UUID("00000000-0000-0000-0000-000000000099")
+
+    with pytest.raises(AssociationPolicyNotFoundError):
+        service.patch_user_policy(
+            principal=ALICE,
+            country_id="us",
+            association_id=created.id,
+            association_input=UserPolicyUpdateInput(policy_id=missing_policy_id),
+        )
+    with pytest.raises(AssociationCountryConflictError):
+        service.patch_user_policy(
+            principal=ALICE,
+            country_id="us",
+            association_id=created.id,
+            association_input=UserPolicyUpdateInput(policy_id=selected.uk_policy_id),
+        )
+    with selected.sessions.begin() as session:
+        mapped_policy = session.get(Policy, selected.policy_id)
+        assert mapped_policy is not None
+        other_us_policy = Policy(
+            country_id="us",
+            tax_benefit_model_id=mapped_policy.tax_benefit_model_id,
+            tax_benefit_model_version_id=mapped_policy.tax_benefit_model_version_id,
+            canonicalization_version=1,
+            content_hash="e" * 64,
+        )
+        session.add(other_us_policy)
+        session.flush()
+        other_us_policy_id = other_us_policy.id
+    with pytest.raises(AssociationReferenceConflictError):
+        service.patch_user_policy(
+            principal=ALICE,
+            country_id="us",
+            association_id=created.id,
+            association_input=UserPolicyUpdateInput(
+                policy_id=other_us_policy_id,
+                legacy_policy_id=MAPPED_LEGACY_POLICY_ID,
+            ),
+        )
+
+    unchanged = service.get_user_policy(
+        principal=ALICE, country_id="us", association_id=created.id
+    )
+    assert (unchanged.policy_id, unchanged.legacy_policy_id) == (None, 1)
+
+
 def test_rows_owned_by_the_stage_10_mirror_conflict(store) -> None:
     service, selected, _legacy_references = store
     created = service.create_user_policy(principal=ALICE, association_input=_fields())
@@ -331,7 +429,18 @@ def test_patch_input_requires_a_supported_field() -> None:
         UserPolicyUpdateInput()
     with pytest.raises(ValueError):
         UserPolicyUpdateInput.model_validate(
-            {"policy_id": "00000000-0000-0000-0000-000000000001"}
+            {"user_id": "00000000-0000-0000-0000-0000000000a1"}
         )
     with pytest.raises(ValueError):
         UserPolicyUpdateInput(legacy_policy_id=None)
+    with pytest.raises(ValueError):
+        UserPolicyUpdateInput(policy_id=None)
+
+
+def test_patch_input_accepts_a_v2_policy_reference() -> None:
+    repoint = UserPolicyUpdateInput.model_validate(
+        {"policy_id": "00000000-0000-0000-0000-000000000001"}
+    )
+
+    assert repoint.policy_id == UUID("00000000-0000-0000-0000-000000000001")
+    assert repoint.model_fields_set == {"policy_id"}