(PC-34361)[API] feat: add url in PostDraftOfferBodyModel

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

@@ -197,33 +197,17 @@ def _get_internal_accessibility_compliance(venue: offerers_models.Venue) -> dict
     }
 
 
-def _get_coherent_venue_with_subcategory(
-    venue: offerers_models.Venue, offer_subcategory_id: str
-) -> offerers_models.Venue:
-    # FIXME: ogeber 30.08.2024 - This wont be useful when
-    # virtual venues will be removed
-    subcategory = subcategories.ALL_SUBCATEGORIES_DICT[offer_subcategory_id]
-    if not subcategory.is_online_only and venue.isVirtual:
-        raise exceptions.OfferVenueShouldNotBeVirtual()
-    if (subcategory.is_online_only and venue.isVirtual) or (not subcategory.is_online_only and not venue.isVirtual):
-        return venue
-    # venue is physical and offer is digital : we look for virtual venue
-    virtual_venue = offerers_repository.find_virtual_venue_by_offerer_id(venue.managingOffererId)
-    if not virtual_venue:
-        raise exceptions.OffererVirtualVenueNotFound()
-    return virtual_venue
-
-
 def create_draft_offer(
     body: offers_schemas.PostDraftOfferBodyModel,
     venue: offerers_models.Venue,
     product: offers_models.Product | None = None,
     is_from_private_api: bool = True,
 ) -> models.Offer:
     validation.check_offer_subcategory_is_valid(body.subcategory_id)
+    subcategory = subcategories.ALL_SUBCATEGORIES_DICT[body.subcategory_id]
+    if feature.FeatureToggle.WIP_URL_IN_OFFER_DRAFT.is_active():
+        validation.check_url_is_coherent_with_subcategory(subcategory, body.url)
     validation.check_offer_name_does_not_contain_ean(body.name)
-    if feature.FeatureToggle.WIP_SUGGESTED_SUBCATEGORIES.is_active():
-        venue = _get_coherent_venue_with_subcategory(venue, body.subcategory_id)
 
     body.extra_data = _format_extra_data(body.subcategory_id, body.extra_data) or {}
     validation.check_offer_extra_data(body.subcategory_id, body.extra_data, venue, is_from_private_api)
@@ -235,7 +219,6 @@ def create_draft_offer(
     fields.update(_get_accessibility_compliance_fields(venue))
     fields.update({"withdrawalDetails": venue.withdrawalDetails})
 
-    subcategory = subcategories.ALL_SUBCATEGORIES_DICT.get(fields.get("subcategoryId", None))
     fields.update({"isDuo": bool(subcategory and subcategory.is_event and subcategory.can_be_duo)})
 
     offer = models.Offer(
@@ -311,6 +294,7 @@ def create_offer(
     validation.check_offer_extra_data(body.subcategory_id, body.extra_data, venue, is_from_private_api)
     subcategory = subcategories.ALL_SUBCATEGORIES_DICT[body.subcategory_id]
     validation.check_is_duo_compliance(body.is_duo, subcategory)
+    validation.check_url_is_coherent_with_subcategory(subcategory, body.url)
     validation.check_can_input_id_at_provider(provider, body.id_at_provider)
     validation.check_can_input_id_at_provider_for_this_venue(venue.id, body.id_at_provider)
     validation.check_offer_name_does_not_contain_ean(body.name)
@@ -330,7 +314,6 @@ def create_offer(
         isActive=False,
         validation=models.OfferValidationStatus.DRAFT,
     )
-    validation.check_digital_offer_fields(offer)
     repository.add_to_session(offer)
     db.session.flush()
 
@@ -434,7 +417,8 @@ def update_offer(
     for key, value in updates.items():
         setattr(offer, key, value)
     with db.session.no_autoflush:
-        validation.check_digital_offer_fields(offer)
+        validation.check_url_is_coherent_with_subcategory(offer.subcategory, offer.url)
+        validation.check_url_and_offererAddress_are_not_both_set(offer.url, offer.offererAddress)
     if offer.isFromAllocine:
         offer.fieldsUpdated = list(set(offer.fieldsUpdated) | updates_set)
     repository.add_to_session(offer)
@@ -1805,7 +1789,7 @@ def create_price_category(
     id_at_provider: str | None = None,
 ) -> models.PriceCategory:
     validation.check_stock_price(price, offer)
-    validation.check_digital_offer_fields(offer)
+
     if id_at_provider is not None:
         validation.check_can_input_id_at_provider_for_this_price_category(offer.id, id_at_provider)
 

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

@@ -20,6 +20,7 @@ class PostDraftOfferBodyModel(BaseModel):
     call_id: str | None
     venue_id: int
     description: str | None = None
+    url: HttpUrl | None = None
     extra_data: typing.Any = None
     duration_minutes: int | None = None
     product_id: int | None
@@ -37,6 +38,7 @@ class Config:
 class PatchDraftOfferBodyModel(BaseModel):
     name: str | None = None
     subcategory_id: str | None = None
+    url: HttpUrl | None = None
     description: str | None = None
     extra_data: dict[str, typing.Any] | None = None
     duration_minutes: int | None = None

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

@@ -10,6 +10,7 @@
 from PIL import UnidentifiedImageError
 from dateutil.relativedelta import relativedelta
 import flask
+from pydantic.v1 import HttpUrl
 import sqlalchemy as sqla
 
 from pcapi.core.categories import subcategories_v2 as subcategories
@@ -18,7 +19,6 @@
 import pcapi.core.educational.api.national_program as np_api
 from pcapi.core.finance import repository as finance_repository
 from pcapi.core.offerers import models as offerers_models
-from pcapi.core.offerers.repository import find_venue_by_id
 from pcapi.core.offerers.schemas import VenueTypeCode
 from pcapi.core.offers import exceptions
 from pcapi.core.offers import models
@@ -413,28 +413,31 @@ def check_offer_is_digital(offer: models.Offer) -> None:
         raise errors
 
 
-def check_digital_offer_fields(offer: models.Offer) -> None:
-    venue = offer.venue if offer.venue else find_venue_by_id(offer.venueId)
-    assert venue is not None  # helps mypy below
-    errors = api_errors.ApiErrors()
+def check_url_is_coherent_with_subcategory(subcategory: subcategories.Subcategory, url: HttpUrl | str | None) -> None:
+    if url and subcategory.is_offline_only:
+        raise api_errors.ApiErrors(
+            {"url": [f'Une offre de sous-catégorie "{subcategory.pro_label}" ne peut contenir un champ `url`']}
+        )
 
-    if offer.isDigital:
-        if offer.subcategory.is_offline_only:
-            errors.add_error(
-                "url", f"Une offre de sous-catégorie {offer.subcategory.pro_label} ne peut pas être numérique"
-            )
-            raise errors
-        if offer.offererAddress is not None:
-            errors.add_error("offererAddress", "Une offre numérique ne peut pas avoir d'adresse")
-            raise errors
-    else:
-        if offer.subcategory.is_online_only:
-            errors.add_error("url", f'Une offre de catégorie {offer.subcategory.id} doit contenir un champ "url"')
-        if offer.offererAddress is None and FeatureToggle.WIP_ENABLE_OFFER_ADDRESS.is_active():
-            errors.add_error("offererAddress", "Une offre physique doit avoir une adresse")
+    if not url and subcategory.is_online_only:
+        raise api_errors.ApiErrors(
+            {"url": [f'Une offre de catégorie "{subcategory.pro_label}" doit contenir un champ `url`']}
+        )
 
-    if errors.errors:
-        raise errors
+
+def check_url_and_offererAddress_are_not_both_set(
+    url: HttpUrl | str | None, offererAddress: offerers_models.OffererAddress | None
+) -> None:
+    """
+    An offer is either:
+        - digital -> `url` is not null or empty & `offererAddress=None`
+        - physicial -> `offererAddress` is not null and `url=None`
+    """
+    if url and offererAddress is not None:
+        raise api_errors.ApiErrors({"offererAddress": ["Une offre numérique ne peut pas avoir d'adresse"]})
+
+    if not url and offererAddress is None:
+        raise api_errors.ApiErrors({"offererAddress": ["Une offre physique doit avoir une adresse"]})
 
 
 def check_can_input_id_at_provider_for_this_price_category(

api/src/pcapi/models/feature.py

@@ -145,6 +145,7 @@ class FeatureToggle(enum.Enum):
     WIP_HEADLINE_OFFER = "Activer l'offre à la une"
     WIP_IS_OPEN_TO_PUBLIC = "Activer l'utilisation du critère 'ouvert au public' pour les synchro"
     WIP_2025_SIGN_UP = "Activer le nouveau parcours d’inscription au portail pro"
+    WIP_URL_IN_OFFER_DRAFT = "Activer la validation concernant l'url dans la création d'un brouillon d'offre"
 
     def is_active(self) -> bool:
         if flask.has_request_context():
@@ -222,6 +223,7 @@ def nameKey(self) -> str:
     FeatureToggle.WIP_OFFERER_STATS_V2,
     FeatureToggle.WIP_SUGGESTED_SUBCATEGORIES,
     FeatureToggle.WIP_UBBLE_V2,
+    FeatureToggle.WIP_URL_IN_OFFER_DRAFT,
     # Please keep alphabetic order
 )
 

api/tests/core/offers/test_api.py

@@ -1255,6 +1255,7 @@ def test_rollback_if_exception(self, mock_store_public_object, clear_tests_asset
         assert len(os.listdir(self.THUMBS_DIR)) == existing_number_of_files
 
 
+@pytest.mark.features(WIP_URL_IN_OFFER_DRAFT=True)
 @pytest.mark.usefixtures("db_session")
 class CreateDraftOfferTest:
     def test_create_draft_offer_from_scratch(self):
@@ -1291,6 +1292,7 @@ def test_create_draft_digitaloffer_with_virtual_venue(self):
         body = offers_schemas.PostDraftOfferBodyModel(
             name="La Poudre",
             subcategoryId=subcategories.PODCAST.id,
+            url="https://coucou.com",
             venueId=venue.id,
         )
         offer = api.create_draft_offer(body, venue=venue)
@@ -1307,19 +1309,6 @@ def test_create_draft_digitaloffer_with_virtual_venue(self):
         assert offer.motorDisabilityCompliant == None
         assert offer.visualDisabilityCompliant == None
 
-    @pytest.mark.features(WIP_SUGGESTED_SUBCATEGORIES=True)
-    def test_create_draft_physical_offer_on_virtual_venue_must_fail(self):
-        physical_venue = offerers_factories.VenueFactory(isVirtual=False)
-        virtual_venue = offerers_factories.VirtualVenueFactory(managingOffererId=physical_venue.managingOffererId)
-
-        body = offers_schemas.PostDraftOfferBodyModel(
-            name="La Marguerite et le Maître",
-            subcategoryId=subcategories.LIVRE_PAPIER.id,
-            venueId=virtual_venue.id,
-        )
-        with pytest.raises(exceptions.OfferVenueShouldNotBeVirtual):
-            api.create_draft_offer(body, venue=virtual_venue)
-
     def test_create_draft_offer_with_accessibility_provider(self):
         # when venue is synchronized with acceslibre, create draft offer should
         # have acceslibre accessibility informations
@@ -1392,45 +1381,6 @@ def test_cannot_create_offer_when_invalid_subcategory(self):
 
         assert error.value.errors["subcategory"] == ["La sous-catégorie de cette offre est inconnue"]
 
-    @pytest.mark.features(WIP_SUGGESTED_SUBCATEGORIES=True)
-    def test_create_offer_with_online_subcategory_must_be_on_virtual_venue(self):
-        venue = offerers_factories.VenueFactory(isVirtual=False)
-        virtual_venue = offerers_factories.VirtualVenueFactory(
-            managingOfferer=venue.managingOfferer,
-        )
-        body = offers_schemas.PostDraftOfferBodyModel(
-            name="La Poudre",
-            subcategoryId="PODCAST",
-            venueId=venue.id,
-        )
-        offer = api.create_draft_offer(body, venue=venue)
-        assert offer.venue == virtual_venue
-
-    @pytest.mark.features(WIP_SUGGESTED_SUBCATEGORIES=True)
-    def test_create_offer_with_offline_subcategory_must_be_on_physical_venue(self):
-        physical_venue = offerers_factories.VenueFactory(isVirtual=False)
-        virtual_venue = offerers_factories.VirtualVenueFactory(
-            managingOfferer=physical_venue.managingOfferer,
-        )
-        body = offers_schemas.PostDraftOfferBodyModel(
-            name="Le Maître et Marguerite",
-            subcategoryId="LIVRE_PAPIER",
-            venueId=virtual_venue.id,
-        )
-        offer = api.create_draft_offer(body, venue=physical_venue)
-        assert offer.venue == physical_venue
-
-    @pytest.mark.features(WIP_SUGGESTED_SUBCATEGORIES=True)
-    def test_create_offer_with_online_subcategory_with_no_virtual_venue_must_fail(self):
-        venue = offerers_factories.VenueFactory(isVirtual=False)
-        body = offers_schemas.PostDraftOfferBodyModel(
-            name="Dua Lipa - Physical",
-            subcategoryId="TELECHARGEMENT_MUSIQUE",
-            venueId=venue.id,
-        )
-        with pytest.raises(exceptions.OffererVirtualVenueNotFound):
-            api.create_draft_offer(body, venue=venue)
-
     @pytest.mark.features(WIP_SUGGESTED_SUBCATEGORIES=True)
     def test_create_offer_sends_complete_log_to_data(self, caplog):
         venue = offerers_factories.VenueFactory(isVirtual=False)
@@ -1641,6 +1591,37 @@ def test_cannot_create_offer_with_ean_in_name(self):
 
         assert error.value.errors["name"] == ["Le titre d'une offre ne peut contenir l'EAN"]
 
+    @pytest.mark.parametrize(
+        "url, subcategory_id, expected_error",
+        [
+            (
+                "https://coucou.com",
+                subcategories.SEANCE_CINE.id,
+                ['Une offre de sous-catégorie "Séance de cinéma" ne peut contenir un champ `url`'],
+            ),
+            (
+                None,
+                subcategories.ABO_LIVRE_NUMERIQUE.id,
+                ['Une offre de catégorie "Abonnement livres numériques" doit contenir un champ `url`'],
+            ),
+        ],
+    )
+    def test_raise_error_if_url_not_coherent_with_subcategory(self, url, subcategory_id, expected_error):
+        venue = offerers_factories.VenueFactory()
+        body = offers_schemas.CreateOffer(
+            name="An offer he can't refuse",
+            subcategoryId=subcategory_id,
+            url=url,
+            audioDisabilityCompliant=True,
+            mentalDisabilityCompliant=True,
+            motorDisabilityCompliant=True,
+            visualDisabilityCompliant=True,
+        )
+        with pytest.raises(api_errors.ApiErrors) as error:
+            api.create_offer(body, venue=venue)
+
+        assert error.value.errors["url"] == expected_error
+
     def test_raise_error_if_extra_data_mandatory_fields_not_provided(self):
         venue = offerers_factories.VenueFactory()
         body = offers_schemas.CreateOffer(