Merge pull request #2896 from alphagov/remove-domains-yml

Use organisations from database rather than YAML file

app/main/forms.py

@@ -36,10 +36,8 @@ from wtforms.widgets import CheckboxInput, ListWidget
 
 from app.main.validators import (
     Blacklist,
-    CanonicalGovernmentDomain,
     CsvFileValidator,
     DoesNotStartWithDoubleZero,
-    KnownGovernmentDomain,
     LettersNumbersAndFullStopsOnly,
     NoCommasInPlaceHolders,
     OnlyGSMCharacters,
@@ -47,7 +45,7 @@ from app.main.validators import (
     ValidGovEmail,
 )
 from app.models.user import permissions, roles
-from app.utils import AgreementInfo, guess_name_from_email_address
+from app.utils import guess_name_from_email_address
 
 
 def get_time_value_and_label(future_time):
@@ -990,23 +988,9 @@ class PreviewBranding(StripWhitespaceForm):
     branding_style = HiddenFieldWithNoneOption('branding_style')
 
 
-class GovernmentDomainField(StringField):
-    validators = [
-        KnownGovernmentDomain(),
-        CanonicalGovernmentDomain(),
-    ]
-
-    def post_validate(self, form, validation_stopped):
-        if self.data == '':
-            self.data = None
-        if self.data and not self.errors:
-            self.data = AgreementInfo(self.data).canonical_domain
-
-
 class ServiceUpdateEmailBranding(StripWhitespaceForm):
     name = StringField('Name of brand')
     text = StringField('Text')
-    domain = GovernmentDomainField('Domain')
     colour = StringField(
         'Colour',
         validators=[
@@ -1041,7 +1025,6 @@ class SVGFileUpload(StripWhitespaceForm):
 
 class ServiceLetterBrandingDetails(StripWhitespaceForm):
     name = StringField('Name of brand', validators=[DataRequired()])
-    domain = GovernmentDomainField('Domain')
 
 
 class PDFUploadForm(StripWhitespaceForm):

app/main/validators.py

@@ -11,7 +11,7 @@ from wtforms.validators import Email
 
 from app import formatted_list
 from app.main._blacklisted_passwords import blacklisted_passwords
-from app.utils import AgreementInfo, Spreadsheet, is_gov_user
+from app.utils import Spreadsheet, is_gov_user
 
 
 class Blacklist:
@@ -111,34 +111,3 @@ class DoesNotStartWithDoubleZero:
     def __call__(self, form, field):
         if field.data and field.data.startswith("00"):
             raise ValidationError(self.message)
-
-
-class KnownGovernmentDomain:
-
-    message = 'Not a known government domain (you might need to update domains.yml)'
-
-    def __call__(self, form, field):
-        if field.data and AgreementInfo(field.data).owner is None:
-            raise ValidationError(self.message)
-
-
-class CanonicalGovernmentDomain:
-
-    message = 'Not {} domain (use {} if appropriate)'
-
-    def __call__(self, form, field):
-
-        if not field.data:
-            return
-
-        domain = AgreementInfo(field.data)
-
-        if not domain.is_canonical:
-            raise ValidationError(
-                self.message.format('a canonical', domain.canonical_domain)
-            )
-
-        if field.data != domain.canonical_domain:
-            raise ValidationError(
-                self.message.format('an organisation-level', domain.canonical_domain)
-            )

app/main/views/add_service.py

@@ -2,17 +2,15 @@ from flask import current_app, redirect, render_template, session, url_for
 from flask_login import login_required
 from notifications_python_client.errors import HTTPError
 
-from app import billing_api_client, email_branding_client, service_api_client
+from app import billing_api_client, service_api_client
 from app.main import main
 from app.main.forms import CreateServiceForm
-from app.utils import AgreementInfo, email_safe, user_is_gov_user
+from app.utils import email_safe, user_is_gov_user
 
 
 def _create_service(service_name, organisation_type, email_from, form):
     free_sms_fragment_limit = current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get(organisation_type)
 
-    domain = 'nhs.uk' if organisation_type == 'nhs' else AgreementInfo.from_current_user().canonical_domain
-    email_branding = email_branding_client.get_email_branding_id_for_domain(domain)
     try:
         service_id = service_api_client.create_service(
             service_name=service_name,
@@ -21,13 +19,9 @@ def _create_service(service_name, organisation_type, email_from, form):
             restricted=True,
             user_id=session['user_id'],
             email_from=email_from,
-            service_domain=domain
         )
         session['service_id'] = service_id
 
-        if email_branding:
-            service_api_client.update_service(service_id, email_branding=email_branding)
-
         billing_api_client.create_or_update_free_sms_fragment_limit(service_id, free_sms_fragment_limit)
 
         return service_id, None

app/main/views/agreement.py

@@ -1,21 +1,17 @@
 from flask import abort, render_template, request, send_file, url_for
-from flask_login import login_required
+from flask_login import current_user, login_required
 
 from app.main import main
 from app.main.views.sub_navigation_dictionaries import features_nav
 from app.s3_client.s3_mou_client import get_mou
-from app.utils import AgreementInfo
 
 
 @main.route('/agreement')
 @login_required
 def agreement():
-
-    agreement_info = AgreementInfo.from_current_user()
-
     return render_template(
-        'views/{}.html'.format(agreement_info.as_jinja_template),
+        'views/{}.html'.format(current_user.default_organisation.as_jinja_template),
-        owner=agreement_info.owner,
+        owner=current_user.default_organisation.name,
         navigation_links=features_nav(),
     )
 
@@ -24,7 +20,7 @@ def agreement():
 @login_required
 def download_agreement():
     return send_file(**get_mou(
-        AgreementInfo.from_current_user().crown_status_or_404
+        current_user.default_organisation.crown_status_or_404
     ))
 
 
@@ -40,10 +36,8 @@ def public_agreement(variant):
             organisation_is_crown=(variant == 'crown')
         ))
 
-    agreement_info = AgreementInfo.from_current_user()
-
     return render_template(
         'views/agreement-public.html',
-        owner=agreement_info.owner,
+        owner=current_user.default_organisation.name,
         download_link=url_for('.public_download_agreement', variant=variant),
     )

app/main/views/email_branding.py

@@ -39,7 +39,6 @@ def update_email_branding(branding_id, logo=None):
         name=email_branding['name'],
         text=email_branding['text'],
         colour=email_branding['colour'],
-        domain=email_branding['domain'],
         brand_type=email_branding['brand_type']
     )
 
@@ -67,7 +66,6 @@ def update_email_branding(branding_id, logo=None):
             name=form.name.data,
             text=form.text.data,
             colour=form.colour.data,
-            domain=form.domain.data,
             brand_type=form.brand_type.data,
         )
 
@@ -115,7 +113,6 @@ def create_email_branding(logo=None):
             name=form.name.data,
             text=form.text.data,
             colour=form.colour.data,
-            domain=form.domain.data,
             brand_type=form.brand_type.data,
         )
 

app/main/views/index.py

@@ -16,7 +16,7 @@ from app import email_branding_client, letter_branding_client, status_api_client
 from app.main import main
 from app.main.forms import FieldWithNoneOption, SearchByNameForm
 from app.main.views.sub_navigation_dictionaries import features_nav
-from app.utils import AgreementInfo, get_logo_cdn_domain
+from app.utils import get_logo_cdn_domain
 
 
 @main.route('/')
@@ -80,7 +80,6 @@ def pricing():
             for cc, country in INTERNATIONAL_BILLING_RATES.items()
         ], key=lambda x: x[0]),
         search_form=SearchByNameForm(),
-        agreement_info=AgreementInfo.from_current_user(),
     )
 
 
@@ -254,7 +253,6 @@ def terms():
     return render_template(
         'views/terms-of-use.html',
         navigation_links=features_nav(),
-        agreement_info=AgreementInfo.from_current_user(),
     )
 
 

app/main/views/letter_branding.py

@@ -56,7 +56,6 @@ def update_letter_branding(branding_id, logo=None):
     file_upload_form = SVGFileUpload()
     letter_branding_details_form = ServiceLetterBrandingDetails(
         name=letter_branding['name'],
-        domain=letter_branding['domain']
     )
 
     file_upload_form_submitted = file_upload_form.file.data
@@ -87,7 +86,6 @@ def update_letter_branding(branding_id, logo=None):
                     branding_id=branding_id,
                     filename=db_filename,
                     name=letter_branding_details_form.name.data,
-                    domain=letter_branding_details_form.domain.data
                 )
 
                 return redirect(url_for('main.letter_branding'))
@@ -98,7 +96,6 @@ def update_letter_branding(branding_id, logo=None):
                     branding_id=branding_id,
                     filename=db_filename,
                     name=letter_branding_details_form.name.data,
-                    domain=letter_branding_details_form.domain.data
                 )
 
                 upload_letter_logos(logo, db_filename, png_file, session['user_id'])
@@ -106,9 +103,7 @@ def update_letter_branding(branding_id, logo=None):
                 return redirect(url_for('main.letter_branding'))
 
         except HTTPError as e:
-            if 'domain' in e.message:
+            if 'name' in e.message:
-                letter_branding_details_form.domain.errors.append(e.message['domain'][0])
-            elif 'name' in e.message:
                 letter_branding_details_form.name.errors.append(e.message['name'][0])
             else:
                 raise e
@@ -118,7 +113,6 @@ def update_letter_branding(branding_id, logo=None):
                 branding_id=branding_id,
                 filename=letter_branding['filename'],
                 name=letter_branding['name'],
-                domain=letter_branding['domain']
             )
             file_upload_form.file.errors = ['Error saving uploaded file - try uploading again']
 
@@ -165,7 +159,6 @@ def create_letter_branding(logo=None):
                 letter_branding_client.create_letter_branding(
                     filename=db_filename,
                     name=letter_branding_details_form.name.data,
-                    domain=letter_branding_details_form.domain.data,
                 )
 
                 upload_letter_logos(logo, db_filename, png_file, session['user_id'])
@@ -173,9 +166,7 @@ def create_letter_branding(logo=None):
                 return redirect(url_for('main.letter_branding'))
 
             except HTTPError as e:
-                if 'domain' in e.message:
+                if 'name' in e.message:
-                    letter_branding_details_form.domain.errors.append(e.message['domain'][0])
-                elif 'name' in e.message:
                     letter_branding_details_form.name.errors.append(e.message['name'][0])
                 else:
                     raise e

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=current_service.request_to_go_live_tags,
     )
 
     flash('Thanks for your request to go live. We’ll get back to you within one working day.', 'default')
@@ -980,7 +979,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,
@@ -1068,37 +1067,6 @@ 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, agreement_signed):
-
-    BASE = 'notify_request_to_go_live'
-
-    yield BASE
-
-    if service.go_live_checklist_completed and agreement_signed:
-        yield BASE + '_complete'
-        return
-
-    for test, tag in (
-        (True, ''),
-        (not service.volumes, '_volumes'),
-        (not service.go_live_checklist_completed, '_checklist'),
-        (not 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'),
-        (service.needs_to_change_sms_sender, '_sms_sender'),
-    ):
-        if test:
-            yield BASE + '_incomplete' + tag
-
-
 def print_if_number(value):
     return value if isinstance(value, int) else ''
 

app/models/__init__.py

@@ -0,0 +1,27 @@
+from flask import abort
+
+
+class JSONModel():
+
+    ALLOWED_PROPERTIES = set()
+
+    def __init__(self, _dict):
+        # in the case of a bad request _dict may be `None`
+        self._dict = _dict or {}
+
+    def __bool__(self):
+        return self._dict != {}
+
+    def __getattr__(self, attr):
+        if attr in self.ALLOWED_PROPERTIES:
+            return self._dict[attr]
+        raise AttributeError('`{}` is not a {} attribute'.format(
+            attr,
+            self.__class__.__name__.lower(),
+        ))
+
+    def _get_by_id(self, things, id):
+        try:
+            return next(thing for thing in things if thing['id'] == str(id))
+        except StopIteration:
+            abort(404)

app/models/organisation.py

@@ -0,0 +1,134 @@
+from flask import Markup, abort
+
+from app.models import JSONModel
+
+
+class Organisation(JSONModel):
+
+    ALLOWED_PROPERTIES = {
+        'id',
+        'name',
+        'active',
+        'crown',
+        'organisation_type',
+        'letter_branding_id',
+        'email_branding_id',
+        'agreement_signed',
+        'agreement_signed_at',
+        'agreement_signed_by_id',
+        'agreement_signed_version',
+        'domains',
+    }
+
+    def __init__(self, _dict):
+
+        super().__init__(_dict)
+
+        if self._dict == {}:
+            self.name, self.crown, self.agreement_signed = None, None, None
+
+    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.name:
+            return '{} (organisation is {}, {})'.format(
+                {
+                    False: 'No',
+                    None: 'Can’t tell',
+                }.get(self.agreement_signed),
+                self.name,
+                {
+                    True: 'a crown body',
+                    False: 'a non-crown body',
+                    None: 'crown status unknown',
+                }.get(self.crown),
+            )
+        else:
+            return 'Can’t tell (domain is {})'.format(fallback_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):
+        if self.crown is None:
+            return 'agreement-choose'
+        if self.agreement_signed:
+            return 'agreement-signed'
+        return 'agreement'
+
+    def as_terms_of_use_paragraph(self, **kwargs):
+        return Markup(self._as_terms_of_use_paragraph(**kwargs))
+
+    def _as_terms_of_use_paragraph(self, terms_link, download_link, support_link, signed_in):
+
+        if not signed_in:
+            return ((
+                '{} <a href="{}">Sign in</a> to download a copy '
+                'or find out if one is already in place.'
+            ).format(self._acceptance_required, terms_link))
+
+        if self.agreement_signed is None:
+            return ((
+                '{} <a href="{}">Download the agreement</a> or '
+                '<a href="{}">contact us</a> to find out if we already '
+                'have one in place with your organisation.'
+            ).format(self._acceptance_required, download_link, support_link))
+
+        if self.agreement_signed is False:
+            return ((
+                '{} <a href="{}">Download a copy</a>.'
+            ).format(self._acceptance_required, download_link))
+
+        return (
+            'Your organisation ({}) has already accepted the '
+            'GOV.UK&nbsp;Notify data sharing and financial '
+            'agreement.'.format(self.name)
+        )
+
+    def as_pricing_paragraph(self, **kwargs):
+        return Markup(self._as_pricing_paragraph(**kwargs))
+
+    def _as_pricing_paragraph(self, pricing_link, download_link, support_link, signed_in):
+
+        if not signed_in:
+            return ((
+                '<a href="{}">Sign in</a> to download a copy or find '
+                'out if one is already in place with your organisation.'
+            ).format(pricing_link))
+
+        if self.agreement_signed is None:
+            return ((
+                '<a href="{}">Download the agreement</a> or '
+                '<a href="{}">contact us</a> to find out if we already '
+                'have one in place with your organisation.'
+            ).format(download_link, support_link))
+
+        return (
+            '<a href="{}">Download the agreement</a> '
+            '({} {}).'.format(
+                download_link,
+                self.name,
+                {
+                    True: 'has already accepted it',
+                    False: 'hasn’t accepted it yet'
+                }.get(self.agreement_signed)
+            )
+        )
+
+    @property
+    def _acceptance_required(self):
+        return (
+            'Your organisation {} must also accept our data sharing '
+            'and financial agreement.'.format(
+                '({})'.format(self.name) if self.name else '',
+            )
+        )
+
+    @property
+    def crown_status_or_404(self):
+        if self.crown is None:
+            abort(404)
+        return self.crown

app/models/service.py

@@ -2,6 +2,8 @@ from flask import abort, current_app
 from notifications_utils.field import Field
 from werkzeug.utils import cached_property
 
+from app.models import JSONModel
+from app.models.organisation import Organisation
 from app.notify_client.api_key_api_client import api_key_api_client
 from app.notify_client.billing_api_client import billing_api_client
 from app.notify_client.email_branding_client import email_branding_client
@@ -18,7 +20,7 @@ from app.notify_client.user_api_client import user_api_client
 from app.utils import get_default_sms_sender
 
 
-class Service():
+class Service(JSONModel):
 
     ALLOWED_PROPERTIES = {
         'active',
@@ -50,25 +52,12 @@ class Service():
     )
 
     def __init__(self, _dict):
-        # in the case of a bad request current service may be `None`
+
-        self._dict = _dict or {}
+        super().__init__(_dict)
+
         if 'permissions' not in self._dict:
             self.permissions = {'email', 'sms', 'letter'}
 
-    def __bool__(self):
-        return self._dict != {}
-
-    def __getattr__(self, attr):
-        if attr in self.ALLOWED_PROPERTIES:
-            return self._dict[attr]
-        raise AttributeError('`{}` is not a service attribute'.format(attr))
-
-    def _get_by_id(self, things, id):
-        try:
-            return next(thing for thing in things if thing['id'] == str(id))
-        except StopIteration:
-            abort(404)
-
     def update(self, **kwargs):
         return service_api_client.update_service(self.id, **kwargs)
 
@@ -405,8 +394,10 @@ class Service():
         return None
 
     @cached_property
-    def organisation_name(self):
+    def organisation(self):
-        return organisations_client.get_service_organisation(self.id).get('name', None)
+        return Organisation(
+            organisations_client.get_service_organisation(self.id)
+        )
 
     @cached_property
     def inbound_number(self):
@@ -555,3 +546,30 @@ class Service():
 
     def get_api_key(self, id):
         return self._get_by_id(self.api_keys, id)
+
+    @property
+    def request_to_go_live_tags(self):
+        return list(self._get_request_to_go_live_tags())
+
+    def _get_request_to_go_live_tags(self):
+
+        BASE = 'notify_request_to_go_live'
+
+        yield BASE
+
+        if self.go_live_checklist_completed and self.organisation.agreement_signed:
+            yield BASE + '_complete'
+            return
+
+        for test, tag in (
+            (True, ''),
+            (not self.volumes, '_volumes'),
+            (not self.go_live_checklist_completed, '_checklist'),
+            (not self.organisation.agreement_signed, '_mou'),
+            (self.needs_to_add_email_reply_to_address, '_email_reply_to'),
+            (not self.has_team_members, '_team_member'),
+            (not self.has_templates, '_template_content'),
+            (self.needs_to_change_sms_sender, '_sms_sender'),
+        ):
+            if test:
+                yield BASE + '_incomplete' + tag

app/models/user.py

@@ -2,7 +2,10 @@ from itertools import chain
 
 from flask import abort, request, session
 from flask_login import AnonymousUserMixin, UserMixin
+from werkzeug.utils import cached_property
 
+from app.models.organisation import Organisation
+from app.notify_client.organisations_api_client import organisations_client
 from app.utils import is_gov_user
 
 roles = {
@@ -192,6 +195,16 @@ class User(UserMixin):
     def is_locked(self):
         return self.failed_login_count >= self.max_failed_login_count
 
+    @property
+    def email_domain(self):
+        return self.email_address.split('@')[-1]
+
+    @cached_property
+    def default_organisation(self):
+        return Organisation(
+            organisations_client.get_organisation_by_domain(self.email_domain)
+        )
+
     def serialize(self):
         dct = {
             "id": self.id,
@@ -322,3 +335,7 @@ class AnonymousUser(AnonymousUserMixin):
     # set the anonymous user so that if a new browser hits us we don't error http://stackoverflow.com/a/19275188
     def logged_in_elsewhere(self):
         return False
+
+    @property
+    def default_organisation(self):
+        return Organisation(None)

app/notify_client/email_branding_client.py

@@ -14,33 +14,25 @@ class EmailBrandingClient(NotifyAdminAPIClient):
             brandings.sort(key=lambda branding: branding[sort_key].lower())
         return brandings
 
-    def get_email_branding_id_for_domain(self, domain):
-        for branding in self.get_all_email_branding():
-            if domain and branding.get('domain') == domain:
-                return branding['id']
-        return None
-
     @cache.delete('email_branding')
-    def create_email_branding(self, logo, name, text, colour, domain, brand_type):
+    def create_email_branding(self, logo, name, text, colour, brand_type):
         data = {
             "logo": logo,
             "name": name,
             "text": text,
             "colour": colour,
-            "domain": domain,
             "brand_type": brand_type
         }
         return self.post(url="/email-branding", data=data)
 
     @cache.delete('email_branding')
     @cache.delete('email_branding-{branding_id}')
-    def update_email_branding(self, branding_id, logo, name, text, colour, domain, brand_type):
+    def update_email_branding(self, branding_id, logo, name, text, colour, brand_type):
         data = {
             "logo": logo,
             "name": name,
             "text": text,
             "colour": colour,
-            "domain": domain,
             "brand_type": brand_type
         }
         return self.post(url="/email-branding/{}".format(branding_id), data=data)

app/notify_client/letter_branding_client.py

@@ -12,22 +12,19 @@ class LetterBrandingClient(NotifyAdminAPIClient):
         return self.get(url='/letter-branding')
 
     @cache.delete('letter_branding')
-    def create_letter_branding(self, filename, name, domain):
+    def create_letter_branding(self, filename, name):
         data = {
             "filename": filename,
             "name": name,
-            "domain": domain,
         }
         return self.post(url="/letter-branding", data=data)
 
     @cache.delete('letter_branding')
     @cache.delete('letter_branding-{branding_id}')
-    def update_letter_branding(self, branding_id, filename, name, domain):
+    def update_letter_branding(self, branding_id, filename, name):
         data = {
             "filename": filename,
             "name": name,
-            "domain": domain,
-
         }
         return self.post(url="/letter-branding/{}".format(branding_id), data=data)
 

app/notify_client/organisations_api_client.py

@@ -1,3 +1,5 @@
+from notifications_python_client.errors import HTTPError
+
 from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache
 
 
@@ -9,6 +11,16 @@ class OrganisationsClient(NotifyAdminAPIClient):
     def get_organisation(self, org_id):
         return self.get(url='/organisations/{}'.format(org_id))
 
+    def get_organisation_by_domain(self, domain):
+        try:
+            return self.get(
+                url='/organisations/by-domain?domain={}'.format(domain),
+            )
+        except HTTPError as error:
+            if error.status_code == 404:
+                return None
+            raise error
+
     def create_organisation(self, name):
         data = {
             "name": name

app/templates/views/email-branding/manage-branding.html

@@ -28,7 +28,6 @@
           <div style='margin-top:15px;'>{{textbox(form.name)}}</div>
           <div style='margin-top:15px;'>{{textbox(form.text)}}</div>
           {{ textbox(form.colour, width='1-4', colour_preview=True) }}
-           <div style='margin-top:15px;'>{{textbox(form.domain)}}</div>
            {{ radios(form.brand_type) }}
           {{ page_footer(
             'Save',

app/templates/views/email-branding/select-branding.html

@@ -24,13 +24,6 @@
             {{ brand.name or 'Unnamed' }}
           </a>
         </div>
-        <p class="message-type">
-          {% if brand.domain %}
-            Default for {{ brand.domain }}
-          {% else %}
-            –
-          {% endif %}
-        </p>
       </div>
     {% endfor %}
   </nav>

app/templates/views/letter-branding/manage-letter-branding.html

@@ -25,7 +25,6 @@
       {% call form_wrapper() %}
         <div class="form-group">
           <div style='margin-top:15px;'>{{textbox(letter_branding_details_form.name)}}</div>
-          <div style='margin-top:15px;'>{{textbox(letter_branding_details_form.domain)}}</div>
           {{ page_footer(
             'Save',
             button_name='operation',

app/templates/views/letter-branding/select-letter-branding.html

@@ -24,13 +24,6 @@
             {{ brand.name }}
           </a>
         </div>
-        <p class="message-type">
-          {% if brand.domain %}
-            Default for {{ brand.domain }}
-          {% else %}
-            –
-          {% endif %}
-        </p>
       </div>
     {% endfor %}
   </nav>

app/templates/views/pricing.html

@@ -123,7 +123,7 @@
     <h2 class="heading-medium" id="paying">How to pay</h2>
     <p>You can find details of how to pay for Notify in our data sharing and financial agreement.</p>
     <p>
-      {{ agreement_info.as_pricing_paragraph(
+      {{ current_user.default_organisation.as_pricing_paragraph(
         pricing_link=url_for('main.sign_in', next=url_for('main.pricing', _anchor='paying')),
         download_link=url_for('main.agreement'),
         support_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement'),

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() %}

app/templates/views/terms-of-use.html

@@ -21,7 +21,7 @@ Terms of use
     </p>
 
     <p>
-      {{ agreement_info.as_terms_of_use_paragraph(
+      {{ current_user.default_organisation.as_terms_of_use_paragraph(
         terms_link=url_for('main.sign_in', next=url_for('main.terms')),
         download_link=url_for('.agreement'),
         support_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement'),

app/utils.py

@@ -13,15 +13,7 @@ import ago
 import dateutil
 import pyexcel
 import yaml
-from flask import (
+from flask import abort, current_app, redirect, request, session, url_for
-    Markup,
-    abort,
-    current_app,
-    redirect,
-    request,
-    session,
-    url_for,
-)
 from flask_login import current_user
 from notifications_utils.field import Field
 from notifications_utils.formatters import make_quotes_smart
@@ -44,6 +36,11 @@ FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure',
                     'technical-failure', 'virus-scan-failed', 'validation-failed']
 REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES
 
+with open('{}/email_domains.yml'.format(
+    os.path.dirname(os.path.realpath(__file__))
+)) as email_domains:
+    GOVERNMENT_EMAIL_DOMAIN_NAMES = yaml.safe_load(email_domains)
+
 
 def user_has_permissions(*permissions, **permission_kwargs):
     def wrap(func):
@@ -288,11 +285,13 @@ def get_help_argument():
 
 
 def is_gov_user(email_address):
-    try:
+    return any(
-        GovernmentEmailDomain(email_address)
+        email_address.lower().endswith((
-        return True
+            "@{}".format(known),
-    except NotGovernmentEmailDomain:
+            ".{}".format(known),
-        return False
+        ))
+        for known in GOVERNMENT_EMAIL_DOMAIN_NAMES
+    )
 
 
 def get_template(
@@ -406,204 +405,6 @@ def set_status_filters(filter_args):
     )))
 
 
-_dir_path = os.path.dirname(os.path.realpath(__file__))
-
-
-class AgreementInfo:
-
-    with open('{}/domains.yml'.format(_dir_path)) as domains:
-        domains = yaml.safe_load(domains)
-        domain_names = sorted(domains.keys(), key=len, reverse=True)
-
-    def __init__(self, email_address_or_domain):
-
-        self._match = next(filter(
-            self.get_matching_function(email_address_or_domain),
-            self.domain_names,
-        ), None)
-
-        self._domain = email_address_or_domain.split('@')[-1]
-
-        (
-            self.owner,
-            self.crown_status,
-            self.agreement_signed,
-            self.canonical_domain,
-        ) = self._get_info()
-
-    @classmethod
-    def from_user(cls, user):
-        return cls(user.email_address if user.is_authenticated else '')
-
-    @classmethod
-    def from_current_user(cls):
-        return cls.from_user(current_user)
-
-    @property
-    def as_human_readable(self):
-        if self.canonical_domain and 'dwp' in self.canonical_domain:
-            return 'DWP - Requires OED approval'
-        if self.agreement_signed:
-            return 'Yes, on behalf of {}'.format(self.owner)
-        elif self.owner:
-            return '{} (organisation is {}, {})'.format(
-                {
-                    False: 'No',
-                    None: 'Can’t tell',
-                }.get(self.agreement_signed),
-                self.owner,
-                {
-                    True: 'a crown body',
-                    False: 'a non-crown body',
-                    None: 'crown status unknown',
-                }.get(self.crown_status),
-            )
-        else:
-            return 'Can’t tell (domain is {})'.format(self._domain)
-
-    @property
-    def as_info_for_branding_request(self):
-        return self.owner or 'Can’t tell (domain is {})'.format(self._domain)
-
-    @property
-    def as_jinja_template(self):
-        if self.crown_status is None:
-            return 'agreement-choose'
-        if self.agreement_signed:
-            return 'agreement-signed'
-        return 'agreement'
-
-    def as_terms_of_use_paragraph(self, **kwargs):
-        return Markup(self._as_terms_of_use_paragraph(**kwargs))
-
-    def _as_terms_of_use_paragraph(self, terms_link, download_link, support_link, signed_in):
-
-        if not signed_in:
-            return ((
-                '{} <a href="{}">Sign in</a> to download a copy '
-                'or find out if one is already in place.'
-            ).format(self._acceptance_required, terms_link))
-
-        if self.agreement_signed is None:
-            return ((
-                '{} <a href="{}">Download the agreement</a> or '
-                '<a href="{}">contact us</a> to find out if we already '
-                'have one in place with your organisation.'
-            ).format(self._acceptance_required, download_link, support_link))
-
-        if self.agreement_signed is False:
-            return ((
-                '{} <a href="{}">Download a copy</a>.'
-            ).format(self._acceptance_required, download_link))
-
-        return (
-            'Your organisation ({}) has already accepted the '
-            'GOV.UK&nbsp;Notify data sharing and financial '
-            'agreement.'.format(self.owner)
-        )
-
-    def as_pricing_paragraph(self, **kwargs):
-        return Markup(self._as_pricing_paragraph(**kwargs))
-
-    def _as_pricing_paragraph(self, pricing_link, download_link, support_link, signed_in):
-
-        if not signed_in:
-            return ((
-                '<a href="{}">Sign in</a> to download a copy or find '
-                'out if one is already in place with your organisation.'
-            ).format(pricing_link))
-
-        if self.agreement_signed is None:
-            return ((
-                '<a href="{}">Download the agreement</a> or '
-                '<a href="{}">contact us</a> to find out if we already '
-                'have one in place with your organisation.'
-            ).format(download_link, support_link))
-
-        return (
-            '<a href="{}">Download the agreement</a> '
-            '({} {}).'.format(
-                download_link,
-                self.owner,
-                {
-                    True: 'has already accepted it',
-                    False: 'hasn’t accepted it yet'
-                }.get(self.agreement_signed)
-            )
-        )
-
-    @property
-    def _acceptance_required(self):
-        return (
-            'Your organisation {} must also accept our data sharing '
-            'and financial agreement.'.format(
-                '({})'.format(self.owner) if self.owner else '',
-            )
-        )
-
-    @property
-    def crown_status_or_404(self):
-        if self.crown_status is None:
-            abort(404)
-        return self.crown_status
-
-    @staticmethod
-    def get_matching_function(email_address_or_domain):
-
-        email_address_or_domain = email_address_or_domain.lower()
-
-        def fn(domain):
-
-            return (
-                email_address_or_domain == domain
-            ) or (
-                email_address_or_domain.endswith("@{}".format(domain))
-            ) or (
-                email_address_or_domain.endswith(".{}".format(domain))
-            )
-
-        return fn
-
-    def _get_info(self):
-
-        details = self.domains.get(self._match, {})
-
-        if details is None:
-            raise TypeError('Domain must have details ({})'.format(self._domain))
-
-        if isinstance(details, str):
-            self.is_canonical = False
-            return AgreementInfo(details)._get_info()
-
-        elif isinstance(details, dict):
-            self.is_canonical = bool(details)
-            return(
-                details.get("owner"),
-                details.get("crown"),
-                details.get("agreement_signed"),
-                self._match,
-            )
-
-
-class NotGovernmentEmailDomain(Exception):
-    pass
-
-
-class GovernmentEmailDomain(AgreementInfo):
-
-    with open('{}/email_domains.yml'.format(_dir_path)) as email_domains:
-        domain_names = yaml.safe_load(email_domains)
-
-    def __init__(self, email_address_or_domain):
-        try:
-            self._match = next(filter(
-                self.get_matching_function(email_address_or_domain),
-                self.domain_names,
-            ))
-        except StopIteration:
-            raise NotGovernmentEmailDomain()
-
-
 def unicode_truncate(s, length):
     encoded = s.encode('utf-8')[:length]
     return encoded.decode('utf-8', 'ignore')

tests/__init__.py

@@ -184,7 +184,7 @@ def service_json(
 
 def organisation_json(
     id_='1234',
-    name='Test Organisation',
+    name=False,
     users=None,
     active=True,
     created_at=None,
@@ -192,6 +192,8 @@ def organisation_json(
     letter_branding_id=None,
     email_branding_id=None,
     domains=None,
+    crown=True,
+    agreement_signed=False,
 ):
     if users is None:
         users = []
@@ -199,7 +201,7 @@ def organisation_json(
         services = []
     return {
         'id': id_,
-        'name': name,
+        'name': 'Test Organisation' if name is False else name,
         'active': active,
         'users': users,
         'services': services,
@@ -207,8 +209,8 @@ def organisation_json(
         'email_branding_id': email_branding_id,
         'letter_branding_id': letter_branding_id,
         'organisation_type': '',
-        'crown': True,
+        'crown': crown,
-        'agreement_signed': False,
+        'agreement_signed': agreement_signed,
         'agreement_signed_at': None,
         'agreement_signed_by': None,
         'domains': domains or [],

tests/app/main/views/test_add_service.py

@@ -56,7 +56,6 @@ def test_should_add_service_and_redirect_to_tour_when_no_services(
         restricted=True,
         user_id=api_user_active.id,
         email_from='testing.the.post',
-        service_domain=None
     )
     mock_create_service_template.assert_called_once_with(
         'Example text message template',
@@ -71,10 +70,10 @@ def test_should_add_service_and_redirect_to_tour_when_no_services(
     mock_create_or_update_free_sms_fragment_limit.assert_called_once_with(101, 25000)
 
 
-@pytest.mark.parametrize('organisation_type, free_allowance, service_domain', [
+@pytest.mark.parametrize('organisation_type, free_allowance', [
-    ('central', 250 * 1000, None),
+    ('central', 250 * 1000),
-    ('local', 25 * 1000, None),
+    ('local', 25 * 1000),
-    ('nhs', 25 * 1000, 'nhs.uk'),
+    ('nhs', 25 * 1000),
 ])
 def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
     app_,
@@ -82,11 +81,9 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
     mock_create_service,
     mock_create_service_template,
     mock_get_services,
-    mock_update_service,
     api_user_active,
     organisation_type,
     free_allowance,
-    service_domain,
     mock_create_or_update_free_sms_fragment_limit,
     mock_get_all_email_branding,
 ):
@@ -111,56 +108,12 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service(
         restricted=True,
         user_id=api_user_active.id,
         email_from='testing.the.post',
-        service_domain=service_domain
     )
     mock_create_or_update_free_sms_fragment_limit.assert_called_once_with(101, free_allowance)
     assert len(mock_create_service_template.call_args_list) == 0
     assert session['service_id'] == 101
 
 
-@pytest.mark.parametrize('organisation_type, email_address, expected_branding', [
-    ('central', 'test@example.voa.gsi.gov.uk', '5'),
-    ('central', 'test@example.voa.gov.uk', '5'),
-    ('central', 'test@example.gov.uk', None),
-    # Anyone choosing ‘NHS’ for organisation type gets NHS branding no
-    # matter what their email domain is (but we look it up based on the
-    # `nhs.uk` domain to avoid hard-coding a branding ID anywhere)
-    ('nhs', 'test@example.voa.gov.uk', '4'),
-    ('nhs', 'test@nhs.uk', '4'),
-])
-def test_should_lookup_branding_for_known_domain(
-    app_,
-    client_request,
-    active_user_with_permissions,
-    mock_create_service,
-    mock_get_services,
-    mock_update_service,
-    mock_create_or_update_free_sms_fragment_limit,
-    mock_get_all_email_branding,
-    organisation_type,
-    email_address,
-    expected_branding,
-):
-    active_user_with_permissions.email_address = email_address
-    client_request.login(active_user_with_permissions)
-    client_request.post(
-        'main.add_service',
-        _data={
-            'name': 'testing the post',
-            'organisation_type': organisation_type,
-        }
-    )
-    mock_get_all_email_branding.assert_called_once_with()
-    assert mock_create_service.called is True
-    if expected_branding:
-        mock_update_service.assert_called_once_with(
-            101,
-            email_branding=expected_branding,
-        )
-    else:
-        assert mock_update_service.called is False
-
-
 def test_should_return_form_errors_when_service_name_is_empty(
     client_request
 ):

tests/app/main/views/test_agreement.py

@@ -4,7 +4,7 @@ from io import BytesIO
 import pytest
 from flask import url_for
 
-from tests.conftest import active_user_with_permissions
+from tests.conftest import mock_get_organisation_by_domain
 
 
 class _MockS3Object():
@@ -16,22 +16,22 @@ class _MockS3Object():
         return {'Body': BytesIO(self.data)}
 
 
-@pytest.mark.parametrize('email_address, expected_links', [
+@pytest.mark.parametrize('agreement_signed, crown, expected_links', [
     (
-        'test@cabinet-office.gov.uk',
+        True, True,
         [
             partial(url_for, 'main.download_agreement'),
         ]
     ),
     (
-        'test@aylesburytowncouncil.gov.uk',
+        False, False,
         [
             partial(url_for, 'main.download_agreement'),
             lambda: 'mailto:notify-support@digital.cabinet-office.gov.uk',
         ]
     ),
     (
-        'test@unknown.gov.uk',
+        None, None,
         [
             partial(url_for, 'main.public_download_agreement', variant='crown'),
             partial(url_for, 'main.public_download_agreement', variant='non-crown'),
@@ -44,12 +44,15 @@ def test_show_agreement_page(
     client_request,
     mocker,
     fake_uuid,
-    email_address,
+    agreement_signed,
+    crown,
     expected_links,
 ):
-    user = active_user_with_permissions(fake_uuid)
+    mock_get_organisation_by_domain(
-    user.email_address = email_address
+        mocker,
-    mocker.patch('app.user_api_client.get_user', return_value=user)
+        crown=crown,
+        agreement_signed=agreement_signed,
+    )
     page = client_request.get('main.agreement')
     links = page.select('main .column-two-thirds a')
     assert len(links) == len(expected_links)
@@ -57,14 +60,14 @@ def test_show_agreement_page(
         assert link['href'] == expected_links[index]()
 
 
-@pytest.mark.parametrize('email_address, expected_file_fetched, expected_file_served', [
+@pytest.mark.parametrize('crown, expected_file_fetched, expected_file_served', [
     (
-        'test@cabinet-office.gov.uk',
+        True,
         'crown.pdf',
         'GOV.UK Notify data sharing and financial agreement.pdf',
     ),
     (
-        'test@aylesburytowncouncil.gov.uk',
+        False,
         'non-crown.pdf',
         'GOV.UK Notify data sharing and financial agreement (non-crown).pdf',
     ),
@@ -73,7 +76,7 @@ def test_downloading_agreement(
     logged_in_client,
     mocker,
     fake_uuid,
-    email_address,
+    crown,
     expected_file_fetched,
     expected_file_served,
 ):
@@ -81,9 +84,10 @@ def test_downloading_agreement(
         'app.s3_client.s3_mou_client.get_s3_object',
         return_value=_MockS3Object(b'foo')
     )
-    user = active_user_with_permissions(fake_uuid)
+    mock_get_organisation_by_domain(
-    user.email_address = email_address
+        mocker,
-    mocker.patch('app.user_api_client.get_user', return_value=user)
+        crown=crown,
+    )
     response = logged_in_client.get(url_for('main.download_agreement'))
     assert response.status_code == 200
     assert response.get_data() == b'foo'
@@ -103,9 +107,10 @@ def test_agreement_cant_be_downloaded_unknown_crown_status(
         'app.s3_client.s3_mou_client.get_s3_object',
         return_value=_MockS3Object()
     )
-    user = active_user_with_permissions(fake_uuid)
+    mock_get_organisation_by_domain(
-    user.email_address = 'test@unknown.gov.uk'
+        mocker,
-    mocker.patch('app.user_api_client.get_user', return_value=user)
+        crown=None,
+    )
     response = logged_in_client.get(url_for('main.download_agreement'))
     assert response.status_code == 404
     assert mock_get_s3_object.call_args_list == []

tests/app/main/views/test_email_branding.py

@@ -28,7 +28,6 @@ def test_email_branding_page_shows_full_branding_list(
     links = page.select('.message-name a')
     brand_names = [normalize_spaces(link.text) for link in links]
     hrefs = [link['href'] for link in links]
-    brand_hints = [normalize_spaces(hint.text) for hint in page.select('.message-type')]
 
     assert normalize_spaces(
         page.select_one('h1').text
@@ -36,14 +35,12 @@ def test_email_branding_page_shows_full_branding_list(
 
     assert page.select_one('.column-three-quarters a')['href'] == url_for('main.create_email_branding')
 
-    assert list(zip(
+    assert brand_names == [
-        brand_names, brand_hints
+        'org 1',
-    )) == [
+        'org 2',
-        ('org 1', '–'),
+        'org 3',
-        ('org 2', '–'),
+        'org 4',
-        ('org 3', '–'),
+        'org 5',
-        ('org 4', 'Default for nhs.uk'),
-        ('org 5', 'Default for voa.gov.uk'),
     ]
     assert hrefs == [
         url_for('.update_email_branding', branding_id=1),
@@ -70,7 +67,6 @@ def test_edit_email_branding_shows_the_correct_branding_info(
     assert page.select_one('#name').attrs.get('value') == 'Organisation name'
     assert page.select_one('#text').attrs.get('value') == 'Organisation text'
     assert page.select_one('#colour').attrs.get('value') == '#f00'
-    assert page.select_one('#domain').attrs.get('value') == 'sample.com'
 
 
 def test_create_email_branding_does_not_show_any_branding_info(
@@ -89,27 +85,19 @@ def test_create_email_branding_does_not_show_any_branding_info(
     assert page.select_one('#name').attrs.get('value') == ''
     assert page.select_one('#text').attrs.get('value') == ''
     assert page.select_one('#colour').attrs.get('value') == ''
-    assert page.select_one('#domain').attrs.get('value') == ''
 
 
-@pytest.mark.parametrize('posted_domain, persisted_domain', [
-    ('voa.gov.uk', 'voa.gov.uk'),
-    ('', None),
-])
 def test_create_new_email_branding_without_logo(
     logged_in_platform_admin_client,
     mocker,
     fake_uuid,
     mock_create_email_branding,
-    posted_domain,
-    persisted_domain,
 ):
     data = {
         'logo': None,
         'colour': '#ff0000',
         'text': 'new text',
         'name': 'new name',
-        'domain': posted_domain,
         'brand_type': 'org'
     }
 
@@ -128,96 +116,11 @@ def test_create_new_email_branding_without_logo(
         name=data['name'],
         text=data['text'],
         colour=data['colour'],
-        domain=persisted_domain,
         brand_type=data['brand_type']
     )
     assert mock_persist.call_args_list == []
 
 
-def test_cant_create_new_email_branding_with_unknown_domain(
-    client_request,
-    mocker,
-    fake_uuid,
-    mock_create_email_branding
-):
-    mock_persist = mocker.patch('app.main.views.email_branding.persist_logo')
-    mocker.patch('app.main.views.email_branding.delete_email_temp_files_created_by')
-
-    client_request.login(platform_admin_user(fake_uuid))
-    page = client_request.post(
-        '.create_email_branding',
-        content_type='multipart/form-data',
-        _data={
-            'logo': None,
-            'colour': '#ff0000',
-            'text': 'new text',
-            'name': 'new name',
-            'domain': 'example.gov.uk',
-            'brand_type': 'org',
-        },
-        _expected_status=200,
-    )
-
-    assert mock_create_email_branding.called is False
-    assert mock_persist.called is False
-
-    assert page.select_one('.error-message').text.strip() == (
-        'Not a known government domain (you might need to update domains.yml)'
-    )
-    assert page.select_one('input[name=domain]')['value'] == (
-        'example.gov.uk'
-    )
-
-
-@pytest.mark.parametrize('posted_domain, expected_error', [
-    (
-        'voa.gsi.gov.uk',
-        'Not a canonical domain (use voa.gov.uk if appropriate)',
-    ),
-    (
-        'hmcts.net',
-        'Not a canonical domain (use hmcts.gov.uk if appropriate)',
-    ),
-    (
-        'southend.essex.gov.uk',
-        'Not an organisation-level domain (use essex.gov.uk if appropriate)',
-    ),
-    pytest.param(
-        'voa.gov.uk',
-        '',
-        marks=pytest.mark.xfail(raises=AssertionError)
-    ),
-])
-def test_rejects_non_canonical_domain_when_adding_email_branding(
-    client_request,
-    mocker,
-    fake_uuid,
-    mock_create_email_branding,
-    posted_domain,
-    expected_error,
-):
-    mocker.patch('app.main.views.email_branding.persist_logo')
-    mocker.patch('app.main.views.email_branding.delete_email_temp_files_created_by')
-    data = {
-        'logo': None,
-        'colour': '#ff0000',
-        'text': 'new text',
-        'name': 'new name',
-        'domain': posted_domain,
-        'brand_type': 'org',
-    }
-    client_request.login(platform_admin_user(fake_uuid))
-    page = client_request.post(
-        '.create_email_branding',
-        content_type='multipart/form-data',
-        _data=data,
-        _expected_status=200,
-    )
-
-    assert page.select_one('.error-message').text.strip() == expected_error
-    assert mock_create_email_branding.called is False
-
-
 def test_create_email_branding_requires_a_name_when_submitting_logo_details(
     client_request,
     mocker,
@@ -232,7 +135,6 @@ def test_create_email_branding_requires_a_name_when_submitting_logo_details(
         'colour': '#ff0000',
         'text': 'new text',
         'name': '',
-        'domain': '',
         'brand_type': 'org',
     }
     client_request.login(platform_admin_user(fake_uuid))
@@ -258,7 +160,6 @@ def test_create_email_branding_does_not_require_a_name_when_uploading_a_file(
         'colour': '',
         'text': '',
         'name': '',
-        'domain': '',
         'brand_type': 'org',
     }
     client_request.login(platform_admin_user(fake_uuid))
@@ -286,7 +187,6 @@ def test_create_new_email_branding_when_branding_saved(
         'colour': '#ff0000',
         'text': 'new text',
         'name': 'new name',
-        'domain': 'voa.gov.uk',
         'brand_type': 'org_banner'
     }
 
@@ -307,7 +207,6 @@ def test_create_new_email_branding_when_branding_saved(
             'name': data['name'],
             'text': data['text'],
             'cdn_url': 'https://static-logos.cdn.com',
-            'domain': data['domain'],
             'brand_type': data['brand_type']
         }
     )
@@ -320,7 +219,6 @@ def test_create_new_email_branding_when_branding_saved(
         name=data['name'],
         text=data['text'],
         colour=data['colour'],
-        domain=data['domain'],
         brand_type=data['brand_type']
     )
 
@@ -387,7 +285,6 @@ def test_update_existing_branding(
         'colour': '#0000ff',
         'text': 'new text',
         'name': 'new name',
-        'domain': 'voa.gov.uk',
         'brand_type': 'both'
     }
 
@@ -405,7 +302,7 @@ def test_update_existing_branding(
         content_type='multipart/form-data',
         data={'colour': data['colour'], 'name': data['name'], 'text': data['text'],
               'cdn_url': 'https://static-logos.cdn.com',
-              'domain': data['domain'], 'brand_type': data['brand_type']
+              'brand_type': data['brand_type']
               }
     )
 
@@ -418,7 +315,6 @@ def test_update_existing_branding(
         name=data['name'],
         text=data['text'],
         colour=data['colour'],
-        domain=data['domain'],
         brand_type=data['brand_type']
     )
 
@@ -526,7 +422,6 @@ def test_colour_regex_validation(
         'colour': colour_hex,
         'text': 'new text',
         'name': 'new name',
-        'domain': 'voa.gov.uk',
         'brand_type': 'org'
     }
 

tests/app/main/views/test_index.py

@@ -6,7 +6,7 @@ from flask import url_for
 
 from app.main.forms import FieldWithNoneOption
 from tests.conftest import (
-    active_user_with_permissions,
+    mock_get_organisation_by_domain,
     normalize_spaces,
     sample_uuid,
 )
@@ -82,6 +82,7 @@ def test_robots(client):
 ])
 def test_static_pages(
     client_request,
+    mock_get_organisation_by_domain,
     view,
 ):
     page = client_request.get('main.{}'.format(view))
@@ -179,13 +180,15 @@ def test_pricing_is_generic_if_user_is_not_logged_in(
 
 
 @pytest.mark.parametrize((
-    'email_address,'
+    'name,'
+    'agreement_signed,'
     'expected_terms_paragraph,'
     'expected_terms_link,'
     'expected_pricing_paragraph'
 ), [
     (
-        'test@cabinet-office.gov.uk',
+        'Cabinet Office',
+        True,
         (
             'Your organisation (Cabinet Office) has already accepted '
             'the GOV.UK Notify data sharing and financial agreement.'
@@ -197,7 +200,8 @@ def test_pricing_is_generic_if_user_is_not_logged_in(
         ),
     ),
     (
-        'test@aylesburytowncouncil.gov.uk',
+        'Aylesbury Town Council',
+        False,
         (
             'Your organisation (Aylesbury Town Council) must also '
             'accept our data sharing and financial agreement. Download '
@@ -213,7 +217,8 @@ def test_pricing_is_generic_if_user_is_not_logged_in(
         ),
     ),
     (
-        'larry@downing-street.gov.uk',
+        None,
+        None,
         (
             'Your organisation must also accept our data sharing and '
             'financial agreement. Download the agreement or contact us '
@@ -230,7 +235,8 @@ def test_pricing_is_generic_if_user_is_not_logged_in(
         ),
     ),
     (
-        'michael.fish@metoffice.gov.uk',
+        'Met Office',
+        False,
         (
             'Your organisation (Met Office) must also accept our data '
             'sharing and financial agreement. Download a copy.'
@@ -249,14 +255,17 @@ def test_terms_tells_logged_in_users_what_we_know_about_their_agreement(
     mocker,
     fake_uuid,
     client_request,
-    email_address,
+    name,
+    agreement_signed,
     expected_terms_paragraph,
     expected_terms_link,
     expected_pricing_paragraph,
 ):
-    user = active_user_with_permissions(fake_uuid)
+    mock_get_organisation_by_domain(
-    user.email_address = email_address
+        mocker,
-    mocker.patch('app.user_api_client.get_user', return_value=user)
+        name=name,
+        agreement_signed=agreement_signed,
+    )
     terms_page = client_request.get('main.terms')
     pricing_page = client_request.get('main.pricing')
     assert normalize_spaces(terms_page.select('main p')[1].text) == expected_terms_paragraph
@@ -269,7 +278,7 @@ def test_terms_tells_logged_in_users_what_we_know_about_their_agreement(
 
 def test_css_is_served_from_correct_path(client_request):
 
-    page = client_request.get('main.pricing')  # easy static page
+    page = client_request.get('main.documentation')  # easy static page
 
     for index, link in enumerate(
         page.select('link[rel=stylesheet]')

tests/app/main/views/test_letter_branding.py

@@ -2,7 +2,6 @@ from io import BytesIO
 from unittest.mock import Mock, call
 from uuid import UUID
 
-import pytest
 from botocore.exceptions import ClientError as BotoClientError
 from bs4 import BeautifulSoup
 from flask import current_app, url_for
@@ -29,7 +28,6 @@ def test_letter_branding_page_shows_full_branding_list(
     links = page.select('.message-name a')
     brand_names = [normalize_spaces(link.text) for link in links]
     hrefs = [link['href'] for link in links]
-    brand_hints = [normalize_spaces(hint.text) for hint in page.select('.message-type')]
 
     assert normalize_spaces(
         page.select_one('h1').text
@@ -37,12 +35,10 @@ def test_letter_branding_page_shows_full_branding_list(
 
     assert page.select_one('.column-three-quarters a')['href'] == url_for('main.create_letter_branding')
 
-    assert list(zip(
+    assert brand_names == [
-        brand_names, brand_hints
+        'HM Government',
-    )) == [
+        'Land Registry',
-        ('HM Government', '–'),
+        'Animal and Plant Health Agency',
-        ('Land Registry', 'Default for landregistry.gov.uk'),
-        ('Animal and Plant Health Agency', '–'),
     ]
 
     assert hrefs == [
@@ -66,7 +62,6 @@ def test_update_letter_branding_shows_the_current_letter_brand(
     assert page.find('h1').text == 'Update letter branding'
     assert page.select_one('#logo-img > img')['src'].endswith('/hm-government.svg')
     assert page.select_one('#name').attrs.get('value') == 'HM Government'
-    assert page.select_one('#domain').attrs.get('value') == 'cabinet-office.gov.uk'
 
 
 def test_update_letter_branding_with_new_valid_file(
@@ -95,7 +90,6 @@ def test_update_letter_branding_with_new_valid_file(
 
     assert page.select_one('#logo-img > img')['src'].endswith(expected_temp_filename)
     assert page.select_one('#name').attrs.get('value') == 'HM Government'
-    assert page.select_one('#domain').attrs.get('value') == 'cabinet-office.gov.uk'
 
     assert mock_s3_upload.called
     mock_delete_temp_files.assert_not_called()
@@ -161,7 +155,6 @@ def test_update_letter_branding_with_original_file_and_new_details(
         url_for('.update_letter_branding', branding_id=fake_uuid),
         data={
             'name': 'Updated name',
-            'domain': 'bl.uk',
             'operation': 'branding-details'
         },
         follow_redirects=True
@@ -175,42 +168,12 @@ def test_update_letter_branding_with_original_file_and_new_details(
 
     mock_client_update.assert_called_once_with(
         branding_id=fake_uuid,
-        domain='bl.uk',
         filename='hm-government',
         name='Updated name'
     )
 
 
-def test_update_letter_branding_does_not_require_a_domain(
+def test_update_letter_branding_shows_form_errors_on_name_fields(
-    mocker,
-    logged_in_platform_admin_client,
-    mock_get_all_letter_branding,
-    mock_get_letter_branding_by_id,
-    fake_uuid
-):
-    mock_client_update = mocker.patch('app.main.views.letter_branding.letter_branding_client.update_letter_branding')
-    logo = permanent_letter_logo_name('hm-government', 'svg')
-
-    response = logged_in_platform_admin_client.post(
-        url_for('.update_letter_branding', branding_id=fake_uuid, logo=logo),
-        data={
-            'name': 'Updated name',
-            'domain': '',
-            'operation': 'branding-details'
-        },
-        follow_redirects=True
-    )
-    assert response.status_code == 200
-
-    mock_client_update.assert_called_once_with(
-        branding_id=fake_uuid,
-        domain=None,
-        filename='hm-government',
-        name='Updated name'
-    )
-
-
-def test_update_letter_branding_shows_form_errors_on_name_and_domain_fields(
     mocker,
     logged_in_platform_admin_client,
     mock_get_letter_branding_by_id,
@@ -224,7 +187,6 @@ def test_update_letter_branding_shows_form_errors_on_name_and_domain_fields(
         url_for('.update_letter_branding', branding_id=fake_uuid, logo=logo),
         data={
             'name': '',
-            'domain': 'example.com',
             'operation': 'branding-details'
         },
         follow_redirects=True
@@ -234,18 +196,15 @@ def test_update_letter_branding_shows_form_errors_on_name_and_domain_fields(
     error_messages = page.find_all('span', class_='error-message')
 
     assert page.find('h1').text == 'Update letter branding'
-    assert len(error_messages) == 2
+    assert len(error_messages) == 1
     assert error_messages[0].text.strip() == 'This field is required.'
-    assert error_messages[1].text.strip() == 'Not a known government domain (you might need to update domains.yml)'
 
 
-@pytest.mark.parametrize('error_field', ['name', 'domain'])
+def test_update_letter_branding_shows_database_errors_on_name_field(
-def test_update_letter_branding_shows_database_errors_on_name_and_domain_fields(
     mocker,
     logged_in_platform_admin_client,
     mock_get_letter_branding_by_id,
     fake_uuid,
-    error_field
 ):
     mocker.patch('app.main.views.letter_branding.get_png_file_from_svg')
     mocker.patch('app.main.views.letter_branding.letter_branding_client.update_letter_branding', side_effect=HTTPError(
@@ -254,20 +213,19 @@ def test_update_letter_branding_shows_database_errors_on_name_and_domain_fields(
             json={
                 'result': 'error',
                 'message': {
-                    error_field: {
+                    'name': {
-                        '{} already in use'.format(error_field)
+                        'name already in use'
                     }
                 }
             }
         ),
-        message={error_field: ['{} already in use'.format(error_field)]}
+        message={'name': ['name already in use']}
     ))
 
     response = logged_in_platform_admin_client.post(
         url_for('.update_letter_branding', branding_id='abc'),
         data={
             'name': 'my brand',
-            'domain': None,
             'operation': 'branding-details'
         }
     )
@@ -276,7 +234,7 @@ def test_update_letter_branding_shows_database_errors_on_name_and_domain_fields(
     error_message = page.find('span', class_='error-message').text.strip()
 
     assert page.find('h1').text == 'Update letter branding'
-    assert error_message == '{} already in use'.format(error_field)
+    assert error_message == 'name already in use'
 
 
 def test_update_letter_branding_with_new_file_and_new_details(
@@ -300,7 +258,6 @@ def test_update_letter_branding_with_new_file_and_new_details(
         url_for('.update_letter_branding', branding_id=fake_uuid, logo=temp_logo),
         data={
             'name': 'Updated name',
-            'domain': 'bl.uk',
             'operation': 'branding-details'
         },
         follow_redirects=True
@@ -312,7 +269,6 @@ def test_update_letter_branding_with_new_file_and_new_details(
     assert mock_template_preview.called
     mock_client_update.assert_called_once_with(
         branding_id=fake_uuid,
-        domain='bl.uk',
         filename='{}-new_file'.format(fake_uuid),
         name='Updated name'
     )
@@ -343,7 +299,6 @@ def test_update_letter_branding_rolls_back_db_changes_and_shows_error_if_saving_
         url_for('.update_letter_branding', branding_id=fake_uuid, logo=temp_logo),
         data={
             'name': 'Updated name',
-            'domain': 'bl.uk',
             'operation': 'branding-details'
         },
         follow_redirects=True
@@ -355,8 +310,8 @@ def test_update_letter_branding_rolls_back_db_changes_and_shows_error_if_saving_
 
     assert mock_client_update.call_count == 2
     assert mock_client_update.call_args_list == [
-        call(branding_id=fake_uuid, domain='bl.uk', filename='{}-new_file'.format(fake_uuid), name='Updated name'),
+        call(branding_id=fake_uuid, filename='{}-new_file'.format(fake_uuid), name='Updated name'),
-        call(branding_id=fake_uuid, domain='cabinet-office.gov.uk', filename='hm-government', name='HM Government')
+        call(branding_id=fake_uuid, filename='hm-government', name='HM Government')
     ]
 
 
@@ -370,7 +325,6 @@ def test_create_letter_branding_does_not_show_branding_info(logged_in_platform_a
 
     assert page.select_one('#logo-img > img') is None
     assert page.select_one('#name').attrs.get('value') == ''
-    assert page.select_one('#domain').attrs.get('value') == ''
 
 
 def test_create_letter_branding_when_uploading_valid_file(
@@ -472,7 +426,6 @@ def test_create_letter_branding_shows_an_error_when_submitting_details_with_no_l
         url_for('.create_letter_branding'),
         data={
             'name': 'Test brand',
-            'domain': 'bl.uk',
             'operation': 'branding-details'
         }
     )
@@ -506,7 +459,6 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid(
         url_for('.create_letter_branding', logo=temp_logo),
         data={
             'name': 'Test brand',
-            'domain': 'bl.uk',
             'operation': 'branding-details'
         },
         follow_redirects=True
@@ -517,7 +469,7 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid(
     assert page.find('h1').text == 'Letter branding'
 
     mock_letter_client.create_letter_branding.assert_called_once_with(
-        domain='bl.uk', filename='{}-test'.format(fake_uuid), name='Test brand'
+        filename='{}-test'.format(fake_uuid), name='Test brand'
     )
     assert mock_template_preview.called
     mock_persist_logo.assert_called_once_with(
@@ -532,7 +484,7 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid(
     mock_delete_temp_files.assert_called_once_with(user_id)
 
 
-def test_create_letter_branding_shows_form_errors_on_name_and_domain_fields(
+def test_create_letter_branding_shows_form_errors_on_name_field(
     logged_in_platform_admin_client,
     fake_uuid
 ):
@@ -545,7 +497,6 @@ def test_create_letter_branding_shows_form_errors_on_name_and_domain_fields(
         url_for('.create_letter_branding', logo=temp_logo),
         data={
             'name': '',
-            'domain': 'example.com',
             'operation': 'branding-details'
         }
     )
@@ -554,17 +505,14 @@ def test_create_letter_branding_shows_form_errors_on_name_and_domain_fields(
     error_messages = page.find_all('span', class_='error-message')
 
     assert page.find('h1').text == 'Add letter branding'
-    assert len(error_messages) == 2
+    assert len(error_messages) == 1
     assert error_messages[0].text.strip() == 'This field is required.'
-    assert error_messages[1].text.strip() == 'Not a known government domain (you might need to update domains.yml)'
 
 
-@pytest.mark.parametrize('error_field', ['name', 'domain'])
+def test_create_letter_branding_shows_database_errors_on_name_fields(
-def test_create_letter_branding_shows_database_errors_on_name_and_domain_fields(
     mocker,
     logged_in_platform_admin_client,
     fake_uuid,
-    error_field
 ):
     with logged_in_platform_admin_client.session_transaction() as session:
         user_id = session["user_id"]
@@ -576,13 +524,13 @@ def test_create_letter_branding_shows_database_errors_on_name_and_domain_fields(
             json={
                 'result': 'error',
                 'message': {
-                    error_field: {
+                    'name': {
-                        '{} already in use'.format(error_field)
+                        'name already in use'
                     }
                 }
             }
         ),
-        message={error_field: ['{} already in use'.format(error_field)]}
+        message={'name': ['name already in use']}
     ))
 
     temp_logo = LETTER_TEMP_LOGO_LOCATION.format(user_id=user_id, unique_id=fake_uuid, filename='test.svg')
@@ -591,7 +539,6 @@ def test_create_letter_branding_shows_database_errors_on_name_and_domain_fields(
         url_for('.create_letter_branding', logo=temp_logo),
         data={
             'name': 'my brand',
-            'domain': None,
             'operation': 'branding-details'
         }
     )
@@ -600,7 +547,7 @@ def test_create_letter_branding_shows_database_errors_on_name_and_domain_fields(
     error_message = page.find('span', class_='error-message').text.strip()
 
     assert page.find('h1').text == 'Add letter branding'
-    assert error_message == '{} already in use'.format(error_field)
+    assert error_message == 'name already in use'
 
 
 def test_get_png_file_from_svg(client, mocker, fake_uuid):

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',
+        'app.models.organisation.Organisation.agreement_signed',
-        side_effect=_agreement_info,
+        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())
+    mock_get_service_organisation(
-    user.email_address = email_address
+        mocker,
-    client_request.login(user)
+        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',
@@ -1546,10 +1555,9 @@ def test_ready_to_go_live(
         'id': SERVICE_ONE_ID
     }).go_live_checklist_completed_as_yes_no == expected_readyness
 
-    assert list(app.main.views.service_settings._get_request_to_go_live_tags(
+    assert app.models.service.Service(
-        app.models.service.Service({'id': SERVICE_ONE_ID}),
+        {'id': SERVICE_ONE_ID}
-        agreement_signed,
+    ).request_to_go_live_tags == expected_tags
-    )) == expected_tags
 
 
 @pytest.mark.parametrize('route', [
@@ -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/app/notify_client/test_email_branding_client.py

@@ -53,13 +53,13 @@ def test_get_all_email_branding(mocker):
 
 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'}
+                'brand_type': 'org'}
 
     mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post')
     mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     EmailBrandingClient().create_email_branding(
         logo=org_data['logo'], name=org_data['name'], text=org_data['text'], colour=org_data['colour'],
-        domain=org_data['domain'], brand_type='org'
+        brand_type='org'
     )
 
     mock_post.assert_called_once_with(
@@ -72,13 +72,13 @@ def test_create_email_branding(mocker):
 
 def test_update_email_branding(mocker, fake_uuid):
     org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red',
-                'domain': 'sample.com', 'brand_type': 'org'}
+                'brand_type': 'org'}
 
     mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post')
     mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     EmailBrandingClient().update_email_branding(
         branding_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], text=org_data['text'],
-        colour=org_data['colour'], domain=org_data['domain'], brand_type='org')
+        colour=org_data['colour'], brand_type='org')
 
     mock_post.assert_called_once_with(
         url='/email-branding/{}'.format(fake_uuid),

tests/app/notify_client/test_letter_branding_client.py

@@ -39,13 +39,13 @@ def test_get_all_letter_branding(mocker):
 
 
 def test_create_letter_branding(mocker):
-    new_branding = {'filename': 'uuid-test', 'name': 'my letters', 'domain': 'example.com'}
+    new_branding = {'filename': 'uuid-test', 'name': 'my letters'}
 
     mock_post = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.post')
     mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
 
     LetterBrandingClient().create_letter_branding(
-        filename=new_branding['filename'], name=new_branding['name'], domain=new_branding['domain']
+        filename=new_branding['filename'], name=new_branding['name'],
     )
     mock_post.assert_called_once_with(
         url='/letter-branding',
@@ -56,12 +56,12 @@ def test_create_letter_branding(mocker):
 
 
 def test_update_letter_branding(mocker, fake_uuid):
-    branding = {'filename': 'uuid-test', 'name': 'my letters', 'domain': 'example.com'}
+    branding = {'filename': 'uuid-test', 'name': 'my letters'}
 
     mock_post = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.post')
     mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
     LetterBrandingClient().update_letter_branding(
-        branding_id=fake_uuid, filename=branding['filename'], name=branding['name'], domain=branding['domain'])
+        branding_id=fake_uuid, filename=branding['filename'], name=branding['name'])
 
     mock_post.assert_called_once_with(
         url='/letter-branding/{}'.format(fake_uuid),

tests/app/test_utils.py

@@ -1,16 +1,13 @@
-from collections import Counter, OrderedDict
+from collections import OrderedDict
 from csv import DictReader
 from io import StringIO
 from pathlib import Path
 
 import pytest
 from freezegun import freeze_time
-from notifications_utils.recipients import validate_email_address
 
 from app import format_datetime_relative
 from app.utils import (
-    AgreementInfo,
-    GovernmentEmailDomain,
     Spreadsheet,
     email_safe,
     generate_next_dict,
@@ -292,159 +289,6 @@ def test_get_cdn_domain_on_non_localhost(client, mocker):
     assert domain == 'static-logos.admintest.com'
 
 
-@pytest.mark.parametrize("domain_or_email_address", (
-    "test@dclgdatamart.co.uk", "test@communities.gsi.gov.uk", "test@communities.gov.uk",
-))
-def test_get_valid_agreement_info_known_details(domain_or_email_address):
-    agreement_info = AgreementInfo(domain_or_email_address)
-    assert agreement_info.crown_status is None
-    assert agreement_info.owner == "Ministry of Housing, Communities & Local Government"
-    assert agreement_info.agreement_signed is True
-    assert agreement_info.as_human_readable == (
-        'Yes, on behalf of Ministry of Housing, Communities & Local Government'
-    )
-
-
-@pytest.mark.parametrize("domain_or_email_address", (
-    "test@dwp.gov.uk", "test@dwp.gsi.gov.uk",
-))
-def test_dwp_go_live_requests_are_flagged(domain_or_email_address):
-    agreement_info = AgreementInfo(domain_or_email_address)
-    assert agreement_info.owner == "Department for Work and Pensions"
-    assert agreement_info.agreement_signed is True
-    assert agreement_info.as_human_readable == (
-        'DWP - Requires OED approval'
-    )
-
-
-@pytest.mark.parametrize("domain_or_email_address, is_canonical", (
-    ("test@dclgdatamart.co.uk", False),
-    ("test@communities.gsi.gov.uk", False),
-    ("test@communities.gov.uk", True),
-))
-def test_get_canonical_domain(domain_or_email_address, is_canonical):
-    assert AgreementInfo(domain_or_email_address).canonical_domain == 'communities.gov.uk'
-    assert AgreementInfo(domain_or_email_address).is_canonical == is_canonical
-
-
-def test_get_canonical_domain_passes_through_unknown_domain():
-    assert AgreementInfo('example.com').canonical_domain is None
-    assert AgreementInfo('example.com').is_canonical is False
-
-
-@pytest.mark.parametrize("domain_or_email_address", (
-    "test@police.gov.uk", "police.gov.uk",
-))
-def test_get_valid_agreement_info_unknown_details(domain_or_email_address):
-    government_domain = AgreementInfo(domain_or_email_address)
-    assert government_domain.crown_status is None
-    assert government_domain.owner is None
-    assert government_domain.agreement_signed is None
-    assert government_domain.as_human_readable == 'Can’t tell (domain is police.gov.uk)'
-
-
-def test_get_valid_agreement_info_only_org_known():
-    agreement_info = AgreementInfo('nhs.net')
-    # Some parts of the NHS are Crown, some aren’t
-    assert agreement_info.crown_status is None
-    assert agreement_info.owner == 'NHS'
-    assert agreement_info.agreement_signed is None
-    assert agreement_info.as_human_readable == 'Can’t tell (organisation is NHS, crown status unknown)'
-
-
-def test_get_valid_agreement_info_some_known_details():
-    agreement_info = AgreementInfo("marinemanagement.org.uk")
-    assert agreement_info.crown_status is None
-    assert agreement_info.owner == "Marine Management Organisation"
-    assert agreement_info.agreement_signed is True
-    assert agreement_info.as_human_readable == (
-        'Yes, on behalf of Marine Management Organisation'
-    )
-
-
-def test_get_valid_local_agreement_info_some_known_details():
-    # This example may need to be updated to use a different council if
-    # Babergh every sign the agreement
-    agreement_info = AgreementInfo("babergh.gov.uk")
-    assert agreement_info.crown_status is False
-    assert agreement_info.owner == "Babergh District Council"
-    assert agreement_info.agreement_signed is False
-    assert agreement_info.as_human_readable == (
-        'No (organisation is Babergh District Council, a non-crown body)'
-    )
-
-
-def test_get_valid_government_domain_gets_most_specific_first():
-
-    generic = AgreementInfo("gov.uk")
-    assert generic.crown_status is None
-    assert generic.owner is None
-    assert generic.agreement_signed is None
-    assert generic.as_human_readable == (
-        'Can’t tell (domain is gov.uk)'
-    )
-
-    specific = AgreementInfo("dacorum.gov.uk")
-    assert specific.crown_status is False
-    assert specific.owner == 'Dacorum Borough Council'
-    assert specific.agreement_signed is True
-    assert specific.as_human_readable == (
-        'Yes, on behalf of Dacorum Borough Council'
-    )
-
-
-def test_get_domain_info_for_branding_request():
-
-    assert AgreementInfo("gov.uk").as_info_for_branding_request == (
-        'Can’t tell (domain is gov.uk)'
-    )
-    assert AgreementInfo("dacorum.gov.uk").as_info_for_branding_request == (
-        'Dacorum Borough Council'
-    )
-
-
-def test_domains_are_lowercased():
-    for domain in AgreementInfo.domains.keys():
-        assert domain == domain.lower()
-
-
-def test_validate_government_domain_data():
-
-    for domain in AgreementInfo.domains.keys():
-
-        validate_email_address('test@{}'.format(domain))
-
-        agreement_info = AgreementInfo(domain)
-
-        assert agreement_info.crown_status in {
-            True, False, None
-        }
-
-        assert isinstance(agreement_info.owner, str) and agreement_info.owner.strip()
-
-        assert agreement_info.agreement_signed in {
-            True, False, None
-        }
-
-
-def test_domain_data_is_canonicalized():
-    for owner, count in Counter(
-        AgreementInfo(domain).owner
-        for domain in AgreementInfo.domains.keys()
-        if AgreementInfo(domain).is_canonical
-    ).most_common():
-        if count > 1:
-            raise ValueError(
-                '{} entries in domains.yml for {}'.format(count, owner)
-            )
-
-
-def test_validate_email_domain_data():
-
-    for domain in GovernmentEmailDomain.domains.keys():
-        validate_email_address('test@{}'.format(domain))
-
-
 @pytest.mark.parametrize('time, human_readable_datetime', [
     ('2018-03-14 09:00', '14 March at 9:00am'),
     ('2018-03-14 15:00', '14 March at 3:00pm'),

tests/conftest.py

@@ -625,7 +625,6 @@ def mock_create_service(mocker):
         restricted,
         user_id,
         email_from,
-        service_domain,
     ):
         service = service_json(
             101, service_name, [user_id], message_limit=message_limit, restricted=restricted, email_from=email_from)
@@ -644,7 +643,6 @@ def mock_create_duplicate_service(mocker):
         restricted,
         user_id,
         email_from,
-        service_domain,
     ):
         json_mock = Mock(return_value={'message': {'name': ["Duplicate service name '{}'".format(service_name)]}})
         resp_mock = Mock(status_code=400, json=json_mock)
@@ -2541,8 +2539,8 @@ def mock_get_all_email_branding(mocker):
         non_standard_values = [
             {'idx': 1, 'colour': 'red'},
             {'idx': 2, 'colour': 'orange'},
-            {'idx': 3, 'text': None, 'domain': 'nhs.uk'},
+            {'idx': 3, 'text': None},
-            {'idx': 4, 'colour': 'blue', 'domain': 'voa.gov.uk'},
+            {'idx': 4, 'colour': 'blue'},
         ]
         shuffle = sort_key is None
         return create_email_brandings(5, non_standard_values=non_standard_values, shuffle=shuffle)
@@ -2561,19 +2559,16 @@ def mock_get_all_letter_branding(mocker):
                 'id': str(UUID(int=0)),
                 'name': 'HM Government',
                 'filename': 'hm-government',
-                'domain': None,
             },
             {
                 'id': str(UUID(int=1)),
                 'name': 'Land Registry',
                 'filename': 'land-registry',
-                'domain': 'landregistry.gov.uk',
             },
             {
                 'id': str(UUID(int=2)),
                 'name': 'Animal and Plant Health Agency',
                 'filename': 'animal',
-                'domain': None,
             }
         ]
 
@@ -2589,7 +2584,6 @@ def mock_get_letter_branding_by_id(mocker):
             'id': _id,
             'name': 'HM Government',
             'filename': 'hm-government',
-            'domain': 'cabinet-office.gov.uk',
         }
     return mocker.patch(
         'app.letter_branding_client.get_letter_branding', side_effect=_get_branding_by_id
@@ -2613,7 +2607,6 @@ def create_email_branding(id, non_standard_values={}):
         'text': 'Organisation text',
         'id': id,
         'colour': '#f00',
-        'domain': 'sample.com',
         'brand_type': 'org',
     }
 
@@ -2676,7 +2669,7 @@ def mock_get_email_branding_without_brand_text(mocker, fake_uuid):
 
 @pytest.fixture(scope='function')
 def mock_create_email_branding(mocker):
-    def _create_email_branding(logo, name, text, colour, domain, brand_type):
+    def _create_email_branding(logo, name, text, colour, brand_type):
         return
 
     return mocker.patch(
@@ -2686,7 +2679,7 @@ def mock_create_email_branding(mocker):
 
 @pytest.fixture(scope='function')
 def mock_update_email_branding(mocker):
-    def _update_email_branding(branding_id, logo, name, text, colour, domain, brand_type):
+    def _update_email_branding(branding_id, logo, name, text, colour, brand_type):
         return
 
     return mocker.patch(
@@ -3126,9 +3119,40 @@ def mock_get_organisation(
 
 
 @pytest.fixture(scope='function')
-def mock_get_service_organisation(mocker):
+def mock_get_organisation_by_domain(
+    mocker,
+    name=False,
+    crown=True,
+    agreement_signed=False,
+):
+    def _get_organisation_by_domain(org_id):
+        return organisation_json(
+            org_id,
+            name,
+            crown=crown,
+            agreement_signed=agreement_signed,
+        )
+
+    return mocker.patch(
+        'app.organisations_client.get_organisation_by_domain',
+        side_effect=_get_organisation_by_domain,
+    )
+
+
+@pytest.fixture(scope='function')
+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)