From 9ad4435d94d361d679fbab6b184f85a6a3b8e246 Mon Sep 17 00:00:00 2001 From: chrisw Date: Wed, 7 Feb 2018 10:30:49 +0000 Subject: [PATCH] Change organisations to email branding --- app/__init__.py | 6 +- .../stylesheets/components/textbox.scss | 11 + app/main/__init__.py | 2 +- app/main/forms.py | 54 ++- app/main/views/email_branding.py | 134 ++++++++ app/main/views/organisations.py | 99 ------ app/main/views/service_settings.py | 52 +-- ...ons_client.py => email_branding_client.py} | 20 +- app/notify_client/service_api_client.py | 2 +- app/templates/components/textbox.html | 26 ++ .../views/email-branding/manage-branding.html | 45 +++ .../select-branding.html} | 8 +- .../views/organisations/manage-org.html | 54 --- .../views/platform-admin/_base_template.html | 2 +- app/templates/views/service-settings.html | 8 +- ...g-and-org.html => set-email-branding.html} | 4 +- tests/__init__.py | 4 +- .../test_service_setting_permissions.py | 4 +- tests/app/main/views/test_email_branding.py | 313 ++++++++++++++++++ tests/app/main/views/test_organisations.py | 281 ---------------- tests/app/main/views/test_service_settings.py | 38 +-- .../test_email_branding_client.py | 50 +++ .../test_organisations_client.py | 50 --- tests/conftest.py | 63 +++- 24 files changed, 736 insertions(+), 594 deletions(-) create mode 100644 app/main/views/email_branding.py delete mode 100644 app/main/views/organisations.py rename app/notify_client/{organisations_client.py => email_branding_client.py} (50%) create mode 100644 app/templates/views/email-branding/manage-branding.html rename app/templates/views/{organisations/select-org.html => email-branding/select-branding.html} (69%) delete mode 100644 app/templates/views/organisations/manage-org.html rename app/templates/views/service-settings/{set-branding-and-org.html => set-email-branding.html} (85%) create mode 100644 tests/app/main/views/test_email_branding.py delete mode 100644 tests/app/main/views/test_organisations.py create mode 100644 tests/app/notify_client/test_email_branding_client.py delete mode 100644 tests/app/notify_client/test_organisations_client.py diff --git a/app/__init__.py b/app/__init__.py index 1f6288974..67ff8ca72 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -49,7 +49,7 @@ from app.notify_client.template_statistics_api_client import TemplateStatisticsA from app.notify_client.user_api_client import UserApiClient from app.notify_client.events_api_client import EventsApiClient from app.notify_client.provider_client import ProviderClient -from app.notify_client.organisations_client import OrganisationsClient +from app.notify_client.email_branding_client import EmailBrandingClient from app.notify_client.models import AnonymousUser from app.notify_client.letter_jobs_client import LetterJobsClient from app.notify_client.inbound_number_client import InboundNumberClient @@ -71,7 +71,7 @@ invite_api_client = InviteApiClient() template_statistics_client = TemplateStatisticsApiClient() events_api_client = EventsApiClient() provider_client = ProviderClient() -organisations_client = OrganisationsClient() +email_branding_client = EmailBrandingClient() asset_fingerprinter = AssetFingerprinter() statsd_client = StatsdClient() deskpro_client = DeskproClient() @@ -107,7 +107,7 @@ def create_app(application): template_statistics_client.init_app(application) events_api_client.init_app(application) provider_client.init_app(application) - organisations_client.init_app(application) + email_branding_client.init_app(application) letter_jobs_client.init_app(application) inbound_number_client.init_app(application) billing_api_client.init_app(application) diff --git a/app/assets/stylesheets/components/textbox.scss b/app/assets/stylesheets/components/textbox.scss index 9975df803..0555c0586 100644 --- a/app/assets/stylesheets/components/textbox.scss +++ b/app/assets/stylesheets/components/textbox.scss @@ -59,3 +59,14 @@ padding-left: 5px; letter-spacing: 0.04em; } + +.textbox-colour-preview { + @include media(desktop) { + width: 34px; + height: 34px; + margin-left: 5px; + border: 2px solid #0B0C0C; + display: inline-block; + vertical-align: top; + } +} diff --git a/app/main/__init__.py b/app/main/__init__.py index 042aa1e3d..ceea4e4b0 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -28,7 +28,7 @@ from app.main.views import ( # noqa providers, platform_admin, letter_jobs, - organisations, + email_branding, conversation, notifications, inbound_number diff --git a/app/main/forms.py b/app/main/forms.py index 49279d0d3..2004d1661 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -598,52 +598,70 @@ class ServiceSwitchLettersForm(StripWhitespaceForm): ) -class ServiceBrandingOrg(StripWhitespaceForm): +class ServiceSetBranding(StripWhitespaceForm): - def __init__(self, organisations=[], *args, **kwargs): - self.organisation.choices = organisations - super(ServiceBrandingOrg, self).__init__(*args, **kwargs) + def __init__(self, email_branding=[], *args, **kwargs): + self.branding_style.choices = email_branding + super(ServiceSetBranding, self).__init__(*args, **kwargs) branding_type = RadioField( - 'Branding', + 'Branding type', choices=[ ('govuk', 'GOV.UK only'), - ('both', 'GOV.UK and organisation'), - ('org', 'Organisation only'), - ('org_banner', 'Organisation banner') + ('both', 'GOV.UK and branding'), + ('org', 'Branding only'), + ('org_banner', 'Branding banner') ], validators=[ DataRequired() ] ) - organisation = RadioField( - 'Organisation', + branding_style = RadioField( + 'Branding style', validators=[ DataRequired() ] ) -class ServiceSelectOrg(StripWhitespaceForm): +class ServiceSelectEmailBranding(StripWhitespaceForm): - def __init__(self, organisations=[], *args, **kwargs): - self.organisation.choices = organisations - super(ServiceSelectOrg, self).__init__(*args, **kwargs) + def __init__(self, email_brandings=[], *args, **kwargs): + self.email_branding.choices = email_brandings + super(ServiceSelectEmailBranding, self).__init__(*args, **kwargs) - organisation = RadioField( - 'Organisation', + email_branding = RadioField( + 'Email branding', validators=[ DataRequired() ] ) -class ServiceManageOrg(StripWhitespaceForm): +class ServiceUpdateEmailBranding(StripWhitespaceForm): name = StringField('Name') + colour = StringField( + 'Colour', + render_kw={'onchange': 'update_colour(this)'}, + validators=[ + Regexp(regex="^$|^#(?:[0-9a-fA-F]{3}){1,2}$", message='Must be a valid color hex code') + ] + ) + file = FileField_wtf('Upload a PNG logo', validators=[FileAllowed(['png'], 'PNG Images only!')]) - colour = StringField('Colour', render_kw={'onkeyup': 'update_colour_span()', 'onblur': 'update_colour_span()'}) + +class ServiceCreateEmailBranding(StripWhitespaceForm): + + name = StringField('Name') + colour = StringField( + 'Colour', + render_kw={'onchange': 'update_colour(this)'}, + validators=[ + Regexp(regex="^$|^#(?:[0-9a-fA-F]{3}){1,2}$", message='Must be a valid color hex code') + ] + ) file = FileField_wtf('Upload a PNG logo', validators=[FileAllowed(['png'], 'PNG Images only!')]) diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py new file mode 100644 index 000000000..ec1457e5a --- /dev/null +++ b/app/main/views/email_branding.py @@ -0,0 +1,134 @@ +from flask import current_app, redirect, render_template, session, url_for +from flask_login import login_required + +from app import email_branding_client +from app.main import main +from app.main.forms import ( + ServiceSelectEmailBranding, + ServiceUpdateEmailBranding, + ServiceCreateEmailBranding +) +from app.utils import user_has_permissions, get_cdn_domain +from app.main.s3_client import ( + TEMP_TAG, + upload_logo, + delete_temp_file, + delete_temp_files_created_by, + persist_logo +) +from app.main.views.service_settings import get_branding_as_value_and_label, get_branding_as_dict + + +@main.route("/email-branding", methods=['GET', 'POST']) +@login_required +@user_has_permissions(admin_override=True) +def email_branding(): + brandings = email_branding_client.get_all_email_branding() + + form = ServiceSelectEmailBranding() + form.email_branding.choices = get_branding_as_value_and_label(brandings) + [('None', 'Create a new email branding')] + + if form.validate_on_submit(): + if form.email_branding.data != 'None': + return redirect(url_for('.update_email_branding', branding_id=form.email_branding.data)) + else: + return redirect(url_for('.create_email_branding')) + + return render_template( + 'views/email-branding/select-branding.html', + form=form, + branding_dict=get_branding_as_dict(brandings), + ) + + +@main.route("/email-branding//edit", methods=['GET', 'POST']) +@main.route("/email-branding//edit/", methods=['GET', 'POST']) +@login_required +@user_has_permissions(admin_override=True) +def update_email_branding(branding_id, logo=None): + email_branding = email_branding_client.get_email_branding(branding_id)['email_branding'] + + form = ServiceUpdateEmailBranding() + + logo = logo if logo else email_branding.get('logo') if email_branding else None + + if form.validate_on_submit(): + if form.file.data: + upload_filename = upload_logo( + form.file.data.filename, + form.file.data, + current_app.config['AWS_REGION'], + user_id=session["user_id"] + ) + + if logo and logo.startswith(TEMP_TAG.format(user_id=session['user_id'])): + delete_temp_file(logo) + + return redirect(url_for('.update_email_branding', branding_id=branding_id, logo=upload_filename)) + + if logo: + logo = persist_logo(logo, session["user_id"]) + + delete_temp_files_created_by(session["user_id"]) + + email_branding_client.update_email_branding( + branding_id=branding_id, + logo=logo, + name=form.name.data, + colour=form.colour.data + ) + + return redirect(url_for('.email_branding', branding_id=branding_id)) + + form.name.data = email_branding['name'] + form.colour.data = email_branding['colour'] + + return render_template( + 'views/email-branding/manage-branding.html', + form=form, + email_branding=email_branding, + cdn_url=get_cdn_domain(), + logo=logo + ) + + +@main.route("/email-branding/create", methods=['GET', 'POST']) +@main.route("/email-branding/create/", methods=['GET', 'POST']) +@login_required +@user_has_permissions(admin_override=True) +def create_email_branding(logo=None): + form = ServiceCreateEmailBranding() + + if form.validate_on_submit(): + if form.file.data: + upload_filename = upload_logo( + form.file.data.filename, + form.file.data, + current_app.config['AWS_REGION'], + user_id=session["user_id"] + ) + + if logo and logo.startswith(TEMP_TAG.format(user_id=session['user_id'])): + delete_temp_file(logo) + + return redirect(url_for('.create_email_branding', logo=upload_filename)) + + if logo: + logo = persist_logo(logo, session["user_id"]) + + delete_temp_files_created_by(session["user_id"]) + + email_branding_client.create_email_branding( + logo=logo, + name=form.name.data, + colour=form.colour.data + ) + + return redirect(url_for('.email_branding')) + + return render_template( + 'views/email-branding/manage-branding.html', + form=form, + cdn_url=get_cdn_domain(), + logo=logo + ) diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py deleted file mode 100644 index 735329193..000000000 --- a/app/main/views/organisations.py +++ /dev/null @@ -1,99 +0,0 @@ -from flask import current_app, redirect, render_template, session, url_for -from flask_login import login_required - -from app import organisations_client -from app.main import main -from app.main.forms import ( - ServiceSelectOrg, - ServiceManageOrg) -from app.utils import user_has_permissions, get_cdn_domain -from app.main.s3_client import ( - TEMP_TAG, - upload_logo, - delete_temp_file, - delete_temp_files_created_by, - persist_logo -) -from app.main.views.service_settings import get_branding_as_value_and_label, get_branding_as_dict - - -@main.route("/organisations", methods=['GET', 'POST']) -@main.route("/organisations/", methods=['GET', 'POST']) -@login_required -@user_has_permissions(admin_override=True) -def organisations(organisation_id=None): - orgs = organisations_client.get_organisations() - - form = ServiceSelectOrg() - form.organisation.choices = get_branding_as_value_and_label(orgs) + [('None', 'Create a new organisation')] - - if form.validate_on_submit(): - if form.organisation.data != 'None': - session['organisation'] = [o for o in orgs if o['id'] == form.organisation.data][0] - elif session.get('organisation'): - del session['organisation'] - - return redirect(url_for('.manage_org')) - - form.organisation.data = organisation_id if organisation_id in [o['id'] for o in orgs] else 'None' - - return render_template( - 'views/organisations/select-org.html', - form=form, - branding_dict=get_branding_as_dict(orgs), - organisation_id=organisation_id - ) - - -@main.route("/organisations/manage", methods=['GET', 'POST']) -@main.route("/organisations/manage/", methods=['GET', 'POST']) -@login_required -@user_has_permissions(admin_override=True) -def manage_org(logo=None): - form = ServiceManageOrg() - - org = session.get("organisation") - - logo = logo if logo else org.get('logo') if org else None - - if form.validate_on_submit(): - if form.file.data: - upload_filename = upload_logo( - form.file.data.filename, - form.file.data, - current_app.config['AWS_REGION'], - user_id=session["user_id"] - ) - - if logo and logo.startswith(TEMP_TAG.format(user_id=session['user_id'])): - delete_temp_file(logo) - - return redirect( - url_for('.manage_org', logo=upload_filename)) - - if logo: - logo = persist_logo(logo, session["user_id"]) - - delete_temp_files_created_by(session["user_id"]) - - if org: - organisations_client.update_organisation( - org_id=org['id'], logo=logo, name=form.name.data, colour=form.colour.data) - org_id = org['id'] - else: - resp = organisations_client.create_organisation( - logo=logo, name=form.name.data, colour=form.colour.data) - org_id = resp['data']['id'] - - return redirect(url_for('.organisations', organisation_id=org_id)) - if org: - form.name.data = org['name'] - form.colour.data = org['colour'] - - return render_template( - 'views/organisations/manage-org.html', - form=form, - organisation=org, - cdn_url=get_cdn_domain(), - logo=logo - ) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index b99ef1807..e5e5da052 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -28,7 +28,7 @@ from app.main.forms import ( ServiceInboundNumberForm, ServiceSmsSenderForm, ServiceLetterContactBlockForm, - ServiceBrandingOrg, + ServiceSetBranding, LetterBranding, InternationalSMSForm, OrganisationTypeForm, @@ -37,7 +37,7 @@ from app.main.forms import ( SMSPrefixForm, ServiceSwitchLettersForm, ) -from app import user_api_client, current_service, organisations_client, inbound_number_client, billing_api_client +from app import user_api_client, current_service, email_branding_client, inbound_number_client, billing_api_client from notifications_utils.formatters import formatted_list @@ -45,11 +45,11 @@ from notifications_utils.formatters import formatted_list @login_required @user_has_permissions('manage_settings', admin_override=True) def service_settings(service_id): - letter_branding_organisations = organisations_client.get_letter_organisations() - if current_service['organisation']: - organisation = organisations_client.get_organisation(current_service['organisation'])['organisation'] + letter_branding_organisations = email_branding_client.get_letter_email_branding() + if current_service['email_branding']: + email_branding = email_branding_client.get_email_branding(current_service['email_branding'])['email_branding'] else: - organisation = None + email_branding = None inbound_number = inbound_number_client.get_inbound_sms_number_for_service(service_id) disp_inbound_number = inbound_number['data'].get('number', '') @@ -73,7 +73,7 @@ def service_settings(service_id): return render_template( 'views/service-settings.html', - organisation=organisation, + email_branding=email_branding, letter_branding=letter_branding_organisations.get( current_service.get('dvla_organisation', '001') ), @@ -711,32 +711,32 @@ def set_free_sms_allowance(service_id): ) -@main.route("/services//service-settings/set-branding-and-org", methods=['GET', 'POST']) +@main.route("/services//service-settings/set-email-branding", methods=['GET', 'POST']) @login_required @user_has_permissions(admin_override=True) -def service_set_branding_and_org(service_id): - organisations = organisations_client.get_organisations() +def service_set_email_branding(service_id): + email_branding = email_branding_client.get_all_email_branding() - form = ServiceBrandingOrg(branding_type=current_service.get('branding')) + form = ServiceSetBranding(branding_type=current_service.get('branding')) # dynamically create org choices, including the null option - form.organisation.choices = [('None', 'None')] + get_branding_as_value_and_label(organisations) + form.branding_style.choices = [('None', 'None')] + get_branding_as_value_and_label(email_branding) if form.validate_on_submit(): - organisation = None if form.organisation.data == 'None' else form.organisation.data + branding_style = None if form.branding_style.data == 'None' else form.branding_style.data service_api_client.update_service( service_id, branding=form.branding_type.data, - organisation=organisation + email_branding=branding_style ) return redirect(url_for('.service_settings', service_id=service_id)) - form.organisation.data = current_service['organisation'] or 'None' + form.branding_style.data = current_service['email_branding'] or 'None' return render_template( - 'views/service-settings/set-branding-and-org.html', + 'views/service-settings/set-email-branding.html', form=form, - branding_dict=get_branding_as_dict(organisations) + branding_dict=get_branding_as_dict(email_branding) ) @@ -745,7 +745,7 @@ def service_set_branding_and_org(service_id): @user_has_permissions(admin_override=True) def set_letter_branding(service_id): - form = LetterBranding(choices=organisations_client.get_letter_organisations().items()) + form = LetterBranding(choices=email_branding_client.get_letter_email_branding().items()) if form.validate_on_submit(): service_api_client.update_service( @@ -762,17 +762,17 @@ def set_letter_branding(service_id): ) -def get_branding_as_value_and_label(organisations): +def get_branding_as_value_and_label(email_branding): return [ - (organisation['id'], organisation['name']) - for organisation in organisations + (branding['id'], branding['name']) + for branding in email_branding ] -def get_branding_as_dict(organisations): +def get_branding_as_dict(email_branding): return { - organisation['id']: { - 'logo': 'https://{}/{}'.format(get_cdn_domain(), organisation['logo']), - 'colour': organisation['colour'] - } for organisation in organisations + branding['id']: { + 'logo': 'https://{}/{}'.format(get_cdn_domain(), branding['logo']), + 'colour': branding['colour'] + } for branding in email_branding } diff --git a/app/notify_client/organisations_client.py b/app/notify_client/email_branding_client.py similarity index 50% rename from app/notify_client/organisations_client.py rename to app/notify_client/email_branding_client.py index aae8d4c07..b82981adc 100644 --- a/app/notify_client/organisations_client.py +++ b/app/notify_client/email_branding_client.py @@ -1,7 +1,7 @@ from app.notify_client import NotifyAdminAPIClient -class OrganisationsClient(NotifyAdminAPIClient): +class EmailBrandingClient(NotifyAdminAPIClient): def __init__(self): super().__init__("a" * 73, "b") @@ -11,27 +11,27 @@ class OrganisationsClient(NotifyAdminAPIClient): self.service_id = app.config['ADMIN_CLIENT_USER_NAME'] self.api_key = app.config['ADMIN_CLIENT_SECRET'] - def get_organisation(self, id): - return self.get(url='/organisation/{}'.format(id)) + def get_email_branding(self, branding_id): + return self.get(url='/email-branding/{}'.format(branding_id)) - def get_organisations(self): - return self.get(url='/organisation')['organisations'] + def get_all_email_branding(self): + return self.get(url='/email-branding')['email_branding'] - def get_letter_organisations(self): + def get_letter_email_branding(self): return self.get(url='/dvla_organisations') - def create_organisation(self, logo, name, colour): + def create_email_branding(self, logo, name, colour): data = { "logo": logo, "name": name, "colour": colour } - return self.post(url="/organisation", data=data) + return self.post(url="/email-branding", data=data) - def update_organisation(self, org_id, logo, name, colour): + def update_email_branding(self, branding_id, logo, name, colour): data = { "logo": logo, "name": name, "colour": colour } - return self.post(url="/organisation/{}".format(org_id), data=data) + return self.post(url="/email-branding/{}".format(branding_id), data=data) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 848da40b4..eb8fa8273 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -99,7 +99,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): 'sms_sender', 'created_by', 'branding', - 'organisation', + 'email_branding', 'letter_contact_block', 'dvla_organisation', 'permissions', diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html index 9d032d1ca..955ffb7d5 100644 --- a/app/templates/components/textbox.html +++ b/app/templates/components/textbox.html @@ -57,3 +57,29 @@ {% endif %} {% endmacro %} + +{% macro colour_textbox( + field, + width='2-3', + colour='#FFFFFF' +) %} +
+ + {% set field_class = 'form-control-{} {}'.format(width, 'textbox-right-aligned' if suffix else '') %} + {% set field_class = 'form-control ' + field_class + (' form-control-error' if field.errors else '') %} + {{ field( + class=field_class, + data_module='', + rows='1', + **kwargs + ) }} + +
+{% endmacro %} diff --git a/app/templates/views/email-branding/manage-branding.html b/app/templates/views/email-branding/manage-branding.html new file mode 100644 index 000000000..b64fa9db0 --- /dev/null +++ b/app/templates/views/email-branding/manage-branding.html @@ -0,0 +1,45 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/file-upload.html" import file_upload %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/textbox.html" import textbox, colour_textbox %} + +{% block service_page_title %} + {{ '{} email branding'.format('Update' if email_branding else 'Create')}} +{% endblock %} + +{% block platform_admin_content %} + +

{{ '{} email branding'.format('Update' if email_branding else 'Create')}}

+
+
+ + {% if logo %} +
+ +
+ {% endif %} + {{ file_upload(form.file, button_text='{} logo'.format('Update' if email_branding else 'Upload')) }} +
+
+
{{textbox(form.name)}}
+ {{colour_textbox(form.colour, width='1-4', colour=email_branding.colour if email_branding)}} + {{ page_footer( + 'Save', + back_link=url_for('.email_branding'), + back_link_text='Back to email branding selection', + ) }} +
+
+
+
+ +{% endblock %} diff --git a/app/templates/views/organisations/select-org.html b/app/templates/views/email-branding/select-branding.html similarity index 69% rename from app/templates/views/organisations/select-org.html rename to app/templates/views/email-branding/select-branding.html index b1ebc9dbd..732adee43 100644 --- a/app/templates/views/organisations/select-org.html +++ b/app/templates/views/email-branding/select-branding.html @@ -3,19 +3,19 @@ {% from "components/page-footer.html" import page_footer %} {% block service_page_title %} - Select organisation + Select email branding {% endblock %} {% block platform_admin_content %}

-
Select an organisation to update
-
or create a new organisation
+
Select an email branding to update
+
or create a new email branding

- {{ branding_radios(form.organisation, branding_dict=branding_dict, show_header=False) }} + {{ branding_radios(form.email_branding, branding_dict=branding_dict, show_header=False) }} {{ page_footer( 'Next' ) }} diff --git a/app/templates/views/organisations/manage-org.html b/app/templates/views/organisations/manage-org.html deleted file mode 100644 index c0385bbf9..000000000 --- a/app/templates/views/organisations/manage-org.html +++ /dev/null @@ -1,54 +0,0 @@ -{% extends "views/platform-admin/_base_template.html" %} -{% from "components/file-upload.html" import file_upload %} -{% from "components/page-footer.html" import page_footer %} -{% from "components/textbox.html" import textbox %} - -{% block service_page_title %} - {{ '{} an organisations logo'.format('Update' if organisation else 'Create')}} -{% endblock %} - -{% block platform_admin_content %} - -

{{ '{} an organisations logo'.format('Update' if organisation else 'Create')}}

-
-
- - {% if logo %} -
- -
- {% endif %} - {{ - file_upload( - form.file, - button_text='{} logo'.format('Update' if organisation else 'Upload') - ) }} - -
-
{{textbox(form.name)}}
-
{{textbox(form.colour, width='1-4')}} - -
- {{ page_footer( - 'Save', - back_link=url_for('.organisations', organisation_id=organisation.id if organisation else 'None'), - back_link_text='Back to organisation selection', - ) }} -
- -
-
- - -{% endblock %} diff --git a/app/templates/views/platform-admin/_base_template.html b/app/templates/views/platform-admin/_base_template.html index 0866eb897..da8d6dd9f 100644 --- a/app/templates/views/platform-admin/_base_template.html +++ b/app/templates/views/platform-admin/_base_template.html @@ -16,7 +16,7 @@ ('Live services', url_for('main.live_services')), ('Trial mode services', url_for('main.trial_services')), ('Providers', url_for('main.view_providers')), - ('Organisations', url_for('main.organisations')), + ('Email branding', url_for('main.email_branding')), ('Letter jobs', url_for('main.letter_jobs')), ('Inbound SMS numbers', url_for('main.inbound_sms_admin')) ] %} diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 99a6b4e7b..b378f5320 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -211,14 +211,14 @@ {% if current_service.branding == 'govuk' %} GOV.UK {% elif current_service.branding == 'both' %} - GOV.UK and {{ organisation.name if organisation else None }} + GOV.UK and {{ email_branding.name if email_branding else None }} {% elif current_service.branding == 'org' %} - Only {{ organisation.name if organisation else None }} + Only {{ email_branding.name if email_branding else None }} {% elif current_service.branding == 'org_banner' %} - Only {{ organisation.name if organisation else None }} banner + Only {{ email_branding.name if email_branding else None }} banner {% endif %} {% endcall %} - {{ edit_field('Change', url_for('.service_set_branding_and_org', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.service_set_email_branding', service_id=current_service.id)) }} {% endcall %} {% call row() %} {{ text_field('Letter branding')}} diff --git a/app/templates/views/service-settings/set-branding-and-org.html b/app/templates/views/service-settings/set-email-branding.html similarity index 85% rename from app/templates/views/service-settings/set-branding-and-org.html rename to app/templates/views/service-settings/set-email-branding.html index 16d353d61..6aef3a71e 100644 --- a/app/templates/views/service-settings/set-branding-and-org.html +++ b/app/templates/views/service-settings/set-email-branding.html @@ -3,7 +3,7 @@ {% from "components/page-footer.html" import page_footer %} {% block service_page_title %} - Set branding and organisation + Set email branding {% endblock %} {% block maincolumn_content %} @@ -13,7 +13,7 @@
{{ radios(form.branding_type) }} - {{ branding_radios(form.organisation, branding_dict=branding_dict) }} + {{ branding_radios(form.branding_style, branding_dict=branding_dict) }} {{ page_footer( 'Save', back_link=url_for('.service_settings', service_id=current_service.id), diff --git a/tests/__init__.py b/tests/__init__.py index f84d57c05..ad095fdc6 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -48,7 +48,7 @@ def service_json( reply_to_email_address=None, sms_sender='GOVUK', research_mode=False, - organisation=None, + email_branding=None, branding='govuk', created_at=None, letter_contact_block=None, @@ -76,7 +76,7 @@ def service_json( 'sms_sender': sms_sender, 'research_mode': research_mode, 'organisation_type': organisation_type, - 'organisation': organisation, + 'email_branding': email_branding, 'branding': branding, 'created_at': created_at or str(datetime.utcnow()), 'letter_contact_block': letter_contact_block, diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index 4e0d08b36..a126e454b 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -11,7 +11,7 @@ def get_service_settings_page( logged_in_platform_admin_client, service_one, mock_get_inbound_number_for_service, - mock_get_letter_organisations, + mock_get_letter_email_branding, mock_get_free_sms_fragment_limit, no_reply_to_email_addresses, no_letter_contact_blocks, @@ -104,7 +104,7 @@ def test_normal_user_doesnt_see_any_toggle_buttons( no_reply_to_email_addresses, no_letter_contact_blocks, single_sms_sender, - mock_get_letter_organisations, + mock_get_letter_email_branding, mock_get_inbound_number_for_service, mock_get_free_sms_fragment_limit, ): diff --git a/tests/app/main/views/test_email_branding.py b/tests/app/main/views/test_email_branding.py new file mode 100644 index 000000000..c892d52a0 --- /dev/null +++ b/tests/app/main/views/test_email_branding.py @@ -0,0 +1,313 @@ +from io import BytesIO +from unittest.mock import call + +from bs4 import BeautifulSoup +from flask import url_for +import pytest + +from tests.conftest import ( + mock_get_email_branding, + normalize_spaces +) + +from app.main.s3_client import TEMP_TAG, LOGO_LOCATION_STRUCTURE + + +def test_email_branding_page_shows_full_branding_list( + logged_in_platform_admin_client, + mock_get_all_email_branding +): + + response = logged_in_platform_admin_client.get( + url_for('.email_branding') + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert normalize_spaces( + page.select_one('h1').text + ) == "Select an email branding to update or create a new email branding" + + first_label = page.select('div.multiple-choice > label')[0] + assert 'background: red;' in first_label.find('span')['style'] + assert normalize_spaces(first_label.text) == 'org 1' + assert first_label.find('img')['src'].endswith('/logo1.png') + + assert normalize_spaces((page.select('div.multiple-choice > label')[-1]).text) == 'Create a new email branding' + + +def test_edit_email_branding_shows_the_correct_branding_info( + logged_in_platform_admin_client, + mock_get_email_branding, + fake_uuid +): + response = logged_in_platform_admin_client.get( + url_for('.update_email_branding', branding_id=fake_uuid) + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.select_one('#logo-img > img')['src'].endswith('/example.png') + assert page.select_one('#name').attrs.get('value') == 'Organisation name' + assert page.select_one('#colour').attrs.get('value') == '#f00' + + +def test_create_email_branding_does_not_show_any_branding_info( + logged_in_platform_admin_client, + mock_no_email_branding +): + + response = logged_in_platform_admin_client.get( + url_for('.create_email_branding') + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.select_one('#logo-img > img') is None + assert page.select_one('#name').attrs.get('value') == '' + assert page.select_one('#colour').attrs.get('value') == '' + + +def test_create_new_email_branding_without_logo( + logged_in_platform_admin_client, + mocker, + fake_uuid, + mock_create_email_branding +): + data = { + 'logo': None, + 'colour': '#ff0000', + 'name': 'new name' + } + + mock_persist = mocker.patch('app.main.views.email_branding.persist_logo') + mocker.patch('app.main.views.email_branding.delete_temp_files_created_by') + + logged_in_platform_admin_client.post( + url_for('.create_email_branding'), + content_type='multipart/form-data', + data=data + ) + + assert mock_create_email_branding.called + assert mock_create_email_branding.call_args == call( + logo=data['logo'], + name=data['name'], + colour=data['colour'] + ) + assert mock_persist.call_args_list == [] + + +def test_create_new_email_branding_when_branding_saved( + logged_in_platform_admin_client, + mocker, + mock_create_email_branding, + fake_uuid +): + with logged_in_platform_admin_client.session_transaction() as session: + user_id = session["user_id"] + + data = { + 'logo': 'test.png', + 'colour': '#ff0000', + 'name': 'new name' + } + + temp_filename = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), + unique_id=fake_uuid, + filename=data['logo'] + ) + + mocker.patch('app.main.views.email_branding.persist_logo', return_value=data['logo']) + mocker.patch('app.main.views.email_branding.delete_temp_files_created_by') + + logged_in_platform_admin_client.post( + url_for('.create_email_branding', logo=temp_filename), + content_type='multipart/form-data', + data={ + 'colour': data['colour'], + 'name': data['name'], + 'cdn_url': 'https://static-logos.cdn.com' + } + ) + + assert mock_create_email_branding.called + assert mock_create_email_branding.call_args == call( + logo=data['logo'], + name=data['name'], + colour=data['colour'] + ) + + +@pytest.mark.parametrize('endpoint, has_data', [ + ('main.create_email_branding', False), + ('main.update_email_branding', True), +]) +def test_deletes_previous_temp_logo_after_uploading_logo( + logged_in_platform_admin_client, + mocker, + endpoint, + has_data, + fake_uuid +): + if has_data: + mock_get_email_branding(mocker, fake_uuid) + + with logged_in_platform_admin_client.session_transaction() as session: + user_id = session["user_id"] + + temp_old_filename = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), + unique_id=fake_uuid, + filename='old_test.png' + ) + + temp_filename = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), + unique_id=fake_uuid, + filename='test.png' + ) + + mocked_upload_logo = mocker.patch( + 'app.main.views.email_branding.upload_logo', + return_value=temp_filename + ) + + mocked_delete_temp_file = mocker.patch('app.main.views.email_branding.delete_temp_file') + + logged_in_platform_admin_client.post( + url_for('main.create_email_branding', logo=temp_old_filename, branding_id=fake_uuid), + data={'file': (BytesIO(''.encode('utf-8')), 'test.png')}, + content_type='multipart/form-data' + ) + + assert mocked_upload_logo.called + assert mocked_delete_temp_file.called + assert mocked_delete_temp_file.call_args == call(temp_old_filename) + + +def test_update_exisiting_branding( + logged_in_platform_admin_client, + mocker, + fake_uuid, + mock_get_email_branding, + mock_update_email_branding +): + with logged_in_platform_admin_client.session_transaction() as session: + user_id = session["user_id"] + + data = { + 'logo': 'test.png', + 'colour': '#0000ff', + 'name': 'new name' + } + + temp_filename = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), + unique_id=fake_uuid, + filename=data['logo'] + ) + + mocker.patch('app.main.views.email_branding.persist_logo', return_value=data['logo']) + mocker.patch('app.main.views.email_branding.delete_temp_files_created_by') + + logged_in_platform_admin_client.post( + url_for('.update_email_branding', logo=temp_filename, branding_id=fake_uuid), + content_type='multipart/form-data', + data={'colour': data['colour'], 'name': data['name'], 'cdn_url': 'https://static-logos.cdn.com'} + ) + + assert mock_update_email_branding.called + assert mock_update_email_branding.call_args == call( + branding_id=fake_uuid, + logo=data['logo'], + name=data['name'], + colour=data['colour'] + ) + + +def test_temp_logo_is_shown_after_uploading_logo( + logged_in_platform_admin_client, + mocker, + fake_uuid, +): + with logged_in_platform_admin_client.session_transaction() as session: + user_id = session["user_id"] + + temp_filename = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), + unique_id=fake_uuid, + filename='test.png' + ) + + mocker.patch('app.main.views.email_branding.upload_logo', return_value=temp_filename) + mocker.patch('app.main.views.email_branding.delete_temp_file') + + response = logged_in_platform_admin_client.post( + url_for('main.create_email_branding'), + data={'file': (BytesIO(''.encode('utf-8')), 'test.png')}, + content_type='multipart/form-data', + follow_redirects=True + ) + + assert response.status_code == 200 + + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.select_one('#logo-img > img').attrs['src'].endswith(temp_filename) + + +def test_logo_persisted_when_organisation_saved(logged_in_platform_admin_client, mocker, fake_uuid): + with logged_in_platform_admin_client.session_transaction() as session: + user_id = session["user_id"] + + temp_filename = LOGO_LOCATION_STRUCTURE.format( + temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename='test.png') + + mocked_upload_logo = mocker.patch('app.main.views.email_branding.upload_logo') + mocked_persist_logo = mocker.patch('app.main.views.email_branding.persist_logo', return_value='test.png') + mocked_delete_temp_files_by = mocker.patch('app.main.views.email_branding.delete_temp_files_created_by') + + logged_in_platform_admin_client.post( + url_for('.create_email_branding', logo=temp_filename), + content_type='multipart/form-data' + ) + + assert not mocked_upload_logo.called + assert mocked_persist_logo.called + assert mocked_delete_temp_files_by.called + assert mocked_delete_temp_files_by.call_args == call(user_id) + + +@pytest.mark.parametrize('colour_hex, form_validates', [ + ('#FF00FF', True), + ('hello', False), + ('', True), +]) +def test_colour_regex_validation( + logged_in_platform_admin_client, + mocker, + fake_uuid, + colour_hex, + form_validates, + mock_create_email_branding +): + data = { + 'logo': None, + 'colour': colour_hex, + 'name': 'new name' + } + + mocker.patch('app.main.views.email_branding.delete_temp_files_created_by') + + response = logged_in_platform_admin_client.post( + url_for('.create_email_branding'), + content_type='multipart/form-data', + data=data + ) + + assert (response.status_code == 302) == form_validates diff --git a/tests/app/main/views/test_organisations.py b/tests/app/main/views/test_organisations.py deleted file mode 100644 index 14828e655..000000000 --- a/tests/app/main/views/test_organisations.py +++ /dev/null @@ -1,281 +0,0 @@ -from io import BytesIO -from unittest.mock import call - -from bs4 import BeautifulSoup -from flask import url_for -import pytest - -from app.main.s3_client import TEMP_TAG, LOGO_LOCATION_STRUCTURE - -sample_orgs = [ - {'id': '1', 'name': 'org 1', 'colour': 'red', 'logo': 'logo1.png'}, - {'id': '2', 'name': 'org 2', 'colour': 'orange', 'logo': 'logo2.png'}, - {'id': '3', 'name': None, 'colour': None, 'logo': 'logo3.png'}, - {'id': '4', 'name': 'org 4', 'colour': None, 'logo': 'logo4.png'}, - {'id': '5', 'name': None, 'colour': 'blue', 'logo': 'logo5.png'}, -] - - -@pytest.fixture -def request_get_manage_org_with_org(logged_in_platform_admin_client): - with logged_in_platform_admin_client.session_transaction() as session: - session['organisation'] = sample_orgs[0] - - response = logged_in_platform_admin_client.get( - url_for('.manage_org') - ) - assert response.status_code == 200 - return BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - -@pytest.fixture -def request_get_manage_org_without_org(logged_in_platform_admin_client): - response = logged_in_platform_admin_client.get( - url_for('.manage_org') - ) - assert response.status_code == 200 - return BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - -def test_organisations_page_shows_full_orgs_list(logged_in_platform_admin_client, mocker): - mocker.patch('app.organisations_client.get_organisations', return_value=sample_orgs) - - response = logged_in_platform_admin_client.get( - url_for('.organisations') - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert ' '.join(page.find('h1').text.split()) == "Select an organisation to update or create a new organisation" - for index, label in enumerate(page.select('div.multiple-choice > label')): - if index < len(sample_orgs): - if sample_orgs[index]['colour']: - assert 'background: {};'.format(sample_orgs[index]['colour']) in label.find('span')['style'] - - assert ' '.join(label.text.split()) == str(sample_orgs[index]['name']) - assert label.find('img')['src'].endswith('/' + sample_orgs[index]['logo']) - else: - assert ' '.join(label.text.split()) == 'Create a new organisation' - - -@pytest.mark.parametrize("org_id", [ - 'None', '1', '2' -]) -def test_organisations_radio_default_to_just_updated_or_new_org( - logged_in_platform_admin_client, mocker, org_id): - mocker.patch('app.organisations_client.get_organisations', return_value=sample_orgs) - - response = logged_in_platform_admin_client.get( - url_for('.organisations', organisation_id=org_id) - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - selected = [r for r in page.select('div.multiple-choice > input') if r.attrs.get('checked')][0] - assert selected["value"] == org_id - - -def test_organisations_post_sets_organisation_in_session_after_selecting_org( - logged_in_platform_admin_client, mocker): - mocker.patch('app.organisations_client.get_organisations', return_value=sample_orgs) - response = logged_in_platform_admin_client.post( - url_for('.organisations'), - data={'organisation': sample_orgs[0]['id']} - ) - with logged_in_platform_admin_client.session_transaction() as session: - assert session['organisation'] == sample_orgs[0] - assert response.status_code == 302 - assert response.location == url_for('.manage_org', _external=True) - - -def test_organisations_post_deletes_organisation_session_on_new_org( - logged_in_platform_admin_client, mocker): - mocker.patch('app.organisations_client.get_organisations', return_value=sample_orgs) - with logged_in_platform_admin_client.session_transaction() as session: - session['organisation'] = sample_orgs[0] - - response = logged_in_platform_admin_client.post( - url_for('.organisations'), - data={'organisation': 'None'} - ) - - with logged_in_platform_admin_client.session_transaction() as session: - assert session.get('organisation') is None - assert response.status_code == 302 - assert response.location == url_for('.manage_org', _external=True) - - -def test_manage_orgs_shows_correct_org_info(request_get_manage_org_with_org): - assert request_get_manage_org_with_org.select_one('#logo-img > img')['src'].endswith('/' + sample_orgs[0]['logo']) - assert request_get_manage_org_with_org.select_one('#name').attrs.get('value') == sample_orgs[0]['name'] - assert request_get_manage_org_with_org.select_one('#colour').attrs.get('value') == sample_orgs[0]['colour'] - - -def test_manage_orgs_does_not_show_data_for_new_org(request_get_manage_org_without_org): - assert request_get_manage_org_without_org.select_one('#logo-img > img') is None - assert request_get_manage_org_without_org.select_one('#name').attrs.get('value') == '' - assert request_get_manage_org_without_org.select_one('#colour').attrs.get('value') == '' - - -@pytest.fixture -def request_post_manage_org_redirect(logged_in_platform_admin_client, mocker, fake_uuid): - with logged_in_platform_admin_client.session_transaction() as session: - user_id = session["user_id"] - - temp_filename = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename='test.png') - - mocker.patch('app.main.views.organisations.upload_logo', return_value=temp_filename) - mocker.patch('app.main.views.organisations.delete_temp_file') - mocker.patch('app.main.views.organisations.delete_temp_files_created_by') - - response = logged_in_platform_admin_client.post( - url_for('.manage_org'), - data={'file': (BytesIO(''.encode('utf-8')), 'test.png')}, - content_type='multipart/form-data', - follow_redirects=True - ) - - assert response.status_code == 200 - return BeautifulSoup(response.data.decode('utf-8'), 'html.parser'), temp_filename - - -def test_shows_temp_logo_after_uploading_logo(request_post_manage_org_redirect): - page, temp_filename = request_post_manage_org_redirect - assert page.select_one('#logo-img > img').attrs['src'].endswith(temp_filename) - - -def test_save_enabled_after_uploading_logo(request_post_manage_org_redirect): - page, _ = request_post_manage_org_redirect - assert not page.select_one('div.page-footer button.button').has_attr('disabled') - - -def test_deletes_previous_temp_logo_after_uploading_logo(logged_in_platform_admin_client, mocker, fake_uuid): - with logged_in_platform_admin_client.session_transaction() as session: - user_id = session["user_id"] - - temp_old_filename = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename='old_test.png') - temp_filename = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename='test.png') - - mocked_upload_logo = mocker.patch( - 'app.main.views.organisations.upload_logo', - return_value=temp_filename - ) - mocked_delete_temp_file = mocker.patch('app.main.views.organisations.delete_temp_file') - - logged_in_platform_admin_client.post( - url_for('.manage_org', logo=temp_old_filename), - data={'file': (BytesIO(''.encode('utf-8')), 'test.png')}, - content_type='multipart/form-data' - ) - - assert mocked_upload_logo.called - assert mocked_delete_temp_file.called - assert mocked_delete_temp_file.call_args == call(temp_old_filename) - - -def test_logo_persisted_when_organisation_saved(logged_in_platform_admin_client, mocker, fake_uuid): - with logged_in_platform_admin_client.session_transaction() as session: - user_id = session["user_id"] - - temp_filename = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename='test.png') - - mocked_upload_logo = mocker.patch('app.main.views.organisations.upload_logo') - mocked_persist_logo = mocker.patch('app.main.views.organisations.persist_logo', return_value='test.png') - mocked_delete_temp_files_by = mocker.patch('app.main.views.organisations.delete_temp_files_created_by') - - logged_in_platform_admin_client.post( - url_for('.manage_org', logo=temp_filename), - content_type='multipart/form-data' - ) - - assert not mocked_upload_logo.called - assert mocked_persist_logo.called - assert mocked_delete_temp_files_by.called - assert mocked_delete_temp_files_by.call_args == call(user_id) - - -def test_existing_organisation_updated_when_organisation_saved(logged_in_platform_admin_client, mocker, fake_uuid): - with logged_in_platform_admin_client.session_transaction() as session: - session["organisation"] = sample_orgs[0] - user_id = session["user_id"] - - update_org = {'logo': 'test.png', 'colour': 'blue', 'name': 'new name'} - - temp_filename = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename=update_org['logo']) - - mocked_update_org = mocker.patch('app.organisations_client.update_organisation') - mocker.patch('app.main.views.organisations.persist_logo', return_value=update_org['logo']) - mocker.patch('app.main.views.organisations.delete_temp_files_created_by') - - logged_in_platform_admin_client.post( - url_for('.manage_org', logo=temp_filename), - content_type='multipart/form-data', - data={'colour': update_org['colour'], 'name': update_org['name'], 'cdn_url': 'https://static-logos.cdn.com'} - ) - - assert mocked_update_org.called - assert mocked_update_org.call_args == call( - org_id=sample_orgs[0]['id'], - logo=update_org['logo'], - name=update_org['name'], - colour=update_org['colour'] - ) - - -def test_create_new_organisation_when_organisation_saved(logged_in_platform_admin_client, mocker, fake_uuid): - with logged_in_platform_admin_client.session_transaction() as session: - user_id = session["user_id"] - - new_org = {'logo': 'test.png', 'colour': 'red', 'name': 'new name'} - - temp_filename = LOGO_LOCATION_STRUCTURE.format( - temp=TEMP_TAG.format(user_id=user_id), unique_id=fake_uuid, filename=new_org['logo']) - - mocked_new_org = mocker.patch('app.organisations_client.create_organisation') - mocker.patch('app.main.views.organisations.persist_logo', return_value=new_org['logo']) - mocker.patch('app.main.views.organisations.delete_temp_files_created_by') - - logged_in_platform_admin_client.post( - url_for('.manage_org', logo=temp_filename), - content_type='multipart/form-data', - data={'colour': new_org['colour'], 'name': new_org['name'], 'cdn_url': 'https://static-logos.cdn.com'} - ) - - assert mocked_new_org.called - assert mocked_new_org.call_args == call( - logo=new_org['logo'], - name=new_org['name'], - colour=new_org['colour'] - ) - - -def test_create_new_organisation_without_logo(logged_in_platform_admin_client, mocker, fake_uuid): - - new_org = {'logo': None, 'colour': 'red', 'name': 'new name'} - - mocked_new_org = mocker.patch('app.organisations_client.create_organisation') - mock_persist = mocker.patch('app.main.views.organisations.persist_logo') - mocker.patch('app.main.views.organisations.delete_temp_files_created_by') - - logged_in_platform_admin_client.post( - url_for('.manage_org'), - content_type='multipart/form-data', - data={ - 'colour': new_org['colour'], - 'name': new_org['name'], - } - ) - - assert mocked_new_org.called - assert mocked_new_org.call_args == call( - logo=new_org['logo'], - name=new_org['name'], - colour=new_org['colour'] - ) - assert mock_persist.call_args_list == [] diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index efd719c0f..ba31594c2 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -33,7 +33,7 @@ from freezegun import freeze_time @pytest.fixture def mock_get_service_settings_page_common( - mock_get_letter_organisations, + mock_get_letter_email_branding, mock_get_inbound_number_for_service, mock_get_free_sms_fragment_limit, ): @@ -169,7 +169,7 @@ def test_should_show_overview_for_service_with_more_things_set( single_reply_to_email_address, single_letter_contact_block, single_sms_sender, - mock_get_organisation, + mock_get_email_branding, mock_get_service_settings_page_common, permissions, expected_rows @@ -589,7 +589,7 @@ def test_route_for_platform_admin_update_service( client, platform_admin_user, service_one, - mock_get_letter_organisations, + mock_get_letter_email_branding, route, ): mocker.patch('app.service_api_client.archive_service') @@ -1342,7 +1342,7 @@ def test_set_letter_branding_platform_admin_only( def test_set_letter_branding_prepopulates( logged_in_platform_admin_client, service_one, - mock_get_letter_organisations, + mock_get_letter_email_branding, current_dvla_org_id, expected_selected, ): @@ -1358,7 +1358,7 @@ def test_set_letter_branding_saves( logged_in_platform_admin_client, service_one, mock_update_service, - mock_get_letter_organisations, + mock_get_letter_email_branding, ): response = logged_in_platform_admin_client.post( url_for('main.set_letter_branding', service_id=service_one['id']), @@ -1372,11 +1372,11 @@ def test_set_letter_branding_saves( def test_should_show_branding( logged_in_platform_admin_client, service_one, - mock_get_organisations, - mock_get_letter_organisations, + mock_get_all_email_branding, + mock_get_letter_email_branding, ): response = logged_in_platform_admin_client.get(url_for( - 'main.service_set_branding_and_org', service_id=service_one['id'] + 'main.service_set_email_branding', service_id=service_one['id'] )) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -1391,17 +1391,17 @@ def test_should_show_branding( assert 'checked' not in page.find('input', attrs={"id": "branding_type-2"}).attrs assert 'checked' not in page.find('input', attrs={"id": "branding_type-3"}).attrs - app.organisations_client.get_organisations.assert_called_once_with() + app.email_branding_client.get_all_email_branding.assert_called_once_with() app.service_api_client.get_service.assert_called_once_with(service_one['id']) def test_should_show_organisations( logged_in_platform_admin_client, service_one, - mock_get_organisations, + mock_get_all_email_branding, ): response = logged_in_platform_admin_client.get(url_for( - 'main.service_set_branding_and_org', service_id=service_one['id'] + 'main.service_set_email_branding', service_id=service_one['id'] )) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') @@ -1416,33 +1416,33 @@ def test_should_show_organisations( assert 'checked' not in page.find('input', attrs={"id": "branding_type-2"}).attrs assert 'checked' not in page.find('input', attrs={"id": "branding_type-3"}).attrs - app.organisations_client.get_organisations.assert_called_once_with() + app.email_branding_client.get_all_email_branding.assert_called_once_with() app.service_api_client.get_service.assert_called_once_with(service_one['id']) def test_should_set_branding_and_organisations( logged_in_platform_admin_client, service_one, - mock_get_organisations, + mock_get_all_email_branding, mock_update_service, ): response = logged_in_platform_admin_client.post( url_for( - 'main.service_set_branding_and_org', service_id=service_one['id'] + 'main.service_set_email_branding', service_id=service_one['id'] ), data={ 'branding_type': 'org', - 'organisation': 'organisation-id' + 'organisation': '1' } ) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - mock_get_organisations.assert_called_once_with() + mock_get_all_email_branding.assert_called_once_with() mock_update_service.assert_called_once_with( service_one['id'], branding='org', - organisation='organisation-id' + email_branding=None ) @@ -1873,7 +1873,7 @@ def test_service_settings_when_inbound_number_is_not_set( single_letter_contact_block, single_sms_sender, mocker, - mock_get_letter_organisations, + mock_get_letter_email_branding, mock_get_free_sms_fragment_limit, ): mocker.patch('app.inbound_number_client.get_inbound_sms_number_for_service', @@ -1890,7 +1890,7 @@ def test_set_inbound_sms_when_inbound_number_is_not_set( single_reply_to_email_address, single_letter_contact_block, mocker, - mock_get_letter_organisations, + mock_get_letter_email_branding, ): mocker.patch('app.inbound_number_client.get_inbound_sms_number_for_service', return_value={'data': {}}) diff --git a/tests/app/notify_client/test_email_branding_client.py b/tests/app/notify_client/test_email_branding_client.py new file mode 100644 index 000000000..64d658d4d --- /dev/null +++ b/tests/app/notify_client/test_email_branding_client.py @@ -0,0 +1,50 @@ +from app.notify_client.email_branding_client import EmailBrandingClient + + +def test_get_email_branding(mocker, fake_uuid): + mock_get = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.get') + EmailBrandingClient().get_email_branding(fake_uuid) + mock_get.assert_called_once_with( + url='/email-branding/{}'.format(fake_uuid) + ) + + +def test_get_all_email_branding(mocker): + mock_get = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.get') + EmailBrandingClient().get_all_email_branding() + mock_get.assert_called_once_with( + url='/email-branding' + ) + + +def test_get_letter_email_branding(mocker): + mock_get = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.get') + EmailBrandingClient().get_letter_email_branding() + mock_get.assert_called_once_with( + url='/dvla_organisations' + ) + + +def test_create_email_branding(mocker): + org_data = {'logo': 'test.png', 'name': 'test name', 'colour': 'red'} + + mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post') + EmailBrandingClient().create_email_branding(logo=org_data['logo'], name=org_data['name'], colour=org_data['colour']) + + mock_post.assert_called_once_with( + url='/email-branding', + data=org_data + ) + + +def test_update_email_branding(mocker, fake_uuid): + org_data = {'logo': 'test.png', 'name': 'test name', 'colour': 'red'} + + mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post') + EmailBrandingClient().update_email_branding( + branding_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], colour=org_data['colour']) + + mock_post.assert_called_once_with( + url='/email-branding/{}'.format(fake_uuid), + data=org_data + ) diff --git a/tests/app/notify_client/test_organisations_client.py b/tests/app/notify_client/test_organisations_client.py deleted file mode 100644 index a0eaa3d1c..000000000 --- a/tests/app/notify_client/test_organisations_client.py +++ /dev/null @@ -1,50 +0,0 @@ -from app.notify_client.organisations_client import OrganisationsClient - - -def test_get_organisation(mocker, fake_uuid): - mock_get = mocker.patch('app.notify_client.organisations_client.OrganisationsClient.get') - OrganisationsClient().get_organisation(fake_uuid) - mock_get.assert_called_once_with( - url='/organisation/{}'.format(fake_uuid) - ) - - -def test_get_organisations(mocker): - mock_get = mocker.patch('app.notify_client.organisations_client.OrganisationsClient.get') - OrganisationsClient().get_organisations() - mock_get.assert_called_once_with( - url='/organisation' - ) - - -def test_get_letter_organisations(mocker): - mock_get = mocker.patch('app.notify_client.organisations_client.OrganisationsClient.get') - OrganisationsClient().get_letter_organisations() - mock_get.assert_called_once_with( - url='/dvla_organisations' - ) - - -def test_create_organisations(mocker): - org_data = {'logo': 'test.png', 'name': 'test name', 'colour': 'red'} - - mock_post = mocker.patch('app.notify_client.organisations_client.OrganisationsClient.post') - OrganisationsClient().create_organisation(logo=org_data['logo'], name=org_data['name'], colour=org_data['colour']) - - mock_post.assert_called_once_with( - url='/organisation', - data=org_data - ) - - -def test_update_organisations(mocker, fake_uuid): - org_data = {'logo': 'test.png', 'name': 'test name', 'colour': 'red'} - - mock_post = mocker.patch('app.notify_client.organisations_client.OrganisationsClient.post') - OrganisationsClient().update_organisation( - org_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], colour=org_data['colour']) - - mock_post.assert_called_once_with( - url='/organisation/{}'.format(fake_uuid), - data=org_data - ) diff --git a/tests/conftest.py b/tests/conftest.py index 6eb9f75a0..eb6435a68 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2246,49 +2246,78 @@ def mock_send_already_registered_email(mocker): @pytest.fixture(scope='function') -def mock_get_organisations(mocker): - def _get_organisations(): +def mock_get_all_email_branding(mocker): + def _get_all_email_branding(): return [ - { - 'logo': 'example.png', - 'name': 'Organisation name', - 'id': 'organisation-id', - 'colour': '#f00' - } + {'id': '1', 'name': 'org 1', 'colour': 'red', 'logo': 'logo1.png'}, + {'id': '2', 'name': 'org 2', 'colour': 'orange', 'logo': 'logo2.png'}, + {'id': '3', 'name': None, 'colour': None, 'logo': 'logo3.png'}, + {'id': '4', 'name': 'org 4', 'colour': None, 'logo': 'logo4.png'}, + {'id': '5', 'name': None, 'colour': 'blue', 'logo': 'logo5.png'}, ] return mocker.patch( - 'app.organisations_client.get_organisations', side_effect=_get_organisations + 'app.email_branding_client.get_all_email_branding', side_effect=_get_all_email_branding ) @pytest.fixture(scope='function') -def mock_get_letter_organisations(mocker): - def _get_organisations(): +def mock_get_letter_email_branding(mocker): + def _get_letter_email_branding(): return { '001': 'HM Government', '500': 'Land Registry', } return mocker.patch( - 'app.organisations_client.get_letter_organisations', side_effect=_get_organisations + 'app.email_branding_client.get_letter_email_branding', side_effect=_get_letter_email_branding ) @pytest.fixture(scope='function') -def mock_get_organisation(mocker): - def _get_organisation(id): +def mock_no_email_branding(mocker): + def _get_email_branding(): + return [] + + return mocker.patch( + 'app.email_branding_client.get_letter_email_branding', side_effect=_get_email_branding + ) + + +@pytest.fixture(scope='function') +def mock_get_email_branding(mocker, fake_uuid): + def _get_email_branding(id): return { - 'organisation': { + 'email_branding': { 'logo': 'example.png', 'name': 'Organisation name', - 'id': 'organisation-id', + 'id': fake_uuid, 'colour': '#f00' } } return mocker.patch( - 'app.organisations_client.get_organisation', side_effect=_get_organisation + 'app.email_branding_client.get_email_branding', side_effect=_get_email_branding + ) + + +@pytest.fixture(scope='function') +def mock_create_email_branding(mocker): + def _create_email_branding(logo, name, colour): + return + + return mocker.patch( + 'app.email_branding_client.create_email_branding', side_effect=_create_email_branding + ) + + +@pytest.fixture(scope='function') +def mock_update_email_branding(mocker): + def _update_email_branding(branding_id, logo, name, colour): + return + + return mocker.patch( + 'app.email_branding_client.update_email_branding', side_effect=_update_email_branding )