Refactor code

- removed redundant dao method
- also handle letter jobs

app/celery/tasks.py

@@ -29,7 +29,6 @@ from app.dao.jobs_dao import (
     dao_update_job_status)
 from app.dao.notifications_dao import (
     get_notification_by_id,
-    dao_get_total_notifications_for_job_id,
     dao_update_notifications_for_job_to_sent_to_dvla,
     dao_update_notifications_by_reference
 )
@@ -89,8 +88,7 @@ def process_job(job_id):
 
     current_app.logger.info("Starting job {} processing {} notifications".format(job_id, job.notification_count))
 
-    if template.template_type != LETTER_TYPE:
-        check_job_status.apply_async([str(job.id)], queue=QueueNames.JOBS)
+    check_job_status.apply_async([str(job.id)], queue=QueueNames.JOBS)
 
     for row_number, recipient, personalisation in RecipientCSV(
             s3.get_job_from_s3(str(service.id), str(job_id)),
@@ -334,7 +332,9 @@ def update_dvla_job_to_error(self, job_id):
 @statsd(namespace="tasks")
 def check_job_status(self, job_id):
     job = dao_get_job_by_id(job_id)
-    if job.job_status != JOB_STATUS_FINISHED:
+
+    if (job.template.template_type == LETTER_TYPE and job.job_status != JOB_STATUS_SENT_TO_DVLA) or\
+            (job.template.template_type != LETTER_TYPE and job.job_status != JOB_STATUS_FINISHED):
         raise JobIncompleteError("Job {} did not complete".format(job_id))
 
 

app/dao/notifications_dao.py

@@ -442,16 +442,6 @@ def get_total_sent_notifications_in_date_range(start_date, end_date, notificatio
     return result or 0
 
 
-def dao_get_total_notifications_for_job_id(job_id):
-    result = db.session.query(
-        func.count(Notification.id).label('count')
-    ).filter(
-        Notification.job_id == job_id
-    ).scalar()
-
-    return result or 0
-
-
 def is_delivery_slow_for_provider(
     sent_at,
     provider,

tests/app/celery/test_tasks.py

@@ -32,6 +32,7 @@ from app.models import (
     EMAIL_TYPE,
     JOB_STATUS_ERROR,
     JOB_STATUS_FINISHED,
+    JOB_STATUS_SENT_TO_DVLA,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEAM,
     KEY_TYPE_TEST,
@@ -1234,6 +1235,14 @@ def test_job_incomplete_raises_job_incomplete_error(sample_job):
     assert e.value.message == 'Job {} did not complete'.format(sample_job.id)
 
 
-def test_job_finished_does_not_raises_job_incomplete_error(sample_job):
-    sample_job.job_status = JOB_STATUS_FINISHED
+@pytest.mark.parametrize('job_status,notification_type',
+    [
+        (JOB_STATUS_FINISHED, SMS_TYPE),
+        (JOB_STATUS_SENT_TO_DVLA, LETTER_TYPE)
+    ]
+)
+def test_job_finished_does_not_raises_job_incomplete_error(
+        sample_job, job_status, notification_type):
+    sample_job.job_status = job_status
+    sample_job.template.template_type = notification_type
     check_job_status(str(sample_job.id))

tests/app/dao/notification_dao/test_notification_dao.py

@@ -35,7 +35,6 @@ from app.dao.notifications_dao import (
     dao_get_potential_notification_statistics_for_day,
     dao_get_scheduled_notifications,
     dao_get_template_usage,
-    dao_get_total_notifications_for_job_id,
     dao_timeout_notifications,
     dao_update_notification,
     dao_update_notifications_for_job_to_sent_to_dvla,
@@ -2001,13 +2000,3 @@ def test_dao_get_notification_ememail_reply_toail_reply_for_notification(sample_
 
 def test_dao_get_notification_email_reply_for_notification_where_no_mapping(notify_db_session, fake_uuid):
     assert dao_get_notification_email_reply_for_notification(fake_uuid) is None
-
-
-def test_dao_get_total_notifications_for_job_id(sample_job):
-    job = create_job(sample_job.template)
-    create_notification(sample_job.template, job=sample_job)
-    create_notification(sample_job.template, job=sample_job)
-    create_notification(sample_job.template, job=sample_job)
-    create_notification(sample_job.template, job=job)
-
-    assert dao_get_total_notifications_for_job_id(sample_job.id) == 3