From 7c58d1df0d6e2e2b746abcf9ccec099fc4bdc043 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 9 May 2019 18:51:56 +0200 Subject: [PATCH 1/2] forms: implement locked fields by renaming and widget change (#32954) It simplifies the code (no need to implement a special clean() method) and it covers the case of field with widget not supporting the readonly HTML attribute like those based on tags. --- src/authentic2/forms/mixins.py | 63 +++++++++++++++++++++++++++++++++ src/authentic2/forms/profile.py | 36 ++++++++++--------- tests/test_profile.py | 41 ++++++++++++++------- 3 files changed, 111 insertions(+), 29 deletions(-) create mode 100644 src/authentic2/forms/mixins.py diff --git a/src/authentic2/forms/mixins.py b/src/authentic2/forms/mixins.py new file mode 100644 index 00000000..da8de2a0 --- /dev/null +++ b/src/authentic2/forms/mixins.py @@ -0,0 +1,63 @@ +# authentic2 - versatile identity manager +# Copyright (C) 2010-2019 Entr'ouvert +# +# This program is free software: you can redistribute it and/or modify it +# under the terms of the GNU Affero General Public License as published +# by the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from collections import OrderedDict + +from django import forms + + +class LockedFieldFormMixin(object): + def __init__(self, *args, **kwargs): + super(LockedFieldFormMixin, self).__init__(*args, **kwargs) + self.__lock_fields() + + def __lock_fields(self): + # Locked fields are modified to use a read-only TextInput + # widget remapped to a name which will be ignored by Form + # implementation + locked_fields = {} + for name in self.fields: + if not self.is_field_locked(name): + continue + field = self.fields[name] + initial = self.initial[name] + try: + choices = field.choices + except AttributeError: + initial = field.widget.format_value(initial) + else: + for key, label in choices: + if initial == key: + initial = label + break + locked_fields[name] = forms.CharField( + label=field.label, + help_text=field.help_text, + initial=initial, + widget=forms.TextInput(attrs={'readonly': ''})) + if not locked_fields: + return + + new_fields = OrderedDict() + for name in self.fields: + if name in locked_fields: + new_fields[name + '@disabled'] = locked_fields[name] + else: + new_fields[name] = self.fields[name] + self.fields = new_fields + + def is_field_locked(self, name): + raise NotImplementedError diff --git a/src/authentic2/forms/profile.py b/src/authentic2/forms/profile.py index 5dd81a0d..3d3e9f86 100644 --- a/src/authentic2/forms/profile.py +++ b/src/authentic2/forms/profile.py @@ -14,6 +14,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from collections import OrderedDict from django.forms.models import modelform_factory as dj_modelform_factory from django import forms @@ -22,6 +23,7 @@ from django.utils.translation import ugettext_lazy as _, ugettext from ..custom_user.models import User from .. import app_settings, models from .utils import NextUrlFormMixin +from .mixins import LockedFieldFormMixin class DeleteAccountForm(forms.Form): @@ -66,7 +68,7 @@ class EmailChangeForm(EmailChangeFormNoPassword): return password -class BaseUserForm(forms.ModelForm): +class BaseUserForm(LockedFieldFormMixin, forms.ModelForm): error_messages = { 'duplicate_username': _("A user with that username already exists."), } @@ -76,23 +78,25 @@ class BaseUserForm(forms.ModelForm): self.attributes = models.Attribute.objects.all() initial = kwargs.setdefault('initial', {}) - if kwargs.get('instance'): - instance = kwargs['instance'] - for av in models.AttributeValue.objects.with_owner(instance): - if av.attribute.name in self.declared_fields: - if av.verified: - self.declared_fields[av.attribute.name].widget.attrs['readonly'] = 'readonly' - initial[av.attribute.name] = av.to_python() + instance = kwargs.get('instance') + # extended attributes are not model fields, their initial value must be + # explicitely defined + self.atvs = [] + self.locked_fields = set() + if instance: + self.atvs = models.AttributeValue.objects.select_related('attribute').with_owner(instance) + for atv in self.atvs: + name = atv.attribute.name + if name in self.declared_fields: + initial[name] = atv.to_python() + # helper data for LockedFieldFormMixin + if atv.verified: + self.locked_fields.add(name) super(BaseUserForm, self).__init__(*args, **kwargs) - def clean(self): - from authentic2 import models - - # make sure verified fields are not modified - for av in models.AttributeValue.objects.with_owner( - self.instance).filter(verified=True): - self.cleaned_data[av.attribute.name] = av.to_python() - super(BaseUserForm, self).clean() + def is_field_locked(self, name): + # helper method for LockedFieldFormMixin + return name in self.locked_fields def save_attributes(self): # only save non verified attributes here diff --git a/tests/test_profile.py b/tests/test_profile.py index 8482fa0a..76f412aa 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -35,12 +35,11 @@ def test_account_edit_view(app, simple_user): resp = app.get(url, status=200) resp = app.post(url, params={ - 'csrfmiddlewaretoken': resp.form['csrfmiddlewaretoken'].value, - 'edit-profile-first_name': resp.form['edit-profile-first_name'].value, - 'edit-profile-last_name': resp.form['edit-profile-last_name'].value, - 'edit-profile-phone': '1234' - }, - status=302) + 'csrfmiddlewaretoken': resp.form['csrfmiddlewaretoken'].value, + 'edit-profile-first_name': resp.form['edit-profile-first_name'].value, + 'edit-profile-last_name': resp.form['edit-profile-last_name'].value, + 'edit-profile-phone': '1234' + }, status=302) # verify that missing next_url in POST is ok assert resp['Location'].endswith(reverse('account_management')) assert attribute.get_value(simple_user) == '1234' @@ -57,19 +56,15 @@ def test_account_edit_view(app, simple_user): attribute.set_value(simple_user, '0123456789', verified=True) resp = app.get(url, status=200) - resp.form.set('edit-profile-phone', '1234567890') - assert 'readonly' in resp.form['edit-profile-phone'].attrs + assert 'edit-profile-phone' not in resp.form.fields + assert 'readonly' in resp.form['edit-profile-phone@disabled'].attrs resp = resp.form.submit().follow() assert attribute.get_value(simple_user) == '0123456789' - resp = app.get(url, status=200) - assert 'phone' in resp - assert 'readonly' in resp.form['edit-profile-phone'].attrs - attribute.disabled = True attribute.save() resp = app.get(url, status=200) - assert 'phone' not in resp + assert 'edit-profile-phone@disabled' not in resp assert attribute.get_value(simple_user) == '0123456789' @@ -135,3 +130,23 @@ def test_account_edit_scopes(app, simple_user): resp = app.get(reverse('profile_edit_with_scope', kwargs={'scope': 'address'}), status=200) assert get_fields(resp) == set(['city', 'zipcode', 'next_url']) + + +def test_account_edit_locked_title(app, simple_user): + Attribute.objects.create( + name='title', label='title', + kind='title', user_visible=True, user_editable=True) + simple_user.attributes.title = 'Monsieur' + + utils.login(app, simple_user) + url = reverse('profile_edit') + response = app.get(url, status=200) + assert len(response.pyquery('input[type="radio"][name="edit-profile-title"]')) == 2 + assert len(response.pyquery('input[type="radio"][name="edit-profile-title"][readonly="true"]')) == 0 + assert len(response.pyquery('select[name="edit-profile-title"]')) == 0 + + simple_user.verified_attributes.title = 'Monsieur' + + response = app.get(url, status=200) + assert len(response.pyquery('input[type="radio"][name="edit-profile-title"]')) == 0 + assert len(response.pyquery('input[type="text"][name="edit-profile-title@disabled"][readonly]')) == 1 -- 2.20.1