Refactor update to notifcations to be a bulk update. This is much better for performance.

app/celery/tasks.py

@@ -24,7 +24,8 @@ from app.dao.jobs_dao import (
     all_notifications_are_created_for_job,
     dao_get_all_notifications_for_job,
     dao_update_job_status)
-from app.dao.notifications_dao import get_notification_by_id, dao_update_notification
+from app.dao.notifications_dao import get_notification_by_id, dao_update_notification, \
+    dao_update_notifications_sent_to_dvla
 from app.dao.provider_details_dao import get_current_provider
 from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count
 from app.dao.templates_dao import dao_get_template_by_id
@@ -38,7 +39,7 @@ from app.models import (
     JOB_STATUS_IN_PROGRESS,
     JOB_STATUS_FINISHED,
     JOB_STATUS_READY_TO_SEND,
-    JOB_STATUS_SENT_TO_DVLA, NOTIFICATION_SENDING)
+    JOB_STATUS_SENT_TO_DVLA, NOTIFICATION_SENDING, Notification)
 from app.notifications.process_notifications import persist_notification
 from app.service.utils import service_allowed_to_send_to
 from app.statsd_decorators import statsd
@@ -298,14 +299,13 @@ def update_job_to_sent_to_dvla(self, job_id):
     # This task will be called by the FTP app to update the job to sent to dvla
     # and update all notifications for this job to sending, provider = DVLA
     provider = get_current_provider(LETTER_TYPE)
-    notifications = dao_get_all_notifications_for_job(job_id)
-    for n in notifications:
-        n.status = NOTIFICATION_SENDING
-        n.sent_by = provider.identifier
-        dao_update_notification(n)
 
+    updated_count = dao_update_notifications_sent_to_dvla(job_id, provider.identifier)
     dao_update_job_status(job_id, JOB_STATUS_SENT_TO_DVLA)
 
+    current_app.logger.info("Updated {} letter notifications to sending. "
+                            "Updated {} job to {}".format(updated_count, job_id, JOB_STATUS_SENT_TO_DVLA))
+
 
 def create_dvla_file_contents(job_id):
         file_contents = '\n'.join(

app/dao/notifications_dao.py

@@ -439,3 +439,18 @@ def is_delivery_slow_for_provider(
         (Notification.updated_at - Notification.sent_at) >= delivery_time,
     ).count()
     return count >= threshold
+
+
+@statsd(namespace="dao")
+@transactional
+def dao_update_notifications_sent_to_dvla(job_id, provider):
+    updated_count = db.session.query(Notification
+                                     ).filter(Notification.job_id == job_id
+                                              ).update({'status': 'sending', "sent_by": provider})
+    # only update NotificationHistory if api key typ is not test
+    db.session.query(NotificationHistory
+                     ).filter(NotificationHistory.job_id == job_id,
+                              NotificationHistory.key_type != KEY_TYPE_TEST
+                              ).update({'status': 'sending', "sent_by": provider})
+
+    return updated_count

tests/app/dao/test_notification_dao.py

@@ -41,8 +41,8 @@ from app.dao.notifications_dao import (
     dao_timeout_notifications,
     get_financial_year,
     get_april_fools,
-    is_delivery_slow_for_provider
+    is_delivery_slow_for_provider,
-)
+    dao_update_notifications_sent_to_dvla)
 
 from app.dao.services_dao import dao_update_service
 from tests.app.db import create_notification
@@ -1551,3 +1551,33 @@ def test_slow_provider_delivery_does_not_return_for_standard_delivery_time(
     )
 
     assert not slow_delivery
+
+
+def test_dao_update_notifications_sent_to_dvla(notify_db, notify_db_session, sample_letter_template):
+    job = sample_job(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_letter_template)
+    notification = create_notification(template=sample_letter_template, job=job)
+
+    updated_count = dao_update_notifications_sent_to_dvla(job_id=job.id, provider='some provider')
+
+    assert updated_count == 1
+    updated_notification = Notification.query.get(notification.id)
+    assert updated_notification.status == 'sending'
+    assert updated_notification.sent_by == 'some provider'
+    history = NotificationHistory.query.get(notification.id)
+    assert history.status == 'sending'
+    assert history.sent_by == 'some provider'
+
+
+def test_dao_update_notifications_sent_to_dvla_does_update_history_if_test_key(
+        notify_db, notify_db_session, sample_letter_template, sample_api_key):
+    job = sample_job(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_letter_template)
+    notification = create_notification(template=sample_letter_template, job=job, api_key_id=sample_api_key.id,
+                                       key_type='test')
+
+    updated_count = dao_update_notifications_sent_to_dvla(job_id=job.id, provider='some provider')
+
+    assert updated_count == 1
+    updated_notification = Notification.query.get(notification.id)
+    assert updated_notification.status == 'sending'
+    assert updated_notification.sent_by == 'some provider'
+    assert not NotificationHistory.query.get(notification.id)