diff --git a/app/models.py b/app/models.py index 92c9b0128..1465e2f15 100644 --- a/app/models.py +++ b/app/models.py @@ -1,3 +1,4 @@ +import itertools import time import uuid import datetime @@ -797,6 +798,9 @@ NOTIFICATION_STATUS_TYPES_NON_BILLABLE = list(set(NOTIFICATION_STATUS_TYPES) - s NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type') +NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted' +NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY = 'Accepted' + class NotificationStatusTypes(db.Model): __tablename__ = 'notification_status_types' @@ -900,10 +904,10 @@ class Notification(db.Model): - > IN - ['failed', 'created'] + ['failed', 'created', 'accepted'] < OUT - ['technical-failure', 'temporary-failure', 'permanent-failure', 'created'] + ['technical-failure', 'temporary-failure', 'permanent-failure', 'created', 'sending'] :param status_or_statuses: a single status or list of statuses @@ -911,18 +915,17 @@ class Notification(db.Model): """ def _substitute_status_str(_status): - return NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else _status + return ( + NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else + [NOTIFICATION_CREATED, NOTIFICATION_SENDING] if _status == NOTIFICATION_STATUS_LETTER_ACCEPTED else + [_status] + ) def _substitute_status_seq(_statuses): - if NOTIFICATION_FAILED in _statuses: - _statuses = list(set( - NOTIFICATION_STATUS_TYPES_FAILED + [_s for _s in _statuses if _s != NOTIFICATION_FAILED] - )) - return _statuses + return list(set(itertools.chain.from_iterable(_substitute_status_str(status) for status in _statuses))) if isinstance(status_or_statuses, str): return _substitute_status_str(status_or_statuses) - return _substitute_status_seq(status_or_statuses) @property @@ -962,17 +965,29 @@ class Notification(db.Model): 'sent': 'Sent internationally' }, 'letter': { - 'failed': 'Failed', 'technical-failure': 'Technical failure', - 'temporary-failure': 'Temporary failure', - 'permanent-failure': 'Permanent failure', - 'delivered': 'Delivered', - 'sending': 'Sending', - 'created': 'Sending', - 'sent': 'Delivered' + 'sending': NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY, + 'created': NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY, } }[self.template.template_type].get(self.status, self.status) + def get_letter_status(self): + """ + Return the notification_status, as we should present for letters. The distinction between created and sending is + a bit more confusing for letters, not to mention that there's no concept of temporary or permanent failure yet. + + + """ + # this should only ever be called for letter notifications - it makes no sense otherwise and I'd rather not + # get the two code flows mixed up at all + assert self.notification_type == LETTER_TYPE + + if self.status == NOTIFICATION_CREATED or NOTIFICATION_SENDING: + return NOTIFICATION_STATUS_LETTER_ACCEPTED + else: + # Currently can only be technical-failure + return status + def serialize_for_csv(self): created_at_in_bst = convert_utc_to_bst(self.created_at) serialized = { @@ -1007,7 +1022,7 @@ class Notification(db.Model): "line_6": None, "postcode": None, "type": self.notification_type, - "status": self.status, + "status": self.get_letter_status() if self.notification_type == LETTER_TYPE else self.status, "template": template_dict, "body": self.content, "subject": self.subject, diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index a22ad863b..9ff028d1f 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,4 +1,4 @@ -from app.models import NOTIFICATION_STATUS_TYPES, TEMPLATE_TYPES +from app.models import NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_LETTER_ACCEPTED, TEMPLATE_TYPES from app.schema_validation.definitions import (uuid, personalisation, letter_personalisation) @@ -59,7 +59,7 @@ get_notifications_request = { "status": { "type": "array", "items": { - "enum": NOTIFICATION_STATUS_TYPES + "enum": NOTIFICATION_STATUS_TYPES + [NOTIFICATION_STATUS_LETTER_ACCEPTED] } }, "template_type": { diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index 15c1d8ef0..9ee35d24b 100755 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -33,7 +33,6 @@ pip3 install -r requirements_for_test.txt # Create Postgres databases createdb notification_api -createdb test_notification_api # Upgrade databases source environment.sh diff --git a/tests/app/dao/test_monthly_billing.py b/tests/app/dao/test_monthly_billing.py index 23c445f92..e5813c9bf 100644 --- a/tests/app/dao/test_monthly_billing.py +++ b/tests/app/dao/test_monthly_billing.py @@ -198,7 +198,10 @@ def test_add_monthly_billing_for_multiple_months_populate_correctly( create_or_update_monthly_billing(service_id=sample_template.service_id, billing_month=FEB_2016_MONTH_START) create_or_update_monthly_billing(service_id=sample_template.service_id, billing_month=MAR_2016_MONTH_START) - monthly_billing = MonthlyBilling.query.order_by(MonthlyBilling.notification_type).all() + monthly_billing = MonthlyBilling.query.order_by( + MonthlyBilling.notification_type, + MonthlyBilling.start_date + ).all() assert len(monthly_billing) == 4 _assert_monthly_billing( diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 56a121e01..a20e83001 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -10,10 +10,12 @@ from app.models import ( MOBILE_TYPE, EMAIL_TYPE, NOTIFICATION_CREATED, + NOTIFICATION_SENDING, NOTIFICATION_PENDING, NOTIFICATION_FAILED, NOTIFICATION_TECHNICAL_FAILURE, - NOTIFICATION_STATUS_TYPES_FAILED + NOTIFICATION_STATUS_TYPES_FAILED, + NOTIFICATION_STATUS_LETTER_ACCEPTED ) from tests.app.conftest import ( sample_template as create_sample_template, @@ -55,8 +57,9 @@ def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, @pytest.mark.parametrize('initial_statuses, expected_statuses', [ # passing in single statuses as strings (NOTIFICATION_FAILED, NOTIFICATION_STATUS_TYPES_FAILED), - (NOTIFICATION_CREATED, NOTIFICATION_CREATED), - (NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TECHNICAL_FAILURE), + (NOTIFICATION_STATUS_LETTER_ACCEPTED, [NOTIFICATION_SENDING, NOTIFICATION_CREATED]), + (NOTIFICATION_CREATED, [NOTIFICATION_CREATED]), + (NOTIFICATION_TECHNICAL_FAILURE, [NOTIFICATION_TECHNICAL_FAILURE]), # passing in lists containing single statuses ([NOTIFICATION_FAILED], NOTIFICATION_STATUS_TYPES_FAILED), ([NOTIFICATION_CREATED], [NOTIFICATION_CREATED]), @@ -65,13 +68,17 @@ def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, ([NOTIFICATION_FAILED, NOTIFICATION_CREATED], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED]), ([NOTIFICATION_CREATED, NOTIFICATION_PENDING], [NOTIFICATION_CREATED, NOTIFICATION_PENDING]), ([NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE]), + ( + [NOTIFICATION_FAILED, NOTIFICATION_STATUS_LETTER_ACCEPTED], + NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_SENDING, NOTIFICATION_CREATED] + ), # checking we don't end up with duplicates ( [NOTIFICATION_FAILED, NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED] ), ]) -def test_status_conversion_handles_failed_statuses(initial_statuses, expected_statuses): +def test_status_conversion(initial_statuses, expected_statuses): converted_statuses = Notification.substitute_status(initial_statuses) assert len(converted_statuses) == len(expected_statuses) assert set(converted_statuses) == set(expected_statuses) @@ -116,8 +123,9 @@ def test_notification_for_csv_returns_correct_job_row_number(notify_db, notify_d ('sms', 'temporary-failure', 'Phone not accepting messages right now'), ('sms', 'permanent-failure', 'Phone number doesn’t exist'), ('sms', 'sent', 'Sent internationally'), - ('letter', 'permanent-failure', 'Permanent failure'), - ('letter', 'delivered', 'Delivered') + ('letter', 'created', 'Accepted'), + ('letter', 'sending', 'Accepted'), + ('letter', 'technical-failure', 'Technical failure') ]) def test_notification_for_csv_returns_formatted_status( notify_db, diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 814e6dd6c..926c8c1d8 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -1,6 +1,6 @@ import datetime import pytest -from flask import json +from flask import json, url_for from app import DATETIME_FORMAT from tests import create_authorization_header @@ -23,13 +23,20 @@ from tests.app.conftest import ( def test_get_notification_by_id_returns_200( client, billable_units, provider, sample_template ): - sample_notification = create_notification(template=sample_template, billable_units=billable_units, sent_by=provider, - scheduled_for="2017-05-12 15:15" - ) + sample_notification = create_notification( + template=sample_template, + billable_units=billable_units, + sent_by=provider, + scheduled_for="2017-05-12 15:15" + ) + + another = create_notification( + template=sample_template, + billable_units=billable_units, + sent_by=provider, + scheduled_for="2017-06-12 15:15" + ) - another = create_notification(template=sample_template, billable_units=billable_units, sent_by=provider, - scheduled_for="2017-06-12 15:15" - ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( path='/v2/notifications/{}'.format(sample_notification.id), @@ -75,9 +82,10 @@ def test_get_notification_by_id_returns_200( def test_get_notification_by_id_with_placeholders_returns_200( client, sample_email_template_with_placeholders ): - sample_notification = create_notification(template=sample_email_template_with_placeholders, - personalisation={"name": "Bob"} - ) + sample_notification = create_notification( + template=sample_email_template_with_placeholders, + personalisation={"name": "Bob"} + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get( @@ -385,7 +393,7 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no assert json_response['status_code'] == 400 assert len(json_response['errors']) == 1 assert json_response['errors'][0]['message'] == "status elephant is not one of [created, sending, sent, " \ - "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure]" + "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, accepted]" def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): @@ -549,3 +557,40 @@ def test_get_all_notifications_filter_multiple_query_parameters(client, sample_e assert len(json_response['notifications']) == 1 assert json_response['notifications'][0]['id'] == str(older_notification.id) + + +def test_get_all_notifications_renames_letter_statuses( + client, + sample_letter_notification, + sample_notification, + sample_email_notification, +): + auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) + response = client.get( + path=url_for('v2_notifications.get_notifications'), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + json_response = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + + for noti in json_response['notifications']: + if noti['type'] == 'sms' or noti['type'] == 'email': + assert noti['status'] == 'created' + elif noti['type'] == 'letter': + assert noti['status'] == 'accepted' + else: + pytest.fail() + + +def test_get_notifications_renames_letter_statuses(client, sample_letter_notification): + auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) + response = client.get( + path=url_for('v2_notifications.get_notification_by_id', id=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['status'] == 'accepted' diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 9e3040194..c667def11 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -26,7 +26,7 @@ def test_get_notifications_request_invalid_statuses( ): partial_error_status = "is not one of " \ "[created, sending, sent, delivered, pending, failed, " \ - "technical-failure, temporary-failure, permanent-failure]" + "technical-failure, temporary-failure, permanent-failure, accepted]" with pytest.raises(ValidationError) as e: validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) @@ -73,7 +73,7 @@ def test_get_notifications_request_invalid_statuses_and_template_types(): error_messages = [error['message'] for error in errors] for invalid_status in ["elephant", "giraffe"]: assert "status {} is not one of [created, sending, sent, delivered, " \ - "pending, failed, technical-failure, temporary-failure, permanent-failure]".format( + "pending, failed, technical-failure, temporary-failure, permanent-failure, accepted]".format( invalid_status ) in error_messages