Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Création de compte candidat : déconnecter la modification de compte du parcours apply #5177

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

EwenKorr
Copy link
Contributor

@EwenKorr EwenKorr commented Nov 29, 2024

🤔 Pourquoi ?

Dans l'optique de Créer un compte candidat depuis l'espace Mes candidats, on Extrait la création de compte candidat du parcours de candidature.

Dans cette PR, on déconnecte les étapes de modification de candidat, pour que ces vues ne dépendent plus de classes dans apply.

Il y a un peu plus de changements que dans les vues précédentes, puisque les vues de Update* utilisaient une session un peu différente. Les URLs utilisaient le job_seeker.public_id.

On introduit ici le concept de vue start, comme il existe pour apply. Le bloc apply dirige vers cette vue lorsqu'il s'agit de modifier un compte candidat, et c'est cette vue qui initialise la session.

À noter :
Pour ne pas casser les parcours de candidature en cours en production, nous devons temporairement avoir 2 jeux d'URLs fonctionnels. Les deux fonctionnements sont trop différents, on a également 2 jeux de vues.


Les autres étapes : https://www.notion.so/plateforme-inclusion/Extraire-le-parcours-de-cr-ation-de-compte-candidat-130e8fa5c35b80b9947cea2573cf90e7?pvs=4#130e8fa5c35b800b966fdd4722014657

@EwenKorr EwenKorr added the no-changelog Ne doit pas figurer dans le journal des changements. label Nov 29, 2024
@EwenKorr EwenKorr self-assigned this Nov 29, 2024
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 2 times, most recently from 949cadb to 421d9eb Compare November 29, 2024 16:29
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 2 times, most recently from ad489ff to cfc65f2 Compare December 3, 2024 16:28
@EwenKorr
Copy link
Contributor Author

EwenKorr commented Dec 3, 2024

Grosse PR en apparence, mais il y a encore plein de copiers-collers, pour des vues trop différentes pour être utilisées avec 2 systèmes d'URLs.
Et j'ai voulu couvrir les 2 versions de tests, ce qui duplique pas mal de code aussi.

Je vais séparer les commits, en faire au moins 1 temporaire/fixup pour pouvoir facilement lire les différences.

@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 11 times, most recently from f6733c7 to 0e3160e Compare December 5, 2024 16:02
itou/www/apply/views/process_views.py Outdated Show resolved Hide resolved
tests/gps/test_create_beneficiary.py Outdated Show resolved Hide resolved
itou/www/apply/views/submit_views.py Outdated Show resolved Hide resolved
itou/www/apply/views/submit_views.py Outdated Show resolved Hide resolved
itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
itou/www/apply/views/submit_views.py Outdated Show resolved Hide resolved
itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
itou/www/job_seekers_views/views.py Show resolved Hide resolved
Comment on lines +933 to +944
if not request.user.is_authenticated:
# Do nothing, LoginRequiredMixin will raise in dispatch()
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comportement précédent :

Ce bout de code n'était présent que dans Step1 : si non connecté, on sort de setup() et le dispatch() s'occupe du reste. Si on est connecté, on init une session.

Dans les étapes suivantes : si on n'a pas de session, on a une erreur 403.


Ce comportement ne fonctionne plus, vu qu'on initialise la session avant d'arriver à la page.
Un peu plus bas, on a besoin de self.user.can_edit_personal_information(), donc j'ai forcé la vérification de la connexion dans le setup(), comme à l'étape 1.

Ce qui devait être, j'imagine, un petit hack, se retrouve copié plusieurs fois. Il faudra peut-être retravailler ce qui se fait dans les setup() et dispatch() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On sera sauvé par #5178 si je ne me trompe pas :)

Copy link
Contributor Author

@EwenKorr EwenKorr Dec 5, 2024

Choose a reason for hiding this comment

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

Difficile à relire. Je ne sais pas trop comment améliorer ça. C'est déjà en 2 étapes :

  1. nouvelles URLs/vues
  2. réinjection des URLs/vues dépréciées

Ça permet de lire un peu plus facilement la PR.

@EwenKorr EwenKorr marked this pull request as ready for review December 5, 2024 16:07
@EwenKorr
Copy link
Contributor Author

EwenKorr commented Dec 5, 2024

J'ai déjà hâte d'enlever les vues/URLs dépréciées 😅

D'ailleurs @xavfernandez je vois des # TODO(xfernandez): remove birthdate in a week dans job_seekers_views.views. Ce serait à nettoyer aussi ?

@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 3 times, most recently from e91653d to 4f54c16 Compare December 6, 2024 10:04
@EwenKorr
Copy link
Contributor Author

EwenKorr commented Dec 6, 2024

Mmh, je remarque un petit comportement dérangeant : quand on valide un des formulaires, on enregistre en session. Si on annule, on sort du bloc, mais la session contient toujours les informations modifiées. Lorsqu'on retourne dans le bloc, ce sont les informations modifiées (en session) qu'on voit.

Il faudrait soit :

  • ajouter à Step1 un mode où on réécrit les données en session (ou en miroir, un mode "retour" où on ne réécrit pas les données)
  • supprimer la session quand on clique sur Annuler => en faire un bouton POST ?

Edit : le fonctionnement avec la vue Start règle ce problème, une nouvelle session est crée !

@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch from 4f54c16 to ab3018a Compare December 6, 2024 13:04
@EwenKorr EwenKorr marked this pull request as draft December 9, 2024 14:51
@EwenKorr
Copy link
Contributor Author

EwenKorr commented Dec 9, 2024

Vu avec @xavfernandez cet après-midi :
Pour ne pas avoir à trop modifier www.apply (créer une nouvelle session pour afficher chaque bouton "Mettre à jour les informations" etc), on va introduire dans cette PR une vue start.

On va modifier un peu les URLs aussi :
au lieu de <uuid:session_uuid>/hire/update/1, on aurait update/start (voire update/ tout court pour la vue Start) , puis update/<uuid_session_uuid>/1.

L'info sur le tunnel irait dans la session.

@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 3 times, most recently from 8406d5e to 79b75c0 Compare December 13, 2024 12:23
itou/www/job_seekers_views/views.py Show resolved Hide resolved
Comment on lines +933 to +944
if not request.user.is_authenticated:
# Do nothing, LoginRequiredMixin will raise in dispatch()
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On sera sauvé par #5178 si je ne me trompe pas :)

] or [None]
return job_seeker_session_key

def get_step_url(self, step, client):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On change les URLs, maintenant il y a le nom de la session (un UUID) dans l'URL. Plutôt que d'avoir des attributs dans cette classe de tests, j'utilise des méthodes pour retrouver les URLs et aussi le nom de la session.

Comment on lines 3486 to 3503
for url in [
self.step_1_url,
self.step_2_url,
self.step_3_url,
self.step_end_url,
]:
response = client.get(url)
assert response.status_code == 403
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ça n'a plus trop du sens de vérifier ces URLs, vu qu'il faut un UUID correct de session.
Il n'y a pas de risque de sécurité, ou bien je me trompe ?

itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
@EwenKorr EwenKorr marked this pull request as ready for review December 13, 2024 12:33
itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
itou/templates/apply/submit/application/base.html Outdated Show resolved Hide resolved
@EwenKorr
Copy link
Contributor Author

Notes de micro-réunion :

  • pour simplifier les vues : si on n'affiche pas d'info, on ne vérifie pas (les tests dans dispatch)
  • is_gps : a priori pas besoin, donc plutôt assert pour être sûr qu'on n'est pas en GPS

@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch from 79b75c0 to 30be648 Compare December 19, 2024 13:29
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch from 30be648 to e4c7986 Compare December 19, 2024 16:17
itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
tests/www/apply/test_submit.py Outdated Show resolved Hide resolved
tests/www/apply/test_submit.py Outdated Show resolved Hide resolved
tests/www/apply/test_submit.py Outdated Show resolved Hide resolved
tests/www/apply/test_submit.py Outdated Show resolved Hide resolved
itou/www/job_seekers_views/views.py Outdated Show resolved Hide resolved
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 2 times, most recently from 8d398a8 to 7093256 Compare January 6, 2025 14:24
At this point, when leaving CheckNir views for
CheckJobSeekerInformations, we are leaving the job_seeker_views
block. Deleting the job_seeker_session when leaving avoids getting
too many unused sessions.
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch from 7093256 to 0954c9d Compare January 6, 2025 15:34
The UpdateJobSeeker* views do not inherit from `apply` views anymore.
The URL is changed to use the `session_uuid` parameter. The same set of
URLs is used for applying and for hiring. For those Update views, the
session is initialized by a new `start` view.
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch 3 times, most recently from b6899a0 to 46810fe Compare January 7, 2025 13:55
This commit allows two things:
- prevent errors in production (b/c continuous integration)
- keep track of the deprecated code that will have to be removed when we
  are assured that the old URLs aren't in use anymore
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch from 46810fe to 1ccbb61 Compare January 7, 2025 14:13
…ed for

When initializing the job_seeker_session, we set a `tunnel` key to
determine which action will be performed in the job_seekers_views block
with that session (here, `job-seeker-update`).

This is to prevent a (malicious) user from playing with session, and
using a session that was not intended for updating a job seeker to do
so.
@EwenKorr EwenKorr force-pushed the ewen/compte-candidat-deco-update branch from 1ccbb61 to 7357926 Compare January 7, 2025 16:25
self.job_seeker_session = SessionNamespace.create_uuid_namespace(
request.session,
data={
"config": {"from_url": from_url, "tunnel": "job-seeker-update"},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je me pose la question de la pertinence du nom.
On parlait tout à l'heure de kind ou session_kind, c'est peut-être plus générique ?

Copy link
Contributor

Choose a reason for hiding this comment

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

J'ai une préférence pour kind ou session_kind plutôt que tunnel oui :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Ne doit pas figurer dans le journal des changements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants