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-30539)[BO] feat: make manual address edition compatible with new "Offer Address" model #12932

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

prouzet
Copy link
Contributor

@prouzet prouzet commented Jun 25, 2024

But de la pull request

Ticket Jira : https://passculture.atlassian.net/browse/PC-30539

Vérifications

  • J'ai écrit les tests nécessaires
  • 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

@prouzet prouzet force-pushed the prouzet/pc-30539-oa-manual-address-in-bo branch 2 times, most recently from d15f5c1 to e5a56e7 Compare June 26, 2024 09:40
op.create_index(
"ix_complete_unique_address",
"address",
["banId", "inseeCode", "street", "postalCode", "city", "latitude", "longitude"],
Copy link
Contributor

Choose a reason for hiding this comment

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

C'est une contrainte très forte encore plus sur des contraintes décimale. Je ne connais pas la fréquence de mise à jour de l'api adresse mais pour une petite différence ou approximation on se retrouvera avec des adresses différentes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lorsque l'adresse provient de l'API Adresse, on a désormais isManualEdition à false, et c'est l'index ix_partial_unique_address_per_street_and_insee_code qui évite tout doublon sur les seuls champs street et postalCode. Ça ne change pas.

Ce nouvel index permet d'éviter les doublons complets de tous les champs, lors de l'utilisation de l'édition manuelle d'adresse dans le backoffice, qui ne valide pas forcément les résultats de l'API Adresse, ou les ajuste volontairement (ça peut être seulement un "bis" ajouté, l'ajustement de la latitude et longitude, un nom de ville qui précise un hameau, etc.).

Si un utilisateur du backoffice a saisi manuellement une petite différence, alors on doit la conserver dans une nouvelle entrée, car c'est le principe de l'édition manuelle : enregistrer ce qui a été saisi 🙂 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Autre solution : je peux ne pas ajouter cet index, et simplement dans le cas d'une édition manuelle, rechercher par un select s'il existe une entrée identique, et la retourner. C'était surtout pour garder le fonctionnement que vous aviez implémenté côté activation avec IntegrityError pour détecter les doublons, mais peut-être est-ce trop lourd avec cet index complet ?

Copy link
Contributor

Choose a reason for hiding this comment

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

je crains surtout que coté api adresse certaines coordonnées changes de peu ( pour représenter une correction lors de la prise de mesure ou autre chose) et on se retrouve a ne pas pouvoir inserer cette adresse. Il y a déjà une exception qui gère ça. personnelement ca ne me choque pas tant que ça parce que ça restera très exceptionnel. pareil pour la correction manuelle on aura des "quasi doublon" mais pas sur toute la base

Copy link
Contributor Author

@prouzet prouzet Jun 26, 2024

Choose a reason for hiding this comment

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

Je ne comprends pas dans quel cas tu penses qu'on ne pourra pas insérer l'adresse.

Si les coordonnées changent de peu sur l'API Adresse, rien ne change dans le comportement que la squad activation a implémenté, car on aura l'IntegrityError sur votre index. Et ça ne mettra pas à jour, mais je n'ai pas changé le comportement.

@prouzet prouzet requested a review from Vencespass June 26, 2024 14:05
@prouzet prouzet force-pushed the prouzet/pc-30539-oa-manual-address-in-bo branch from e5a56e7 to 48727a8 Compare June 26, 2024 15:42
@prouzet prouzet merged commit f96219a into master Jun 26, 2024
21 of 22 checks passed
@prouzet prouzet deleted the prouzet/pc-30539-oa-manual-address-in-bo branch June 26, 2024 16:44
Copy link
Contributor

@xordoquy xordoquy left a comment

Choose a reason for hiding this comment

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

arg, je n'avais envoyé ma review.

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