diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index ed45c3c1d..7b3c701f9 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,5 +1,4 @@ import json -from datetime import datetime from flask import ( current_app, @@ -29,8 +28,7 @@ def new_password(token): email_address = json.loads(token_data)['email'] user = User.from_email_address(email_address) - if user.password_changed_at and datetime.strptime(user.password_changed_at, '%Y-%m-%d %H:%M:%S.%f') > \ - datetime.strptime(json.loads(token_data)['created_at'], '%Y-%m-%d %H:%M:%S.%f'): + if user.password_changed_more_recently_than(json.loads(token_data)['created_at']): flash('The link in the email has already been used') return redirect(url_for('main.index')) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 4408cd85d..75de09ad6 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -14,6 +14,7 @@ from flask import ( ) from flask_login import current_user from notifications_python_client.errors import HTTPError +from notifications_utils.timezones import utc_string_to_aware_gmt_datetime from app import ( billing_api_client, @@ -475,8 +476,10 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id): email_address=notification["to"], is_default=is_default ) - created_at_no_tz = notification["created_at"][:-6] - seconds_since_sending = (datetime.utcnow() - datetime.strptime(created_at_no_tz, '%Y-%m-%dT%H:%M:%S.%f')).seconds + seconds_since_sending = ( + utc_string_to_aware_gmt_datetime(datetime.utcnow().isoformat()) - + utc_string_to_aware_gmt_datetime(notification['created_at']) + ).seconds if notification["status"] in FAILURE_STATUSES or ( notification["status"] in SENDING_STATUSES and seconds_since_sending > current_app.config['REPLY_TO_EMAIL_ADDRESS_VALIDATION_TIMEOUT'] diff --git a/app/models/job.py b/app/models/job.py index 384edfd4d..f03b045fc 100644 --- a/app/models/job.py +++ b/app/models/job.py @@ -5,6 +5,10 @@ from notifications_utils.letter_timings import ( get_letter_timings, letter_can_be_cancelled, ) +from notifications_utils.timezones import ( + local_timezone, + utc_string_to_aware_gmt_datetime, +) from werkzeug.utils import cached_property from app.models import JSONModel, ModelList @@ -23,6 +27,7 @@ class Job(JSONModel): 'template_version', 'original_file_name', 'created_at', + 'processing_started', 'notification_count', 'created_by', } @@ -47,6 +52,12 @@ class Job(JSONModel): def scheduled_for(self): return self._dict.get('scheduled_for') + @property + def processing_started(self): + if not self._dict.get('processing_started'): + return None + return utc_string_to_aware_gmt_datetime(self._dict['processing_started']) + def _aggregate_statistics(self, *statuses): return sum( outcome['count'] for outcome in self._dict['statistics'] @@ -94,6 +105,17 @@ class Job(JSONModel): def finished_processing(self): return self.notification_count == self.notifications_sent + @property + def awaiting_processing_or_recently_processed(self): + if not self.processing_started: + # Assume that if processing hasn’t started yet then the job + # must have been created recently enough to not have any + # notifications yet + return True + return ( + datetime.utcnow().astimezone(local_timezone) - self.processing_started + ).days < 1 + @property def template_id(self): return self._dict['template'] @@ -124,7 +146,8 @@ class Job(JSONModel): return False if not letter_can_be_cancelled( - 'created', datetime.strptime(self.created_at[:-6], '%Y-%m-%dT%H:%M:%S.%f') + 'created', + utc_string_to_aware_gmt_datetime(self.created_at).replace(tzinfo=None) ): return False diff --git a/app/models/user.py b/app/models/user.py index bbe89fa2b..a1876d765 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -1,6 +1,7 @@ from flask import abort, current_app, request, session from flask_login import AnonymousUserMixin, UserMixin, login_user from notifications_python_client.errors import HTTPError +from notifications_utils.timezones import utc_string_to_aware_gmt_datetime from werkzeug.utils import cached_property from app.models import JSONModel, ModelList @@ -108,6 +109,15 @@ class User(JSONModel, UserMixin): response = user_api_client.update_password(self.id, password) self.__init__(response) + def password_changed_more_recently_than(self, datetime_string): + if not self.password_changed_at: + return False + return utc_string_to_aware_gmt_datetime( + self.password_changed_at + ) > utc_string_to_aware_gmt_datetime( + datetime_string + ) + def set_permissions(self, service_id, permissions, folder_permissions): user_api_client.set_user_permissions( self.id, diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index fa7ea707b..869573da2 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -47,7 +47,7 @@ notifications, caption=uploaded_file_name, caption_visible=False, - empty_message='These messages have been deleted because they were sent more than {} days ago'.format(service_data_retention_days) if job.status == 'finished' else 'No messages to show yet…', + empty_message='No messages to show yet…' if job.awaiting_processing_or_recently_processed else 'These messages have been deleted because they were sent more than {} days ago'.format(service_data_retention_days), field_headings=[ 'Recipient', 'Status' diff --git a/tests/__init__.py b/tests/__init__.py index 3a3554cb4..d1382c2b3 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -361,7 +361,8 @@ def job_json( notifications_sent=1, notifications_requested=1, job_status='finished', - scheduled_for='' + scheduled_for='', + processing_started=None, ): if job_id is None: job_id = str(generate_uuid()) @@ -391,8 +392,11 @@ def job_json( created_by['name'], created_by['email_address'], ), - 'scheduled_for': scheduled_for } + if scheduled_for: + data.update(scheduled_for=scheduled_for) + if processing_started: + data.update(processing_started=processing_started) return data diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index b18f2ccce..f6e9b0ea4 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -1,5 +1,6 @@ import json import uuid +from datetime import datetime, timezone import pytest from flask import url_for @@ -329,28 +330,68 @@ def test_should_show_job_without_notifications( assert page.select_one('tbody').text.strip() == 'No messages to show yet…' +@freeze_time("2020-01-10 1:0:0") +@pytest.mark.parametrize('created_at, processing_started, expected_message', ( + # Recently created, not yet started + (datetime(2020, 1, 10, 0, 0, 0), None, ( + 'No messages to show yet…' + )), + # Just started + (datetime(2020, 1, 10, 0, 0, 0), datetime(2020, 1, 10, 0, 0, 1), ( + 'No messages to show yet…' + )), + # Created a while ago, just started + (datetime(2020, 1, 1, 0, 0, 0), datetime(2020, 1, 10, 0, 0, 1), ( + 'No messages to show yet…' + )), + # Created a while ago, started just within the last 24h + (datetime(2020, 1, 1, 0, 0, 0), datetime(2020, 1, 9, 1, 0, 1), ( + 'No messages to show yet…' + )), + # Created a while ago, started exactly 24h ago + # --- + # It doesn’t matter that 24h (1 day) and 7 days (the service’s data + # retention) don’t match up. We’re testing the case of no + # notifications existing more than 1 day after the job started + # processing. In this case we assume it’s because the service’s + # data retention has kicked in. + (datetime(2020, 1, 1, 0, 0, 0), datetime(2020, 1, 9, 1, 0, 0), ( + 'These messages have been deleted because they were sent more than 7 days ago' + )), +)) def test_should_show_old_job( client_request, service_one, active_user_with_permissions, mock_get_service_template, - mock_get_job, mocker, mock_get_notifications_with_no_notifications, mock_get_service_data_retention, fake_uuid, + created_at, + processing_started, + expected_message, ): + mocker.patch('app.job_api_client.get_job', return_value={ + "data": job_json( + SERVICE_ONE_ID, + active_user_with_permissions, + created_at=created_at.replace(tzinfo=timezone.utc).isoformat(), + processing_started=( + processing_started.replace(tzinfo=timezone.utc).isoformat() + if processing_started else None + ), + ), + }) page = client_request.get( 'main.view_job', - service_id=service_one['id'], + service_id=SERVICE_ONE_ID, job_id=fake_uuid, ) assert not page.select('.pill a') assert not page.select('p.hint') assert not page.select('a[download]') - assert page.select_one('tbody').text.strip() == ( - 'These messages have been deleted because they were sent more than 7 days ago' - ) + assert page.select_one('tbody').text.strip() == expected_message @freeze_time("2016-01-01 11:09:00.061258")