Skip to content

Commit

Permalink
(PC-31217)[API]feat: changes after review, merged validation over pro…
Browse files Browse the repository at this point in the history
…duct and ean code, commit will be merged with previous one after review
  • Loading branch information
pcharlet-pass committed Sep 3, 2024
1 parent 03d3553 commit 5f2d29a
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 46 deletions.
4 changes: 1 addition & 3 deletions api/src/pcapi/core/offers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,7 @@ def create_draft_offer(
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)

validation.check_offer_product_ean(body, venue.venueTypeCode)

validation.check_product_exists(body.product_id)
validation.check_offer_product(body, venue.venueTypeCode)

fields = {key: value for key, value in body.dict(by_alias=True).items() if key != "venueId"}
fields.update(_get_accessibility_compliance_fields(venue))
Expand Down
4 changes: 0 additions & 4 deletions api/src/pcapi/core/offers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,6 @@ class EanCodeMissing(OfferCreationBaseException):
pass


class ProductNotFoundForOffer(OfferCreationBaseException):
pass


class FutureOfferException(OfferCreationBaseException):
pass

Expand Down
46 changes: 17 additions & 29 deletions api/src/pcapi/core/offers/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from pcapi.domain import music_types
from pcapi.domain import show_types
from pcapi.models import api_errors
from pcapi.models import db
from pcapi.models.offer_mixin import OfferValidationStatus
from pcapi.routes.native.v1.serialization.offerers import VenueTypeCode
from pcapi.routes.public.books_stocks import serialization
Expand Down Expand Up @@ -508,47 +507,36 @@ def check_offer_subcategory_is_valid(offer_subcategory_id: str | None) -> None:
raise exceptions.SubCategoryIsInactive()


def check_offer_product_ean(
body: offers_schemas.PostDraftOfferBodyModel | offers_schemas.PatchDraftOfferBodyModel,
def check_offer_product(
body: offers_schemas.PostDraftOfferBodyModel,
venue_type_code: VenueTypeCode,
) -> None:
ean_code = body.extra_data.get("ean", None) if body.extra_data is not None else None
product_id = body.product_id if body.product_id is not None else None
if ean_code is not None or product_id is not None:
product_exists = (
models.Product.query.filter(models.Product.extraData["ean"].astext == ean_code)
.filter(models.Product.id == product_id)
.one_or_none()
)
if product_exists:
return
raise exceptions.EanCodeMissing(
ExtraDataFieldEnum.EAN.value, "EAN non reconnu. Assurez-vous qu'il n'y ait pas d'erreur de saisie."
)

if venue_type_code != VenueTypeCode.RECORD_STORE:
return

subcategory_id = body.subcategory_id if body.subcategory_id else None
subcategory_id = body.subcategory_id if body.subcategory_id is not None else None
if subcategory_id not in [
subcategories.SUPPORT_PHYSIQUE_MUSIQUE_CD.id,
subcategories.SUPPORT_PHYSIQUE_MUSIQUE_VINYLE.id,
]:
return

ean_code = body.extra_data.get("ean", None) if body.extra_data else None
if ean_code:
ean_code_exists = db.session.query(
models.Product.query.filter(models.Product.extraData["ean"].astext == ean_code).exists()
).scalar()
if ean_code_exists:
return
raise exceptions.EanCodeMissing(
ExtraDataFieldEnum.EAN.value, "EAN non reconnu. Assurez-vous qu'il n'y ait pas d'erreur de saisie."
)
raise exceptions.EanCodeMissing(ExtraDataFieldEnum.EAN.value, "Cette catégorie nécessite un EAN.")


def check_product_exists(product_id: int | None) -> None:
if product_id is None:
return
product_exists = db.session.query(models.Product.query.filter(models.Product.id == product_id).exists()).scalar()
if product_exists:
return
# Raising an error over the product_id should never exists in a classic use of this API.
# product_id is sent by frontend after requesting get_product_by_ean_code route.
# So product existance is already checked in this route and, therefor,
# product_id should always match an existing product id.
# This is why the error message, even if linked to a 400 error and usually in french, is in english.
raise exceptions.ProductNotFoundForOffer("productId", "This product does not exists.")


def check_booking_limit_datetime(
stock: educational_models.CollectiveStock | models.Stock | None,
beginning: datetime.datetime | None,
Expand Down
6 changes: 0 additions & 6 deletions api/tests/routes/pro/patch_draft_offer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ def when_trying_to_patch_forbidden_attributes(self, client):
assert key in response.json

def when_trying_to_patch_ean(self, client):
# Given
user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
venue = offerers_factories.VenueFactory(
managingOfferer=user_offerer.offerer, venueTypeCode=VenueTypeCode.RECORD_STORE
Expand All @@ -101,16 +100,13 @@ def when_trying_to_patch_ean(self, client):
description="description",
)

# When
data = {"extraData": {"ean": "1234567891234"}}
response = client.with_session_auth("[email protected]").patch(f"offers/draft/{offer.id}", json=data)

# Then
assert response.status_code == 400
assert response.json["ean"] == ["Vous ne pouvez pas changer cette information"]

def when_trying_to_patch_product(self, client):
# Given
user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
venue = offerers_factories.VenueFactory(
managingOfferer=user_offerer.offerer, venueTypeCode=VenueTypeCode.RECORD_STORE
Expand All @@ -123,11 +119,9 @@ def when_trying_to_patch_product(self, client):
)
product = offers_factories.ProductFactory(subcategoryId=subcategories.LIVRE_PAPIER.id)

# When
data = {"product_id": product.id}
response = client.with_session_auth("[email protected]").patch(f"offers/draft/{offer.id}", json=data)

# Then
assert response.status_code == 400
assert response.json["product_id"] == ["Vous ne pouvez pas changer cette information"]

Expand Down
10 changes: 6 additions & 4 deletions api/tests/routes/pro/post_draft_offer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_created_offer_should_be_inactive(self, client):
"name": "Celeste",
"subcategoryId": subcategories.LIVRE_PAPIER.id,
"venueId": venue.id,
"extraData": {"ean": "9782123456803", "gtl_id": "07000000"},
"extraData": {"gtl_id": "07000000"},
}
response = client.with_session_auth("[email protected]").post("/offers/draft", json=data)

Expand All @@ -33,14 +33,16 @@ def test_created_offer_should_be_inactive(self, client):
assert response_dict["name"] == "Celeste"
assert response_dict["id"] == offer.id
assert response_dict["productId"] == None
assert response_dict["extraData"] == {"ean": "9782123456803", "gtl_id": "07000000"}
assert response_dict["extraData"] == {"gtl_id": "07000000"}
assert not offer.product

def test_created_offer_from_product_should_return_product_id(self, client):
venue = offerers_factories.VenueFactory()
offerer = venue.managingOfferer
offerers_factories.UserOffererFactory(offerer=offerer, user__email="[email protected]")
product = offers_factories.ProductFactory(subcategoryId=subcategories.LIVRE_PAPIER.id)
product = offers_factories.ProductFactory(
subcategoryId=subcategories.LIVRE_PAPIER.id, extraData=dict({"ean": "9782123456803"})
)

data = {
"name": "Celeste",
Expand Down Expand Up @@ -309,7 +311,7 @@ def test_fail_if_product_does_not_exists(self, client):
response = client.with_session_auth("[email protected]").post("/offers/draft", json=data)

assert response.status_code == 400
assert response.json["productId"] == ["This product does not exists."]
assert response.json["ean"] == ["EAN non reconnu. Assurez-vous qu'il n'y ait pas d'erreur de saisie."]

@pytest.mark.parametrize("subcategory_id", ["OEUVRE_ART", "BON_ACHAT_INSTRUMENT"])
def test_fail_if_inactive_subcategory(self, client, subcategory_id):
Expand Down

0 comments on commit 5f2d29a

Please sign in to comment.