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

20241031 - Acteurs inactifs reparation lvao #990

Merged

Conversation

maxcorbeau
Copy link
Contributor

@maxcorbeau maxcorbeau commented Oct 31, 2024

Description succincte du problème résolu

Ticket

  • QUOI: modifier 6264 acteurs LVAO _reparation_ en changeant statut = INACTIF
  • POURQUOI: les entrées dans la base sont obsolètes (SIREN cessé ou SIRET fermé)
  • COMMENT:
    • intégration avec la base SIRENE en local
    • export et revue manuelle avec Christian (tableur fichier /data/6264_acteurs_a_desactiver.csv non inclu dans le commit)
    • execution des requettes SQL pour modifier qfdmo_acteur

Quality assurance

  • Export: filtre sur ACTIF, vérification du nombre, pas de doublon sur identifiant
  • Requettes SQL: vérifier existance de l'acteur avec identifiant_unique, s'assurer que modifie_le dans la base est pas plus récent que l'export, et seulement après MAJ de l'entrée dans la base

@maxcorbeau maxcorbeau requested a review from a team as a code owner October 31, 2024 12:53
@maxcorbeau maxcorbeau requested review from kolok and fabienheureux and removed request for a team October 31, 2024 12:53
@maxcorbeau maxcorbeau requested review from chrischarousset and removed request for fabienheureux October 31, 2024 12:56
Copy link
Contributor

@kolok kolok left a comment

Choose a reason for hiding this comment

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

Je pense qu'il y a un loup !

les acteurs (qfdm_acteur) sont surdéfinis par les révision d'acteur (qfdmo_revisionacteur) du coup, si tu désactives les acteurs sans désactiver leur révision, les acteurs vont quand même apparaitre après le calcul des acteurs affichés

Si il n'y a pas de révisions sur les acteurs concernés alors ma remarque peut-être ignoré dans le cadre de cette PR mais il faudra y penser lors de l'automatisation

Pour résoudre le problème il y a 2 manières de faire:

  • soit tu mets à jour les acteurs et leur révision si elles existent
  • soit tu crées les revisions des acteurs et tu mets à jour le statut des révisions

@maxcorbeau maxcorbeau changed the title 20241031 - Acteurs inactifs reparation lvao ancien cma 20241031 - Acteurs inactifs reparation lvao Oct 31, 2024
@maxcorbeau
Copy link
Contributor Author

Je pense qu'il y a un loup !

Du coup si je comprends bien:

  • qfdmp_acteur: on ne devrait jamais modifier cette table autre que par le biais des DAG d'import
  • qfdmp_revisionacteur: si je viens créer un acteur (si manquant) et mettre le statut = INACTIF, ce dernier prendra précédence sur qfdmp_acteur quel que soit l'état de qfdmp_acteur

@kolok
Copy link
Contributor

kolok commented Nov 4, 2024

Je pense qu'il y a un loup !

Du coup si je comprends bien:

* `qfdmp_acteur`:  on ne devrait jamais modifier cette table autre que par le biais des DAG d'import

Oui, encore qu'ici on intervient sur des données histroriques qui ne seront plus jamais mise à jour par un DAG d'import donc ça se discute

* `qfdmp_revisionacteur`: si je viens créer un acteur (si manquant) et mettre le `statut = INACTIF`, ce dernier prendra précédence sur `qfdmp_acteur` quel que soit l'état de `qfdmp_acteur`

Oui exactement

@maxcorbeau
Copy link
Contributor Author

Oui, encore qu'ici on intervient sur des données histroriques qui ne seront plus jamais mise à jour par un DAG d'import donc ça se discute

Si:

  • le point 2 est vrai (qfdmp_revisionacteur > qfdmp_acteur)
  • la règle est que qfdmp_acteur = import, qfdmp_revisionacteur = révision
  • on décide de modifier qfdmp_revisionacteur parce qu'on fait de la révision

Quel est l'intérêt de créer une nouvelle exception en modifiant qfdmp_acteur? (quelle valeur ajoutée vs. encore une exception qu'il faudra documenter, gérer dans le code ad-hoc, et qui peut-être amènera de la confusion à l'avenir: ex: "pourquoi le qfdmp_acteur.modifie_le est récent alors qu'on n'a pas importé la source depuis ____ ?")

@kolok
Copy link
Contributor

kolok commented Nov 4, 2024

Oui, encore qu'ici on intervient sur des données histroriques qui ne seront plus jamais mise à jour par un DAG d'import donc ça se discute

Si:

* le point 2 est vrai (`qfdmp_revisionacteur` > `qfdmp_acteur`)

* la règle est que `qfdmp_acteur` = import, `qfdmp_revisionacteur` = révision

* on décide de modifier `qfdmp_revisionacteur` parce qu'on fait de la révision

Quel est l'intérêt de créer une nouvelle exception en modifiant qfdmp_acteur? (quelle valeur ajoutée vs. encore une exception qu'il faudra documenter, gérer dans le code ad-hoc, et qui peut-être amènera de la confusion à l'avenir: ex: "pourquoi le qfdmp_acteur.modifie_le est récent alors qu'on n'a pas importé la source depuis ____ ?")

Faisons la modification sur qfdmo_revisionacteur, tu as raison, ça fer une exception en moins

@kolok kolok self-requested a review November 4, 2024 15:42
Copy link
Contributor

@kolok kolok left a comment

Choose a reason for hiding this comment

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

Review avec Max

@maxcorbeau maxcorbeau requested a review from kolok November 4, 2024 16:21
Copy link
Contributor

@kolok kolok left a comment

Choose a reason for hiding this comment

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

Il faut eviter de commit ce type de résultat

CleanShot 2024-11-05 at 09 21 22

Ici rien de PPI donc c'est OK mais une habiture à prendre.

J'ai fait un commit pour nettoyer les ouputs des cellules

@maxcorbeau maxcorbeau merged commit fdd2886 into main Nov 6, 2024
7 checks passed
@maxcorbeau maxcorbeau deleted the 20241031_acteurs_inactifs_reparation_lvao_ancien_cma branch November 6, 2024 05:37
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