GEIQ: Bilan d'exécution #4110

xavfernandez · 2024-05-21T12:14:40Z

🤔 Pourquoi ?

Indiquez le problème que nous sommes en train de résoudre et les objectifs métiers ou techniques qui sont visés par ces changements.

🍰 Comment ?

Décrivez en quelques mots la solution retenue et mise en oeuvre, les difficultés ou problèmes rencontrés. Attirez l'attention sur les décisions d'architecture ou de conception importantes.

💻 Captures d'écran

🚨 À vérifier

Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ Comment tester

Les instructions pour reproduire le problème, les profils de test, le parcours spécifique à utiliser, etc. Si vous disposez d'une recette jetable, mettre l'URL pour tester dans cette partie.

github-actions · 2024-05-21T12:21:10Z

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

francoisfreitag

La synchro est un peu compliquée à cause des défauts de l’API, mais le code est super propre ! 👍

itou/geiq/models.py

itou/geiq/sync.py

francoisfreitag · 2024-06-13T09:12:44Z

itou/geiq/sync.py

+            current_end = new_end
+        else:  # new_start > current_end
+            # new distinct period found: count the current one and switch to the new one
+            nb_days += (current_end - current_start).days + 1


J’espère qu’il est bien documenté que la date de fin est inclusive…

Pas documenté mais ça me semblait évident 😬 .
Et si ce n'est pas le cas, on nous le dira 😇 .

itou/geiq/sync.py

itou/utils/apis/geiq_label.py

francoisfreitag

C’est top, les vues donnent vraiment l’impression d’être une transformation bête et méchante des données, la complexité étant déjà gérée dans le mécanisme de sync. Les permissions m’ont l’air OK, et c’est très simple à suivre 💯

itou/geiq/admin.py

itou/templates/geiq/assessment_info_for_employer.html

francoisfreitag · 2024-06-13T13:41:26Z

itou/templates/geiq/assessment_info_for_employer.html

+                        </form>
+                    </div>
+                </div>
+                <div class="col-12 col-lg-4">


J’espère que le commentaire de la DDETS sera court…

francoisfreitag · 2024-06-13T14:18:21Z

itou/www/geiq_views/urls.py

+        name="employee_details",
+    ),
+    path(
+        "list/<int:institution_pk>",


Ne faudrait-il pas prévoir de passer l’assessment dans l’URL pour historiser ces données ?

francoisfreitag · 2024-06-13T14:31:49Z

itou/www/geiq_views/views.py

+            logger.warning("Error while syncing Label data for assessement=%s", assessment)
+            messages.error(request, "Erreur lors de la synchronisation avec Label.")
+
+    match info_type:


francoisfreitag · 2024-06-13T14:36:44Z

itou/www/geiq_views/views.py

+        employees = Employee.objects.filter(
+            assessment__company_id__in={org.pk for org in request.organizations},
+        ).select_related("assessment__campaign")
+    employee = get_object_or_404(employees, pk=employee_pk)


Juste pour confirmer, le même employé ne devrait pas apparaître dans deux assessments différents, parce qu’on devrait recréer un employé à chaque fois ? (cas DDETS)

Alors un même employé peut apparaître dans plusieurs ImplementationAssessments différents mais cela sera sous la forme de 2 objets Employee différents chacun rattaché à son ImplementationAssessment.

francoisfreitag · 2024-06-13T14:41:46Z

tests/www/dashboard/tests.py

+    ):
+        membership = InstitutionMembershipFactory(institution__kind=institution_kind)
+        user = membership.user
+        self.client.force_login(user)


Romain m’a fait remarqué qu’on peut passer l’assertion directement dans le parametrize.

francoisfreitag · 2024-06-13T14:46:10Z

tests/www/geiq_views/tests.py

+    response = client.post(reverse("geiq:label_sync", kwargs={"assessment_pk": assessment.pk}))
+    assessment.refresh_from_db()
+    assert assessment.last_synced_at is not None
+    assertContains(response, "Dernière synchronisation:")


Ce serait aussi le cas s’il n’y avait pas eu de dernière synchronisation (le texte serait Dernière synchronisation: -). Peut-être mettre la date ?

tests/www/geiq_views/tests.py

francoisfreitag

Gros gros boulot, mais finalement suffisamment digeste pour qu’on suive bien.
Sans connaissance de l’API Label et ses petites facéties, difficile de trouver des cas qui mettront en défaut le code. Mais au moins, je suis plutôt serein sur le fait que les erreurs nous remonteront rapidement, et surtout qu’il sera facile d’intervenir. 🎩

xavfernandez added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC ajouté Ajouté dans le changelog. labels May 21, 2024

xavfernandez self-assigned this May 21, 2024

xavfernandez force-pushed the xfernandez/api_label branch 10 times, most recently from 9854f7a to 251a901 Compare May 27, 2024 14:53

xavfernandez force-pushed the xfernandez/api_label branch 16 times, most recently from 0ddf52a to 45cd526 Compare June 6, 2024 21:27

xavfernandez force-pushed the xfernandez/api_label branch 3 times, most recently from f5f2321 to daff556 Compare June 13, 2024 08:59

xavfernandez requested a review from francoisfreitag June 13, 2024 09:13

xavfernandez force-pushed the xfernandez/api_label branch from daff556 to 95610b2 Compare June 13, 2024 09:14

xavfernandez added no-changelog Ne doit pas figurer dans le journal des changements. and removed no-changelog Ne doit pas figurer dans le journal des changements. labels Jun 13, 2024

francoisfreitag reviewed Jun 13, 2024

View reviewed changes

xavfernandez force-pushed the xfernandez/api_label branch from 015dbff to 9abceaf Compare June 13, 2024 12:33

francoisfreitag reviewed Jun 13, 2024

View reviewed changes

francoisfreitag approved these changes Jun 13, 2024

View reviewed changes

xavfernandez force-pushed the xfernandez/api_label branch 2 times, most recently from 5fe1da8 to cd8bf65 Compare June 14, 2024 07:41

xavfernandez marked this pull request as ready for review June 14, 2024 12:03

xavfernandez force-pushed the xfernandez/api_label branch from eb5d6ee to 524e86f Compare June 17, 2024 09:13

xavfernandez added 2 commits June 17, 2024 14:13

geiq: add models for implementation assessment

0ccee4b

geiq: add sync from Label

099d76d

xavfernandez force-pushed the xfernandez/api_label branch from 524e86f to 9d24cae Compare June 17, 2024 12:19

xavfernandez added this pull request to the merge queue Jun 17, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 17, 2024

xavfernandez force-pushed the xfernandez/api_label branch from 9d24cae to cac2e0c Compare June 17, 2024 12:45

xavfernandez added 3 commits June 17, 2024 14:46

geiq: add admin interfaces

1116f88

geiq_views: add views for implementation assessment

0de2133

dashboard: conditional admonition

5d2aa15

xavfernandez force-pushed the xfernandez/api_label branch from cac2e0c to 5d2aa15 Compare June 17, 2024 12:46

xavfernandez enabled auto-merge June 17, 2024 12:49

xavfernandez added this pull request to the merge queue Jun 17, 2024

Merged via the queue into master with commit 11a1481 Jun 17, 2024
10 of 11 checks passed

xavfernandez deleted the xfernandez/api_label branch June 17, 2024 13:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GEIQ: Bilan d'exécution #4110

GEIQ: Bilan d'exécution #4110

xavfernandez commented May 21, 2024

github-actions bot commented May 21, 2024

francoisfreitag left a comment

francoisfreitag Jun 13, 2024

xavfernandez Jun 13, 2024

francoisfreitag left a comment

francoisfreitag Jun 13, 2024

francoisfreitag Jun 13, 2024

francoisfreitag Jun 13, 2024

francoisfreitag Jun 13, 2024

xavfernandez Jun 14, 2024

francoisfreitag Jun 13, 2024

francoisfreitag Jun 13, 2024

francoisfreitag left a comment

GEIQ: Bilan d'exécution #4110

GEIQ: Bilan d'exécution #4110

Conversation

xavfernandez commented May 21, 2024

🤔 Pourquoi ?

🍰 Comment ?

💻 Captures d'écran

🚨 À vérifier

🏝️ Comment tester

github-actions bot commented May 21, 2024

francoisfreitag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

francoisfreitag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

francoisfreitag left a comment

Choose a reason for hiding this comment