diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 25e2f29c3..09909769b 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -108,8 +108,7 @@ def service_name_change_confirm(service_id): if form.validate_on_submit(): try: - service_api_client.update_service( - current_service.id, + current_service.update( name=session['service_name_change'], email_from=email_safe(session['service_name_change']) ) @@ -209,8 +208,7 @@ def submit_request_to_go_live(service_id): @login_required @user_is_platform_admin def service_switch_live(service_id): - service_api_client.update_service( - current_service.id, + current_service.update( # TODO This limit should be set depending on the agreement signed by # with Notify. message_limit=250000 if current_service.trial_mode else 50, @@ -223,49 +221,15 @@ def service_switch_live(service_id): @login_required @user_is_platform_admin def service_switch_research_mode(service_id): - service_api_client.update_service_with_properties( - service_id, - {"research_mode": not current_service.research_mode} - ) + current_service.toggle_research_mode() return redirect(url_for('.service_settings', service_id=service_id)) -def switch_service_permissions(service_id, permission, sms_sender=None): - - force_service_permission( - service_id, - permission, - on=permission not in current_service.permissions, - sms_sender=sms_sender - ) - - -def force_service_permission(service_id, permission, on=False, sms_sender=None): - - permissions, permission = set(current_service.permissions), {permission} - - update_service_permissions( - service_id, - permissions | permission if on else permissions - permission, - sms_sender=sms_sender - ) - - -def update_service_permissions(service_id, permissions, sms_sender=None): - - data = {'permissions': list(permissions)} - - if sms_sender: - data['sms_sender'] = sms_sender - - service_api_client.update_service_with_properties(service_id, data) - - @main.route("/services//service-settings/can-send-email") @login_required @user_is_platform_admin def service_switch_can_send_email(service_id): - switch_service_permissions(service_id, 'email') + current_service.switch_permission('email') return redirect(url_for('.service_settings', service_id=service_id)) @@ -273,7 +237,7 @@ def service_switch_can_send_email(service_id): @login_required @user_is_platform_admin def service_switch_can_send_sms(service_id): - switch_service_permissions(service_id, 'sms') + current_service.switch_permission('sms') return redirect(url_for('.service_settings', service_id=service_id)) @@ -281,7 +245,7 @@ def service_switch_can_send_sms(service_id): @login_required @user_is_platform_admin def service_switch_email_auth(service_id): - switch_service_permissions(service_id, 'email_auth') + current_service.switch_permission('email_auth') return redirect(url_for('.service_settings', service_id=service_id)) @@ -289,7 +253,7 @@ def service_switch_email_auth(service_id): @login_required @user_is_platform_admin def service_switch_can_send_precompiled_letter(service_id): - switch_service_permissions(service_id, 'precompiled_letter') + current_service.switch_permission('precompiled_letter') return redirect(url_for('.service_settings', service_id=service_id)) @@ -302,17 +266,16 @@ def service_switch_can_upload_document(service_id): # If turning the permission off, or turning it on and the service already has a contact_link, # don't show the form to add the link if current_service.has_permission('upload_document') or current_service.contact_link: - switch_service_permissions(service_id, 'upload_document') + current_service.switch_permission('upload_document') return redirect(url_for('.service_settings', service_id=service_id)) if form.validate_on_submit(): contact_type = form.contact_details_type.data - service_api_client.update_service( - current_service.id, + current_service.update( contact_link=form.data[contact_type] ) - switch_service_permissions(service_id, 'upload_document') + current_service.switch_permission('upload_document') return redirect(url_for('.service_settings', service_id=service_id)) return render_template('views/service-settings/contact_link.html', form=form) @@ -322,7 +285,7 @@ def service_switch_can_upload_document(service_id): @login_required @user_is_platform_admin def service_switch_can_edit_folders(service_id): - switch_service_permissions(service_id, 'edit_folders') + current_service.switch_permission('edit_folders') return redirect(url_for('.service_settings', service_id=service_id)) @@ -380,8 +343,7 @@ def service_set_contact_link(service_id): if form.validate_on_submit(): contact_type = form.contact_details_type.data - service_api_client.update_service( - current_service.id, + current_service.update( contact_link=form.data[contact_type] ) return redirect(url_for('.service_settings', service_id=current_service.id)) @@ -495,7 +457,7 @@ def service_set_inbound_number(service_id): is_default=True, inbound_number_id=form.inbound_number.data ) - switch_service_permissions(current_service.id, 'inbound_sms') + current_service.force_permission('inbound_sms', on=True) return redirect(url_for('.service_settings', service_id=service_id)) return render_template( 'views/service-settings/set-inbound-number.html', @@ -525,8 +487,7 @@ def service_set_sms_prefix(service_id): form.enabled.label.text = 'Start all text messages with ‘{}:’'.format(current_service.name) if form.validate_on_submit(): - service_api_client.update_service( - current_service.id, + current_service.update( prefix_sms=(form.enabled.data == 'on') ) return redirect(url_for('.service_settings', service_id=service_id)) @@ -545,8 +506,7 @@ def service_set_international_sms(service_id): enabled='on' if current_service.has_permission('international_sms') else 'off' ) if form.validate_on_submit(): - force_service_permission( - service_id, + current_service.force_permission( 'international_sms', on=(form.enabled.data == 'on'), ) @@ -576,8 +536,7 @@ def service_set_letters(service_id): enabled='on' if current_service.has_permission('letter') else 'off' ) if form.validate_on_submit(): - force_service_permission( - service_id, + current_service.force_permission( 'letter', on=(form.enabled.data == 'on'), ) @@ -597,7 +556,7 @@ def service_set_postage(service_id): form = ServicePostageForm(postage=current_service.postage) if form.validate_on_submit(): - service_api_client.update_service(service_id, postage=form.postage.data) + current_service.update(postage=form.postage.data) return redirect(url_for(".service_settings", service_id=service_id)) return render_template('views/service-settings/set-postage.html', form=form) @@ -762,8 +721,7 @@ def service_set_letter_contact_block(service_id): form = ServiceLetterContactBlockForm(letter_contact_block=current_service.letter_contact_block) if form.validate_on_submit(): - service_api_client.update_service( - current_service.id, + current_service.update( letter_contact_block=form.letter_contact_block.data.replace('\r', '') or None ) if request.args.get('from_template'): @@ -788,8 +746,7 @@ def set_organisation_type(service_id): free_sms_fragment_limit = current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get( form.organisation_type.data) - service_api_client.update_service( - service_id, + current_service.update( organisation_type=form.organisation_type.data, ) billing_api_client.create_or_update_free_sms_fragment_limit(service_id, free_sms_fragment_limit) @@ -857,8 +814,7 @@ def service_preview_email_branding(service_id): if form.validate_on_submit(): branding_style = None if form.branding_style.data == 'None' else form.branding_style.data - service_api_client.update_service( - service_id, + current_service.update( email_branding=branding_style ) return redirect(url_for('.service_settings', service_id=service_id)) @@ -879,8 +835,7 @@ def set_letter_branding(service_id): form = LetterBranding(choices=email_branding_client.get_letter_email_branding().items()) if form.validate_on_submit(): - service_api_client.update_service( - service_id, + current_service.update( dvla_organisation=form.dvla_org_id.data ) return redirect(url_for('.service_settings', service_id=service_id)) diff --git a/app/models/service.py b/app/models/service.py index c663962d3..6e3bb9794 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -47,6 +47,29 @@ class Service(): return self._dict[attr] raise AttributeError('`{}` is not a service attribute'.format(attr)) + def update(self, **kwargs): + return service_api_client.update_service(self.id, **kwargs) + + def switch_permission(self, permission): + return self.force_permission( + permission, + on=not self.has_permission(permission), + ) + + def force_permission(self, permission, on=False): + + permissions, permission = set(self.permissions), {permission} + + return self.update_permissions( + permissions | permission if on else permissions - permission, + ) + + def update_permissions(self, permissions): + return self.update(permissions=list(permissions)) + + def toggle_research_mode(self): + self.update(research_mode=not self.research_mode) + @property def trial_mode(self): return self._dict['restricted'] diff --git a/tests/app/main/views/service_settings/test_inbound_sms_setting.py b/tests/app/main/views/service_settings/test_inbound_sms_setting.py index dfbb7abfd..8114c4faf 100644 --- a/tests/app/main/views/service_settings/test_inbound_sms_setting.py +++ b/tests/app/main/views/service_settings/test_inbound_sms_setting.py @@ -12,7 +12,7 @@ def test_set_inbound_sms_sets_a_number_for_service( mock_no_inbound_number_for_service, mocker ): - mocker.patch('app.service_api_client.update_service_with_properties') + mocker.patch('app.service_api_client.update_service') data = { "inbound_number": "781d9c60-7a7e-46b7-9896-7b045b992fa5", } diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 27c038082..cb45e6ef0 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1170,7 +1170,10 @@ def test_enabling_and_disabling_email_and_sms( mock_get_inbound_number_for_service ): service_one['permissions'] = permissions_before_switch - mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + mocked_fn = mocker.patch( + 'app.notify_client.service_api_client.service_api_client.update_service', + return_value=service_one, + ) response = logged_in_platform_admin_client.get( url_for('main.service_switch_can_send_{}'.format(notification_type), service_id=service_one['id']) @@ -1178,7 +1181,7 @@ def test_enabling_and_disabling_email_and_sms( assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - assert mocked_fn.call_args == call(service_one['id'], {'permissions': permissions_after_switch}) + assert mocked_fn.call_args == call(service_one['id'], permissions=permissions_after_switch) def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender( @@ -1930,7 +1933,7 @@ def test_switch_service_from_research_mode_to_normal( research_mode=True ) mocker.patch('app.service_api_client.get_service', return_value={"data": service}) - update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service) + update_service_mock = mocker.patch('app.service_api_client.update_service', return_value=service) response = logged_in_platform_admin_client.get( url_for('main.service_switch_research_mode', service_id=service['id']) @@ -1938,7 +1941,7 @@ def test_switch_service_from_research_mode_to_normal( assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service['id'], _external=True) update_service_mock.assert_called_with( - service['id'], {"research_mode": False} + service['id'], research_mode=False ) @@ -1953,7 +1956,7 @@ def test_shows_research_mode_indicator( mock_get_service_settings_page_common, ): service_one['research_mode'] = True - mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + mocker.patch('app.service_api_client.update_service', return_value=service_one) response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id'])) assert response.status_code == 200 @@ -2441,7 +2444,7 @@ def test_switch_service_enable_letters( initial_permissions, expected_updated_permissions, ): - mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + mocked_fn = mocker.patch('app.service_api_client.update_service', return_value=service_one) service_one['permissions'] = initial_permissions page = client_request.get( @@ -2462,8 +2465,7 @@ def test_switch_service_enable_letters( _external=True ) ) - - assert set(mocked_fn.call_args[0][1]['permissions']) == set(expected_updated_permissions) + assert set(mocked_fn.call_args[1]['permissions']) == set(expected_updated_permissions) assert mocked_fn.call_args[0][0] == service_one['id'] @@ -2502,7 +2504,7 @@ def test_switch_service_enable_international_sms( post_value, international_sms_permission_expected_in_api_call, ): - mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + mocked_fn = mocker.patch('app.service_api_client.update_service', return_value=service_one) client_request.post( 'main.service_set_international_sms', service_id=service_one['id'], @@ -2513,9 +2515,9 @@ def test_switch_service_enable_international_sms( ) if international_sms_permission_expected_in_api_call: - assert 'international_sms' in mocked_fn.call_args[0][1]['permissions'] + assert 'international_sms' in mocked_fn.call_args[1]['permissions'] else: - assert 'international_sms' not in mocked_fn.call_args[0][1]['permissions'] + assert 'international_sms' not in mocked_fn.call_args[1]['permissions'] assert mocked_fn.call_args[0][0] == service_one['id']