diff --git a/app/main/forms.py b/app/main/forms.py index 27d836190..5eaa400e0 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -591,10 +591,6 @@ class CreateServiceForm(StripWhitespaceForm): organisation_type = organisation_type() -class OrganisationTypeForm(StripWhitespaceForm): - organisation_type = organisation_type() - - class NewOrganisationForm( RenameOrganisationForm, OrganisationOrganisationTypeForm, diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 39fc331b4..7beab3951 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -36,7 +36,6 @@ from app.main.forms import ( FreeSMSAllowance, InternationalSMSForm, LinkOrganisationsForm, - OrganisationTypeForm, PreviewBranding, RenameServiceForm, SearchByNameForm, @@ -860,29 +859,6 @@ def service_set_letter_contact_block(service_id): ) -@main.route("/services//service-settings/set-organisation-type", methods=['GET', 'POST']) -@user_is_platform_admin -def set_organisation_type(service_id): - - form = OrganisationTypeForm(organisation_type=current_service.organisation_type) - - if form.validate_on_submit(): - free_sms_fragment_limit = current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get( - form.organisation_type.data) - - 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) - - return redirect(url_for('.service_settings', service_id=service_id)) - - return render_template( - 'views/service-settings/set-organisation-type.html', - form=form, - ) - - @main.route("/services//service-settings/set-free-sms-allowance", methods=['GET', 'POST']) @user_is_platform_admin def set_free_sms_allowance(service_id): diff --git a/app/navigation.py b/app/navigation.py index c893f8210..05627813a 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -286,7 +286,6 @@ class HeaderNavigation(Navigation): 'service_verify_reply_to_address_updates', 'services_or_dashboard', 'set_free_sms_allowance', - 'set_organisation_type', 'set_sender', 'set_template_sender', 'show_accounts_or_dashboard', @@ -411,7 +410,6 @@ class MainNavigation(Navigation): 'service_sms_senders', 'set_free_sms_allowance', 'service_set_letter_branding', - 'set_organisation_type', 'submit_request_to_go_live', }, 'api-integration': { @@ -815,7 +813,6 @@ class CaseworkNavigation(Navigation): 'services_or_dashboard', 'set_free_sms_allowance', 'service_set_letter_branding', - 'set_organisation_type', 'set_sender', 'set_template_sender', 'show_accounts_or_dashboard', @@ -1088,7 +1085,6 @@ class OrgNavigation(Navigation): 'services_or_dashboard', 'set_free_sms_allowance', 'service_set_letter_branding', - 'set_organisation_type', 'set_sender', 'set_template_sender', 'show_accounts_or_dashboard', diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 3f76d36da..1f8bf3f43 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -308,16 +308,27 @@ {% call row() %} {{ text_field('Organisation')}} - {{ optional_text_field(current_service.organisation.name) }} + {% call field() %} + {% if current_service.organisation_id %} + + {{ current_service.organisation.name }} + + {% else %} + Not set + {% endif %} + {% if current_service.organisation_type %} +
+ {{ { + 'central': 'Central government', + 'local': 'Local government', + 'nhs': 'NHS', + }.get(current_service.organisation_type) }} +
+ {% endif %} + {% endcall %} {{ edit_field('Change', url_for('.link_service_to_organisation', service_id=current_service.id)) }} {% endcall %} - {% call row() %} - {{ text_field('Organisation type')}} - {{ optional_text_field( - (current_service.organisation_type or '')|title - ) }} - {{ edit_field('Change', url_for('.set_organisation_type', service_id=current_service.id)) }} - {% endcall %} + {% call row() %} {{ text_field('Free text message allowance')}} {{ text_field('{:,}'.format(current_service.free_sms_fragment_limit)) }} diff --git a/app/templates/views/service-settings/set-organisation-type.html b/app/templates/views/service-settings/set-organisation-type.html deleted file mode 100644 index 13a693eb3..000000000 --- a/app/templates/views/service-settings/set-organisation-type.html +++ /dev/null @@ -1,23 +0,0 @@ -{% extends "withnav_template.html" %} -{% from "components/radios.html" import radios %} -{% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer %} -{% from "components/form.html" import form_wrapper %} - -{% block service_page_title %} - Set branding and organisation -{% endblock %} - -{% block maincolumn_content %} - - {{ page_header( - 'Set organisation type', - back_link=url_for('.service_settings', service_id=current_service.id) - ) }} - - {% call form_wrapper() %} - {{ radios(form.organisation_type) }} - {{ page_footer('Save') }} - {% endcall %} - -{% endblock %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 0ad64397a..f20ec3f93 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -60,7 +60,7 @@ def mock_get_service_settings_page_common( (active_user_with_permissions, [ 'Label Value Action', - 'Service name service one Change', + 'Service name Test Service Change', 'Sign-in method Text message code Change', 'Label Value Action', @@ -82,7 +82,7 @@ def mock_get_service_settings_page_common( (platform_admin_user, [ 'Label Value Action', - 'Service name service one Change', + 'Service name Test Service Change', 'Sign-in method Text message code Change', 'Label Value Action', @@ -103,8 +103,7 @@ def mock_get_service_settings_page_common( 'Label Value Action', 'Live Off Change', 'Count in list of live services Yes Change', - 'Organisation Test Organisation Change', - 'Organisation type Central Change', + 'Organisation Test Organisation Central government Change', 'Free text message allowance 250,000 Change', 'Email branding GOV.UK Change', 'Letter branding Not set Change', @@ -117,7 +116,7 @@ def mock_get_service_settings_page_common( def test_should_show_overview( client, mocker, - service_one, + api_user_active, fake_uuid, no_reply_to_email_addresses, no_letter_contact_blocks, @@ -127,12 +126,15 @@ def test_should_show_overview( expected_rows, mock_get_service_settings_page_common, ): - - service_one['permissions'] = ['sms', 'email'] + service_one = service_json(SERVICE_ONE_ID, + users=[api_user_active['id']], + permissions=['sms', 'email'], + organisation_id=ORGANISATION_ID) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) client.login(user(fake_uuid), mocker, service_one) response = client.get(url_for( - 'main.service_settings', service_id=service_one['id'] + 'main.service_settings', service_id=SERVICE_ONE_ID )) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -141,7 +143,7 @@ def test_should_show_overview( assert len(rows) == len(expected_rows) for index, row in enumerate(expected_rows): assert row == " ".join(rows[index].text.split()) - app.service_api_client.get_service.assert_called_with(service_one['id']) + app.service_api_client.get_service.assert_called_with(SERVICE_ONE_ID) def test_no_go_live_link_for_service_without_organisation( @@ -149,7 +151,6 @@ def test_no_go_live_link_for_service_without_organisation( mocker, no_reply_to_email_addresses, no_letter_contact_blocks, - mock_get_service_organisation, single_sms_sender, platform_admin_user, mock_get_service_settings_page_common, @@ -163,10 +164,35 @@ def test_no_go_live_link_for_service_without_organisation( 'Live No (organisation must be set first)' ) assert normalize_spaces(page.select('tr')[18].text) == ( - 'Organisation Not set Change' + 'Organisation Not set Central government Change' ) +def test_organisation_name_links_to_org_dashboard( + client_request, + platform_admin_user, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, + mock_get_service_settings_page_common, + mocker, + mock_get_service_organisation, +): + service_one = service_json(SERVICE_ONE_ID, + permissions=['sms', 'email'], + organisation_id=ORGANISATION_ID) + mocker.patch('app.service_api_client.get_service', return_value={'data': service_one}) + + client_request.login(platform_admin_user, service_one) + response = client_request.get( + 'main.service_settings', service_id=SERVICE_ONE_ID + ) + + org_row = response.select('tr')[18] + assert org_row.find('a')['href'] == url_for('main.organisation_dashboard', org_id=ORGANISATION_ID) + assert normalize_spaces(org_row.find('a').text) == 'Test Organisation' + + @pytest.mark.parametrize('permissions, expected_rows', [ (['email', 'sms', 'inbound_sms', 'international_sms'], [ @@ -3288,7 +3314,6 @@ def test_should_set_branding_and_organisations( @pytest.mark.parametrize('method', ['get', 'post']) @pytest.mark.parametrize('endpoint', [ - 'main.set_organisation_type', 'main.set_free_sms_allowance', ]) def test_organisation_type_pages_are_platform_admin_only( @@ -3304,64 +3329,6 @@ def test_organisation_type_pages_are_platform_admin_only( ) -def test_should_show_page_to_set_organisation_type( - logged_in_platform_admin_client, -): - response = logged_in_platform_admin_client.get(url_for( - 'main.set_organisation_type', - service_id=SERVICE_ONE_ID - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - labels = page.select('label') - checked_radio_buttons = page.select('input[checked]') - - assert len(checked_radio_buttons) == 1 - assert checked_radio_buttons[0]['value'] == 'central' - - assert len(labels) == 3 - for index, expected in enumerate(( - 'Central government', - 'Local government', - 'NHS', - )): - assert normalize_spaces(labels[index].text) == expected - - -@pytest.mark.parametrize('organisation_type, free_allowance', [ - ('central', 250000), - ('local', 25000), - ('nhs', 25000), - pytest.param('private sector', 1000, marks=pytest.mark.xfail) -]) -def test_should_set_organisation_type( - logged_in_platform_admin_client, - mock_update_service, - organisation_type, - free_allowance, - mock_create_or_update_free_sms_fragment_limit -): - response = logged_in_platform_admin_client.post( - url_for( - 'main.set_organisation_type', - service_id=SERVICE_ONE_ID, - ), - data={ - 'organisation_type': organisation_type, - 'organisation': 'organisation-id' - }, - ) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=SERVICE_ONE_ID, _external=True) - - mock_update_service.assert_called_once_with( - SERVICE_ONE_ID, - organisation_type=organisation_type, - ) - mock_create_or_update_free_sms_fragment_limit.assert_called_once_with(SERVICE_ONE_ID, free_allowance) - - def test_should_show_page_to_set_sms_allowance( logged_in_platform_admin_client, mock_get_free_sms_fragment_limit