From 8a7eed9b269e8b5517022aa94a948f5145d9a616 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Tue, 22 Mar 2022 17:29:39 +0000 Subject: [PATCH] Minor refactorings to help read branding rules I did consider having a "has_govuk_branding" variable, but I think it's better to reinforce what this means with comments. --- app/utils/branding.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/app/utils/branding.py b/app/utils/branding.py index 121366e2e..7ecf8ef95 100644 --- a/app/utils/branding.py +++ b/app/utils/branding.py @@ -6,26 +6,25 @@ NHS_EMAIL_BRANDING_ID = 'a7dc4e56-660b-4db7-8cff-12c37b12b5ea' def get_email_choices(service): organisation_branding_id = service.organisation.email_branding_id if service.organisation else None - service_branding_id = service.email_branding_id if ( service.organisation_type == Organisation.TYPE_CENTRAL - and organisation_branding_id is None - and service_branding_id is not None + and service.email_branding_id is not None # GOV.UK is not current branding + and organisation_branding_id is None # no default to supersede it (GOV.UK) ): yield ('govuk', 'GOV.UK') if ( service.organisation_type == Organisation.TYPE_CENTRAL and service.organisation - and organisation_branding_id is None - and service.email_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower() + and organisation_branding_id is None # don't offer both if org has default + and service.email_branding_name.lower() != f'GOV.UK and {service.organisation.name}'.lower() ): - yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name)) + yield ('govuk_and_org', f'GOV.UK and {service.organisation.name}') if ( service.organisation_type in NHS_TYPES - and service_branding_id != NHS_EMAIL_BRANDING_ID + and service.email_branding_id != NHS_EMAIL_BRANDING_ID ): yield ('nhs', 'NHS') @@ -33,8 +32,8 @@ def get_email_choices(service): service.organisation and service.organisation_type not in NHS_TYPES and ( - service_branding_id is None - or service_branding_id != organisation_branding_id + service.email_branding_id is None # GOV.UK is current branding + or service.email_branding_id != organisation_branding_id ) ): yield ('organisation', service.organisation.name) @@ -42,7 +41,6 @@ def get_email_choices(service): def get_letter_choices(service): organisation_branding_id = service.organisation.letter_branding_id if service.organisation else None - service_branding_id = service.letter_branding_id if ( service.organisation_type in NHS_TYPES @@ -54,8 +52,8 @@ def get_letter_choices(service): service.organisation and service.organisation_type not in NHS_TYPES and ( - service_branding_id is None - or service_branding_id != organisation_branding_id + service.letter_branding_id is None # GOV.UK is current branding + or service.letter_branding_id != organisation_branding_id ) ): yield ('organisation', service.organisation.name)