From 468a43bd68837168100dbce4da48d921e6cedc78 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 31 Jul 2020 15:16:56 +0100 Subject: [PATCH] Revert support for old user permissions params The fields used for user permissions on permissions forms were changed as part of the work converting the checkboxes to GOVUK Frontend. This removes code added to protect against a situation where the server-side app was running this updated code but clients were POSTing from pages that were not, and so sending the old HTTP params. --- app/main/forms.py | 40 +----------- tests/app/main/views/test_manage_users.py | 77 ++++------------------- 2 files changed, 16 insertions(+), 101 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 4301861fb..c0ffc6e68 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -755,20 +755,6 @@ def filter_by_broadcast_permissions(valuelist): return [entry for entry in valuelist if any(entry in role for role in broadcast_permissions)] -# Included to support both versions of how user permissions are handled in permissions forms -# Remove when changeover to new version (permissions_field) is complete -PermissionsAbstract = type("PermissionsAbstract", (StripWhitespaceForm,), { - permission: BooleanField(label) for permission, label in permissions -}) - - -# Included to support both versions of how user permissions are handled in permissions forms -# Remove when changeover to new version (permissions_field) is complete -BroadcastPermissionsAbstract = type("BroadcastPermissionsAbstract", (StripWhitespaceForm,), { - permission: BooleanField(label) for permission, label in broadcast_permissions -}) - - class BasePermissionsForm(StripWhitespaceForm): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) @@ -804,25 +790,9 @@ class BasePermissionsForm(StripWhitespaceForm): } ) - # Modified to support both versions of how user permissions are handled in permissions forms - # Remove when changeover to new version (permissions_field) is complete @property def permissions(self): - permissions_field_data = set(self.permissions_field.data) - permissions_fields_data = {field.id for field in self.permissions_fields if field.data is True} - if len(permissions_field_data) == 0 and len(permissions_fields_data) != 0: - return permissions_fields_data - else: - return permissions_field_data - - # Included to support both versions of how user permissions are handled in permissions forms - # Remove when changeover to new version (permissions_field) is complete - @property - def permissions_fields(self): - return ( - getattr(self, permission) for permission, field in self.__dict__.items() - if isinstance(field, BooleanField) - ) + return set(self.permissions_field.data) @classmethod def from_user(cls, user, service_id, **kwargs): @@ -836,15 +806,11 @@ class BasePermissionsForm(StripWhitespaceForm): ) -# Included to support both versions of how user permissions are handled in permissions forms -# Remove when changeover to new version (permissions_field) is complete -class PermissionsForm(PermissionsAbstract, BasePermissionsForm): +class PermissionsForm(BasePermissionsForm): pass -# Included to support both versions of how user permissions are handled in permissions forms -# Remove when changeover to new version (permissions_field) is complete -class BroadcastPermissionsForm(BroadcastPermissionsAbstract, BasePermissionsForm): +class BroadcastPermissionsForm(BasePermissionsForm): permissions_field = govukCheckboxesField( 'Permssions', diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 82790baac..ea8433e19 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -514,22 +514,6 @@ def test_should_not_show_page_for_non_team_member( {}, set(), ), - ( # should be able to handle permissions being sent as booleans, until changeover to a list is complete - { - 'view_activity': 'y', - 'send_messages': 'y', - 'manage_templates': 'y', - 'manage_service': 'y', - 'manage_api_keys': 'y', - }, - { - 'view_activity', - 'send_messages', - 'manage_templates', - 'manage_service', - 'manage_api_keys', - } - ), ]) def test_edit_user_permissions( client_request, @@ -600,20 +584,6 @@ def test_edit_user_permissions( 'view_activity', } ), - ( # should be able to handle permissions being sent as booleans, until changeover to a list is complete - { - 'send_messages': 'y', - 'manage_templates': 'y', - 'manage_service': 'y', - 'manage_api_keys': 'y', - }, - { - 'view_activity', - 'send_messages', - 'manage_service', - 'manage_templates', - } - ), ]) def test_edit_user_permissions_for_broadcast_service( client_request, @@ -858,10 +828,9 @@ def test_should_show_folder_permission_form_if_service_has_folder_permissions_en assert len(folder_checkboxes) == 3 -@pytest.mark.parametrize('email_address, gov_user, old_permissions', [ - ('test@example.gov.uk', True, False), - ('test@example.com', False, False), - ('test@example.gov.uk', True, True) +@pytest.mark.parametrize('email_address, gov_user', [ + ('test@example.gov.uk', True), + ('test@example.com', False) ]) def test_invite_user( client_request, @@ -870,7 +839,6 @@ def test_invite_user( sample_invite, email_address, gov_user, - old_permissions, mock_get_template_folders, mock_get_organisations, ): @@ -880,25 +848,19 @@ def test_invite_user( mocker.patch('app.models.user.InvitedUsers.client_method', return_value=[sample_invite]) mocker.patch('app.models.user.Users.client_method', return_value=[active_user_with_permissions]) mocker.patch('app.invite_api_client.create_invite', return_value=sample_invite) - data = {'email_address': email_address} - if old_permissions: - data['view_activity'] = 'y' - data['send_messages'] = 'y' - data['manage_templates'] = 'y' - data['manage_service'] = 'y' - data['manage_api_keys'] = 'y' - else: - data['permissions_field'] = [ - 'view_activity', - 'send_messages', - 'manage_templates', - 'manage_service', - 'manage_api_keys', - ] page = client_request.post( 'main.invite_user', service_id=SERVICE_ONE_ID, - _data=data, + _data={ + 'email_address': email_address, + 'permissions_field': [ + 'view_activity', + 'send_messages', + 'manage_templates', + 'manage_service', + 'manage_api_keys', + ] + }, _follow_redirects=True, ) assert page.h1.string.strip() == 'Team members' @@ -1003,19 +965,6 @@ def test_invite_user_with_email_auth_service( 'view_activity', }, ), - ( # should be able to handle permissions being sent as booleans, until changeover to a list is complete - { - 'send_messages': 'y', - 'manage_templates': 'y', - 'manage_service': 'y', - }, - { - 'view_activity', - 'send_messages', - 'manage_templates', - 'manage_service', - }, - ) )) def test_invite_user_to_broadcast_service( client_request,