server/product: fix eager load error triggered after attached custom fields validation error

When we validate custom fields, we remove all the existing fields, and create a savepoint. If one of the new fields is invalid, we rollback to the savepoint, which removes all the newly created fields. This behavior apparently invalidates all the eagerly loaded relationships, like prices, causing an error when we try to access them later in the request lifecycle.

The fix is to first make the prices validation, and do the custom fields validation at the end.

Author: frankie567

server/polar/product/service.py

@@ -278,6 +278,24 @@ async def update(
     ) -> Product:
         errors: list[ValidationError] = []
 
+        # Validate prices
+        existing_prices = set(product.prices)
+        added_prices: list[ProductPrice] = []
+        if update_schema.prices is not None:
+            (
+                _,
+                existing_prices,
+                added_prices,
+                prices_errors,
+            ) = await self._get_validated_prices(
+                session,
+                update_schema.prices,
+                product.recurring_interval,
+                product,
+                auth_subject,
+            )
+            errors.extend(prices_errors)
+
         # Prevent non-legacy products from changing their recurring interval
         if (
             update_schema.recurring_interval is not None
@@ -381,23 +399,6 @@ async def update(
                 await nested.rollback()
                 errors.extend(attached_custom_fields_errors)
 
-        existing_prices = set(product.prices)
-        added_prices: list[ProductPrice] = []
-        if update_schema.prices is not None:
-            (
-                _,
-                existing_prices,
-                added_prices,
-                prices_errors,
-            ) = await self._get_validated_prices(
-                session,
-                update_schema.prices,
-                product.recurring_interval,
-                product,
-                auth_subject,
-            )
-            errors.extend(prices_errors)
-
         if errors:
             raise PolarRequestValidationError(errors)
 

server/tests/product/test_endpoints.py

@@ -13,9 +13,12 @@
     ProductPriceFixed,
     UserOrganization,
 )
+from polar.models.custom_field import CustomFieldType
 from polar.postgres import AsyncSession
 from tests.fixtures.database import SaveFixture
 from tests.fixtures.random_objects import (
+    create_custom_field,
+    create_product,
     set_product_benefits,
 )
 
@@ -426,6 +429,44 @@ async def test_existing_price_with_full_schema(
         assert isinstance(product_price, ProductPriceFixed)
         assert price["price_amount"] == product_price.price_amount
 
+    @pytest.mark.auth
+    async def test_invalid_attached_custom_fields(
+        self,
+        save_fixture: SaveFixture,
+        session: AsyncSession,
+        client: AsyncClient,
+        organization: Organization,
+        user_organization: UserOrganization,
+    ) -> None:
+        custom_field = await create_custom_field(
+            save_fixture,
+            type=CustomFieldType.text,
+            slug="test-field",
+            organization=organization,
+        )
+        product = await create_product(
+            save_fixture,
+            organization=organization,
+            recurring_interval=None,
+            attached_custom_fields=[
+                (custom_field, True),
+            ],
+        )
+
+        response = await client.patch(
+            f"/v1/products/{product.id}",
+            json={
+                "attached_custom_fields": [
+                    {
+                        "custom_field_id": str(uuid.uuid4()),
+                        "required": False,
+                    }
+                ]
+            },
+        )
+
+        assert response.status_code == 422
+
 
 @pytest.mark.asyncio
 class TestUpdateProductBenefits: