Refactor to add separate letter branding client

We were getting all letter logos from a method in the email branding
client. Since we will be adding more client methods to deal with
letters, it makes things clearer to separate the email and letter
branding clients.

app/__init__.py

@@ -61,6 +61,7 @@ from app.notify_client.user_api_client import user_api_client
 from app.notify_client.events_api_client import events_api_client
 from app.notify_client.provider_client import provider_client
 from app.notify_client.email_branding_client import email_branding_client
+from app.notify_client.letter_branding_client import letter_branding_client
 from app.notify_client.organisations_api_client import organisations_client
 from app.notify_client.org_invite_api_client import org_invite_api_client
 from app.notify_client.letter_jobs_client import letter_jobs_client
@@ -125,6 +126,7 @@ def create_app(application):
     events_api_client.init_app(application)
     provider_client.init_app(application)
     email_branding_client.init_app(application)
+    letter_branding_client.init_app(application)
     organisations_client.init_app(application)
     letter_jobs_client.init_app(application)
     inbound_number_client.init_app(application)

app/main/views/service_settings.py

@@ -19,6 +19,7 @@ from app import (
     current_service,
     email_branding_client,
     inbound_number_client,
+    letter_branding_client,
     organisations_client,
     service_api_client,
     user_api_client,
@@ -854,7 +855,7 @@ def service_preview_email_branding(service_id):
 def set_letter_branding(service_id):
 
     form = LetterBranding(
-        choices=email_branding_client.get_letter_email_branding().items(),
+        choices=letter_branding_client.get_letter_branding().items(),
         dvla_org_id=current_service.dvla_organisation,
     )
 

app/models/service.py

@@ -8,6 +8,7 @@ from app.notify_client.email_branding_client import email_branding_client
 from app.notify_client.inbound_number_client import inbound_number_client
 from app.notify_client.invite_api_client import invite_api_client
 from app.notify_client.job_api_client import job_api_client
+from app.notify_client.letter_branding_client import letter_branding_client
 from app.notify_client.organisations_api_client import organisations_client
 from app.notify_client.service_api_client import service_api_client
 from app.notify_client.template_folder_api_client import (
@@ -304,7 +305,7 @@ class Service():
 
     @cached_property
     def letter_branding(self):
-        return email_branding_client.get_letter_email_branding().get(
+        return letter_branding_client.get_letter_branding().get(
             self.dvla_organisation, '001'
         )
 

app/notify_client/email_branding_client.py

@@ -23,9 +23,6 @@ class EmailBrandingClient(NotifyAdminAPIClient):
                 return branding['id']
         return None
 
-    def get_letter_email_branding(self):
-        return self.get(url='/dvla_organisations')
-
     @cache.delete('email_branding')
     def create_email_branding(self, logo, name, text, colour, domain, brand_type):
         data = {

app/notify_client/letter_branding_client.py

@@ -0,0 +1,13 @@
+from app.notify_client import NotifyAdminAPIClient
+
+
+class LetterBrandingClient(NotifyAdminAPIClient):
+
+    def __init__(self):
+        super().__init__("a" * 73, "b")
+
+    def get_letter_branding(self):
+        return self.get(url='/dvla_organisations')
+
+
+letter_branding_client = LetterBrandingClient()

tests/app/main/views/service_settings/test_service_setting_permissions.py

@@ -10,7 +10,7 @@ def get_service_settings_page(
     platform_admin_user,
     service_one,
     mock_get_inbound_number_for_service,
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
     mock_get_service_organisation,
     mock_get_free_sms_fragment_limit,
     no_reply_to_email_addresses,
@@ -113,7 +113,7 @@ def test_normal_user_doesnt_see_any_toggle_buttons(
     no_letter_contact_blocks,
     mock_get_service_organisation,
     single_sms_sender,
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
     mock_get_inbound_number_for_service,
     mock_get_free_sms_fragment_limit,
     mock_get_service_data_retention

tests/app/main/views/test_service_settings.py

@@ -38,7 +38,7 @@ from tests.conftest import (
 
 @pytest.fixture
 def mock_get_service_settings_page_common(
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
     mock_get_inbound_number_for_service,
     mock_get_free_sms_fragment_limit,
     mock_get_service_data_retention,
@@ -1216,7 +1216,7 @@ def test_route_for_platform_admin_update_service(
         client,
         platform_admin_user,
         service_one,
-        mock_get_letter_email_branding,
+        mock_get_letter_branding,
         route,
 ):
     mocker.patch('app.service_api_client.archive_service')
@@ -2142,7 +2142,7 @@ def test_set_letter_contact_block_has_max_10_lines(
 
 def test_request_letter_branding(
     client_request,
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
 ):
     request_page = client_request.get(
         'main.request_letter_branding',
@@ -2186,7 +2186,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_email_branding,
+    mock_get_letter_branding,
     current_dvla_org_id,
     expected_selected,
     expected_items,
@@ -2213,7 +2213,7 @@ def test_set_letter_branding_saves(
     logged_in_platform_admin_client,
     service_one,
     mock_update_service,
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
 ):
     response = logged_in_platform_admin_client.post(
         url_for('main.set_letter_branding', service_id=service_one['id']),
@@ -3098,7 +3098,7 @@ def test_service_settings_when_inbound_number_is_not_set(
     mock_get_service_organisation,
     single_sms_sender,
     mocker,
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
     mock_get_free_sms_fragment_limit,
     mock_get_service_data_retention,
 ):
@@ -3116,7 +3116,7 @@ def test_set_inbound_sms_when_inbound_number_is_not_set(
     single_reply_to_email_address,
     single_letter_contact_block,
     mocker,
-    mock_get_letter_email_branding,
+    mock_get_letter_branding,
 ):
     mocker.patch('app.inbound_number_client.get_inbound_sms_number_for_service',
                  return_value={'data': {}})

tests/app/notify_client/test_email_branding_client.py

@@ -51,14 +51,6 @@ def test_get_all_email_branding(mocker):
     )
 
 
-def test_get_letter_email_branding(mocker):
-    mock_get = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.get')
-    EmailBrandingClient().get_letter_email_branding()
-    mock_get.assert_called_once_with(
-        url='/dvla_organisations'
-    )
-
-
 def test_create_email_branding(mocker):
     org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red',
                 'domain': 'sample.com', 'brand_type': 'org'}

tests/app/notify_client/test_letter_branding_client.py

@@ -0,0 +1,9 @@
+from app.notify_client.letter_branding_client import LetterBrandingClient
+
+
+def test_get_letter_branding(mocker):
+    mock_get = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.get')
+    LetterBrandingClient().get_letter_branding()
+    mock_get.assert_called_once_with(
+        url='/dvla_organisations'
+    )

tests/conftest.py

@@ -2547,8 +2547,8 @@ def mock_get_email_branding_that_can_fit_onscreen(mocker):
 
 
 @pytest.fixture(scope='function')
-def mock_get_letter_email_branding(mocker):
-    def _get_letter_email_branding():
+def mock_get_letter_branding(mocker):
+    def _get_letter_branding():
         return {
             '001': 'HM Government',
             '500': 'Land Registry',
@@ -2556,7 +2556,7 @@ def mock_get_letter_email_branding(mocker):
         }
 
     return mocker.patch(
-        'app.email_branding_client.get_letter_email_branding', side_effect=_get_letter_email_branding
+        'app.letter_branding_client.get_letter_branding', side_effect=_get_letter_branding
     )
 
 
@@ -2566,7 +2566,7 @@ def mock_no_email_branding(mocker):
         return []
 
     return mocker.patch(
-        'app.email_branding_client.get_letter_email_branding', side_effect=_get_email_branding
+        'app.email_branding_client.get_all_email_branding', side_effect=_get_email_branding
     )