From 936883bf7bd74a1c3f0892344973de18c14a747c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 19 Feb 2019 17:26:16 +0000 Subject: [PATCH] =?UTF-8?q?Allow=20editing=20of=20an=20organisation?= =?UTF-8?q?=E2=80=99s=20details?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a user interface for updating all the columns added in https://github.com/alphagov/notifications-api/pull/2368 Sorry for the mega commit 😓 --- app/main/forms.py | 66 +++- app/main/views/organisations.py | 234 ++++++++++++ app/main/views/service_settings.py | 14 +- app/navigation.py | 37 ++ app/notify_client/organisations_api_client.py | 8 +- .../organisation/settings/edit-agreement.html | 28 ++ .../settings/edit-crown-status.html | 21 ++ .../organisation/settings/edit-domains.html | 31 ++ .../organisation/settings/edit-type.html | 21 ++ .../organisation/settings/index.html | 90 ++++- .../settings/preview-email-branding.html | 23 ++ .../settings/preview-letter-branding.html | 22 ++ .../settings/set-email-branding.html | 30 ++ .../settings/set-letter-branding.html | 30 ++ .../preview-email-branding.html | 5 +- tests/__init__.py | 15 +- .../test_organisation_invites.py | 311 +++++++++++++++- tests/app/main/views/test_service_settings.py | 338 ++++++++++++++---- tests/conftest.py | 49 ++- 19 files changed, 1247 insertions(+), 126 deletions(-) create mode 100644 app/templates/views/organisations/organisation/settings/edit-agreement.html create mode 100644 app/templates/views/organisations/organisation/settings/edit-crown-status.html create mode 100644 app/templates/views/organisations/organisation/settings/edit-domains.html create mode 100644 app/templates/views/organisations/organisation/settings/edit-type.html create mode 100644 app/templates/views/organisations/organisation/settings/preview-email-branding.html create mode 100644 app/templates/views/organisations/organisation/settings/preview-letter-branding.html create mode 100644 app/templates/views/organisations/organisation/settings/set-email-branding.html create mode 100644 app/templates/views/organisations/organisation/settings/set-letter-branding.html diff --git a/app/main/forms.py b/app/main/forms.py index 701b4a89f..bd3267e4c 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -243,9 +243,9 @@ class ForgivingIntegerField(StringField): return super().__call__(value=value, **kwargs) -def organisation_type(): +def organisation_type(label='Who runs this service?'): return RadioField( - 'Who runs this service?', + label, choices=[ ('central', 'Central government'), ('local', 'Local government'), @@ -530,6 +530,62 @@ class RenameOrganisationForm(StripWhitespaceForm): ]) +class OrganisationOrganisationTypeForm(StripWhitespaceForm): + organisation_type = organisation_type(label='What type of organisation is this?') + + +class OrganisationCrownStatusForm(StripWhitespaceForm): + crown_status = RadioField( + ( + 'Is this organisation a crown body?' + ), + choices=[ + ('crown', 'Yes'), + ('non-crown', 'No'), + ('unknown', 'Not sure'), + ], + validators=[ + DataRequired(message='Can’t be empty') + ], + ) + + +class OrganisationAgreementSignedForm(StripWhitespaceForm): + agreement_signed = RadioField( + ( + 'Has this organisation signed the agreement?' + ), + choices=[ + ('yes', 'Yes'), + ('no', 'No'), + ('unknown', 'No (but we have some service-specific agreements in place)'), + ], + validators=[ + DataRequired(message='Can’t be empty') + ], + ) + + +class OrganisationDomainsForm(StripWhitespaceForm): + + def populate(self, domains_list): + for index, value in enumerate(domains_list): + self.domains[index].data = value + + domains = FieldList( + StripWhitespaceStringField( + '', + validators=[ + Optional(), + ], + default='' + ), + min_entries=10, + max_entries=10, + label="Domain names" + ) + + class CreateServiceForm(StripWhitespaceForm): name = StringField( u'What’s your service called?', @@ -895,7 +951,7 @@ class ServiceSwitchChannelForm(ServiceOnOffSettingForm): super().__init__(name, *args, **kwargs) -class ServiceSetEmailBranding(StripWhitespaceForm): +class SetEmailBranding(StripWhitespaceForm): branding_style = RadioFieldWithNoneOption( 'Branding style', @@ -920,12 +976,12 @@ class ServiceSetEmailBranding(StripWhitespaceForm): ) -class ServiceSetLetterBranding(ServiceSetEmailBranding): +class SetLetterBranding(SetEmailBranding): # form is the same, but instead of GOV.UK we have None as a valid option DEFAULT = (FieldWithNoneOption.NONE_OPTION_VALUE, 'None') -class ServicePreviewBranding(StripWhitespaceForm): +class PreviewBranding(StripWhitespaceForm): branding_style = HiddenFieldWithNoneOption('branding_style') diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index 4a0ba7065..288588e92 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -5,6 +5,8 @@ from werkzeug.exceptions import abort from app import ( current_organisation, + email_branding_client, + letter_branding_client, org_invite_api_client, organisations_client, user_api_client, @@ -14,9 +16,18 @@ from app.main.forms import ( ConfirmPasswordForm, CreateOrUpdateOrganisation, InviteOrgUserForm, + OrganisationAgreementSignedForm, + OrganisationCrownStatusForm, + OrganisationDomainsForm, + OrganisationOrganisationTypeForm, + PreviewBranding, RenameOrganisationForm, + SearchByNameForm, SearchUsersForm, + SetEmailBranding, + SetLetterBranding, ) +from app.main.views.service_settings import get_branding_as_value_and_label from app.utils import user_has_permissions, user_is_platform_admin @@ -165,8 +176,25 @@ def cancel_invited_org_user(org_id, invited_user_id): @login_required @user_has_permissions() def organisation_settings(org_id): + + email_branding = 'GOV.UK' + + if current_organisation['email_branding_id']: + email_branding = email_branding_client.get_email_branding( + current_organisation['email_branding_id'] + )['email_branding']['name'] + + letter_branding = None + + if current_organisation['letter_branding_id']: + letter_branding = letter_branding_client.get_letter_branding( + current_organisation['letter_branding_id'] + )['name'] + return render_template( 'views/organisations/organisation/settings/index.html', + email_branding=email_branding, + letter_branding=letter_branding, ) @@ -193,6 +221,212 @@ def edit_organisation_name(org_id): ) +@main.route("/organisations//settings/edit-type", methods=['GET', 'POST']) +@login_required +@user_has_permissions() +@user_is_platform_admin +def edit_organisation_type(org_id): + + form = OrganisationOrganisationTypeForm( + organisation_type=current_organisation['organisation_type'] + ) + + if form.validate_on_submit(): + organisations_client.update_organisation( + current_organisation['id'], + organisation_type=form.organisation_type.data, + ) + return redirect(url_for('.organisation_settings', org_id=org_id)) + + return render_template( + 'views/organisations/organisation/settings/edit-type.html', + form=form, + ) + + +@main.route("/organisations//settings/edit-crown-status", methods=['GET', 'POST']) +@login_required +@user_has_permissions() +@user_is_platform_admin +def edit_organisation_crown_status(org_id): + + form = OrganisationCrownStatusForm( + crown_status={ + True: 'crown', + False: 'non-crown', + None: 'unknown', + }.get(current_organisation['crown']) + ) + + if form.validate_on_submit(): + organisations_client.update_organisation( + current_organisation['id'], + crown={ + 'crown': True, + 'non-crown': False, + 'unknown': None, + }.get(form.crown_status.data), + ) + return redirect(url_for('.organisation_settings', org_id=org_id)) + + return render_template( + 'views/organisations/organisation/settings/edit-crown-status.html', + form=form, + ) + + +@main.route("/organisations//settings/edit-agreement", methods=['GET', 'POST']) +@login_required +@user_has_permissions() +@user_is_platform_admin +def edit_organisation_agreement(org_id): + + form = OrganisationAgreementSignedForm( + agreement_signed={ + True: 'yes', + False: 'no', + None: 'unknown', + }.get(current_organisation['agreement_signed']) + ) + + if form.validate_on_submit(): + organisations_client.update_organisation( + current_organisation['id'], + agreement_signed={ + 'yes': True, + 'no': False, + 'unknown': None, + }.get(form.agreement_signed.data), + ) + return redirect(url_for('.organisation_settings', org_id=org_id)) + + return render_template( + 'views/organisations/organisation/settings/edit-agreement.html', + form=form, + ) + + +@main.route("/organisations//settings/set-email-branding", methods=['GET', 'POST']) +@login_required +@user_has_permissions() +@user_is_platform_admin +def edit_organisation_email_branding(org_id): + + email_branding = email_branding_client.get_all_email_branding() + + form = SetEmailBranding( + all_branding_options=get_branding_as_value_and_label(email_branding), + current_branding=current_organisation['email_branding_id'], + ) + + if form.validate_on_submit(): + return redirect(url_for( + '.organisation_preview_email_branding', + org_id=org_id, + branding_style=form.branding_style.data, + )) + + return render_template( + 'views/organisations/organisation/settings/set-email-branding.html', + form=form, + search_form=SearchByNameForm() + ) + + +@main.route("/organisations//settings/preview-email-branding", methods=['GET', 'POST']) +@login_required +@user_has_permissions() +@user_is_platform_admin +def organisation_preview_email_branding(org_id): + + branding_style = request.args.get('branding_style', None) + + form = PreviewBranding(branding_style=branding_style) + + if form.validate_on_submit(): + organisations_client.update_organisation( + org_id, + email_branding_id=form.branding_style.data + ) + return redirect(url_for('.organisation_settings', org_id=org_id)) + + return render_template( + 'views/organisations/organisation/settings/preview-email-branding.html', + form=form, + action=url_for('main.organisation_preview_email_branding', org_id=org_id), + ) + + +@main.route("/organisations//settings/set-letter-branding", methods=['GET', 'POST']) +@login_required +@user_is_platform_admin +def edit_organisation_letter_branding(org_id): + letter_branding = letter_branding_client.get_all_letter_branding() + + form = SetLetterBranding( + all_branding_options=get_branding_as_value_and_label(letter_branding), + current_branding=current_organisation['letter_branding_id'], + ) + + if form.validate_on_submit(): + return redirect(url_for( + '.organisation_preview_letter_branding', + org_id=org_id, + branding_style=form.branding_style.data, + )) + + return render_template( + 'views/organisations/organisation/settings/set-letter-branding.html', + form=form, + search_form=SearchByNameForm() + ) + + +@main.route("/organisations//settings/preview-letter-branding", methods=['GET', 'POST']) +@login_required +@user_is_platform_admin +def organisation_preview_letter_branding(org_id): + branding_style = request.args.get('branding_style') + + form = PreviewBranding(branding_style=branding_style) + + if form.validate_on_submit(): + organisations_client.update_organisation( + org_id, + letter_branding_id=form.branding_style.data + ) + return redirect(url_for('.organisation_settings', org_id=org_id)) + + return render_template( + 'views/organisations/organisation/settings/preview-letter-branding.html', + form=form, + action=url_for('main.organisation_preview_letter_branding', org_id=org_id), + ) + + +@main.route("/organisations//settings/edit-organisation-domains", methods=['GET', 'POST']) +@login_required +@user_has_permissions() +@user_is_platform_admin +def edit_organisation_domains(org_id): + + form = OrganisationDomainsForm() + + if form.validate_on_submit(): + organisations_client.update_organisation( + org_id, + domains=list(filter(None, form.domains.data)), + ) + return redirect(url_for('.organisation_settings', org_id=org_id)) + + form.populate(current_organisation.get('domains', [])) + + return render_template( + 'views/organisations/organisation/settings/edit-domains.html', + form=form, + ) + + @main.route("/organisations//settings/edit-name/confirm", methods=['GET', 'POST']) @login_required @user_has_permissions() diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index f8b7822ae..ab8f17337 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -35,6 +35,7 @@ from app.main.forms import ( InternationalSMSForm, LinkOrganisationsForm, OrganisationTypeForm, + PreviewBranding, RenameServiceForm, SearchByNameForm, ServiceContactDetailsForm, @@ -44,12 +45,11 @@ from app.main.forms import ( ServiceInboundNumberForm, ServiceLetterContactBlockForm, ServiceOnOffSettingForm, - ServicePreviewBranding, ServiceReplyToEmailForm, - ServiceSetEmailBranding, - ServiceSetLetterBranding, ServiceSmsSenderForm, ServiceSwitchChannelForm, + SetEmailBranding, + SetLetterBranding, SMSPrefixForm, branding_options_dict, ) @@ -811,7 +811,7 @@ def set_free_sms_allowance(service_id): def service_set_email_branding(service_id): email_branding = email_branding_client.get_all_email_branding() - form = ServiceSetEmailBranding( + form = SetEmailBranding( all_branding_options=get_branding_as_value_and_label(email_branding), current_branding=current_service.email_branding_id, ) @@ -836,7 +836,7 @@ def service_set_email_branding(service_id): def service_preview_email_branding(service_id): branding_style = request.args.get('branding_style', None) - form = ServicePreviewBranding(branding_style=branding_style) + form = PreviewBranding(branding_style=branding_style) if form.validate_on_submit(): current_service.update( @@ -858,7 +858,7 @@ def service_preview_email_branding(service_id): def service_set_letter_branding(service_id): letter_branding = letter_branding_client.get_all_letter_branding() - form = ServiceSetLetterBranding( + form = SetLetterBranding( all_branding_options=get_branding_as_value_and_label(letter_branding), current_branding=current_service.letter_branding_id, ) @@ -883,7 +883,7 @@ def service_set_letter_branding(service_id): def service_preview_letter_branding(service_id): branding_style = request.args.get('branding_style') - form = ServicePreviewBranding(branding_style=branding_style) + form = PreviewBranding(branding_style=branding_style) if form.validate_on_submit(): current_service.update( diff --git a/app/navigation.py b/app/navigation.py index fd6f18711..2d6ce3a8c 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -154,7 +154,13 @@ class HeaderNavigation(Navigation): 'download_agreement', 'download_notifications_csv', 'edit_data_retention', + 'edit_organisation_agreement', + 'edit_organisation_crown_status', + 'edit_organisation_domains', + 'edit_organisation_email_branding', + 'edit_organisation_letter_branding', 'edit_organisation_name', + 'edit_organisation_type', 'edit_provider', 'edit_service_template', 'edit_template_postage', @@ -194,6 +200,8 @@ class HeaderNavigation(Navigation): 'old_using_notify', 'organisation_dashboard', 'organisation_settings', + 'organisation_preview_email_branding', + 'organisation_preview_letter_branding', 'privacy', 'public_agreement', 'public_download_agreement', @@ -428,7 +436,13 @@ class MainNavigation(Navigation): 'download_agreement', 'download_notifications_csv', 'edit_data_retention', + 'edit_organisation_agreement', + 'edit_organisation_crown_status', + 'edit_organisation_email_branding', + 'edit_organisation_domains', + 'edit_organisation_letter_branding', 'edit_organisation_name', + 'edit_organisation_type', 'edit_provider', 'edit_user_org_permissions', 'email_branding', @@ -462,6 +476,8 @@ class MainNavigation(Navigation): 'old_terms', 'old_using_notify', 'organisation_dashboard', + 'organisation_preview_email_branding', + 'organisation_preview_letter_branding', 'organisation_settings', 'organisations', 'platform_admin', @@ -590,6 +606,11 @@ class CaseworkNavigation(Navigation): 'choose_service', 'choose_template_to_copy', 'clear_cache', + 'edit_organisation_agreement', + 'edit_organisation_crown_status', + 'edit_organisation_domains', + 'edit_organisation_email_branding', + 'edit_organisation_letter_branding', 'confirm_edit_organisation_name', 'confirm_edit_user_email', 'confirm_edit_user_mobile_number', @@ -613,7 +634,11 @@ class CaseworkNavigation(Navigation): 'download_agreement', 'download_notifications_csv', 'edit_data_retention', + 'edit_organisation_agreement', + 'edit_organisation_crown_status', + 'edit_organisation_domains', 'edit_organisation_name', + 'edit_organisation_type', 'edit_provider', 'edit_service_template', 'edit_template_postage', @@ -659,6 +684,8 @@ class CaseworkNavigation(Navigation): 'old_terms', 'old_using_notify', 'organisation_dashboard', + 'organisation_preview_email_branding', + 'organisation_preview_letter_branding', 'organisation_settings', 'organisations', 'platform_admin', @@ -791,8 +818,18 @@ class OrgNavigation(Navigation): }, 'settings': { 'confirm_edit_organisation_name', + 'edit_organisation_agreement', + 'edit_organisation_crown_status', + 'edit_organisation_domains', + 'edit_organisation_email_branding', + 'edit_organisation_letter_branding', + 'edit_organisation_domains', 'edit_organisation_name', + 'edit_organisation_type', + 'organisation_preview_email_branding', + 'organisation_preview_letter_branding', 'organisation_settings', + }, 'team-members': { 'edit_user_org_permissions', diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index 60025856d..d1d082e82 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -15,11 +15,11 @@ class OrganisationsClient(NotifyAdminAPIClient): } return self.post(url="/organisations", data=data) + def update_organisation(self, org_id, **kwargs): + return self.post(url="/organisations/{}".format(org_id), data=kwargs) + def update_organisation_name(self, org_id, name): - data = { - "name": name - } - return self.post(url="/organisations/{}".format(org_id), data=data) + return self.update_organisation(org_id, name=name) def get_service_organisation(self, service_id): return self.get(url="/service/{}/organisation".format(service_id)) diff --git a/app/templates/views/organisations/organisation/settings/edit-agreement.html b/app/templates/views/organisations/organisation/settings/edit-agreement.html new file mode 100644 index 000000000..76f587cb9 --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/edit-agreement.html @@ -0,0 +1,28 @@ +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% extends "org_template.html" %} + +{% block org_page_title %} + Data sharing and financial agreement +{% endblock %} + +{% block maincolumn_content %} +

Data sharing and financial agreement

+
+
+ {% call form_wrapper() %} + {{ radios( + form.agreement_signed, + option_hints={ + 'yes': 'Users will be told their organisation has already signed the agreement', + 'no': 'Users will be prompted to sign the agreement before they can go live', + 'unknown': 'Users won’t be prompted to sign the agreement' + } + ) }} + {{ page_footer('Save') }} + {% endcall %} +
+
+{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/edit-crown-status.html b/app/templates/views/organisations/organisation/settings/edit-crown-status.html new file mode 100644 index 000000000..795cf12c3 --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/edit-crown-status.html @@ -0,0 +1,21 @@ +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% extends "org_template.html" %} + +{% block org_page_title %} + Crown organisation +{% endblock %} + +{% block maincolumn_content %} +

Crown organisation

+
+
+ {% call form_wrapper() %} + {{ radios(form.crown_status) }} + {{ page_footer('Save') }} + {% endcall %} +
+
+{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/edit-domains.html b/app/templates/views/organisations/organisation/settings/edit-domains.html new file mode 100644 index 000000000..295cd417d --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/edit-domains.html @@ -0,0 +1,31 @@ +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/list-entry.html" import list_entry %} +{% from "components/form.html" import form_wrapper %} + +{% extends "org_template.html" %} + +{% block org_page_title %} + Known email domains +{% endblock %} + +{% block maincolumn_content %} +

Known email domains

+
+
+

+ If a user’s email addresses ends with one of these domains then + any services they create will be associated with this organisation. +

+ {% call form_wrapper() %} + {{ list_entry( + form.domains, + item_name='domain', + autocomplete=False, + hint='For example cabinet-office.gov.uk' + ) }} + {{ page_footer('Save') }} + {% endcall %} +
+
+{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/edit-type.html b/app/templates/views/organisations/organisation/settings/edit-type.html new file mode 100644 index 000000000..f362e8565 --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/edit-type.html @@ -0,0 +1,21 @@ +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% extends "org_template.html" %} + +{% block org_page_title %} + Organisation type +{% endblock %} + +{% block maincolumn_content %} +

Organisation type

+
+
+ {% call form_wrapper() %} + {{ radios(form.organisation_type) }} + {{ page_footer('Save') }} + {% endcall %} +
+
+{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/index.html b/app/templates/views/organisations/organisation/settings/index.html index d8c08593a..929c56908 100644 --- a/app/templates/views/organisations/organisation/settings/index.html +++ b/app/templates/views/organisations/organisation/settings/index.html @@ -1,5 +1,5 @@ {% extends "org_template.html" %} -{% from "components/table.html" import mapping_table, row, text_field, edit_field with context %} +{% from "components/table.html" import mapping_table, optional_text_field, row, text_field, edit_field with context %} {% block org_page_title %} Organisation settings @@ -24,4 +24,92 @@ }} {% endcall %} {% endcall %} + + + + {% if current_user.platform_admin %} +

Platform admin settings

+
+ {% call mapping_table( + caption='Platform admin settings', + field_headings=['Label', 'Value', 'Action'], + field_headings_visible=False, + caption_visible=False + ) %} + {% call row() %} + {{ text_field('Organisation type') }} + {{ optional_text_field({ + 'central': 'Central government', + 'local': 'Local government', + 'nhs': 'NHS', + }.get(current_org.organisation_type)) }} + {{ edit_field( + 'Change', + url_for('.edit_organisation_type', org_id=current_org.id) + ) + }} + {% endcall %} + {% call row() %} + {{ text_field('Crown organisation') }} + {{ optional_text_field( + { + True: 'Yes', + False: 'No', + }.get(current_org.crown), + default='Not sure' + ) }} + {{ edit_field( + 'Change', + url_for('.edit_organisation_crown_status', org_id=current_org.id) + ) + }} + {% endcall %} + {% call row() %} + {{ text_field('Data sharing and financial agreement') }} + {{ text_field( + { + True: 'Signed', + False: 'Not signed', + None: 'Not signed (but we have some service-specific agreements in place)' + }.get(current_org.agreement_signed) + ) }} + {{ edit_field( + 'Change', + url_for('.edit_organisation_agreement', org_id=current_org.id) + ) + }} + {% endcall %} + {% call row() %} + {{ text_field('Default email branding') }} + {{ text_field(email_branding) }} + {{ edit_field( + 'Change', + url_for('.edit_organisation_email_branding', org_id=current_org.id) + ) + }} + {% endcall %} + {% call row() %} + {{ text_field('Default letter branding') }} + {{ optional_text_field( + letter_branding, + default='No branding' + ) }} + {{ edit_field( + 'Change', + url_for('.edit_organisation_letter_branding', org_id=current_org.id) + ) + }} + {% endcall %} + {% call row() %} + {{ text_field('Known email domains') }} + {{ optional_text_field(current_org.domains or None, default='None') }} + {{ edit_field( + 'Change', + url_for('.edit_organisation_domains', org_id=current_org.id) + ) + }} + {% endcall %} + {% endcall %} +
+ {% endif %} {% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/preview-email-branding.html b/app/templates/views/organisations/organisation/settings/preview-email-branding.html new file mode 100644 index 000000000..a03b06180 --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/preview-email-branding.html @@ -0,0 +1,23 @@ +{% extends "org_template.html" %} +{% from "components/form.html" import form_wrapper %} +{% block org_page_title %} + Preview email branding +{% endblock %} + +{% block maincolumn_content %} + +

Preview email branding

+
+
+ + {% call form_wrapper(action=action) %} +
+ {{ form.hidden_tag() }} +
+ +
+
+ {% endcall %} +
+
+{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/preview-letter-branding.html b/app/templates/views/organisations/organisation/settings/preview-letter-branding.html new file mode 100644 index 000000000..38e69f39b --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/preview-letter-branding.html @@ -0,0 +1,22 @@ +{% extends "org_template.html" %} +{% from "components/form.html" import form_wrapper %} +{% from "components/page-footer.html" import page_footer %} +{% block per_page_title %} + Preview letter branding +{% endblock %} + +{% block maincolumn_content %} + +

Preview letter branding

+
+
+ + {% call form_wrapper(action=action) %} +
+ {{ form.hidden_tag() }} + {{ page_footer('Save') }} +
+ {% endcall %} +
+
+{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/set-email-branding.html b/app/templates/views/organisations/organisation/settings/set-email-branding.html new file mode 100644 index 000000000..71034e51d --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/set-email-branding.html @@ -0,0 +1,30 @@ +{% extends "org_template.html" %} +{% from "components/radios.html" import radios %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/live-search.html" import live_search %} +{% from "components/form.html" import form_wrapper %} + +{% block org_page_title %} + Default email branding +{% endblock %} + +{% block maincolumn_content %} + +

Default email branding

+ {% call form_wrapper(data_kwargs={'preview-type': 'email'}) %} +
+
+
+
+
+
+ {{ live_search(target_selector='.multiple-choice', show=True, form=search_form, label='Search branding styles by name') }} + {{ radios(form.branding_style) }} +
+
+
+ {{ page_footer('Preview') }} +
+ {% endcall %} + +{% endblock %} diff --git a/app/templates/views/organisations/organisation/settings/set-letter-branding.html b/app/templates/views/organisations/organisation/settings/set-letter-branding.html new file mode 100644 index 000000000..d8c157798 --- /dev/null +++ b/app/templates/views/organisations/organisation/settings/set-letter-branding.html @@ -0,0 +1,30 @@ +{% extends "org_template.html" %} +{% from "components/radios.html" import radios %} +{% from "components/live-search.html" import live_search %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% block org_page_title %} + Default letter branding +{% endblock %} + +{% block maincolumn_content %} + +

Default letter branding

+ {% call form_wrapper(data_kwargs={'preview-type': 'letter'}) %} +
+
+
+
+
+
+ {{ live_search(target_selector='.multiple-choice', show=True, form=search_form, label='Search by name') }} + {{ radios(form.branding_style, hide_legend=True) }} +
+
+
+ {{ page_footer('Preview') }} +
+ {% endcall %} + +{% endblock %} diff --git a/app/templates/views/service-settings/preview-email-branding.html b/app/templates/views/service-settings/preview-email-branding.html index d966d0b03..6b38992b4 100644 --- a/app/templates/views/service-settings/preview-email-branding.html +++ b/app/templates/views/service-settings/preview-email-branding.html @@ -1,10 +1,10 @@ -{% extends "views/platform-admin/_base_template.html" %} +{% extends "withnav_template.html" %} {% from "components/form.html" import form_wrapper %} {% block service_page_title %} Preview email branding {% endblock %} -{% block platform_admin_content %} +{% block maincolumn_content %}

Preview email branding

@@ -15,7 +15,6 @@ {{ form.hidden_tag() }}
- Back to service settings
{% endcall %} diff --git a/tests/__init__.py b/tests/__init__.py index 30119aad9..9a6239e0d 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -186,7 +186,10 @@ def organisation_json( users=None, active=True, created_at=None, - services=None + services=None, + letter_branding_id=None, + email_branding_id=None, + domains=None, ): if users is None: users = [] @@ -198,7 +201,15 @@ def organisation_json( 'active': active, 'users': users, 'services': services, - 'created_at': created_at or str(datetime.utcnow()) + 'created_at': created_at or str(datetime.utcnow()), + 'email_branding_id': email_branding_id, + 'letter_branding_id': letter_branding_id, + 'organisation_type': '', + 'crown': True, + 'agreement_signed': False, + 'agreement_signed_at': None, + 'agreement_signed_by': None, + 'domains': domains or [], } diff --git a/tests/app/main/views/organisations/test_organisation_invites.py b/tests/app/main/views/organisations/test_organisation_invites.py index 8960acbf2..906a1f72d 100644 --- a/tests/app/main/views/organisations/test_organisation_invites.py +++ b/tests/app/main/views/organisations/test_organisation_invites.py @@ -7,7 +7,13 @@ from flask import url_for from notifications_python_client.errors import HTTPError from app.models.user import InvitedOrgUser -from tests.conftest import ORGANISATION_ID, normalize_spaces +from tests import organisation_json +from tests.conftest import ( + ORGANISATION_ID, + active_user_with_permissions, + normalize_spaces, + platform_admin_user, +) def test_organisation_page_shows_all_organisations( @@ -467,6 +473,26 @@ def test_verified_org_user_redirects_to_dashboard( def test_organisation_settings( + client_request, + mock_get_organisation, + organisation_one +): + expected_rows = [ + 'Label Value Action', + 'Organisation name Org 1 Change', + ] + + page = client_request.get('.organisation_settings', org_id=organisation_one['id']) + + assert page.find('h1').text == 'Organisation settings' + rows = page.select('tr') + assert len(rows) == len(expected_rows) + for index, row in enumerate(expected_rows): + assert row == " ".join(rows[index].text.split()) + mock_get_organisation.assert_called_with(organisation_one['id']) + + +def test_organisation_settings_for_platform_admin( logged_in_platform_admin_client, mock_get_organisation, organisation_one @@ -474,6 +500,14 @@ def test_organisation_settings( expected_rows = [ 'Label Value Action', 'Organisation name Org 1 Change', + + 'Label Value Action', + 'Organisation type Not set Change', + 'Crown organisation Yes Change', + 'Data sharing and financial agreement Signed Change', + 'Default email branding Not set Change', + 'Default letter branding Not set Change', + 'Known email domains None Change', ] response = logged_in_platform_admin_client.get(url_for('.organisation_settings', org_id=organisation_one['id'])) @@ -489,6 +523,277 @@ def test_organisation_settings( mock_get_organisation.assert_called_with(organisation_one['id']) +@pytest.mark.parametrize('endpoint, expected_options, expected_selected', ( + ( + '.edit_organisation_type', + ( + ('central', 'Central government'), + ('local', 'Local government'), + ('nhs', 'NHS'), + ), + None, + ), + ( + '.edit_organisation_crown_status', + ( + ('crown', 'Yes'), + ('non-crown', 'No'), + ('unknown', 'Not sure'), + ), + 'crown', + ), + ( + '.edit_organisation_agreement', + ( + ('yes', ( + 'Yes ' + 'Users will be told their organisation has already signed the agreement' + )), + ('no', ( + 'No ' + 'Users will be prompted to sign the agreement before they can go live' + )), + ('unknown', ( + 'No (but we have some service-specific agreements in place) ' + 'Users won’t be prompted to sign the agreement' + )), + ), + 'no', + ), +)) +@pytest.mark.parametrize('user', ( + pytest.param( + platform_admin_user, + ), + pytest.param( + active_user_with_permissions, + marks=pytest.mark.xfail + ), +)) +def test_view_organisation_settings( + client_request, + fake_uuid, + organisation_one, + mock_get_organisation, + endpoint, + expected_options, + expected_selected, + user, +): + client_request.login(user(fake_uuid)) + + page = client_request.get(endpoint, org_id=organisation_one['id']) + + radios = page.select('input[type=radio]') + + for index, option in enumerate(expected_options): + label = page.select_one('label[for={}]'.format(radios[index]['id'])) + assert ( + radios[index]['value'], + normalize_spaces(label.text), + ) == option + + if expected_selected: + assert page.select_one('input[checked]')['value'] == expected_selected + else: + assert not page.select_one('input[checked]') + + +@pytest.mark.parametrize('endpoint, post_data, expected_persisted', ( + ( + '.edit_organisation_type', + {'organisation_type': 'central'}, + {'organisation_type': 'central'}, + ), + ( + '.edit_organisation_type', + {'organisation_type': 'local'}, + {'organisation_type': 'local'}, + ), + ( + '.edit_organisation_type', + {'organisation_type': 'nhs'}, + {'organisation_type': 'nhs'}, + ), + ( + '.edit_organisation_crown_status', + {'crown_status': 'crown'}, + {'crown': True}, + ), + ( + '.edit_organisation_crown_status', + {'crown_status': 'non-crown'}, + {'crown': False}, + ), + ( + '.edit_organisation_crown_status', + {'crown_status': 'unknown'}, + {'crown': None}, + ), + ( + '.edit_organisation_agreement', + {'agreement_signed': 'yes'}, + {'agreement_signed': True}, + ), + ( + '.edit_organisation_agreement', + {'agreement_signed': 'no'}, + {'agreement_signed': False}, + ), + ( + '.edit_organisation_agreement', + {'agreement_signed': 'unknown'}, + {'agreement_signed': None}, + ), +)) +@pytest.mark.parametrize('user', ( + pytest.param( + platform_admin_user, + ), + pytest.param( + active_user_with_permissions, + marks=pytest.mark.xfail + ), +)) +def test_update_organisation_settings( + client_request, + fake_uuid, + organisation_one, + mock_get_organisation, + mock_update_organisation, + endpoint, + post_data, + expected_persisted, + user, +): + client_request.login(user(fake_uuid)) + + client_request.post( + endpoint, + org_id=organisation_one['id'], + _data=post_data, + _expected_status=302, + _expected_redirect=url_for( + 'main.organisation_settings', + org_id=organisation_one['id'], + _external=True, + ), + ) + + mock_update_organisation.assert_called_once_with( + organisation_one['id'], + **expected_persisted, + ) + + +@pytest.mark.parametrize('user', ( + pytest.param( + platform_admin_user, + ), + pytest.param( + active_user_with_permissions, + marks=pytest.mark.xfail + ), +)) +def test_view_organisation_domains( + mocker, + client_request, + fake_uuid, + user, +): + client_request.login(user(fake_uuid)) + + mocker.patch( + 'app.organisations_client.get_organisation', + side_effect=lambda org_id: organisation_json( + org_id, + 'Org 1', + domains=['example.gov.uk', 'test.example.gov.uk'], + ) + ) + + page = client_request.get( + 'main.edit_organisation_domains', + org_id=ORGANISATION_ID, + ) + + assert [textbox['value'] for textbox in page.select('input[type=text]')] == [ + 'example.gov.uk', + 'test.example.gov.uk', + '', + '', + '', + '', + '', + '', + '', + '', + ] + + +@pytest.mark.parametrize('post_data, expected_persisted', ( + ( + { + 'domains-0': 'example.gov.uk', + 'domains-5': 'test.gov.uk', + }, + { + 'domains': [ + 'example.gov.uk', + 'test.gov.uk', + ] + } + ), + ( + { + 'domains-0': '', + 'domains-1': '', + 'domains-2': '', + }, + { + 'domains': [] + } + ), +)) +@pytest.mark.parametrize('user', ( + pytest.param( + platform_admin_user, + ), + pytest.param( + active_user_with_permissions, + marks=pytest.mark.xfail + ), +)) +def test_update_organisation_domains( + client_request, + fake_uuid, + organisation_one, + mock_get_organisation, + mock_update_organisation, + post_data, + expected_persisted, + user, +): + client_request.login(user(fake_uuid)) + + client_request.post( + 'main.edit_organisation_domains', + org_id=ORGANISATION_ID, + _data=post_data, + _expected_status=302, + _expected_redirect=url_for( + 'main.organisation_settings', + org_id=organisation_one['id'], + _external=True, + ), + ) + + mock_update_organisation.assert_called_once_with( + ORGANISATION_ID, + **expected_persisted, + ) + + def test_update_organisation_name( logged_in_platform_admin_client, organisation_one, @@ -553,7 +858,7 @@ def test_confirm_update_organisation( organisation_one, mock_get_organisation, mock_verify_password, - mock_update_organisation_name, + mock_update_organisation, mocker ): with logged_in_platform_admin_client.session_transaction() as session: @@ -570,7 +875,7 @@ def test_confirm_update_organisation( assert response.status_code == 302 assert response.location == url_for('.organisation_settings', org_id=organisation_one['id'], _external=True) - mock_update_organisation_name.assert_called_with( + mock_update_organisation.assert_called_with( organisation_one['id'], name=session['organisation_name_change'] ) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 4509ae979..551fa0a51 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -11,8 +11,14 @@ from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient import app from app.utils import email_safe -from tests import sample_uuid, service_json, validate_route_permission +from tests import ( + organisation_json, + sample_uuid, + service_json, + validate_route_permission, +) from tests.conftest import ( + ORGANISATION_ID, SERVICE_ONE_ID, active_user_no_api_key_permission, active_user_no_settings_permission, @@ -2415,20 +2421,44 @@ def test_service_set_letter_branding_platform_admin_only( (str(UUID(int=1)), 'Land Registry'), )), ]) +@pytest.mark.parametrize('endpoint, extra_args', ( + ( + 'main.service_set_letter_branding', + {'service_id': SERVICE_ONE_ID}, + ), + ( + 'main.edit_organisation_letter_branding', + {'org_id': ORGANISATION_ID}, + ), +)) def test_service_set_letter_branding_prepopulates( - logged_in_platform_admin_client, + mocker, + client_request, + platform_admin_user, service_one, + mock_get_organisation, mock_get_all_letter_branding, letter_branding, expected_selected, expected_items, + endpoint, + extra_args, ): service_one['letter_branding'] = letter_branding - response = logged_in_platform_admin_client.get( - url_for('main.service_set_letter_branding', service_id=service_one['id']) + mocker.patch( + 'app.organisations_client.get_organisation', + side_effect=lambda org_id: organisation_json( + org_id, + 'Org 1', + letter_branding_id=letter_branding, + ) + ) + + client_request.login(platform_admin_user) + page = client_request.get( + endpoint, + **extra_args, ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert len(page.select('input[checked]')) == 1 assert page.select('input[checked]')[0]['value'] == expected_selected @@ -2446,36 +2476,70 @@ def test_service_set_letter_branding_prepopulates( (str(UUID(int=1)), str(UUID(int=1))), ('__NONE__', None), ]) +@pytest.mark.parametrize('endpoint, extra_args, expected_redirect', ( + ( + 'main.service_set_letter_branding', + {'service_id': SERVICE_ONE_ID}, + 'main.service_preview_letter_branding', + ), + ( + 'main.edit_organisation_letter_branding', + {'org_id': ORGANISATION_ID}, + 'main.organisation_preview_letter_branding', + ), +)) def test_service_set_letter_branding_redirects_to_preview_page_when_form_submitted( - logged_in_platform_admin_client, - service_one, + client_request, + platform_admin_user, + mock_get_organisation, mock_get_all_letter_branding, selected_letter_branding, - expected_post_data + expected_post_data, + endpoint, + extra_args, + expected_redirect, ): - response = logged_in_platform_admin_client.post( - url_for('main.service_set_letter_branding', service_id=service_one['id']), - data={'branding_style': selected_letter_branding}, + client_request.login(platform_admin_user) + client_request.post( + endpoint, + _data={'branding_style': selected_letter_branding}, + _expected_status=302, + _expected_redirect=url_for( + expected_redirect, + branding_style=expected_post_data, + _external=True, + **extra_args + ), + **extra_args ) - assert response.status_code == 302 - assert response.location == url_for( + + +@pytest.mark.parametrize('endpoint, extra_args', ( + ( 'main.service_preview_letter_branding', - service_id=service_one['id'], - branding_style=expected_post_data, - _external=True) - - + {'service_id': SERVICE_ONE_ID}, + ), + ( + 'main.organisation_preview_letter_branding', + {'org_id': ORGANISATION_ID}, + ), +)) def test_service_preview_letter_branding_shows_preview_letter( - logged_in_platform_admin_client, - service_one, + client_request, + platform_admin_user, + mock_get_organisation, mock_get_all_letter_branding, + endpoint, + extra_args, ): - response = logged_in_platform_admin_client.get( - url_for('main.service_preview_letter_branding', service_id=service_one['id'], branding_style='hm-government') - ) - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + client_request.login(platform_admin_user) + + page = client_request.get( + endpoint, + branding_style='hm-government', + **extra_args + ) - assert response.status_code == 200 assert page.find('iframe')['src'] == url_for('main.letter_template', branding_style='hm-government') @@ -2483,21 +2547,60 @@ def test_service_preview_letter_branding_shows_preview_letter( (str(UUID(int=1)), str(UUID(int=1))), ('__NONE__', None), ]) +@pytest.mark.parametrize('endpoint, extra_args, expected_redirect', ( + ( + 'main.service_preview_letter_branding', + {'service_id': SERVICE_ONE_ID}, + 'main.service_settings', + ), + ( + 'main.organisation_preview_letter_branding', + {'org_id': ORGANISATION_ID}, + 'main.organisation_settings', + ), +)) def test_service_preview_letter_branding_saves( - logged_in_platform_admin_client, - service_one, + client_request, + platform_admin_user, + mock_get_organisation, mock_update_service, + mock_update_organisation, mock_get_all_letter_branding, selected_letter_branding, - expected_post_data + expected_post_data, + endpoint, + extra_args, + expected_redirect, ): - response = logged_in_platform_admin_client.post( - url_for('main.service_preview_letter_branding', service_id=service_one['id']), - data={'branding_style': selected_letter_branding} + client_request.login(platform_admin_user) + client_request.post( + endpoint, + _data={'branding_style': selected_letter_branding}, + _expected_status=302, + _expected_redirect=url_for( + expected_redirect, + _external=True, + **extra_args + ), + **extra_args ) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - mock_update_service.assert_called_once_with(service_one['id'], letter_branding=expected_post_data) + + if endpoint == 'main.service_preview_letter_branding': + mock_update_service.assert_called_once_with( + SERVICE_ONE_ID, + letter_branding=expected_post_data, + ) + assert mock_update_organisation.called is False + + elif endpoint == 'main.organisation_preview_letter_branding': + mock_update_organisation.assert_called_once_with( + ORGANISATION_ID, + letter_branding_id=expected_post_data, + ) + assert mock_update_service.called is False + + else: + raise Exception @pytest.mark.parametrize('current_branding, expected_values, expected_labels', [ @@ -2512,20 +2615,41 @@ def test_service_preview_letter_branding_saves( 'org 5', 'GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4', ]), ]) +@pytest.mark.parametrize('endpoint, extra_args', ( + ( + 'main.service_set_email_branding', + {'service_id': SERVICE_ONE_ID}, + ), + ( + 'main.edit_organisation_email_branding', + {'org_id': ORGANISATION_ID}, + ), +)) def test_should_show_branding_styles( - logged_in_platform_admin_client, + mocker, + client_request, + platform_admin_user, service_one, mock_get_all_email_branding, current_branding, expected_values, expected_labels, + endpoint, + extra_args, ): service_one['email_branding'] = current_branding - response = logged_in_platform_admin_client.get(url_for( - 'main.service_set_email_branding', service_id=service_one['id'] - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + mocker.patch( + 'app.organisations_client.get_organisation', + side_effect=lambda org_id: organisation_json( + org_id, + 'Org 1', + email_branding_id=current_branding, + ) + ) + + client_request.login(platform_admin_user) + page = client_request.get(endpoint, **extra_args) + branding_style_choices = page.find_all('input', attrs={"name": "branding_style"}) radio_labels = [ @@ -2551,39 +2675,74 @@ def test_should_show_branding_styles( app.service_api_client.get_service.assert_called_once_with(service_one['id']) +@pytest.mark.parametrize('endpoint, extra_args, expected_redirect', ( + ( + 'main.service_set_email_branding', + {'service_id': SERVICE_ONE_ID}, + 'main.service_preview_email_branding', + ), + ( + 'main.edit_organisation_email_branding', + {'org_id': ORGANISATION_ID}, + 'main.organisation_preview_email_branding', + ), +)) def test_should_send_branding_and_organisations_to_preview( - logged_in_platform_admin_client, + client_request, + platform_admin_user, service_one, + mock_get_organisation, mock_get_all_email_branding, mock_update_service, + endpoint, + extra_args, + expected_redirect, ): - response = logged_in_platform_admin_client.post( - url_for( - 'main.service_set_email_branding', service_id=service_one['id'] - ), + client_request.login(platform_admin_user) + client_request.post( + endpoint, data={ 'branding_type': 'org', 'branding_style': '1' - } + }, + _expected_status=302, + _expected_location=url_for( + expected_redirect, + branding_style='1', + _external=True, + **extra_args + ), + **extra_args ) - assert response.status_code == 302 - assert response.location == url_for('main.service_preview_email_branding', - service_id=service_one['id'], branding_style='1', - _external=True) mock_get_all_email_branding.assert_called_once_with() +@pytest.mark.parametrize('endpoint, extra_args', ( + ( + 'main.service_preview_email_branding', + {'service_id': SERVICE_ONE_ID}, + ), + ( + 'main.organisation_preview_email_branding', + {'org_id': ORGANISATION_ID}, + ), +)) def test_should_preview_email_branding( - logged_in_platform_admin_client, - service_one, + client_request, + platform_admin_user, + mock_get_organisation, + endpoint, + extra_args, ): - response = logged_in_platform_admin_client.get(url_for( - 'main.service_preview_email_branding', service_id=service_one['id'], - branding_type='org', branding_style='1' - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + client_request.login(platform_admin_user) + page = client_request.get( + endpoint, + branding_type='org', + branding_style='1', + **extra_args + ) + iframe = page.find('iframe', attrs={"class": "branding-preview"}) iframeURLComponents = urlparse(iframe['src']) iframeQString = parse_qs(iframeURLComponents.query) @@ -2592,37 +2751,66 @@ def test_should_preview_email_branding( assert iframeURLComponents.path == '/_email' assert iframeQString['branding_style'] == ['1'] - app.service_api_client.get_service.assert_called_once_with(service_one['id']) - @pytest.mark.parametrize('posted_value, submitted_value', ( ('1', '1'), ('__NONE__', None), pytest.param('None', None, marks=pytest.mark.xfail(raises=AssertionError)), )) +@pytest.mark.parametrize('endpoint, extra_args, expected_redirect', ( + ( + 'main.service_preview_email_branding', + {'service_id': SERVICE_ONE_ID}, + 'main.service_settings', + ), + ( + 'main.organisation_preview_email_branding', + {'org_id': ORGANISATION_ID}, + 'main.organisation_settings', + ), +)) def test_should_set_branding_and_organisations( - logged_in_platform_admin_client, + client_request, + platform_admin_user, service_one, + mock_get_organisation, mock_update_service, + mock_update_organisation, posted_value, submitted_value, + endpoint, + extra_args, + expected_redirect, ): - response = logged_in_platform_admin_client.post( - url_for( - 'main.service_preview_email_branding', service_id=service_one['id'] - ), - data={ + client_request.login(platform_admin_user) + client_request.post( + endpoint, + _data={ 'branding_style': posted_value - } + }, + _expected_status=302, + _expected_redirect=url_for( + expected_redirect, + _external=True, + **extra_args + ), + **extra_args ) - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', - service_id=service_one['id'], _external=True) - mock_update_service.assert_called_once_with( - service_one['id'], - email_branding=submitted_value - ) + if endpoint == 'main.service_preview_email_branding': + mock_update_service.assert_called_once_with( + SERVICE_ONE_ID, + email_branding=submitted_value, + ) + assert mock_update_organisation.called is False + elif endpoint == 'main.organisation_preview_email_branding': + mock_update_organisation.assert_called_once_with( + ORGANISATION_ID, + email_branding_id=submitted_value + ) + assert mock_update_service.called is False + else: + raise Exception @pytest.mark.parametrize('method', ['get', 'post']) diff --git a/tests/conftest.py b/tests/conftest.py index 7e96605fe..5100fb000 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3100,34 +3100,27 @@ def organisation_one(api_user_active): def mock_get_organisations(mocker): def _get_organisations(): return [ - { - 'name': 'Org 1', - 'id': '7aa5d4e9-4385-4488-a489-07812ba13383', - 'active': True - }, - { - 'name': 'Org 2', - 'id': '7aa5d4e9-4385-4488-a489-07812ba13384', - 'active': True - }, - { - 'name': 'Org 3', - 'id': '7aa5d4e9-4385-4488-a489-07812ba13385', - 'active': True - } + organisation_json('7aa5d4e9-4385-4488-a489-07812ba13383', 'Org 1'), + organisation_json('7aa5d4e9-4385-4488-a489-07812ba13384', 'Org 2'), + organisation_json('7aa5d4e9-4385-4488-a489-07812ba13385', 'Org 3'), ] return mocker.patch('app.organisations_client.get_organisations', side_effect=_get_organisations) @pytest.fixture(scope='function') -def mock_get_organisation(mocker): +def mock_get_organisation( + mocker, + email_branding_id=None, + letter_branding_id=None, +): def _get_organisation(org_id): - return { - 'name': 'Org 1', - 'id': org_id, - 'active': True - } + return organisation_json( + org_id, + 'Org 1', + email_branding_id=email_branding_id, + letter_branding_id=letter_branding_id, + ) return mocker.patch('app.organisations_client.get_organisation', side_effect=_get_organisation) @@ -3135,11 +3128,7 @@ def mock_get_organisation(mocker): @pytest.fixture(scope='function') def mock_get_service_organisation(mocker): def _get_service_organisation(service_id): - return { - 'name': 'Org 1', - 'id': '7aa5d4e9-4385-4488-a489-07812ba13383', - 'active': True - } + return organisation_json('7aa5d4e9-4385-4488-a489-07812ba13383', 'Org 1') return mocker.patch('app.organisations_client.get_service_organisation', side_effect=_get_service_organisation) @@ -3269,6 +3258,14 @@ def mock_update_organisation_name(mocker): return mocker.patch('app.organisations_client.update_organisation_name', side_effect=_update_org_name) +@pytest.fixture(scope='function') +def mock_update_organisation(mocker): + def _update_org(organisation_id, **kwargs): + return + + return mocker.patch('app.organisations_client.update_organisation', side_effect=_update_org) + + @pytest.fixture def mock_get_organisations_and_services_for_user(mocker, organisation_one, api_user_active): def _get_orgs_and_services(user_id):