remove unused pytest flags

Validate email access based on code_type and not auth_type

app/aws/s3.py

@@ -97,13 +97,6 @@ def remove_s3_object(bucket_name, object_key):
     return obj.delete()
 
 
-def remove_transformed_dvla_file(job_id):
-    bucket_name = current_app.config['DVLA_BUCKETS']['job']
-    file_location = '{}-dvla-job.text'.format(job_id)
-    obj = get_s3_object(bucket_name, file_location)
-    return obj.delete()
-
-
 def get_list_of_files_by_suffix(bucket_name, subfolder='', suffix='', last_modified=None):
     s3_client = client('s3', current_app.config['AWS_REGION'])
     paginator = s3_client.get_paginator('list_objects_v2')

app/celery/nightly_tasks.py

@@ -214,16 +214,6 @@ def delete_inbound_sms():
         raise
 
 
-@notify_celery.task(name="remove_transformed_dvla_files")
-@cronitor("remove_transformed_dvla_files")
-@statsd(namespace="tasks")
-def remove_transformed_dvla_files():
-    jobs = dao_get_jobs_older_than_data_retention(notification_types=[LETTER_TYPE])
-    for job in jobs:
-        s3.remove_transformed_dvla_file(job.id)
-        current_app.logger.info("Transformed dvla file for job {} has been removed from s3.".format(job.id))
-
-
 # TODO: remove me, i'm not being run by anything
 @notify_celery.task(name="delete_dvla_response_files")
 @statsd(namespace="tasks")

app/config.py

@@ -263,11 +263,6 @@ class Config(object):
             'schedule': crontab(hour=2, minute=0),
             'options': {'queue': QueueNames.PERIODIC}
         },
-        'remove_transformed_dvla_files': {
-            'task': 'remove_transformed_dvla_files',
-            'schedule': crontab(hour=3, minute=40),
-            'options': {'queue': QueueNames.PERIODIC}
-        },
         'remove_sms_email_jobs': {
             'task': 'remove_sms_email_jobs',
             'schedule': crontab(hour=4, minute=0),
@@ -275,7 +270,7 @@ class Config(object):
         },
         'remove_letter_jobs': {
             'task': 'remove_letter_jobs',
-            'schedule': crontab(hour=4, minute=20),  # this has to run AFTER remove_transformed_dvla_files
+            'schedule': crontab(hour=4, minute=20),
             # since we mark jobs as archived
             'options': {'queue': QueueNames.PERIODIC},
         },
@@ -330,11 +325,6 @@ class Config(object):
 
     SIMULATED_SMS_NUMBERS = ('+447700900000', '+447700900111', '+447700900222')
 
-    DVLA_BUCKETS = {
-        'job': '{}-dvla-file-per-job'.format(os.getenv('NOTIFY_ENVIRONMENT')),
-        'notification': '{}-dvla-letter-api-files'.format(os.getenv('NOTIFY_ENVIRONMENT'))
-    }
-
     FREE_SMS_TIER_FRAGMENT_COUNT = 250000
 
     SMS_INBOUND_WHITELIST = json.loads(os.environ.get('SMS_INBOUND_WHITELIST', '[]'))

app/dao/notifications_dao.py

@@ -30,6 +30,7 @@ from app.dao.dao_utils import transactional
 from app.errors import InvalidRequest
 from app.letters.utils import get_letter_pdf_filename
 from app.models import (
+    FactNotificationStatus,
     Notification,
     NotificationHistory,
     ProviderDetails,
@@ -70,6 +71,29 @@ def dao_get_last_template_usage(template_id, template_type, service_id):
     ).first()
 
 
+@statsd(namespace="dao")
+def dao_get_last_date_template_was_used(template_id, service_id):
+    last_date_from_notifications = db.session.query(
+        functions.max(Notification.created_at)
+    ).filter(
+        Notification.service_id == service_id,
+        Notification.template_id == template_id,
+        Notification.key_type != KEY_TYPE_TEST
+    ).scalar()
+
+    if last_date_from_notifications:
+        return last_date_from_notifications
+
+    last_date = db.session.query(
+        functions.max(FactNotificationStatus.bst_date)
+    ).filter(
+        FactNotificationStatus.template_id == template_id,
+        FactNotificationStatus.key_type != KEY_TYPE_TEST
+    ).scalar()
+
+    return last_date
+
+
 @statsd(namespace="dao")
 @transactional
 def dao_create_notification(notification):

app/models.py

@@ -114,7 +114,9 @@ class User(db.Model):
     platform_admin = db.Column(db.Boolean, nullable=False, default=False)
     current_session_id = db.Column(UUID(as_uuid=True), nullable=True)
     auth_type = db.Column(db.String, db.ForeignKey('auth_type.name'), index=True, nullable=False, default=SMS_AUTH_TYPE)
-    email_access_validated_at = db.Column(db.DateTime, index=False, unique=False, nullable=False)
+    email_access_validated_at = db.Column(
+        db.DateTime, index=False, unique=False, nullable=False, default=datetime.datetime.utcnow
+    )
 
     # either email auth or a mobile number must be provided
     CheckConstraint("auth_type = 'email_auth' or mobile_number is not null")
@@ -163,15 +165,11 @@ class User(db.Model):
             'auth_type': self.auth_type,
             'current_session_id': self.current_session_id,
             'failed_login_count': self.failed_login_count,
-            'email_access_validated_at': self.email_access_validated_at,
+            'email_access_validated_at': self.email_access_validated_at.strftime(DATETIME_FORMAT),
             'logged_in_at': self.logged_in_at.strftime(DATETIME_FORMAT) if self.logged_in_at else None,
             'mobile_number': self.mobile_number,
             'organisations': [x.id for x in self.organisations if x.active],
-            'password_changed_at': (
-                self.password_changed_at.strftime(DATETIME_FORMAT_NO_TIMEZONE)
-                if self.password_changed_at
-                else None
-            ),
+            'password_changed_at': self.password_changed_at.strftime(DATETIME_FORMAT_NO_TIMEZONE),
             'permissions': self.get_permissions(),
             'platform_admin': self.platform_admin,
             'services': [x.id for x in self.services if x.active],

app/template_statistics/rest.py

@@ -1,5 +1,7 @@
 from flask import Blueprint, jsonify, request
-from app.dao.notifications_dao import dao_get_last_template_usage
+
+from app import DATETIME_FORMAT
+from app.dao.notifications_dao import dao_get_last_template_usage, dao_get_last_date_template_was_used
 from app.dao.templates_dao import dao_get_template_by_id_and_service_id
 from app.dao.fact_notification_status_dao import fetch_notification_status_for_service_for_today_and_7_previous_days
 
@@ -52,3 +54,14 @@ def get_template_statistics_for_template_id(service_id, template_id):
         data = notification_with_template_schema.dump(notification).data
 
     return jsonify(data=data)
+
+
+@template_statistics.route('/last-used/<uuid:template_id>')
+def get_last_used_datetime_for_template(service_id, template_id):
+    # Check the template and service exist
+    dao_get_template_by_id_and_service_id(template_id, service_id)
+
+    last_date_used = dao_get_last_date_template_was_used(template_id=template_id,
+                                                         service_id=service_id)
+
+    return jsonify(last_date_used=last_date_used.strftime(DATETIME_FORMAT) if last_date_used else last_date_used)

app/user/rest.py

@@ -197,7 +197,7 @@ def verify_user_code(user_id):
 
     user_to_verify.current_session_id = str(uuid.uuid4())
     user_to_verify.logged_in_at = datetime.utcnow()
-    if user_to_verify.auth_type == 'email_auth':
+    if data['code_type'] == 'email':
         user_to_verify.email_access_validated_at = datetime.utcnow()
     user_to_verify.failed_login_count = 0
     save_model_user(user_to_verify)

migrations/versions/0314_populate_email_access.py

@@ -0,0 +1,43 @@
+"""
+
+Revision ID: 0314_populate_email_access
+Revises: 0313_email_access_validated_at
+Create Date: 2020-01-31 10:35:44.524606
+
+"""
+from alembic import op
+
+
+revision = '0314_populate_email_access'
+down_revision = '0313_email_access_validated_at'
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    # if user has email_auth, set email_access_validated_at on last login, else set it at user created_at date.
+    op.execute("""
+        UPDATE
+            users
+        SET
+            email_access_validated_at = created_at
+        WHERE
+            email_access_validated_at IS NULL
+    """)
+    op.execute("""
+        UPDATE
+            users
+        SET
+            email_access_validated_at = logged_in_at
+        WHERE
+            auth_type = 'email_auth'
+        AND
+            logged_in_at IS NOT NULL
+    """)
+    op.alter_column('users', 'email_access_validated_at', nullable=False)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column('users', 'email_access_validated_at', nullable=True)
+    # ### end Alembic commands ###

requirements-app.txt

@@ -11,7 +11,7 @@ git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd
 Flask==1.1.1
 click-datetime==0.2
 eventlet==0.25.1
-gunicorn==19.7.1  # pyup: ignore, >19.8 breaks eventlet patching
+gunicorn==20.0.4
 iso8601==0.1.12
 jsonschema==3.1.1
 marshmallow-sqlalchemy==0.19.0

requirements.txt

@@ -13,7 +13,7 @@ git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd
 Flask==1.1.1
 click-datetime==0.2
 eventlet==0.25.1
-gunicorn==19.7.1  # pyup: ignore, >19.8 breaks eventlet patching
+gunicorn==20.0.4
 iso8601==0.1.12
 jsonschema==3.1.1
 marshmallow-sqlalchemy==0.19.0
@@ -40,12 +40,12 @@ alembic==1.3.3
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.17.8
+awscli==1.17.9
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.0
 boto3==1.10.38
-botocore==1.14.8
+botocore==1.14.9
 certifi==2019.11.28
 chardet==3.0.4
 Click==7.0
@@ -56,15 +56,14 @@ flask-redis==0.4.0
 future==0.18.2
 greenlet==0.4.15
 idna==2.8
-importlib-metadata==1.4.0
-Jinja2==2.10.3
+importlib-metadata==1.5.0
+Jinja2==2.11.1
 jmespath==0.9.4
 kombu==3.0.37
 Mako==1.1.1
 MarkupSafe==1.1.1
 mistune==0.8.4
 monotonic==1.5
-more-itertools==8.1.0
 orderedset==2.0.1
 phonenumbers==8.11.1
 pyasn1==0.4.8
@@ -76,7 +75,7 @@ python-editor==1.0.4
 python-json-logger==0.1.11
 pytz==2019.3
 PyYAML==5.2
-redis==3.3.11
+redis==3.4.1
 requests==2.22.0
 rsa==3.4.2
 s3transfer==0.3.2
@@ -85,5 +84,5 @@ smartypants==2.0.1
 statsd==3.3.0
 urllib3==1.25.8
 webencodings==0.5.1
-Werkzeug==0.16.0
-zipp==2.0.1
+Werkzeug==0.16.1
+zipp==2.1.0

scripts/run_tests.sh

@@ -31,5 +31,5 @@ flake8 .
 display_result $? 1 "Code style check"
 
 # run with four concurrent threads
-py.test --disable-pytest-warnings --cov=app --cov-report=term-missing tests/ --junitxml=test_results.xml -n4 -v --maxfail=10
+py.test -n4 -v --maxfail=10
 display_result $? 2 "Unit tests"

tests/app/aws/test_s3.py

@@ -2,7 +2,6 @@ from unittest.mock import call
 from datetime import datetime, timedelta
 import pytest
 import pytz
-from flask import current_app
 
 from freezegun import freeze_time
 
@@ -10,7 +9,6 @@ from app.aws.s3 import (
     get_s3_bucket_objects,
     get_s3_file,
     filter_s3_bucket_objects_within_date_range,
-    remove_transformed_dvla_file,
     get_list_of_files_by_suffix,
 )
 from tests.app.conftest import datetime_in_past
@@ -34,18 +32,6 @@ def test_get_s3_file_makes_correct_call(notify_api, mocker):
     )
 
 
-def test_remove_transformed_dvla_file_makes_correct_call(notify_api, mocker):
-    s3_mock = mocker.patch('app.aws.s3.get_s3_object')
-    fake_uuid = '5fbf9799-6b9b-4dbb-9a4e-74a939f3bb49'
-
-    remove_transformed_dvla_file(fake_uuid)
-
-    s3_mock.assert_has_calls([
-        call(current_app.config['DVLA_BUCKETS']['job'], '{}-dvla-job.text'.format(fake_uuid)),
-        call().delete()
-    ])
-
-
 def test_get_s3_bucket_objects_make_correct_pagination_call(notify_api, mocker):
     paginator_mock = mocker.patch('app.aws.s3.client')
 

tests/app/celery/test_nightly_tasks.py

@@ -1,5 +1,4 @@
 from datetime import datetime, timedelta, date
-from functools import partial
 from unittest.mock import call, patch, PropertyMock
 
 import pytest
@@ -18,7 +17,6 @@ from app.celery.nightly_tasks import (
     raise_alert_if_letter_notifications_still_sending,
     remove_letter_csv_files,
     remove_sms_email_csv_files,
-    remove_transformed_dvla_files,
     s3,
     send_daily_performance_platform_stats,
     send_total_sent_notifications_to_performance_platform,
@@ -291,63 +289,6 @@ def test_should_call_delete_inbound_sms(notify_api, mocker):
     assert nightly_tasks.delete_inbound_sms_older_than_retention.call_count == 1
 
 
-@freeze_time('2017-01-01 10:00:00')
-def test_remove_dvla_transformed_files_removes_expected_files(mocker, sample_service):
-    mocker.patch('app.celery.nightly_tasks.s3.remove_transformed_dvla_file')
-
-    letter_template = create_template(service=sample_service, template_type=LETTER_TYPE)
-
-    job = partial(create_job, template=letter_template)
-
-    seven_days_ago = datetime.utcnow() - timedelta(days=7)
-    just_under_seven_days = seven_days_ago + timedelta(seconds=1)
-    just_over_seven_days = seven_days_ago - timedelta(seconds=1)
-    eight_days_ago = seven_days_ago - timedelta(days=1)
-    nine_days_ago = eight_days_ago - timedelta(days=1)
-    ten_days_ago = nine_days_ago - timedelta(days=1)
-    just_under_nine_days = nine_days_ago + timedelta(seconds=1)
-    just_over_nine_days = nine_days_ago - timedelta(seconds=1)
-    just_over_ten_days = ten_days_ago - timedelta(seconds=1)
-
-    job(created_at=just_under_seven_days)
-    job(created_at=just_over_seven_days)
-    job_to_delete_1 = job(created_at=eight_days_ago)
-    job_to_delete_2 = job(created_at=nine_days_ago)
-    job_to_delete_3 = job(created_at=just_under_nine_days)
-    job_to_delete_4 = job(created_at=just_over_nine_days)
-    job(created_at=just_over_ten_days)
-    remove_transformed_dvla_files()
-
-    s3.remove_transformed_dvla_file.assert_has_calls([
-        call(job_to_delete_1.id),
-        call(job_to_delete_2.id),
-        call(job_to_delete_3.id),
-        call(job_to_delete_4.id),
-    ], any_order=True)
-
-
-def test_remove_dvla_transformed_files_does_not_remove_files(mocker, sample_service):
-    mocker.patch('app.celery.nightly_tasks.s3.remove_transformed_dvla_file')
-
-    letter_template = create_template(service=sample_service, template_type=LETTER_TYPE)
-
-    job = partial(create_job, template=letter_template)
-
-    yesterday = datetime.utcnow() - timedelta(days=1)
-    six_days_ago = datetime.utcnow() - timedelta(days=6)
-    seven_days_ago = six_days_ago - timedelta(days=1)
-    just_over_nine_days = seven_days_ago - timedelta(days=2, seconds=1)
-
-    job(created_at=yesterday)
-    job(created_at=six_days_ago)
-    job(created_at=seven_days_ago)
-    job(created_at=just_over_nine_days)
-
-    remove_transformed_dvla_files()
-
-    s3.remove_transformed_dvla_file.assert_has_calls([])
-
-
 @freeze_time("2016-01-01 11:00:00")
 def test_delete_dvla_response_files_older_than_seven_days_removes_old_files(notify_api, mocker):
     AFTER_SEVEN_DAYS = datetime_in_past(days=8)

tests/app/dao/notification_dao/test_notification_dao_template_usage.py

@@ -1,7 +1,7 @@
 from datetime import datetime, timedelta
 import pytest
-from app.dao.notifications_dao import dao_get_last_template_usage
-from tests.app.db import create_notification, create_template
+from app.dao.notifications_dao import dao_get_last_template_usage, dao_get_last_date_template_was_used
+from tests.app.db import create_notification, create_template, create_ft_notification_status
 
 
 def test_last_template_usage_should_get_right_data(sample_notification):
@@ -54,3 +54,33 @@ def test_last_template_usage_should_be_able_to_get_no_template_usage_history_if_
         sample_template):
     results = dao_get_last_template_usage(sample_template.id, 'sms', sample_template.service_id)
     assert not results
+
+
+def test_dao_get_last_date_template_was_used_returns_bst_date_from_stats_table(
+        sample_template
+):
+    last_status_date = (datetime.utcnow() - timedelta(days=2)).date()
+    create_ft_notification_status(bst_date=last_status_date,
+                                  template=sample_template)
+
+    last_used_date = dao_get_last_date_template_was_used(template_id=sample_template.id,
+                                                         service_id=sample_template.service_id)
+    assert last_used_date == last_status_date
+
+
+def test_dao_get_last_date_template_was_used_returns_created_at_from_notifications(
+        sample_template
+):
+    last_notification_date = datetime.utcnow() - timedelta(hours=2)
+    create_notification(template=sample_template, created_at=last_notification_date)
+
+    last_status_date = (datetime.utcnow() - timedelta(days=2)).date()
+    create_ft_notification_status(bst_date=last_status_date, template=sample_template)
+    last_used_date = dao_get_last_date_template_was_used(template_id=sample_template.id,
+                                                         service_id=sample_template.service_id)
+    assert last_used_date == last_notification_date
+
+
+def test_dao_get_last_date_template_was_used_returns_none_if_never_used(sample_template):
+    assert not dao_get_last_date_template_was_used(template_id=sample_template.id,
+                                                   service_id=sample_template.service_id)

tests/app/template_statistics/test_rest.py

@@ -1,10 +1,12 @@
 import uuid
+from datetime import datetime, timedelta
 from unittest.mock import Mock
 
 import pytest
 from freezegun import freeze_time
 
-from tests.app.db import create_notification
+from app import DATETIME_FORMAT
+from tests.app.db import create_ft_notification_status, create_notification
 
 
 def set_up_get_all_from_hash(mock_redis, side_effect):
@@ -189,3 +191,53 @@ def test_get_template_statistics_for_template_returns_empty_for_old_notification
     )
 
     assert not json_resp['data']
+
+
+def test_get_last_used_datetime_for_template(
+    admin_request, sample_template
+):
+    date_from_notification = datetime.utcnow() - timedelta(hours=2)
+    create_notification(template=sample_template, created_at=date_from_notification)
+    date_from_ft_status = (datetime.utcnow() - timedelta(days=2)).date()
+    create_ft_notification_status(bst_date=date_from_ft_status,
+                                  template=sample_template)
+
+    json_resp = admin_request.get(
+        'template_statistics.get_last_used_datetime_for_template',
+        service_id=str(sample_template.service_id),
+        template_id=sample_template.id
+    )
+    assert json_resp['last_date_used'] == date_from_notification.strftime(DATETIME_FORMAT)
+
+
+def test_get_last_used_datetime_for_template_returns_none_if_no_usage_of_template(
+    admin_request, sample_template
+):
+    json_resp = admin_request.get(
+        'template_statistics.get_last_used_datetime_for_template',
+        service_id=str(sample_template.service_id),
+        template_id=sample_template.id
+    )
+    assert json_resp['last_date_used'] is None
+
+
+def test_get_last_used_datetime_for_template_returns_400_if_service_does_not_exist(
+    admin_request, sample_template
+):
+    admin_request.get(
+        'template_statistics.get_last_used_datetime_for_template',
+        service_id=uuid.uuid4(),
+        template_id=sample_template.id,
+        _expected_status=404
+    )
+
+
+def test_get_last_used_datetime_for_template_returns_404_if_template_does_not_exist(
+    admin_request, sample_template
+):
+    admin_request.get(
+        'template_statistics.get_last_used_datetime_for_template',
+        service_id=sample_template.service_id,
+        template_id=uuid.uuid4(),
+        _expected_status=404
+    )

tests/app/user/test_rest_verify.py

@@ -344,6 +344,8 @@ def test_reset_failed_login_count_returns_404_when_user_does_not_exist(client):
     assert resp.status_code == 404
 
 
+# we send sms_auth users email code to validate their email access
+@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth'])
 @pytest.mark.parametrize('data, expected_auth_url', (
     (
         {},
@@ -365,8 +367,10 @@ def test_send_user_email_code(
     email_2fa_code_template,
     data,
     expected_auth_url,
+    auth_type
 ):
     deliver_email = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+    sample_user.auth_type = auth_type
 
     admin_request.post(
         'user.send_user_2fa_code',
@@ -417,10 +421,12 @@ def test_send_email_code_returns_404_for_bad_input_data(admin_request):
 
 
 @freeze_time('2016-01-01T12:00:00')
-def test_user_verify_email_code(admin_request, sample_user):
+# we send sms_auth users email code to validate their email access
+@pytest.mark.parametrize('auth_type', ['email_auth', 'sms_auth'])
+def test_user_verify_email_code(admin_request, sample_user, auth_type):
     sample_user.logged_in_at = datetime.utcnow() - timedelta(days=1)
     sample_user.email_access_validated_at = datetime.utcnow() - timedelta(days=1)
-    sample_user.auth_type = "email_auth"
+    sample_user.auth_type = auth_type
     magic_code = str(uuid.uuid4())
     verify_code = create_user_code(sample_user, magic_code, EMAIL_TYPE)