Candidature : Modification des filtres Eligibilité IAE et PASS IAE dans la liste de candidatures [GEN-1182] - #5746
Conversation
48319a0 to
8f9ded1
Compare
| ).valid() | ||
| ) | ||
|
|
||
| def eligibility_validated(self): |
There was a problem hiding this comment.
En effet !
Le plus simple je pense serait de fusionner cette présente PR (jeudi ?), puis je rebase, non ?
There was a problem hiding this comment.
Pas sûr qu'elle soit mergée avant la tienne 😅
| queryset = queryset.eligibility_validated() | ||
|
|
||
| if self.cleaned_data.get("eligibility_pending"): | ||
| queryset = queryset.eligibility_pending() |
There was a problem hiding this comment.
Un utilisateur peut tout à fait cocher les deux cases (eligibility_validated & eligibility_pending).
Dans ce cas je m'attendrais à retrouver toutes les candidatures.
Du coup, il faudrait faire:
# If both or none of eligibility_validated & eligibility_pending are checked, no need to filter anything
if self.cleaned_data.get("eligibility_validated") and not self.cleaned_data.get("eligibility_pending"):
queryset = queryset.eligibility_validated()
elif self.cleaned_data.get("eligibility_pending") and not self.cleaned_data.get("eligibility_validated"):
queryset = queryset.eligibility_pending()
There was a problem hiding this comment.
Bien vu, effectivement c'est un OU et non un ET pour les autres filtres (statut candidature, fiches de poste). Je m'aligne.
There was a problem hiding this comment.
Je vais tenter un joli match bien DRY.
| assert response.context["job_applications_page"].object_list == [] | ||
| response = client.get(reverse("apply:list_for_siae"), {"eligibility_pending": True}) | ||
| assert set(response.context["job_applications_page"].object_list) == set([job_app, _another_job_app]) | ||
|
|
There was a problem hiding this comment.
Il faudrait rajouter un test où eligibility_pending & eligibility_validated sont tous les deux cochées.
| pass_status_filter |= Q(has_suspended_approval=True) | ||
| filters.append(pass_status_filter) | ||
|
|
||
| if data.get("pass_iae_expired"): |
There was a problem hiding this comment.
Pareil, pass_iae_expired est à intégrer dans le bloc if data.get("pass_iae_active") or data.get("pass_iae_suspended"):.
Et suivre la même logique qu'ici: https://github.com/gip-inclusion/les-emplois/pull/5670/files#diff-5eca69df6e9e7f0cfc8d48a61b5824e0c10f46f4c947b1b33f3351a0006f0b95R98-R102
There was a problem hiding this comment.
Après ici, on a le gros soucis de "De quel PASS IAE parle-t-on ?" celui de la candidature ou celui du candidat ?
Actuellement, les filtres ne s'occupent que du PASS de la candidature qui n'existe que pour les candidatures ayant été au moins une fois acceptée...
There was a problem hiding this comment.
Ok je m'aligne ici aussi sur un OU logique. Pour la question à 100 francs "De quel PASS IAE parle-t-on ?" je la joue conservateur en restant sur la logique existante (PASS de la candidature).
8f9ded1 to
175c406
Compare
738b1b1 to
a0d8816
Compare
|
Je veux bien une nouvelle revue stp @xavfernandez 🙏. Je te conseille de regarder les 3 derniers des 5 commits. |
3 participants
🤔 Pourquoi ?
Aujourd’hui, on a ça, il manque la case “Eligibilité à valider” et le statut du PASS IAE “expiré” :
🍰 Comment ?
Dans Eligibilité IAE :
On ajoute une case “A valider”
On renomme “Eligibilité validée” en “Valide”
Dans statut du PASS IAE :
On ajoute une case “Expiré”
Résultat :
Notes pour la revue technique
Je ne suis historiquement ni familier ni à l'aise avec la logique assez complexe des PASS et diagnostics. Du coup cette PR est un bon exercice pour moi, mais attention des choses restent floues pour moi, je m'explique ci-dessous.
Je recommande de lire chacun des deux commits séparement. Ce ticket regroupe deux nouveaux filtres qui sont techniquement complètement indépendants.
Notes sur le filtre PASS IAE archivé
Je me base sur un
filters.append(Q(approval__end_at__lt=timezone.localdate())).En pratique cela montre bien les candidatures des candidats qui ont un ou plusieurs PASS expirés mais cela montre aussi quelques candidats qui ont 2 PASS, 1 valide et 1 expiré.
L'UI ne mentionnant dans ce cas que le PASS valide cela peut être déroutant pour l'utilisateur. Ou pas, je ne sais pas. Peut-être qu'on veut vivre avec ces "faux positifs". Ou peut-être qu'on veut montrer seulement les candidats avec seulement des PASS expirés. Je veux bien une seconde opinion d'un dev plus à l'aise sur le sujet.
Notes sur le filtre éligibilité à valider
Après réflexion j'ai choisi de considérer que c'était simplement le contraire de l'existant "éligibilité valide" (voir
eligibility_validated_lookup).Il y avait peut-être une autre possibilité que je n'ai pas retenue :
with_jobseeker_valid_eligibility_diagnosis().filter(jobseeker_valid_eligibility_diagnosis=None)Je veux bien là aussi une seconde opinion. 🙏