Remove dependence on domains.yml from settings

Settings looked at `domains.yml` when users were making go live requests
or email branding requests.

This will allow us to remove the `domains.yml` file, by using
information about organisations that is now stored in the database
instead.

app/main/views/service_settings.py

@@ -54,7 +54,6 @@ from app.main.forms import (
     branding_options_dict,
 )
 from app.utils import (
-    AgreementInfo,
     email_safe,
     user_has_permissions,
     user_is_gov_user,
@@ -180,7 +179,7 @@ def estimate_usage(service_id):
 @user_has_permissions('manage_service')
 def request_to_go_live(service_id):
 
-    agreement_signed = AgreementInfo.from_current_user().agreement_signed
+    agreement_signed = current_service.organisation.agreement_signed
 
     return render_template(
         'views/service-settings/request-to-go-live.html',
@@ -226,7 +225,7 @@ def submit_request_to_go_live(service_id):
             service_name=current_service.name,
             service_dashboard=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
             organisation_type=str(current_service.organisation_type).title(),
-            agreement=AgreementInfo.from_current_user().as_human_readable,
+            agreement=current_service.organisation.as_human_readable(current_user.email_domain),
             checklist=current_service.go_live_checklist_completed_as_yes_no,
             volume_email=print_if_number(current_service.volume_email),
             volume_email_formatted=format_if_number(current_service.volume_email),
@@ -237,7 +236,7 @@ def submit_request_to_go_live(service_id):
             research_consent='Yes' if current_service.consent_to_research else 'No',
             existing_live='Yes' if user_api_client.user_has_live_services(current_user) else 'No',
             service_id=current_service.id,
-            organisation=AgreementInfo.from_current_user().owner,
+            organisation=current_service.organisation.name,
             user_name=current_user.name,
             user_email=current_user.email_address,
             date=datetime.now(tz=pytz.timezone('Europe/London')).strftime('%d/%m/%Y'),
@@ -245,7 +244,7 @@ def submit_request_to_go_live(service_id):
         ticket_type=zendesk_client.TYPE_QUESTION,
         user_email=current_user.email_address,
         user_name=current_user.name,
-        tags=get_request_to_go_live_tags(current_service, current_user),
+        tags=get_request_to_go_live_tags(current_service),
     )
 
     flash('Thanks for your request to go live. We’ll get back to you within one working day.', 'default')
@@ -985,7 +984,7 @@ def branding_request(service_id):
                 '\nCurrent branding: {current_branding}'
                 '\nBranding requested: {branding_requested}'
             ).format(
-                organisation=AgreementInfo.from_current_user().as_info_for_branding_request,
+                organisation=current_service.organisation.as_info_for_branding_request(current_user.email_domain),
                 service_name=current_service.name,
                 dashboard_url=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
                 current_branding=current_service.email_branding_name,
@@ -1073,20 +1072,17 @@ def check_contact_details_type(contact_details):
         return 'phone_number'
 
 
-def get_request_to_go_live_tags(service, user):
-    return list(_get_request_to_go_live_tags(
-        service,
-        AgreementInfo.from_user(user).agreement_signed,
-    ))
+def get_request_to_go_live_tags(service):
+    return list(_get_request_to_go_live_tags(service))
 
 
-def _get_request_to_go_live_tags(service, agreement_signed):
+def _get_request_to_go_live_tags(service):
 
     BASE = 'notify_request_to_go_live'
 
     yield BASE
 
-    if service.go_live_checklist_completed and agreement_signed:
+    if service.go_live_checklist_completed and service.organisation.agreement_signed:
         yield BASE + '_complete'
         return
 
@@ -1094,7 +1090,7 @@ def _get_request_to_go_live_tags(service, agreement_signed):
         (True, ''),
         (not service.volumes, '_volumes'),
         (not service.go_live_checklist_completed, '_checklist'),
-        (not agreement_signed, '_mou'),
+        (not service.organisation.agreement_signed, '_mou'),
         (service.needs_to_add_email_reply_to_address, '_email_reply_to'),
         (not service.has_team_members, '_team_member'),
         (not service.has_templates, '_template_content'),

app/models/organisation.py

@@ -31,13 +31,12 @@ class Organisation(JSONModel):
     def crown_status(self):
         return self.crown
 
-    @property
-    def as_human_readable(self):
+    def as_human_readable(self, fallback_domain):
         if 'dwp.' in ''.join(self.domains):
             return 'DWP - Requires OED approval'
         if self.agreement_signed:
             return 'Yes, on behalf of {}'.format(self.name)
-        elif self.owner:
+        elif self.name:
             return '{} (organisation is {}, {})'.format(
                 {
                     False: 'No',
@@ -51,11 +50,10 @@ class Organisation(JSONModel):
                 }.get(self.crown_status),
             )
         else:
-            return 'Can’t tell (domain is {})'.format(self._domain)
+            return 'Can’t tell (domain is {})'.format(fallback_domain)
 
-    @property
-    def as_info_for_branding_request(self):
-        return self.owner or 'Can’t tell (domain is {})'.format(self._domain)
+    def as_info_for_branding_request(self, fallback_domain):
+        return self.name or 'Can’t tell (domain is {})'.format(fallback_domain)
 
     @property
     def as_jinja_template(self):

app/models/service.py

@@ -396,10 +396,6 @@ class Service(JSONModel):
             organisations_client.get_service_organisation(self.id)
         )
 
-    @property
-    def organisation_name(self):
-        return self.organisation.name
-
     @cached_property
     def inbound_number(self):
         return inbound_number_client.get_inbound_sms_number_for_service(self.id)['data'].get('number', '')

app/templates/views/service-settings.html

@@ -300,7 +300,7 @@
 
         {% call row() %}
           {{ text_field('Organisation')}}
-          {{ optional_text_field(current_service.organisation_name) }}
+          {{ optional_text_field(current_service.organisation.name) }}
           {{ edit_field('Change', url_for('.link_service_to_organisation', service_id=current_service.id)) }}
         {% endcall %}
         {% call row() %}

tests/app/main/views/test_service_settings.py

@@ -1,4 +1,3 @@
-from collections import namedtuple
 from functools import partial
 from unittest.mock import ANY, PropertyMock, call
 from urllib.parse import parse_qs, urlparse
@@ -31,6 +30,7 @@ from tests.conftest import (
     get_non_default_letter_contact_block,
     get_non_default_reply_to_email_address,
     get_non_default_sms_sender,
+    mock_get_service_organisation,
     multiple_letter_contact_blocks,
     multiple_reply_to_email_addresses,
     multiple_sms_senders,
@@ -101,7 +101,7 @@ def mock_get_service_settings_page_common(
         'Label Value Action',
         'Live Off Change',
         'Count in list of live services Yes Change',
-        'Organisation Org 1 Change',
+        'Organisation Test Organisation Change',
         'Organisation type Central Change',
         'Free text message allowance 250,000 Change',
         'Email branding GOV.UK Change',
@@ -616,6 +616,7 @@ def test_should_check_if_estimated_volumes_provided(
     single_reply_to_email_address,
     mock_get_service_templates,
     mock_get_users_by_service,
+    mock_get_service_organisation,
     volumes,
     consent_to_research,
     expected_estimated_volumes_item,
@@ -675,6 +676,7 @@ def test_should_check_if_estimated_volumes_provided(
 def test_should_check_for_sending_things_right(
     client_request,
     mocker,
+    mock_get_service_organisation,
     single_sms_sender,
     count_of_users_with_manage_service,
     expected_user_checklist_item,
@@ -685,7 +687,6 @@ def test_should_check_for_sending_things_right(
     reply_to_email_addresses,
     expected_reply_to_checklist_item,
 ):
-
     def _templates_by_type(template_type):
         return {
             'email': list(range(0, count_of_email_templates)),
@@ -751,25 +752,22 @@ def test_should_not_show_go_live_button_if_checklist_not_complete(
     mocker,
     mock_get_service_templates,
     mock_get_users_by_service,
+    mock_get_service_organisation,
     single_sms_sender,
     checklist_completed,
     agreement_signed,
     expected_button,
 ):
-
-    def _agreement_info():
-        return namedtuple(
-            'AgreementInfo', ['agreement_signed']
-        )(agreement_signed=agreement_signed)
-
     mocker.patch(
         'app.models.service.Service.go_live_checklist_completed',
         new_callable=PropertyMock,
         return_value=checklist_completed,
     )
     mocker.patch(
-        'app.utils.AgreementInfo.from_current_user',
-        side_effect=_agreement_info,
+        'app.models.organisation.Organisation.agreement_signed',
+        new_callable=PropertyMock,
+        return_value=agreement_signed,
+        create=True,
     )
 
     for channel in ('email', 'sms', 'letter'):
@@ -894,13 +892,13 @@ def test_should_check_for_sms_sender_on_go_live(
     client_request,
     service_one,
     mocker,
+    mock_get_service_organisation,
     organisation_type,
     count_of_sms_templates,
     sms_senders,
     expected_sms_sender_checklist_item,
     estimated_sms_volume,
 ):
-
     service_one['organisation_type'] = organisation_type
 
     def _templates_by_type(template_type):
@@ -953,18 +951,18 @@ def test_should_check_for_sms_sender_on_go_live(
     mock_get_sms_senders.assert_called_once_with(SERVICE_ONE_ID)
 
 
-@pytest.mark.parametrize('email_address, expected_item', (
+@pytest.mark.parametrize('agreement_signed, expected_item', (
     pytest.param(
-        'test@unknown.gov.uk',
+        None,
         '',
         marks=pytest.mark.xfail(raises=IndexError)
     ),
     (
-        'test@education.gov.uk',
+        True,
         'Sign our data sharing and financial agreement Completed',
     ),
     (
-        'test@aylesbury.gov.uk',
+        False,
         'Sign our data sharing and financial agreement Not completed',
     ),
 ))
@@ -972,7 +970,7 @@ def test_should_check_for_mou_on_request_to_go_live(
     client_request,
     service_one,
     mocker,
-    email_address,
+    agreement_signed,
     expected_item,
 ):
     mocker.patch(
@@ -1000,9 +998,10 @@ def test_should_check_for_mou_on_request_to_go_live(
             return_value=None,
         )
 
-    user = active_user_with_permissions(uuid4())
-    user.email_address = email_address
-    client_request.login(user)
+    mock_get_service_organisation(
+        mocker,
+        agreement_signed=agreement_signed,
+    )
 
     page = client_request.get(
         'main.request_to_go_live', service_id=SERVICE_ONE_ID
@@ -1017,6 +1016,7 @@ def test_non_gov_user_is_told_they_cant_go_live(
     client_request,
     api_nongov_user_active,
     mocker,
+    mock_get_service_organisation,
 ):
     mocker.patch(
         'app.main.views.service_settings.user_api_client.get_count_of_users_with_permission',
@@ -1287,7 +1287,6 @@ def test_should_redirect_after_request_to_go_live(
     active_user_with_permissions,
     single_reply_to_email_address,
     single_letter_contact_block,
-    mock_get_service_organisation,
     mock_get_organisations_and_services_for_user,
     single_sms_sender,
     mock_get_service_settings_page_common,
@@ -1298,6 +1297,11 @@ def test_should_redirect_after_request_to_go_live(
     formatted_displayed_volumes,
     extra_tags,
 ):
+    mock_get_service_organisation(
+        mocker,
+        name=None,
+        agreement_signed=None,
+    )
     for channel, volume in volumes:
         mocker.patch(
             'app.models.service.Service.volume_{}'.format(channel),
@@ -1516,6 +1520,11 @@ def test_ready_to_go_live(
     agreement_signed,
     expected_tags,
 ):
+    mock_get_service_organisation(
+        mocker,
+        agreement_signed=agreement_signed,
+    )
+
     for prop in {
         'has_team_members',
         'has_templates',
@@ -1547,8 +1556,7 @@ def test_ready_to_go_live(
     }).go_live_checklist_completed_as_yes_no == expected_readyness
 
     assert list(app.main.views.service_settings._get_request_to_go_live_tags(
-        app.models.service.Service({'id': SERVICE_ONE_ID}),
-        agreement_signed,
+        app.models.service.Service({'id': SERVICE_ONE_ID})
     )) == expected_tags
 
 
@@ -3966,6 +3974,10 @@ def test_show_email_branding_request_page_when_email_branding_is_set(
     ('org_banner', 'Your logo on a colour'),
     pytest.param('foo', 'Nope', marks=pytest.mark.xfail(raises=AssertionError)),
 ))
+@pytest.mark.parametrize('org_name, expected_organisation', (
+    (None, 'Can’t tell (domain is user.gov.uk)'),
+    ('Test Organisation', 'Test Organisation'),
+))
 def test_submit_email_branding_request(
     client_request,
     mocker,
@@ -3974,10 +3986,16 @@ def test_submit_email_branding_request(
     mock_get_service_settings_page_common,
     no_reply_to_email_addresses,
     no_letter_contact_blocks,
-    mock_get_service_organisation,
     single_sms_sender,
+    org_name,
+    expected_organisation,
 ):
 
+    mock_get_service_organisation(
+        mocker,
+        name=org_name,
+    )
+
     zendesk = mocker.patch(
         'app.main.views.service_settings.zendesk_client.create_ticket',
         autospec=True,
@@ -3993,14 +4011,14 @@ def test_submit_email_branding_request(
 
     zendesk.assert_called_once_with(
         message='\n'.join([
-            'Organisation: Can’t tell (domain is user.gov.uk)',
+            'Organisation: {}',
             'Service: service one',
             'http://localhost/services/596364a0-858e-42c8-9062-a8fe822260eb',
             '',
             '---',
             'Current branding: GOV.UK',
             'Branding requested: {}',
-        ]).format(requested_branding),
+        ]).format(expected_organisation, requested_branding),
         subject='Email branding request - service one',
         ticket_type='question',
         user_email='test@user.gov.uk',

tests/conftest.py

@@ -3145,9 +3145,19 @@ def mock_get_organisation_by_domain(
 
 
 @pytest.fixture(scope='function')
-def mock_get_service_organisation(mocker):
+def mock_get_service_organisation(
+    mocker,
+    name=False,
+    crown=True,
+    agreement_signed=None,
+):
     def _get_service_organisation(service_id):
-        return organisation_json('7aa5d4e9-4385-4488-a489-07812ba13383', 'Org 1')
+        return organisation_json(
+            '7aa5d4e9-4385-4488-a489-07812ba13383',
+            name,
+            crown=crown,
+            agreement_signed=agreement_signed,
+        )
 
     return mocker.patch('app.organisations_client.get_service_organisation', side_effect=_get_service_organisation)