BO : afficher le premier onglet en erreur à la validation d’un formulaire (#81845) #138

pmarillonnet · 2023-10-02T14:07:04+02:00

pmarillonnet commented

2023-10-02 14:07:04 +02:00

No description provided.

pmarillonnet force-pushed wip/81845-bo-form-error-on-hidden-tab from 7f0600ede8 to 406162c16c

2023-10-02 15:44:18 +02:00

Compare

pmarillonnet changed title from ~~WIP: BO : afficher le premier onglet en erreur à la validation d’un formulaire (#81845)~~ to BO : afficher le premier onglet en erreur à la validation d’un formulaire (#81845)

2023-10-02 15:51:13 +02:00

vdeniaud requested changes 2023-10-05 18:05:52 +02:00

vdeniaud left a comment

Je pense qu'on peut s'en sortir en décidant quel onglet est sélectionné directement dans la vue, pour qu'ensuite niveau template on ait juste à vérifier un attribut booléen genre form.selected.

Un truc genre

--- a/src/authentic2/apps/authenticators/views.py
+++ b/src/authentic2/apps/authenticators/views.py
@@ -114,18 +114,23 @@ class AuthenticatorEditView(AuthenticatorsMixin, MultipleFormsUpdateView):
             form.tab_slug = slugify(label)
             form.is_not_default = build_tab_is_not_default(form)
             forms.append(form)
+
+        forms[0].selected = True
         return forms
 
     def post(self, request, *args, **kwargs):
         self.object = self.get_object()
         forms = self.get_forms()
 
-        all_valid = all(form.is_valid() for form in forms)
-        if all_valid:
+        invalid_forms = [form for form in forms if not form.is_valid()]
+        if not invalid_forms:
             for form in forms:
                 form.save()
             self.request.journal.record('authenticator.edit', forms=forms)
             return HttpResponseRedirect(self.get_success_url())
+        else:
+            forms[0].selected = False
+            invalid_forms[0].selected = True
 
         return self.render_to_response(self.get_context_data(forms=forms))

Je pense qu'on peut s'en sortir en décidant quel onglet est sélectionné directement dans la vue, pour qu'ensuite niveau template on ait juste à vérifier un attribut booléen genre `form.selected`. Un truc genre ```diff --- a/src/authentic2/apps/authenticators/views.py +++ b/src/authentic2/apps/authenticators/views.py @@ -114,18 +114,23 @@ class AuthenticatorEditView(AuthenticatorsMixin, MultipleFormsUpdateView): form.tab_slug = slugify(label) form.is_not_default = build_tab_is_not_default(form) forms.append(form) + + forms[0].selected = True return forms def post(self, request, *args, **kwargs): self.object = self.get_object() forms = self.get_forms() - all_valid = all(form.is_valid() for form in forms) - if all_valid: + invalid_forms = [form for form in forms if not form.is_valid()] + if not invalid_forms: for form in forms: form.save() self.request.journal.record('authenticator.edit', forms=forms) return HttpResponseRedirect(self.get_success_url()) + else: + forms[0].selected = False + invalid_forms[0].selected = True return self.render_to_response(self.get_context_data(forms=forms)) ```

src/authentic2/apps/authenticators/templates/authentic2/authenticators/authenticator_edit_form.html Outdated

						
				@ -20,1 +15,3 @@

				                  {% if form.is_not_default %}class="pk-tabs--button-marker"{% endif %}>{{ form.tab_name }}</button>

				          {% with is_first_selected=forloop.first|yesno:"true,false" %}

				            <button role="tab"

				                    aria-selected="{% if erroneous_forms %}{% if form is first_erroneous_form %}true{% else %}false{% endif %}{% else %}{{ is_first_selected }}{% endif %}"

vdeniaud commented

2023-10-05 18:01:37 +02:00

Un peu trop de logique côté template à mon goût, je serais pour déplacer ça dans la vue

vdeniaud marked this conversation as resolved

src/authentic2/apps/authenticators/views.py Outdated

						
				@ -124,0 +126,4 @@

				            if not form.is_valid():

				                first_erroneous_form = form

				                all_valid = False

				                break

vdeniaud commented

2023-10-05 18:02:27 +02:00

On ne valide plus tous les formulaires, c'est une (petite) régression

pmarillonnet commented

2023-10-10 09:53:05 +02:00

C’était volontaire mais je loupe sans doute un truc :
lorsque l’un des formulaires (i.e. l’un des onglets) est invalide, on sait d’ores et déjà que l’usager va devoir corriger ce qu’il y a saisi (et que bien sûr il va revalider le formulaire sans parcourir les onglets suivants pour y chercher d’autres erreurs).
Que les onglets suivants soient en état de validité inconnue, on s’en fiche, non ? L’usager de toute façon s’y serait repris à plusieurs fois quand bien même ceux-ci eussent été en état d’invalidité avérée, tu crois pas ?

C’était volontaire mais je loupe sans doute un truc : lorsque l’un des formulaires (i.e. l’un des onglets) est invalide, on sait d’ores et déjà que l’usager va devoir corriger ce qu’il y a saisi (et que bien sûr il va revalider le formulaire sans parcourir les onglets suivants pour y chercher d’autres erreurs). Que les onglets suivants soient en état de validité inconnue, on s’en fiche, non ? L’usager de toute façon s’y serait repris à plusieurs fois quand bien même ceux-ci eussent été en état d’invalidité avérée, tu crois pas ?

pmarillonnet commented

2023-10-10 09:53:08 +02:00

C’était volontaire mais je loupe sans doute un truc :
lorsque l’un des formulaires (i.e. l’un des onglets) est invalide, on sait d’ores et déjà que l’usager va devoir corriger ce qu’il y a saisi (et que bien sûr il va revalider le formulaire sans parcourir les onglets suivants pour y chercher d’autres erreurs).
Que les onglets suivants soient en état de validité inconnue, on s’en fiche, non ? L’usager de toute façon s’y serait repris à plusieurs fois quand bien même ceux-ci eussent été en état d’invalidité avérée, tu crois pas ?

C’était volontaire mais je loupe sans doute un truc : lorsque l’un des formulaires (i.e. l’un des onglets) est invalide, on sait d’ores et déjà que l’usager va devoir corriger ce qu’il y a saisi (et que bien sûr il va revalider le formulaire sans parcourir les onglets suivants pour y chercher d’autres erreurs). Que les onglets suivants soient en état de validité inconnue, on s’en fiche, non ? L’usager de toute façon s’y serait repris à plusieurs fois quand bien même ceux-ci eussent été en état d’invalidité avérée, tu crois pas ?

vdeniaud commented

2023-10-10 16:17:06 +02:00

Oui c'est pour ça que je disais petite, en tout cas j'ai suggéré un patch qui n'a pas cet inconvénient et qui me paraît vraiment plus simple

pmarillonnet commented

2023-10-19 10:37:39 +02:00

Ok je pensais que la “(petite) régression” était une touche d’ironie de ta part, je cherchais la grosse régression, en vain :)

Je regarde comment appliquer ton patch.

Ok je pensais que la “(petite) régression” était une touche d’ironie de ta part, je cherchais la grosse régression, en vain :) Je regarde comment appliquer ton patch.

vdeniaud marked this conversation as resolved

pmarillonnet force-pushed wip/81845-bo-form-error-on-hidden-tab from 406162c16c to 90accc3369

2023-10-19 10:38:32 +02:00

Compare

pmarillonnet force-pushed wip/81845-bo-form-error-on-hidden-tab from 90accc3369 to 5145950be9

2023-10-19 10:51:44 +02:00

Compare

pmarillonnet commented

2023-10-19 10:53:50 +02:00

Bien vu, nettement plus simple ainsi, en effet, merci.

pmarillonnet requested review from vdeniaud 2023-10-19 10:53:54 +02:00

vdeniaud approved these changes 2023-10-30 11:25:48 +01:00

pmarillonnet force-pushed wip/81845-bo-form-error-on-hidden-tab from 5145950be9 to 90b6502263

2023-10-30 11:38:45 +01:00

Compare

pmarillonnet commented

2023-10-30 11:54:23 +01:00

Ça vient de pair avec la modif’ JS côté gadjo, #81867, relue et modifiée une première fois mais pas encore validée. J’attendrai que ce soit validé de ce côté là pour merger ici.

pmarillonnet commented

2023-10-30 11:55:07 +01:00

Il y a la suite logique de cette PR aussi, i.e. appliquer cela à une séparation en onglet de la configuration des clients d’API, #81334, bon pour relecture aussi.

gitea/authentic/pipeline/head This commit looks good

Details

This pull request has changes conflicting with the target branch.

tests/test_manager_authenticators.py

You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.

git checkout -b wip/81845-bo-form-error-on-hidden-tab main

git pull origin wip/81845-bo-form-error-on-hidden-tab

Step 2:

Merge the changes and update on Gitea.

git checkout main

git merge --no-ff wip/81845-bo-form-error-on-hidden-tab

git push origin main

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#138