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

Ajout d'une colonne contenant le nom + SIRET d'une siae #352

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

YannickPassa
Copy link
Contributor

@YannickPassa YannickPassa commented Sep 21, 2024

**Carte Notion : **

Pourquoi ?

Certaines SIAE ont le même nom alors que ce ne sont pas les mêmes structures, ajout d'une colonne Nom + ID afin de pouvoir montrer des données non agrégées sur nos TBs.

Checks

  • J'ai lancé le modèle ou seed sur un dump local (si pertinent)
  • J'ai ajouté des tests à mon code Python, ou des assertions DBT sur le modèle SQL
  • J'ai documenté ce modèle voire certains de ses champs (usage métier, tableau de bord, etc)

app_geo.nom_region as nom_region_structure,
app_geo.code_dept as code_dept_structure,
app_geo.nom_departement_complet as nom_departement_structure,
concat_ws('-', structure_denomination, structure_id_siae) as structure_denomination_unique
Copy link
Contributor

Choose a reason for hiding this comment

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

Pas sûr de bien comprendre pourquoi faire une nouvelle colonne qui se base en plus sur le nom alors que structure_id_siae (qui est l'ASP ID) me semble suffisant pour l’agrégation 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parce que ça va servir comme colonne pour filtrer sur metabase !
Par exemple y a 4 structures EUREKA avec des id différents, et vu que les utilisateurs filtrent par nom de structure ils voient les données de 4 d'un coup là

Copy link
Contributor

Choose a reason for hiding this comment

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

structure_id_siae c'est l'ASP ID et il ne me semble pas qu'il soit connu des employeurs car c'est un id technique interne donc ça va pas leur servir à grand chose si il ne peuvent pas identifier la structure qu'il visualise, enfin je suppose ?!

Pour l'histoire du filtrage, est-ce qu'ajouter un filtre sur le SIRET ne serais pas plus utile ? Pour le cas présent mais aussi dans d'autre cas.
Les employeurs travaillent souvent avec cette information et on est à peu près sûr de la qualité et du contenu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EN gros ils auront Eureka - 1234 : effectivement ça leur dira rien mais ça permettre d'identifier de façon unique les structures.
J'ai reproduit ce qui a déjà été fait côté emplois et qui perturbe pas les utilisateurs visiblement : https://github.com/gip-inclusion/les-emplois/blob/23aa6025650d5dd61f25d5ca5006cba66f0fd4c6/itou/metabase/tables/job_applications.py#L229

Sinon oui le SIRET c'est une possibilité mais je voulais eviter les noms a rallonge

Copy link
Contributor

Choose a reason for hiding this comment

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

EN gros ils auront Eureka - 1234 : effectivement ça leur dira rien mais ça permettre d'identifier de façon unique les structures.

Oui, mais comment je sais que "Eureka - 1234" c'est la SIAE de Lyon et que "Eureka - 4321" c'est la SIAE de Villeurbanne ?

Et par rapport à ce que tu as lié :

  1. to_company_id c'est l'ID C1 et non pas l'ID ASP. Avec les antennes les ID C1 sont plus discriminant que les ID ASP, il est aussi affiché dans le tableau de bord et parfois utilisé pour le support donc pas totalement obscure pour les employeurs.
    image
  2. to_company.display_name n'est pas forcément le nom du flux IAE mais possiblement celui que l'employeur décide (justement à cause de la source de donnée ;)) : https://github.com/gip-inclusion/les-emplois/blob/a4c44ff157a302d8227720cc8d4a73bafe3ef0cf/itou/companies/models.py#L308-L312

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oui, mais comment je sais que "Eureka - 1234" c'est la SIAE de Lyon et que "Eureka - 4321" c'est la SIAE de Villeurbanne ?
Ils peuvent le voir avec un tableau juste en dessous (et merci d'avoir utilisé des communes du 69 haha)
Mais ok pour le SIRET plutot !

@YannickPassa YannickPassa changed the title Ajout d'une colonne contenant le nom + id d'une siae Ajout d'une colonne contenant le nom + SIRET d'une siae Sep 24, 2024
Copy link
Contributor

@rsebille rsebille left a comment

Choose a reason for hiding this comment

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

Ils peuvent le voir avec un tableau juste en dessous

Effectivement j'ai vu cette histoire de tableau à certain moment mais c'était pas clair dans la PR qu'on aurait cela de dispo ;).

Au moins avec le SIRET ils n'auront pas à chercher laquelle des n structures contient la bonne 👍.
D'ailleurs truc auquel je pense, c'est que si ils changent de SIRET alors ça fera automatiquement une nouvelle ligne, je sais pas si c'est OK ou pas dans l'utilisation envisagée 🤷.

@YannickPassa
Copy link
Contributor Author

D'ailleurs truc auquel je pense, c'est que si ils changent de SIRET alors ça fera automatiquement une nouvelle ligne, je sais pas si c'est OK ou pas dans l'utilisation envisagée

Oui car ça sera associé à une ancienne annexe financière donc c nickel chrome

@YannickPassa YannickPassa added this pull request to the merge queue Sep 24, 2024
Merged via the queue into main with commit f1e4ad1 Sep 24, 2024
5 checks passed
@YannickPassa YannickPassa deleted the YannickPassa/name_siae_with_id branch September 24, 2024 13:36
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