Display the two new virus states for letters

Precompiled letters can now have two additional states:
* pending-virus-check
* virus-scan-failed

Both new states should show in the notifications dashboard, and
virus-scan-failed should appear as an error state, with a descriptive
message. You should not be able to preview a letter in one of the two
new states, so the preview link has been removed for precompiled letters
in these states.
This commit is contained in:
Katie Smith
2018-03-19 15:25:26 +00:00
parent 6ba4cbc93c
commit 4db75f6a58
9 changed files with 101 additions and 14 deletions

View File

@@ -344,7 +344,9 @@ def format_notification_status(status, template_type):
'delivered': 'Delivered', 'delivered': 'Delivered',
'sending': 'Sending', 'sending': 'Sending',
'created': 'Sending', 'created': 'Sending',
'sent': 'Delivered' 'sent': 'Delivered',
'pending-virus-check': 'Pending virus check',
'virus-scan-failed': 'Virus detected',
} }
}[template_type].get(status, status) }[template_type].get(status, status)
@@ -368,6 +370,8 @@ def format_notification_status_as_field_status(status, notification_type):
'sending': None, 'sending': None,
'created': None, 'created': None,
'accepted': None, 'accepted': None,
'pending-virus-check': None,
'virus-scan-failed': 'error',
} }
}.get( }.get(
notification_type, notification_type,

View File

@@ -140,7 +140,7 @@
{% if notification.status|format_notification_status_as_url %} {% if notification.status|format_notification_status_as_url %}
<a href="{{ notification.status|format_notification_status_as_url }}"> <a href="{{ notification.status|format_notification_status_as_url }}">
{% endif %} {% endif %}
{% if notification['notification_type'] != "letter" %} {% if notification['notification_type'] != "letter" or notification.status == 'virus-scan-failed' %}
{{ notification.status|format_notification_status( {{ notification.status|format_notification_status(
notification.template.template_type notification.template.template_type
) }} ) }}

View File

@@ -16,7 +16,11 @@
field_headings_visible=False field_headings_visible=False
) %} ) %}
{% call row_heading() %} {% call row_heading() %}
<a class="file-list-filename" href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=item.id) }}">{{ item.to }}</a> {% if item.status in ('pending-virus-check', 'virus-scan-failed') %}
<span class="file-list-filename">{{ item.to }}</span>
{% else %}
<a class="file-list-filename" href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=item.id) }}">{{ item.to }}</a>
{% endif %}
<p class="file-list-hint"> <p class="file-list-hint">
{{ item.preview_of_content }} {{ item.preview_of_content }}
</p> </p>

View File

@@ -73,7 +73,7 @@
<dd class="api-notifications-item-data-item">{{ notification[key] }}</dd> <dd class="api-notifications-item-data-item">{{ notification[key] }}</dd>
{% endif %} {% endif %}
{% endfor %} {% endfor %}
{% if notification['notification_type'] == 'letter' %} {% if notification['notification_type'] == 'letter' and notification.status not in ('pending-virus-check', 'virus-scan-failed') %}
<a href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=notification.id) }}">View letter</a> <a href="{{ url_for('.view_notification', service_id=current_service.id, notification_id=notification.id) }}">View letter</a>
{% endif %} {% endif %}
</dl> </dl>

View File

@@ -27,9 +27,9 @@ from notifications_utils.template import (
from orderedset._orderedset import OrderedSet from orderedset._orderedset import OrderedSet
from werkzeug.datastructures import MultiDict from werkzeug.datastructures import MultiDict
SENDING_STATUSES = ['created', 'pending', 'sending'] SENDING_STATUSES = ['created', 'pending', 'sending', 'pending-virus-check']
DELIVERED_STATUSES = ['delivered', 'sent'] DELIVERED_STATUSES = ['delivered', 'sent']
FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed']
REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES

View File

@@ -26,14 +26,14 @@ from app.main.views.jobs import get_status_filters, get_time_left
( (
'', '',
[ [
'created', 'pending', 'sending', 'created', 'pending', 'sending', 'pending-virus-check',
'delivered', 'sent', 'delivered', 'sent',
'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed',
] ]
), ),
( (
'sending', 'sending',
['sending', 'created', 'pending'] ['sending', 'created', 'pending', 'pending-virus-check']
), ),
( (
'delivered', 'delivered',
@@ -41,7 +41,7 @@ from app.main.views.jobs import get_status_filters, get_time_left
), ),
( (
'failed', 'failed',
['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed']
) )
] ]
) )
@@ -135,6 +135,60 @@ def test_can_show_notifications(
assert json_content.keys() == {'counts', 'notifications'} assert json_content.keys() == {'counts', 'notifications'}
def test_letters_with_status_virus_scan_failed_shows_a_failure_description(
mocker,
active_user_with_permissions,
logged_in_client,
service_one,
mock_get_detailed_service,
):
mock_get_notifications(
mocker,
active_user_with_permissions,
is_precompiled_letter=True,
noti_status='virus-scan-failed'
)
response = logged_in_client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
message_type='letter',
status='',
))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
error_description = page.find('div', attrs={'class': 'table-field-status-error'}).text.strip()
assert 'Virus detected\n' in error_description
@pytest.mark.parametrize('letter_status', [
'pending-virus-check', 'virus-scan-failed'
])
def test_should_not_show_preview_link_for_precompiled_letters_in_virus_states(
mocker,
active_user_with_permissions,
logged_in_client,
service_one,
mock_get_detailed_service,
letter_status,
):
mock_get_notifications(
mocker,
active_user_with_permissions,
is_precompiled_letter=True,
noti_status=letter_status
)
response = logged_in_client.get(url_for(
'main.view_notifications',
service_id=service_one['id'],
message_type='letter',
status='',
))
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert not page.find('a', attrs={'class': 'file-list-filename'})
def test_shows_message_when_no_notifications( def test_shows_message_when_no_notifications(
client_request, client_request,
mock_get_detailed_service, mock_get_detailed_service,

View File

@@ -89,6 +89,27 @@ def test_letter_notifications_should_have_link_to_view_letter(
assert (page.select_one('details a') is not None) == has_links assert (page.select_one('details a') is not None) == has_links
@pytest.mark.parametrize('status', [
'pending-virus-check', 'virus-scan-failed'
])
def test_should_not_have_link_to_view_letter_for_precompiled_letters_in_virus_states(
client_request,
api_user_active,
fake_uuid,
mock_has_permissions,
mocker,
status
):
mock_get_notifications(mocker, api_user_active, noti_status=status)
page = client_request.get(
'main.api_integration',
service_id=fake_uuid,
)
assert not page.select_one('details a')
@pytest.mark.parametrize('client_reference, shows_ref', [ @pytest.mark.parametrize('client_reference, shows_ref', [
('foo', True), ('foo', True),
(None, False), (None, False),

View File

@@ -49,14 +49,14 @@ def test_get_jobs_shows_page_links(
( (
'', '',
[ [
'created', 'pending', 'sending', 'created', 'pending', 'sending', 'pending-virus-check',
'delivered', 'sent', 'delivered', 'sent',
'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed',
] ]
), ),
( (
'sending', 'sending',
['sending', 'created', 'pending'] ['sending', 'created', 'pending', 'pending-virus-check']
), ),
( (
'delivered', 'delivered',
@@ -64,7 +64,7 @@ def test_get_jobs_shows_page_links(
), ),
( (
'failed', 'failed',
['failed', 'temporary-failure', 'permanent-failure', 'technical-failure'] ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed']
) )
] ]
) )
@@ -200,12 +200,14 @@ def test_should_show_letter_job(
'created', 'created',
'pending', 'pending',
'sending', 'sending',
'pending-virus-check',
'delivered', 'delivered',
'sent', 'sent',
'failed', 'failed',
'temporary-failure', 'temporary-failure',
'permanent-failure', 'permanent-failure',
'technical-failure', 'technical-failure',
'virus-scan-failed',
], ],
) )

View File

@@ -1766,6 +1766,7 @@ def mock_get_notifications(
redact_personalisation=False, redact_personalisation=False,
is_precompiled_letter=False, is_precompiled_letter=False,
client_reference=None, client_reference=None,
noti_status=None,
): ):
def _get_notifications( def _get_notifications(
service_id, service_id,
@@ -1807,6 +1808,7 @@ def mock_get_notifications(
personalisation=personalisation, personalisation=personalisation,
template_type=diff_template_type, template_type=diff_template_type,
client_reference=client_reference, client_reference=client_reference,
status=noti_status,
) )
return mocker.patch( return mocker.patch(