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-31953)[API] feat: Handle manual edition on pro side #14238

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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/offerers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Copy link
Contributor

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é 🙏 ?

Copy link
Contributor Author

@damien-ramelet damien-ramelet Sep 19, 2024

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:

    def get_single_address_result(
        self,
        address: str,
        postcode: str | None,
        city: str | None = None,
        citycode: str | None = None,
    ) -> AddressInfo:
        """
        No human interaction so we limit to 1 result, and add a filter on the INSEE code (Code Officiel Géographique)
        This will get the highest score result from the query, for a specific INSEE code.
        An incorrect result would still be in the vicinity of the expected result, and can be later edited in pc pro
        see https://forum.etalab.gouv.fr/t/interpretation-du-score/3852/4
        If no result is found, we return the centroid of the municipality
        """
        params = {
            "q": address,
            "postcode": postcode,
            "citycode": citycode,
            "autocomplete": 0,
            "limit": 1,
        }

        data = self._cached_search(params=params)
        if self._is_result_empty(data):
            logger.info(
                "No result from API Adresse for queried address",
                extra={"queried_address": address, "postcode": postcode},
            )
            if city is not None and postcode is not None:
                return self.get_municipality_centroid(city=city, postcode=postcode, citycode=citycode)
            raise NoResultException

Donc:

  • l'api adresse ne nous aura pas renvoyé de résultat, donc il fallback sur le centroïde de la commune qui ne renvoit pas de résultat non plus
  • NoResultException est raised, que l’on catch ici, pour à nouveau demander le centroïde de la commune, qui va de nouveau raised NoResultException ?

Ça n’a pas de sens

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,
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

@damien-ramelet damien-ramelet Sep 19, 2024

Choose a reason for hiding this comment

The 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)


Expand Down
1 change: 1 addition & 0 deletions api/src/pcapi/core/offerers/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,4 @@ class AddressBodyModel(BaseModel):
longitude: float | str
postalCode: VenuePostalCode
street: VenueAddress
isManualEdition: bool = False
1 change: 1 addition & 0 deletions api/src/pcapi/core/offers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ def get_offerer_address_from_address(
city=address.city,
latitude=float(address.latitude),
longitude=float(address.longitude),
is_manual_edition=address.isManualEdition,
)
return offerers_api.get_or_create_offerer_address(
venue.managingOffererId,
Expand Down
43 changes: 43 additions & 0 deletions api/tests/routes/pro/patch_offer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down
9 changes: 3 additions & 6 deletions api/tests/routes/pro/post_offer_test.py
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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pro/src/apiClient/v1/models/AddressBodyModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/* eslint-disable */
export type AddressBodyModel = {
city: string;
isManualEdition?: boolean;
label?: string | null;
latitude: (number | string);
longitude: (number | string);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ describe('test updateIndividualOffer::serializers', () => {
postalCode: '75001',
street: '3 Rue de Valois',
locationLabel: 'Bureau',
manuallySetAddress: true,
}
patchBody = {
...patchBody,
Expand All @@ -156,6 +157,7 @@ describe('test updateIndividualOffer::serializers', () => {
postalCode: '75001',
street: '3 Rue de Valois',
label: 'Bureau',
isManualEdition: true,
},
}
expect(
Expand All @@ -164,6 +166,37 @@ describe('test updateIndividualOffer::serializers', () => {
formValues,
})
).toEqual(patchBody)

// Test with empty label and manual edition false
formValues = {
...formValues,
city: 'Paris',
latitude: '48.853320',
longitude: '2.348979',
postalCode: '75001',
street: '3 Rue de Valois',
locationLabel: '',
manuallySetAddress: false,
}
patchBody = {
...patchBody,
address: {
city: 'Paris',
latitude: '48.853320',
longitude: '2.348979',
postalCode: '75001',
street: '3 Rue de Valois',
label: '',
isManualEdition: false,
},
}

expect(
serializePatchOffer({
offer: getIndividualOfferFactory(),
formValues,
})
).toEqual(patchBody)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ export const serializePatchOffer = ({
sentValues.latitude &&
sentValues.longitude &&
sentValues.postalCode &&
sentValues.street &&
sentValues.locationLabel
sentValues.street
) {
addressValues = {
address: {
Expand All @@ -94,6 +93,7 @@ export const serializePatchOffer = ({
postalCode: sentValues.postalCode,
street: sentValues.street,
label: sentValues.locationLabel,
isManualEdition: sentValues.manuallySetAddress,
},
}
}
Expand Down
Loading