-
Notifications
You must be signed in to change notification settings - Fork 38
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-31953)[API] feat: Handle manual edition on pro side #14238
base: master
Are you sure you want to change the base?
Changes from all commits
e561d56
f53377e
022bee4
47f068f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2727,9 +2727,9 @@ def create_offerer_address_from_address_api( | |
city: str, | ||
latitude: float, | ||
longitude: float, | ||
is_manual_edition: bool, | ||
) -> geography_models.Address: | ||
is_manual_edition: bool = False | ||
try: | ||
if not is_manual_edition: | ||
address_info = api_adresse.get_address(street, postal_code, city) | ||
location_data = LocationData( | ||
city=address_info.city, | ||
|
@@ -2740,23 +2740,16 @@ def create_offerer_address_from_address_api( | |
insee_code=address_info.citycode, | ||
ban_id=address_info.id, | ||
) | ||
except api_adresse.NoResultException: | ||
insee_code = None | ||
if city and postal_code: | ||
try: | ||
insee_code = api_adresse.get_municipality_centroid(city, postal_code).citycode | ||
except api_adresse.AdresseException: | ||
pass | ||
else: | ||
location_data = LocationData( | ||
city=city, | ||
postal_code=postal_code, | ||
latitude=latitude, | ||
longitude=longitude, | ||
street=street, | ||
insee_code=insee_code, | ||
ban_id=None, | ||
insee_code=None, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. idem, même si c'est édité manuellement, je pense que l'on veut conserver autant que possible le code Insee. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. D’où viendrait-il ? Il n’y a pas de champ code insee dans le formulaire du front |
||
) | ||
is_manual_edition = True | ||
return get_or_create_address(location_data, is_manual_edition=is_manual_edition) | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,6 +132,48 @@ def test_patch_offer_with_address(self, get_address_mock, label, offer_has_oa, a | |
assert address.postalCode == "75102" | ||
assert address.latitude == Decimal("48.85660") | ||
assert address.longitude == Decimal("2.3522") | ||
assert address.isManualEdition is False | ||
|
||
def test_patch_offer_with_manual_address_edition(self, client): | ||
# Given | ||
user_offerer = offerers_factories.UserOffererFactory(user__email="[email protected]") | ||
venue = offerers_factories.VenueFactory(managingOfferer=user_offerer.offerer) | ||
offer = offers_factories.OfferFactory( | ||
subcategoryId=subcategories.ABO_MEDIATHEQUE.id, | ||
venue=venue, | ||
name="New name", | ||
description="description", | ||
) | ||
|
||
# When | ||
data = { | ||
"name": "New name", | ||
"externalTicketOfficeUrl": "http://example.net", | ||
"mentalDisabilityCompliant": True, | ||
"address": { | ||
"street": "42, Rue qui n’existe pas", | ||
"city": "Ville inconnue", | ||
"postalCode": "00000", | ||
"latitude": 1, | ||
"longitude": 1, | ||
"label": "label", | ||
"isManualEdition": 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 | ||
updated_offer = Offer.query.get(offer.id) | ||
address = updated_offer.offererAddress.address | ||
assert updated_offer.offererAddress.label == "label" | ||
assert address.street == "42, Rue qui n’existe pas" | ||
assert address.city == "Ville inconnue" | ||
assert address.postalCode == "00000" | ||
assert address.latitude == Decimal("1") | ||
assert address.longitude == Decimal("1") | ||
assert address.isManualEdition is True | ||
|
||
@pytest.mark.parametrize("label", ["", None, True]) | ||
@patch("pcapi.connectors.api_adresse.get_address") | ||
|
@@ -193,6 +235,7 @@ def test_patch_offer_with_address_twice(self, get_address_mock, label, client): | |
assert address.postalCode == "75102" | ||
assert address.latitude == Decimal("48.85660") | ||
assert address.longitude == Decimal("2.3522") | ||
assert address.isManualEdition is False | ||
|
||
def test_withdrawal_can_be_updated(self, client): | ||
offer = offers_factories.OfferFactory( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,7 @@ | ||
from decimal import Decimal | ||
from unittest.mock import patch | ||
|
||
import pytest | ||
|
||
from pcapi.connectors import api_adresse | ||
from pcapi.core.categories import subcategories_v2 as subcategories | ||
import pcapi.core.offerers.factories as offerers_factories | ||
from pcapi.core.offers.models import Offer | ||
|
@@ -210,20 +208,19 @@ def test_create_event_offer_with_manual_offerer_address(self, oa_label, client): | |
"longitude": "2.308289", | ||
"postalCode": "75001", | ||
"street": "3 Rue de Valois", | ||
"isManualEdition": True, | ||
}, | ||
} | ||
with patch("pcapi.connectors.api_adresse.get_address") as mocked_get_address: | ||
mocked_get_address.side_effect = api_adresse.NoResultException | ||
response = client.with_session_auth("[email protected]").post("/offers", json=data) | ||
response = client.with_session_auth("[email protected]").post("/offers", json=data) | ||
|
||
# Then | ||
assert response.status_code == 201 | ||
offer_id = response.json["id"] | ||
offer = Offer.query.get(offer_id) | ||
assert offer.offererAddress.address.isManualEdition | ||
assert offer.offererAddress.label == oa_label | ||
assert offer.offererAddress.address.inseeCode == "06029" | ||
assert not offer.offererAddress.address.banId | ||
assert offer.offererAddress.address.isManualEdition is True | ||
|
||
def when_creating_new_thing_offer(self, client): | ||
# Given | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ca me semble risqué d'enlever le try/except malgré tout. Est-ce que l'on peut le garder pour plus de sécurité 🙏 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je ne vois pas à quoi ça va servir, si
NoResultException
est raised par le connecteur de l’API adresse, c’est suite à ce bout de code:Donc:
NoResultException
est raised, que l’on catch ici, pour à nouveau demander le centroïde de la commune, qui va de nouveau raisedNoResultException
?Ça n’a pas de sens