Skip to content

Candidature : API Particulier -> interface#4786

Merged
celine-m-s merged 9 commits into
masterfrom
celinems/api-particulier-ui
Oct 17, 2024
Merged

Candidature : API Particulier -> interface#4786
celine-m-s merged 9 commits into
masterfrom
celinems/api-particulier-ui

Conversation

@celine-m-s
Copy link
Copy Markdown
Contributor

@celine-m-s celine-m-s commented Sep 20, 2024

⚠️ Je recommande une relecture commit par commit.

🤔 Pourquoi ?

En tant qu'employeur (SIAE ou GEIQ) ayant embauché un candidat pour lequel j'ai réalisé moi-même le diagnostic d’éligibilité, je souhaite voir les critères qui ont été certifiés par l'API Particulier dans la page du diagnostic.

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 ?

  • Réorganisation du gabarit qui affiche les critères.
  • Calcul de la validité de la certification dans une Queryset pour qu'il soit réutilisable ailleurs (dans le contrôle a posteriori par exemple).

🚨 À vérifier

  • Mettre à jour le CHANGELOG_breaking_changes.md ?

🏝️ Comment tester ?

En tant qu'employeur (SIAE ou GEIQ), réaliser une auto-prescription pour un candidat connu de l'API Particulier avec le critère BRSA.
Dans la page de candidature, un macaron « certifié » devrait apparaître au-dessus de l'encart informatif.
Réaliser un test pour un candidat n'ayant aucun critère ou n'était pas connu de l'API.
Pour aller plus vite, il est possible de modifier directement dans l'admin Django les valeurs retournées par l'API Particulier. C'est pratique pour vérifier la période de grâce (les critères certifiés même si la date de candidature n'entre pas parfaitement dans leur date de validité).

@celine-m-s celine-m-s self-assigned this Sep 20, 2024
@celine-m-s celine-m-s changed the base branch from master to celinems/api-particuliers September 20, 2024 12:54
@celine-m-s celine-m-s force-pushed the celinems/api-particuliers branch 4 times, most recently from bc5e5d6 to 3cc4049 Compare September 22, 2024 18:14
@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch from 731bdc9 to 5907a62 Compare September 22, 2024 18:38
@celine-m-s celine-m-s added the ajouté Ajouté dans le changelog. label Sep 22, 2024
@celine-m-s celine-m-s marked this pull request as draft September 23, 2024 05:50
@notion-workspace
Copy link
Copy Markdown

@celine-m-s celine-m-s force-pushed the celinems/api-particuliers branch 3 times, most recently from 7bba6dc to 5cfd213 Compare September 25, 2024 12:04
@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch from 5907a62 to 3aec9a3 Compare September 27, 2024 13:58
@celine-m-s celine-m-s force-pushed the celinems/api-particuliers branch 3 times, most recently from fa3c986 to 86d23d4 Compare October 1, 2024 12:48
Base automatically changed from celinems/api-particuliers to master October 1, 2024 13:08
@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch from 3aec9a3 to ba2ceaa Compare October 1, 2024 16:09
@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch 2 times, most recently from 3de38eb to c44e088 Compare October 10, 2024 15:31
@celine-m-s celine-m-s added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Oct 10, 2024
@github-actions
Copy link
Copy Markdown

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

@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch from c44e088 to 0282559 Compare October 11, 2024 07:43
@celine-m-s celine-m-s marked this pull request as ready for review October 11, 2024 07:56
Copy link
Copy Markdown
Contributor

@leo-naeka leo-naeka left a comment

Choose a reason for hiding this comment

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

La plupart des mes retours sont des bricoles.
Reste à trancher le grace time en durée absolue ou relative.

Également je ne sais pas dans quelle mesure les badges vont apparaître (j'ai testé via l'admin en recette), mais si les employeurs n'en voient pas au début, sans contexte, c'est difficile de comprendre la présence de l'encart :

Capture d’écran 2024-10-16 à 00 16 18

J'aurais rajouté une phrase style "Les critères certifiés seront indiqués par un badge <insérer le badge certifié>." dans l'encart

Details Capture d’écran 2024-10-16 à 00 26 57

Si en revanche il y aura déjà quasi systématiquement des badges pour au moins un critère, c'est tout bon 😇

Comment thread itou/eligibility/models/common.py
Comment thread itou/templates/apply/includes/eligibility_diagnosis.html
Comment thread itou/templates/apply/includes/eligibility_diagnosis.html Outdated
Comment thread itou/templates/apply/includes/eligibility_diagnosis.html Outdated
Comment thread itou/templates/apply/includes/eligibility_diagnosis.html Outdated
Comment thread tests/www/apply/test_templates.py
Comment thread tests/www/apply/test_templates.py Outdated
Comment thread tests/www/apply/test_templates.py Outdated
Comment thread tests/www/apply/test_templates.py Outdated
Comment thread itou/eligibility/models/common.py Outdated
Comment thread itou/eligibility/models/common.py Outdated
@celine-m-s
Copy link
Copy Markdown
Contributor Author

En fait, nous avons des données ! 572 critères sont marqués comme certifiés en base.
Effectivement, toutes les périodes commencent le premier du mois et finissent le premier du quatrième mois. Cela étant, c'est valable pour le RSA mais pas pour les autres.
Je vais en discuter avec les UX mais ça ne me semble pas bloquant pour cette PR. Un réajustement pourra être fait très rapidement par la suite pour aller vers une prise en compte par mois et non par jour.

{% endif %}

{% if diagnosis.criteria_can_be_certified %}
<div class="c-box bg-info-lightest my-4 p-0">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

J'ai l'impression que cela ressemble très fort à ce qu'il y a dans itou/templates/apply/includes/eligibility_diagnosis.html: ça pourrait aller dans un include à part ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tu as raison. J'ai isolé dans un autre gabarit et j'en ai profité pour le faire aussi pour les critères.

Comment thread tests/www/apply/test_templates.py Outdated
Copy link
Copy Markdown
Member

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Tout me semble y être, je suis sûr que les employeurs vont beaucoup apprécier !

Pas convaincu par tests/www/apply/test_templates.py, j’ai mis des commentaires pour expliquer ce que je lui reproche.

Comment thread itou/eligibility/models/geiq.py
Comment thread itou/www/apply/views/submit_views.py Outdated
Comment thread itou/www/apply/views/submit_views.py Outdated
Comment thread itou/www/employees_views/views.py Outdated
Comment thread itou/templates/apply/includes/eligibility_diagnosis.html Outdated
Comment thread tests/www/apply/test_templates.py Outdated
Comment thread tests/www/apply/test_templates.py
Comment thread tests/www/apply/test_templates.py Outdated


@pytest.mark.ignore_unknown_variable_template_error("request")
class TestCertifiedBadgeGEIQ:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Il y a un parametrize qui démange ici.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

J'y ai pensé mais les gabarits sont légèrement différents et les objets passés dans le contexte le sont encore plus. J'ai préféré différencier les deux au lieu de continuer de mettre des conditions un peu partout.

<li>
<span>{{ criterion.administrative_criteria.name }}</span>
{% if eligibility_diagnosis.is_from_employer and criterion.is_considered_certified %}
<span class="badge badge-sm rounded-pill bg-info-lighter text-info ms-3 me-1">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pas besoin de marge après, c’est le dernier élément.

Suggested change
<span class="badge badge-sm rounded-pill bg-info-lighter text-info ms-3 me-1">
<span class="badge badge-sm rounded-pill bg-info-lighter text-info ms-3">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Et si on veut bien aligner le badge avec le nom du critère:

diff --git a/itou/templates/apply/includes/eligibility_diagnosis.html b/itou/templates/apply/includes/eligibility_diagnosis.html
index bc6a28c6b..6418e08be 100644
--- a/itou/templates/apply/includes/eligibility_diagnosis.html
+++ b/itou/templates/apply/includes/eligibility_diagnosis.html
@@ -46,13 +46,15 @@
                             <ul>
                                 {% for criterion in criteria %}
                                     <li>
-                                        <span>{{ criterion.administrative_criteria.name }}</span>
-                                        {% if eligibility_diagnosis.is_from_employer and criterion.is_considered_certified %}
-                                            <span class="badge badge-sm rounded-pill bg-info-lighter text-info ms-3 me-1">
-                                                <i class="ri-verified-badge-fill" aria-hidden="true"></i>
-                                                Certifié
-                                            </span>
-                                        {% endif %}
+                                        <div class="d-flex align-items-center">
+                                            <span>{{ criterion.administrative_criteria.name }}</span>
+                                            {% if eligibility_diagnosis.is_from_employer and criterion.is_considered_certified %}
+                                                <span class="badge badge-sm rounded-pill bg-info-lighter text-info ms-3">
+                                                    <i class="ri-verified-badge-fill" aria-hidden="true"></i>
+                                                    Certifié
+                                                </span>
+                                            {% endif %}
+                                        </div>
                                     </li>
                                 {% endfor %}
                             </ul>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Merci !

@francoisfreitag
Copy link
Copy Markdown
Member

Effectivement, toutes les périodes commencent le premier du mois et finissent le premier du quatrième mois. Cela étant, c'est valable pour le RSA mais pas pour les autres.

À voir si on n’implémenterait pas ces règles directement, tant qu’à faire.

@celine-m-s
Copy link
Copy Markdown
Contributor Author

À voir si on n’implémenterait pas ces règles directement, tant qu’à faire.

J'en discute avec le métier aujourd'hui.

@celine-m-s
Copy link
Copy Markdown
Contributor Author

@francoisfreitag @leo-naeka J'ai discuté avec le métier. Ils souhaitent rester sur un système de jours pour que l'ajout de nouveaux critères soit simple. Ils ont voulu ajouter deux jours pour prendre en compte les mois extra-longs (juin + juillet + août = 92).

Comment thread itou/www/apply/views/process_views.py Outdated
Copy link
Copy Markdown
Member

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

J’ai refait une passe rapide, ça me semble bien 👍

@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch 2 times, most recently from 0bb4af1 to 02e6a10 Compare October 17, 2024 15:09
@celine-m-s
Copy link
Copy Markdown
Contributor Author

Retours traités ! Je mets en prod ce soir.

It's easier to read.
The related name is the same for GEIQ and IAE's diagnosis
to share the same interface
(eligibility_diagnosis.selected_administrative_criteria).
A criteria may have been valid in the past but not when a job really
begins.
Consider criteria are certified only in regard to a
job application hiring date
@celine-m-s celine-m-s force-pushed the celinems/api-particulier-ui branch from 02e6a10 to bc20046 Compare October 17, 2024 20:04
@celine-m-s celine-m-s added this pull request to the merge queue Oct 17, 2024
Merged via the queue into master with commit 1d88170 Oct 17, 2024
@celine-m-s celine-m-s deleted the celinems/api-particulier-ui branch October 17, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC ajouté Ajouté dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants