Add filter to get jobs to delete (sms, email, letter)

app/dao/jobs_dao.py

@@ -1,17 +1,15 @@
-from datetime import datetime
+from datetime import datetime, timedelta
 
 from flask import current_app
 from sqlalchemy import func, desc, asc, cast, Date as sql_date
 
 from app import db
 from app.dao import days_ago
-from app.models import (Job,
-                        Notification,
-                        NotificationHistory,
-                        Template,
-                        JOB_STATUS_SCHEDULED,
-                        JOB_STATUS_PENDING,
-                        LETTER_TYPE, JobStatistics)
+from app.models import (
+    Job, JobStatistics, Notification, NotificationHistory, Template,
+    JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING,
+    EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
+)
 from app.statsd_decorators import statsd
 
 
@@ -129,10 +127,14 @@ def dao_update_job_status(job_id, status):
     db.session.commit()
 
 
-def dao_get_jobs_older_than_limited_by(older_than=7, limit_days=2):
-    return Job.query.filter(
-        cast(Job.created_at, sql_date) < days_ago(older_than),
-        cast(Job.created_at, sql_date) >= days_ago(older_than + limit_days)
+def dao_get_jobs_older_than_limited_by(job_types, older_than=7, limit_days=2):
+    end_date = datetime.utcnow() - timedelta(days=older_than)
+    start_date = end_date - timedelta(days=limit_days)
+
+    return Job.query.join(Template).filter(
+        Job.created_at < end_date,
+        Job.created_at >= start_date,
+        Template.template_type.in_(job_types)
     ).order_by(desc(Job.created_at)).all()
 
 

tests/app/conftest.py

@@ -280,7 +280,8 @@ def sample_team_api_key(notify_db, notify_db_session, service=None):
 
 
 @pytest.fixture(scope='function')
-def sample_job(notify_db,
+def sample_job(
+    notify_db,
     notify_db_session,
     service=None,
     template=None,
@@ -289,7 +290,8 @@ def sample_job(notify_db,
     job_status='pending',
     scheduled_for=None,
     processing_started=None,
-               original_file_name='some.csv'):
+    original_file_name='some.csv'
+):
     if service is None:
         service = sample_service(notify_db, notify_db_session)
     if template is None:

tests/app/dao/test_jobs_dao.py

@@ -17,7 +17,10 @@ from app.dao.jobs_dao import (
     dao_update_job_status,
     dao_get_all_notifications_for_job,
     dao_get_jobs_older_than_limited_by)
-from app.models import Job, JobStatistics
+from app.models import (
+    Job, JobStatistics,
+    EMAIL_TYPE, SMS_TYPE, LETTER_TYPE
+)
 
 from tests.app.conftest import sample_notification as create_notification
 from tests.app.conftest import sample_job as create_job
@@ -285,33 +288,30 @@ def test_get_future_scheduled_job_gets_a_job_yet_to_send(sample_scheduled_job):
     assert result.id == sample_scheduled_job.id
 
 
-def test_should_get_jobs_seven_days_old(notify_db, notify_db_session):
-    # job runs at some point on each day
-    # shouldn't matter when, we are deleting things 7 days ago
-    job_run_time = '2016-10-31T10:00:00'
+@freeze_time('2016-10-31 10:00:00')
+def test_should_get_jobs_seven_days_old(notify_db, notify_db_session, sample_template):
+    """
+    Jobs older than seven days are deleted, but only two day's worth (two-day window)
+    """
+    seven_days_ago = datetime.utcnow() - timedelta(days=7)
+    within_seven_days = seven_days_ago + timedelta(seconds=1)
 
-    # running on the 31st means the previous 7 days are ignored
+    eight_days_ago = seven_days_ago - timedelta(days=1)
 
-    # 2 day window for delete jobs
-    # 7 days of files to skip includes the 30,29,28,27,26,25,24th, so the....
-    last_possible_time_for_eligible_job = '2016-10-23T23:59:59'
-    first_possible_time_for_eligible_job = '2016-10-22T00:00:00'
+    nine_days_ago = eight_days_ago - timedelta(days=2)
+    nine_days_one_second_ago = nine_days_ago - timedelta(seconds=1)
 
-    job_1 = create_job(notify_db, notify_db_session, created_at=last_possible_time_for_eligible_job)
-    job_2 = create_job(notify_db, notify_db_session, created_at=first_possible_time_for_eligible_job)
+    job = partial(create_job, notify_db, notify_db_session)
+    job(created_at=seven_days_ago)
+    job(created_at=within_seven_days)
+    job_to_delete = job(created_at=eight_days_ago)
+    job(created_at=nine_days_ago)
+    job(created_at=nine_days_one_second_ago)
 
-    # bookmarks for jobs that should be ignored
-    last_possible_time_for_ineligible_job = '2016-10-24T00:00:00'
-    create_job(notify_db, notify_db_session, created_at=last_possible_time_for_ineligible_job)
+    jobs = dao_get_jobs_older_than_limited_by(job_types=[sample_template.template_type])
 
-    first_possible_time_for_ineligible_job = '2016-10-21T23:59:59'
-    create_job(notify_db, notify_db_session, created_at=first_possible_time_for_ineligible_job)
-
-    with freeze_time(job_run_time):
-        jobs = dao_get_jobs_older_than_limited_by()
-        assert len(jobs) == 2
-        assert jobs[0].id == job_1.id
-        assert jobs[1].id == job_2.id
+    assert len(jobs) == 1
+    assert jobs[0].id == job_to_delete.id
 
 
 def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_service, sample_template):
@@ -391,3 +391,23 @@ def test_dao_update_job_status(sample_job):
     updated_job = Job.query.get(sample_job.id)
     assert updated_job.job_status == 'sent to dvla'
     assert updated_job.updated_at
+
+
+@freeze_time('2016-10-31 10:00:00')
+def test_should_get_jobs_seven_days_old_filters_type(notify_db, notify_db_session):
+    eight_days_ago = datetime.utcnow() - timedelta(days=8)
+    letter_template = create_template(notify_db, notify_db_session, template_type=LETTER_TYPE)
+    sms_template = create_template(notify_db, notify_db_session, template_type=SMS_TYPE)
+    email_template = create_template(notify_db, notify_db_session, template_type=EMAIL_TYPE)
+
+    job = partial(create_job, notify_db, notify_db_session, created_at=eight_days_ago)
+    job_to_remain = job(template=letter_template)
+    job(template=sms_template)
+    job(template=email_template)
+
+    jobs = dao_get_jobs_older_than_limited_by(
+        job_types=[EMAIL_TYPE, SMS_TYPE]
+    )
+
+    assert len(jobs) == 2
+    assert job_to_remain.id not in [job.id for job in jobs]