Use constants for organisation type

This reduces the chances of making a typo, because doing so will raise
an exception rather than fail silently.
This commit is contained in:
Chris Hill-Scott
2019-09-16 11:33:50 +01:00
parent 077dc194c6
commit 20f857753a
5 changed files with 41 additions and 22 deletions
+12 -6
View File
@@ -1375,14 +1375,14 @@ class BrandingOptionsEmail(StripWhitespaceForm):
def get_available_choices(service): def get_available_choices(service):
if ( if (
service.organisation_type == 'central' and service.organisation_type == Organisation.TYPE_CENTRAL and
service.organisation.email_branding_id is None and service.organisation.email_branding_id is None and
service.email_branding_id is not None service.email_branding_id is not None
): ):
yield ('govuk', 'GOV.UK') yield ('govuk', 'GOV.UK')
if ( if (
service.organisation_type == 'central' and service.organisation_type == Organisation.TYPE_CENTRAL and
service.organisation and service.organisation and
service.organisation.email_branding_id is None and service.organisation.email_branding_id is None and
service.email_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower() service.email_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower()
@@ -1390,15 +1390,21 @@ class BrandingOptionsEmail(StripWhitespaceForm):
yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name)) yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name))
if ( if (
service.organisation_type in {'nhs_local', 'nhs_central', 'nhs_gp'} and service.organisation_type in {
service.email_branding_name != 'NHS' Organisation.TYPE_NHS_CENTRAL,
Organisation.TYPE_NHS_LOCAL,
Organisation.TYPE_NHS_GP,
} and service.email_branding_name != 'NHS'
): ):
yield ('nhs', 'NHS') yield ('nhs', 'NHS')
if ( if (
service.organisation and service.organisation and
service.organisation_type not in {'nhs_local', 'nhs_central', 'nhs_gp'} and service.organisation_type not in {
( Organisation.TYPE_NHS_LOCAL,
Organisation.TYPE_NHS_CENTRAL,
Organisation.TYPE_NHS_GP,
} and (
service.email_branding_id is None or service.email_branding_id is None or
service.email_branding_id != service.organisation.email_branding_id service.email_branding_id != service.organisation.email_branding_id
) )
+3 -2
View File
@@ -6,6 +6,7 @@ from flask_login import current_user
from app import current_service from app import current_service
from app.main import main from app.main import main
from app.main.forms import AcceptAgreementForm from app.main.forms import AcceptAgreementForm
from app.models.organisation import Organisation
from app.s3_client.s3_mou_client import get_mou from app.s3_client.s3_mou_client import get_mou
from app.utils import user_has_permissions from app.utils import user_has_permissions
@@ -14,11 +15,11 @@ from app.utils import user_has_permissions
@user_has_permissions('manage_service') @user_has_permissions('manage_service')
def service_agreement(service_id): def service_agreement(service_id):
if not current_service.organisation: if not current_service.organisation:
if current_service.organisation_type == 'nhs_gp': if current_service.organisation_type == Organisation.TYPE_NHS_GP:
return redirect( return redirect(
url_for('main.add_organisation_from_gp_service', service_id=current_service.id) url_for('main.add_organisation_from_gp_service', service_id=current_service.id)
) )
if current_service.organisation_type == 'nhs_local': if current_service.organisation_type == Organisation.TYPE_NHS_LOCAL:
return redirect( return redirect(
url_for('main.add_organisation_from_nhs_local_service', service_id=current_service.id) url_for('main.add_organisation_from_nhs_local_service', service_id=current_service.id)
) )
+3 -3
View File
@@ -69,7 +69,7 @@ def add_organisation():
@main.route('/services/<uuid:service_id>/add-gp-organisation', methods=['GET', 'POST']) @main.route('/services/<uuid:service_id>/add-gp-organisation', methods=['GET', 'POST'])
@user_has_permissions('manage_service') @user_has_permissions('manage_service')
def add_organisation_from_gp_service(service_id): def add_organisation_from_gp_service(service_id):
if (not current_service.organisation_type == 'nhs_gp') or current_service.organisation: if (not current_service.organisation_type == Organisation.TYPE_NHS_GP) or current_service.organisation:
abort(403) abort(403)
form = AddGPOrganisationForm(service_name=current_service.name) form = AddGPOrganisationForm(service_name=current_service.name)
@@ -97,13 +97,13 @@ def add_organisation_from_gp_service(service_id):
@main.route('/services/<uuid:service_id>/add-nhs-local-organisation', methods=['GET', 'POST']) @main.route('/services/<uuid:service_id>/add-nhs-local-organisation', methods=['GET', 'POST'])
@user_has_permissions('manage_service') @user_has_permissions('manage_service')
def add_organisation_from_nhs_local_service(service_id): def add_organisation_from_nhs_local_service(service_id):
if (not current_service.organisation_type == 'nhs_local') or current_service.organisation: if (not current_service.organisation_type == Organisation.TYPE_NHS_LOCAL) or current_service.organisation:
abort(403) abort(403)
form = AddNHSLocalOrganisationForm(organisation_choices=[ form = AddNHSLocalOrganisationForm(organisation_choices=[
(organisation.id, organisation.name) (organisation.id, organisation.name)
for organisation in Organisations() for organisation in Organisations()
if organisation.organisation_type == 'nhs_local' if organisation.organisation_type == Organisation.TYPE_NHS_LOCAL
]) ])
search_form = SearchByNameForm() search_form = SearchByNameForm()
+17 -8
View File
@@ -9,15 +9,24 @@ from app.notify_client.organisations_api_client import organisations_client
class Organisation(JSONModel): class Organisation(JSONModel):
TYPE_CENTRAL = 'central'
TYPE_LOCAL = 'local'
TYPE_NHS_CENTRAL = 'nhs_central'
TYPE_NHS_LOCAL = 'nhs_local'
TYPE_NHS_GP = 'nhs_gp'
TYPE_EMERGENCY_SERVICE = 'emergency_service'
TYPE_SCHOOL_OR_COLLEGE = 'school_or_college'
TYPE_OTHER = 'other'
TYPES = ( TYPES = (
('central', 'Central government'), (TYPE_CENTRAL, 'Central government'),
('local', 'Local government'), (TYPE_LOCAL, 'Local government'),
('nhs_central', 'NHS central government agency or public body'), (TYPE_NHS_CENTRAL, 'NHS central government agency or public body'),
('nhs_local', 'NHS Trust or Clinical Commissioning Group'), (TYPE_NHS_LOCAL, 'NHS Trust or Clinical Commissioning Group'),
('nhs_gp', 'GP practice'), (TYPE_NHS_GP, 'GP practice'),
('emergency_service', 'Emergency service'), (TYPE_EMERGENCY_SERVICE, 'Emergency service'),
('school_or_college', 'School or college'), (TYPE_SCHOOL_OR_COLLEGE, 'School or college'),
('other', 'Other'), (TYPE_OTHER, 'Other'),
) )
ALLOWED_PROPERTIES = { ALLOWED_PROPERTIES = {
+6 -3
View File
@@ -270,7 +270,7 @@ class Service(JSONModel):
@property @property
def shouldnt_use_govuk_as_sms_sender(self): def shouldnt_use_govuk_as_sms_sender(self):
return self.organisation_type != 'central' return self.organisation_type != Organisation.TYPE_CENTRAL
@cached_property @cached_property
def sms_senders(self): def sms_senders(self):
@@ -419,7 +419,7 @@ class Service(JSONModel):
@property @property
def needs_to_change_email_branding(self): def needs_to_change_email_branding(self):
return self.email_branding_id is None and self.organisation_type != 'central' return self.email_branding_id is None and self.organisation_type != Organisation.TYPE_CENTRAL
@property @property
def letter_branding_id(self): def letter_branding_id(self):
@@ -599,7 +599,10 @@ class Service(JSONModel):
def able_to_accept_agreement(self): def able_to_accept_agreement(self):
return ( return (
self.organisation.agreement_signed is not None self.organisation.agreement_signed is not None
or self.organisation_type in {'nhs_gp', 'nhs_local'} or self.organisation_type in {
Organisation.TYPE_NHS_GP,
Organisation.TYPE_NHS_LOCAL,
}
) )
@property @property