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

Open
pmarillonnet wants to merge 1 commits from wip/81845-bo-form-error-on-hidden-tab into main
3 changed files with 23 additions and 4 deletions

View File

@ -13,7 +13,7 @@
<div class="pk-tabs--tab-list" role="tablist" aria-label="{% trans "Configuration tabs" %}">
{% for form in forms %}
<button role="tab"
aria-selected="{{ forloop.first|yesno:"true,false" }}"
aria-selected="{{ form.selected|yesno:"true,false" }}"
aria-controls="panel-{{ form.tab_slug }}"
vdeniaud marked this conversation as resolved Outdated

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

Un peu trop de logique côté template à mon goût, je serais pour déplacer ça dans la vue
id="tab-{{ form.tab_slug }}"
tabindex="{{ forloop.first|yesno:"0,-1" }}"
@ -28,7 +28,7 @@
{% if forms|length > 1 %}
{% for form in forms %}
<div id="panel-{{ form.tab_slug }}"
role="tabpanel" tabindex="0" {% if not forloop.first %}hidden{% endif %}
role="tabpanel" tabindex="0" {{ form.selected|yesno:",hidden" }}
data-tab-slug="{{ form.tab_slug }}"
aria-labelledby="tab-{{ form.tab_slug }}">
{{ form|with_template }}

View File

@ -114,18 +114,22 @@ 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())
vdeniaud marked this conversation as resolved Outdated

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

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

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 ?

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 ?

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

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

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.
else:
forms[0].selected = False
invalid_forms[0].selected = True
return self.render_to_response(self.get_context_data(forms=forms))

View File

@ -263,6 +263,21 @@ def test_authenticators_oidc(app, superuser, ou1, ou2):
assert 'Issuer: https://oidc.example.com' in resp.text
assert 'Scopes: profile email' in resp.text
# only the first tab is selected and displayed by default
resp = app.get('/manage/authenticators/%s/edit/' % provider.pk)
assert resp.pyquery('#tab-general')[0].get('aria-selected') == 'true'
assert resp.pyquery('#tab-advanced')[0].get('aria-selected') == 'false'
assert 'hidden' not in resp.pyquery('#panel-general')[0].keys()
assert 'hidden' in resp.pyquery('#panel-advanced')[0].keys()
# but a validation error on second tab gives the latter precedence over the former
resp.form['max_auth_age'] = -2 # will fail at validation time
resp = resp.form.submit()
assert resp.pyquery('#tab-general')[0].get('aria-selected') == 'false'
assert resp.pyquery('#tab-advanced')[0].get('aria-selected') == 'true'
assert 'hidden' in resp.pyquery('#panel-general')[0].keys()
assert 'hidden' not in resp.pyquery('#panel-advanced')[0].keys()
resp = app.get('/manage/authenticators/')
assert 'OpenID Connect - Test' in resp.text
assert 'class="section disabled"' in resp.text