From e261b53e0bf6fdc92c59fe562838df13871dd1c7 Mon Sep 17 00:00:00 2001 From: maxcorbeau Date: Mon, 28 Apr 2025 15:48:34 +0200 Subject: [PATCH 01/12] =?UTF-8?q?Acteurs=20ferm=C3=A9s:=20pas=20cr=C3=A9er?= =?UTF-8?q?=20de=20parent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dags/enrich/config/columns.py | 2 + .../enrich_dbt_model_to_suggestions.py | 103 ++++++++---------- .../tasks/test_enrich_suggestions_closed.py | 99 ++++++++--------- ...marts_enrich_acteurs_closed_candidates.sql | 6 +- 4 files changed, 97 insertions(+), 113 deletions(-) diff --git a/dags/enrich/config/columns.py b/dags/enrich/config/columns.py index 1ec997172..4f1df52b1 100644 --- a/dags/enrich/config/columns.py +++ b/dags/enrich/config/columns.py @@ -12,6 +12,8 @@ class COLS: # Acteurs ACTEUR_ID: str = "acteur_id" + ACTEUR_ID_EXTERNE: str = "acteur_id_externe" + ACTEUR_PARENT_ID: str = "acteur_parent_id" ACTEUR_TYPE_ID: str = "acteur_type_id" ACTEUR_SOURCE_ID: str = "acteur_source_id" ACTEUR_SIRET: str = "acteur_siret" diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 7354d8ef0..a2c8254c9 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -2,17 +2,11 @@ from datetime import datetime, timezone import pandas as pd -from cluster.tasks.business_logic.cluster_acteurs_parents_choose_new import ( - parent_id_generate, -) from enrich.config.cohorts import COHORTS from enrich.config.columns import COLS -from enrich.tasks.business_logic.enrich_dbt_model_row_to_suggest_data import ( - dbt_model_row_to_suggest_data, -) -from utils import logging_utils as log from data.models.changes.acteur_rgpd_anonymize import rgpd_data_get +from utils import logging_utils as log logger = logging.getLogger(__name__) @@ -130,79 +124,72 @@ def changes_prepare_closed_replaced( row: dict, ) -> tuple[list[dict], dict]: """Prepare suggestion changes for closed replaced cohorts""" - from data.models.changes import ( - ChangeActeurCreateAsChild, - ChangeActeurCreateAsParent, - ChangeActeurUpdateData, - ) + from data.models.changes import ChangeActeurCreateAsChild, ChangeActeurUpdateData from qfdmo.models import ActeurStatus changes = [] today = datetime.now(timezone.utc).strftime("%Y-%m-%d") - # Parent - parent_id = parent_id_generate([str(row[COLS.SUGGEST_SIRET])]) - parent_data = dbt_model_row_to_suggest_data(row) - parent_data["source"] = None - parent_data["statut"] = ActeurStatus.ACTIF - params_parent = { - "id": parent_id, - "data": parent_data, + + # We don't create a parent because we decided it was + # not a good idea (1 parent for 1 child will only create + # complications for the clustering and no added value) + + # Existing Child: must be updated BEFORE new child to prevent + # conflict on same external ID having 2+ active children + child_old = { + "id": row[COLS.ACTEUR_ID], + "data": { + "identifiant_unique": row[COLS.ACTEUR_ID], + "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], + "parent_reason": ( + f"SIRET {row[COLS.ACTEUR_SIRET]} " + f"détecté le {today} comme fermé dans AE, " + f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" + ), + "siret_is_closed": True, + "statut": ActeurStatus.INACTIF, + }, } changes.append( changes_prepare( - model=ChangeActeurCreateAsParent, - model_params=params_parent, + model=ChangeActeurUpdateData, + model_params=child_old, order=1, - reason="besoin d'un parent pour rattaché acteur fermé", + reason="ancien enfant fermé", entity_type="acteur_displayed", ) ) - # New child to hold the reference data as standalone - # as parents are surrogates (e.g. they can be deleted - # during clustering) + # (since old child will be closed) now = datetime.now(timezone.utc).strftime("%Y%m%d%H%M%S") child_new_id = f"{row[COLS.ACTEUR_ID]}_{row[COLS.ACTEUR_SIRET]}_{now}" - params_child_new = params_parent.copy() - params_child_new["id"] = child_new_id - params_child_new["data"]["source"] = row[COLS.ACTEUR_SOURCE_ID] - params_child_new["data"]["parent"] = parent_id - params_child_new["data"]["parent_reason"] = ( - f"Nouvel enfant pour conserver les données suite à: " - f"SIRET {row[COLS.ACTEUR_SIRET]} " - f"détecté le {today} comme fermé dans AE, " - f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" - ) + child_new = { + "id": child_new_id, + "data": { + "identifiant_unique": child_new_id, + "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], + "source": row[COLS.ACTEUR_SOURCE_ID], + "parent": row[COLS.ACTEUR_PARENT_ID], + "parent_reason": ( + f"Nouvel enfant pour conserver les données suite à: " + f"SIRET {row[COLS.ACTEUR_SIRET]} " + f"détecté le {today} comme fermé dans AE, " + f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}, " + "= même parent que l'ancien enfant" + ), + "statut": ActeurStatus.ACTIF, + }, + } changes.append( changes_prepare( model=ChangeActeurCreateAsChild, - model_params=params_child_new, + model_params=child_new, order=2, - reason="besoin nouvel enfant pour conserver les données", + reason="nouvel enfant pour conserver les données", entity_type="acteur_displayed", ) ) - # Existing Child - params_child_old = params_child_new.copy() - params_child_old["id"] = row[COLS.ACTEUR_ID] - params_child_old["data"]["parent"] = parent_id - params_child_old["data"]["parent_reason"] = ( - f"SIRET {row[COLS.ACTEUR_SIRET]} " - f"détecté le {today} comme fermé dans AE, " - f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" - ) - params_child_old["data"]["siret_is_closed"] = True - params_child_old["data"]["statut"] = ActeurStatus.INACTIF - changes.append( - changes_prepare( - model=ChangeActeurUpdateData, - model_params=params_child_old, - order=3, - reason="rattacher enfant fermé à un parent", - entity_type="acteur_displayed", - ) - ) contexte = {} # changes are self-explanatory return changes, contexte diff --git a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py index 27f1e7d28..dff66d6f2 100644 --- a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py +++ b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py @@ -2,9 +2,6 @@ import pandas as pd import pytest -from cluster.tasks.business_logic.cluster_acteurs_parents_choose_new import ( - parent_id_generate, -) from django.contrib.gis.geos import Point from enrich.config.cohorts import COHORTS from enrich.config.columns import COLS @@ -30,12 +27,24 @@ def atype(self): return ActeurType.objects.create(code="at1") + @pytest.fixture + def parent(self): + from unit_tests.qfdmo.acteur_factory import RevisionActeurFactory + + return RevisionActeurFactory( + nom="Parent", + adresse="Adresse parent", + code_postal="12345", + ville="Ville parent", + ) + @pytest.fixture def df_not_replaced(self, atype, source): return pd.DataFrame( { # Acteurs data COLS.ACTEUR_ID: ["a01", "a02"], + COLS.ACTEUR_ID_EXTERNE: ["ext_a01", "ext_a02"], COLS.ACTEUR_SIRET: ["00000000000001", "00000000000002"], COLS.ACTEUR_NOM: ["AVANT a01", "AVANT a02"], COLS.ACTEUR_TYPE_ID: [atype.pk, atype.pk], @@ -45,11 +54,15 @@ def df_not_replaced(self, atype, source): ) @pytest.fixture - def df_replaced(self, atype, source): + def df_replaced(self, atype, source, parent): return pd.DataFrame( { # Acteurs data COLS.ACTEUR_ID: ["a1", "a2", "a3"], + COLS.ACTEUR_ID_EXTERNE: ["ext_a1", "ext_a2", "ext_a3"], + # We test 1 acteur of each cohort with a parent, and + # the case with no parent + COLS.ACTEUR_PARENT_ID: [parent.pk, parent.pk, None], COLS.ACTEUR_SIRET: [ "11111111100001", "22222222200001", @@ -146,14 +159,18 @@ def test_cohorte_not_replaced(self, acteurs, df_not_replaced): assert a01.parent is None assert a01.parent_reason == "" # consequence of empty strings in DB assert a01.siret_is_closed is True + assert a01.identifiant_externe == "ext_a01" a02 = RevisionActeur.objects.get(pk="a02") assert a02.statut == ActeurStatus.INACTIF assert a02.parent is None assert a02.parent_reason == "" assert a02.siret_is_closed is True + assert a02.identifiant_externe == "ext_a02" - def test_cohorte_meme_siren(self, acteurs, atype, source, df_replaced_meme_siret): + def test_cohorte_meme_siren( + self, acteurs, parent, atype, source, df_replaced_meme_siret + ): from data.models.suggestion import Suggestion, SuggestionCohorte from qfdmo.models import ActeurStatus, RevisionActeur @@ -174,34 +191,25 @@ def test_cohorte_meme_siren(self, acteurs, atype, source, df_replaced_meme_siret for suggestion in suggestions: suggestion.apply() - # Verify changes - # 1 parent should be created in revision with replacement data - # 1 child should be created in revision with status=INACT and parent_id pointing - parent_id = parent_id_generate(["11111111100002"]) - parent = RevisionActeur.objects.get(pk=parent_id) - assert parent.pk == parent_id - assert parent.nom == "APRES a1" - assert parent.adresse == "Adresse1" - assert parent.code_postal == "12345" - assert parent.ville == "Ville1" - assert parent.naf_principal == "naf1" - assert parent.acteur_type == atype - assert parent.source is None - assert parent.location.x == 1 - assert parent.location.y == 11 - - child = RevisionActeur.objects.get(pk="a1") - assert child.statut == ActeurStatus.INACTIF - assert child.parent == parent - assert child.parent_reason == ( + # Closed acteur + closed = RevisionActeur.objects.get(pk="a1") + assert closed.statut == ActeurStatus.INACTIF + assert closed.parent_reason == ( f"SIRET 11111111100001 détecté le {TODAY} comme fermé dans AE, " f"remplacé par SIRET 11111111100002" ) - assert child.siret_is_closed is True - assert child.location.x == 1 - assert child.location.y == 11 - - def test_cohorte_autre_siren(self, acteurs, df_replaced_autre_siret): + assert closed.siret_is_closed is True + assert closed.location.x == 1 + assert closed.location.y == 11 + + # Replacement acteur + new = RevisionActeur.objects.get(identifiant_externe="ext_a1") + assert new.statut == ActeurStatus.ACTIF + assert new.parent.pk == parent.pk + assert new.source.pk == source.pk + assert "même parent que l'ancien enfant" in new.parent_reason + + def test_cohorte_autre_siren(self, acteurs, parent, df_replaced_autre_siret): from data.models.suggestion import Suggestion, SuggestionCohorte from qfdmo.models import ActeurStatus, RevisionActeur @@ -223,21 +231,12 @@ def test_cohorte_autre_siren(self, acteurs, df_replaced_autre_siret): suggestion.apply() # Verify changes - # 1 parent should be created in revision with replacement data - # 1 child should be created in revision with status=INACT and parent_id pointing - parent_id = parent_id_generate(["33333333300001"]) - parent = RevisionActeur.objects.get(pk=parent_id) - assert parent.nom == "APRES a2" - assert parent.adresse == "Adresse2" - assert parent.code_postal == "67890" - assert parent.ville == "Ville2" - assert parent.naf_principal == "naf2" - assert parent.location.x == 2 - assert parent.location.y == 22 - + # TODO: repeat the comparison between closed and replacements + # for both acteurs, the only different is the first has a parent + # and not the second child = RevisionActeur.objects.get(pk="a2") assert child.statut == ActeurStatus.INACTIF - assert child.parent == parent + assert child.parent.pk == parent.pk assert child.parent_reason == ( f"SIRET 22222222200001 détecté le {TODAY} comme fermé dans AE, " f"remplacé par SIRET 33333333300001" @@ -245,23 +244,15 @@ def test_cohorte_autre_siren(self, acteurs, df_replaced_autre_siret): assert child.siret_is_closed is True assert child.location.x == 2 assert child.location.y == 22 - - parent_id = parent_id_generate(["55555555500001"]) - parent = RevisionActeur.objects.get(pk=parent_id) - assert parent.nom == "APRES a3" - assert parent.adresse == "Adresse3" - assert parent.code_postal == "12345" - assert parent.ville == "Ville3" - assert parent.naf_principal == "naf3" - assert parent.location.x == 3 - assert parent.location.y == 33 + assert child.identifiant_externe == "ext_a2" child = RevisionActeur.objects.get(pk="a3") assert child.statut == ActeurStatus.INACTIF - assert child.parent == parent + assert child.parent is None # 2nd child had no parent assert child.parent_reason == ( f"SIRET 44444444400001 détecté le {TODAY} comme fermé dans AE, " f"remplacé par SIRET 55555555500001" ) assert child.location.x == 3 assert child.location.y == 33 + assert child.identifiant_externe == "ext_a3" diff --git a/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql b/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql index 51d775377..cf7dfe944 100644 --- a/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql +++ b/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql @@ -16,6 +16,7 @@ WITH acteurs_with_siret AS ( SELECT -- Acteur columns identifiant_unique AS acteur_id, + identifiant_externe AS acteur_id_externe, siret AS acteur_siret, LEFT(siret,9) AS acteur_siren, nom AS acteur_nom, @@ -27,7 +28,8 @@ WITH acteurs_with_siret AS ( adresse AS acteur_adresse, code_postal AS acteur_code_postal, ville AS acteur_ville, - location AS acteur_location + location AS acteur_location, + parent_id AS acteur_parent_id FROM {{ ref('marts_carte_acteur') }} WHERE siret IS NOT NULL AND siret != '' AND LENGTH(siret) = 14 @@ -39,6 +41,7 @@ etab_closed_candidates AS ( SELECT -- acteurs acteurs.acteur_id, + acteurs.acteur_id_externe, acteurs.acteur_siret, acteurs.acteur_siren, acteurs.acteur_type_id, @@ -50,6 +53,7 @@ SELECT acteurs.acteur_adresse, acteurs.acteur_code_postal, acteurs.acteur_ville, + acteurs.acteur_parent_id, CASE WHEN acteurs.acteur_location IS NULL THEN NULL ELSE ST_X(acteurs.acteur_location) END AS acteur_longitude, CASE WHEN acteurs.acteur_location IS NULL THEN NULL ELSE ST_Y(acteurs.acteur_location) END AS acteur_latitude, From 50183bde757191c4aad51db32303885daac98e84 Mon Sep 17 00:00:00 2001 From: maxcorbeau Date: Wed, 30 Apr 2025 11:06:05 +0200 Subject: [PATCH 02/12] =?UTF-8?q?Pas=20cr=C3=A9er=20de=20parent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../enrich_dbt_model_to_suggestions.py | 26 +-- .../tasks/test_enrich_suggestions_closed.py | 176 +++++++++++------- data/models/changes/acteur_create_as_child.py | 27 ++- data/models/changes/acteur_update_data.py | 1 + .../changes/test_acteur_create_as_child.py | 10 +- 5 files changed, 143 insertions(+), 97 deletions(-) diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index a2c8254c9..0359efe87 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -140,12 +140,6 @@ def changes_prepare_closed_replaced( "id": row[COLS.ACTEUR_ID], "data": { "identifiant_unique": row[COLS.ACTEUR_ID], - "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], - "parent_reason": ( - f"SIRET {row[COLS.ACTEUR_SIRET]} " - f"détecté le {today} comme fermé dans AE, " - f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" - ), "siret_is_closed": True, "statut": ActeurStatus.INACTIF, }, @@ -169,17 +163,23 @@ def changes_prepare_closed_replaced( "identifiant_unique": child_new_id, "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], "source": row[COLS.ACTEUR_SOURCE_ID], + "acteur_type": row[COLS.ACTEUR_TYPE_ID], "parent": row[COLS.ACTEUR_PARENT_ID], - "parent_reason": ( - f"Nouvel enfant pour conserver les données suite à: " - f"SIRET {row[COLS.ACTEUR_SIRET]} " - f"détecté le {today} comme fermé dans AE, " - f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}, " - "= même parent que l'ancien enfant" - ), + "latitude": row[COLS.ACTEUR_LATITUDE], + "longitude": row[COLS.ACTEUR_LONGITUDE], "statut": ActeurStatus.ACTIF, + "siret": row[COLS.SUGGEST_SIRET], + "siren": row[COLS.SUGGEST_SIRET][:9], }, } + # We only give a reason for the parent if there is one + if row[COLS.ACTEUR_PARENT_ID]: + child_new["data"]["parent_reason"] = ( + f"Nouvel enfant pour conserver les données suite à: " + f"SIRET {row[COLS.ACTEUR_SIRET]} " + f"détecté le {today} comme fermé dans AE, " + f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" + ) changes.append( changes_prepare( model=ChangeActeurCreateAsChild, diff --git a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py index dff66d6f2..b94efbe9e 100644 --- a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py +++ b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py @@ -1,3 +1,4 @@ +import re from datetime import datetime, timezone import pandas as pd @@ -121,15 +122,27 @@ def acteurs(self, df_not_replaced, df_replaced, atype, source): from qfdmo.models import Acteur df_concat = pd.concat([df_not_replaced, df_replaced]) - acteur_ids = df_concat[COLS.ACTEUR_ID].tolist() - for acteur_id in acteur_ids: - Acteur.objects.create( - identifiant_unique=acteur_id, - nom=f"AVANT {acteur_id}", + for _, row in df_concat.iterrows(): + acteur = Acteur( + identifiant_unique=row[COLS.ACTEUR_ID], + identifiant_externe=row[COLS.ACTEUR_ID_EXTERNE], + nom=f"AVANT {row[COLS.ACTEUR_ID]}", acteur_type=atype, source=source, - location=Point(x=0, y=0), + location=Point( + x=row[COLS.ACTEUR_LONGITUDE], y=row[COLS.ACTEUR_LATITUDE] + ), ) + acteur.save() + # Check that acteur has been properly created + acteur = Acteur.objects.get(identifiant_unique=row[COLS.ACTEUR_ID]) + assert acteur.identifiant_externe == row[COLS.ACTEUR_ID_EXTERNE] + assert acteur.nom == f"AVANT {row[COLS.ACTEUR_ID]}" + assert acteur.acteur_type == atype + assert acteur.source == source + # print(f"{acteur.location=}") + # assert acteur.location.x == row[COLS.ACTEUR_LONGITUDE] + # assert acteur.location.y == row[COLS.ACTEUR_LATITUDE] def test_cohorte_not_replaced(self, acteurs, df_not_replaced): from data.models.suggestion import Suggestion, SuggestionCohorte @@ -152,27 +165,61 @@ def test_cohorte_not_replaced(self, acteurs, df_not_replaced): for suggestion in suggestions: suggestion.apply() - # Verify changes - # 2 revisions should be created but not parent - a01 = RevisionActeur.objects.get(pk="a01") - assert a01.statut == ActeurStatus.INACTIF - assert a01.parent is None - assert a01.parent_reason == "" # consequence of empty strings in DB - assert a01.siret_is_closed is True - assert a01.identifiant_externe == "ext_a01" - - a02 = RevisionActeur.objects.get(pk="a02") - assert a02.statut == ActeurStatus.INACTIF - assert a02.parent is None - assert a02.parent_reason == "" - assert a02.siret_is_closed is True - assert a02.identifiant_externe == "ext_a02" + # Verify changes: both acteurs are closed with only relevant + # fields updated + for id in ["a01", "a02"]: + closed = RevisionActeur.objects.get(pk=id) + assert closed.statut == ActeurStatus.INACTIF + assert closed.parent is None + assert closed.parent_reason == "" # consequence of empty strings in DB + assert closed.siret_is_closed is True + + def check_replaced_acteur( + self, + id, + id_ext, + siret_closed, + siret_new, + parent, + parent_reason, + source, + acteur_type, + ): + """Utility to check replaced acteur to avoid repetition""" + from qfdmo.models import ActeurStatus, RevisionActeur + + # Closed acteur + closed = RevisionActeur.objects.get(pk=id) + assert closed.statut == ActeurStatus.INACTIF + assert closed.siret_is_closed is True + # for closed acteur we don't revision the parent data + assert closed.parent is None + assert closed.parent_reason == "" + + # The new acteur which about from siret_is_closed AND + # identifiant_externe has the same data as the closed acteur + new = RevisionActeur.objects.filter(identifiant_externe=id_ext).first() + assert new is not None + # ID is concatenation of closed acteur ID, new siret and today's datetime + assert re.search( + f"^{id}_{siret_new}_{TODAY}[0-9]{{4}}$", new.identifiant_unique + ) + assert new.statut == ActeurStatus.ACTIF + assert new.siret == siret_new + assert new.siret_is_closed is None # we have detected no closure yet + + # foreign keys are iso with the closed acteur + assert new.parent.pk == parent.pk + assert new.source.pk == source.pk # type: ignore + assert new.acteur_type.pk == acteur_type.pk + assert new.identifiant_externe == id_ext + # Explanation as to why + assert new.parent_reason == parent_reason def test_cohorte_meme_siren( self, acteurs, parent, atype, source, df_replaced_meme_siret ): from data.models.suggestion import Suggestion, SuggestionCohorte - from qfdmo.models import ActeurStatus, RevisionActeur # Write suggestions to DB enrich_dbt_model_to_suggestions( @@ -192,26 +239,26 @@ def test_cohorte_meme_siren( suggestion.apply() # Closed acteur - closed = RevisionActeur.objects.get(pk="a1") - assert closed.statut == ActeurStatus.INACTIF - assert closed.parent_reason == ( - f"SIRET 11111111100001 détecté le {TODAY} comme fermé dans AE, " - f"remplacé par SIRET 11111111100002" + self.check_replaced_acteur( + id="a1", + id_ext="ext_a1", + siret_closed="11111111100001", + siret_new="11111111100002", + parent=parent, + parent_reason=( + f"Nouvel enfant pour conserver les données suite à: " + f"SIRET 11111111100001 " + f"détecté le {TODAY} comme fermé dans AE, " + f"remplacé par SIRET 11111111100002" + ), + source=source, + acteur_type=atype, ) - assert closed.siret_is_closed is True - assert closed.location.x == 1 - assert closed.location.y == 11 - - # Replacement acteur - new = RevisionActeur.objects.get(identifiant_externe="ext_a1") - assert new.statut == ActeurStatus.ACTIF - assert new.parent.pk == parent.pk - assert new.source.pk == source.pk - assert "même parent que l'ancien enfant" in new.parent_reason - def test_cohorte_autre_siren(self, acteurs, parent, df_replaced_autre_siret): + def test_cohorte_autre_siren( + self, acteurs, parent, atype, source, df_replaced_autre_siret + ): from data.models.suggestion import Suggestion, SuggestionCohorte - from qfdmo.models import ActeurStatus, RevisionActeur # Write suggestions to DB enrich_dbt_model_to_suggestions( @@ -224,35 +271,36 @@ def test_cohorte_autre_siren(self, acteurs, parent, df_replaced_autre_siret): # Check suggestions have been written to DB cohort = SuggestionCohorte.objects.get(identifiant_action="test_autre_siren") suggestions = Suggestion.objects.filter(suggestion_cohorte=cohort) - assert len(suggestions) == 2 # (1 parent + 1 child) x 2 acteurs fermés - + assert len(suggestions) == 1 # 1 suggestion containing 2 changes (closed + new) # Apply suggestions for suggestion in suggestions: suggestion.apply() # Verify changes - # TODO: repeat the comparison between closed and replacements - # for both acteurs, the only different is the first has a parent - # and not the second - child = RevisionActeur.objects.get(pk="a2") - assert child.statut == ActeurStatus.INACTIF - assert child.parent.pk == parent.pk - assert child.parent_reason == ( - f"SIRET 22222222200001 détecté le {TODAY} comme fermé dans AE, " - f"remplacé par SIRET 33333333300001" + self.check_replaced_acteur( + id="a2", + id_ext="ext_a2", + siret_closed="22222222200001", + siret_new="44444444400001", + parent=parent, + parent_reason=( + f"Nouvel enfant pour conserver les données suite à: " + f"SIRET 22222222200001 " + f"détecté le {TODAY} comme fermé dans AE, " + f"remplacé par SIRET 44444444400001" + ), + source=source, + acteur_type=atype, ) - assert child.siret_is_closed is True - assert child.location.x == 2 - assert child.location.y == 22 - assert child.identifiant_externe == "ext_a2" - - child = RevisionActeur.objects.get(pk="a3") - assert child.statut == ActeurStatus.INACTIF - assert child.parent is None # 2nd child had no parent - assert child.parent_reason == ( - f"SIRET 44444444400001 détecté le {TODAY} comme fermé dans AE, " - f"remplacé par SIRET 55555555500001" + + self.check_replaced_acteur( + id="a3", + id_ext="ext_a3", + siret_closed="33333333300001", + siret_new="55555555500001", + parent=parent, + # This closed acteur has no parent so there should be no parent_reason + parent_reason="", + source=source, + acteur_type=atype, ) - assert child.location.x == 3 - assert child.location.y == 33 - assert child.identifiant_externe == "ext_a3" diff --git a/data/models/changes/acteur_create_as_child.py b/data/models/changes/acteur_create_as_child.py index 74282eb89..17c5e73f7 100644 --- a/data/models/changes/acteur_create_as_child.py +++ b/data/models/changes/acteur_create_as_child.py @@ -35,29 +35,24 @@ def apply(self): from qfdmo.models import Acteur, RevisionActeur # Ensure parent exists in RevisionActeur - parent = RevisionActeur.objects.get(pk=self.data["parent"]) + if self.data["parent"]: + RevisionActeur.objects.get(pk=self.data["parent"]) # Reconstruct data from RevisionActeur data = data_reconstruct(RevisionActeur, self.data) + # Consequence of mixing modification and presentation + # data in suggestions to have it visible in Django Admin + # FIXME: change Django Admin so it can show live acteur data + # and reduce pydantic data to only reflect changes to be made + if "identifiant_unique" in data: + del data["identifiant_unique"] # Create child in Acteur to hold data data_base = data.copy() del data_base["parent"] del data_base["parent_reason"] - # TODO: if we flatten our pydantic models, then we wouldn't - if "identifiant_unique" in data_base: - del data_base["identifiant_unique"] - Acteur.objects.create( - identifiant_unique=self.id, - **data_base, - ) + Acteur.objects.create(identifiant_unique=self.id, **data_base) # Create child in RevisionActeur to hold reference to parent - RevisionActeur.objects.create( - identifiant_unique=self.id, - parent_reason=data["parent_reason"], - parent=parent, - statut="ACTIF", - source=data["source"], - acteur_type=data["acteur_type"], - ) + print(f"Creating child in RevisionActeur: {data}") + RevisionActeur.objects.create(**data) diff --git a/data/models/changes/acteur_update_data.py b/data/models/changes/acteur_update_data.py index d629e3603..1e2d08caf 100644 --- a/data/models/changes/acteur_update_data.py +++ b/data/models/changes/acteur_update_data.py @@ -32,5 +32,6 @@ def apply(self): ) data = data_reconstruct(RevisionActeur, self.data) for key, value in data.items(): + print(f"update_data: {key=} {value=}") setattr(acteur, key, value) acteur.save() diff --git a/unit_tests/data/models/changes/test_acteur_create_as_child.py b/unit_tests/data/models/changes/test_acteur_create_as_child.py index 325b5bcdf..e258451c7 100644 --- a/unit_tests/data/models/changes/test_acteur_create_as_child.py +++ b/unit_tests/data/models/changes/test_acteur_create_as_child.py @@ -47,10 +47,11 @@ def test_working(self): "nom": "my child1", "source": source, "acteur_type": atype, - "statut": "ACFIF", + "statut": "ACTIF", "location": Point(1, 1), "parent": parent, "parent_reason": "test", + "identifiant_externe": "test_ext", }, ) change.apply() @@ -61,12 +62,13 @@ def test_working(self): assert base.nom == "my child1" assert base.source.pk == source.pk assert base.acteur_type.pk == atype.pk - assert base.statut == "ACFIF" + assert base.statut == "ACTIF" assert base.location.x == 1 assert base.location.y == 1 # Acteur created in revision to hold the parent reference - revision = RevisionActeur.objects.get(pk="child1") + revision = RevisionActeur.objects.get( + pk=f"{source.code}_{base.identifiant_externe}" + ) assert revision.parent.pk == parent.pk assert revision.parent_reason == "test" - assert not revision.nom From aab413c189e038dd213aea17f477e7928757cb82 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Wed, 7 May 2025 15:59:37 +0200 Subject: [PATCH 03/12] Adaptation to make it works --- Makefile | 5 +- Procfile.all.dev | 3 ++ dags/enrich/config/dbt.py | 3 ++ dags/enrich/dags/enrich_dbt_models_refresh.py | 9 +++- .../enrich_dbt_model_to_suggestions.py | 51 +++++++++--------- data/models/changes/__init__.py | 2 + data/models/changes/acteur_create_as_child.py | 16 ++---- data/models/changes/acteur_create_as_copy.py | 53 +++++++++++++++++++ ...marts_enrich_acteurs_closed_candidates.sql | 4 +- .../marts_enrich_acteurs_rgpd_suggest.sql | 4 +- ...marts_enrich_acteurs_villes_candidates.sql | 4 +- dbt/models/source/source_enrich.yml | 8 +++ docs/explications/data/enrich/README.md | 30 +++++++++++ qfdmo/models/acteur.py | 20 +++---- .../changes/test_acteur_create_as_child.py | 6 +-- 15 files changed, 157 insertions(+), 61 deletions(-) create mode 100644 Procfile.all.dev create mode 100644 data/models/changes/acteur_create_as_copy.py create mode 100644 dbt/models/source/source_enrich.yml create mode 100644 docs/explications/data/enrich/README.md diff --git a/Makefile b/Makefile index 5a8c5082a..4a88d4bd2 100644 --- a/Makefile +++ b/Makefile @@ -57,13 +57,12 @@ run-airflow: .PHONY: run-django run-django: - rm -rf .parcel-cache + docker compose --profile lvao up -d honcho start -f Procfile.dev run-all: docker compose --profile airflow up -d - rm -rf .parcel-cache - honcho start -f Procfile.dev + honcho start -f Procfile.all.dev # Local django operations .PHONY: migrate diff --git a/Procfile.all.dev b/Procfile.all.dev new file mode 100644 index 000000000..64359960c --- /dev/null +++ b/Procfile.all.dev @@ -0,0 +1,3 @@ +services: docker compose --profile airflow up +web: poetry run python manage.py runserver 0.0.0.0:8000 +npm: rm -rf .parcel-cache; npm run watch diff --git a/dags/enrich/config/dbt.py b/dags/enrich/config/dbt.py index ab7f09ef1..e1f00d2c6 100644 --- a/dags/enrich/config/dbt.py +++ b/dags/enrich/config/dbt.py @@ -10,12 +10,15 @@ class DBT: # Closed MARTS_ENRICH_AE_CLOSED_CANDIDATES: str = "marts_enrich_acteurs_closed_candidates" + # replace SIRET only MARTS_ENRICH_AE_CLOSED_REPLACED_SAME_SIREN: str = ( "marts_enrich_acteurs_closed_suggest_replaced_same_siren" ) + # replace SIREN and SIRET MARTS_ENRICH_AE_CLOSED_REPLACED_OTHER_SIREN: str = ( "marts_enrich_acteurs_closed_suggest_replaced_other_siren" ) + # inactive acteur, no new SIRET / SIREN found MARTS_ENRICH_AE_CLOSED_NOT_REPLACED: str = ( "marts_enrich_acteurs_closed_suggest_not_replaced" ) diff --git a/dags/enrich/dags/enrich_dbt_models_refresh.py b/dags/enrich/dags/enrich_dbt_models_refresh.py index 03bc9c41f..dae0f663e 100644 --- a/dags/enrich/dags/enrich_dbt_models_refresh.py +++ b/dags/enrich/dags/enrich_dbt_models_refresh.py @@ -1,5 +1,6 @@ """DAG to refresh DBT models needed for enrich DAGs""" +import logging import re from airflow import DAG @@ -9,6 +10,8 @@ from enrich.config.models import EnrichDbtModelsRefreshConfig from shared.config import CATCHUPS, SCHEDULES, START_DATES, config_to_airflow_params +logger = logging.getLogger(__name__) + with DAG( dag_id="enrich_dbt_models_refresh", dag_display_name="🔄 Enrichir - Rafraîchir les modèles DBT", @@ -39,10 +42,12 @@ tasks = [] for cmd in dag.params.get("dbt_models_refresh_commands", []): cmd = cmd.strip() + cmd_id = re.sub(r"__+", "_", re.sub(r"[^a-zA-Z0-9]+", "_", cmd)) + cmd = "cd /opt/airflow/dbt/ && " + cmd if not cmd: continue - cmd_id = re.sub(r"__+", "_", re.sub(r"[^a-zA-Z0-9]+", "_", cmd)) - cmd += " --debug --threads 1" + # cmd += " --debug --threads 1" + logger.info(f"Create bash operator with command: {cmd}") tasks.append( BashOperator( task_id=f"enrich_{cmd_id}", diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 0359efe87..7f0d5619c 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -124,7 +124,7 @@ def changes_prepare_closed_replaced( row: dict, ) -> tuple[list[dict], dict]: """Prepare suggestion changes for closed replaced cohorts""" - from data.models.changes import ChangeActeurCreateAsChild, ChangeActeurUpdateData + from data.models.changes import ChangeActeurCreateAsCopy, ChangeActeurUpdateData from qfdmo.models import ActeurStatus changes = [] @@ -136,7 +136,7 @@ def changes_prepare_closed_replaced( # Existing Child: must be updated BEFORE new child to prevent # conflict on same external ID having 2+ active children - child_old = { + old_acteur = { "id": row[COLS.ACTEUR_ID], "data": { "identifiant_unique": row[COLS.ACTEUR_ID], @@ -147,7 +147,7 @@ def changes_prepare_closed_replaced( changes.append( changes_prepare( model=ChangeActeurUpdateData, - model_params=child_old, + model_params=old_acteur, order=1, reason="ancien enfant fermé", entity_type="acteur_displayed", @@ -156,34 +156,33 @@ def changes_prepare_closed_replaced( # New child to hold the reference data as standalone # (since old child will be closed) now = datetime.now(timezone.utc).strftime("%Y%m%d%H%M%S") - child_new_id = f"{row[COLS.ACTEUR_ID]}_{row[COLS.ACTEUR_SIRET]}_{now}" - child_new = { - "id": child_new_id, + new_acteur_id = f"{row[COLS.ACTEUR_ID]}_{row[COLS.ACTEUR_SIRET]}_{now}" + new_acteur = { + "id": row[COLS.ACTEUR_ID], "data": { - "identifiant_unique": child_new_id, - "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], - "source": row[COLS.ACTEUR_SOURCE_ID], - "acteur_type": row[COLS.ACTEUR_TYPE_ID], - "parent": row[COLS.ACTEUR_PARENT_ID], - "latitude": row[COLS.ACTEUR_LATITUDE], - "longitude": row[COLS.ACTEUR_LONGITUDE], - "statut": ActeurStatus.ACTIF, + "identifiant_unique": new_acteur_id, + # FIXME : all these fields will be inherited from the old acteur + # "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], + # "source": row[COLS.ACTEUR_SOURCE_ID], + # "acteur_type": row[COLS.ACTEUR_TYPE_ID], + # "parent": row[COLS.ACTEUR_PARENT_ID], + # "latitude": row[COLS.ACTEUR_LATITUDE], + # "longitude": row[COLS.ACTEUR_LONGITUDE], + # "statut": ActeurStatus.ACTIF, "siret": row[COLS.SUGGEST_SIRET], "siren": row[COLS.SUGGEST_SIRET][:9], + "parent_reason": ( # FIXME : renommer parent_reason + "Nouvel enfant pour conserver les données suite à: " + f"SIRET {row[COLS.ACTEUR_SIRET]} " + f"détecté le {today} comme fermé dans AE, " + f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" + ), }, } - # We only give a reason for the parent if there is one - if row[COLS.ACTEUR_PARENT_ID]: - child_new["data"]["parent_reason"] = ( - f"Nouvel enfant pour conserver les données suite à: " - f"SIRET {row[COLS.ACTEUR_SIRET]} " - f"détecté le {today} comme fermé dans AE, " - f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" - ) changes.append( changes_prepare( - model=ChangeActeurCreateAsChild, - model_params=child_new, + model=ChangeActeurCreateAsCopy, + model_params=new_acteur, order=2, reason="nouvel enfant pour conserver les données", entity_type="acteur_displayed", @@ -246,7 +245,9 @@ def enrich_dbt_model_to_suggestions( row = dict(row) try: - changes, contexte = COHORTS_TO_PREPARE_CHANGES[cohort](row) + interpret_function = COHORTS_TO_PREPARE_CHANGES[cohort] + logger.info(f"Interprétation de {cohort=}") + changes, contexte = interpret_function(row) suggestion = { "contexte": contexte, "suggestion": {"title": cohort, "changes": changes}, diff --git a/data/models/changes/__init__.py b/data/models/changes/__init__.py index 6d1b5e002..9e5add9b3 100644 --- a/data/models/changes/__init__.py +++ b/data/models/changes/__init__.py @@ -1,5 +1,6 @@ from .acteur_change_nothing_in_base import ChangeActeurNothingBase from .acteur_create_as_child import ChangeActeurCreateAsChild +from .acteur_create_as_copy import ChangeActeurCreateAsCopy from .acteur_create_as_parent import ChangeActeurCreateAsParent from .acteur_delete_as_parent import ChangeActeurDeleteAsParent from .acteur_keep_as_parent import ChangeActeurKeepAsParent @@ -12,6 +13,7 @@ CHANGE_MODELS = { ChangeActeurUpdateData.name(): ChangeActeurUpdateData, ChangeActeurCreateAsChild.name(): ChangeActeurCreateAsChild, + ChangeActeurCreateAsCopy.name(): ChangeActeurCreateAsCopy, ChangeActeurCreateAsParent.name(): ChangeActeurCreateAsParent, ChangeActeurDeleteAsParent.name(): ChangeActeurDeleteAsParent, ChangeActeurUpdateParentId.name(): ChangeActeurUpdateParentId, diff --git a/data/models/changes/acteur_create_as_child.py b/data/models/changes/acteur_create_as_child.py index 17c5e73f7..7550f9027 100644 --- a/data/models/changes/acteur_create_as_child.py +++ b/data/models/changes/acteur_create_as_child.py @@ -1,3 +1,5 @@ +## FIXME : voir si cette classe doit être dépréciée + from pydantic import BaseModel from data.models.changes.utils import data_reconstruct @@ -32,7 +34,7 @@ def validate(self): def apply(self): self.validate() - from qfdmo.models import Acteur, RevisionActeur + from qfdmo.models import RevisionActeur # Ensure parent exists in RevisionActeur if self.data["parent"]: @@ -41,18 +43,6 @@ def apply(self): # Reconstruct data from RevisionActeur data = data_reconstruct(RevisionActeur, self.data) - # Consequence of mixing modification and presentation - # data in suggestions to have it visible in Django Admin - # FIXME: change Django Admin so it can show live acteur data - # and reduce pydantic data to only reflect changes to be made - if "identifiant_unique" in data: - del data["identifiant_unique"] - # Create child in Acteur to hold data - data_base = data.copy() - del data_base["parent"] - del data_base["parent_reason"] - Acteur.objects.create(identifiant_unique=self.id, **data_base) - # Create child in RevisionActeur to hold reference to parent print(f"Creating child in RevisionActeur: {data}") RevisionActeur.objects.create(**data) diff --git a/data/models/changes/acteur_create_as_copy.py b/data/models/changes/acteur_create_as_copy.py new file mode 100644 index 000000000..713aa694f --- /dev/null +++ b/data/models/changes/acteur_create_as_copy.py @@ -0,0 +1,53 @@ +from pydantic import BaseModel + + +class ChangeActeurCreateAsCopy(BaseModel): + id: str # id of the acteur to copy + data: dict = {} # data to set on the new acteur + + def _get_acteur_to_copy(self): + from qfdmo.models import Acteur, RevisionActeur + + return RevisionActeur.objects.filter(pk=self.id).first() or Acteur.objects.get( + pk=self.id + ) + + @classmethod + def name(cls) -> str: + return "acteur_create_as_copy" + + def validate(self): + from qfdmo.models import Acteur, ActeurStatus + + # Ensure source exists + try: + acteur = self._get_acteur_to_copy() + except Acteur.DoesNotExist: + msg = f"Copy d'acteur: l'acteur source '{self.id}' n'existe pas dans Acteur" + raise ValueError(msg) + + # Ensure Acteur is not active + if acteur.statut == ActeurStatus.ACTIF: + msg = f"Copy d'acteur: l'acteur source '{self.id}' ne doit pas être actif" + raise ValueError(msg) + + # Ensure identifiant_unique is overridden + if "identifiant_unique" not in self.data: + msg = "Copy d'acteur: l'acteur cible doit surdefinir son identifiant_unique" + raise ValueError(msg) + + # Ensure identifiant_unique is not the same as the source + if self.data["identifiant_unique"] == acteur.identifiant_unique: + msg = ( + "Copy d'acteur: l'acteur cible doit avoir un identifiant_unique" + " différent de l'acteur source" + ) + raise ValueError(msg) + + def apply(self): + self.validate() + from qfdmo.models import Acteur + + acteur_to_copy = Acteur.objects.get(pk=self.id) + revision_acteur_to_copy = acteur_to_copy.get_or_create_revision() + revision_acteur_to_copy.duplicate(fields_to_reset=self.data) diff --git a/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql b/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql index cf7dfe944..6062d74e8 100644 --- a/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql +++ b/dbt/models/marts/enrich/marts_enrich_acteurs_closed_candidates.sql @@ -31,8 +31,8 @@ WITH acteurs_with_siret AS ( location AS acteur_location, parent_id AS acteur_parent_id - FROM {{ ref('marts_carte_acteur') }} - WHERE siret IS NOT NULL AND siret != '' AND LENGTH(siret) = 14 + FROM {{ source('enrich', 'qfdmo_vueacteur') }} AS acteurs + WHERE siret IS NOT NULL AND siret != '' AND LENGTH(siret) = 14/* AND (acteurs.source_id is null or acteurs.source_id in (45, 252)) */ ), /* Filtering on etab closed (NOT etab.est_actif) BUT not on unite closed (NOT unite_est_actif) because diff --git a/dbt/models/marts/enrich/marts_enrich_acteurs_rgpd_suggest.sql b/dbt/models/marts/enrich/marts_enrich_acteurs_rgpd_suggest.sql index 5177fc57c..a1a2bb323 100644 --- a/dbt/models/marts/enrich/marts_enrich_acteurs_rgpd_suggest.sql +++ b/dbt/models/marts/enrich/marts_enrich_acteurs_rgpd_suggest.sql @@ -25,12 +25,12 @@ WITH acteurs_with_siren AS ( udf_normalize_string_for_match(CONCAT(nom || ' ' || nom_officiel || ' ' || nom_commercial)) AS noms_normalises, commentaires, statut - FROM {{ ref('marts_carte_acteur') }} + FROM {{ source('enrich', 'qfdmo_vueacteur') }} AS acteurs /* We have normalization issues with our SIREN field in our DB and we obtain better matching by reconstructing SIREN via SIRET */ - WHERE siret IS NOT NULL AND siret != '' AND LENGTH(siret) = 14 + WHERE siret IS NOT NULL AND siret != '' AND LENGTH(siret) = 14/* AND (acteurs.source_id is null or acteurs.source_id in (45, 252)) */ ), unite_matching_acteurs_on_siren AS ( SELECT acteurs.id AS acteur_id, diff --git a/dbt/models/marts/enrich/marts_enrich_acteurs_villes_candidates.sql b/dbt/models/marts/enrich/marts_enrich_acteurs_villes_candidates.sql index d5835192c..025995ee3 100644 --- a/dbt/models/marts/enrich/marts_enrich_acteurs_villes_candidates.sql +++ b/dbt/models/marts/enrich/marts_enrich_acteurs_villes_candidates.sql @@ -17,9 +17,9 @@ SELECT ban.ville AS ban_ville, ban.code_postal AS ban_code_postal, ban.ville AS suggest_ville -FROM {{ ref('marts_carte_acteur') }} AS acteurs +FROM {{ source('enrich', 'qfdmo_vueacteur') }} AS acteurs JOIN {{ ref('int_ban_villes') }} AS ban ON ban.code_postal = acteurs.code_postal -WHERE acteurs.statut = 'ACTIF' +WHERE acteurs.statut = 'ACTIF'/* AND (acteurs.source_id is null or acteurs.source_id in (45, 252)) */ AND acteurs.code_postal IS NOT NULL and acteurs.code_postal != '' and LENGTH(acteurs.code_postal) = 5 /* Only suggest if 1 difference */ AND ( diff --git a/dbt/models/source/source_enrich.yml b/dbt/models/source/source_enrich.yml new file mode 100644 index 000000000..504215622 --- /dev/null +++ b/dbt/models/source/source_enrich.yml @@ -0,0 +1,8 @@ +version: 2 + +sources: + - name: enrich + description: "Enrich" + schema: public + tables: + - name: qfdmo_vueacteur diff --git a/docs/explications/data/enrich/README.md b/docs/explications/data/enrich/README.md new file mode 100644 index 000000000..c9b9372ff --- /dev/null +++ b/docs/explications/data/enrich/README.md @@ -0,0 +1,30 @@ +# Enrichissement de données + +Le principe de l'enrichissement de données est d'hidrater et/ou corriger les données de «Longue vie aux objets» grâce à des sources partenaires ou exécutant des scripts de cohérence. + +## Enrichissements via des sources partenaires + +Les sources aujourd'hui utilisées sont : + +* [Annuaire entreprise](https://annuaire-entreprises.data.gouv.fr/) : agrégateur de données sur les entreprises en France +* [La BAN : Banque d'adresse nationnale](https://adresse.data.gouv.fr/) : référencement et géolocalisation de toutes les adresses en France + +### Comment ça marche + +Plusieurs étapes : + +1. Téléchargement de la base de données partenaire et copie sur notre propre base de données (DAG Airflow) + * Cloner - AE - Etablissement + * Cloner - AE - Unite Legale + * Cloner - BAN - Adresses + * Cloner - BAN - Lieux-dits +1. Préparation de la donnée (Airflow + DBT) : + * 🔄 Enrichir - Rafraîchir les modèles DBT +1. Création des suggestions (Airflow + DBT) : + * 🚪 Enrichir - Acteurs Fermés + +## Script de cohérence + +### Vérification des URLs + +le DAG `🔗 Crawl - URLs - Suggestions` collecte les URLs des acteurs et parcourt ces URL pour vérifier qu'elles sont valident diff --git a/qfdmo/models/acteur.py b/qfdmo/models/acteur.py index 84fac26d9..c757f0f17 100644 --- a/qfdmo/models/acteur.py +++ b/qfdmo/models/acteur.py @@ -895,7 +895,14 @@ def create_parent(self): self.save() return revision_acteur_parent - def duplicate(self): + def duplicate( + self, + fields_to_reset={ + "identifiant_unique": None, + "identifiant_externe": None, + "source": None, + }, + ): if self.is_parent: raise Exception("Impossible de dupliquer un acteur parent") @@ -903,11 +910,6 @@ def duplicate(self): acteur = Acteur.objects.get(identifiant_unique=self.identifiant_unique) - fields_to_reset = [ - "identifiant_unique", - "identifiant_externe", - "source", - ] fields_to_ignore = [ "labels", "acteur_services", @@ -916,12 +918,12 @@ def duplicate(self): "parent_reason", ] - for field in fields_to_reset: - setattr(revision_acteur, field, None) + for field, value in fields_to_reset.items(): + setattr(revision_acteur, field, value) for field in revision_acteur._meta.fields: if ( not getattr(revision_acteur, field.name) - and field.name not in fields_to_reset + and field.name not in fields_to_reset.keys() and field.name not in fields_to_ignore ): setattr(revision_acteur, field.name, getattr(acteur, field.name)) diff --git a/unit_tests/data/models/changes/test_acteur_create_as_child.py b/unit_tests/data/models/changes/test_acteur_create_as_child.py index e258451c7..8a71205c8 100644 --- a/unit_tests/data/models/changes/test_acteur_create_as_child.py +++ b/unit_tests/data/models/changes/test_acteur_create_as_child.py @@ -35,7 +35,7 @@ def test_working(self): atype = ActeurTypeFactory(code="atype1") parent = RevisionActeur.objects.create( identifiant_unique="parent1", - source=source, + source=None, acteur_type=atype, statut="ACTIF", location=Point(1, 1), @@ -57,8 +57,8 @@ def test_working(self): change.apply() # Acteur created in base to hold the core data - base = Acteur.objects.get(pk="child1") - assert base.identifiant_unique == "child1" + base = Acteur.objects.get(pk="source1_test_ext") + assert base.identifiant_unique == "source1_test_ext" assert base.nom == "my child1" assert base.source.pk == source.pk assert base.acteur_type.pk == atype.pk From 58df07866c96d0667aaa4b8c01bf8c39e6642b50 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Wed, 7 May 2025 17:23:50 +0200 Subject: [PATCH 04/12] deprecate acteurCreateAsChild --- data/models/changes/__init__.py | 2 - data/models/changes/acteur_create_as_child.py | 48 ------------ .../changes/test_acteur_create_as_child.py | 74 ------------------- 3 files changed, 124 deletions(-) delete mode 100644 data/models/changes/acteur_create_as_child.py delete mode 100644 unit_tests/data/models/changes/test_acteur_create_as_child.py diff --git a/data/models/changes/__init__.py b/data/models/changes/__init__.py index 9e5add9b3..743161404 100644 --- a/data/models/changes/__init__.py +++ b/data/models/changes/__init__.py @@ -1,5 +1,4 @@ from .acteur_change_nothing_in_base import ChangeActeurNothingBase -from .acteur_create_as_child import ChangeActeurCreateAsChild from .acteur_create_as_copy import ChangeActeurCreateAsCopy from .acteur_create_as_parent import ChangeActeurCreateAsParent from .acteur_delete_as_parent import ChangeActeurDeleteAsParent @@ -12,7 +11,6 @@ CHANGE_MODELS = { ChangeActeurUpdateData.name(): ChangeActeurUpdateData, - ChangeActeurCreateAsChild.name(): ChangeActeurCreateAsChild, ChangeActeurCreateAsCopy.name(): ChangeActeurCreateAsCopy, ChangeActeurCreateAsParent.name(): ChangeActeurCreateAsParent, ChangeActeurDeleteAsParent.name(): ChangeActeurDeleteAsParent, diff --git a/data/models/changes/acteur_create_as_child.py b/data/models/changes/acteur_create_as_child.py deleted file mode 100644 index 7550f9027..000000000 --- a/data/models/changes/acteur_create_as_child.py +++ /dev/null @@ -1,48 +0,0 @@ -## FIXME : voir si cette classe doit être dépréciée - -from pydantic import BaseModel - -from data.models.changes.utils import data_reconstruct - - -class ChangeActeurCreateAsChild(BaseModel): - id: str - data: dict = {} - - @classmethod - def name(cls) -> str: - return "acteur_create_as_child" - - def validate(self): - from qfdmo.models import Acteur, DisplayedActeur, RevisionActeur - - # Parent field must be SET (but we can't check if parent exists yet - # as it could be a new parent to be created) - for field in ["parent", "parent_reason"]: - if not self.data.get(field): - msg = f"Création d'enfant: champ '{field}' à renseigner {self.data}" - raise ValueError(msg) - - # Ensure child exists nowhere - for model in [Acteur, RevisionActeur, DisplayedActeur]: - obj = model.objects.filter(pk=self.id) - if obj.exists(): - msg = ( - f"Création d'enfant: '{self.id}' existe déjà dans {model.__name__}" - ) - raise ValueError(msg) - - def apply(self): - self.validate() - from qfdmo.models import RevisionActeur - - # Ensure parent exists in RevisionActeur - if self.data["parent"]: - RevisionActeur.objects.get(pk=self.data["parent"]) - - # Reconstruct data from RevisionActeur - data = data_reconstruct(RevisionActeur, self.data) - - # Create child in RevisionActeur to hold reference to parent - print(f"Creating child in RevisionActeur: {data}") - RevisionActeur.objects.create(**data) diff --git a/unit_tests/data/models/changes/test_acteur_create_as_child.py b/unit_tests/data/models/changes/test_acteur_create_as_child.py deleted file mode 100644 index 8a71205c8..000000000 --- a/unit_tests/data/models/changes/test_acteur_create_as_child.py +++ /dev/null @@ -1,74 +0,0 @@ -import pytest -from django.contrib.gis.geos import Point - -from data.models.changes.acteur_create_as_child import ChangeActeurCreateAsChild -from qfdmo.models import Acteur, RevisionActeur -from unit_tests.qfdmo.acteur_factory import ( - ActeurFactory, - ActeurTypeFactory, - SourceFactory, -) - - -@pytest.mark.django_db -class TestChangeActeurCreateAsChild: - @pytest.mark.parametrize( - "data,missing", - [({"parent": "456"}, "parent_reason"), ({"parent_reason": "test"}, "parent")], - ) - def test_raise_if_missing_params(self, data, missing): - change = ChangeActeurCreateAsChild(id="123", data=data) - with pytest.raises(ValueError, match=f"champ '{missing}' à renseigner"): - change.apply() - - def test_raise_if_acteur_exists(self): - ActeurFactory(identifiant_unique="123") - change = ChangeActeurCreateAsChild( - id="123", data={"parent": "456", "parent_reason": "test"} - ) - with pytest.raises(ValueError, match="existe déjà"): - change.apply() - - def test_working(self): - # Create parent - source = SourceFactory(code="source1") - atype = ActeurTypeFactory(code="atype1") - parent = RevisionActeur.objects.create( - identifiant_unique="parent1", - source=None, - acteur_type=atype, - statut="ACTIF", - location=Point(1, 1), - ) - # Create child - change = ChangeActeurCreateAsChild( - id="child1", - data={ - "nom": "my child1", - "source": source, - "acteur_type": atype, - "statut": "ACTIF", - "location": Point(1, 1), - "parent": parent, - "parent_reason": "test", - "identifiant_externe": "test_ext", - }, - ) - change.apply() - - # Acteur created in base to hold the core data - base = Acteur.objects.get(pk="source1_test_ext") - assert base.identifiant_unique == "source1_test_ext" - assert base.nom == "my child1" - assert base.source.pk == source.pk - assert base.acteur_type.pk == atype.pk - assert base.statut == "ACTIF" - assert base.location.x == 1 - assert base.location.y == 1 - - # Acteur created in revision to hold the parent reference - revision = RevisionActeur.objects.get( - pk=f"{source.code}_{base.identifiant_externe}" - ) - assert revision.parent.pk == parent.pk - assert revision.parent_reason == "test" From cf3aea2fa2095e9fc6ef3f62956238d771f12e35 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Mon, 12 May 2025 15:40:04 +0200 Subject: [PATCH 05/12] finalize --- .../tables/create_table_ban_lieux_dits.sql | 2 +- .../business_logic/clone_table_validate.py | 2 +- .../enrich_dbt_model_to_suggestions.py | 19 +++----- data/models/changes/__init__.py | 2 + data/models/changes/acteur_create_as_copy.py | 38 ++++++++++++---- data/models/changes/acteur_update_statut.py | 32 ++++++++++++++ docs/explications/data/enrich/README.md | 18 ++++++++ qfdmo/models/acteur.py | 43 ++++++++++++++++--- 8 files changed, 128 insertions(+), 28 deletions(-) create mode 100644 data/models/changes/acteur_update_statut.py diff --git a/dags/clone/config/sql/creation/tables/create_table_ban_lieux_dits.sql b/dags/clone/config/sql/creation/tables/create_table_ban_lieux_dits.sql index 9b4b1bcde..c0b2f2253 100644 --- a/dags/clone/config/sql/creation/tables/create_table_ban_lieux_dits.sql +++ b/dags/clone/config/sql/creation/tables/create_table_ban_lieux_dits.sql @@ -4,7 +4,7 @@ Schema generated by scripts/db_schema_create_from_csv.py CREATE TABLE {{table_name}} ( "id" VARCHAR(12), -- 🟡 on reste scrict (min/max=10/12) - "nom_lieu_dit" VARCHAR(125), -- 98 -> 125 + "nom_lieu_dit" VARCHAR(255), -- 98 -> 255 "code_postal" CHAR(5), -- 🟡 on reste scrict (min/max=5) "code_insee" CHAR(5), -- 🟡 on reste scrict (min/max=5) "nom_commune" VARCHAR(60), -- 45 -> 60 diff --git a/dags/clone/tasks/business_logic/clone_table_validate.py b/dags/clone/tasks/business_logic/clone_table_validate.py index 18ba6a8cf..011ab0900 100644 --- a/dags/clone/tasks/business_logic/clone_table_validate.py +++ b/dags/clone/tasks/business_logic/clone_table_validate.py @@ -2,6 +2,7 @@ from clone.config import DIR_SQL_VALIDATION from django.db import connection + from utils import logging_utils as log logger = logging.getLogger(__name__) @@ -23,7 +24,6 @@ def clone_table_validate(table_kind: str, table_name: str, dry_run: bool) -> Non logger.info("Mode dry-run, on ne valide pas") continue with connection.cursor() as cursor: - # Running validation and getting results cursor.execute(sql) row = cursor.fetchone() diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 7f0d5619c..717d69608 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -124,7 +124,7 @@ def changes_prepare_closed_replaced( row: dict, ) -> tuple[list[dict], dict]: """Prepare suggestion changes for closed replaced cohorts""" - from data.models.changes import ChangeActeurCreateAsCopy, ChangeActeurUpdateData + from data.models.changes import ChangeActeurCreateAsCopy, ChangeActeurUpdateStatut from qfdmo.models import ActeurStatus changes = [] @@ -146,33 +146,26 @@ def changes_prepare_closed_replaced( } changes.append( changes_prepare( - model=ChangeActeurUpdateData, + model=ChangeActeurUpdateStatut, model_params=old_acteur, order=1, - reason="ancien enfant fermé", + reason="ancienne version de l'acteur inactive", entity_type="acteur_displayed", ) ) # New child to hold the reference data as standalone # (since old child will be closed) now = datetime.now(timezone.utc).strftime("%Y%m%d%H%M%S") - new_acteur_id = f"{row[COLS.ACTEUR_ID]}_{row[COLS.ACTEUR_SIRET]}_{now}" + new_acteur_id = f"{row[COLS.ACTEUR_ID]}_{now}" new_acteur = { "id": row[COLS.ACTEUR_ID], "data": { "identifiant_unique": new_acteur_id, - # FIXME : all these fields will be inherited from the old acteur - # "identifiant_externe": row[COLS.ACTEUR_ID_EXTERNE], - # "source": row[COLS.ACTEUR_SOURCE_ID], - # "acteur_type": row[COLS.ACTEUR_TYPE_ID], - # "parent": row[COLS.ACTEUR_PARENT_ID], - # "latitude": row[COLS.ACTEUR_LATITUDE], - # "longitude": row[COLS.ACTEUR_LONGITUDE], - # "statut": ActeurStatus.ACTIF, + "statut": ActeurStatus.ACTIF, "siret": row[COLS.SUGGEST_SIRET], "siren": row[COLS.SUGGEST_SIRET][:9], "parent_reason": ( # FIXME : renommer parent_reason - "Nouvel enfant pour conserver les données suite à: " + "Nouvelle version de l'acteur conservées suite aux modifications: " f"SIRET {row[COLS.ACTEUR_SIRET]} " f"détecté le {today} comme fermé dans AE, " f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" diff --git a/data/models/changes/__init__.py b/data/models/changes/__init__.py index 743161404..1a26cb756 100644 --- a/data/models/changes/__init__.py +++ b/data/models/changes/__init__.py @@ -6,6 +6,7 @@ from .acteur_rgpd_anonymize import ChangeActeurRgpdAnonymize from .acteur_update_data import ChangeActeurUpdateData from .acteur_update_parent_id import ChangeActeurUpdateParentId +from .acteur_update_statut import ChangeActeurUpdateStatut from .acteur_verify_in_revision import ChangeActeurVerifyRevision from .sample_model_do_nothing import SampleModelDoNothing @@ -20,4 +21,5 @@ ChangeActeurKeepAsParent.name(): ChangeActeurKeepAsParent, SampleModelDoNothing.name(): SampleModelDoNothing, ChangeActeurRgpdAnonymize.name(): ChangeActeurRgpdAnonymize, + ChangeActeurUpdateStatut.name(): ChangeActeurUpdateStatut, } diff --git a/data/models/changes/acteur_create_as_copy.py b/data/models/changes/acteur_create_as_copy.py index 713aa694f..d2f55402b 100644 --- a/data/models/changes/acteur_create_as_copy.py +++ b/data/models/changes/acteur_create_as_copy.py @@ -1,5 +1,9 @@ +import logging + from pydantic import BaseModel +logger = logging.getLogger(__name__) + class ChangeActeurCreateAsCopy(BaseModel): id: str # id of the acteur to copy @@ -17,7 +21,7 @@ def name(cls) -> str: return "acteur_create_as_copy" def validate(self): - from qfdmo.models import Acteur, ActeurStatus + from qfdmo.models import Acteur # Ensure source exists try: @@ -26,11 +30,6 @@ def validate(self): msg = f"Copy d'acteur: l'acteur source '{self.id}' n'existe pas dans Acteur" raise ValueError(msg) - # Ensure Acteur is not active - if acteur.statut == ActeurStatus.ACTIF: - msg = f"Copy d'acteur: l'acteur source '{self.id}' ne doit pas être actif" - raise ValueError(msg) - # Ensure identifiant_unique is overridden if "identifiant_unique" not in self.data: msg = "Copy d'acteur: l'acteur cible doit surdefinir son identifiant_unique" @@ -44,10 +43,33 @@ def validate(self): ) raise ValueError(msg) + def copy_acteur( + self, + overriden_fields={ + "identifiant_unique": None, + "identifiant_externe": None, + "source": None, + }, + ): + from qfdmo.models import Acteur + + acteur_to_copy = Acteur.objects.get(pk=self.id) + revision_acteur_to_copy = acteur_to_copy.get_or_create_revision() + + revision_acteur_to_copy.instance_copy( + revision_acteur_to_copy, overriden_fields=overriden_fields + ) + def apply(self): self.validate() - from qfdmo.models import Acteur + from qfdmo.models import Acteur, ActeurStatus acteur_to_copy = Acteur.objects.get(pk=self.id) revision_acteur_to_copy = acteur_to_copy.get_or_create_revision() - revision_acteur_to_copy.duplicate(fields_to_reset=self.data) + + # Ensure Acteur is not active + if acteur_to_copy.statut == ActeurStatus.ACTIF: + msg = f"Copy d'acteur: l'acteur source '{self.id}' ne doit pas être actif" + raise ValueError(msg) + + revision_acteur_to_copy.instance_copy(overriden_fields=self.data) diff --git a/data/models/changes/acteur_update_statut.py b/data/models/changes/acteur_update_statut.py new file mode 100644 index 000000000..57562efea --- /dev/null +++ b/data/models/changes/acteur_update_statut.py @@ -0,0 +1,32 @@ +"""Change model to update an acteur's statut. We need a speciific change model because +the statut is updated in acteur and revisionacteur tables.""" + +from data.models.changes.acteur_abstract import ChangeActeurAbstract +from qfdmo.models import Acteur, ActeurStatus, RevisionActeur + + +class ChangeActeurUpdateStatut(ChangeActeurAbstract): + @classmethod + def name(cls) -> str: + return "acteur_update_statut" + + def validate(self): + if not self.data: + raise ValueError("No data provided") + + if "statut" not in self.data: + raise ValueError("No statut provided") + + if self.data["statut"] not in ActeurStatus.values: + raise ValueError(f"Invalid statut: {self.data['statut']}") + + def apply(self): + instances = self.validate() + + instances: list[Acteur | RevisionActeur] = [Acteur.objects.get(pk=self.id)] + if revision := RevisionActeur.objects.filter(pk=self.id).first(): + instances.append(revision) + + for instance in instances: + instance.statut = self.data["statut"] + instance.save() diff --git a/docs/explications/data/enrich/README.md b/docs/explications/data/enrich/README.md index c9b9372ff..9b79d6681 100644 --- a/docs/explications/data/enrich/README.md +++ b/docs/explications/data/enrich/README.md @@ -19,10 +19,28 @@ Plusieurs étapes : * Cloner - BAN - Adresses * Cloner - BAN - Lieux-dits 1. Préparation de la donnée (Airflow + DBT) : + * DBT - Rafraîchir les acteurs affichés * 🔄 Enrichir - Rafraîchir les modèles DBT 1. Création des suggestions (Airflow + DBT) : * 🚪 Enrichir - Acteurs Fermés +```mermaid +graph LR + A[Cloner - AE - Etablissement] + B[Cloner - AE - Unite Legale] + C[Cloner - BAN - Adresses] + D[Cloner - BAN - Lieux-dits] + E[DBT - Rafraîchir les acteurs affichés] + F[🔄 Enrichir - Rafraîchir les modèles DBT] + G[🚪 Enrichir - Acteurs Fermés] + A --> F + B --> F + C --> F + D --> F + F --> G + E --> G +``` + ## Script de cohérence ### Vérification des URLs diff --git a/qfdmo/models/acteur.py b/qfdmo/models/acteur.py index c757f0f17..3d9bcda77 100644 --- a/qfdmo/models/acteur.py +++ b/qfdmo/models/acteur.py @@ -638,6 +638,41 @@ def commentaires_ajouter(self, added): self.save() + def instance_copy( + self, + overriden_fields={ + "identifiant_unique": None, + "identifiant_externe": None, + "source": None, + }, + ): + if isinstance(self, RevisionActeur) and self.is_parent: + raise Exception("Impossible de dupliquer un acteur parent") + + if isinstance(self, RevisionActeur): + acteur = Acteur.objects.get(identifiant_unique=self.identifiant_unique) + acteur.instance_copy(overriden_fields=overriden_fields) + + new_instance = deepcopy(self) + + for field, value in overriden_fields.items(): + setattr(new_instance, field, value) + + new_instance.save() + new_instance.labels.set(self.labels.all()) + new_instance.acteur_services.set(self.acteur_services.all()) + + # recreate proposition_services for the new revision_acteur + for proposition_service in self.proposition_services.all(): + new_proposition_service = proposition_service.__class__.objects.create( + acteur=new_instance, + action=proposition_service.action, + ) + new_proposition_service.sous_categories.set( + proposition_service.sous_categories.all() + ) + return new_instance + def clean_parent(parent): try: @@ -933,11 +968,9 @@ def duplicate( # recreate proposition_services for the new revision_acteur for proposition_service in self.proposition_services.all(): - revision_proposition_service = revision_proposition_service = ( - RevisionPropositionService.objects.create( - acteur=revision_acteur, - action=proposition_service.action, - ) + revision_proposition_service = RevisionPropositionService.objects.create( + acteur=revision_acteur, + action=proposition_service.action, ) revision_proposition_service.sous_categories.set( proposition_service.sous_categories.all() From ec89a47ec5c72564b219102074f027cf4d0fe9ff Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Mon, 12 May 2025 18:27:48 +0200 Subject: [PATCH 06/12] update tests --- .../enrich_dbt_model_to_suggestions.py | 2 +- .../test_cluster_acteurs_clusters.py | 3 - ...est_cluster_acteurs_parents_choose_data.py | 9 +- ...test_cluster_acteurs_parents_choose_new.py | 3 - .../test_cluster_acteurs_read_orphans.py | 4 - .../test_e2e_dag_cluster_acteurs_airflow.py | 3 - .../tasks/test_enrich_suggestions_closed.py | 75 +++++---- .../test_packages_compatibility.py | 2 +- .../tasks/transform/test_transform_df.py | 1 - .../utils/test_data_serialize_reconstruct.py | 4 +- dags_unit_tests/utils/test_django.py | 4 +- data/models/changes/acteur_update_statut.py | 7 + .../changes/test_acteur_create_as_copy.py | 94 +++++++++++ .../changes/test_acteur_update_statut.py | 150 ++++++++++++++++++ 14 files changed, 300 insertions(+), 61 deletions(-) create mode 100644 unit_tests/data/models/changes/test_acteur_create_as_copy.py create mode 100644 unit_tests/data/models/changes/test_acteur_update_statut.py diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 717d69608..9d5b8c24e 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -139,7 +139,6 @@ def changes_prepare_closed_replaced( old_acteur = { "id": row[COLS.ACTEUR_ID], "data": { - "identifiant_unique": row[COLS.ACTEUR_ID], "siret_is_closed": True, "statut": ActeurStatus.INACTIF, }, @@ -164,6 +163,7 @@ def changes_prepare_closed_replaced( "statut": ActeurStatus.ACTIF, "siret": row[COLS.SUGGEST_SIRET], "siren": row[COLS.SUGGEST_SIRET][:9], + "siret_is_closed": False, "parent_reason": ( # FIXME : renommer parent_reason "Nouvelle version de l'acteur conservées suite aux modifications: " f"SIRET {row[COLS.ACTEUR_SIRET]} " diff --git a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_clusters.py b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_clusters.py index 5fac44131..72a34031f 100644 --- a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_clusters.py +++ b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_clusters.py @@ -7,7 +7,6 @@ score_tuples_to_clusters, similarity_matrix_to_tuples, ) -from rich import print def df_clusters_to_dict(df: pd.DataFrame) -> dict[str, list[str]]: @@ -169,11 +168,9 @@ def test_cols_group_fuzzy_single(self, df_cols_group_fuzzy): cluster_fields_fuzzy=["nom"], cluster_fuzzy_threshold=0.7, ) - print(df_clusters.to_dict(orient="records")) assert df_clusters["cluster_id"].nunique() == 3 assert len(df_clusters) == 6 clusters = df_clusters_to_dict(df_clusters) - print(clusters) assert clusters == { "10000_nom_3": [ "id0", # "centre commercial auchan" diff --git a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_data.py b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_data.py index c27b8d9fb..362fad4ba 100644 --- a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_data.py +++ b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_data.py @@ -2,8 +2,6 @@ import pandas as pd import pytest from django.contrib.gis.geos import Point -from rich import print -from utils.django import django_setup_full from dags.cluster.config.constants import COL_PARENT_DATA_NEW from dags.cluster.tasks.business_logic.cluster_acteurs_parents_choose_data import ( @@ -11,6 +9,7 @@ field_pick_value, parent_choose_data, ) +from utils.django import django_setup_full django_setup_full() @@ -209,9 +208,7 @@ def df_clusters_parent_create(self, parent, acteurs_revision, acteurs_base): ChangeActeurUpdateParentId.name() ) df["cluster_id"] = "c1" - print("df_clusters_parent_create before drops", df.to_dict(orient="records")) df = df.drop_duplicates(subset=["identifiant_unique"], keep="first") - print("df_clusters_parent_create", df.to_dict(orient="records")) return df @pytest.fixture @@ -261,7 +258,6 @@ def test_cluster_acteurs_parents_choose_data( df_before = dfs[scenario] DisplayedActeur(**parent).save() - print("BEFORE", df_before.to_dict(orient="records")) df_after = cluster_acteurs_parents_choose_data( df_clusters=df_before, fields_to_include=["nom", "siret", "email"], @@ -269,7 +265,6 @@ def test_cluster_acteurs_parents_choose_data( prioritize_source_ids=[10, 20], keep_empty=EMPTY_IGNORE, ) - print("AFTER", df_after.to_dict(orient="records")) filter_parent = df_after["identifiant_unique"] == "p1" parent_data = df_after[filter_parent][COL_PARENT_DATA_NEW].values[0] df_children = df_after[~filter_parent] @@ -293,7 +288,6 @@ def test_parent_create( df_before = df_clusters_parent_create DisplayedActeur(**parent).save() - print("BEFORE", df_before.to_dict(orient="records")) df_after = cluster_acteurs_parents_choose_data( df_clusters=df_before, fields_to_include=["nom", "siret", "email"], @@ -301,7 +295,6 @@ def test_parent_create( prioritize_source_ids=[10, 20], keep_empty=EMPTY_IGNORE, ) - print("AFTER", df_after.to_dict(orient="records")) filter_parent = df_after["identifiant_unique"] == "p1" parent_data = df_after[filter_parent][COL_PARENT_DATA_NEW].values[0] df_children = df_after[~filter_parent] diff --git a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_new.py b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_new.py index 15e74c22b..248946f1b 100755 --- a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_new.py +++ b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_parents_choose_new.py @@ -22,7 +22,6 @@ cluster_acteurs_parents_choose_new, parent_id_generate, ) -from rich import print from data.models.change import ( COL_CHANGE_ENTITY_TYPE, @@ -150,7 +149,6 @@ def test_case_no_parent(self, df_no_parent, parent_id_new): ], ignore_index=True, ) - print(f"{df=}") df = cluster_acteurs_one_cluster_parent_changes_mark( df, parent_id_new, CHANGE_CREATE, "Nouveau parent" ) @@ -259,7 +257,6 @@ def test_working_overall_changes(self, df_working, parent_id_new, df_combined): # On veut aucune valeur nan résultant des divers manipulations # On ne peut pas juste faire df.isna().any().any() car isna # inclut les None qu'on tolère - print(df_working.to_dict(orient="records")) df_nan = df_working.map(lambda x: 1 if pd.isna(x) and x is not None else 0) assert df_nan.values.sum() == 0 diff --git a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_read_orphans.py b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_read_orphans.py index 8a646e88d..0722b111c 100644 --- a/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_read_orphans.py +++ b/dags_unit_tests/cluster/tasks/business_logic/test_cluster_acteurs_read_orphans.py @@ -160,10 +160,6 @@ def df_ideal(self, ideal_scenario_apply_function): """La dataframe correspondant au cas idéal de fonctionnement de la fonction""" df = ideal_scenario_apply_function[0] - # Pour faciliter le debug si besoin, bcp plus simple - # d'utiliser les dicts que du print df potentiellement - # tronqué - # print("contenu df_ideal", df.to_dict(orient="records")) return df @pytest.fixture diff --git a/dags_unit_tests/e2e/test_e2e_dag_cluster_acteurs_airflow.py b/dags_unit_tests/e2e/test_e2e_dag_cluster_acteurs_airflow.py index 71c455590..720fe7b81 100644 --- a/dags_unit_tests/e2e/test_e2e_dag_cluster_acteurs_airflow.py +++ b/dags_unit_tests/e2e/test_e2e_dag_cluster_acteurs_airflow.py @@ -4,7 +4,6 @@ import pytest from airflow.utils.state import State from django.contrib.gis.geos import Point -from rich import print from dags.cluster.config.tasks import TASKS from dags.shared.config import START_DATES @@ -132,8 +131,6 @@ def test_up_to_selection_and_normalize(self, db_sources_acteur_types, conf): dag.test(execution_date=START_DATES.YESTERDAY, run_conf=conf) tis = dag.get_task_instances() - for ti in tis: - print(f"{ti.task_id}", f"{ti.state=}") # Tasks which should have completed successfully assert ti_get(tis, TASKS.CONFIG_CREATE).state == State.SUCCESS diff --git a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py index b94efbe9e..da194e00f 100644 --- a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py +++ b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py @@ -10,6 +10,8 @@ enrich_dbt_model_to_suggestions, ) +from qfdmo.models.acteur import Acteur + TODAY = datetime.now(timezone.utc).strftime("%Y-%m-%d") @@ -140,9 +142,6 @@ def acteurs(self, df_not_replaced, df_replaced, atype, source): assert acteur.nom == f"AVANT {row[COLS.ACTEUR_ID]}" assert acteur.acteur_type == atype assert acteur.source == source - # print(f"{acteur.location=}") - # assert acteur.location.x == row[COLS.ACTEUR_LONGITUDE] - # assert acteur.location.y == row[COLS.ACTEUR_LATITUDE] def test_cohorte_not_replaced(self, acteurs, df_not_replaced): from data.models.suggestion import Suggestion, SuggestionCohorte @@ -189,32 +188,40 @@ def check_replaced_acteur( from qfdmo.models import ActeurStatus, RevisionActeur # Closed acteur - closed = RevisionActeur.objects.get(pk=id) - assert closed.statut == ActeurStatus.INACTIF - assert closed.siret_is_closed is True - # for closed acteur we don't revision the parent data - assert closed.parent is None - assert closed.parent_reason == "" + a_closed = Acteur.objects.get(pk=id) + ra_closed = RevisionActeur.objects.get(pk=id) + assert a_closed.statut == ActeurStatus.INACTIF + assert ra_closed.statut == ActeurStatus.INACTIF + assert ra_closed.siret_is_closed or a_closed.siret_is_closed # The new acteur which about from siret_is_closed AND # identifiant_externe has the same data as the closed acteur - new = RevisionActeur.objects.filter(identifiant_externe=id_ext).first() - assert new is not None + a_new = Acteur.objects.filter( + identifiant_externe=id_ext, statut=ActeurStatus.ACTIF + ).first() + ra_new = RevisionActeur.objects.filter( + identifiant_unique=a_new.identifiant_unique + ).first() + assert a_new is not None + assert ra_new is not None # ID is concatenation of closed acteur ID, new siret and today's datetime assert re.search( - f"^{id}_{siret_new}_{TODAY}[0-9]{{4}}$", new.identifiant_unique + f"^{id}_{TODAY.replace('-', '')}[0-9]{{6}}$", a_new.identifiant_unique ) - assert new.statut == ActeurStatus.ACTIF - assert new.siret == siret_new - assert new.siret_is_closed is None # we have detected no closure yet + assert a_new.statut == ActeurStatus.ACTIF + assert ra_new.statut == ActeurStatus.ACTIF + assert a_new.siret == siret_new + assert ra_new.siret == siret_new + assert a_new.siret_is_closed is False # we have detected no closure yet + assert ra_new.siret_is_closed is False # we have detected no closure yet # foreign keys are iso with the closed acteur - assert new.parent.pk == parent.pk - assert new.source.pk == source.pk # type: ignore - assert new.acteur_type.pk == acteur_type.pk - assert new.identifiant_externe == id_ext + assert a_new.source.pk == source.pk # type: ignore + assert a_new.acteur_type.pk == acteur_type.pk + assert a_new.identifiant_externe == id_ext # Explanation as to why - assert new.parent_reason == parent_reason + print(f"{parent_reason} === {ra_new.parent_reason}") + assert ra_new.parent_reason == parent_reason def test_cohorte_meme_siren( self, acteurs, parent, atype, source, df_replaced_meme_siret @@ -246,10 +253,9 @@ def test_cohorte_meme_siren( siret_new="11111111100002", parent=parent, parent_reason=( - f"Nouvel enfant pour conserver les données suite à: " - f"SIRET 11111111100001 " - f"détecté le {TODAY} comme fermé dans AE, " - f"remplacé par SIRET 11111111100002" + "Nouvelle version de l'acteur conservées suite aux modifications:" + " SIRET 11111111100001 détecté le 2025-05-12 comme fermé dans AE," + " remplacé par SIRET 11111111100002" ), source=source, acteur_type=atype, @@ -271,7 +277,9 @@ def test_cohorte_autre_siren( # Check suggestions have been written to DB cohort = SuggestionCohorte.objects.get(identifiant_action="test_autre_siren") suggestions = Suggestion.objects.filter(suggestion_cohorte=cohort) - assert len(suggestions) == 1 # 1 suggestion containing 2 changes (closed + new) + + # 2 suggestions containing 2 changes (inactive + new) + assert len(suggestions) == 2 # Apply suggestions for suggestion in suggestions: suggestion.apply() @@ -281,13 +289,12 @@ def test_cohorte_autre_siren( id="a2", id_ext="ext_a2", siret_closed="22222222200001", - siret_new="44444444400001", + siret_new="33333333300001", parent=parent, parent_reason=( - f"Nouvel enfant pour conserver les données suite à: " - f"SIRET 22222222200001 " - f"détecté le {TODAY} comme fermé dans AE, " - f"remplacé par SIRET 44444444400001" + "Nouvelle version de l'acteur conservées suite aux modifications:" + " SIRET 22222222200001 détecté le 2025-05-12 comme fermé dans AE," + " remplacé par SIRET 33333333300001" ), source=source, acteur_type=atype, @@ -296,11 +303,15 @@ def test_cohorte_autre_siren( self.check_replaced_acteur( id="a3", id_ext="ext_a3", - siret_closed="33333333300001", + siret_closed="44444444400001", siret_new="55555555500001", parent=parent, # This closed acteur has no parent so there should be no parent_reason - parent_reason="", + parent_reason=( + "Nouvelle version de l'acteur conservées suite aux modifications:" + " SIRET 44444444400001 détecté le 2025-05-12 comme fermé dans AE," + " remplacé par SIRET 55555555500001" + ), source=source, acteur_type=atype, ) diff --git a/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py b/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py index 030d9bcfb..459a35fe7 100644 --- a/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py +++ b/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py @@ -26,7 +26,7 @@ def test_pandas_read_sql_table(): - mettre à jour ce teste pour qu'il fonctionne - mettre à jour les codes DAGs pour qu'ils fonctionnent également """ - print(f"Pandas version: {pd.__version__}") + # print(f"Pandas version: {pd.__version__}") engine = create_engine("sqlite:///:memory:") engine.execute("CREATE TABLE my_table (id INT, name TEXT)") pd.read_sql_table("my_table", engine) diff --git a/dags_unit_tests/sources/tasks/transform/test_transform_df.py b/dags_unit_tests/sources/tasks/transform/test_transform_df.py index 5570a89a0..874a624a2 100644 --- a/dags_unit_tests/sources/tasks/transform/test_transform_df.py +++ b/dags_unit_tests/sources/tasks/transform/test_transform_df.py @@ -790,7 +790,6 @@ def test_compute_location(self, latitude, longitude, expected_location): result = compute_location( pd.Series({"latitude": latitude, "longitude": longitude}), None ) - print(result["location"]) assert result["location"] == expected_location diff --git a/dags_unit_tests/utils/test_data_serialize_reconstruct.py b/dags_unit_tests/utils/test_data_serialize_reconstruct.py index dc3af503a..478719e6b 100644 --- a/dags_unit_tests/utils/test_data_serialize_reconstruct.py +++ b/dags_unit_tests/utils/test_data_serialize_reconstruct.py @@ -2,8 +2,6 @@ import pytest from django.contrib.gis.geos import Point -from rich import print -from utils.data_serialize_reconstruct import data_serialize from data.models.changes.utils import data_reconstruct from qfdmo.models.acteur import RevisionActeur @@ -12,6 +10,7 @@ ActionFactory, SourceFactory, ) +from utils.data_serialize_reconstruct import data_serialize DATETIME = datetime(2023, 10, 1, 14, 30, 4) POINT = Point(1, 2) @@ -53,7 +52,6 @@ def test_data_reconstructed(self, data_reconstructed): assert isinstance(data["cree_le"], str) def test_data_reconstructed_compatible_with_model(self, data_reconstructed): - print("test_data_is_compatible", data_reconstructed) rev = RevisionActeur(**data_reconstructed) rev.save() # FIXME: setting cree_le doesn't work the 1st time due diff --git a/dags_unit_tests/utils/test_django.py b/dags_unit_tests/utils/test_django.py index 58ee69816..9124eff72 100644 --- a/dags_unit_tests/utils/test_django.py +++ b/dags_unit_tests/utils/test_django.py @@ -1,5 +1,6 @@ import pandas as pd import pytest + from utils.django import ( django_model_fields_get, django_model_queryset_generate, @@ -59,7 +60,7 @@ def test_django_model_queryset_generate(): ) # Convert queryset to SQL string - sql = django_model_queryset_to_sql(queryset) + django_model_queryset_to_sql(queryset) # Expected SQL string (adjust to match your backend, structure may vary slightly) expected_sql = r""" @@ -69,7 +70,6 @@ def test_django_model_queryset_generate(): AND NOT ("adresse" IS NULL) AND NOT ("adresse" = '')) AND NOT (("siret" IS NOT NULL) OR ("siret" != '')) """ - print(sql) # TODO: activer ce test une fois qu'on est content # avec le résultat final via Airflow # assert sql.strip() == expected_sql.strip() diff --git a/data/models/changes/acteur_update_statut.py b/data/models/changes/acteur_update_statut.py index 57562efea..5485ad8ac 100644 --- a/data/models/changes/acteur_update_statut.py +++ b/data/models/changes/acteur_update_statut.py @@ -17,6 +17,11 @@ def validate(self): if "statut" not in self.data: raise ValueError("No statut provided") + if self.data.keys() - {"statut", "siret_is_closed"}: + raise ValueError( + "Invalid data, only statut and siret_is_closed are allowed" + ) + if self.data["statut"] not in ActeurStatus.values: raise ValueError(f"Invalid statut: {self.data['statut']}") @@ -29,4 +34,6 @@ def apply(self): for instance in instances: instance.statut = self.data["statut"] + if "siret_is_closed" in self.data: + instance.siret_is_closed = self.data["siret_is_closed"] instance.save() diff --git a/unit_tests/data/models/changes/test_acteur_create_as_copy.py b/unit_tests/data/models/changes/test_acteur_create_as_copy.py new file mode 100644 index 000000000..1de94e78a --- /dev/null +++ b/unit_tests/data/models/changes/test_acteur_create_as_copy.py @@ -0,0 +1,94 @@ +from unittest.mock import patch + +import pytest + +from data.models.changes.acteur_create_as_copy import ChangeActeurCreateAsCopy +from qfdmo.models import Acteur, ActeurStatus, RevisionActeur +from unit_tests.qfdmo.acteur_factory import ActeurFactory, PropositionServiceFactory + + +@pytest.mark.django_db +class TestChangeActeurCreateAsCopy: + def test_model_name(self): + assert ChangeActeurCreateAsCopy.name() == "acteur_create_as_copy" + + def test_raise_if_acteur_does_not_exist(self): + change = ChangeActeurCreateAsCopy( + id="dummy", data={"identifiant_unique": "new_id"} + ) + with pytest.raises(ValueError): + change.validate() + + def test_raise_if_no_identifiant_unique_provided(self): + acteur = ActeurFactory() + change = ChangeActeurCreateAsCopy(id=acteur.pk, data={}) + with pytest.raises( + ValueError, match="l'acteur cible doit surdefinir son identifiant_unique" + ): + change.validate() + + def test_raise_if_same_identifiant_unique(self): + acteur = ActeurFactory() + change = ChangeActeurCreateAsCopy( + id=acteur.pk, data={"identifiant_unique": acteur.identifiant_unique} + ) + with pytest.raises( + ValueError, + match="l'acteur cible doit avoir un identifiant_unique différent", + ): + change.validate() + + @patch.object(ChangeActeurCreateAsCopy, "validate") + def test_validate_is_called_by_apply(self, mock_validate): + acteur = ActeurFactory(statut=ActeurStatus.INACTIF) + change = ChangeActeurCreateAsCopy( + id=acteur.pk, data={"identifiant_unique": "new_id"} + ) + change.apply() + mock_validate.assert_called_once() + + def test_raise_if_source_acteur_is_active(self): + acteur = ActeurFactory(statut=ActeurStatus.ACTIF) + change = ChangeActeurCreateAsCopy( + id=acteur.pk, data={"identifiant_unique": "new_id"} + ) + with pytest.raises(ValueError, match="ne doit pas être actif"): + change.apply() + + def test_working_case(self): + # Création d'un acteur inactif avec des données de base + acteur_to_copy = ActeurFactory( + nom="test", + statut=ActeurStatus.INACTIF, + ) + proposition = PropositionServiceFactory() + acteur_to_copy.proposition_services.add(proposition) + + change = ChangeActeurCreateAsCopy( + id=acteur_to_copy.pk, + data={"identifiant_unique": "new_id", "statut": ActeurStatus.ACTIF}, + ) + change.apply() + + assert acteur_to_copy.statut == ActeurStatus.INACTIF + + # Revision exists + revision_acteur_to_copy = RevisionActeur.objects.get(pk=acteur_to_copy.pk) + assert revision_acteur_to_copy is not None + assert revision_acteur_to_copy.proposition_services.count() == 1 + assert revision_acteur_to_copy.statut == ActeurStatus.INACTIF + + # New acteur created + new_acteur = Acteur.objects.get(identifiant_unique="new_id") + assert new_acteur.nom == "test" + assert new_acteur.acteur_type == acteur_to_copy.acteur_type + assert new_acteur.source == acteur_to_copy.source + assert new_acteur.location == acteur_to_copy.location + assert new_acteur.statut == ActeurStatus.ACTIF + + assert new_acteur.proposition_services.count() == 1 + + new_revision_acteur = RevisionActeur.objects.get(pk=new_acteur.pk) + assert new_revision_acteur is not None + assert new_revision_acteur.statut == ActeurStatus.ACTIF + assert new_revision_acteur.proposition_services.count() == 1 diff --git a/unit_tests/data/models/changes/test_acteur_update_statut.py b/unit_tests/data/models/changes/test_acteur_update_statut.py new file mode 100644 index 000000000..706275ff1 --- /dev/null +++ b/unit_tests/data/models/changes/test_acteur_update_statut.py @@ -0,0 +1,150 @@ +from unittest.mock import patch + +import pytest + +from data.models.changes.acteur_update_statut import ChangeActeurUpdateStatut +from qfdmo.models import RevisionActeur +from unit_tests.qfdmo.acteur_factory import ActeurFactory + + +@pytest.mark.django_db +class TestChangeActeurUpdateStatut: + def test_model_name(self): + assert ChangeActeurUpdateStatut.name() == "acteur_update_statut" + + def test_raise_if_no_data_provided(self): + change = ChangeActeurUpdateStatut(id="dummy", data={}) + with pytest.raises(ValueError, match="No data provided"): + change.validate() + + def test_raise_if_no_statut_provided(self): + change = ChangeActeurUpdateStatut(id="dummy", data={"siret_is_closed": True}) + with pytest.raises(ValueError, match="No statut provided"): + change.validate() + + def test_raise_if_invalid_statut_provided(self): + change = ChangeActeurUpdateStatut(id="dummy", data={"statut": "not_active"}) + with pytest.raises(ValueError, match="Invalid statut"): + change.validate() + + def test_raise_if_column_not_allowed(self): + change = ChangeActeurUpdateStatut( + id="dummy", data={"statut": "ACTIF", "fake": "boo"} + ) + with pytest.raises( + ValueError, + match="Invalid data, only statut and siret_is_closed are allowed", + ): + change.validate() + + @patch.object(ChangeActeurUpdateStatut, "validate") + def test_validate_is_called_by_apply(self, mock_validate): + acteur = ActeurFactory() + change = ChangeActeurUpdateStatut(id=acteur.pk, data={"statut": "INACTIF"}) + change.apply() + mock_validate.assert_called_once() + + def test_with_acteur(self): + acteur = ActeurFactory() + change = ChangeActeurUpdateStatut( + id=acteur.pk, data={"statut": "INACTIF", "siret_is_closed": True} + ) + + change.apply() + acteur.refresh_from_db() + + assert acteur.statut == "INACTIF" + assert acteur.siret_is_closed is True + + revision_acteur = RevisionActeur.objects.filter(pk=acteur.pk).first() + assert revision_acteur is None + + def test_with_revision_acteur(self): + acteur = ActeurFactory() + revision_acteur = acteur.get_or_create_revision() + change = ChangeActeurUpdateStatut( + id=revision_acteur.pk, data={"statut": "INACTIF", "siret_is_closed": True} + ) + + change.apply() + acteur.refresh_from_db() + revision_acteur.refresh_from_db() + + assert acteur.statut == "INACTIF" + assert acteur.siret_is_closed is True + assert revision_acteur.statut == "INACTIF" + assert revision_acteur.siret_is_closed is True + + # def test_raise_if_no_identifiant_unique_provided(self): + # acteur = ActeurFactory() + # change = ChangeActeurCreateAsCopy(id=acteur.pk, data={}) + # with pytest.raises( + # ValueError, match="l'acteur cible doit surdefinir son identifiant_unique" + # ): + # change.validate() + + # def test_raise_if_same_identifiant_unique(self): + # acteur = ActeurFactory() + # change = ChangeActeurCreateAsCopy( + # id=acteur.pk, data={"identifiant_unique": acteur.identifiant_unique} + # ) + # with pytest.raises( + # ValueError, + # match="l'acteur cible doit avoir un identifiant_unique différent", + # ): + # change.validate() + + # @patch.object(ChangeActeurCreateAsCopy, "validate") + # def test_validate_is_called_by_apply(self, mock_validate): + # acteur = ActeurFactory(statut=ActeurStatus.INACTIF) + # change = ChangeActeurCreateAsCopy( + # id=acteur.pk, data={"identifiant_unique": "new_id"} + # ) + # change.apply() + # mock_validate.assert_called_once() + + # def test_raise_if_source_acteur_is_active(self): + # acteur = ActeurFactory(statut=ActeurStatus.ACTIF) + # change = ChangeActeurCreateAsCopy( + # id=acteur.pk, data={"identifiant_unique": "new_id"} + # ) + # with pytest.raises(ValueError, match="ne doit pas être actif"): + # change.apply() + + # def test_working_case(self): + # # Création d'un acteur inactif avec des données de base + # acteur_to_copy = ActeurFactory( + # nom="test", + # statut=ActeurStatus.INACTIF, + # ) + # proposition = PropositionServiceFactory() + # acteur_to_copy.proposition_services.add(proposition) + + # change = ChangeActeurCreateAsCopy( + # id=acteur_to_copy.pk, + # data={"identifiant_unique": "new_id", "statut": ActeurStatus.ACTIF}, + # ) + # change.apply() + + # assert acteur_to_copy.statut == ActeurStatus.INACTIF + + # # Revision exists + # revision_acteur_to_copy = RevisionActeur.objects.get(pk=acteur_to_copy.pk) + # assert revision_acteur_to_copy is not None + # assert revision_acteur_to_copy.proposition_services.count() == 1 + # assert revision_acteur_to_copy.statut == ActeurStatus.INACTIF + + # # New acteur created + # new_acteur = Acteur.objects.get(identifiant_unique="new_id") + # assert new_acteur.nom == "test" + # assert new_acteur.acteur_type == acteur_to_copy.acteur_type + # assert new_acteur.source == acteur_to_copy.source + # assert new_acteur.location == acteur_to_copy.location + # assert new_acteur.statut == ActeurStatus.ACTIF + + # assert new_acteur.proposition_services.count() == 1 + + # new_revision_acteur = RevisionActeur.objects.get(pk=new_acteur.pk) + # assert new_revision_acteur is not None + # assert new_revision_acteur.statut == ActeurStatus.ACTIF + # assert new_revision_acteur.proposition_services.count() == 1 From 57957cd47e3e7b29d740369b2166e6839a1e0a66 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Wed, 14 May 2025 10:35:55 +0200 Subject: [PATCH 07/12] =?UTF-8?q?Mises=20=C3=A0=20jour=20apr=C3=A8s=20revi?= =?UTF-8?q?ew?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Makefile | 8 +- Procfile.dev => Procfile.django.dev | 0 dags/acteurs/dags/compute_acteur.py | 52 ++++--------- dags/enrich/config/dbt.py | 3 - dags/enrich/dags/enrich_dbt_models_refresh.py | 4 +- .../enrich_dbt_model_to_suggestions.py | 8 +- .../tasks/test_enrich_suggestions_closed.py | 15 ++-- .../test_packages_compatibility.py | 1 - data/models/changes/acteur_create_as_copy.py | 17 ----- data/models/changes/acteur_update_data.py | 1 - .../changes/test_acteur_update_statut.py | 74 ------------------- 11 files changed, 35 insertions(+), 148 deletions(-) rename Procfile.dev => Procfile.django.dev (100%) diff --git a/Makefile b/Makefile index 4a88d4bd2..8e55140df 100644 --- a/Makefile +++ b/Makefile @@ -57,11 +57,13 @@ run-airflow: .PHONY: run-django run-django: - docker compose --profile lvao up -d - honcho start -f Procfile.dev + docker compose --profile airflow down + docker compose --profile lvao down + honcho start -f Procfile.django.dev run-all: - docker compose --profile airflow up -d + docker compose --profile airflow down + docker compose --profile lvao down honcho start -f Procfile.all.dev # Local django operations diff --git a/Procfile.dev b/Procfile.django.dev similarity index 100% rename from Procfile.dev rename to Procfile.django.dev diff --git a/dags/acteurs/dags/compute_acteur.py b/dags/acteurs/dags/compute_acteur.py index fbdbe4b4f..b3a5685db 100755 --- a/dags/acteurs/dags/compute_acteur.py +++ b/dags/acteurs/dags/compute_acteur.py @@ -36,90 +36,70 @@ ) as dag: dbt_run_base_acteurs = BashOperator( task_id="dbt_run_base_acteurs", - bash_command=("cd /opt/airflow/dbt/ && dbt run --models base.acteurs"), + bash_command=("dbt run --models base.acteurs"), ) dbt_test_base_acteurs = BashOperator( task_id="dbt_test_base_acteurs", - bash_command=("cd /opt/airflow/dbt/ && dbt test --models base.acteurs"), + bash_command=("dbt test --models base.acteurs"), ) dbt_run_intermediate_acteurs = BashOperator( task_id="dbt_run_intermediate_acteurs", - bash_command=("cd /opt/airflow/dbt/ && dbt run --models intermediate.acteurs"), + bash_command=("dbt run --models intermediate.acteurs"), ) dbt_test_intermediate_acteurs = BashOperator( task_id="dbt_test_intermediate_acteurs", - bash_command=("cd /opt/airflow/dbt/ && dbt test --models intermediate.acteurs"), + bash_command=("dbt test --models intermediate.acteurs"), ) dbt_run_marts_acteurs_exhaustive = BashOperator( task_id="dbt_run_marts_acteurs_exhaustive", - bash_command=( - "cd /opt/airflow/dbt/ && dbt run --models marts.acteurs.exhaustive" - ), + bash_command=("dbt run --models marts.acteurs.exhaustive"), ) dbt_test_marts_acteurs_exhaustive = BashOperator( task_id="dbt_test_marts_acteurs_exhaustive", - bash_command=( - "cd /opt/airflow/dbt/ && dbt test --models marts.acteurs.exhaustive" - ), + bash_command=("dbt test --models marts.acteurs.exhaustive"), ) dbt_run_exposure_acteurs_exhaustive = BashOperator( task_id="dbt_run_exposure_acteurs_exhaustive", - bash_command=( - "cd /opt/airflow/dbt/ && dbt run --models exposure.acteurs.exhaustive" - ), + bash_command=("dbt run --models exposure.acteurs.exhaustive"), ) dbt_test_exposure_acteurs_exhaustive = BashOperator( task_id="dbt_test_exposure_acteurs_exhaustive", - bash_command=( - "cd /opt/airflow/dbt/ && dbt test --models exposure.acteurs.exhaustive" - ), + bash_command=("dbt test --models exposure.acteurs.exhaustive"), ) dbt_run_marts_acteurs_carte = BashOperator( task_id="dbt_run_marts_acteurs_carte", - bash_command=("cd /opt/airflow/dbt/ && dbt run --models marts.acteurs.carte"), + bash_command=("dbt run --models marts.acteurs.carte"), ) dbt_test_marts_acteurs_carte = BashOperator( task_id="dbt_test_marts_acteurs_carte", - bash_command=("cd /opt/airflow/dbt/ && dbt test --models marts.acteurs.carte"), + bash_command=("dbt test --models marts.acteurs.carte"), ) dbt_run_exposure_acteurs_carte = BashOperator( task_id="dbt_run_exposure_acteurs_carte", - bash_command=( - "cd /opt/airflow/dbt/ && dbt run --models exposure.acteurs.carte" - ), + bash_command=("dbt run --models exposure.acteurs.carte"), ) dbt_test_exposure_acteurs_carte = BashOperator( task_id="dbt_test_exposure_acteurs_carte", - bash_command=( - "cd /opt/airflow/dbt/ && dbt test --models exposure.acteurs.carte" - ), + bash_command=("dbt test --models exposure.acteurs.carte"), ) dbt_run_marts_acteurs_opendata = BashOperator( task_id="dbt_run_marts_acteurs_opendata", - bash_command=( - "cd /opt/airflow/dbt/ && dbt run --models marts.acteurs.opendata" - ), + bash_command=("dbt run --models marts.acteurs.opendata"), ) dbt_test_marts_acteurs_opendata = BashOperator( task_id="dbt_test_marts_acteurs_opendata", - bash_command=( - "cd /opt/airflow/dbt/ && dbt test --models marts.acteurs.opendata" - ), + bash_command=("dbt test --models marts.acteurs.opendata"), ) dbt_run_exposure_acteurs_opendata = BashOperator( task_id="dbt_run_exposure_acteurs_opendata", - bash_command=( - "cd /opt/airflow/dbt/ && dbt run --models exposure.acteurs.opendata" - ), + bash_command=("dbt run --models exposure.acteurs.opendata"), ) dbt_test_exposure_acteurs_opendata = BashOperator( task_id="dbt_test_exposure_acteurs_opendata", - bash_command=( - "cd /opt/airflow/dbt/ && dbt test --models exposure.acteurs.opendata" - ), + bash_command=("dbt test --models exposure.acteurs.opendata"), ) check_model_table_displayedacteur_task = check_model_table_consistency_task( diff --git a/dags/enrich/config/dbt.py b/dags/enrich/config/dbt.py index e1f00d2c6..ab7f09ef1 100644 --- a/dags/enrich/config/dbt.py +++ b/dags/enrich/config/dbt.py @@ -10,15 +10,12 @@ class DBT: # Closed MARTS_ENRICH_AE_CLOSED_CANDIDATES: str = "marts_enrich_acteurs_closed_candidates" - # replace SIRET only MARTS_ENRICH_AE_CLOSED_REPLACED_SAME_SIREN: str = ( "marts_enrich_acteurs_closed_suggest_replaced_same_siren" ) - # replace SIREN and SIRET MARTS_ENRICH_AE_CLOSED_REPLACED_OTHER_SIREN: str = ( "marts_enrich_acteurs_closed_suggest_replaced_other_siren" ) - # inactive acteur, no new SIRET / SIREN found MARTS_ENRICH_AE_CLOSED_NOT_REPLACED: str = ( "marts_enrich_acteurs_closed_suggest_not_replaced" ) diff --git a/dags/enrich/dags/enrich_dbt_models_refresh.py b/dags/enrich/dags/enrich_dbt_models_refresh.py index dae0f663e..6394972a9 100644 --- a/dags/enrich/dags/enrich_dbt_models_refresh.py +++ b/dags/enrich/dags/enrich_dbt_models_refresh.py @@ -42,11 +42,9 @@ tasks = [] for cmd in dag.params.get("dbt_models_refresh_commands", []): cmd = cmd.strip() - cmd_id = re.sub(r"__+", "_", re.sub(r"[^a-zA-Z0-9]+", "_", cmd)) - cmd = "cd /opt/airflow/dbt/ && " + cmd if not cmd: continue - # cmd += " --debug --threads 1" + cmd_id = re.sub(r"__+", "_", re.sub(r"[^a-zA-Z0-9]+", "_", cmd)) logger.info(f"Create bash operator with command: {cmd}") tasks.append( BashOperator( diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 9d5b8c24e..5192af17e 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -164,8 +164,8 @@ def changes_prepare_closed_replaced( "siret": row[COLS.SUGGEST_SIRET], "siren": row[COLS.SUGGEST_SIRET][:9], "siret_is_closed": False, - "parent_reason": ( # FIXME : renommer parent_reason - "Nouvelle version de l'acteur conservées suite aux modifications: " + "parent_reason": ( + "Nouvelle version de l'acteur conservée suite aux modifications: " f"SIRET {row[COLS.ACTEUR_SIRET]} " f"détecté le {today} comme fermé dans AE, " f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" @@ -238,9 +238,9 @@ def enrich_dbt_model_to_suggestions( row = dict(row) try: - interpret_function = COHORTS_TO_PREPARE_CHANGES[cohort] logger.info(f"Interprétation de {cohort=}") - changes, contexte = interpret_function(row) + change_preparation_function = COHORTS_TO_PREPARE_CHANGES[cohort] + changes, contexte = change_preparation_function(row) suggestion = { "contexte": contexte, "suggestion": {"title": cohort, "changes": changes}, diff --git a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py index da194e00f..89c30cf38 100644 --- a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py +++ b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py @@ -246,6 +246,7 @@ def test_cohorte_meme_siren( suggestion.apply() # Closed acteur + today = datetime.now(timezone.utc).strftime("%Y-%m-%d") self.check_replaced_acteur( id="a1", id_ext="ext_a1", @@ -253,8 +254,8 @@ def test_cohorte_meme_siren( siret_new="11111111100002", parent=parent, parent_reason=( - "Nouvelle version de l'acteur conservées suite aux modifications:" - " SIRET 11111111100001 détecté le 2025-05-12 comme fermé dans AE," + "Nouvelle version de l'acteur conservée suite aux modifications:" + f" SIRET 11111111100001 détecté le {today} comme fermé dans AE," " remplacé par SIRET 11111111100002" ), source=source, @@ -285,6 +286,7 @@ def test_cohorte_autre_siren( suggestion.apply() # Verify changes + today = datetime.now(timezone.utc).strftime("%Y-%m-%d") self.check_replaced_acteur( id="a2", id_ext="ext_a2", @@ -292,14 +294,15 @@ def test_cohorte_autre_siren( siret_new="33333333300001", parent=parent, parent_reason=( - "Nouvelle version de l'acteur conservées suite aux modifications:" - " SIRET 22222222200001 détecté le 2025-05-12 comme fermé dans AE," + "Nouvelle version de l'acteur conservée suite aux modifications:" + f" SIRET 22222222200001 détecté le {today} comme fermé dans AE," " remplacé par SIRET 33333333300001" ), source=source, acteur_type=atype, ) + today = datetime.now(timezone.utc).strftime("%Y-%m-%d") self.check_replaced_acteur( id="a3", id_ext="ext_a3", @@ -308,8 +311,8 @@ def test_cohorte_autre_siren( parent=parent, # This closed acteur has no parent so there should be no parent_reason parent_reason=( - "Nouvelle version de l'acteur conservées suite aux modifications:" - " SIRET 44444444400001 détecté le 2025-05-12 comme fermé dans AE," + "Nouvelle version de l'acteur conservée suite aux modifications:" + f" SIRET 44444444400001 détecté le {today} comme fermé dans AE," " remplacé par SIRET 55555555500001" ), source=source, diff --git a/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py b/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py index 459a35fe7..b7e1d6eff 100644 --- a/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py +++ b/dags_unit_tests/shared/tasks/airflow_logic/test_packages_compatibility.py @@ -26,7 +26,6 @@ def test_pandas_read_sql_table(): - mettre à jour ce teste pour qu'il fonctionne - mettre à jour les codes DAGs pour qu'ils fonctionnent également """ - # print(f"Pandas version: {pd.__version__}") engine = create_engine("sqlite:///:memory:") engine.execute("CREATE TABLE my_table (id INT, name TEXT)") pd.read_sql_table("my_table", engine) diff --git a/data/models/changes/acteur_create_as_copy.py b/data/models/changes/acteur_create_as_copy.py index d2f55402b..b6d4b684a 100644 --- a/data/models/changes/acteur_create_as_copy.py +++ b/data/models/changes/acteur_create_as_copy.py @@ -43,23 +43,6 @@ def validate(self): ) raise ValueError(msg) - def copy_acteur( - self, - overriden_fields={ - "identifiant_unique": None, - "identifiant_externe": None, - "source": None, - }, - ): - from qfdmo.models import Acteur - - acteur_to_copy = Acteur.objects.get(pk=self.id) - revision_acteur_to_copy = acteur_to_copy.get_or_create_revision() - - revision_acteur_to_copy.instance_copy( - revision_acteur_to_copy, overriden_fields=overriden_fields - ) - def apply(self): self.validate() from qfdmo.models import Acteur, ActeurStatus diff --git a/data/models/changes/acteur_update_data.py b/data/models/changes/acteur_update_data.py index 1e2d08caf..d629e3603 100644 --- a/data/models/changes/acteur_update_data.py +++ b/data/models/changes/acteur_update_data.py @@ -32,6 +32,5 @@ def apply(self): ) data = data_reconstruct(RevisionActeur, self.data) for key, value in data.items(): - print(f"update_data: {key=} {value=}") setattr(acteur, key, value) acteur.save() diff --git a/unit_tests/data/models/changes/test_acteur_update_statut.py b/unit_tests/data/models/changes/test_acteur_update_statut.py index 706275ff1..bcb2b531e 100644 --- a/unit_tests/data/models/changes/test_acteur_update_statut.py +++ b/unit_tests/data/models/changes/test_acteur_update_statut.py @@ -74,77 +74,3 @@ def test_with_revision_acteur(self): assert acteur.siret_is_closed is True assert revision_acteur.statut == "INACTIF" assert revision_acteur.siret_is_closed is True - - # def test_raise_if_no_identifiant_unique_provided(self): - # acteur = ActeurFactory() - # change = ChangeActeurCreateAsCopy(id=acteur.pk, data={}) - # with pytest.raises( - # ValueError, match="l'acteur cible doit surdefinir son identifiant_unique" - # ): - # change.validate() - - # def test_raise_if_same_identifiant_unique(self): - # acteur = ActeurFactory() - # change = ChangeActeurCreateAsCopy( - # id=acteur.pk, data={"identifiant_unique": acteur.identifiant_unique} - # ) - # with pytest.raises( - # ValueError, - # match="l'acteur cible doit avoir un identifiant_unique différent", - # ): - # change.validate() - - # @patch.object(ChangeActeurCreateAsCopy, "validate") - # def test_validate_is_called_by_apply(self, mock_validate): - # acteur = ActeurFactory(statut=ActeurStatus.INACTIF) - # change = ChangeActeurCreateAsCopy( - # id=acteur.pk, data={"identifiant_unique": "new_id"} - # ) - # change.apply() - # mock_validate.assert_called_once() - - # def test_raise_if_source_acteur_is_active(self): - # acteur = ActeurFactory(statut=ActeurStatus.ACTIF) - # change = ChangeActeurCreateAsCopy( - # id=acteur.pk, data={"identifiant_unique": "new_id"} - # ) - # with pytest.raises(ValueError, match="ne doit pas être actif"): - # change.apply() - - # def test_working_case(self): - # # Création d'un acteur inactif avec des données de base - # acteur_to_copy = ActeurFactory( - # nom="test", - # statut=ActeurStatus.INACTIF, - # ) - # proposition = PropositionServiceFactory() - # acteur_to_copy.proposition_services.add(proposition) - - # change = ChangeActeurCreateAsCopy( - # id=acteur_to_copy.pk, - # data={"identifiant_unique": "new_id", "statut": ActeurStatus.ACTIF}, - # ) - # change.apply() - - # assert acteur_to_copy.statut == ActeurStatus.INACTIF - - # # Revision exists - # revision_acteur_to_copy = RevisionActeur.objects.get(pk=acteur_to_copy.pk) - # assert revision_acteur_to_copy is not None - # assert revision_acteur_to_copy.proposition_services.count() == 1 - # assert revision_acteur_to_copy.statut == ActeurStatus.INACTIF - - # # New acteur created - # new_acteur = Acteur.objects.get(identifiant_unique="new_id") - # assert new_acteur.nom == "test" - # assert new_acteur.acteur_type == acteur_to_copy.acteur_type - # assert new_acteur.source == acteur_to_copy.source - # assert new_acteur.location == acteur_to_copy.location - # assert new_acteur.statut == ActeurStatus.ACTIF - - # assert new_acteur.proposition_services.count() == 1 - - # new_revision_acteur = RevisionActeur.objects.get(pk=new_acteur.pk) - # assert new_revision_acteur is not None - # assert new_revision_acteur.statut == ActeurStatus.ACTIF - # assert new_revision_acteur.proposition_services.count() == 1 From 2dd0d9b0d4497128909164e97c516382e32fe11a Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Wed, 14 May 2025 12:41:05 +0200 Subject: [PATCH 08/12] simplification de la modification des acteurs --- .../enrich_dbt_model_to_suggestions.py | 52 ++---- .../tasks/test_enrich_suggestions_closed.py | 170 +++++------------- data/models/changes/__init__.py | 14 +- data/models/changes/acteur_create_as_copy.py | 58 ------ data/models/changes/acteur_update_revision.py | 31 ++++ data/models/changes/acteur_update_statut.py | 39 ---- .../changes/test_acteur_create_as_copy.py | 94 ---------- .../changes/test_acteur_update_revision.py | 58 ++++++ .../changes/test_acteur_update_statut.py | 76 -------- 9 files changed, 149 insertions(+), 443 deletions(-) delete mode 100644 data/models/changes/acteur_create_as_copy.py create mode 100644 data/models/changes/acteur_update_revision.py delete mode 100644 data/models/changes/acteur_update_statut.py delete mode 100644 unit_tests/data/models/changes/test_acteur_create_as_copy.py create mode 100644 unit_tests/data/models/changes/test_acteur_update_revision.py delete mode 100644 unit_tests/data/models/changes/test_acteur_update_statut.py diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 5192af17e..2bc882cc0 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -124,63 +124,33 @@ def changes_prepare_closed_replaced( row: dict, ) -> tuple[list[dict], dict]: """Prepare suggestion changes for closed replaced cohorts""" - from data.models.changes import ChangeActeurCreateAsCopy, ChangeActeurUpdateStatut - from qfdmo.models import ActeurStatus + from data.models.changes import ChangeActeurUpdateRevision changes = [] today = datetime.now(timezone.utc).strftime("%Y-%m-%d") - # We don't create a parent because we decided it was - # not a good idea (1 parent for 1 child will only create - # complications for the clustering and no added value) - - # Existing Child: must be updated BEFORE new child to prevent - # conflict on same external ID having 2+ active children - old_acteur = { + update_revision = { "id": row[COLS.ACTEUR_ID], "data": { - "siret_is_closed": True, - "statut": ActeurStatus.INACTIF, - }, - } - changes.append( - changes_prepare( - model=ChangeActeurUpdateStatut, - model_params=old_acteur, - order=1, - reason="ancienne version de l'acteur inactive", - entity_type="acteur_displayed", - ) - ) - # New child to hold the reference data as standalone - # (since old child will be closed) - now = datetime.now(timezone.utc).strftime("%Y%m%d%H%M%S") - new_acteur_id = f"{row[COLS.ACTEUR_ID]}_{now}" - new_acteur = { - "id": row[COLS.ACTEUR_ID], - "data": { - "identifiant_unique": new_acteur_id, - "statut": ActeurStatus.ACTIF, "siret": row[COLS.SUGGEST_SIRET], "siren": row[COLS.SUGGEST_SIRET][:9], "siret_is_closed": False, "parent_reason": ( - "Nouvelle version de l'acteur conservée suite aux modifications: " - f"SIRET {row[COLS.ACTEUR_SIRET]} " - f"détecté le {today} comme fermé dans AE, " - f"remplacé par SIRET {row[COLS.SUGGEST_SIRET]}" + f"Modifications de l'acteur le {today}: " + f"SIRET {row[COLS.ACTEUR_SIRET]} détecté comme fermé dans AE, " + f"remplacé par le SIRET {row[COLS.SUGGEST_SIRET]}" ), }, } - changes.append( + changes = [ changes_prepare( - model=ChangeActeurCreateAsCopy, - model_params=new_acteur, - order=2, - reason="nouvel enfant pour conserver les données", + model=ChangeActeurUpdateRevision, + model_params=update_revision, + order=1, + reason="Modification du SIRET", entity_type="acteur_displayed", ) - ) + ] contexte = {} # changes are self-explanatory return changes, contexte diff --git a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py index 89c30cf38..44a143e66 100644 --- a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py +++ b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py @@ -1,4 +1,3 @@ -import re from datetime import datetime, timezone import pandas as pd @@ -10,7 +9,13 @@ enrich_dbt_model_to_suggestions, ) -from qfdmo.models.acteur import Acteur +from data.models.suggestion import Suggestion, SuggestionCohorte +from qfdmo.models.acteur import Acteur, ActeurStatus, RevisionActeur +from unit_tests.qfdmo.acteur_factory import ( + ActeurFactory, + ActeurTypeFactory, + SourceFactory, +) TODAY = datetime.now(timezone.utc).strftime("%Y-%m-%d") @@ -20,26 +25,11 @@ class TestEnrichActeursClosedSuggestions: @pytest.fixture def source(self): - from qfdmo.models import Source - - return Source.objects.create(code="s1") + return SourceFactory(code="s1") @pytest.fixture def atype(self): - from qfdmo.models import ActeurType - - return ActeurType.objects.create(code="at1") - - @pytest.fixture - def parent(self): - from unit_tests.qfdmo.acteur_factory import RevisionActeurFactory - - return RevisionActeurFactory( - nom="Parent", - adresse="Adresse parent", - code_postal="12345", - ville="Ville parent", - ) + return ActeurTypeFactory(code="at1") @pytest.fixture def df_not_replaced(self, atype, source): @@ -57,7 +47,7 @@ def df_not_replaced(self, atype, source): ) @pytest.fixture - def df_replaced(self, atype, source, parent): + def df_replaced(self, atype, source): return pd.DataFrame( { # Acteurs data @@ -65,7 +55,6 @@ def df_replaced(self, atype, source, parent): COLS.ACTEUR_ID_EXTERNE: ["ext_a1", "ext_a2", "ext_a3"], # We test 1 acteur of each cohort with a parent, and # the case with no parent - COLS.ACTEUR_PARENT_ID: [parent.pk, parent.pk, None], COLS.ACTEUR_SIRET: [ "11111111100001", "22222222200001", @@ -121,14 +110,15 @@ def df_replaced_autre_siret(self, df_replaced): @pytest.fixture def acteurs(self, df_not_replaced, df_replaced, atype, source): # Creating acteurs as presence required to apply changes - from qfdmo.models import Acteur df_concat = pd.concat([df_not_replaced, df_replaced]) for _, row in df_concat.iterrows(): - acteur = Acteur( + acteur = ActeurFactory( identifiant_unique=row[COLS.ACTEUR_ID], identifiant_externe=row[COLS.ACTEUR_ID_EXTERNE], nom=f"AVANT {row[COLS.ACTEUR_ID]}", + siret=row[COLS.ACTEUR_SIRET], + siren=row[COLS.ACTEUR_SIRET][:9], acteur_type=atype, source=source, location=Point( @@ -173,60 +163,7 @@ def test_cohorte_not_replaced(self, acteurs, df_not_replaced): assert closed.parent_reason == "" # consequence of empty strings in DB assert closed.siret_is_closed is True - def check_replaced_acteur( - self, - id, - id_ext, - siret_closed, - siret_new, - parent, - parent_reason, - source, - acteur_type, - ): - """Utility to check replaced acteur to avoid repetition""" - from qfdmo.models import ActeurStatus, RevisionActeur - - # Closed acteur - a_closed = Acteur.objects.get(pk=id) - ra_closed = RevisionActeur.objects.get(pk=id) - assert a_closed.statut == ActeurStatus.INACTIF - assert ra_closed.statut == ActeurStatus.INACTIF - assert ra_closed.siret_is_closed or a_closed.siret_is_closed - - # The new acteur which about from siret_is_closed AND - # identifiant_externe has the same data as the closed acteur - a_new = Acteur.objects.filter( - identifiant_externe=id_ext, statut=ActeurStatus.ACTIF - ).first() - ra_new = RevisionActeur.objects.filter( - identifiant_unique=a_new.identifiant_unique - ).first() - assert a_new is not None - assert ra_new is not None - # ID is concatenation of closed acteur ID, new siret and today's datetime - assert re.search( - f"^{id}_{TODAY.replace('-', '')}[0-9]{{6}}$", a_new.identifiant_unique - ) - assert a_new.statut == ActeurStatus.ACTIF - assert ra_new.statut == ActeurStatus.ACTIF - assert a_new.siret == siret_new - assert ra_new.siret == siret_new - assert a_new.siret_is_closed is False # we have detected no closure yet - assert ra_new.siret_is_closed is False # we have detected no closure yet - - # foreign keys are iso with the closed acteur - assert a_new.source.pk == source.pk # type: ignore - assert a_new.acteur_type.pk == acteur_type.pk - assert a_new.identifiant_externe == id_ext - # Explanation as to why - print(f"{parent_reason} === {ra_new.parent_reason}") - assert ra_new.parent_reason == parent_reason - - def test_cohorte_meme_siren( - self, acteurs, parent, atype, source, df_replaced_meme_siret - ): - from data.models.suggestion import Suggestion, SuggestionCohorte + def test_cohorte_meme_siren(self, acteurs, df_replaced_meme_siret): # Write suggestions to DB enrich_dbt_model_to_suggestions( @@ -245,26 +182,23 @@ def test_cohorte_meme_siren( for suggestion in suggestions: suggestion.apply() - # Closed acteur - today = datetime.now(timezone.utc).strftime("%Y-%m-%d") - self.check_replaced_acteur( - id="a1", - id_ext="ext_a1", - siret_closed="11111111100001", - siret_new="11111111100002", - parent=parent, - parent_reason=( - "Nouvelle version de l'acteur conservée suite aux modifications:" - f" SIRET 11111111100001 détecté le {today} comme fermé dans AE," - " remplacé par SIRET 11111111100002" - ), - source=source, - acteur_type=atype, + acteur = Acteur.objects.get(pk="a1") + assert acteur.statut == ActeurStatus.ACTIF + assert acteur.siret == "11111111100001" + assert acteur.siren == "111111111" + + revision = RevisionActeur.objects.get(pk="a1") + assert revision.statut == ActeurStatus.ACTIF + assert revision.siret == "11111111100002" + assert acteur.siren == "111111111" + assert revision.parent is None + assert revision.parent_reason == ( + f"Modifications de l'acteur le {TODAY}: SIRET 11111111100001 détecté comme" + " fermé dans AE, remplacé par le SIRET 11111111100002" ) + assert revision.siret_is_closed is False - def test_cohorte_autre_siren( - self, acteurs, parent, atype, source, df_replaced_autre_siret - ): + def test_cohorte_autre_siren(self, acteurs, df_replaced_autre_siret): from data.models.suggestion import Suggestion, SuggestionCohorte # Write suggestions to DB @@ -285,36 +219,18 @@ def test_cohorte_autre_siren( for suggestion in suggestions: suggestion.apply() - # Verify changes - today = datetime.now(timezone.utc).strftime("%Y-%m-%d") - self.check_replaced_acteur( - id="a2", - id_ext="ext_a2", - siret_closed="22222222200001", - siret_new="33333333300001", - parent=parent, - parent_reason=( - "Nouvelle version de l'acteur conservée suite aux modifications:" - f" SIRET 22222222200001 détecté le {today} comme fermé dans AE," - " remplacé par SIRET 33333333300001" - ), - source=source, - acteur_type=atype, - ) - - today = datetime.now(timezone.utc).strftime("%Y-%m-%d") - self.check_replaced_acteur( - id="a3", - id_ext="ext_a3", - siret_closed="44444444400001", - siret_new="55555555500001", - parent=parent, - # This closed acteur has no parent so there should be no parent_reason - parent_reason=( - "Nouvelle version de l'acteur conservée suite aux modifications:" - f" SIRET 44444444400001 détecté le {today} comme fermé dans AE," - " remplacé par SIRET 55555555500001" - ), - source=source, - acteur_type=atype, + acteur = Acteur.objects.get(pk="a2") + assert acteur.statut == ActeurStatus.ACTIF + assert acteur.siret == "22222222200001" + assert acteur.siren == "222222222" + + revision = RevisionActeur.objects.get(pk="a2") + assert revision.statut == ActeurStatus.ACTIF + assert revision.siret == "33333333300001" + assert revision.siren == "333333333" + assert revision.parent is None + assert revision.parent_reason == ( + f"Modifications de l'acteur le {TODAY}: SIRET 22222222200001 détecté comme" + " fermé dans AE, remplacé par le SIRET 33333333300001" ) + assert revision.siret_is_closed is False diff --git a/data/models/changes/__init__.py b/data/models/changes/__init__.py index 1a26cb756..c85081e38 100644 --- a/data/models/changes/__init__.py +++ b/data/models/changes/__init__.py @@ -1,25 +1,23 @@ from .acteur_change_nothing_in_base import ChangeActeurNothingBase -from .acteur_create_as_copy import ChangeActeurCreateAsCopy from .acteur_create_as_parent import ChangeActeurCreateAsParent from .acteur_delete_as_parent import ChangeActeurDeleteAsParent from .acteur_keep_as_parent import ChangeActeurKeepAsParent from .acteur_rgpd_anonymize import ChangeActeurRgpdAnonymize from .acteur_update_data import ChangeActeurUpdateData from .acteur_update_parent_id import ChangeActeurUpdateParentId -from .acteur_update_statut import ChangeActeurUpdateStatut +from .acteur_update_revision import ChangeActeurUpdateRevision from .acteur_verify_in_revision import ChangeActeurVerifyRevision from .sample_model_do_nothing import SampleModelDoNothing CHANGE_MODELS = { - ChangeActeurUpdateData.name(): ChangeActeurUpdateData, - ChangeActeurCreateAsCopy.name(): ChangeActeurCreateAsCopy, ChangeActeurCreateAsParent.name(): ChangeActeurCreateAsParent, ChangeActeurDeleteAsParent.name(): ChangeActeurDeleteAsParent, + ChangeActeurKeepAsParent.name(): ChangeActeurKeepAsParent, + ChangeActeurNothingBase.name(): ChangeActeurNothingBase, + ChangeActeurRgpdAnonymize.name(): ChangeActeurRgpdAnonymize, + ChangeActeurUpdateData.name(): ChangeActeurUpdateData, ChangeActeurUpdateParentId.name(): ChangeActeurUpdateParentId, + ChangeActeurUpdateRevision.name(): ChangeActeurUpdateRevision, ChangeActeurVerifyRevision.name(): ChangeActeurVerifyRevision, - ChangeActeurNothingBase.name(): ChangeActeurNothingBase, - ChangeActeurKeepAsParent.name(): ChangeActeurKeepAsParent, SampleModelDoNothing.name(): SampleModelDoNothing, - ChangeActeurRgpdAnonymize.name(): ChangeActeurRgpdAnonymize, - ChangeActeurUpdateStatut.name(): ChangeActeurUpdateStatut, } diff --git a/data/models/changes/acteur_create_as_copy.py b/data/models/changes/acteur_create_as_copy.py deleted file mode 100644 index b6d4b684a..000000000 --- a/data/models/changes/acteur_create_as_copy.py +++ /dev/null @@ -1,58 +0,0 @@ -import logging - -from pydantic import BaseModel - -logger = logging.getLogger(__name__) - - -class ChangeActeurCreateAsCopy(BaseModel): - id: str # id of the acteur to copy - data: dict = {} # data to set on the new acteur - - def _get_acteur_to_copy(self): - from qfdmo.models import Acteur, RevisionActeur - - return RevisionActeur.objects.filter(pk=self.id).first() or Acteur.objects.get( - pk=self.id - ) - - @classmethod - def name(cls) -> str: - return "acteur_create_as_copy" - - def validate(self): - from qfdmo.models import Acteur - - # Ensure source exists - try: - acteur = self._get_acteur_to_copy() - except Acteur.DoesNotExist: - msg = f"Copy d'acteur: l'acteur source '{self.id}' n'existe pas dans Acteur" - raise ValueError(msg) - - # Ensure identifiant_unique is overridden - if "identifiant_unique" not in self.data: - msg = "Copy d'acteur: l'acteur cible doit surdefinir son identifiant_unique" - raise ValueError(msg) - - # Ensure identifiant_unique is not the same as the source - if self.data["identifiant_unique"] == acteur.identifiant_unique: - msg = ( - "Copy d'acteur: l'acteur cible doit avoir un identifiant_unique" - " différent de l'acteur source" - ) - raise ValueError(msg) - - def apply(self): - self.validate() - from qfdmo.models import Acteur, ActeurStatus - - acteur_to_copy = Acteur.objects.get(pk=self.id) - revision_acteur_to_copy = acteur_to_copy.get_or_create_revision() - - # Ensure Acteur is not active - if acteur_to_copy.statut == ActeurStatus.ACTIF: - msg = f"Copy d'acteur: l'acteur source '{self.id}' ne doit pas être actif" - raise ValueError(msg) - - revision_acteur_to_copy.instance_copy(overriden_fields=self.data) diff --git a/data/models/changes/acteur_update_revision.py b/data/models/changes/acteur_update_revision.py new file mode 100644 index 000000000..8b1819982 --- /dev/null +++ b/data/models/changes/acteur_update_revision.py @@ -0,0 +1,31 @@ +"""Generic change model to update an acteur's data. If your use-case +is very specific (e.g. RGPD), create dedicated model for more clarity/reliability.""" + +from data.models.changes.acteur_abstract import ChangeActeurAbstract +from qfdmo.models import Acteur + + +class ChangeActeurUpdateRevision(ChangeActeurAbstract): + @classmethod + def name(cls) -> str: + return "acteur_update_revision" + + def validate(self) -> Acteur: + if not self.data: + raise ValueError("Aucune donnée fournie") + # The parent should already exist in revision or base + # We tolerate absence from revision + try: + acteur = Acteur.objects.get(pk=self.id) + except Acteur.DoesNotExist: + raise ValueError(f"L'acteur cible {self.id} n'existe pas") + return acteur + + def apply(self): + acteur = self.validate() + + revision = acteur.get_or_create_revision() + + for key, value in self.data.items(): + setattr(revision, key, value) + revision.save() diff --git a/data/models/changes/acteur_update_statut.py b/data/models/changes/acteur_update_statut.py deleted file mode 100644 index 5485ad8ac..000000000 --- a/data/models/changes/acteur_update_statut.py +++ /dev/null @@ -1,39 +0,0 @@ -"""Change model to update an acteur's statut. We need a speciific change model because -the statut is updated in acteur and revisionacteur tables.""" - -from data.models.changes.acteur_abstract import ChangeActeurAbstract -from qfdmo.models import Acteur, ActeurStatus, RevisionActeur - - -class ChangeActeurUpdateStatut(ChangeActeurAbstract): - @classmethod - def name(cls) -> str: - return "acteur_update_statut" - - def validate(self): - if not self.data: - raise ValueError("No data provided") - - if "statut" not in self.data: - raise ValueError("No statut provided") - - if self.data.keys() - {"statut", "siret_is_closed"}: - raise ValueError( - "Invalid data, only statut and siret_is_closed are allowed" - ) - - if self.data["statut"] not in ActeurStatus.values: - raise ValueError(f"Invalid statut: {self.data['statut']}") - - def apply(self): - instances = self.validate() - - instances: list[Acteur | RevisionActeur] = [Acteur.objects.get(pk=self.id)] - if revision := RevisionActeur.objects.filter(pk=self.id).first(): - instances.append(revision) - - for instance in instances: - instance.statut = self.data["statut"] - if "siret_is_closed" in self.data: - instance.siret_is_closed = self.data["siret_is_closed"] - instance.save() diff --git a/unit_tests/data/models/changes/test_acteur_create_as_copy.py b/unit_tests/data/models/changes/test_acteur_create_as_copy.py deleted file mode 100644 index 1de94e78a..000000000 --- a/unit_tests/data/models/changes/test_acteur_create_as_copy.py +++ /dev/null @@ -1,94 +0,0 @@ -from unittest.mock import patch - -import pytest - -from data.models.changes.acteur_create_as_copy import ChangeActeurCreateAsCopy -from qfdmo.models import Acteur, ActeurStatus, RevisionActeur -from unit_tests.qfdmo.acteur_factory import ActeurFactory, PropositionServiceFactory - - -@pytest.mark.django_db -class TestChangeActeurCreateAsCopy: - def test_model_name(self): - assert ChangeActeurCreateAsCopy.name() == "acteur_create_as_copy" - - def test_raise_if_acteur_does_not_exist(self): - change = ChangeActeurCreateAsCopy( - id="dummy", data={"identifiant_unique": "new_id"} - ) - with pytest.raises(ValueError): - change.validate() - - def test_raise_if_no_identifiant_unique_provided(self): - acteur = ActeurFactory() - change = ChangeActeurCreateAsCopy(id=acteur.pk, data={}) - with pytest.raises( - ValueError, match="l'acteur cible doit surdefinir son identifiant_unique" - ): - change.validate() - - def test_raise_if_same_identifiant_unique(self): - acteur = ActeurFactory() - change = ChangeActeurCreateAsCopy( - id=acteur.pk, data={"identifiant_unique": acteur.identifiant_unique} - ) - with pytest.raises( - ValueError, - match="l'acteur cible doit avoir un identifiant_unique différent", - ): - change.validate() - - @patch.object(ChangeActeurCreateAsCopy, "validate") - def test_validate_is_called_by_apply(self, mock_validate): - acteur = ActeurFactory(statut=ActeurStatus.INACTIF) - change = ChangeActeurCreateAsCopy( - id=acteur.pk, data={"identifiant_unique": "new_id"} - ) - change.apply() - mock_validate.assert_called_once() - - def test_raise_if_source_acteur_is_active(self): - acteur = ActeurFactory(statut=ActeurStatus.ACTIF) - change = ChangeActeurCreateAsCopy( - id=acteur.pk, data={"identifiant_unique": "new_id"} - ) - with pytest.raises(ValueError, match="ne doit pas être actif"): - change.apply() - - def test_working_case(self): - # Création d'un acteur inactif avec des données de base - acteur_to_copy = ActeurFactory( - nom="test", - statut=ActeurStatus.INACTIF, - ) - proposition = PropositionServiceFactory() - acteur_to_copy.proposition_services.add(proposition) - - change = ChangeActeurCreateAsCopy( - id=acteur_to_copy.pk, - data={"identifiant_unique": "new_id", "statut": ActeurStatus.ACTIF}, - ) - change.apply() - - assert acteur_to_copy.statut == ActeurStatus.INACTIF - - # Revision exists - revision_acteur_to_copy = RevisionActeur.objects.get(pk=acteur_to_copy.pk) - assert revision_acteur_to_copy is not None - assert revision_acteur_to_copy.proposition_services.count() == 1 - assert revision_acteur_to_copy.statut == ActeurStatus.INACTIF - - # New acteur created - new_acteur = Acteur.objects.get(identifiant_unique="new_id") - assert new_acteur.nom == "test" - assert new_acteur.acteur_type == acteur_to_copy.acteur_type - assert new_acteur.source == acteur_to_copy.source - assert new_acteur.location == acteur_to_copy.location - assert new_acteur.statut == ActeurStatus.ACTIF - - assert new_acteur.proposition_services.count() == 1 - - new_revision_acteur = RevisionActeur.objects.get(pk=new_acteur.pk) - assert new_revision_acteur is not None - assert new_revision_acteur.statut == ActeurStatus.ACTIF - assert new_revision_acteur.proposition_services.count() == 1 diff --git a/unit_tests/data/models/changes/test_acteur_update_revision.py b/unit_tests/data/models/changes/test_acteur_update_revision.py new file mode 100644 index 000000000..92486a6da --- /dev/null +++ b/unit_tests/data/models/changes/test_acteur_update_revision.py @@ -0,0 +1,58 @@ +from unittest.mock import patch + +import pytest + +from data.models.changes.acteur_update_revision import ChangeActeurUpdateRevision +from qfdmo.models import ActeurStatus, RevisionActeur +from unit_tests.qfdmo.acteur_factory import ActeurFactory, PropositionServiceFactory + + +@pytest.mark.django_db +class TestChangeActeurUpdateRsevision: + def test_model_name(self): + assert ChangeActeurUpdateRevision.name() == "acteur_update_revision" + + def test_raise_if_acteur_does_not_exist(self): + change = ChangeActeurUpdateRevision( + id="dummy", data={"identifiant_unique": "new_id"} + ) + with pytest.raises(ValueError, match="L'acteur cible dummy n'existe pas"): + change.validate() + + def test_raise_if_no_data_provided(self): + acteur = ActeurFactory() + change = ChangeActeurUpdateRevision(id=acteur.pk, data={}) + with pytest.raises(ValueError, match="Aucune donnée fournie"): + change.validate() + + @patch.object(ChangeActeurUpdateRevision, "validate") + def test_validate_is_called_by_apply(self, mock_validate): + acteur = ActeurFactory(statut=ActeurStatus.INACTIF) + change = ChangeActeurUpdateRevision(id=acteur.pk, data={"statut": "ACTIF"}) + change.apply() + mock_validate.assert_called_once() + + def test_without_revision_ok(self): + # Création d'un acteur inactif avec des données de base + acteur = ActeurFactory( + nom="foo", + statut=ActeurStatus.ACTIF, + ) + proposition = PropositionServiceFactory() + acteur.proposition_services.add(proposition) + + change = ChangeActeurUpdateRevision( + id=acteur.pk, + data={"nom": "bar", "statut": ActeurStatus.INACTIF}, + ) + change.apply() + + assert acteur.statut == ActeurStatus.ACTIF + assert acteur.nom == "foo" + assert acteur.proposition_services.count() == 1 + + revision = RevisionActeur.objects.get(pk=acteur.pk) + + assert revision.statut == ActeurStatus.INACTIF + assert revision.nom == "bar" + assert revision.proposition_services.count() == 1 diff --git a/unit_tests/data/models/changes/test_acteur_update_statut.py b/unit_tests/data/models/changes/test_acteur_update_statut.py deleted file mode 100644 index bcb2b531e..000000000 --- a/unit_tests/data/models/changes/test_acteur_update_statut.py +++ /dev/null @@ -1,76 +0,0 @@ -from unittest.mock import patch - -import pytest - -from data.models.changes.acteur_update_statut import ChangeActeurUpdateStatut -from qfdmo.models import RevisionActeur -from unit_tests.qfdmo.acteur_factory import ActeurFactory - - -@pytest.mark.django_db -class TestChangeActeurUpdateStatut: - def test_model_name(self): - assert ChangeActeurUpdateStatut.name() == "acteur_update_statut" - - def test_raise_if_no_data_provided(self): - change = ChangeActeurUpdateStatut(id="dummy", data={}) - with pytest.raises(ValueError, match="No data provided"): - change.validate() - - def test_raise_if_no_statut_provided(self): - change = ChangeActeurUpdateStatut(id="dummy", data={"siret_is_closed": True}) - with pytest.raises(ValueError, match="No statut provided"): - change.validate() - - def test_raise_if_invalid_statut_provided(self): - change = ChangeActeurUpdateStatut(id="dummy", data={"statut": "not_active"}) - with pytest.raises(ValueError, match="Invalid statut"): - change.validate() - - def test_raise_if_column_not_allowed(self): - change = ChangeActeurUpdateStatut( - id="dummy", data={"statut": "ACTIF", "fake": "boo"} - ) - with pytest.raises( - ValueError, - match="Invalid data, only statut and siret_is_closed are allowed", - ): - change.validate() - - @patch.object(ChangeActeurUpdateStatut, "validate") - def test_validate_is_called_by_apply(self, mock_validate): - acteur = ActeurFactory() - change = ChangeActeurUpdateStatut(id=acteur.pk, data={"statut": "INACTIF"}) - change.apply() - mock_validate.assert_called_once() - - def test_with_acteur(self): - acteur = ActeurFactory() - change = ChangeActeurUpdateStatut( - id=acteur.pk, data={"statut": "INACTIF", "siret_is_closed": True} - ) - - change.apply() - acteur.refresh_from_db() - - assert acteur.statut == "INACTIF" - assert acteur.siret_is_closed is True - - revision_acteur = RevisionActeur.objects.filter(pk=acteur.pk).first() - assert revision_acteur is None - - def test_with_revision_acteur(self): - acteur = ActeurFactory() - revision_acteur = acteur.get_or_create_revision() - change = ChangeActeurUpdateStatut( - id=revision_acteur.pk, data={"statut": "INACTIF", "siret_is_closed": True} - ) - - change.apply() - acteur.refresh_from_db() - revision_acteur.refresh_from_db() - - assert acteur.statut == "INACTIF" - assert acteur.siret_is_closed is True - assert revision_acteur.statut == "INACTIF" - assert revision_acteur.siret_is_closed is True From 19d94b47b9c7f662f7ee85a63c8cae7569e0bb62 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Wed, 14 May 2025 14:23:04 +0200 Subject: [PATCH 09/12] simplifie encore l'update d'acteur --- .../business_logic/crawl_urls_suggest.py | 5 +- .../enrich_dbt_model_to_suggestions.py | 18 ++-- .../test_crawl_urls_suggest_prepare.py | 5 +- .../tasks/test_enrich_suggestions_closed.py | 6 +- data/models/changes/__init__.py | 2 - data/models/changes/acteur_update_data.py | 36 -------- .../models/changes/test_acteur_update_data.py | 82 ------------------- 7 files changed, 20 insertions(+), 134 deletions(-) delete mode 100644 data/models/changes/acteur_update_data.py delete mode 100644 unit_tests/data/models/changes/test_acteur_update_data.py diff --git a/dags/crawl/tasks/business_logic/crawl_urls_suggest.py b/dags/crawl/tasks/business_logic/crawl_urls_suggest.py index f878a4b24..4d286ec61 100644 --- a/dags/crawl/tasks/business_logic/crawl_urls_suggest.py +++ b/dags/crawl/tasks/business_logic/crawl_urls_suggest.py @@ -3,6 +3,7 @@ import pandas as pd from crawl.config.columns import COLS from crawl.config.constants import LABEL_URL_ORIGINE, LABEL_URL_PROPOSEE + from utils import logging_utils as log from utils.dataframes import ( df_col_assert_get_unique, @@ -39,7 +40,7 @@ def suggestions_prepare( - df_crawl_ok_diff = successful AND different = propose - df_crawl_fail = failed = propose None""" from data.models.change import SuggestionChange - from data.models.changes import ChangeActeurUpdateData + from data.models.changes import ChangeActeurUpdateRevision if df_none_or_empty(df): return [] @@ -54,7 +55,7 @@ def suggestions_prepare( order=1, reason=row[COLS.COHORT], entity_type="acteur_displayed", - model_name=ChangeActeurUpdateData.name(), + model_name=ChangeActeurUpdateRevision.name(), model_params={ "id": acteur[COLS.ID], "data": {COLS.URL_DB: row[COLS.SUGGEST_VALUE]}, diff --git a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py index 2bc882cc0..74997e0e0 100644 --- a/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py +++ b/dags/enrich/tasks/business_logic/enrich_dbt_model_to_suggestions.py @@ -34,7 +34,7 @@ def changes_prepare( def changes_prepare_villes(row: dict) -> tuple[list[dict], dict]: """Prepare suggestions for villes cohorts""" - from data.models.changes import ChangeActeurUpdateData + from data.models.changes import ChangeActeurUpdateRevision changes = [] model_params = { @@ -45,7 +45,7 @@ def changes_prepare_villes(row: dict) -> tuple[list[dict], dict]: } changes.append( changes_prepare( - model=ChangeActeurUpdateData, + model=ChangeActeurUpdateRevision, model_params=model_params, order=1, reason="On fait confiance à la BAN", @@ -92,24 +92,26 @@ def changes_prepare_closed_not_replaced( row: dict, ) -> tuple[list[dict], dict]: """Prepare suggestions for closed not replaced cohorts""" - from data.models.changes import ChangeActeurUpdateData + from data.models.changes import ChangeActeurUpdateRevision from qfdmo.models import ActeurStatus changes = [] + today = datetime.now(timezone.utc).strftime("%Y-%m-%d") model_params = { "id": row[COLS.ACTEUR_ID], "data": { "statut": ActeurStatus.INACTIF, - "siret": row[COLS.ACTEUR_SIRET], - "siren": row[COLS.ACTEUR_SIRET][:9], "siret_is_closed": True, - "acteur_type": row[COLS.ACTEUR_TYPE_ID], - "source": row[COLS.ACTEUR_SOURCE_ID], + "parent_reason": ( + f"Modifications de l'acteur le {today}: " + f"SIRET {row[COLS.ACTEUR_SIRET]} détecté comme fermé dans AE," + " Pas de remplacement" + ), }, } changes.append( changes_prepare( - model=ChangeActeurUpdateData, + model=ChangeActeurUpdateRevision, model_params=model_params, order=1, reason="SIRET & SIREN fermés, 0 remplacement trouvé", diff --git a/dags_unit_tests/crawl/tasks/business_logic/test_crawl_urls_suggest_prepare.py b/dags_unit_tests/crawl/tasks/business_logic/test_crawl_urls_suggest_prepare.py index 03a545108..0daff1784 100644 --- a/dags_unit_tests/crawl/tasks/business_logic/test_crawl_urls_suggest_prepare.py +++ b/dags_unit_tests/crawl/tasks/business_logic/test_crawl_urls_suggest_prepare.py @@ -2,11 +2,11 @@ import pytest from crawl.fixtures import acteurs_create, df_syntax_fail # noqa from sources.config.shared_constants import EMPTY_ACTEUR_FIELD -from utils import logging_utils as log from dags.crawl.config.cohorts import COHORTS from dags.crawl.config.columns import COLS from dags.crawl.tasks.business_logic.crawl_urls_suggest import suggestions_prepare +from utils import logging_utils as log @pytest.mark.django_db @@ -30,7 +30,6 @@ def test_json_serializable(self, suggestions): pass def test_raise_if_acteurs_missing(self): - from qfdmo.models.acteur import Acteur df = pd.DataFrame( { @@ -42,5 +41,5 @@ def test_raise_if_acteurs_missing(self): COLS.SUGGEST_VALUE: [EMPTY_ACTEUR_FIELD], } ) - with pytest.raises(Acteur.DoesNotExist): + with pytest.raises(ValueError): suggestions_prepare(df=df) diff --git a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py index 44a143e66..7341903f0 100644 --- a/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py +++ b/dags_unit_tests/enrich/tasks/test_enrich_suggestions_closed.py @@ -160,7 +160,11 @@ def test_cohorte_not_replaced(self, acteurs, df_not_replaced): closed = RevisionActeur.objects.get(pk=id) assert closed.statut == ActeurStatus.INACTIF assert closed.parent is None - assert closed.parent_reason == "" # consequence of empty strings in DB + assert closed.parent_reason == ( + f"Modifications de l'acteur le {TODAY}: " + f"SIRET {'00000000000001' if id == 'a01' else '00000000000002'} détecté" + " comme fermé dans AE, Pas de remplacement" + ) assert closed.siret_is_closed is True def test_cohorte_meme_siren(self, acteurs, df_replaced_meme_siret): diff --git a/data/models/changes/__init__.py b/data/models/changes/__init__.py index c85081e38..e43e0500a 100644 --- a/data/models/changes/__init__.py +++ b/data/models/changes/__init__.py @@ -3,7 +3,6 @@ from .acteur_delete_as_parent import ChangeActeurDeleteAsParent from .acteur_keep_as_parent import ChangeActeurKeepAsParent from .acteur_rgpd_anonymize import ChangeActeurRgpdAnonymize -from .acteur_update_data import ChangeActeurUpdateData from .acteur_update_parent_id import ChangeActeurUpdateParentId from .acteur_update_revision import ChangeActeurUpdateRevision from .acteur_verify_in_revision import ChangeActeurVerifyRevision @@ -15,7 +14,6 @@ ChangeActeurKeepAsParent.name(): ChangeActeurKeepAsParent, ChangeActeurNothingBase.name(): ChangeActeurNothingBase, ChangeActeurRgpdAnonymize.name(): ChangeActeurRgpdAnonymize, - ChangeActeurUpdateData.name(): ChangeActeurUpdateData, ChangeActeurUpdateParentId.name(): ChangeActeurUpdateParentId, ChangeActeurUpdateRevision.name(): ChangeActeurUpdateRevision, ChangeActeurVerifyRevision.name(): ChangeActeurVerifyRevision, diff --git a/data/models/changes/acteur_update_data.py b/data/models/changes/acteur_update_data.py deleted file mode 100644 index d629e3603..000000000 --- a/data/models/changes/acteur_update_data.py +++ /dev/null @@ -1,36 +0,0 @@ -"""Generic change model to update an acteur's data. If your use-case -is very specific (e.g. RGPD), create dedicated model for more clarity/reliability.""" - -from data.models.changes.acteur_abstract import ChangeActeurAbstract -from data.models.changes.utils import data_reconstruct -from qfdmo.models import Acteur, RevisionActeur - - -class ChangeActeurUpdateData(ChangeActeurAbstract): - @classmethod - def name(cls) -> str: - return "acteur_update_data" - - def validate(self) -> Acteur | RevisionActeur: - if not self.data: - raise ValueError("No data provided") - # The parent should already exist in revision or base - # We tolerate absence from revision - result = RevisionActeur.objects.filter(pk=self.id).first() - if not result: - # But if not in revision, must be in base - result = Acteur.objects.get(pk=self.id) - return result - - def apply(self): - acteur = self.validate() - # If acteur is only in base, we need to create a revision - if isinstance(acteur, Acteur): - acteur = RevisionActeur( - identifiant_unique=acteur.identifiant_unique, - acteur_type=acteur.acteur_type, - ) - data = data_reconstruct(RevisionActeur, self.data) - for key, value in data.items(): - setattr(acteur, key, value) - acteur.save() diff --git a/unit_tests/data/models/changes/test_acteur_update_data.py b/unit_tests/data/models/changes/test_acteur_update_data.py deleted file mode 100644 index 4ffd48a8d..000000000 --- a/unit_tests/data/models/changes/test_acteur_update_data.py +++ /dev/null @@ -1,82 +0,0 @@ -""" -Test file for the ChangeActeurUpdateData model. - -""" - -import pytest -from django.contrib.gis.geos import Point -from pydantic import ValidationError - -from dags.sources.config.shared_constants import EMPTY_ACTEUR_FIELD -from data.models.changes.acteur_update_data import ChangeActeurUpdateData -from qfdmo.models.acteur import Acteur, ActeurType, RevisionActeur - - -@pytest.mark.django_db -class TestChangeActeurUpdateData: - def test_name(self): - assert ChangeActeurUpdateData.name() == "acteur_update_data" - - def test_raise_if_no_data_provided(self): - change = ChangeActeurUpdateData(id="dummy", data={}) - with pytest.raises(ValueError, match="No data provided"): - change.apply() - - @pytest.mark.parametrize("data", [None, True]) - def test_raise_if_data_wrong_type(self, data): - with pytest.raises(ValidationError): - ChangeActeurUpdateData(id="dummy", data=data) - - def test_raise_if_acteur_does_not_exist(self): - change = ChangeActeurUpdateData(id="dummy", data={"nom": "test"}) - with pytest.raises(Acteur.DoesNotExist): - change.apply() - - def test_working_case(self): - # We start by creating acteur only in base - at1 = ActeurType.objects.create(code="at1") - at2 = ActeurType.objects.create(code="at2") - base = Acteur.objects.create(nom="test", acteur_type=at1, location=Point(1, 2)) - - # We check that acteur isn't in revision yet - assert RevisionActeur.objects.filter(pk=base.pk).count() == 0 - - # Then we modify its data, notably some Python and Django types - data = {"nom": "test2", "acteur_type": at2.pk, "location": Point(2, 3)} - ChangeActeurUpdateData(id=base.pk, data=data).apply() - - # We check that acteur was created in revision with the right data - rev = RevisionActeur.objects.get(pk=base.pk) - assert rev.nom == "test2" - assert rev.acteur_type == at2 - assert rev.location.x == 2 - assert rev.location.y == 3 - - # We check that acteur in base wasn't modified - base = Acteur.objects.get(pk=base.pk) - assert base.nom == "test" - assert base.acteur_type == at1 - assert base.location.x == 1 - assert base.location.y == 2 - - # If we perform the some changes again, revision is again - # updated AND there is no conflit / creation of a new revision - data = {"nom": "test3"} - ChangeActeurUpdateData(id=base.pk, data=data).apply() - rev = RevisionActeur.objects.get(pk=base.pk) - assert rev.nom == "test3" - assert rev.acteur_type == at2 - assert RevisionActeur.objects.filter(pk=base.pk).count() == 1 - - def test_set_to_empty(self): - # This is a special case for the Crawl URLs DAG - # which prompted to create this change model, so we - # need to ensure that specific case works - at1 = ActeurType.objects.create(code="at1") - base = Acteur.objects.create( - nom="test", acteur_type=at1, location=Point(1, 2), url="https://example.com" - ) - data = {"url": EMPTY_ACTEUR_FIELD} - ChangeActeurUpdateData(id=base.pk, data=data).apply() - rev = RevisionActeur.objects.get(pk=base.pk) - assert rev.url == EMPTY_ACTEUR_FIELD From f8d7f94e55f486f210b7b91e57998319c2018645 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Wed, 14 May 2025 18:27:44 +0200 Subject: [PATCH 10/12] minifix --- templates/data/_partials/value_details.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/data/_partials/value_details.html b/templates/data/_partials/value_details.html index 693887542..88315ed1e 100644 --- a/templates/data/_partials/value_details.html +++ b/templates/data/_partials/value_details.html @@ -47,7 +47,7 @@ {% elif key == "identifiant_unique" or key == "id" %} {{ value }} (base, - rev, + rev, disp) {% elif key == "statut" %} {{ value }} From f9dbbdbda8b16ab4a6449831793130098dd1dc1e Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Thu, 15 May 2025 11:14:08 +0200 Subject: [PATCH 11/12] Remove docker down --- Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile b/Makefile index 8e55140df..6205469d9 100644 --- a/Makefile +++ b/Makefile @@ -57,8 +57,6 @@ run-airflow: .PHONY: run-django run-django: - docker compose --profile airflow down - docker compose --profile lvao down honcho start -f Procfile.django.dev run-all: From 3792b26559bbb49760a21dc184546842b0c093b2 Mon Sep 17 00:00:00 2001 From: Nicolas Oudard Date: Thu, 15 May 2025 11:14:17 +0200 Subject: [PATCH 12/12] Remove docker down --- Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile b/Makefile index 6205469d9..554457f01 100644 --- a/Makefile +++ b/Makefile @@ -60,8 +60,6 @@ run-django: honcho start -f Procfile.django.dev run-all: - docker compose --profile airflow down - docker compose --profile lvao down honcho start -f Procfile.all.dev # Local django operations