Remove from_request method on the Notification db model, was not needed anymore.

Use Notifications.query.one(), rather than assert count is 1 and query for all and get first item in list.

app/models.py

@@ -537,34 +537,6 @@ class Notification(db.Model):
         if personalisation:
             self._personalisation = encryption.encrypt(personalisation)
 
-    @classmethod
-    def from_request(cls,
-                     template_id,
-                     template_version,
-                     recipient,
-                     service_id,
-                     personalisation,
-                     notification_type,
-                     api_key_id,
-                     key_type,
-                     job_id,
-                     job_row_number,
-                     created_at):
-        return cls(
-            template_id=template_id,
-            template_version=template_version,
-            to=recipient,
-            service_id=service_id,
-            status='created',
-            created_at=created_at,
-            personalisation=personalisation,
-            notification_type=notification_type,
-            api_key_id=api_key_id,
-            key_type=key_type,
-            job_id=job_id,
-            job_row_number=job_row_number
-        )
-
 
 class NotificationHistory(db.Model):
     __tablename__ = 'notification_history'

app/notifications/process_notifications.py

@@ -47,10 +47,10 @@ def persist_notification(template_id,
                          created_at=None,
                          job_id=None,
                          job_row_number=None):
-    notification = Notification.from_request(
+    notification = Notification(
         template_id=template_id,
         template_version=template_version,
-        recipient=recipient,
+        to=recipient,
         service_id=service_id,
         personalisation=personalisation,
         notification_type=notification_type,

tests/app/celery/test_tasks.py

@@ -339,8 +339,7 @@ def test_should_send_template_to_correct_sms_task_and_persist(sample_template_wi
         datetime.utcnow().strftime(DATETIME_FORMAT)
     )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     assert persisted_notification.to == '+447234123123'
     assert persisted_notification.template_id == sample_template_with_placeholders.id
     assert persisted_notification.template_version == sample_template_with_placeholders.version
@@ -377,8 +376,7 @@ def test_should_put_send_sms_task_in_research_mode_queue_if_research_mode_servic
         encryption.encrypt(notification),
         datetime.utcnow().strftime(DATETIME_FORMAT)
     )
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     provider_tasks.deliver_sms.apply_async.assert_called_once_with(
         [persisted_notification.id],
         queue="research-mode"
@@ -392,7 +390,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
     template = sample_template(notify_db, notify_db_session, service=service)
     notification = _notification_json(template, "+447700900890")  # The user’s own number, but in a different format
 
-    mocked_deliver_sms = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+    mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
 
     notification_id = uuid.uuid4()
     encrypt_notification = encryption.encrypt(notification)
@@ -403,8 +401,7 @@ def test_should_send_sms_if_restricted_service_and_valid_number(notify_db, notif
         datetime.utcnow().strftime(DATETIME_FORMAT)
     )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     assert persisted_notification.to == '+447700900890'
     assert persisted_notification.template_id == template.id
     assert persisted_notification.template_version == template.version
@@ -440,8 +437,7 @@ def test_should_send_sms_if_restricted_service_and_non_team_number_with_test_key
         key_type=KEY_TYPE_TEST
     )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     mocked_deliver_sms.assert_called_once_with(
         [persisted_notification.id],
         queue="send-sms"
@@ -469,8 +465,7 @@ def test_should_send_email_if_restricted_service_and_non_team_email_address_with
         key_type=KEY_TYPE_TEST
     )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     mocked_deliver_email.assert_called_once_with(
         [persisted_notification.id],
         queue="send-email"
@@ -537,8 +532,7 @@ def test_should_put_send_email_task_in_research_mode_queue_if_research_mode_serv
         datetime.utcnow().strftime(DATETIME_FORMAT)
     )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     provider_tasks.deliver_email.apply_async.assert_called_once_with(
         [persisted_notification.id],
         queue="research-mode"
@@ -563,8 +557,7 @@ def test_should_send_sms_template_to_and_persist_with_job_id(sample_job, sample_
         api_key_id=str(sample_api_key.id),
         key_type=KEY_TYPE_NORMAL
     )
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     assert persisted_notification.to == '+447234123123'
     assert persisted_notification.job_id == sample_job.id
     assert persisted_notification.template_id == sample_job.template.id
@@ -660,8 +653,7 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
             key_type=sample_api_key.key_type
         )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     assert persisted_notification.to == 'my_email@my_email.com'
     assert persisted_notification.template_id == sample_email_template_with_placeholders.id
     assert persisted_notification.template_version == sample_email_template_with_placeholders.version
@@ -698,8 +690,7 @@ def test_send_email_should_use_template_version_from_job_not_latest(sample_email
         now.strftime(DATETIME_FORMAT)
     )
 
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     assert persisted_notification.to == 'my_email@my_email.com'
     assert persisted_notification.template_id == sample_email_template.id
     assert persisted_notification.template_version == version_on_notification
@@ -724,8 +715,7 @@ def test_should_use_email_template_subject_placeholders(sample_email_template_wi
         encryption.encrypt(notification),
         now.strftime(DATETIME_FORMAT)
     )
-    assert Notification.query.count() == 1
-    persisted_notification = Notification.query.all()[0]
+    persisted_notification = Notification.query.one()
     assert persisted_notification.to == 'my_email@my_email.com'
     assert persisted_notification.template_id == sample_email_template_with_placeholders.id
     assert persisted_notification.status == 'created'
@@ -743,8 +733,6 @@ def test_should_use_email_template_and_persist_without_personalisation(sample_em
     notification = _notification_json(sample_email_template, "my_email@my_email.com")
     mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
 
-    assert Notification.query.count() == 0
-
     notification_id = uuid.uuid4()
 
     now = datetime.utcnow()

tests/app/test_model.py

@@ -10,65 +10,6 @@ from app.models import (
     MOBILE_TYPE, EMAIL_TYPE)
 
 
-def test_should_build_notification_from_minimal_set_of_api_derived_params(notify_api):
-    now = datetime.utcnow()
-
-    notification = Notification.from_request(
-        template_id='template',
-        template_version='1',
-        recipient='someone',
-        service_id='service_id',
-        notification_type='SMS',
-        created_at=now,
-        api_key_id='api_key_id',
-        key_type='key_type',
-        personalisation={},
-        job_id=None,
-        job_row_number=None
-    )
-    assert notification.created_at == now
-    assert notification.template_id == 'template'
-    assert notification.template_version == '1'
-    assert not notification.job_row_number
-    assert not notification.job_id
-    assert notification.to == 'someone'
-    assert notification.service_id == 'service_id'
-    assert notification.status == 'created'
-    assert not notification.personalisation
-    assert notification.notification_type == 'SMS'
-    assert notification.api_key_id == 'api_key_id'
-    assert notification.key_type == 'key_type'
-
-
-def test_should_build_notification_from_full_set_of_api_derived_params(notify_api):
-    now = datetime.utcnow()
-    notification = Notification.from_request(template_id='template',
-                                             template_version=1,
-                                             recipient='someone',
-                                             service_id='service_id',
-                                             personalisation={'key': 'value'},
-                                             notification_type='SMS',
-                                             api_key_id='api_key_id',
-                                             key_type='key_type',
-                                             job_id='job_id',
-                                             job_row_number=100,
-                                             created_at=now
-                                             )
-    assert notification.created_at == now
-    assert notification.id is None
-    assert notification.template_id == 'template'
-    assert notification.template_version == 1
-    assert notification.job_row_number == 100
-    assert notification.job_id == 'job_id'
-    assert notification.to == 'someone'
-    assert notification.service_id == 'service_id'
-    assert notification.status == 'created'
-    assert notification.personalisation == {'key': 'value'}
-    assert notification.notification_type == 'SMS'
-    assert notification.api_key_id == 'api_key_id'
-    assert notification.key_type == 'key_type'
-
-
 @pytest.mark.parametrize('mobile_number', [
     '07700 900678',
     '+44 7700 900678'