diff --git a/app/models.py b/app/models.py index a56ba65b6..669fb12f9 100644 --- a/app/models.py +++ b/app/models.py @@ -819,11 +819,12 @@ class Notification(db.Model): @hybrid_property def status(self): - return self._status_fkey + return self._status_enum @status.setter def status(self, status): self._status_fkey = status + self._status_enum = status @property def personalisation(self): @@ -1029,11 +1030,12 @@ class NotificationHistory(db.Model, HistoryModel): @hybrid_property def status(self): - return self._status_fkey + return self._status_enum @status.setter def status(self, status): self._status_fkey = status + self._status_enum = status INVITED_USER_STATUS_TYPES = ['pending', 'accepted', 'cancelled'] diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index d2c00981b..a4517aee5 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -40,8 +40,7 @@ def test_should_get_all_statuses_for_notifications_associated_with_job( notify_db, notify_db_session, sample_service, - sample_job -): + sample_job): notification = partial(create_notification, notify_db, notify_db_session, service=sample_service, job=sample_job) notification(status='created') notification(status='sending') @@ -54,7 +53,7 @@ def test_should_get_all_statuses_for_notifications_associated_with_job( notification(status='sent') results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) - assert set([(row.count, row.status) for row in results]) == set([ + assert [(row.count, row.status) for row in results] == [ (1, 'created'), (1, 'sending'), (1, 'delivered'), @@ -64,19 +63,17 @@ def test_should_get_all_statuses_for_notifications_associated_with_job( (1, 'temporary-failure'), (1, 'permanent-failure'), (1, 'sent') - ]) + ] def test_should_count_of_statuses_for_notifications_associated_with_job( notify_db, notify_db_session, sample_service, - sample_job -): + sample_job): notification = partial(create_notification, notify_db, notify_db_session, service=sample_service, job=sample_job) notification(status='created') notification(status='created') - notification(status='sending') notification(status='sending') notification(status='sending') @@ -85,11 +82,11 @@ def test_should_count_of_statuses_for_notifications_associated_with_job( notification(status='delivered') results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) - assert set([(row.count, row.status) for row in results]) == set([ + assert [(row.count, row.status) for row in results] == [ (2, 'created'), (4, 'sending'), (2, 'delivered') - ]) + ] def test_should_return_zero_length_array_if_no_notifications_for_job(sample_service, sample_job): diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 4c26025a5..52dd115c6 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -410,6 +410,7 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_ data = _notification_json(sample_template, job_id=sample_job.id, status='sending') notification = Notification(**data) dao_create_notification(notification) + assert notification._status_enum == 'sending' assert notification._status_fkey == 'sending' assert Notification.query.get(notification.id).status == 'sending' @@ -421,6 +422,7 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_ assert updated.updated_at == datetime(2000, 1, 2, 12, 0, 0) assert Notification.query.get(notification.id).status == 'delivered' assert notification.updated_at == datetime(2000, 1, 2, 12, 0, 0) + assert notification._status_enum == 'delivered' assert notification._status_fkey == 'delivered' @@ -948,7 +950,7 @@ def test_get_notification_billable_unit_count_per_month(notify_db, notify_db_ses ) == months -def test_update_notification_sets_status(sample_notification): +def test_update_notification(sample_notification): assert sample_notification.status == 'created' sample_notification.status = 'failed' dao_update_notification(sample_notification) @@ -956,6 +958,37 @@ def test_update_notification_sets_status(sample_notification): assert notification_from_db.status == 'failed' +def test_update_notification_with_no_notification_status(sample_notification): + # specifically, it has an old enum status, but not a new status (because the upgrade script has just run) + update_dict = {'_status_enum': 'created', '_status_fkey': None} + Notification.query.filter(Notification.id == sample_notification.id).update(update_dict) + + # now lets update the status to failed - both columns should now be populated + sample_notification.status = 'failed' + dao_update_notification(sample_notification) + + notification_from_db = Notification.query.get(sample_notification.id) + assert notification_from_db.status == 'failed' + assert notification_from_db._status_enum == 'failed' + assert notification_from_db._status_fkey == 'failed' + + +def test_updating_notification_with_no_notification_status_updates_notification_history(sample_notification): + # same as above, but with notification history + update_dict = {'_status_enum': 'created', '_status_fkey': None} + Notification.query.filter(Notification.id == sample_notification.id).update(update_dict) + NotificationHistory.query.filter(NotificationHistory.id == sample_notification.id).update(update_dict) + + # now lets update the notification's status to failed - both columns should now be populated on the history object + sample_notification.status = 'failed' + dao_update_notification(sample_notification) + + hist_from_db = NotificationHistory.query.get(sample_notification.id) + assert hist_from_db.status == 'failed' + assert hist_from_db._status_enum == 'failed' + assert hist_from_db._status_fkey == 'failed' + + @pytest.mark.parametrize('notification_type, expected_sms_count, expected_email_count, expected_letter_count', [ ('sms', 8, 10, 10), ('email', 10, 8, 10), ('letter', 10, 10, 8) ])