From 6691d5645253055fdee03b26780a1ca8d2e71559 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 9 May 2019 18:51:56 +0200 Subject: [PATCH] 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 | 77 ++++++++++++++++++++++++++++ src/authentic2/forms/profile.py | 36 +++++++------ tests/test_profile.py | 90 ++++++++++++++++++++++++--------- 3 files changed, 162 insertions(+), 41 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..79abbf62 --- /dev/null +++ b/src/authentic2/forms/mixins.py @@ -0,0 +1,77 @@ +# 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 +from django.utils.translation import ugettext as _ + + +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: + # BooleanField case + if isinstance(initial, bool): + initial = _('Yes') if initial else _('No') + else: + # Most other fields case + try: + initial = field.widget.format_value(initial) + except AttributeError: + # Django 1.8 + try: + initial = field.widget._format_value(initial) + except AttributeError: + pass + 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, + required=False, + 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..9b8c3fbb 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -29,48 +29,68 @@ def test_account_edit_view(app, simple_user): url = reverse('profile_edit') resp = app.get(url, status=200) - attribute = Attribute.objects.create( + phone = Attribute.objects.create( name='phone', label='phone', - kind='string', user_visible=True, user_editable=True) - - 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) + kind='phone_number', user_visible=True, user_editable=True) + title = Attribute.objects.create( + name='title', label='title', + kind='title', user_visible=True, user_editable=True) + agreement = Attribute.objects.create( + name='agreement', label='agreement', + kind='boolean', user_visible=True, user_editable=True) + + resp = old_resp = app.get(url, status=200) + resp.form['edit-profile-phone'] = '1234' + resp.form['edit-profile-title'] = 'Mrs' + resp.form['edit-profile-agreement'] = False + resp = resp.form.submit() # verify that missing next_url in POST is ok assert resp['Location'].endswith(reverse('account_management')) - assert attribute.get_value(simple_user) == '1234' + assert phone.get_value(simple_user) == '1234' + assert title.get_value(simple_user) == 'Mrs' + assert agreement.get_value(simple_user) is False resp = app.get(url, status=200) resp.form.set('edit-profile-phone', '0123456789') resp = resp.form.submit().follow() - assert attribute.get_value(simple_user) == '0123456789' + assert phone.get_value(simple_user) == '0123456789' resp = app.get(url, status=200) resp.form.set('edit-profile-phone', '9876543210') resp = resp.form.submit('cancel').follow() - assert attribute.get_value(simple_user) == '0123456789' + assert phone.get_value(simple_user) == '0123456789' - attribute.set_value(simple_user, '0123456789', verified=True) + phone.set_value(simple_user, '0123456789', verified=True) + title.set_value(simple_user, 'Mr', verified=True) + agreement.set_value(simple_user, True, 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 'edit-profile-title' not in resp.form.fields + assert 'edit-profile-agreement' not in resp.form.fields + assert 'readonly' in resp.form['edit-profile-phone@disabled'].attrs + assert resp.form['edit-profile-phone@disabled'].value == '0123456789' + assert resp.form['edit-profile-title@disabled'].value == 'Mr' + assert resp.form['edit-profile-agreement@disabled'].value == 'Yes' + resp.form.set('edit-profile-phone@disabled', '1234') + resp.form.set('edit-profile-title@disabled', 'Mrs') + resp.form.set('edit-profile-agreement@disabled', 'False') resp = resp.form.submit().follow() - assert attribute.get_value(simple_user) == '0123456789' + assert phone.get_value(simple_user) == '0123456789' + assert title.get_value(simple_user) == 'Mr' + assert agreement.get_value(simple_user) is True - resp = app.get(url, status=200) - assert 'phone' in resp - assert 'readonly' in resp.form['edit-profile-phone'].attrs + resp = old_resp.form.submit() + assert phone.get_value(simple_user) == '0123456789' + assert title.get_value(simple_user) == 'Mr' + assert agreement.get_value(simple_user) is True - attribute.disabled = True - attribute.save() + phone.disabled = True + phone.save() resp = app.get(url, status=200) - assert 'phone' not in resp - assert attribute.get_value(simple_user) == '0123456789' + assert 'edit-profile-phone@disabled' not in resp + assert 'edit-profile-title@disabled' in resp + assert 'edit-profile-agreement@disabled' in resp + assert phone.get_value(simple_user) == '0123456789' def test_account_edit_next_url(app, simple_user, external_redirect_next_url, assert_external_redirect): @@ -135,3 +155,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