Bloc for modalites #563

JeSuisUnCaillou · 2024-11-27T11:44:40Z

Closes https://linear.app/pole-api/issue/DAT-709/bloc-de-modalite-dacces-dans-le-formulaire

linear · 2024-11-27T11:44:43Z

DAT-709 Bloc de modalité d'accès dans le formulaire

app/helpers/authorization_requests_helpers.rb

skelz0r

cf mes quelques coms + les 2 gros que j'ai fait hors review qui pour moi est le plus important.

app/models/authorization.rb

app/models/authorization_request/api_impot_particulier.rb

app/models/concerns/authorization_extensions/modalities.rb

skelz0r · 2024-11-29T13:16:30Z

app/models/concerns/authorization_extensions/modalities.rb

@@ -0,0 +1,22 @@
+module AuthorizationExtensions::Modalities


ça aurait mérité un commit à part aussi, et de refactor avec API Particulier

API particulier l'utilise comme un array, alors que api impot part l'utilise comme une string.

Par soucis d'en avoir complètement marre de me casser le cul, je propose qu'on refactor pas.

Soit (I.) on refactor en 1 seul module, et on applique, au choix:

API Impot part on transforme le modalities en un vrai array (et on passe des options pour customiser la vue en mode "1 choix ou plusieurs")

On override dans API Impot Part (car on inclut modalities qui implique du pluriel, donc c'est dans la classe où on fait du singulier qu'on doit override). Cela implique d'override la vue aussi (sujet que JB tacle dans sa PR)

Soit (II.) on split en 2 modules : Modalities (pluriel), Modality (singulier) et on duplique le taff

Je ferai I.1. si techniquement ce n'est pas trop compliqué à comprendre, sinon II. (qui est moins dry, mais plus no-brainer)

Bon alors comme va y avoir d'autres forms avec une liste de valeurs possibles (des checkboxes au lieu de radioboutons) va falloir changer mon code pour dtf autoriser un array dans modalities, même pour la dgfip. Donc FLBLBLBL j'avais même pas besoin d'autant m'emmerder avec les radioboutons tout ce temps.

Du coup tout ça tu le mets dans le APIImpotParticulier direct ? (ou son concern), pour éviter que ça soit brainfuck avec API Particulier ?

Je compte importer AuthorizationExtensions::Modalities dans api particulier ainsi que dans DGFIPExtensions::APIImpotParticulierModalities qui l'étend pour rajouter le champ france_connect_authorization_id

je sais, mais là c'est brainfuck d'où ma question

Je viens participer à la conversation (un peu tarfivement), si on part du principe que dans les prochains formulaire DGFiP, on aura d'autres access_mode aka modalité d'acces à integrer, je confirme que l'on aura bien besoin d'autoriser un Array dans modalities (Voir avec les 9 access mode pour l'API Ficoba qui va être mis en prod sur la V1)

Yes je vais m'en occuper dans une PR de followup

app/models/concerns/authorization_extensions/modalities.rb

app/views/authorization_request_forms/shared/_hidden_modalities_fields.html.erb

app/views/authorization_requests/shared/blocks/_modalities.html.erb

JeSuisUnCaillou · 2024-11-29T15:23:46Z

Est-ce qu'on veut valider l'existence de l'authorization france connect linkée dans le modèle ?

skelz0r · 2024-12-02T08:22:17Z

Est-ce qu'on veut valider l'existence de l'authorization france connect linkée dans le modèle ?

Oui, intégrité > *

JeSuisUnCaillou · 2024-12-02T14:34:07Z

Est-ce qu'on peut merge ça comme ça, et que je bosse sur la version avec checkboxes et array dans une autre PR ? @skelz0r

skelz0r

a minima fixer/harden pour ne pas permettre de mettre whatever ce qu'on veut en attributs quand le modèle est persisté.

app/views/authorization_request_forms/_form.html.erb

skelz0r · 2024-12-03T10:52:15Z

app/models/concerns/authorization_extensions/modalities.rb

@@ -0,0 +1,22 @@
+module AuthorizationExtensions::Modalities


Du coup tout ça tu le mets dans le APIImpotParticulier direct ? (ou son concern), pour éviter que ça soit brainfuck avec API Particulier ?

app/helpers/authorization_requests_helpers.rb

app/views/authorization_request_forms/shared/_hidden_attributes.html.erb

skelz0r · 2024-12-03T14:47:51Z

@JeSuisUnCaillou petit conflit faut rebase

app/views/authorization_request_forms/shared/_hidden_params.html.erb

skelz0r

c/c le code du module Modalities dans le code de concern de la DGFIP parce que c'est brainfuck d'avoir un module à part qui a le cul entre 2 chaises.

y'a le new_authorization_request_hidden_params aussi

JeSuisUnCaillou · 2024-12-05T08:36:41Z

c/c le code du module Modalities dans le code de concern de la DGFIP parce que c'est brainfuck d'avoir un module à part qui a le cul entre 2 chaises.

J'ai split les 2 dans l'optique de pouvoir include Modalities directement dans api particulier, donc je peux les fusionner mais ça sera pour les re-split juste derrière, je vois pas bien l'intérêt

…rMissing

- Handle labels and values from convention like other fields - options for classes on intput, fieldset and radio-group elements

Isalafont · 2024-12-05T08:59:03Z

J'ai commencé a regarder le code et j'avoue que j'ai besoin d'un peu plus de temps pour tout bien comprendre.
Mais j'ai une question, que fait on de la première page du formulaire ?

Avec l'implémentation du nouveau bloc de modalité, on en aura plus besoin et surtout l'utilisateur ne comprendra pas pourquoi on lui repose exactement la même question plus tard ?

OK j'ai compris, quand tu selectionnes dans cette page, ça préselectionne également dans le bloc modalities.
Par contre petit side effect. Si on clique france_connect mais que l'on change et clique sur spi, l'habilitation France connect est quand même liée à l'authorization request en back.

JeSuisUnCaillou · 2024-12-05T09:55:11Z

Si on clique france_connect mais que l'on change et clique sur spi, l'habilitation France connect est quand même liée à l'authorization request en back.

Normalement non, j'ai fait gaffe à bien vider le paramètre dans le js. J'arrive pas à reproduire ce que tu dis 🤔

Isalafont · 2024-12-05T13:12:00Z

Normalement non, j'ai fait gaffe à bien vider le paramètre dans le js. J'arrive pas à reproduire ce que tu dis 🤔

Je t'ai induite en erreur sur comment le reproduire, en fait j'avais selectionné france connect sur la premiere page de modalité d'accès et puis j'ai fait le parcours et la j'ai re eut à nouveau un bloc de modalité à selectionner et la j'ai cliqué sur sfip et non France connect comme déjà préselectionné.

Résultat en backend en enregistré, j'ai la modalité with_sfip avec une habilitation France connect attaché.

C'est tricky je sais, mais ne sous estimons pas les utilisateurs pour reproduire ce cas 😅

JeSuisUnCaillou · 2024-12-05T13:16:28Z

Normalement non, j'ai fait gaffe à bien vider le paramètre dans le js. J'arrive pas à reproduire ce que tu dis 🤔

Je t'ai induite en erreur sur comment le reproduire, en fait j'avais selectionné france connect sur la premiere page de modalité d'accès et puis j'ai fait le parcours et la j'ai re eut à nouveau un bloc de modalité à selectionner et la j'ai cliqué sur sfip et non France connect comme déjà préselectionné.

Résultat en backend en enregistré, j'ai la modalité with_sfip avec une habilitation France connect attaché.

C'est tricky je sais, mais ne sous estimons pas les utilisateurs pour reproduire ce cas 😅

Ah ok ! Je vois ce que je peux faire pour éviter ça, merci

skelz0r · 2024-12-05T13:42:42Z

Normalement non, j'ai fait gaffe à bien vider le paramètre dans le js. J'arrive pas à reproduire ce que tu dis 🤔

Je t'ai induite en erreur sur comment le reproduire, en fait j'avais selectionné france connect sur la premiere page de modalité d'accès et puis j'ai fait le parcours et la j'ai re eut à nouveau un bloc de modalité à selectionner et la j'ai cliqué sur sfip et non France connect comme déjà préselectionné.

Résultat en backend en enregistré, j'ai la modalité with_sfip avec une habilitation France connect attaché.

C'est tricky je sais, mais ne sous estimons pas les utilisateurs pour reproduire ce cas 😅

Pour le coup c'est une question d'intégrité de la donnée, ce qui est critique. Bien vu !

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 6 times, most recently from dc26f1f to 78bf6f4 Compare November 29, 2024 11:15

JeSuisUnCaillou marked this pull request as ready for review November 29, 2024 11:31

JeSuisUnCaillou requested a review from skelz0r November 29, 2024 11:31

skelz0r reviewed Nov 29, 2024

View reviewed changes

app/helpers/authorization_requests_helpers.rb Outdated Show resolved Hide resolved

JeSuisUnCaillou force-pushed the bloc_for_modalites branch from 30f1bf3 to c79c9cf Compare November 29, 2024 13:10

skelz0r requested changes Nov 29, 2024

View reviewed changes

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 3 times, most recently from 9894ec0 to 04be184 Compare November 29, 2024 15:05

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 4 times, most recently from 4184842 to 22b34c8 Compare November 29, 2024 16:32

JeSuisUnCaillou requested a review from skelz0r November 29, 2024 16:35

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 2 times, most recently from 393217b to b3fbc3b Compare December 2, 2024 14:25

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 2 times, most recently from 2fd0d9b to 25cb237 Compare December 2, 2024 14:36

skelz0r requested changes Dec 3, 2024

View reviewed changes

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 2 times, most recently from 532257b to 2ef6183 Compare December 3, 2024 12:54

JeSuisUnCaillou force-pushed the bloc_for_modalites branch 3 times, most recently from e3ecfe6 to e90d17c Compare December 3, 2024 13:29

JeSuisUnCaillou requested a review from skelz0r December 3, 2024 14:43

skelz0r reviewed Dec 3, 2024

View reviewed changes

app/views/authorization_request_forms/shared/_hidden_params.html.erb Outdated Show resolved Hide resolved

skelz0r requested changes Dec 3, 2024

View reviewed changes

JeSuisUnCaillou added 6 commits December 5, 2024 09:37

remove useless quotes

f7908ac

add fix-js-lint action to makefile

05cd297

authorization_request_params should return empty params when Paramete…

855dfd7

…rMissing

handle 9 steps in dsfr stepper

05ded47

improve dsfr radio buttons

076b5ee

- Handle labels and values from convention like other fields - options for classes on intput, fieldset and radio-group elements

revoked factory inherits from validated, not submitted

ef142ef

JeSuisUnCaillou force-pushed the bloc_for_modalites branch from e90d17c to e59f18c Compare December 5, 2024 08:38

JeSuisUnCaillou force-pushed the bloc_for_modalites branch from e59f18c to 1a3b24a Compare December 5, 2024 10:00

add modalitites block to api impot particulier

9ba2a98

JeSuisUnCaillou force-pushed the bloc_for_modalites branch from 1a3b24a to 9ba2a98 Compare December 5, 2024 11:43

JeSuisUnCaillou requested a review from skelz0r December 5, 2024 13:09

remove france_connect_authorization_id in model when not needed

f7df638

JeSuisUnCaillou requested a review from Isalafont December 5, 2024 13:38

skelz0r approved these changes Dec 5, 2024

View reviewed changes

JeSuisUnCaillou merged commit 748af33 into develop Dec 5, 2024
11 of 12 checks passed

JeSuisUnCaillou deleted the bloc_for_modalites branch December 5, 2024 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bloc for modalites #563

Bloc for modalites #563

JeSuisUnCaillou commented Nov 27, 2024

linear bot commented Nov 27, 2024

skelz0r left a comment

skelz0r Nov 29, 2024

JeSuisUnCaillou Nov 29, 2024

skelz0r Dec 2, 2024

JeSuisUnCaillou Dec 2, 2024

skelz0r Dec 3, 2024

JeSuisUnCaillou Dec 3, 2024

skelz0r Dec 3, 2024

Isalafont Dec 5, 2024

JeSuisUnCaillou Dec 5, 2024

JeSuisUnCaillou commented Nov 29, 2024 •

edited

Loading

skelz0r commented Dec 2, 2024

JeSuisUnCaillou commented Dec 2, 2024

skelz0r left a comment

skelz0r Dec 3, 2024

skelz0r commented Dec 3, 2024

skelz0r left a comment

JeSuisUnCaillou commented Dec 5, 2024

Isalafont commented Dec 5, 2024 •

edited

Loading

JeSuisUnCaillou commented Dec 5, 2024

Isalafont commented Dec 5, 2024 •

edited

Loading

JeSuisUnCaillou commented Dec 5, 2024

skelz0r commented Dec 5, 2024

Bloc for modalites #563

Bloc for modalites #563

Conversation

JeSuisUnCaillou commented Nov 27, 2024

linear bot commented Nov 27, 2024

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

JeSuisUnCaillou commented Nov 29, 2024 • edited Loading

skelz0r commented Dec 2, 2024

JeSuisUnCaillou commented Dec 2, 2024

skelz0r left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skelz0r commented Dec 3, 2024

skelz0r left a comment

Choose a reason for hiding this comment

JeSuisUnCaillou commented Dec 5, 2024

Isalafont commented Dec 5, 2024 • edited Loading

JeSuisUnCaillou commented Dec 5, 2024

Isalafont commented Dec 5, 2024 • edited Loading

JeSuisUnCaillou commented Dec 5, 2024

skelz0r commented Dec 5, 2024

JeSuisUnCaillou commented Nov 29, 2024 •

edited

Loading

Isalafont commented Dec 5, 2024 •

edited

Loading

Isalafont commented Dec 5, 2024 •

edited

Loading