From 8f4da50c7255029411f4eed291e3a242562f6584 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Tue, 4 Oct 2022 11:30:32 +0200 Subject: [PATCH] manager: improve default role form fields (#58699) --- src/authentic2/forms/fields.py | 52 +++++++++++++++++++++ src/authentic2/manager/app_settings.py | 1 - src/authentic2/manager/forms.py | 62 ++++++++++++++++++++------ src/authentic2/manager/role_views.py | 4 +- tests/test_manager.py | 24 ++++++++++ 5 files changed, 126 insertions(+), 17 deletions(-) diff --git a/src/authentic2/forms/fields.py b/src/authentic2/forms/fields.py index 887a18308..d38f18314 100644 --- a/src/authentic2/forms/fields.py +++ b/src/authentic2/forms/fields.py @@ -18,6 +18,8 @@ import io import warnings import PIL.Image +from django import forms +from django.core import validators from django.core.files import File from django.forms import CharField, EmailField, FileField, ModelChoiceField, ValidationError from django.forms.fields import FILE_INPUT_CONTRADICTION @@ -126,3 +128,53 @@ class RoleChoiceField(ModelChoiceField): def label_from_instance(self, obj): return label_from_role(obj) + + +class ListValidator: + def __init__(self, item_validator): + self.item_validator = item_validator + + def __call__(self, value): + for i, item in enumerate(value): + try: + self.item_validator(item) + except ValidationError as e: + raise ValidationError(_('Item {0} is invalid: {1}').format(i, e.args[0])) + + +class CommaSeparatedInput(forms.TextInput): + def format_value(self, value): + if not value: + return '' + if not isinstance(value, str): + return ', '.join(value) + return value + + +class CommaSeparatedCharField(forms.Field): + widget = CommaSeparatedInput + + def __init__(self, dedup=True, max_length=None, min_length=None, *args, **kwargs): + self.dedup = dedup + self.max_length = max_length + self.min_length = min_length + item_validators = kwargs.pop('item_validators', []) + super().__init__(*args, **kwargs) + for item_validator in item_validators: + self.validators.append(ListValidator(item_validator)) + + def to_python(self, value): + if value in validators.EMPTY_VALUES: + return [] + + value = [item.strip() for item in value.split(',') if item.strip()] + if self.dedup: + value = list(set(value)) + + return value + + def clean(self, value): + value = self.to_python(value) + self.validate(value) + self.run_validators(value) + return value diff --git a/src/authentic2/manager/app_settings.py b/src/authentic2/manager/app_settings.py index 12497470e..9e2ad9844 100644 --- a/src/authentic2/manager/app_settings.py +++ b/src/authentic2/manager/app_settings.py @@ -21,7 +21,6 @@ class AppSettings: __PREFIX = 'A2_MANAGER_' __DEFAULTS = { 'HOMEPAGE_URL': None, - 'ROLE_FORM_CLASS': None, 'SHOW_ALL_OU': True, 'ROLE_MEMBERS_FROM_OU': False, 'SHOW_INTERNAL_ROLES': False, diff --git a/src/authentic2/manager/forms.py b/src/authentic2/manager/forms.py index dec3f54f3..1a01e1eb4 100644 --- a/src/authentic2/manager/forms.py +++ b/src/authentic2/manager/forms.py @@ -30,19 +30,20 @@ from django.utils.translation import pgettext, ugettext from django.utils.translation import ugettext_lazy as _ from django_select2.forms import HeavySelect2Widget -from authentic2.a2_rbac.models import OrganizationalUnit, Permission, Role +from authentic2.a2_rbac.models import OrganizationalUnit, Permission, Role, RoleAttribute from authentic2.a2_rbac.utils import generate_slug, get_default_ou -from authentic2.forms.fields import CheckPasswordField, NewPasswordField, ValidatedEmailField +from authentic2.forms.fields import ( + CheckPasswordField, + CommaSeparatedCharField, + NewPasswordField, + ValidatedEmailField, +) from authentic2.forms.mixins import SlugMixin from authentic2.forms.profile import BaseUserForm from authentic2.models import APIClient, PasswordReset, Service from authentic2.passwords import generate_password -from authentic2.utils.misc import ( - import_module_or_class, - send_email_change_email, - send_password_reset_mail, - send_templated_mail, -) +from authentic2.utils.misc import send_email_change_email, send_password_reset_mail, send_templated_mail +from authentic2.validators import EmailValidator from django_rbac.backends import DjangoRBACBackend from django_rbac.models import Operation @@ -601,6 +602,16 @@ class RoleEditForm(SlugMixin, HideOUFieldMixin, LimitQuerysetFormMixin, CssClass ou = forms.ModelChoiceField( queryset=OrganizationalUnit.objects, required=True, label=_('Organizational unit') ) + details = forms.CharField( + label=_('Role details (frontoffice)'), widget=forms.Textarea, initial='', required=False + ) + emails = CommaSeparatedCharField( + label=_('Emails'), + item_validators=[EmailValidator()], + required=False, + help_text=_('Emails must be separated by commas.'), + ) + emails_to_members = forms.BooleanField(required=False, initial=True, label=_('Emails to members')) class Meta: model = Role @@ -609,6 +620,35 @@ class RoleEditForm(SlugMixin, HideOUFieldMixin, LimitQuerysetFormMixin, CssClass 'name': forms.TextInput(), } + def __init__(self, *args, **kwargs): + instance = kwargs.get('instance') + if instance: + fields = [x.name for x in Role._meta.get_fields()] + initial = kwargs.setdefault('initial', {}) + role_attributes = RoleAttribute.objects.filter(role=instance, kind='json') + for role_attribute in role_attributes: + if role_attribute.name in fields: + continue + initial[role_attribute.name] = json.loads(role_attribute.value) + super().__init__(*args, **kwargs) + + def save(self, commit=True): + fields = [x.name for x in Role._meta.get_fields()] + assert commit + instance = super().save(commit=commit) + for field in self.cleaned_data: + if field in fields: + continue + value = json.dumps(self.cleaned_data[field]) + ra, created = RoleAttribute.objects.get_or_create( + role=instance, name=field, kind='json', defaults={'value': value} + ) + if not created and ra.value != value: + ra.value = value + ra.save() + instance.save() + return instance + class OUEditForm(SlugMixin, CssClass, forms.ModelForm): def __init__(self, *args, **kwargs): @@ -637,12 +677,6 @@ class OUEditForm(SlugMixin, CssClass, forms.ModelForm): ) -def get_role_form_class(): - if app_settings.ROLE_FORM_CLASS: - return import_module_or_class(app_settings.ROLE_FORM_CLASS) - return RoleEditForm - - # we need a model form so that we can use a BaseEditView, a simple Form # would not work class UserChangeEmailForm(CssClass, FormWithRequest, forms.ModelForm): diff --git a/src/authentic2/manager/role_views.py b/src/authentic2/manager/role_views.py index d93bc5c73..3ce469807 100644 --- a/src/authentic2/manager/role_views.py +++ b/src/authentic2/manager/role_views.py @@ -109,7 +109,7 @@ class RoleAddView(views.BaseAddView): return initial def get_form_class(self): - form = forms.get_role_form_class() + form = forms.RoleEditForm fields = [x for x in form.base_fields.keys() if x not in self.exclude_fields] return modelform_factory(self.model, form=form, fields=fields) @@ -158,7 +158,7 @@ class RoleEditView(RoleViewMixin, views.BaseEditView): title = _('Edit role description') def get_form_class(self): - return forms.get_role_form_class() + return forms.RoleEditForm def form_valid(self, form): response = super().form_valid(form) diff --git a/tests/test_manager.py b/tests/test_manager.py index 2664a6268..f908a4f05 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -145,6 +145,30 @@ def test_manager_create_role(superuser_or_admin, app): assert 'New OU' in options +def test_manager_edit_role(superuser_or_admin, app, simple_role): + resp = login(app, superuser_or_admin, '/manage/roles/%s/edit/' % simple_role.pk) + resp.form['details'] = 'xxx' + resp.form['emails'] = 'test@example.com' + resp.form['emails_to_members'] = False + resp = resp.form.submit().follow() + assert set(simple_role.attributes.values_list('name', 'value')) == { + ('emails_to_members', 'false'), + ('emails', '["test@example.com"]'), + ('details', '"xxx"'), + } + + resp = app.get('/manage/roles/%s/edit/' % simple_role.pk) + resp.form['emails'] = 'test@example.com, hop@example.com' + resp = resp.form.submit().follow() + emails = simple_role.attributes.get(name='emails') + assert set(json.loads(emails.value)) == {'test@example.com', 'hop@example.com'} + + resp = app.get('/manage/roles/%s/edit/' % simple_role.pk) + resp.form['emails'] = 'xxx' + resp = resp.form.submit() + assert 'Item 0 is invalid: Enter a valid email address.' in resp.text + + def test_manager_edit_role_slug(superuser_or_admin, app, simple_role): assert Role.objects.get(name='simple role').slug == 'simple-role' resp = login(app, superuser_or_admin, reverse('a2-manager-role-edit', kwargs={'pk': simple_role.pk})) -- 2.35.1