diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 5fbf6cb95..f51194af5 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -564,13 +564,16 @@ def construct_change_message(self, request, form, formsets, add=False): formsets[0] = self._config_formset return super().construct_change_message(request, form, formsets, add) + @admin.action( + description=_('Change group of selected Devices'), permissions=['change'] + ) def change_group(self, request, queryset): # Validate all selected devices belong to the same organization # which is managed by the user. org_id = None if queryset: org_id = queryset[0].organization_id - if not request.user.is_superuser and request.user.is_manager(org_id): + if not request.user.is_superuser and not request.user.is_manager(org_id): logger.warning(f'{request.user} does not manage "{org_id}" organization.') return HttpResponseForbidden() if len(queryset) != queryset.filter(organization_id=org_id).count(): @@ -621,8 +624,6 @@ def change_group(self, request, queryset): request, 'admin/config/change_device_group.html', context ) - change_group.short_description = _('Change group of selected Devices') - def get_fields(self, request, obj=None): """ Do not show readonly fields in add form @@ -820,6 +821,7 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl readonly_fields = ['system_context'] autocomplete_fields = ['vpn'] + @admin.action(permissions=['add']) def clone_selected_templates(self, request, queryset): selectable_orgs = None user = request.user diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index bbc43c252..d1361f9ec 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -13,7 +13,7 @@ from django.urls import reverse from swapper import load_model -from openwisp_utils.tests import catch_signal +from openwisp_utils.tests import AdminActionPermTestMixin, catch_signal from ...geo.tests.utils import TestGeoMixin from ...tests.utils import TestAdminMixin @@ -223,6 +223,7 @@ def test_device_import_config_not_templates(self): class TestAdmin( + AdminActionPermTestMixin, TestImportExportMixin, TestGeoMixin, CreateDeviceGroupMixin, @@ -600,6 +601,22 @@ def test_change_device_group_action(self): response, 'What group do you want to assign to the selected devices?' ) + def test_change_device_group_action_perms(self): + org = self._get_org() + user = self._create_user(is_staff=True) + self._create_org_user(is_admin=True, organization=org, user=user) + device = self._create_device(organization=org) + group = self._create_device_group(name='default', organization=org) + self._test_action_permission( + path=reverse(f'admin:{self.app_label}_device_changelist'), + action='change_group', + user=user, + obj=device, + message='Successfully changed group of selected devices.', + required_perms=['change'], + extra_payload={'device_group': group.pk, 'apply': True}, + ) + def test_device_import_with_group_apply_templates(self): org = self._get_org(org_name='default') template = self._create_template(name='template') @@ -991,6 +1008,20 @@ def test_clone_templates_org_errors(self): with self.subTest('template count should not change'): self.assertEqual(Template.objects.count(), count) + def test_clone_selected_templates_action_perms(self): + org = self._get_org() + user = self._create_user(is_staff=True) + self._create_org_user(is_admin=True, organization=org, user=user) + template = self._create_template(organization=org) + self._test_action_permission( + path=reverse(f'admin:{self.app_label}_template_changelist'), + action='clone_selected_templates', + user=user, + obj=template, + message='Successfully cloned selected templates.', + required_perms=['add'], + ) + def test_change_device_clean_templates(self): o = self._get_org() t = Template.objects.first()