From 6e1bcb84050ec777198b1fe76cd8c1b69b60a2d1 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Mon, 24 Jul 2017 15:20:40 +0100 Subject: [PATCH 01/28] Update emails to use logos cdn --- app/__init__.py | 3 +++ app/config.py | 8 ++++++++ app/utils.py | 10 ++++++++++ tests/app/test_utils.py | 10 ++++++++++ 4 files changed, 31 insertions(+) diff --git a/app/__init__.py b/app/__init__.py index 9ff29af79..85f3f0052 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -55,8 +55,11 @@ from app.notify_client.organisations_client import OrganisationsClient from app.notify_client.models import AnonymousUser from app.notify_client.letter_jobs_client import LetterJobsClient from app.utils import get_cdn_domain +<<<<<<< HEAD from app.utils import gmt_timezones +======= +>>>>>>> Update emails to use logos cdn login_manager = LoginManager() csrf = CSRFProtect() diff --git a/app/config.py b/app/config.py index eabd7391f..77a3e0781 100644 --- a/app/config.py +++ b/app/config.py @@ -87,6 +87,8 @@ class Config(object): r"cqc\.org\.uk", ] + LOGO_UPLOAD_BUCKET_NAME = 'local-notifications-logo-upload' + class Development(Config): DEBUG = True @@ -94,6 +96,7 @@ class Development(Config): SESSION_PROTECTION = None STATSD_ENABLED = False CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' + LOGO_UPLOAD_BUCKET_NAME = 'development-notifications-logo-upload' class Test(Development): @@ -102,6 +105,7 @@ class Test(Development): STATSD_ENABLED = True WTF_CSRF_ENABLED = False CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' + LOGO_UPLOAD_BUCKET_NAME = 'test-notifications-logo-upload' NOTIFY_ENVIRONMENT = 'test' TEMPLATE_PREVIEW_API_HOST = 'http://localhost:9999' @@ -111,6 +115,7 @@ class Preview(Config): HEADER_COLOUR = '#F499BE' # $baby-pink STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' + LOGO_UPLOAD_BUCKET_NAME = 'preview-notifications-logo-upload' NOTIFY_ENVIRONMENT = 'preview' @@ -120,6 +125,7 @@ class Staging(Config): HEADER_COLOUR = '#6F72AF' # $mauve STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload' + LOGO_UPLOAD_BUCKET_NAME = 'staging-notifications-logo-upload' NOTIFY_ENVIRONMENT = 'staging' @@ -129,6 +135,7 @@ class Live(Config): HTTP_PROTOCOL = 'https' STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' + LOGO_UPLOAD_BUCKET_NAME = 'live-notifications-logo-upload' NOTIFY_ENVIRONMENT = 'live' @@ -142,6 +149,7 @@ class Sandbox(CloudFoundryConfig): HEADER_COLOUR = '#F499BE' # $baby-pink STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'cf-sandbox-notifications-csv-upload' + LOGO_UPLOAD_BUCKET_NAME = 'cf-sandbox-notifications-logo-upload' NOTIFY_ENVIRONMENT = 'sandbox' diff --git a/app/utils.py b/app/utils.py index a0c8747e6..23b4d2b3a 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,14 +1,21 @@ import re import csv +<<<<<<< HEAD import pytz +======= +from datetime import datetime, timedelta, timezone +>>>>>>> Update emails to use logos cdn from io import StringIO from os import path from functools import wraps import unicodedata from urllib.parse import urlparse +<<<<<<< HEAD from collections import namedtuple from datetime import datetime, timedelta, timezone from dateutil import parser +======= +>>>>>>> Update emails to use logos cdn import dateutil import ago @@ -330,6 +337,7 @@ def email_or_sms_not_enabled(template_type, permissions): return (template_type in ['email', 'sms']) and (template_type not in permissions) +<<<<<<< HEAD def get_letter_timings(upload_time): LetterTimings = namedtuple( @@ -367,6 +375,8 @@ def gmt_timezones(date): return forced_utc.astimezone(pytz.timezone('Europe/London')) +======= +>>>>>>> Update emails to use logos cdn def get_cdn_domain(): parsed_uri = urlparse(current_app.config['ADMIN_BASE_URL']) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 980f7a6c7..8e2269e32 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -12,7 +12,10 @@ from app.utils import ( generate_previous_dict, generate_next_dict, Spreadsheet, +<<<<<<< HEAD get_letter_timings, +======= +>>>>>>> Update emails to use logos cdn get_cdn_domain ) @@ -156,6 +159,7 @@ def test_generate_notifications_csv_calls_twice_if_next_link(mocker): assert mock_get_notifications.mock_calls[1][2]['page'] == 2 +<<<<<<< HEAD @freeze_time('2017-07-14 14:59:59') # Friday, before print deadline @pytest.mark.parametrize('upload_time, expected_print_time, is_printed, expected_earliest, expected_latest', [ @@ -290,13 +294,19 @@ def test_get_estimated_delivery_date_for_letter( assert timings.latest_delivery.strftime('%A %Y-%m-%d') == expected_latest +======= +>>>>>>> Update emails to use logos cdn def test_get_cdn_domain_on_localhost(client, mocker): mocker.patch.dict('app.current_app.config', values={'ADMIN_BASE_URL': 'http://localhost:6012'}) domain = get_cdn_domain() assert domain == 'static-logos.notify.tools' +<<<<<<< HEAD def test_get_cdn_domain_on_non_localhost(client, mocker): +======= +def test_get_cdn_domain_without_logo_base_domain_env_returns_admin_base_domain(client, mocker): +>>>>>>> Update emails to use logos cdn mocker.patch.dict('app.current_app.config', values={'ADMIN_BASE_URL': 'https://some.admintest.com'}) domain = get_cdn_domain() assert domain == 'static-logos.admintest.com' From 005c8b53d78893dc1702856cffe14ac754f567de Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Fri, 28 Jul 2017 15:17:50 +0100 Subject: [PATCH 02/28] Add org select and manage pages --- app/config.py | 2 +- app/main/__init__.py | 1 + app/main/forms.py | 23 ++ app/main/s3_client.py | 95 +++++++ app/main/uploader.py | 32 --- app/main/views/organisations.py | 98 +++++++ app/main/views/send.py | 2 +- app/notify_client/organisations_client.py | 16 ++ app/templates/admin_template.html | 12 + app/templates/components/page-footer.html | 5 +- app/templates/components/radios.html | 15 +- .../views/organisations/manage-org.html | 55 ++++ .../views/organisations/select-org.html | 26 ++ requirements.txt | 4 + tests/app/main/views/test_organisations.py | 241 ++++++++++++++++++ 15 files changed, 585 insertions(+), 42 deletions(-) create mode 100644 app/main/s3_client.py delete mode 100644 app/main/uploader.py create mode 100644 app/main/views/organisations.py create mode 100644 app/templates/views/organisations/manage-org.html create mode 100644 app/templates/views/organisations/select-org.html create mode 100644 tests/app/main/views/test_organisations.py diff --git a/app/config.py b/app/config.py index 77a3e0781..d9f82ad6a 100644 --- a/app/config.py +++ b/app/config.py @@ -96,7 +96,7 @@ class Development(Config): SESSION_PROTECTION = None STATSD_ENABLED = False CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' - LOGO_UPLOAD_BUCKET_NAME = 'development-notifications-logo-upload' + LOGO_UPLOAD_BUCKET_NAME = 'public-logos-tools' class Test(Development): diff --git a/app/main/__init__.py b/app/main/__init__.py index 506d07858..84f2b1d0b 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -28,6 +28,7 @@ from app.main.views import ( providers, platform_admin, letter_jobs, + organisations, conversation, notifications ) diff --git a/app/main/forms.py b/app/main/forms.py index 0b92466dd..1709a7171 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -25,6 +25,7 @@ from wtforms import ( SelectField) from wtforms.fields.html5 import EmailField, TelField, SearchField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) +from flask_wtf.file import FileField as FileField_wtf, FileAllowed from app.main.validators import (Blacklist, CsvFileValidator, ValidGovEmail, NoCommasInPlaceHolders, OnlyGSMCharacters) @@ -559,6 +560,28 @@ class ServiceBrandingOrg(Form): ) +class ServiceSelectOrg(Form): + + def __init__(self, organisations=[], *args, **kwargs): + self.organisation.choices = organisations + super(ServiceSelectOrg, self).__init__(*args, **kwargs) + + organisation = RadioField( + 'Organisation', + validators=[ + DataRequired() + ] + ) + + +class ServiceManageOrg(Form): + + name = StringField('Name') + + colour = StringField('Colour', render_kw={'onkeyup': 'update_colour_span()', 'onblur': 'update_colour_span()'}) + file = FileField_wtf('Upload a PNG logo', validators=[FileAllowed(['png'], 'PNG Images only!')]) + + class LetterBranding(Form): def __init__(self, choices=[], *args, **kwargs): diff --git a/app/main/s3_client.py b/app/main/s3_client.py new file mode 100644 index 000000000..93764348d --- /dev/null +++ b/app/main/s3_client.py @@ -0,0 +1,95 @@ +import uuid +import botocore +from boto3 import resource, client +from flask import current_app +from notifications_utils.s3 import s3upload as utils_s3upload + +FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' +TEMP_TAG = 'temp-{}_' +LOGO_LOCATION_STRUCTURE = '{}{}-{}' + + +def s3upload(service_id, filedata, region): + upload_id = str(uuid.uuid4()) + upload_file_name = FILE_LOCATION_STRUCTURE.format(service_id, upload_id) + utils_s3upload(filedata=filedata['data'], + region=region, + bucket_name=current_app.config['CSV_UPLOAD_BUCKET_NAME'], + file_location=upload_file_name) + return upload_id + + +def s3download(service_id, upload_id): + contents = '' + try: + s3 = resource('s3') + bucket_name = current_app.config['CSV_UPLOAD_BUCKET_NAME'] + upload_file_name = FILE_LOCATION_STRUCTURE.format(service_id, upload_id) + key = s3.Object(bucket_name, upload_file_name) + contents = key.get()['Body'].read().decode('utf-8') + except botocore.exceptions.ClientError as e: + current_app.logger.error("Unable to download s3 file {}".format( + FILE_LOCATION_STRUCTURE.format(service_id, upload_id))) + raise e + return contents + + +def upload_logo(filename, filedata, region, user_id): + upload_id = str(uuid.uuid4()) + upload_file_name = LOGO_LOCATION_STRUCTURE.format(TEMP_TAG.format(user_id), upload_id, filename) + utils_s3upload(filedata=filedata, + region=region, + bucket_name=current_app.config['LOGO_UPLOAD_BUCKET_NAME'], + file_location=upload_file_name, + content_type='image/png') + return upload_file_name + + +def persist_logo(filename, user_id): + try: + if filename.startswith(TEMP_TAG.format(user_id)): + persisted_filename = filename[len(TEMP_TAG.format(user_id)):] + else: + return filename + + s3 = resource('s3') + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + + s3.Object(bucket_name, persisted_filename).copy_from(CopySource='{}/{}'.format(bucket_name, filename)) + s3.Object(bucket_name, filename).delete() + + return persisted_filename + except botocore.exceptions.ClientError as e: + current_app.logger.error("Unable to get s3 bucket contents {}".format( + bucket_name)) + raise e + + +def delete_temp_files_created_by(user_id): + try: + s3 = resource('s3') + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + + for obj in s3.Bucket(bucket_name).objects.filter(Prefix=TEMP_TAG.format(user_id)): + s3.Object(bucket_name, obj.key).delete() + + except botocore.exceptions.ClientError as e: + current_app.logger.error("Unable to delete s3 bucket temp files created by {} from {}".format( + user_id, bucket_name)) + raise e + + +def delete_temp_file(filename): + try: + if not filename.startswith(TEMP_TAG): + raise ValueError('Not a temp file') + + s3 = resource('s3') + bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] + + s3.Object(bucket_name, filename).delete() + + except botocore.exceptions.ClientError as e: + current_app.logger.error("Unable to delete s3 bucket file {} from {}".format( + filename, bucket_name)) + raise e diff --git a/app/main/uploader.py b/app/main/uploader.py deleted file mode 100644 index 4c00fbce5..000000000 --- a/app/main/uploader.py +++ /dev/null @@ -1,32 +0,0 @@ -import uuid -import botocore -from boto3 import resource -from flask import current_app -from notifications_utils.s3 import s3upload as utils_s3upload - -FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' - - -def s3upload(service_id, filedata, region): - upload_id = str(uuid.uuid4()) - upload_file_name = FILE_LOCATION_STRUCTURE.format(service_id, upload_id) - utils_s3upload(filedata=filedata['data'], - region=region, - bucket_name=current_app.config['CSV_UPLOAD_BUCKET_NAME'], - file_location=upload_file_name) - return upload_id - - -def s3download(service_id, upload_id): - contents = '' - try: - s3 = resource('s3') - bucket_name = current_app.config['CSV_UPLOAD_BUCKET_NAME'] - upload_file_name = FILE_LOCATION_STRUCTURE.format(service_id, upload_id) - key = s3.Object(bucket_name, upload_file_name) - contents = key.get()['Body'].read().decode('utf-8') - except botocore.exceptions.ClientError as e: - current_app.logger.error("Unable to download s3 file {}".format( - FILE_LOCATION_STRUCTURE.format(service_id, upload_id))) - raise e - return contents diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py new file mode 100644 index 000000000..b6a2c7127 --- /dev/null +++ b/app/main/views/organisations.py @@ -0,0 +1,98 @@ +from flask import current_app, redirect, render_template, session, url_for, request +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(session['user_id'])): + delete_temp_file(logo) + + return redirect( + url_for('.manage_org', logo=upload_filename)) + + 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/send.py b/app/main/views/send.py index 557faf0ba..354c5c8e2 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -33,7 +33,7 @@ from app.main.forms import ( ChooseTimeForm, get_placeholder_form_instance ) -from app.main.uploader import ( +from app.main.s3_client import ( s3upload, s3download ) diff --git a/app/notify_client/organisations_client.py b/app/notify_client/organisations_client.py index bf4717976..f75719a42 100644 --- a/app/notify_client/organisations_client.py +++ b/app/notify_client/organisations_client.py @@ -19,3 +19,19 @@ class OrganisationsClient(NotifyAdminAPIClient): def get_letter_organisations(self): return self.get(url='/dvla_organisations') + + def create_organisation(self, logo, name, colour): + data = { + "logo": logo, + "name": name, + "colour": colour + } + return self.post("/organisation", data) + + def update_organisation(self, org_id, logo, name, colour): + data = { + "logo": logo, + "name": name, + "colour": colour + } + return self.post("/organisation/{}".format(org_id), data) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 215563caa..5be2db3b7 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -49,6 +49,18 @@
  • Platform admin
    • +<<<<<<< HEAD +======= +
  • + Providers +
    • +
  • + Organisations +
    • +
  • + Letter jobs +
    • +>>>>>>> Add org select and manage pages {% endif %}
  • Sign out diff --git a/app/templates/components/page-footer.html b/app/templates/components/page-footer.html index 5dcc69e4f..74b847a0c 100644 --- a/app/templates/components/page-footer.html +++ b/app/templates/components/page-footer.html @@ -6,12 +6,13 @@ secondary_link=False, secondary_link_text=None, delete_link=False, - delete_link_text="delete" + delete_link_text="delete", + button_disabled=False ) %}
    {% if button_text %} - + {% endif %} {% if back_link %} {{ back_link_text }} diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index f00ba49b4..fad33d8fe 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -79,16 +79,19 @@ {% macro branding_radios( field, hint=None, - branding_dict={} + branding_dict={}, + show_header=True ) %}
    - {{ field.label.text }} + {% if show_header %} + {{ field.label.text }} + {% endif %} {% if field.errors %} - - {{ field.errors[0] }} - + + {{ field.errors[0] }} + {% endif %} {% for value, option, checked in field.iter_choices() %} @@ -102,7 +105,7 @@ />
    @@ -100,7 +101,6 @@
  • Trial mode
  • Pricing
  • Delivery and failure
    • -
  • Performance
    • From 2eab6bf78108f3afe4939c3ef24f0c3fa753ddf1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Aug 2017 15:13:36 +0100 Subject: [PATCH 24/28] Fix logo bucket name The app was uploading into ...live but then looking for the logos in ...production. The existing logos are in ...production --- app/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 4232e2411..6664ea46b 100644 --- a/app/config.py +++ b/app/config.py @@ -135,7 +135,7 @@ class Live(Config): HTTP_PROTOCOL = 'https' STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' - LOGO_UPLOAD_BUCKET_NAME = 'public-logos-live' + LOGO_UPLOAD_BUCKET_NAME = 'public-logos-production' NOTIFY_ENVIRONMENT = 'live' From 6f1fd0f2ccdba7d9e22172955fbeb669ac03d606 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Wed, 9 Aug 2017 16:48:23 +0100 Subject: [PATCH 25/28] Update pytest-xdist from 1.18.2 to 1.19.0 --- requirements_for_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index f7d0e726e..ae450ef36 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -3,7 +3,7 @@ pycodestyle==2.3.1 pytest==3.2.0 pytest-mock==1.6.2 pytest-cov==2.5.1 -pytest-xdist==1.18.2 +pytest-xdist==1.19.0 coveralls==1.1 moto==1.0.1 httpretty==0.8.14 From 9b9c6d75dcbc99a8103935af01f1e60fc7dc746d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 9 Aug 2017 16:52:10 +0100 Subject: [PATCH 26/28] - Change update service name to check that the name/email_from is unique across all services. - This is done using a new endpoint in the api. - Removed the AddServiceForm in favor or using the ServiceNameForm - Removed ServiceApiClient.find_all_service_email_from --- app/main/forms.py | 26 +------------------ app/main/views/add_service.py | 4 +-- app/main/views/service_settings.py | 6 ++++- app/notify_client/service_api_client.py | 10 ++++--- tests/app/main/views/test_service_settings.py | 10 +++---- tests/conftest.py | 10 +++++++ 6 files changed, 29 insertions(+), 37 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index bc334ab41..45ba28782 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -212,26 +212,8 @@ class TextNotReceivedForm(Form): mobile_number = mobile_number() -class AddServiceForm(Form): - def __init__(self, *args, **kwargs): - super(AddServiceForm, self).__init__(*args, **kwargs) - - name = StringField( - 'Service name', - validators=[ - DataRequired(message='Can’t be empty') - ] - ) - - class ServiceNameForm(Form): - def __init__(self, names_func, *args, **kwargs): - """ - Keyword arguments: - names_func -- Returns a list of unique service_names already registered - on the system. - """ - self._names_func = names_func + def __init__(self, *args, **kwargs): super(ServiceNameForm, self).__init__(*args, **kwargs) name = StringField( @@ -240,12 +222,6 @@ class ServiceNameForm(Form): DataRequired(message='Can’t be empty') ]) - def validate_name(self, a): - from app.utils import email_safe - # make sure the email_from will be unique to all services - if email_safe(a.data) in self._names_func(): - raise ValidationError('This service name is already in use') - class ConfirmPasswordForm(Form): def __init__(self, validate_password_func, *args, **kwargs): diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index fc7f43a22..9b7d5b9fd 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -14,7 +14,7 @@ from notifications_python_client.errors import HTTPError from werkzeug.exceptions import abort from app.main import main -from app.main.forms import AddServiceForm +from app.main.forms import ServiceNameForm from app.notify_client.models import InvitedUser from app import ( @@ -78,7 +78,7 @@ def add_service(): if not is_gov_user(current_user.email_address): abort(403) - form = AddServiceForm() + form = ServiceNameForm() heading = 'Which service do you want to set up notifications for?' if form.validate_on_submit(): diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 7e694cb06..cc5017c99 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -81,12 +81,16 @@ def service_settings(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_name_change(service_id): - form = ServiceNameForm(service_api_client.find_all_service_email_from) + form = ServiceNameForm() if request.method == 'GET': form.name.data = current_service.get('name') if form.validate_on_submit(): + unique_name = service_api_client.is_service_name_unique(form.name.data, email_safe(form.name.data)) + if not unique_name: + form.name.errors.append("This service name is already in use") + return render_template('views/service-settings/name.html', form=form) session['service_name_change'] = form.name.data return redirect(url_for('.service_name_change_confirm', service_id=service_id)) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index b72fe77c9..2173e3402 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -216,9 +216,13 @@ class ServiceAPIClient(NotifyAdminAPIClient): data = _attach_current_user(data) return self.post(endpoint, data=data) - def find_all_service_email_from(self, user_id=None): - resp = self.get_services(user_id) - return [x['email_from'] for x in resp['data']] + def is_service_name_unique(self, name, email_from): + """ + Check that the service name or email from are unique across all services. + """ + endpoint = "/service/unique" + params = {"name": name, "email_from": email_from} + return self.get(url=endpoint, params=params)["result"] # Temp access of service history data. Includes service and api key history def get_service_history(self, service_id): diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 606d4c2c8..8e3281ddf 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -227,7 +227,7 @@ def test_should_redirect_after_change_service_name( logged_in_client, service_one, mock_update_service, - mock_get_services, + mock_service_name_is_unique ): response = logged_in_client.post( url_for('main.service_name_change', service_id=service_one['id']), @@ -237,7 +237,7 @@ def test_should_redirect_after_change_service_name( settings_url = url_for( 'main.service_name_change_confirm', service_id=service_one['id'], _external=True) assert settings_url == response.location - assert mock_get_services.called + assert mock_service_name_is_unique.called def test_show_restricted_service( @@ -302,11 +302,9 @@ def test_switch_service_to_restricted( def test_should_not_allow_duplicate_names( logged_in_client, - mocker, + mock_service_name_is_not_unique, service_one, ): - mocker.patch('app.service_api_client.find_all_service_email_from', - return_value=['service_one', 'service.two']) service_id = service_one['id'] response = logged_in_client.post( url_for('main.service_name_change', service_id=service_id), @@ -315,7 +313,7 @@ def test_should_not_allow_duplicate_names( assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert 'This service name is already in use' in resp_data - app.service_api_client.find_all_service_email_from.assert_called_once_with() + app.service_api_client.is_service_name_unique.assert_called_once_with('SErvICE TWO', 'service.two') def test_should_show_service_name_confirmation( diff --git a/tests/conftest.py b/tests/conftest.py index 45364f807..255f3a62c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -150,6 +150,16 @@ def mock_get_detailed_services(mocker, fake_uuid): return mocker.patch('app.service_api_client.get_services', return_value=services) +@pytest.fixture(scope='function') +def mock_service_name_is_not_unique(mocker): + return mocker.patch('app.service_api_client.is_service_name_unique', return_value=False) + + +@pytest.fixture(scope='function') +def mock_service_name_is_unique(mocker): + return mocker.patch('app.service_api_client.is_service_name_unique', return_value=True) + + @pytest.fixture(scope='function') def mock_get_live_service(mocker, api_user_active): def _get(service_id): From 3f6996334ff784a4eae27fec3501798d2e557bd6 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Wed, 9 Aug 2017 22:09:24 +0100 Subject: [PATCH 27/28] Update pytest from 3.2.0 to 3.2.1 --- requirements_for_test.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index ae450ef36..f6733c8eb 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,6 +1,6 @@ -r requirements.txt pycodestyle==2.3.1 -pytest==3.2.0 +pytest==3.2.1 pytest-mock==1.6.2 pytest-cov==2.5.1 pytest-xdist==1.19.0 From 01d92fdbb37826352f3ab92b9140bae7246fc144 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 10 Aug 2017 12:47:56 +0100 Subject: [PATCH 28/28] Add British Library to email domain whitelist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think that British Library can use Notify. Need to double check though. > BL is an executive non-departmental public body, sponsored by the > Department for Digital, Culture, Media & Sport. > https://www.bl.uk/ — https://www.gov.uk/government/organisations/british-library --- app/config.py | 1 + tests/app/main/test_validators.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 6664ea46b..d05be8647 100644 --- a/app/config.py +++ b/app/config.py @@ -85,6 +85,7 @@ class Config(object): r"assembly\.wales", r"cjsm\.net", r"cqc\.org\.uk", + r"bl\.uk", ] LOGO_UPLOAD_BUCKET_NAME = 'public-logos-local' diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index e9bf96db2..5f17ec2d3 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -93,6 +93,7 @@ def _gen_mock_field(x): 'test@cjsm.net', 'test@cqc.org.uk', 'test@digital.cqc.org.uk', + 'test@bl.uk', ]) def test_valid_list_of_white_list_email_domains( client, @@ -126,7 +127,8 @@ def test_valid_list_of_white_list_email_domains( 'test@ukpolice.uk', 'test@police.uk.uk', 'test@police.test.uk', - 'test@ucds.com' + 'test@ucds.com', + 'test@123bl.uk', ]) def test_invalid_list_of_white_list_email_domains( client,