Get organisations list from API rather than config

Hard coding the organisations means this information is duplicated
between the admin and the API, and could get out of sync.

app/config.py

@@ -83,11 +83,6 @@ class Config(object):
         r"assembly\.wales",
     ]
 
-    LETTER_BRANDING = [
-        ('001', 'HM Government'),
-        ('500', 'Land Registry'),
-    ]
-
 
 class Development(Config):
     DEBUG = True

app/main/views/service_settings.py

@@ -36,6 +36,7 @@ from app import user_api_client, current_service, organisations_client
 @login_required
 @user_has_permissions('manage_settings', admin_override=True)
 def service_settings(service_id):
+    letter_branding_organisations = organisations_client.get_letter_organisations()
     if current_service['organisation']:
         organisation = organisations_client.get_organisation(current_service['organisation'])['organisation']
     else:
@@ -43,9 +44,9 @@ def service_settings(service_id):
     return render_template(
         'views/service-settings.html',
         organisation=organisation,
-        letter_branding=dict(
-            current_app.config['LETTER_BRANDING']
-        ).get(current_service.get('dvla_org_id', '001'))
+        letter_branding=letter_branding_organisations.get(
+            current_service.get('dvla_org_id', '001')
+        )
     )
 
 
@@ -323,7 +324,7 @@ def service_set_branding_and_org(service_id):
 @user_has_permissions(admin_override=True)
 def set_letter_branding(service_id):
 
-    form = LetterBranding(choices=current_app.config['LETTER_BRANDING'])
+    form = LetterBranding(choices=organisations_client.get_letter_organisations().items())
 
     if form.validate_on_submit():
         service_api_client.update_service(

app/notify_client/organisations_client.py

@@ -16,3 +16,6 @@ class OrganisationsClient(NotifyAdminAPIClient):
 
     def get_organisations(self):
         return self.get(url='/organisation')['organisations']
+
+    def get_letter_organisations(self):
+        return self.get(url='/dvla_organisations')

tests/app/main/views/test_service_settings.py

@@ -35,6 +35,7 @@ def test_should_show_overview(
     mocker,
     service_one,
     fake_uuid,
+    mock_get_letter_organisations,
     user,
     expected_rows,
 ):
@@ -58,6 +59,7 @@ def test_should_show_overview_for_service_with_more_things_set(
     mocker,
     service_with_reply_to_addresses,
     mock_get_organisation,
+    mock_get_letter_organisations,
 ):
     client.login(active_user_with_permissions, mocker, service_with_reply_to_addresses)
     response = client.get(url_for(
@@ -75,6 +77,7 @@ def test_should_show_overview_for_service_with_more_things_set(
 def test_if_cant_send_letters_then_cant_see_letter_contact_block(
     logged_in_client,
     service_one,
+    mock_get_letter_organisations,
 ):
     response = logged_in_client.get(url_for(
         'main.service_settings', service_id=service_one['id']
@@ -85,7 +88,8 @@ def test_if_cant_send_letters_then_cant_see_letter_contact_block(
 def test_letter_contact_block_shows_None_if_not_set(
     logged_in_client,
     service_one,
-    mocker
+    mocker,
+    mock_get_letter_organisations,
 ):
     service_one['can_send_letters'] = True
     response = logged_in_client.get(url_for(
@@ -102,6 +106,7 @@ def test_escapes_letter_contact_block(
     logged_in_client,
     service_one,
     mocker,
+    mock_get_letter_organisations,
 ):
     service_one['can_send_letters'] = True
     service_one['letter_contact_block'] = 'foo\nbar<script>alert(1);</script>'
@@ -148,6 +153,7 @@ def test_should_redirect_after_change_service_name(
 def test_show_restricted_service(
     logged_in_client,
     service_one,
+    mock_get_letter_organisations,
 ):
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -176,7 +182,8 @@ def test_switch_service_to_live(
 def test_show_live_service(
     logged_in_client,
     service_one,
-    mock_get_live_service
+    mock_get_live_service,
+    mock_get_letter_organisations,
 ):
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -296,6 +303,7 @@ def test_should_redirect_after_request_to_go_live(
     active_user_with_permissions,
     service_one,
     mocker,
+    mock_get_letter_organisations,
 ):
     mock_post = mocker.patch(
         'app.main.views.feedback.requests.post',
@@ -387,6 +395,7 @@ def test_route_permissions(
     client,
     api_user_active,
     service_one,
+    mock_get_letter_organisations,
     route,
 ):
     validate_route_permission(
@@ -441,6 +450,7 @@ def test_route_for_platform_admin(
     client,
     platform_admin_user,
     service_one,
+    mock_get_letter_organisations,
     route,
 ):
     validate_route_permission(mocker,
@@ -464,6 +474,7 @@ def test_route_for_platform_admin_update_service(
     client,
     platform_admin_user,
     service_one,
+    mock_get_letter_organisations,
     route,
 ):
     mocker.patch('app.service_api_client.archive_service')
@@ -481,6 +492,7 @@ def test_set_reply_to_email_address(
     logged_in_client,
     mock_update_service,
     service_one,
+    mock_get_letter_organisations,
 ):
     data = {"email_address": "test@someservice.gov.uk"}
     response = logged_in_client.post(url_for('main.service_set_reply_to_email', service_id=service_one['id']),
@@ -550,6 +562,7 @@ def test_shows_research_mode_indicator(
     logged_in_client,
     service_one,
     mocker,
+    mock_get_letter_organisations,
 ):
     service_one['research_mode'] = True
     mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one)
@@ -565,6 +578,7 @@ def test_shows_research_mode_indicator(
 def test_does_not_show_research_mode_indicator(
     logged_in_client,
     service_one,
+    mock_get_letter_organisations,
 ):
     response = logged_in_client.get(url_for('main.service_settings', service_id=service_one['id']))
     assert response.status_code == 200
@@ -578,6 +592,7 @@ def test_set_text_message_sender(
     logged_in_client,
     mock_update_service,
     service_one,
+    mock_get_letter_organisations,
 ):
     data = {"sms_sender": "elevenchars"}
     response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id']),
@@ -697,6 +712,7 @@ def test_set_letter_branding_platform_admin_only(
 def test_set_letter_branding_prepopulates(
     logged_in_platform_admin_client,
     service_one,
+    mock_get_letter_organisations,
     current_dvla_org_id,
     expected_selected,
 ):
@@ -712,6 +728,7 @@ def test_set_letter_contact_block_saves(
     logged_in_platform_admin_client,
     service_one,
     mock_update_service,
+    mock_get_letter_organisations,
 ):
     response = logged_in_platform_admin_client.post(
         url_for('main.set_letter_branding', service_id=service_one['id']),
@@ -725,7 +742,8 @@ def test_set_letter_contact_block_saves(
 def test_should_show_branding(
     logged_in_platform_admin_client,
     service_one,
-    mock_get_organisations
+    mock_get_organisations,
+    mock_get_letter_organisations,
 ):
     response = logged_in_platform_admin_client.get(url_for(
         'main.service_set_branding_and_org', service_id=service_one['id']
@@ -845,6 +863,7 @@ def test_archive_service_prompts_user(
     logged_in_platform_admin_client,
     service_one,
     mocker,
+    mock_get_letter_organisations,
 ):
     mocked_fn = mocker.patch('app.service_api_client.post')
 
@@ -859,6 +878,7 @@ def test_archive_service_prompts_user(
 def test_cant_archive_inactive_service(
     logged_in_platform_admin_client,
     service_one,
+    mock_get_letter_organisations,
 ):
     service_one['active'] = False
 
@@ -887,6 +907,7 @@ def test_suspend_service_prompts_user(
     logged_in_platform_admin_client,
     service_one,
     mocker,
+    mock_get_letter_organisations,
 ):
     mocked_fn = mocker.patch('app.service_api_client.post')
 
@@ -902,6 +923,7 @@ def test_suspend_service_prompts_user(
 def test_cant_suspend_inactive_service(
     logged_in_platform_admin_client,
     service_one,
+    mock_get_letter_organisations,
 ):
     service_one['active'] = False
 
@@ -931,6 +953,7 @@ def test_resume_service_prompts_user(
     logged_in_platform_admin_client,
     service_one,
     mocker,
+    mock_get_letter_organisations,
 ):
     service_one['active'] = False
     mocked_fn = mocker.patch('app.service_api_client.post')
@@ -947,6 +970,7 @@ def test_resume_service_prompts_user(
 def test_cant_resume_active_service(
     logged_in_platform_admin_client,
     service_one,
+    mock_get_letter_organisations,
 ):
     response = logged_in_platform_admin_client.get(url_for('main.service_settings', service_id=service_one['id']))
 

tests/conftest.py

@@ -1357,6 +1357,19 @@ def mock_get_organisations(mocker):
     )
 
 
+@pytest.fixture(scope='function')
+def mock_get_letter_organisations(mocker):
+    def _get_organisations():
+        return {
+            '001': 'HM Government',
+            '500': 'Land Registry',
+        }
+
+    return mocker.patch(
+        'app.organisations_client.get_letter_organisations', side_effect=_get_organisations
+    )
+
+
 @pytest.fixture(scope='function')
 def mock_get_organisation(mocker):
     def _get_organisation(id):