BO : afficher le premier onglet en erreur à la validation d’un formulaire (#81845) #138
|
@ -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
|
||||
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 }}
|
||||
|
|
|
@ -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
vdeniaud
commented
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
pmarillonnet
commented
C’était volontaire mais je loupe sans doute un truc : 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
C’était volontaire mais je loupe sans doute un truc : 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
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
pmarillonnet
commented
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))
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Un peu trop de logique côté template à mon goût, je serais pour déplacer ça dans la vue