user: Accommodate Django 3.1 change for model choice iteration

- Before Django 3.1, iterating the .choices for a field would yield (id, label)
tuples directly suitable for use with ChoiceFields. From Django 3.1, id is an
instance of ModelChoiceIteratorValue which helps to easily find the model
instance. In most cases, using the proxy works, but in our case, the value is
being hashed. Access the actual value of the field from the object to avoid this
issue.

- Cleanup widget for disabling individual checkboxes in a group

- When a form is submitted, 'disabled' input field is omitted by the browser
irrespective of its value. So, the last admin user, automatically add the
'admin' group to form values.

Tests:

- On Django 2.2 and Django 3.2 access the user edit page. The form should render
as before the change without errors.

- Test with the current user as the last admin user. The 'admin' checkbox should
be read-only.

- Test with the current user not as the last admin user. The 'admin' checkbox
should not be read-only.

- Add/remove non-admin groups and save the current/different user.

- Access the user edit page as non-admin user, the groups should be disabled.

- Give/take admin permission to/from a user other than current user.

- Take admin permission from current user.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/forms.py

@@ -4,13 +4,10 @@ Common forms for use by modules.
 """
 
 import os
-from itertools import chain
 
 from django import forms
 from django.conf import settings
-from django.forms import CheckboxInput
 from django.utils import translation
-from django.utils.safestring import mark_safe
 from django.utils.translation import get_language_info
 from django.utils.translation import gettext_lazy as _
 
@@ -81,47 +78,25 @@ class LanguageSelectionForm(LanguageSelectionFormMixin, forms.Form):
     language = LanguageSelectionFormMixin.language
 
 
-def _get_value_in_parens(string):
-    return string[string.find("(") + 1:string.find(")")]
-
-
 class CheckboxSelectMultipleWithReadOnly(forms.widgets.CheckboxSelectMultiple):
-    """
-    Subclass of Django's CheckboxSelectMultiple widget that allows setting
-    individual fields as readonly
+    """Multiple checkbox widget that allows setting individual fields readonly.
+
     To mark a feature as readonly an option, pass a dict instead of a string
-    for its label, of the form: {'label': 'option label', 'disabled': True}
+    for its label, of the form: {'label': 'option label', 'disabled': True}.
 
-    Derived from https://djangosnippets.org/snippets/2786/
     """
 
-    def render(self, name, value, attrs=None, choices=(), renderer=None):
-        if value is None:
-            value = []
-        final_attrs = self.build_attrs(attrs)
-        output = [u'<ul>']
-        global_readonly = 'readonly' in final_attrs
-        str_values = set([v for v in value])
-        for i, (option_value,
-                option_label) in enumerate(chain(self.choices, choices)):
-            if not global_readonly and 'readonly' in final_attrs:
-                # If the entire group is readonly keep all options readonly
-                del final_attrs['readonly']
-            if isinstance(option_label, dict):
-                if dict.get(option_label, 'readonly'):
-                    final_attrs = dict(final_attrs, readonly='readonly')
-                option_label = option_label['label']
-            group_name = _get_value_in_parens(option_label)
-            final_attrs = dict(final_attrs,
-                               id='{}_{}'.format(attrs['id'], group_name))
-            label_for = u' for="{}"'.format(final_attrs['id'])
-            cb = CheckboxInput(final_attrs,
-                               check_test=lambda value: value in str_values)
-            rendered_cb = cb.render(name, option_value)
-            output.append(u'<li><label%s>%s %s</label></li>' %
-                          (label_for, rendered_cb, option_label))
-        output.append(u'</ul>')
-        return mark_safe(u'\n'.join(output))
+    def create_option(self, name, value, label, selected, index, subindex=None,
+                      attrs=None):
+        option = super().create_option(name, value, label, selected, index,
+                                       subindex, attrs)
+        if isinstance(option['label'], dict):
+            if option['label'].get('disabled'):
+                option['attrs']['disabled'] = 'disabled'
+
+            option['label'] = option['label']['label']
+
+        return option
 
 
 class CheckboxSelectMultiple(forms.widgets.CheckboxSelectMultiple):

plinth/modules/users/forms.py

@@ -210,19 +210,21 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm,
         })
 
         choices = []
+        django_groups = sorted(self.fields['groups'].choices,
+                               key=lambda choice: choice[1])
+        for group_id, group_name in django_groups:
+            try:
+                group_id = group_id.value
+            except AttributeError:
+                pass
 
-        for c in sorted(self.fields['groups'].choices, key=lambda x: x[1]):
-            # Handle case where groups exist in database for
-            # applications not installed yet.
-            if c[1] in group_choices:
-                # Replace group names with descriptions
-                if c[1] == 'admin' and self.is_last_admin_user:
-                    choices.append((c[0], {
-                        'label': group_choices[c[1]],
-                        'readonly': True
-                    }))
-                else:
-                    choices.append((c[0], group_choices[c[1]]))
+            # Show choices only from groups declared by apps.
+            if group_name in group_choices:
+                label = group_choices[group_name]
+                if group_name == 'admin' and self.is_last_admin_user:
+                    label = {'label': label, 'disabled': True}
+
+                choices.append((group_id, label))
 
         self.fields['groups'].label = _('Permissions')
         self.fields['groups'].choices = choices
@@ -323,18 +325,20 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm,
 
         return user
 
-    def validate_last_admin_user(self, groups):
-        group_names = [group.name for group in groups]
-        if 'admin' not in group_names:
-            raise ValidationError(
-                _('Cannot delete the only administrator in the system.'))
+    def clean_groups(self):
+        """Validate groups to ensure admin group for last admin.
 
-    def clean(self):
-        """Override clean to add form validation logic."""
-        cleaned_data = super().clean()
+        For the last admin user, we disable the checkbox for 'admin' group so
+        that it can't be unchecked. However, this means that browser will no
+        longer submit that value. Forcefully add 'admin' group in this case.
+
+        """
+        groups = self.cleaned_data['groups']
         if self.is_last_admin_user:
-            self.validate_last_admin_user(cleaned_data.get("groups"))
-        return cleaned_data
+            groups = groups | self.fields['groups'].queryset.filter(
+                **{'name': 'admin'})
+
+        return groups
 
 
 class UserChangePasswordForm(PasswordConfirmForm, SetPasswordForm):