Stop allowing the service org type to be changed

The service organisation type will either be the same as the org type of the service's organisation or will be set by a user when creating a new service. This removes the ability to change it from the platform admin settings table.
2026-07-23 09:29:14 -04:00 · 2019-07-08 09:44:22 +01:00
parent 3167c6dc31
commit 53214937a8
6 changed files with 56 additions and 133 deletions
--- 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,
--- 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_id>/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_id>/service-settings/set-free-sms-allowance", methods=['GET', 'POST'])
@user_is_platform_admin
 def set_free_sms_allowance(service_id):
--- 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',
--- 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 %}
+                <a href="{{ url_for('main.organisation_dashboard', org_id=current_service.organisation_id) }}">
+                  {{ current_service.organisation.name }}
+                </a>
+              {% else %}
+                <span class="table-field-status-default">Not set</span>
+              {% endif %}
+              {% if current_service.organisation_type %}
+                <div class="hint">
+                  {{ {
+                    'central': 'Central government',
+                    'local': 'Local government',
+                    'nhs': 'NHS',
+                  }.get(current_service.organisation_type) }}
+                </div>
+              {% 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)) }}
--- a/app/templates/views/service-settings/set-organisation-type.html
+++ b/app/templates/views/service-settings/set-organisation-type.html
@@ -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 %}
--- 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