(PC-31878)[API]fix: Update offer extra data allowed only if offer is …

…not product base and FF WIP_EAN_CREATION id disabled

api/src/pcapi/core/offers/exceptions.py

@@ -298,6 +298,11 @@ def __init__(self) -> None:
         super().__init__("global", "L’accessibilité de l’offre doit être définie")
 
 
+class OfferWithProductShouldNotUpdateExtraData(OfferEditionBaseException):
+    def __init__(self) -> None:
+        super().__init__("global", "Les extraData des offres avec produit ne sont pas modifialbles")
+
+
 class MoveOfferBaseException(Exception):
     pass
 

api/src/pcapi/core/offers/schemas.py

@@ -7,11 +7,9 @@
 from pcapi.core.offerers import models as offerers_models
 from pcapi.core.offerers import schemas as offerers_schemas
 from pcapi.core.offers import models as offers_models
-from pcapi.models import feature
 from pcapi.routes.serialization import BaseModel
 from pcapi.serialization.utils import to_camel
 from pcapi.validation.routes.offers import check_offer_name_length_is_valid
-from pcapi.validation.routes.offers import check_offer_product_update
 
 
 class PostDraftOfferBodyModel(BaseModel):
@@ -45,12 +43,6 @@ def validate_name(cls, name: str, values: dict) -> str:
         check_offer_name_length_is_valid(name)
         return name
 
-    @validator("extra_data", pre=True)
-    def validate_extra_data(cls, extra_data: dict[str, typing.Any]) -> dict[str, typing.Any]:
-        if feature.FeatureToggle.WIP_EAN_CREATION.is_active():
-            check_offer_product_update(extra_data)
-        return extra_data
-
     class Config:
         alias_generator = to_camel
         extra = "forbid"

api/src/pcapi/core/offers/validation.py

@@ -574,6 +574,11 @@ def check_offer_extra_data(
         if not extra_data.get(field):
             errors.add_error(field, "Ce champ est obligatoire")
 
+    try:
+        _check_offer_has_product(offer)
+    except exceptions.OfferWithProductShouldNotUpdateExtraData as e:
+        errors.add_client_error(e)
+
     try:
         ean = extra_data.get(ExtraDataFieldEnum.EAN.value)
         if ean and (not offer or (offer.extraData and ean != offer.extraData.get(ExtraDataFieldEnum.EAN.value))):
@@ -620,6 +625,11 @@ def check_ean_does_not_exist(ean: str | None, venue: offerers_models.Venue) -> N
             raise exceptions.OfferAlreadyExists("ean")
 
 
+def _check_offer_has_product(offer: models.Offer | None) -> None:
+    if FeatureToggle.WIP_EAN_CREATION.is_active() and offer and offer.product is not None:
+        raise exceptions.OfferWithProductShouldNotUpdateExtraData()
+
+
 def check_product_cgu_and_offerer(
     product: models.Product | None, ean: str, offerer: offerers_models.Offerer | None
 ) -> None:

api/src/pcapi/routes/pro/offers.py

@@ -423,8 +423,9 @@ def patch_offer(
     rest.check_user_has_access_to_offerer(current_user, offer.venue.managingOffererId)
     try:
         with repository.transaction():
-            body.extraData = offers_api.deserialize_extra_data(body.extraData, offer.subcategoryId)
             updates = body.dict(by_alias=True, exclude_unset=True)
+            if body_extra_data := offers_api.deserialize_extra_data(body.extraData, offer.subcategoryId):
+                updates["extraData"] = body_extra_data
 
             offer_body = offers_schemas.UpdateOffer(**updates)
 

api/src/pcapi/validation/routes/offers.py

@@ -1,5 +1,3 @@
-from typing import Any
-
 from pcapi.models.api_errors import ApiErrors
 
 
@@ -17,10 +15,3 @@ def check_collective_offer_name_length_is_valid(offer_name: str) -> None:
         api_error = ApiErrors()
         api_error.add_error("name", "Le titre de l’offre doit faire au maximum 110 caractères.")
         raise api_error
-
-
-def check_offer_product_update(extra_data: dict[str, Any]) -> None:
-    if extra_data.get("ean", False):
-        api_error = ApiErrors()
-        api_error.add_error("ean", "Vous ne pouvez pas changer cette information")
-        raise api_error

api/tests/core/offers/test_api.py

@@ -1484,29 +1484,6 @@ def test_update_offer_with_existing_ean(self):
         assert offer.name == "New name"
         assert offer.description == "new Description"
 
-    def test_raise_error_if_update_ean_for_offer_with_existing_ean(self):
-        offer = factories.OfferFactory(
-            name="Old name",
-            extraData={"ean": "1234567890123", "musicSubType": 524, "musicType": 520},
-            subcategoryId=subcategories.SUPPORT_PHYSIQUE_MUSIQUE_CD.id,
-        )
-        body = offers_schemas.UpdateOffer(
-            name="New name",
-            description="new Description",
-            extraData={"ean": "1234567890124", "gtl_id": "02000000"},
-        )
-        offer = api.update_offer(offer, body)
-        db.session.flush()
-
-        assert offer.name == "New name"
-        assert offer.description == "new Description"
-        assert offer.extraData == {
-            "ean": "1234567890124",
-            "gtl_id": "02000000",
-            "musicType": "501",
-            "musicSubType": "-1",
-        }
-
     def test_cannot_update_with_name_too_long(self):
         offer = factories.OfferFactory(name="Old name", extraData={"ean": "1234567890124"})
         body = offers_schemas.UpdateOffer(name="Luftballons" * 99)

api/tests/routes/pro/patch_draft_offer_test.py

@@ -45,36 +45,26 @@ def test_patch_draft_offer(self, client):
         assert updated_offer.description == "New description"
         assert not updated_offer.product
 
-    @override_features(WIP_EAN_CREATION=False)
-    def test_patch_draft_offer_with_ean(self, client):
+    @override_features(WIP_EAN_CREATION=True)
+    def test_patch_draft_offer_without_product_with_new_ean_should_succeed(self, client):
         user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
         venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
         offer = offers_factories.OfferFactory(
             name="Name",
             subcategoryId=subcategories.LIVRE_PAPIER.id,
             venue=venue,
             description="description",
-            url="http://example.com/offer",
+            extraData={"ean": "1111111111111"},
         )
 
-        data = {
-            "name": "New name",
-            "description": "New description",
-            "subcategoryId": subcategories.LIVRE_PAPIER.id,
-            "extraData": {"gtl_id": "07000000", "ean": "1234567891234"},
-        }
-        response = client.with_session_auth("[email protected]").patch(f"/offers/draft/{offer.id}", json=data)
+        data = {"extraData": {"ean": "2222222222222"}}
+        response = client.with_session_auth("[email protected]").patch(f"offers/draft/{offer.id}", json=data)
+
         assert response.status_code == 200
         assert response.json["id"] == offer.id
-        assert response.json["venue"]["id"] == offer.venue.id
-        assert response.json["productId"] == None
 
         updated_offer = Offer.query.get(offer.id)
-        assert updated_offer.name == "New name"
-        assert updated_offer.subcategoryId == subcategories.LIVRE_PAPIER.id
-        assert updated_offer.description == "New description"
-        assert updated_offer.extraData["ean"] == "1234567891234"
-        assert not updated_offer.product
+        assert updated_offer.extraData["ean"] == "2222222222222"
 
     @override_features(WIP_EAN_CREATION=True)
     def test_patch_draft_offer_without_product(self, client):
@@ -177,6 +167,29 @@ def test_patch_draft_offer_with_empty_extra_data(self, client):
             "visa": "",
         }
 
+    @override_features(WIP_EAN_CREATION=False)
+    def test_patch_draft_offer_with_product_with_same_extra_data_should_succeed(self, client):
+        user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
+        venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
+        product = offers_factories.ProductFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id, extraData={"gtl_id": "07000000", "ean": "1111111111111"}
+        )
+        offer = offers_factories.OfferFactory(
+            name="Name",
+            subcategoryId=subcategories.LIVRE_PAPIER.id,
+            venue=venue,
+            description="description",
+            url="http://example.com/offer",
+            extraData={"author": "1111111111111"},
+            product=product,
+        )
+
+        data = {
+            "extraData": {"author": "1111111111112"},
+        }
+        response = client.with_session_auth("[email protected]").patch(f"/offers/draft/{offer.id}", json=data)
+        assert response.status_code == 200
+
     def test_patch_draft_offer_with_existing_extra_data_with_new_extra_data(self, client):
         user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
         venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
@@ -315,23 +328,28 @@ def when_trying_to_patch_forbidden_attributes(self, client):
             assert key in response.json
 
     @override_features(WIP_EAN_CREATION=True)
-    def when_trying_to_patch_ean(self, client):
+    def when_trying_to_patch_offer_with_product_with_new_ean(self, client):
         user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
         venue = offerers_factories.VenueFactory(
             managingOfferer=user_offerer.offerer, venueTypeCode=VenueTypeCode.RECORD_STORE
         )
+        product = offers_factories.ProductFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id, extraData={"ean": "1111111111111"}
+        )
         offer = offers_factories.OfferFactory(
             name="Name",
             subcategoryId=subcategories.LIVRE_PAPIER.id,
             venue=venue,
             description="description",
+            extraData={"ean": "1111111111111"},
+            product=product,
         )
 
-        data = {"extraData": {"ean": "1234567891234"}}
+        data = {"extraData": {"ean": "2222222222222"}}
         response = client.with_session_auth("[email protected]").patch(f"offers/draft/{offer.id}", json=data)
 
         assert response.status_code == 400
-        assert response.json["ean"] == ["Vous ne pouvez pas changer cette information"]
+        assert response.json["global"] == ["Les extraData des offres avec produit ne sont pas modifialbles"]
 
     def when_trying_to_patch_product(self, client):
         user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")

api/tests/routes/pro/patch_offer_test.py

@@ -15,6 +15,7 @@
 from pcapi.core.offers.models import WithdrawalTypeEnum
 import pcapi.core.providers.factories as providers_factories
 from pcapi.core.providers.repository import get_provider_by_local_class
+from pcapi.core.testing import override_features
 import pcapi.core.users.factories as users_factories
 from pcapi.utils.date import format_into_utc_date
 
@@ -55,6 +56,58 @@ def test_patch_offer(self, client):
         assert updated_offer.subcategoryId == subcategories.ABO_PLATEFORME_VIDEO.id
         assert not updated_offer.product
 
+    def test_patch_offer_with_extra_data_should_not_remove_extra_data(self, client):
+        user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
+        venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
+        offer = offers_factories.OfferFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id,
+            venue=venue,
+            extraData={"gtl_id": "01010101", "author": "Kewis Larol"},
+        )
+
+        data = {
+            "name": "New name",
+            "mentalDisabilityCompliant": True,
+        }
+        response = client.with_session_auth("[email protected]").patch(f"/offers/{offer.id}", json=data)
+
+        assert response.status_code == 200
+        assert response.json["id"] == offer.id
+        assert response.json["venue"]["id"] == offer.venue.id
+
+        updated_offer = Offer.query.get(offer.id)
+        assert updated_offer.extraData["gtl_id"] == "01010101"
+        assert updated_offer.extraData["author"] == "Kewis Larol"
+        assert updated_offer.mentalDisabilityCompliant
+        assert updated_offer.subcategoryId == subcategories.LIVRE_PAPIER.id
+        assert not updated_offer.product
+
+    @override_features(WIP_EAN_CREATION=True)
+    def test_patch_offer_with_product_with_same_ean(self, client):
+        user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
+        venue = offerers_factories.VirtualVenueFactory(managingOfferer=user_offerer.offerer)
+        product = offers_factories.ProductFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id, extraData={"ean": "1111111111111"}
+        )
+        offer = offers_factories.OfferFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id,
+            venue=venue,
+            name="New name",
+            url="[email protected]",
+            description="description",
+            extraData={"ean": "1111111111111"},
+            product=product,
+        )
+
+        data = {"extraData": {"ean": "1111111111111"}}
+        response = client.with_session_auth("[email protected]").patch(f"/offers/{offer.id}", json=data)
+
+        assert response.status_code == 200
+        assert response.json["id"] == offer.id
+
+        updated_offer = Offer.query.get(offer.id)
+        assert updated_offer.extraData == {"ean": "1111111111111"}
+
     def test_patch_offer_with_provider_extra_data(self, client):
         user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
         venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
@@ -579,6 +632,29 @@ def test_booking_contact_is_checked_when_changed(self, client):
             "Une offre qui a un ticket retirable doit avoir l'email du contact de réservation"
         ]
 
+    @override_features(WIP_EAN_CREATION=True)
+    def should_fail_when_trying_to_update_offer_with_product_with_new_ean(self, client):
+        user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
+        venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
+        product = offers_factories.ProductFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id, extraData={"ean": "1111111111111"}
+        )
+        offer = offers_factories.OfferFactory(
+            subcategoryId=subcategories.LIVRE_PAPIER.id,
+            venue=venue,
+            name="New name",
+            url="[email protected]",
+            description="description",
+            extraData={"ean": "1111111111111"},
+            product=product,
+        )
+
+        data = {"extraData": {"ean": "2222222222222"}}
+        response = client.with_session_auth("[email protected]").patch(f"/offers/{offer.id}", json=data)
+
+        assert response.status_code == 400
+        assert response.json["global"] == ["Les extraData des offres avec produit ne sont pas modifialbles"]
+
 
 class Returns403Test:
     def when_user_is_not_attached_to_offerer(self, app, client):

pro/src/screens/IndividualOffer/DetailsScreen/DetailsScreen.tsx

@@ -101,16 +101,23 @@ export const DetailsScreen = ({ venues }: DetailsScreenProps): JSX.Element => {
   const onSubmit = async (formValues: DetailsFormValues): Promise<void> => {
     // Submit
     try {
-      const response = !offer
-        ? await api.postDraftOffer(serializeDetailsPostData(formValues))
-        : await api.patchDraftOffer(
-            offer.id,
-            serializeDetailsPatchData(formValues)
-          )
-
-      const receivedOfferId = response.id
-      await handleImageOnSubmit(receivedOfferId)
-      await mutate([GET_OFFER_QUERY_KEY, receivedOfferId])
+      const shouldPatchData = !(
+        isSearchByEanEnabled && Boolean(offer?.productId)
+      )
+      let receivedOfferId = offer?.id
+
+      if (shouldPatchData) {
+        const response = !offer
+          ? await api.postDraftOffer(serializeDetailsPostData(formValues))
+          : await api.patchDraftOffer(
+              offer.id,
+              serializeDetailsPatchData(formValues)
+            )
+
+        receivedOfferId = response.id
+        await handleImageOnSubmit(receivedOfferId)
+        await mutate([GET_OFFER_QUERY_KEY, receivedOfferId])
+      }
 
       // replace url to fix back button
       navigate(

pro/src/screens/IndividualOffer/DetailsScreen/__specs__/DetailsScreen.spec.tsx

@@ -462,6 +462,7 @@ describe('screens:IndividualOffer::Informations', () => {
       extraData: {
         author: 'Chuck Norris',
         gtl_id: '',
+        ean: '1234567891234',
         performer: 'Le Poing de Chuck',
         showSubType: 'PEGI 18',
         showType: 'Cinéma',

pro/src/screens/IndividualOffer/DetailsScreen/utils.ts

@@ -366,15 +366,11 @@ type PatchPayload = {
 export function serializeDetailsPatchData(
   formValues: DetailsFormValues
 ): PatchPayload {
-  // Always remove EAN from serializedExtraData.
-  const extraData = serializeExtraData(formValues)
-  delete extraData.ean
-
   return {
     name: formValues.name,
     subcategoryId: formValues.subcategoryId,
     description: formValues.description,
     durationMinutes: serializeDurationMinutes(formValues.durationMinutes ?? ''),
-    extraData,
+    extraData: serializeExtraData(formValues),
   }
 }