Init a tab for instructors to visualise habilitations #655

JeSuisUnCaillou · 2025-01-13T12:16:06Z

Closes https://linear.app/pole-api/issue/API2-340/afficher-les-habilitations-dune-demande

linear · 2025-01-13T12:16:09Z

API2-340 Afficher les habilitations d'une demande

skelz0r · 2025-01-13T12:23:43Z

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 · 2025-01-13T14:04:50Z

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 · 2025-01-13T14:06:28Z

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 · 2025-01-13T14:14:45Z

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:

j'ai peu d'avis sur l'index car je pense que ce n'est pas important
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 · 2025-01-13T14:18:02Z

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

Non ?

skelz0r · 2025-01-13T14:20:17Z

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 · 2025-01-13T14:20:37Z

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 · 2025-01-13T14:27:09Z

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 · 2025-01-13T14:32:32Z

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 · 2025-01-13T14:34:11Z

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 · 2025-01-15T15:41:36Z

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 :

☝️ Consulter l'habilitation dans l'historique

☝️ Le show d'une habilitation

☝️ La liste des habilitations d'une demande

JeSuisUnCaillou · 2025-01-15T15:42:10Z

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 · 2025-01-15T15:43:17Z

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

skelz0r

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

skelz0r · 2025-01-15T16:18:29Z

app/models/authorization.rb

@@ -22,6 +22,10 @@ class Authorization < ApplicationRecord
    inverse_of: :authorization,
    dependent: :destroy

+  has_many :authorization_request_event,


Suggested change

has_many :authorization_request_event,

has_many :authorization_request_events,

(et en fait avec le commentaire du dessous imo ça peut jarter ici)

skelz0r · 2025-01-15T16:21:42Z

app/models/authorization.rb

+  def approving_instructor
+    authorization_request_event
+      .where(name: 'approve')
+      .order(created_at: :desc)
+      .first
+      .try(:user)
+  end


Suggested change

def approving_instructor

authorization_request_event

.where(name: 'approve')

.order(created_at: :desc)

.first

.try(:user)

end

has_one :approved_authorization_request_event, -> { where(name: 'approve').order(created_at: :asc).limit(1) },

class_name: 'AuthorizationRequest'

has_one :approving_instructor,

through: :approved_authorization_request_event,

class_name: 'User'

pas sûr de la syntaxe. Je pense que c'est mieux d'avoir une relation

Sinon:

def approving_instructor authorization_request_event .where(name: 'approve') .order(created_at: :asc) .limit(1) .first .try(:user) end

app/policies/authorization_request_policy.rb

skelz0r · 2025-01-15T16:31:05Z

app/views/authorization_requests/shared/_reopening_callout.html.erb

@@ -6,5 +6,5 @@
  <p class="fr-callout__text">
    <%= t(".text.#{type}") %>
  </p>
-  <%= link_to t('.link.text', authorization_id: @authorization_request.latest_authorization.id), latest_authorization_path(@authorization_request), class:"fr-btn" %>
+  <%= link_to t('.link.text', authorization_slug: @authorization_request.latest_authorization.slug), latest_authorization_path(@authorization_request), class:"fr-btn" %>


Si il y plusieurs validations le même jour, le slug sera DATE--1. Pour moi il faut utiliser le created_at

Suggested change

<%= link_to t('.link.text', authorization_slug: @authorization_request.latest_authorization.slug), latest_authorization_path(@authorization_request), class:"fr-btn" %>

<%= link_to t('.link.text', authorization_created_at_date: @authorization_request.latest_authorization.created_at.to_date), latest_authorization_path(@authorization_request), class:"fr-btn" %>

skelz0r · 2025-01-15T16:33:52Z

app/views/instruction/authorization_request_events/_authorization_request_event.html.erb

-  <%= time_tag authorization_request_event.created_at do %>
-    <%= authorization_request_event.created_at.strftime("%d/%m/%Y") %>
-  <% end %>
+    <% if authorization_request_event.name == 'approve' %>


Je suis tiraillé sur le fait d'avoir mis ce morceau ici plutôt que dans le décorateur: d'un côté c'est plus simple et ça reste quand même une info importante à afficher, mais du coup on a une partie de logique vue dans le décorateur et ici.

En l'état ça me va, mais j'ai peur que ça devienne peu maintenable à l'avenir.

ceci est donc une simple remarque.

skelz0r · 2025-01-15T16:35:00Z

features/instructeurs/consultation_habilitation.feature

@@ -19,3 +20,37 @@ Fonctionnalité: Instruction: consultation d'une habilitation
    Quand je me rends sur une demande d'habilitation "API Entreprise" réouverte
    Alors il y a un bouton "Consulter l'habilitation"

+  Scénario: Je ne vois pas d'onglet pour consulter les habilitations d'une demande sans habilitations


Suggested change

Scénario: Je ne vois pas d'onglet pour consulter les habilitations d'une demande sans habilitations

Scénario: Je ne vois pas d'onglet pour consulter les habilitations d'une demande sans habilitation

features/instructeurs/consultation_habilitation.feature

skelz0r · 2025-01-15T16:36:57Z

features/instructeurs/historique_habilitation.feature

@@ -92,3 +92,12 @@ Fonctionnalité: Instruction: historique habilitation
    Et que je clique sur "Historique"
    Alors la page contient "Les données suivantes ont été modifiées par rapport aux informations pré-remplies du formulaire"

+  @DisableBullet
+  Scénario: Je un lien vers l'habilitation sur l'évènement de validation


Suggested change

Scénario: Je un lien vers l'habilitation sur l'évènement de validation

Scénario: Je vois un lien vers l'habilitation sur l'évènement de validation

Ce test ne teste pas qu'il y a un lien.

Et la page contient un lien vers "/instructeur/habilitations/"

un truc du genre --^

skelz0r · 2025-01-15T16:38:30Z

spec/factories/authorization_requests.rb

@@ -182,6 +182,7 @@
          applicant: authorization_request.applicant,
          authorization_request_class: authorization_request.definition.stage.previous_stages[0][:definition].authorization_request_class,
          data: authorization_request.data.presence || { 'what' => 'ever' },
+          created_at: authorization_request.created_at - 1.day


Par ce que sinon l'ordre des created_at n'est pas le bon. Ca faisait une ancienne authorization qui est plus récente. Et ça m'ennuyait dans l'ordre de display des habilitations dans la liste.

Le souci ici est que c'est étrange d'avoir une demande d'habilitation qui est plus récente qu'une habilitation.
Possible de créer la demande 1 jour avant à la place ?

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

JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch 3 times, most recently from 6fabd33 to 6d1fce0 Compare January 15, 2025 12:23

Add a tab for instructors to visualise habilitations

f10713d

JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch 4 times, most recently from 3b80fe8 to 2ca006b Compare January 15, 2025 13:58

JeSuisUnCaillou added 5 commits January 15, 2025 15:04

Fix title of habilitation : don't display form informations

625f3a6

display id of habilitation next to the date

ac7dbf5

authorization button on approve event in request history

dfda5d9

improve history UI with flex and better margins

6efec27

don't use habilitation id for the demandeur

f2a47fc

JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch 3 times, most recently from db305af to 2e74cff Compare January 15, 2025 15:04

JeSuisUnCaillou added 2 commits January 15, 2025 16:35

fix policy for next_stage and the start_next_stage margin

85433c0

AuthorizationRequest#latest_authorization_of_class

71af264

JeSuisUnCaillou added 3 commits January 15, 2025 16:35

Authorization#latest? considers stages

fec5f93

test that we hide warning of old authorization when it's just a sandbox

8a658f9

display approving instructor on authorizations

3070820

JeSuisUnCaillou force-pushed the display_all_habilitations_of_a_request branch from 74893bb to 3070820 Compare January 15, 2025 15:35

JeSuisUnCaillou marked this pull request as ready for review January 15, 2025 15:39

improve subtitle of authorization

60e71a3

JeSuisUnCaillou requested a review from skelz0r January 15, 2025 15:51

skelz0r requested changes Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Init a tab for instructors to visualise habilitations #655

Init a tab for instructors to visualise habilitations #655

JeSuisUnCaillou commented Jan 13, 2025

linear bot commented Jan 13, 2025

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025 •

edited

Loading

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

JeSuisUnCaillou commented Jan 15, 2025 •

edited

Loading

JeSuisUnCaillou commented Jan 15, 2025

skelz0r commented Jan 15, 2025

skelz0r left a comment

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

skelz0r Jan 15, 2025

JeSuisUnCaillou Jan 16, 2025

skelz0r Jan 16, 2025

	has_many :authorization_request_event,
	has_many :authorization_request_events,

	<%= link_to t('.link.text', authorization_slug: @authorization_request.latest_authorization.slug), latest_authorization_path(@authorization_request), class:"fr-btn" %>
	<%= link_to t('.link.text', authorization_created_at_date: @authorization_request.latest_authorization.created_at.to_date), latest_authorization_path(@authorization_request), class:"fr-btn" %>

	Scénario: Je ne vois pas d'onglet pour consulter les habilitations d'une demande sans habilitations
	Scénario: Je ne vois pas d'onglet pour consulter les habilitations d'une demande sans habilitation

	Scénario: Je un lien vers l'habilitation sur l'évènement de validation
	Scénario: Je vois un lien vers l'habilitation sur l'évènement de validation

Init a tab for instructors to visualise habilitations #655

Are you sure you want to change the base?

Init a tab for instructors to visualise habilitations #655

Conversation

JeSuisUnCaillou commented Jan 13, 2025

linear bot commented Jan 13, 2025

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025 • edited Loading

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

skelz0r commented Jan 13, 2025

JeSuisUnCaillou commented Jan 13, 2025

JeSuisUnCaillou commented Jan 15, 2025 • edited Loading

JeSuisUnCaillou commented Jan 15, 2025

skelz0r commented Jan 15, 2025

skelz0r left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeSuisUnCaillou commented Jan 13, 2025 •

edited

Loading

JeSuisUnCaillou commented Jan 15, 2025 •

edited

Loading