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

Init a tab for instructors to visualise habilitations #655

Merged
merged 14 commits into from
Jan 20, 2025

Conversation

JeSuisUnCaillou
Copy link
Contributor

Copy link

linear bot commented Jan 13, 2025

@skelz0r
Copy link
Member

skelz0r commented Jan 13, 2025

un quick win est de faire des liens vers les habilitations depuis l'historique, je trouve ça un poill overkill de faire un onglet (alors qu'en vrai... c'est surtout important pour le demandeur et non l'instructeur)

@JeSuisUnCaillou
Copy link
Contributor Author

un quick win est de faire des liens vers les habilitations depuis l'historique, je trouve ça un poill overkill de faire un onglet (alors qu'en vrai... c'est surtout important pour le demandeur et non l'instructeur)

C'est important pour les instructeurs DGFIP de voir les habilitations sandbox & prod d'une demande.

Je vais aussi faire le lien dans l'historique je pense.

@JeSuisUnCaillou
Copy link
Contributor Author

JeSuisUnCaillou commented Jan 13, 2025

Pour le demandeur, lui montrer l'historique c'est un peu overkill je pense. Imo on doit revoir la présentation des demandes et des habilitations pour les séparer, et ça suffira à clarifier le tout.

Les demandes draft, en cours et refusées dans une liste, et les habilitations validées et révoquées dans une autre.

@skelz0r
Copy link
Member

skelz0r commented Jan 13, 2025

un quick win est de faire des liens vers les habilitations depuis l'historique, je trouve ça un poill overkill de faire un onglet (alors qu'en vrai... c'est surtout important pour le demandeur et non l'instructeur)

C'est important pour les instructeurs DGFIP de voir les habilitations sandbox & prod d'une demande.

Je vais aussi faire le lien dans l'historique je pense.

Lien qui existe dans l'historique de la demande côté instructeur.

Pour le demandeur, lui montrer l'historique c'est un peu overkill je pense.

Je n'ai pas d'avis sur l'historique entier, par contre pour le demandeur ça me semble pas déconnant d'avoir un listing des habilitations validées (pour voir les anciennes versions).

Imo on doit revoir la présentation des demandes et des habilitations pour les séparer, et ça suffira à clarifier le tout.

Les demandes draft, en cours et refusées dans une liste, et les habilitations validées et révoquées dans une autre.

imo ça a de la valeur si l'organisation a > 1 demande, séparer les 2 notions je ne suis pas sûr que ça clarifie le tout, je pense même que ça aura l'effet inverse.

Après, l'index des demandes/habilitations c'est à mon sens peu important, 99% des gens vont venir des notifications et donc arriver sur les demandes direct (ou tout du moins on doit pousser vers ça).

tl;dr:

  1. j'ai peu d'avis sur l'index car je pense que ce n'est pas important
  2. vu que l'onglet a du potentiellement du sens pour le demandeur, ça ne coûte en effet pas plus cher de le mettre aussi pour l'instructeur

@JeSuisUnCaillou
Copy link
Contributor Author

Lien qui existe dans l'historique de la demande côté instructeur.

Non ?

@skelz0r
Copy link
Member

skelz0r commented Jan 13, 2025

Lien qui existe dans l'historique de la demande côté instructeur.

Non ?

Oui faut le mettre, mais c'est déjà mentionné, suffit d'ajouter le lien justement (ce qui est faisable facilement vs refaire une page entière).

@JeSuisUnCaillou
Copy link
Contributor Author

imo ça a de la valeur si l'organisation a > 1 demande, séparer les 2 notions je ne suis pas sûr que ça clarifie le tout, je pense même que ça aura l'effet inverse.

Si dans la même page y'a écrit "Liste de mes demandes" et en dessous "Liste de mes habilitations", et que dans le cas où une liste est vide on affiche pas la section, je pense que ça sera pas confusant.

@JeSuisUnCaillou
Copy link
Contributor Author

Oui faut le mettre, mais c'est déjà mentionné, suffit d'ajouter le lien justement (ce qui est faisable facilement vs refaire une page entière).

Je pense que ça vaut le coup d'avoir une page qui résume simplement la liste des habilitations, versus les avoir noyées dans un historique d'allers retours qui peut être long, surtout dans les cas de la dgfip.

@skelz0r
Copy link
Member

skelz0r commented Jan 13, 2025

imo ça a de la valeur si l'organisation a > 1 demande, séparer les 2 notions je ne suis pas sûr que ça clarifie le tout, je pense même que ça aura l'effet inverse.

Si dans la même page y'a écrit "Liste de mes demandes" et en dessous "Liste de mes habilitations", et que dans le cas où une liste est vide on affiche pas la section, je pense que ça sera pas confusant.

as u ouiche, c'est je pense du détail pour les raisons évoquées plus haut (cf lien)

Oui faut le mettre, mais c'est déjà mentionné, suffit d'ajouter le lien justement (ce qui est faisable facilement vs refaire une page entière).

Je pense que ça vaut le coup d'avoir une page qui résume simplement la liste des habilitations, versus les avoir noyées dans un historique d'allers retours qui peut être long, surtout dans les cas de la dgfip.

A/R qui n'existent pas vraiment vu qu'ils refusent et ne demande pas de modifications 😅
mais again, ça a du sens imo pour le demandeur dans la vue show, donc ça coûte pas plus cher pour l'instructeur (qui est censé être "expert" vu qu'il instruit)


btw, sur le listing des habilitations, tu vas mettre toutes les habilitations, ou seulement la dernière ? pour une habilitation donnée tu vas omettre la demande dans la liste ? si non, tu vas mettre les 2 ?

@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch from 4d17c7b to ebbcd59 Compare January 13, 2025 14:32
@JeSuisUnCaillou
Copy link
Contributor Author

Let me cook for a while please, j'ai pas fini c'est juste un draft, on reprend la conversation lorsque je la passerait en PR 🙏

@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch 11 times, most recently from 74893bb to 3070820 Compare January 15, 2025 15:35
@JeSuisUnCaillou JeSuisUnCaillou marked this pull request as ready for review January 15, 2025 15:39
@JeSuisUnCaillou
Copy link
Contributor Author

JeSuisUnCaillou commented Jan 15, 2025

J'ai pour l'instant uniquement géré l'information minimum que je pense qu'il faut pour que l'APIM de la DGFiP puisse bosser + quelques fixs en passant.

Des screens :

Screenshot from 2025-01-15 16-39-13

☝️ Consulter l'habilitation dans l'historique


Screenshot from 2025-01-15 16-39-01

☝️ Le show d'une habilitation


Screenshot from 2025-01-15 16-38-45

☝️ La liste des habilitations d'une demande

@JeSuisUnCaillou
Copy link
Contributor Author

Je m'occuperai de faire le display équivalent pour les demandeurs dans une PR followup, puis j'ajouterai carrément la séparation plus claire des demandes et habilitations dans l'UI dans une autre PR ensuite.

@skelz0r
Copy link
Member

skelz0r commented Jan 15, 2025

(le "durant la demande N" est pas clair)

Copy link
Member

@skelz0r skelz0r left a comment

Choose a reason for hiding this comment

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

Y'a des petits trucs/questions mais globalement c'est OK

app/models/authorization.rb Outdated Show resolved Hide resolved
app/models/authorization.rb Outdated Show resolved Hide resolved
app/policies/authorization_request_policy.rb Show resolved Hide resolved
features/instructeurs/consultation_habilitation.feature Outdated Show resolved Hide resolved
features/instructeurs/historique_habilitation.feature Outdated Show resolved Hide resolved
spec/factories/authorization_requests.rb Outdated Show resolved Hide resolved
@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch from 60e71a3 to c1f882c Compare January 20, 2025 08:41
@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch 4 times, most recently from 626a3ff to 606da2b Compare January 20, 2025 09:27
@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch 2 times, most recently from 7fbb745 to dff5a7f Compare January 20, 2025 09:53
@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch from dff5a7f to c173f81 Compare January 20, 2025 10:07
@JeSuisUnCaillou JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch from c173f81 to ca16c4a Compare January 20, 2025 10:17
@JeSuisUnCaillou JeSuisUnCaillou merged commit 1fecc05 into develop Jan 20, 2025
13 checks passed
@JeSuisUnCaillou JeSuisUnCaillou deleted the display_all_habilitations_of_a_request branch January 20, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants