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

Conversation

damien-ramelet
Copy link
Contributor

@damien-ramelet damien-ramelet commented Sep 19, 2024

But de la pull request

Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-31953

Vérifications

  • J'ai écrit les tests nécessaires
  • J'ai mis à jour le fichier des plans de tests du portail pro si nécessaire
  • J'ai mis à jour la liste des routes et des titres de pages du portail pro si j'en ai rajouté/modifié ou supprimé une.
  • J'ai relu attentivement les migrations, en particulier pour éviter les locks, et je préviens les équipes Shérif et Data
  • J'ai ajouté des screenshots pour d'éventuels changements graphiques

Copy link
Contributor

github-actions bot commented Sep 19, 2024

Visit the preview URL for this PR (updated for commit 47f068f):

https://pc-pro-testing--pr14238-pc-31953-api-crea-do-ye1r8aeh.web.app

(expires Sat, 21 Sep 2024 15:57:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1

@damien-ramelet damien-ramelet force-pushed the PC-31953-api-crea-doffre-recuperer-is-manual-edition-depuis-le-front-pour-sauvegarder-l-oa branch 5 times, most recently from b568ae5 to 5a06159 Compare September 19, 2024 15:43
@damien-ramelet damien-ramelet force-pushed the PC-31953-api-crea-doffre-recuperer-is-manual-edition-depuis-le-front-pour-sauvegarder-l-oa branch from 5a06159 to 47f068f Compare September 19, 2024 15:44
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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants