Prescripteur: ajout de tri dans la liste des candidats [GEN-1844] by xavfernandez · Pull Request #5602 · gip-inclusion/les-emplois

xavfernandez · 2025-02-12T14:58:15Z

🤔 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.

🚨 À vérifier

Mettre à jour le CHANGELOG_breaking_changes.md ?
Ajouter l'étiquette « Bug » ?

🏝️ 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.

💻 Captures d'écran

github-actions · 2025-02-12T15:03:09Z

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

notion-workspace · 2025-02-14T14:47:20Z

🌿 ETQU je veux pouvoir trier mes tableaux par colonne

francoisfreitag

J’imagine qu’il n’a pas été question de multisort ?

francoisfreitag · 2025-02-17T13:39:45Z

+                self.request.GET or {},
                request_user=request.user,
            )
+            self.form.full_clean()  # We do not use is_valid to validate each field independently


Je ne suis vraiment pas convaincu de la nécessité de se casser la tête à éviter le standard form.is_valid() (qui est d’ailleurs recommandé au lieu d’appeler .errors ou full_clean).

On risque de se retrouver à valider le formulaire deux fois, alors que l’appel à is_valid() permet de valider et cacher le résultat, et de centraliser les traitements.

La version actuelle « triche » en appelant full_clean() sans vérifier s’il y a des erreurs, et en traitant uniquement les arguments valides.

D’ailleurs, en faisant ainsi, elle introduit un bug car si le job_seeker est invalide, on va tout de même faire self.form.cleaned_data["job_seeker"].

http://localhost:8000/job-seekers/list?job_seeker=0&order=

(un petit test serait utile 😉)

J’entends bien que pour un formulaire de filtres, ignorer les erreurs est envisageable, mais il me semblerait plus clair pour les devs et les utilisateurs de procéder de la manière suivante :

valider les entrées

afficher les erreurs éventuelles (et une liste de résultats non triée / filtrée)

autrement, appliquer les filtres donnés

Yup, je pense repasser sur is_valid mais aucune idée pour le moment où je vais remonter l'erreur éventuelle de tri 😬 (à part un gros messages.warning) mais bon, ça ne devrait pas se produire

xavfernandez · 2025-02-17T14:47:43Z

J’imagine qu’il n’a pas été question de multisort ?

Non, pas pour le moment 🤞

github-actions · 2025-02-19T13:56:02Z

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

EwenKorr · 2025-02-20T14:53:00Z

                        <th scope="col">Statut du PASS IAE</th>
-                        <th scope="col">Nombre de candidatures</th>
-                        <th scope="col">Dernière mise à jour de candidature</th>
+                        {% include 'common/tables/th_with_sort.html' with order=order ascending_value=order.JOB_APPLICATIONS_NB_ASC name="Nombre de candidatures" only %}


C'est un choix motivé, le tri croissant de nombre de candidatures ?
Instinctivement je pensais que ça allait trier par ordre décroissant.

Mais c'est vrai que ça permet de voir les candidats qui n'ont aucune candidature en premier, et ça peut être utile pour retrouver un candidat qu'on vient de créér par exemple.

Comme discuté, pas vraiment motivé: cela permet de simplifier l'include en considérant le tri par défaut comme étant toujours le tri croissant.

EwenKorr · 2025-02-20T15:08:47Z

        User.objects.filter(kind=UserKind.JOB_SEEKER, pk__in=job_seekers_ids)
        .prefetch_related("approvals")
        .annotate(
+            full_name=Concat(Lower("first_name"), Value(" "), Lower("last_name")),


Peut-être un tri sur le nom de famille plutôt, selon ce que dit le métier ?

EwenKorr · 2025-02-20T15:19:10Z

J'ai vu plein de trucs élégants 🤩

francoisfreitag

Tout propre 🌟

EwenKorr

Rien à ajouter !

since they will be changed in next commit

xavfernandez added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC modifié Modifié dans le changelog. labels Feb 12, 2025

xavfernandez self-assigned this Feb 12, 2025

xavfernandez force-pushed the xfernandez/sort_job_seekers branch 5 times, most recently from e762394 to ce7bd08 Compare February 14, 2025 14:42

xavfernandez marked this pull request as ready for review February 14, 2025 14:43

xavfernandez changed the title ~~Prescripteur: ajout de tri dans la liste des candidats~~ Prescripteur: ajout de tri dans la liste des candidats [GEN-1844] Feb 14, 2025

xavfernandez force-pushed the xfernandez/sort_job_seekers branch from ce7bd08 to 81f7c29 Compare February 14, 2025 15:29

xavfernandez commented Feb 14, 2025

View reviewed changes

Comment thread itou/templates/common/tables/th_with_sort.html Outdated

xavfernandez force-pushed the xfernandez/sort_job_seekers branch from 81f7c29 to e36ec24 Compare February 17, 2025 12:38

xavfernandez requested a review from hellodeloo February 17, 2025 12:39

hellodeloo approved these changes Feb 17, 2025

View reviewed changes

francoisfreitag reviewed Feb 17, 2025

View reviewed changes

xavfernandez force-pushed the xfernandez/sort_job_seekers branch 3 times, most recently from 1aff559 to e594dfc Compare February 18, 2025 08:37

xavfernandez requested a review from francoisfreitag February 18, 2025 11:11

xavfernandez force-pushed the xfernandez/sort_job_seekers branch 2 times, most recently from e16c5b5 to 4c0e851 Compare February 19, 2025 13:46

xavfernandez added 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC and removed 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC labels Feb 19, 2025

xavfernandez force-pushed the xfernandez/sort_job_seekers branch from 4c0e851 to 57e2293 Compare February 19, 2025 16:10

xavfernandez requested a review from EwenKorr February 20, 2025 14:05

EwenKorr reviewed Feb 20, 2025

View reviewed changes

xavfernandez force-pushed the xfernandez/sort_job_seekers branch 2 times, most recently from f11ada4 to accfdfc Compare February 21, 2025 08:24

xavfernandez requested a review from EwenKorr February 21, 2025 08:42

xavfernandez force-pushed the xfernandez/sort_job_seekers branch from dcb83a8 to 1f2df20 Compare February 21, 2025 08:43

francoisfreitag approved these changes Feb 21, 2025

View reviewed changes

Comment thread itou/templates/job_seekers_views/includes/list_results.html Outdated

Comment thread itou/www/job_seekers_views/views.py Outdated

EwenKorr approved these changes Feb 21, 2025

View reviewed changes

tests: snapshot the whole job seekers table

7d0d27b

since they will be changed in next commit

xavfernandez force-pushed the xfernandez/sort_job_seekers branch 2 times, most recently from a3ef626 to f141a25 Compare February 21, 2025 10:03

www.job_seekers: add sort on table headers

51e7540

xavfernandez force-pushed the xfernandez/sort_job_seekers branch from f141a25 to 51e7540 Compare February 21, 2025 10:06

xavfernandez enabled auto-merge February 21, 2025 10:14

xavfernandez added this pull request to the merge queue Feb 21, 2025

Merged via the queue into master with commit 4ecffe7 Feb 21, 2025

xavfernandez deleted the xfernandez/sort_job_seekers branch February 21, 2025 10:25

Conversation

xavfernandez commented Feb 12, 2025

🤔 Pourquoi ?

🍰 Comment ?

🚨 À vérifier

🏝️ Comment tester ?

💻 Captures d'écran

Uh oh!

github-actions Bot commented Feb 12, 2025

Uh oh!

notion-workspace Bot commented Feb 14, 2025

Uh oh!

Uh oh!

francoisfreitag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

francoisfreitag Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

xavfernandez Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xavfernandez commented Feb 17, 2025

Uh oh!

github-actions Bot commented Feb 19, 2025

Uh oh!

Uh oh!

EwenKorr Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

xavfernandez Feb 21, 2025

Choose a reason for hiding this comment

Uh oh!

EwenKorr Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

EwenKorr commented Feb 20, 2025

Uh oh!

francoisfreitag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

EwenKorr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants