Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PC-31878)[API]fix: Mise à jour des extraData d'offres sur les conditional_fields seulement #14194

Merged
merged 1 commit into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions api/src/pcapi/core/offers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,20 +251,16 @@ def create_draft_offer(


def update_draft_offer(offer: models.Offer, body: offers_schemas.PatchDraftOfferBodyModel) -> models.Offer:
aliases = set(body.dict(by_alias=True))
fields = body.dict(by_alias=True, exclude_unset=True)

_extra_data = deserialize_extra_data(fields.get("extraData", offer.extraData), offer.subcategoryId)
fields["extraData"] = _format_extra_data(offer.subcategoryId, _extra_data) or {}

updates = {key: value for key, value in fields.items() if getattr(offer, key) != value}
if not updates:
return offer

if "extraData" in updates:
extra_data = get_field(offer, updates, "extraData", aliases=aliases)
formatted_extra_data = _format_extra_data(offer.subcategoryId, body.extra_data) or {}
validation.check_offer_extra_data(
offer.subcategoryId, extra_data, offer.venue, is_from_private_api=True, offer=offer
offer.subcategoryId, formatted_extra_data, offer.venue, is_from_private_api=True, offer=offer
)

for key, value in updates.items():
Expand Down Expand Up @@ -345,9 +341,6 @@ def update_offer(
aliases = set(body.dict(by_alias=True))
fields = body.dict(by_alias=True, exclude_unset=True)

_extra_data = deserialize_extra_data(fields.get("extraData", offer.extraData), offer.subcategoryId)
fields["extraData"] = _format_extra_data(offer.subcategoryId, _extra_data) or {}

if body.address:
fields["offererAddress"] = get_offerer_address_from_address(offer.venue, body.address)
fields.pop("address", None)
Expand All @@ -372,9 +365,9 @@ def update_offer(
visual_disability_compliant=get_field(offer, updates, "visualDisabilityCompliant", aliases=aliases),
)
if "extraData" in updates:
extra_data = get_field(offer, updates, "extraData", aliases=aliases)
formatted_extra_data = _format_extra_data(offer.subcategoryId, body.extra_data) or {}
validation.check_offer_extra_data(
offer.subcategoryId, extra_data, offer.venue, is_from_private_api, offer=offer
offer.subcategoryId, formatted_extra_data, offer.venue, is_from_private_api, offer=offer
)
if "isDuo" in updates:
is_duo = get_field(offer, updates, "isDuo", aliases=aliases)
Expand Down
4 changes: 4 additions & 0 deletions api/src/pcapi/routes/pro/offers.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ def patch_draft_offer(
rest.check_user_has_access_to_offerer(current_user, offer.venue.managingOffererId)
try:
with repository.transaction():
if body_extra_data := offers_api.deserialize_extra_data(body.extra_data, offer.subcategoryId):
body.extra_data = body_extra_data
offer = offers_api.update_draft_offer(offer, body)
except (exceptions.OfferCreationBaseException, exceptions.OfferEditionBaseException) as error:
raise api_errors.ApiErrors(error.errors, status_code=400)
Expand Down Expand Up @@ -423,6 +425,8 @@ def patch_offer(
try:
with repository.transaction():
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)

Expand Down
23 changes: 0 additions & 23 deletions api/tests/core/offers/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note pour la review: test sans intérêt , aucune erreur raised et comportement nominal d'update des extra data

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)
Expand Down
122 changes: 108 additions & 14 deletions api/tests/routes/pro/patch_draft_offer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,32 @@ def test_patch_draft_offer_without_product(self, client):
assert updated_offer.description == "New description"
assert not updated_offer.product

def test_patch_draft_offer_with_form_required_fields_should_not_update_existing_extra_data(self, client):
def test_patch_draft_with_extra_data(self, client):
user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
offer = offers_factories.EventOfferFactory(
name="Film",
venue=venue,
subcategoryId=subcategories.SEANCE_CINE.id,
isDuo=False,
description="description",
extraData=None,
)

data = {
"name": "Film",
"description": "description",
"subcategoryId": subcategories.SEANCE_CINE.id,
"extraData": {"stageDirector": "Greta Gerwig"},
}
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

updated_offer = Offer.query.get(offer.id)
assert updated_offer.extraData == {"stageDirector": "Greta Gerwig"}

def test_patch_draft_offer_with_empty_extra_data(self, client):
user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
venue = offerers_factories.VirtualVenueFactory(managingOfferer=user_offerer.offerer)
ems_provider = get_provider_by_local_class("EMSStocks")
Expand All @@ -118,7 +143,7 @@ def test_patch_draft_offer_with_form_required_fields_should_not_update_existing_
lastProviderId=cinema_provider_pivot.provider.id,
isDuo=False,
description="description",
extraData={"stageDirector": "Greta Gerwig"},
extraData=None,
)

data = {
Expand All @@ -132,7 +157,7 @@ def test_patch_draft_offer_with_form_required_fields_should_not_update_existing_
"showType": "",
"showSubType": "",
"speaker": "",
"stageDirector": "Greta Gerwig",
"stageDirector": "",
"visa": "",
},
}
Expand All @@ -141,22 +166,48 @@ def test_patch_draft_offer_with_form_required_fields_should_not_update_existing_
assert response.json["id"] == offer.id

updated_offer = Offer.query.get(offer.id)
assert updated_offer.extraData == {"stageDirector": "Greta Gerwig"}
assert updated_offer.extraData == {
"author": "",
"gtl_id": "",
"performer": "",
"showType": "",
"showSubType": "",
"speaker": "",
"stageDirector": "",
"visa": "",
}

def test_patch_draft_offer_with_form_required_fields_should_not_create_extra_data(self, client):
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.VirtualVenueFactory(managingOfferer=user_offerer.offerer)
ems_provider = get_provider_by_local_class("EMSStocks")
venue_provider = providers_factories.VenueProviderFactory(provider=ems_provider, venue=venue)
cinema_provider_pivot = providers_factories.CinemaProviderPivotFactory(venue=venue_provider.venue)
venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
offer = offers_factories.EventOfferFactory(
name="Film",
venue=venue_provider.venue,
venue=venue,
subcategoryId=subcategories.SEANCE_CINE.id,
lastProviderId=cinema_provider_pivot.provider.id,
isDuo=False,
description="description",
extraData=None,
extraData={
"cast": ["Joan Baez", "Joe Cocker", "David Crosby"],
"eidr": "10.5240/ADBD-3CAA-43A0-7BF0-86E2-K",
"type": "FEATURE_FILM",
"visa": "37205",
"title": "Woodstock",
"genres": ["DOCUMENTARY", "HISTORICAL", "MUSIC"],
"credits": [
{"person": {"lastName": "Wadleigh", "firstName": "Michael"}, "position": {"name": "DIRECTOR"}}
],
"runtime": 185,
"theater": {"allocine_room_id": "W0135", "allocine_movie_id": 2634},
"backlink": "https://www.allocine.fr/film/fichefilm_gen_cfilm=2634.html",
"synopsis": "Le plus important rassemblement de la musique pop de ces vingt derni\u00e8res ann\u00e9es. Des groupes qui ont marqu\u00e9 leur \u00e9poque et une jeunesse qui a marqu\u00e9 la sienne.",
"companies": [{"name": "Wadleigh-Maurice", "activity": "Production"}],
"countries": ["USA"],
"posterUrl": "https://fr.web.img2.acsta.net/pictures/14/06/20/12/25/387023.jpg",
"allocineId": 2634,
"originalTitle": "Woodstock",
"stageDirector": "Michael Wadleigh",
"productionYear": 1970,
},
)

data = {
Expand All @@ -170,16 +221,59 @@ def test_patch_draft_offer_with_form_required_fields_should_not_create_extra_dat
"showType": "",
"showSubType": "",
"speaker": "",
"stageDirector": "",
"stageDirector": "Greta Gerwig",
"visa": "",
"cast": ["Joan Baez", "Joe Cocker", "David Crosby"],
"eidr": "10.5240/ADBD-3CAA-43A0-7BF0-86E2-K",
"type": "FEATURE_FILM",
"title": "Woodstock",
"genres": ["DOCUMENTARY", "HISTORICAL", "MUSIC"],
"credits": [
{"person": {"lastName": "Wadleigh", "firstName": "Michael"}, "position": {"name": "DIRECTOR"}}
],
"runtime": 185,
"theater": {"allocine_room_id": "W0135", "allocine_movie_id": 2634},
"backlink": "https://www.allocine.fr/film/fichefilm_gen_cfilm=2634.html",
"synopsis": "Le plus important rassemblement de la musique pop de ces vingt derni\u00e8res ann\u00e9es. Des groupes qui ont marqu\u00e9 leur \u00e9poque et une jeunesse qui a marqu\u00e9 la sienne.",
"companies": [{"name": "Wadleigh-Maurice", "activity": "Production"}],
"countries": ["USA"],
"posterUrl": "https://fr.web.img2.acsta.net/pictures/14/06/20/12/25/387023.jpg",
"allocineId": 2634,
"originalTitle": "Woodstock",
"productionYear": 1970,
},
}
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

updated_offer = Offer.query.get(offer.id)
assert updated_offer.extraData == {}
assert updated_offer.extraData == {
"cast": ["Joan Baez", "Joe Cocker", "David Crosby"],
"eidr": "10.5240/ADBD-3CAA-43A0-7BF0-86E2-K",
"type": "FEATURE_FILM",
"visa": "",
"title": "Woodstock",
"genres": ["DOCUMENTARY", "HISTORICAL", "MUSIC"],
"credits": [{"person": {"lastName": "Wadleigh", "firstName": "Michael"}, "position": {"name": "DIRECTOR"}}],
"runtime": 185,
"theater": {"allocine_room_id": "W0135", "allocine_movie_id": 2634},
"backlink": "https://www.allocine.fr/film/fichefilm_gen_cfilm=2634.html",
"synopsis": "Le plus important rassemblement de la musique pop de ces vingt derni\u00e8res ann\u00e9es. Des groupes qui ont marqu\u00e9 leur \u00e9poque et une jeunesse qui a marqu\u00e9 la sienne.",
"companies": [{"name": "Wadleigh-Maurice", "activity": "Production"}],
"countries": ["USA"],
"posterUrl": "https://fr.web.img2.acsta.net/pictures/14/06/20/12/25/387023.jpg",
"allocineId": 2634,
"originalTitle": "Woodstock",
"stageDirector": "Greta Gerwig",
"productionYear": 1970,
"author": "",
"gtl_id": "",
"performer": "",
"showType": "",
"showSubType": "",
"speaker": "",
}


@pytest.mark.usefixtures("db_session")
Expand Down
93 changes: 93 additions & 0 deletions api/tests/routes/pro/patch_offer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from pcapi.core.offers.models import Offer
from pcapi.core.offers.models import OfferValidationStatus
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
import pcapi.core.users.factories as users_factories
from pcapi.utils.date import format_into_utc_date

Expand Down Expand Up @@ -53,6 +55,97 @@ 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_provider_extra_data(self, client):
user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]")
venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer)
ems_provider = get_provider_by_local_class("EMSStocks")
venue_provider = providers_factories.VenueProviderFactory(provider=ems_provider, venue=venue)
allocine_provider = providers_factories.AllocineProviderFactory(venue=venue_provider.venue)
offer = offers_factories.OfferFactory(
name="Film",
venue=venue,
lastProvider=allocine_provider,
subcategoryId=subcategories.SEANCE_CINE.id,
isDuo=False,
description="description",
extraData={
"cast": ["Joan Baez", "Joe Cocker", "David Crosby"],
"eidr": "10.5240/ADBD-3CAA-43A0-7BF0-86E2-K",
"type": "FEATURE_FILM",
"visa": "37205",
"title": "Woodstock",
"genres": ["DOCUMENTARY", "HISTORICAL", "MUSIC"],
"credits": [
{"person": {"lastName": "Wadleigh", "firstName": "Michael"}, "position": {"name": "DIRECTOR"}}
],
"runtime": 185,
"theater": {"allocine_room_id": "W0135", "allocine_movie_id": 2634},
"backlink": "https://www.allocine.fr/film/fichefilm_gen_cfilm=2634.html",
"synopsis": "Le plus important rassemblement de la musique pop de ces vingt derni\u00e8res ann\u00e9es. Des groupes qui ont marqu\u00e9 leur \u00e9poque et une jeunesse qui a marqu\u00e9 la sienne.",
"companies": [{"name": "Wadleigh-Maurice", "activity": "Production"}],
"countries": ["USA"],
"posterUrl": "https://fr.web.img2.acsta.net/pictures/14/06/20/12/25/387023.jpg",
"allocineId": 2634,
"originalTitle": "Woodstock",
"stageDirector": "Michael Wadleigh",
"productionYear": 1970,
},
)

data = {
"name": "New name",
"externalTicketOfficeUrl": "http://example.net",
"extraData": {
"cast": ["Joan Baez", "Joe Cocker", "David Crosby"],
"eidr": "10.5240/ADBD-3CAA-43A0-7BF0-86E2-K",
"type": "FEATURE_FILM",
"visa": "37205",
"title": "Woodstock",
"genres": ["DOCUMENTARY", "HISTORICAL", "MUSIC"],
"credits": [
{"person": {"lastName": "Wadleigh", "firstName": "Michael"}, "position": {"name": "DIRECTOR"}}
],
"runtime": 185,
"theater": {"allocine_room_id": "W0135", "allocine_movie_id": 2634},
"backlink": "https://www.allocine.fr/film/fichefilm_gen_cfilm=2634.html",
"synopsis": "Le plus important rassemblement de la musique pop de ces vingt derni\u00e8res ann\u00e9es. Des groupes qui ont marqu\u00e9 leur \u00e9poque et une jeunesse qui a marqu\u00e9 la sienne.",
"companies": [{"name": "Wadleigh-Maurice", "activity": "Production"}],
"countries": ["USA"],
"posterUrl": "https://fr.web.img2.acsta.net/pictures/14/06/20/12/25/387023.jpg",
"allocineId": 2634,
"originalTitle": "Woodstock",
"stageDirector": "Michael Wadleigh",
"productionYear": 1970,
},
}
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.name == "New name"
assert updated_offer.extraData == {
"cast": ["Joan Baez", "Joe Cocker", "David Crosby"],
"eidr": "10.5240/ADBD-3CAA-43A0-7BF0-86E2-K",
"type": "FEATURE_FILM",
"visa": "37205",
"title": "Woodstock",
"genres": ["DOCUMENTARY", "HISTORICAL", "MUSIC"],
"credits": [{"person": {"lastName": "Wadleigh", "firstName": "Michael"}, "position": {"name": "DIRECTOR"}}],
"runtime": 185,
"theater": {"allocine_room_id": "W0135", "allocine_movie_id": 2634},
"backlink": "https://www.allocine.fr/film/fichefilm_gen_cfilm=2634.html",
"synopsis": "Le plus important rassemblement de la musique pop de ces vingt derni\u00e8res ann\u00e9es. Des groupes qui ont marqu\u00e9 leur \u00e9poque et une jeunesse qui a marqu\u00e9 la sienne.",
"companies": [{"name": "Wadleigh-Maurice", "activity": "Production"}],
"countries": ["USA"],
"posterUrl": "https://fr.web.img2.acsta.net/pictures/14/06/20/12/25/387023.jpg",
"allocineId": 2634,
"originalTitle": "Woodstock",
"stageDirector": "Michael Wadleigh",
"productionYear": 1970,
}

@pytest.mark.parametrize(
"label, offer_has_oa, address_update_exist",
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def test_update_extra_data_partially(self, client):
assert event_offer.extraData == {
"author": "Maurice",
"performer": "Pink Pâtisserie",
"stageDirector": "Robert",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert de ce test modifié dans cette PR et qui n'aurait pas dû l'être

}

def test_patch_all_fields(self, client):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export const DetailsScreen = ({ venues }: DetailsScreenProps): JSX.Element => {
? await api.postDraftOffer(serializeDetailsPostData(formValues))
: await api.patchDraftOffer(
offer.id,
serializeDetailsPatchData(formValues)
serializeDetailsPatchData(formValues, !!offer.lastProvider)
)

const receivedOfferId = response.id
Expand Down
Loading
Loading