Skip to content

Eligibilité: Refonte des vues d'éligibilité - #6223

Merged
tonial merged 9 commits into
masterfrom
alaurent/merge_eligibility_update_views_2
Jun 11, 2025
Merged

Eligibilité: Refonte des vues d'éligibilité#6223
tonial merged 9 commits into
masterfrom
alaurent/merge_eligibility_update_views_2

Conversation

@tonial

@tonial tonial commented May 30, 2025

Copy link
Copy Markdown
Contributor

🤔 Pourquoi ?

Version alternative de #5810 .

Au lieu d'avoir une seule vue avec un template qui contient des tonnes de if en fonction du tunnel, je crée une vue de base pour les 2 vues pour prescripteurs (comme il existe la vue de base pour les 2 vues pour employeurs).

Je me pose la question de la pertinence d'essayer de fusionner les 2 vues de base (employeur / prescripteur)

🍰 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

@tonial tonial self-assigned this May 30, 2025
@tonial tonial added the modifié Modifié dans le changelog. label May 30, 2025
@tonial
tonial changed the base branch from master to alaurent/apply_sessions_5 May 30, 2025 12:57
@tonial tonial changed the title Eligibilité: Refonge des vues d'éligibilité Eligibilité: Refonte des vues d'éligibilité May 30, 2025
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch from 900ee5d to a803e33 Compare May 30, 2025 13:08
@tonial
tonial force-pushed the alaurent/apply_sessions_5 branch from 6e941f4 to 9fb8d7a Compare May 30, 2025 13:11
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch 3 times, most recently from 3f3de15 to d44acf5 Compare May 31, 2025 04:45
@tonial
tonial force-pushed the alaurent/apply_sessions_5 branch from 9fb8d7a to a8585fb Compare June 2, 2025 09:11
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch 2 times, most recently from 617b1c7 to 9c3e292 Compare June 2, 2025 12:07
@tonial
tonial force-pushed the alaurent/apply_sessions_5 branch 2 times, most recently from d4dab3f to fa82802 Compare June 2, 2025 12:08
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch from 9c3e292 to 65ca45d Compare June 2, 2025 12:08
Base automatically changed from alaurent/apply_sessions_5 to master June 2, 2025 12:36
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch from 65ca45d to 92026f0 Compare June 2, 2025 13:41
rsebille
rsebille reviewed Jun 2, 2025
View reviewed changes

@rsebille rsebille left a comment

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.

Découper en plusieurs classes me semble pas mal effectivement, c'est un peu plus de code mais ça évite des conditions complexes.
Par contre je ne lierais pas à un type d'utilisateur (ForPrescriber, ForEmployer) mais plutôt au tunnel (ForHire), afin de ne pas faciliter la supposition que ça sera toujours que ce type d'utilisateur [1] et que si on veux ouvrir ce tunnel à un autre type d'utilisateur il n'y a (théoriquement) que .test_func() à modifier.

[1] On a souvent tendance à le faire et après c'est chiant à détricoter 😅

@tonial

tonial commented Jun 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Les spécificité de la vue dépendent principalement de si l'acteur est un prescripteur ou un employeur:

  • quelles conditions pour voir la vue
  • quels diagnostics peuvent être vus
  • est que l'utilisateur peut mettre à jour le diag

Donc c'est le découpage le plus logique.
Du coup en tentant de fusionner les 2 vues, je me retrouve à mettre du if request.user.is_employer régulièrement....

@tonial tonial mentioned this pull request Jun 2, 2025
Closed
2 tasks
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch 2 times, most recently from ef910ad to 13be996 Compare June 10, 2025 07:57
rsebille
rsebille approved these changes Jun 11, 2025
View reviewed changes

@rsebille rsebille left a comment

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 pas testé en local mais les tests semblent être contents et normalement on a une bonne couverture sur ces parties.

Comment thread itou/www/apply/views/submit_views.py Outdated
Comment thread itou/www/eligibility_views/urls.py
Comment thread itou/www/apply/urls.py Outdated
tonial added 4 commits June 11, 2025 18:16
@tonial
apply: Clean template
75a3ba4 
We only display eligibility step for authorized prescribers
(prescription_process) and if the company requires it
(is_subject_to_eligibility_rules)
@tonial
eligibility: Rename UpdateEligibilityView and template
d6701d5
@tonial
eligibility: Refactor UpdateIAEEligibilityView
149c238 
Add a base view that ApplicationEligibilityView may inherit from
@tonial
apply: Refactor ApplicationEligibilityView
40712c1
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch from 13be996 to 96768a6 Compare June 11, 2025 16:21
@tonial
tonial force-pushed the alaurent/merge_eligibility_update_views_2 branch from 96768a6 to 5cb8128 Compare June 11, 2025 16:26
@tonial
tonial added this pull request to the merge queue Jun 11, 2025
Merged via the queue into master with commit 33672d9 Jun 11, 2025
@tonial
tonial deleted the alaurent/merge_eligibility_update_views_2 branch June 11, 2025 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xavfernandez xavfernandez Awaiting requested review from xavfernandez
@EwenKorr EwenKorr Awaiting requested review from EwenKorr
1 more reviewer
@rsebille rsebille rsebille approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@tonial tonial

Labels

modifié Modifié dans le changelog.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tonial @rsebille