From ae4c10d2ad069e7f1c3ee097dfe77ec48d448f07 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Fri, 1 Sep 2017 10:31:03 +0100 Subject: [PATCH 1/6] Update sqlalchemy-utils from 0.32.14 to 0.32.16 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index fa34786d4..9f0c653f6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -19,7 +19,7 @@ monotonic==1.3 psycopg2==2.7.3.1 PyJWT==1.5.2 six==1.10.0 -SQLAlchemy-Utils==0.32.14 +SQLAlchemy-Utils==0.32.16 SQLAlchemy==1.1.13 statsd==3.2.1 From b88d6b0a2b9b69a7fc7ff5fee511d1f8f4394f1f Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Tue, 5 Sep 2017 19:47:12 +0100 Subject: [PATCH 2/6] Update pyjwt from 1.5.2 to 1.5.3 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index fa34786d4..d9eec5f96 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,7 @@ marshmallow-sqlalchemy==0.13.1 marshmallow==2.13.6 monotonic==1.3 psycopg2==2.7.3.1 -PyJWT==1.5.2 +PyJWT==1.5.3 six==1.10.0 SQLAlchemy-Utils==0.32.14 SQLAlchemy==1.1.13 From 29a962060f21d1b8263cd55ed2541f8722e273ad Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 11 Sep 2017 14:16:04 +0100 Subject: [PATCH 3/6] Return delivery estimate for letter notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > For get all or get one letter the response needs to be updated so that > it looks similar to admin app. > > status: created|sending --> received letter > new column: `estimated delivery date`: derived from created at date. > (see how the admin app is doing it) > > NOTE: > At the moment we only have 2 statuses for a letter created and > sending, but we will want to have other internal statuses that make > sense to the Notify team but not our services. When we know those > statuses the status map will be updated at that point. – https://www.pivotaltracker.com/story/show/150512525 This commit implements the date (not status) part of this story. --- app/models.py | 5 ++++ tests/app/conftest.py | 12 ++++---- .../notifications/test_get_notifications.py | 28 ++++++++++++++++++- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/models.py b/app/models.py index f689cc3c4..8d1437986 100644 --- a/app/models.py +++ b/app/models.py @@ -16,6 +16,7 @@ from notifications_utils.recipients import ( InvalidPhoneError, InvalidEmailError ) +from notifications_utils.letter_timings import get_letter_timings from app.encryption import ( hashpw, @@ -1029,6 +1030,10 @@ class Notification(db.Model): serialized['line_5'] = self.personalisation.get('address_line_5') serialized['line_6'] = self.personalisation.get('address_line_6') serialized['postcode'] = self.personalisation['postcode'] + serialized['estimated_delivery'] = \ + get_letter_timings(serialized['created_at'])\ + .earliest_delivery\ + .strftime(DATETIME_FORMAT) return serialized diff --git a/tests/app/conftest.py b/tests/app/conftest.py index f23bc6200..0acfb3c39 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -568,12 +568,12 @@ def sample_notification( @pytest.fixture def sample_letter_notification(sample_letter_template): address = { - 'addressline1': 'A1', - 'addressline2': 'A2', - 'addressline3': 'A3', - 'addressline4': 'A4', - 'addressline5': 'A5', - 'addressline6': 'A6', + 'address_line_1': 'A1', + 'address_line_2': 'A2', + 'address_line_3': 'A3', + 'address_line_4': 'A4', + 'address_line_5': 'A5', + 'address_line_6': 'A6', 'postcode': 'A_POST' } return create_notification(sample_letter_template, personalisation=address) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index ff487275c..351b2c82f 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -1,4 +1,4 @@ - +import datetime import pytest from flask import json @@ -207,6 +207,32 @@ def test_get_notification_by_id_invalid_id(client, sample_notification, id): } +@pytest.mark.parametrize('created_at_month, estimated_delivery', [ + ( + 12, '2000-12-06T16:00:00.000000Z', # 4pm GMT in winter + ), + ( + 6, '2000-06-05T15:00:00.000000Z', # 4pm BST in summer + ), +]) +def test_get_notification_adds_delivery_estimate_for_letters( + client, + sample_letter_notification, + created_at_month, + estimated_delivery, +): + sample_letter_notification.created_at = datetime.date(2000, created_at_month, 1) + auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) + response = client.get( + path='/v2/notifications/{}'.format(sample_letter_notification.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + json_response = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + assert json_response['estimated_delivery'] == estimated_delivery + + def test_get_all_notifications_returns_200(client, sample_template): notifications = [create_notification(template=sample_template) for _ in range(2)] notification = notifications[-1] From acf8d663f508905336125ea8ec5a4fadff5cac17 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Sat, 16 Sep 2017 01:23:26 +0100 Subject: [PATCH 4/6] Update pytest-mock from 1.6.2 to 1.6.3 --- 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 02af5606a..0d9b5f87e 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,7 +1,7 @@ -r requirements.txt pycodestyle==2.3.1 pytest==3.2.2 -pytest-mock==1.6.2 +pytest-mock==1.6.3 pytest-cov==2.5.1 pytest-xdist==1.20.0 coveralls==1.2.0 From 2e9cecd3854eb19db804be45298c6f82d1cd4878 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 18 Sep 2017 11:00:21 +0100 Subject: [PATCH 5/6] Confirm no delivery estimate for emails and SMS Only letters have a delivery estimate (which we calculate). This commit adds a test to make sure this remains the case. --- .../notifications/test_get_notifications.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 351b2c82f..814e6dd6c 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -9,6 +9,11 @@ from tests.app.db import ( create_template, create_service) +from tests.app.conftest import ( + sample_notification, + sample_email_notification, +) + @pytest.mark.parametrize('billable_units, provider', [ (1, 'mmg'), @@ -233,6 +238,26 @@ def test_get_notification_adds_delivery_estimate_for_letters( assert json_response['estimated_delivery'] == estimated_delivery +@pytest.mark.parametrize('notification_mock', [ + sample_notification, + sample_email_notification, +]) +def test_get_notification_doesnt_have_delivery_estimate_for_non_letters( + client, + notify_db, + notify_db_session, + notification_mock, +): + mocked_notification = notification_mock(notify_db, notify_db_session) + auth_header = create_authorization_header(service_id=mocked_notification.service_id) + response = client.get( + path='/v2/notifications/{}'.format(mocked_notification.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + assert response.status_code == 200 + assert 'estimated_delivery' not in json.loads(response.get_data(as_text=True)) + + def test_get_all_notifications_returns_200(client, sample_template): notifications = [create_notification(template=sample_template) for _ in range(2)] notification = notifications[-1] From 9ecb666290ef3f082365bbaff05a7f206b88476b Mon Sep 17 00:00:00 2001 From: chrisw Date: Thu, 14 Sep 2017 16:26:46 +0100 Subject: [PATCH 6/6] Fixed bug where a user with an organisation that wanted the GOVUK only branding ended up with both If a user was to have an organisation selected in the email settings within the platform admin section, they would be sending emails that contained both the organisation's branding and GOV.UK's. Fix adds a check to ensure that the branding dictionary does not contain organisation details if the the service branding settings is set to 'gov' --- app/delivery/send_to_providers.py | 15 +++++++++++---- tests/app/delivery/test_send_to_providers.py | 13 +++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index c9aa84a20..9aafe1a35 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -15,9 +15,16 @@ from app.dao.provider_details_dao import ( ) from app.celery.research_mode_tasks import send_sms_response, send_email_response from app.dao.templates_dao import dao_get_template_by_id -from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE, NOTIFICATION_TECHNICAL_FAILURE, \ - NOTIFICATION_SENT, NOTIFICATION_SENDING - +from app.models import ( + SMS_TYPE, + KEY_TYPE_TEST, + BRANDING_ORG, + BRANDING_GOVUK, + EMAIL_TYPE, + NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_SENT, + NOTIFICATION_SENDING +) from app.celery.statistics_tasks import create_initial_notification_statistic_tasks @@ -168,7 +175,7 @@ def get_logo_url(base_url, logo_file): def get_html_email_options(service): govuk_banner = service.branding != BRANDING_ORG - if service.organisation: + if service.organisation and service.branding != BRANDING_GOVUK: logo_url = get_logo_url( current_app.config['ADMIN_BASE_URL'], service.organisation.logo diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 35368becf..5be786361 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -19,6 +19,7 @@ from app.models import ( KEY_TYPE_TEST, KEY_TYPE_TEAM, BRANDING_ORG, + BRANDING_GOVUK, BRANDING_BOTH) from tests.app.db import create_service, create_template, create_notification, create_inbound_number @@ -426,6 +427,18 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann assert options['brand_name'] == 'Justice League' +def test_get_html_email_renderer_with_branding_details_and_render_govuk_banner_only(notify_db, sample_service): + sample_service.branding = BRANDING_GOVUK + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + options = send_to_providers.get_html_email_options(sample_service) + + assert options == {'govuk_banner': True} + + def test_get_html_email_renderer_prepends_logo_path(notify_api): Service = namedtuple('Service', ['branding', 'organisation']) Organisation = namedtuple('Organisation', ['colour', 'name', 'logo'])