Merge branch 'master' into fix-team-recipient-check

This commit is contained in:
Rebecca Law
2019-06-25 15:58:40 +01:00
15 changed files with 473 additions and 106 deletions

View File

@@ -23,6 +23,7 @@ from app.celery.letters_pdf_tasks import (
process_virus_scan_failed,
process_virus_scan_error,
replay_letters_in_error,
_move_invalid_letter_and_update_status,
_sanitise_precompiled_pdf
)
from app.letters.utils import get_letter_pdf_filename, ScanErrorType
@@ -46,6 +47,10 @@ from tests.conftest import set_config_values
def test_should_have_decorated_tasks_functions():
assert create_letters_pdf.__wrapped__.__name__ == 'create_letters_pdf'
assert collate_letter_pdfs_for_day.__wrapped__.__name__ == 'collate_letter_pdfs_for_day'
assert process_virus_scan_passed.__wrapped__.__name__ == 'process_virus_scan_passed'
assert process_virus_scan_failed.__wrapped__.__name__ == 'process_virus_scan_failed'
assert process_virus_scan_error.__wrapped__.__name__ == 'process_virus_scan_error'
@pytest.mark.parametrize('personalisation', [{'name': 'test'}, None])
@@ -517,6 +522,67 @@ def test_process_letter_task_check_virus_scan_passed_when_file_cannot_be_opened(
assert sample_letter_notification.billable_units == 0
@mock_s3
def test_process_virus_scan_passed_logs_error_and_sets_tech_failure_if_s3_error_uploading_to_live_bucket(
mocker,
sample_letter_notification,
):
mock_logger = mocker.patch('app.celery.tasks.current_app.logger.exception')
sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK
filename = 'NOTIFY.{}'.format(sample_letter_notification.reference)
source_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
conn = boto3.resource('s3', region_name='eu-west-1')
conn.create_bucket(Bucket=source_bucket_name)
s3 = boto3.client('s3', region_name='eu-west-1')
s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content')
mocker.patch('app.celery.letters_pdf_tasks._get_page_count', return_value=1)
mocker.patch('app.celery.letters_pdf_tasks._sanitise_precompiled_pdf', return_value=b'pdf_content')
error_response = {
'Error': {
'Code': 'InvalidParameterValue',
'Message': 'some error message from amazon',
'Type': 'Sender'
}
}
mocker.patch('app.celery.letters_pdf_tasks._upload_pdf_to_test_or_live_pdf_bucket',
side_effect=ClientError(error_response, 'operation_name'))
process_virus_scan_passed(filename)
assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE
mock_logger.assert_called_once_with(
'Error uploading letter to live pdf bucket for notification: {}'.format(sample_letter_notification.id)
)
def test_move_invalid_letter_and_update_status_logs_error_and_sets_tech_failure_state_if_s3_error(
mocker,
sample_letter_notification,
):
error_response = {
'Error': {
'Code': 'InvalidParameterValue',
'Message': 'some error message from amazon',
'Type': 'Sender'
}
}
mocker.patch('app.celery.letters_pdf_tasks.move_scan_to_invalid_pdf_bucket',
side_effect=ClientError(error_response, 'operation_name'))
mock_logger = mocker.patch('app.celery.tasks.current_app.logger.exception')
_move_invalid_letter_and_update_status(sample_letter_notification, 'filename', mocker.Mock())
assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE
mock_logger.assert_called_once_with(
'Error when moving letter with id {} to invalid PDF bucket'.format(sample_letter_notification.id)
)
def test_process_letter_task_check_virus_scan_failed(sample_letter_notification, mocker):
filename = 'NOTIFY.{}'.format(sample_letter_notification.reference)
sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK

View File

@@ -13,7 +13,9 @@ from app.celery.scheduled_tasks import (
run_scheduled_jobs,
send_scheduled_notifications,
switch_current_sms_provider_on_slow_delivery,
replay_created_notifications
replay_created_notifications,
check_precompiled_letter_state,
check_templated_letter_state,
)
from app.config import QueueNames, TaskNames
from app.dao.jobs_dao import dao_get_job_by_id
@@ -26,6 +28,8 @@ from app.models import (
JOB_STATUS_IN_PROGRESS,
JOB_STATUS_ERROR,
JOB_STATUS_FINISHED,
NOTIFICATION_DELIVERED,
NOTIFICATION_PENDING_VIRUS_CHECK,
)
from app.v2.errors import JobIncompleteError
@@ -58,7 +62,7 @@ def prepare_current_provider(restore_provider_details):
db.session.commit()
def test_should_call_delete_codes_on_delete_verify_codes_task(notify_api, mocker):
def test_should_call_delete_codes_on_delete_verify_codes_task(notify_db_session, mocker):
mocker.patch('app.celery.scheduled_tasks.delete_codes_older_created_more_than_a_day_ago')
delete_verify_codes()
assert scheduled_tasks.delete_codes_older_created_more_than_a_day_ago.call_count == 1
@@ -334,3 +338,84 @@ def test_check_job_status_task_does_not_raise_error(sample_template):
job_status=JOB_STATUS_FINISHED)
check_job_status()
@freeze_time("2019-05-30 14:00:00")
def test_check_precompiled_letter_state(mocker, sample_letter_template):
mock_logger = mocker.patch('app.celery.tasks.current_app.logger.exception')
mock_create_ticket = mocker.patch('app.celery.nightly_tasks.zendesk_client.create_ticket')
create_notification(template=sample_letter_template,
status=NOTIFICATION_PENDING_VIRUS_CHECK,
created_at=datetime.utcnow() - timedelta(seconds=5400))
create_notification(template=sample_letter_template,
status=NOTIFICATION_DELIVERED,
created_at=datetime.utcnow() - timedelta(seconds=6000))
noti_1 = create_notification(template=sample_letter_template,
status=NOTIFICATION_PENDING_VIRUS_CHECK,
created_at=datetime.utcnow() - timedelta(seconds=5401))
noti_2 = create_notification(template=sample_letter_template,
status=NOTIFICATION_PENDING_VIRUS_CHECK,
created_at=datetime.utcnow() - timedelta(seconds=70000))
check_precompiled_letter_state()
message = "2 precompiled letters have been pending-virus-check for over 90 minutes. " \
"Notifications: ['{}', '{}']".format(noti_2.id, noti_1.id)
mock_logger.assert_called_once_with(message)
mock_create_ticket.assert_called_with(
message=message,
subject='[test] Letters still pending virus check',
ticket_type='incident'
)
@freeze_time("2019-05-30 14:00:00")
def test_check_templated_letter_state_during_bst(mocker, sample_letter_template):
mock_logger = mocker.patch('app.celery.tasks.current_app.logger.exception')
mock_create_ticket = mocker.patch('app.celery.nightly_tasks.zendesk_client.create_ticket')
noti_1 = create_notification(template=sample_letter_template, created_at=datetime(2019, 5, 1, 12, 0))
noti_2 = create_notification(template=sample_letter_template, created_at=datetime(2019, 5, 29, 16, 29))
create_notification(template=sample_letter_template, created_at=datetime(2019, 5, 29, 16, 30))
create_notification(template=sample_letter_template, created_at=datetime(2019, 5, 29, 17, 29))
create_notification(template=sample_letter_template, status='delivered', created_at=datetime(2019, 5, 28, 10, 0))
create_notification(template=sample_letter_template, created_at=datetime(2019, 5, 30, 10, 0))
check_templated_letter_state()
message = "2 letters were created before 17.30 yesterday and still have 'created' status. " \
"Notifications: ['{}', '{}']".format(noti_1.id, noti_2.id)
mock_logger.assert_called_once_with(message)
mock_create_ticket.assert_called_with(
message=message,
subject="[test] Letters still in 'created' status",
ticket_type='incident'
)
@freeze_time("2019-01-30 14:00:00")
def test_check_templated_letter_state_during_utc(mocker, sample_letter_template):
mock_logger = mocker.patch('app.celery.tasks.current_app.logger.exception')
mock_create_ticket = mocker.patch('app.celery.nightly_tasks.zendesk_client.create_ticket')
noti_1 = create_notification(template=sample_letter_template, created_at=datetime(2018, 12, 1, 12, 0))
noti_2 = create_notification(template=sample_letter_template, created_at=datetime(2019, 1, 29, 17, 29))
create_notification(template=sample_letter_template, created_at=datetime(2019, 1, 29, 17, 30))
create_notification(template=sample_letter_template, created_at=datetime(2019, 1, 29, 18, 29))
create_notification(template=sample_letter_template, status='delivered', created_at=datetime(2019, 1, 29, 10, 0))
create_notification(template=sample_letter_template, created_at=datetime(2019, 1, 30, 10, 0))
check_templated_letter_state()
message = "2 letters were created before 17.30 yesterday and still have 'created' status. " \
"Notifications: ['{}', '{}']".format(noti_1.id, noti_2.id)
mock_logger.assert_called_once_with(message)
mock_create_ticket.assert_called_with(
message=message,
subject="[test] Letters still in 'created' status",
ticket_type='incident'
)

View File

@@ -26,8 +26,10 @@ def test_get_all_organisations(admin_request, notify_db_session):
assert len(response) == 2
assert response[0]['name'] == 'active org'
assert response[0]['active'] is True
assert response[0]['count_of_live_services'] == 0
assert response[1]['name'] == 'inactive org'
assert response[1]['active'] is False
assert response[1]['count_of_live_services'] == 0
def test_get_organisation_by_id(admin_request, notify_db_session):
@@ -49,10 +51,13 @@ def test_get_organisation_by_id(admin_request, notify_db_session):
'agreement_signed_at',
'agreement_signed_by_id',
'agreement_signed_version',
'agreement_signed_on_behalf_of_name',
'agreement_signed_on_behalf_of_email_address',
'letter_branding_id',
'email_branding_id',
'domains',
'request_to_go_live_notes',
'count_of_live_services',
}
assert response['id'] == str(org.id)
assert response['name'] == 'test_org_1'
@@ -66,6 +71,9 @@ def test_get_organisation_by_id(admin_request, notify_db_session):
assert response['email_branding_id'] is None
assert response['domains'] == []
assert response['request_to_go_live_notes'] is None
assert response['count_of_live_services'] == 0
assert response['agreement_signed_on_behalf_of_name'] is None
assert response['agreement_signed_on_behalf_of_email_address'] is None
def test_get_organisation_by_id_returns_domains(admin_request, notify_db_session):
@@ -193,6 +201,8 @@ def test_post_update_organisation_updates_fields(
'active': False,
'agreement_signed': agreement_signed,
'crown': crown,
'agreement_signed_on_behalf_of_name': 'Firstname Lastname',
'agreement_signed_on_behalf_of_email_address': 'test@example.com',
}
assert org.agreement_signed is None
assert org.crown is None
@@ -213,6 +223,8 @@ def test_post_update_organisation_updates_fields(
assert organisation[0].agreement_signed == agreement_signed
assert organisation[0].crown == crown
assert organisation[0].domains == []
assert organisation[0].agreement_signed_on_behalf_of_name == 'Firstname Lastname'
assert organisation[0].agreement_signed_on_behalf_of_email_address == 'test@example.com'
@pytest.mark.parametrize('domain_list', (

View File

@@ -1,4 +1,4 @@
from datetime import datetime, timedelta
from datetime import datetime, timedelta, date
from freezegun import freeze_time
@@ -19,7 +19,7 @@ def test_send_processing_time_to_performance_platform_generates_correct_calls(mo
create_notification(sample_template, created_at=created_at, sent_at=created_at + timedelta(seconds=15))
create_notification(sample_template, created_at=datetime.utcnow() - timedelta(days=2))
send_processing_time_to_performance_platform()
send_processing_time_to_performance_platform(date(2016, 10, 17))
send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-total', 2)
send_mock.assert_any_call(datetime(2016, 10, 16, 23, 0), 'messages-within-10-secs', 1)

View File

@@ -835,38 +835,63 @@ def test_get_orgs_and_services_nests_services(admin_request, sample_user):
resp = admin_request.get('user.get_organisations_and_services_for_user', user_id=sample_user.id)
assert resp == {
'organisations': [
{
'name': org1.name,
'id': str(org1.id),
'services': [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
},
{
'name': service2.name,
'id': str(service2.id),
'restricted': False,
}
]
},
{
'name': org2.name,
'id': str(org2.id),
'services': []
}
],
'services_without_organisations': [
{
'name': service3.name,
'id': str(service3.id),
'restricted': False,
}
]
assert set(resp.keys()) == {
'organisations',
'services_without_organisations',
'services',
}
assert resp['organisations'] == [
{
'name': org1.name,
'id': str(org1.id),
'services': [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
},
{
'name': service2.name,
'id': str(service2.id),
'restricted': False,
}
],
'count_of_live_services': 2,
},
{
'name': org2.name,
'id': str(org2.id),
'services': [],
'count_of_live_services': 0,
},
]
assert resp['services_without_organisations'] == [
{
'name': service3.name,
'id': str(service3.id),
'restricted': False,
}
]
assert resp['services'] == [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
'organisation': str(org1.id),
},
{
'name': service2.name,
'id': str(service2.id),
'restricted': False,
'organisation': str(org1.id),
},
{
'name': service3.name,
'id': str(service3.id),
'restricted': False,
'organisation': None,
},
]
def test_get_orgs_and_services_only_returns_active(admin_request, sample_user):
@@ -890,28 +915,52 @@ def test_get_orgs_and_services_only_returns_active(admin_request, sample_user):
resp = admin_request.get('user.get_organisations_and_services_for_user', user_id=sample_user.id)
assert resp == {
'organisations': [
{
'name': org1.name,
'id': str(org1.id),
'services': [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
}
]
}
],
'services_without_organisations': [
{
'name': service4.name,
'id': str(service4.id),
'restricted': False,
}
]
assert set(resp.keys()) == {
'organisations',
'services_without_organisations',
'services',
}
assert resp['organisations'] == [
{
'name': org1.name,
'id': str(org1.id),
'services': [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
}
],
'count_of_live_services': 1,
}
]
assert resp['services_without_organisations'] == [
{
'name': service4.name,
'id': str(service4.id),
'restricted': False,
}
]
assert resp['services'] == [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
'organisation': str(org1.id)
},
{
'name': service3.name,
'id': str(service3.id),
'restricted': False,
'organisation': str(org2.id)
},
{
'name': service4.name,
'id': str(service4.id),
'restricted': False,
'organisation': None,
},
]
def test_get_orgs_and_services_only_shows_users_orgs_and_services(admin_request, sample_user):
@@ -932,23 +981,37 @@ def test_get_orgs_and_services_only_shows_users_orgs_and_services(admin_request,
resp = admin_request.get('user.get_organisations_and_services_for_user', user_id=sample_user.id)
assert resp == {
'organisations': [
{
'name': org2.name,
'id': str(org2.id),
'services': []
}
],
# service1 belongs to org1, but the user doesn't know about org1
'services_without_organisations': [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
}
]
assert set(resp.keys()) == {
'organisations',
'services_without_organisations',
'services',
}
assert resp['organisations'] == [
{
'name': org2.name,
'id': str(org2.id),
'services': [],
'count_of_live_services': 0,
}
]
# service1 belongs to org1, but the user doesn't know about org1
assert resp['services_without_organisations'] == [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
}
]
# 'services' always returns the org_id no matter whether the user
# belongs to that org or not
assert resp['services'] == [
{
'name': service1.name,
'id': str(service1.id),
'restricted': False,
'organisation': str(org1.id),
}
]
def test_find_users_by_email_finds_user_by_partial_email(notify_db, client):