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

feat: add commune deleguee to voie #960

Closed
wants to merge 5 commits into from

Conversation

fufeck
Copy link
Contributor

@fufeck fufeck commented Oct 9, 2024

CONTEXT

Il y a des duplica de numéro dans la BAN lorsque les commune fusionne (plusieurs fois le même numeros par voie)
Il est impossible de les différencier pour le moment coté BAL car la BAN recalculer le commune_insee_deleguee avec gazzeteer
La BAN va enlever gazzeteer de leur coté.

FEAT

Un select apparait (suelement sur les communes qui on des communes déléguées) pour remplir le code commune déléguée des voies et toponymes

https://www.loom.com/share/e83017bba8bc47039007a83eca7cd1e1?sid=1786db41-7331-40dc-995a-4026a0ac145d

PR

@fufeck fufeck requested a review from MaGOs92 October 21, 2024 14:02
components/bal/toponyme-editor.tsx Outdated Show resolved Hide resolved
components/bal/voie-editor.tsx Outdated Show resolved Hide resolved
components/bal/voie-editor.tsx Outdated Show resolved Hide resolved
>
{withOptionNull && <option key="null" value={null}></option>}
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 bizarre de pouvoir sélectionner l'option vide? Je vois pas trop l'intéret à partir du moment où il y'a des communes déléguées, la voie et le toponyme doivent forcément pouvoir matcher avec une d'entre elles non?
Capture d’écran 2024-11-12 à 10 27 35

Nit : aussi pourquoi a-t-on besoin d'un select dédié aux communes, j'ai l'impression qu'un générique pourrait faire le taff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'aime bien l'idée de laisser le champ optionnel, c'est pour cela que il y a un un select vide
Ensuite le était deja un component existant. Si on veut le rendre générique on est obligé de toucher les components qui l'utilisaient deja et ca me dérangeait un peu.

A Discuter

Copy link
Contributor

Choose a reason for hiding this comment

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

Le problème de laisser le champ optionnel c'est qu'en cas de dé-fusion on ne pourra pas savoir à quelle commune déléguée appartenait la voie non? @j-saur t'as un avis là dessus?

lib/geo-api/index.ts Outdated Show resolved Hide resolved
lib/openapi/services/AdminService.ts Outdated Show resolved Hide resolved
@fufeck fufeck requested a review from MaGOs92 November 28, 2024 14:10
Copy link
Contributor

@MaGOs92 MaGOs92 left a comment

Choose a reason for hiding this comment

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

Code and tests LGTM, à discuter pour les 2 commentaires

@fufeck fufeck closed this Jan 15, 2025
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.

2 participants