mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-09-06 19:48:25 -04:00
Fix small issues identified in PR review
In response to: [^1], [^2], [^3], [^4], [^5] and [^6]. [^1]: https://github.com/alphagov/notifications-admin/pull/4182#discussion_r825824485 [^2]: https://github.com/alphagov/notifications-admin/pull/4182#discussion_r825824805 [^3]: https://github.com/alphagov/notifications-admin/pull/4182#discussion_r825857745 [^4]: https://github.com/alphagov/notifications-admin/pull/4182#discussion_r825859850 [^5]: https://github.com/alphagov/notifications-admin/pull/4182#discussion_r825859982 [^6]: https://github.com/alphagov/notifications-admin/pull/4182#discussion_r826001823
This commit is contained in:
@@ -2196,11 +2196,11 @@ class ChooseBrandingForm(StripWhitespaceForm):
|
|||||||
if self.something_else_is_only_option:
|
if self.something_else_is_only_option:
|
||||||
self.options.data = self.FALLBACK_OPTION_VALUE
|
self.options.data = self.FALLBACK_OPTION_VALUE
|
||||||
|
|
||||||
@staticmethod
|
@classmethod
|
||||||
def get_available_choices(service, branding_type):
|
def get_available_choices(cls, service, branding_type):
|
||||||
return (
|
return (
|
||||||
list(branding.get_available_choices(service, branding_type)) +
|
list(branding.get_available_choices(service, branding_type)) +
|
||||||
[ChooseBrandingForm.FALLBACK_OPTION]
|
[cls.FALLBACK_OPTION]
|
||||||
)
|
)
|
||||||
|
|
||||||
@property
|
@property
|
||||||
@@ -2221,12 +2221,12 @@ class ChooseBrandingForm(StripWhitespaceForm):
|
|||||||
|
|
||||||
class ChooseEmailBrandingForm(ChooseBrandingForm):
|
class ChooseEmailBrandingForm(ChooseBrandingForm):
|
||||||
def __init__(self, service_id):
|
def __init__(self, service_id):
|
||||||
ChooseBrandingForm.__init__(self, service_id, branding_type='email')
|
super().__init__(service_id, branding_type='email')
|
||||||
|
|
||||||
|
|
||||||
class ChooseLetterBrandingForm(ChooseBrandingForm):
|
class ChooseLetterBrandingForm(ChooseBrandingForm):
|
||||||
def __init__(self, service_id):
|
def __init__(self, service_id):
|
||||||
ChooseBrandingForm.__init__(self, service_id, branding_type='letter')
|
super().__init__(service_id, branding_type='letter')
|
||||||
|
|
||||||
|
|
||||||
class SomethingElseBrandingForm(StripWhitespaceForm):
|
class SomethingElseBrandingForm(StripWhitespaceForm):
|
||||||
|
|||||||
@@ -8,20 +8,19 @@ from tests import organisation_json
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('branding_type', ['email', 'letter'])
|
@pytest.mark.parametrize('branding_type', ['email', 'letter'])
|
||||||
@pytest.mark.parametrize('org_type, existing_branding, expected_options', [
|
@pytest.mark.parametrize('org_type, expected_options', [
|
||||||
('central', None, []),
|
('central', []),
|
||||||
('local', None, []),
|
('local', []),
|
||||||
('nhs_central', None, [('nhs', 'NHS')]),
|
('nhs_central', [('nhs', 'NHS')]),
|
||||||
('nhs_local', None, [('nhs', 'NHS')]),
|
('nhs_local', [('nhs', 'NHS')]),
|
||||||
('nhs_gp', None, [('nhs', 'NHS')]),
|
('nhs_gp', [('nhs', 'NHS')]),
|
||||||
('emergency_service', None, []),
|
('emergency_service', []),
|
||||||
('other', None, []),
|
('other', []),
|
||||||
])
|
])
|
||||||
def test_get_available_choices_no_org(
|
def test_get_available_choices_service_not_assigned_to_org(
|
||||||
service_one,
|
service_one,
|
||||||
branding_type,
|
branding_type,
|
||||||
org_type,
|
org_type,
|
||||||
existing_branding,
|
|
||||||
expected_options,
|
expected_options,
|
||||||
):
|
):
|
||||||
service_one['organisation_type'] = org_type
|
service_one['organisation_type'] = org_type
|
||||||
@@ -32,20 +31,19 @@ def test_get_available_choices_no_org(
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize('branding_type', ['email', 'letter'])
|
@pytest.mark.parametrize('branding_type', ['email', 'letter'])
|
||||||
@pytest.mark.parametrize('org_type, existing_branding, expected_options', [
|
@pytest.mark.parametrize('org_type, expected_options', [
|
||||||
('local', None, [('organisation', 'Test Organisation')]),
|
('local', [('organisation', 'Test Organisation')]),
|
||||||
('nhs_central', None, [('nhs', 'NHS')]),
|
('nhs_central', [('nhs', 'NHS')]),
|
||||||
('nhs_local', None, [('nhs', 'NHS')]),
|
('nhs_local', [('nhs', 'NHS')]),
|
||||||
('nhs_gp', None, [('nhs', 'NHS')]),
|
('nhs_gp', [('nhs', 'NHS')]),
|
||||||
('emergency_service', None, [('organisation', 'Test Organisation')]),
|
('emergency_service', [('organisation', 'Test Organisation')]),
|
||||||
('other', None, [('organisation', 'Test Organisation')]),
|
('other', [('organisation', 'Test Organisation')]),
|
||||||
])
|
])
|
||||||
def test_get_available_choices_with_org(
|
def test_get_available_choices_service_assigned_to_org(
|
||||||
mocker,
|
mocker,
|
||||||
service_one,
|
service_one,
|
||||||
branding_type,
|
branding_type,
|
||||||
org_type,
|
org_type,
|
||||||
existing_branding,
|
|
||||||
expected_options,
|
expected_options,
|
||||||
mock_get_service_organisation,
|
mock_get_service_organisation,
|
||||||
):
|
):
|
||||||
@@ -125,6 +123,11 @@ def test_get_available_choices_letter_branding_set(
|
|||||||
'app.organisations_client.get_organisation',
|
'app.organisations_client.get_organisation',
|
||||||
return_value=organisation_json()
|
return_value=organisation_json()
|
||||||
)
|
)
|
||||||
|
mocker.patch(
|
||||||
|
'app.models.service.Service.letter_branding_id',
|
||||||
|
new_callable=PropertyMock,
|
||||||
|
return_value='1234-abcd',
|
||||||
|
)
|
||||||
|
|
||||||
options = get_available_choices(service, branding_type='letter')
|
options = get_available_choices(service, branding_type='letter')
|
||||||
assert list(options) == [
|
assert list(options) == [
|
||||||
|
|||||||
Reference in New Issue
Block a user