From b0d3bd9046ff5e07d0e19671394aef6c4a0f12e5 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 12 Mar 2019 19:07:12 +0000 Subject: [PATCH] Update add_user_to_service endoint to only handle new data format Updated the add_user_to_service endpoint to only handle data in the 'new' format (`{"permissions": [...]}` instead of `[permission_1, permission_2]`) since Admin has been updated to send data the new way. This change means that we no longer need the Marshmallow Permission schema, so it can be deleted. --- app/schemas.py | 19 ------- app/service/rest.py | 15 +++--- tests/app/service/test_rest.py | 94 +++++----------------------------- 3 files changed, 22 insertions(+), 106 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index 8a4b1df20..b65c2f5b4 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -539,24 +539,6 @@ class InvitedUserSchema(BaseSchema): raise ValidationError(str(e)) -class PermissionSchema(BaseSchema): - - # Override generated fields - user = field_for(models.Permission, 'user', dump_only=True) - service = field_for(models.Permission, 'service', dump_only=True) - permission = field_for(models.Permission, 'permission') - - __envelope__ = { - 'single': 'permission', - 'many': 'permissions', - } - - class Meta: - model = models.Permission - exclude = ("created_at",) - strict = True - - class EmailDataSchema(ma.Schema): class Meta: @@ -692,7 +674,6 @@ notification_schema = NotificationModelSchema() notification_with_template_schema = NotificationWithTemplateSchema() notification_with_personalisation_schema = NotificationWithPersonalisationSchema() invited_user_schema = InvitedUserSchema() -permission_schema = PermissionSchema() email_data_request_schema = EmailDataSchema() partial_email_data_request_schema = EmailDataSchema(partial_email=True) notifications_filter_schema = NotificationsFilterSchema() diff --git a/app/service/rest.py b/app/service/rest.py index a98c0bc4a..52651e2f8 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -83,7 +83,7 @@ from app.errors import ( register_errors ) from app.letters.utils import letter_print_day -from app.models import LETTER_TYPE, NOTIFICATION_CANCELLED, Service, EmailBranding, LetterBranding +from app.models import LETTER_TYPE, NOTIFICATION_CANCELLED, Permission, Service, EmailBranding, LetterBranding from app.schema_validation import validate from app.service import statistics from app.service.service_data_retention_schema import ( @@ -101,11 +101,11 @@ from app.service.send_notification import send_one_off_notification from app.schemas import ( service_schema, api_key_schema, - permission_schema, notification_with_template_schema, notifications_filter_schema, detailed_service_schema ) +from app.user.users_schema import post_set_permissions_schema from app.utils import pagination_links service_blueprint = Blueprint('service', __name__) @@ -286,12 +286,13 @@ def add_user_to_service(service_id, user_id): raise InvalidRequest(error, status_code=400) data = request.get_json() - if 'permissions' in data: - user_permissions = data['permissions'] - else: - user_permissions = data + validate(data, post_set_permissions_schema) + + permissions = [ + Permission(service_id=service_id, user_id=user_id, permission=p['permission']) + for p in data['permissions'] + ] - permissions = permission_schema.load(user_permissions, many=True).data dao_add_user_to_service(service, user, permissions) data = service_schema.dump(service).data return jsonify(data=data), 201 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 83757fb94..89b4d40d4 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1080,81 +1080,7 @@ def test_default_permissions_are_added_for_user_service(notify_api, assert sorted(default_service_permissions) == sorted(service_permissions) -def test_add_existing_user_to_another_service_with_all_permissions(notify_api, - notify_db, - notify_db_session, - sample_service, - sample_user): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - # check which users part of service - user_already_in_service = sample_service.users[0] - auth_header = create_authorization_header() - - resp = client.get( - '/service/{}/users'.format(sample_service.id), - headers=[('Content-Type', 'application/json'), auth_header] - ) - - assert resp.status_code == 200 - result = resp.json - assert len(result['data']) == 1 - assert result['data'][0]['email_address'] == user_already_in_service.email_address - - # add new user to service - user_to_add = User( - name='Invited User', - email_address='invited@digital.cabinet-office.gov.uk', - password='password', - mobile_number='+4477123456' - ) - # they must exist in db first - save_model_user(user_to_add) - - data = [{"permission": "send_emails"}, - {"permission": "send_letters"}, - {"permission": "send_texts"}, - {"permission": "manage_users"}, - {"permission": "manage_settings"}, - {"permission": "manage_api_keys"}, - {"permission": "manage_templates"}, - {"permission": "view_activity"}] - - auth_header = create_authorization_header() - - resp = client.post( - '/service/{}/users/{}'.format(sample_service.id, user_to_add.id), - headers=[('Content-Type', 'application/json'), auth_header], - data=json.dumps(data) - ) - - assert resp.status_code == 201 - - # check new user added to service - auth_header = create_authorization_header() - - resp = client.get( - '/service/{}'.format(sample_service.id), - headers=[('Content-Type', 'application/json'), auth_header], - ) - assert resp.status_code == 200 - json_resp = resp.json - assert str(user_to_add.id) in json_resp['data']['users'] - - # check user has all permissions - auth_header = create_authorization_header() - resp = client.get(url_for('user.get_user', user_id=user_to_add.id), - headers=[('Content-Type', 'application/json'), auth_header]) - - assert resp.status_code == 200 - json_resp = resp.json - permissions = json_resp['data']['permissions'][str(sample_service.id)] - expected_permissions = ['send_texts', 'send_emails', 'send_letters', 'manage_users', - 'manage_settings', 'manage_templates', 'manage_api_keys', 'view_activity'] - assert sorted(expected_permissions) == sorted(permissions) - - -def test_add_existing_user_to_another_service_with_all_permissions_with_new_data_format( +def test_add_existing_user_to_another_service_with_all_permissions( notify_api, notify_db, notify_db_session, @@ -1250,9 +1176,13 @@ def test_add_existing_user_to_another_service_with_send_permissions(notify_api, ) save_model_user(user_to_add) - data = [{"permission": "send_emails"}, + data = { + "permissions": [ + {"permission": "send_emails"}, {"permission": "send_letters"}, - {"permission": "send_texts"}] + {"permission": "send_texts"}, + ] + } auth_header = create_authorization_header() @@ -1293,9 +1223,13 @@ def test_add_existing_user_to_another_service_with_manage_permissions(notify_api ) save_model_user(user_to_add) - data = [{"permission": "manage_users"}, + data = { + "permissions": [ + {"permission": "manage_users"}, {"permission": "manage_settings"}, - {"permission": "manage_templates"}] + {"permission": "manage_templates"}, + ] + } auth_header = create_authorization_header() @@ -1336,7 +1270,7 @@ def test_add_existing_user_to_another_service_with_manage_api_keys(notify_api, ) save_model_user(user_to_add) - data = [{"permission": "manage_api_keys"}] + data = {"permissions": [{"permission": "manage_api_keys"}]} auth_header = create_authorization_header()