mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-18 05:29:38 -04:00
Move check_messages in admin over to using get notification stats for
day.
This commit is contained in:
@@ -1,5 +1,6 @@
|
|||||||
import json
|
import json
|
||||||
import itertools
|
import itertools
|
||||||
|
from datetime import datetime
|
||||||
from string import ascii_uppercase
|
from string import ascii_uppercase
|
||||||
|
|
||||||
from contextlib import suppress
|
from contextlib import suppress
|
||||||
@@ -205,12 +206,16 @@ def send_from_api(service_id, template_id):
|
|||||||
@login_required
|
@login_required
|
||||||
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
|
@user_has_permissions('send_texts', 'send_emails', 'send_letters')
|
||||||
def check_messages(service_id, template_type, upload_id):
|
def check_messages(service_id, template_type, upload_id):
|
||||||
|
|
||||||
if not session.get('upload_data'):
|
if not session.get('upload_data'):
|
||||||
return redirect(url_for('main.choose_template', service_id=service_id, template_type=template_type))
|
return redirect(url_for('main.choose_template', service_id=service_id, template_type=template_type))
|
||||||
|
|
||||||
users = user_api_client.get_users_for_service(service_id=service_id)
|
users = user_api_client.get_users_for_service(service_id=service_id)
|
||||||
statistics = statistics_api_client.get_statistics_for_service(service_id, limit_days=1)['data']
|
today = datetime.today().date().strftime('%Y-%m-%d')
|
||||||
statistics = statistics[0] if statistics else {}
|
|
||||||
|
statistics = statistics_api_client.get_statistics_for_service_for_day(service_id, today)
|
||||||
|
if not statistics:
|
||||||
|
statistics = {}
|
||||||
|
|
||||||
contents = s3download(service_id, upload_id)
|
contents = s3download(service_id, upload_id)
|
||||||
if not contents:
|
if not contents:
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
from notifications_python_client.base import BaseAPIClient
|
from notifications_python_client.base import BaseAPIClient
|
||||||
|
from notifications_python_client.errors import HTTPError
|
||||||
|
|
||||||
|
|
||||||
class StatisticsApiClient(BaseAPIClient):
|
class StatisticsApiClient(BaseAPIClient):
|
||||||
@@ -21,6 +22,16 @@ class StatisticsApiClient(BaseAPIClient):
|
|||||||
params=params
|
params=params
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def get_statistics_for_service_for_day(self, service_id, day):
|
||||||
|
url = '/service/{}/notifications-statistics/day/{}'.format(service_id, day)
|
||||||
|
try:
|
||||||
|
return self.get(url=url)['data']
|
||||||
|
except HTTPError as e:
|
||||||
|
if e.status_code == 404:
|
||||||
|
return None
|
||||||
|
else:
|
||||||
|
raise e
|
||||||
|
|
||||||
def get_7_day_aggregate_for_service(self, service_id, date_from=None, week_count=None):
|
def get_7_day_aggregate_for_service(self, service_id, date_from=None, week_count=None):
|
||||||
params = {}
|
params = {}
|
||||||
if date_from is not None:
|
if date_from is not None:
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import uuid
|
import uuid
|
||||||
|
from datetime import datetime
|
||||||
|
|
||||||
from app.notify_client.statistics_api_client import StatisticsApiClient
|
from app.notify_client.statistics_api_client import StatisticsApiClient
|
||||||
|
|
||||||
@@ -29,3 +30,19 @@ def test_notifications_statistics_client_calls_correct_api_endpoint_with_params(
|
|||||||
client.get_statistics_for_service(some_service_id, limit_days=99)
|
client.get_statistics_for_service(some_service_id, limit_days=99)
|
||||||
|
|
||||||
mock_get.assert_called_once_with(url=expected_url, params={'limit_days': 99})
|
mock_get.assert_called_once_with(url=expected_url, params={'limit_days': 99})
|
||||||
|
|
||||||
|
|
||||||
|
def test_notifications_statistics_client_for_stats_by_day_calls_correct_api_endpoint(mocker, api_user_active):
|
||||||
|
|
||||||
|
some_service_id = uuid.uuid4()
|
||||||
|
today = datetime.today().date().strftime('%Y-%m-%d')
|
||||||
|
|
||||||
|
expected_url = '/service/{}/notifications-statistics/day/{}'.format(some_service_id, today)
|
||||||
|
|
||||||
|
client = StatisticsApiClient()
|
||||||
|
|
||||||
|
mock_get = mocker.patch('app.notify_client.statistics_api_client.StatisticsApiClient.get')
|
||||||
|
|
||||||
|
client.get_statistics_for_service_for_day(some_service_id, today)
|
||||||
|
|
||||||
|
mock_get.assert_called_once_with(url=expected_url)
|
||||||
|
|||||||
@@ -6,8 +6,8 @@ from os import path
|
|||||||
from glob import glob
|
from glob import glob
|
||||||
from bs4 import BeautifulSoup
|
from bs4 import BeautifulSoup
|
||||||
from flask import url_for
|
from flask import url_for
|
||||||
from unittest.mock import ANY
|
from tests import validate_route_permission
|
||||||
from tests import (validate_route_permission, job_json)
|
from datetime import datetime
|
||||||
|
|
||||||
template_types = ['email', 'sms']
|
template_types = ['email', 'sms']
|
||||||
|
|
||||||
@@ -76,7 +76,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
|
|||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
|
|
||||||
@@ -115,14 +115,16 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
|
|||||||
|
|
||||||
def test_upload_csv_invalid_extension(app_,
|
def test_upload_csv_invalid_extension(app_,
|
||||||
api_user_active,
|
api_user_active,
|
||||||
|
mocker,
|
||||||
mock_login,
|
mock_login,
|
||||||
mock_get_service,
|
mock_get_service,
|
||||||
mock_get_service_template,
|
mock_get_service_template,
|
||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
fake_uuid):
|
fake_uuid):
|
||||||
|
|
||||||
with app_.test_request_context():
|
with app_.test_request_context():
|
||||||
with app_.test_client() as client:
|
with app_.test_client() as client:
|
||||||
client.login(api_user_active)
|
client.login(api_user_active)
|
||||||
@@ -147,7 +149,7 @@ def test_send_test_sms_message(
|
|||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
|
|
||||||
@@ -175,7 +177,7 @@ def test_send_test_email_message(
|
|||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
|
|
||||||
@@ -203,7 +205,6 @@ def test_send_test_sms_message_with_placeholders(
|
|||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
mock_get_service_statistics,
|
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
|
|
||||||
@@ -212,6 +213,7 @@ def test_send_test_sms_message_with_placeholders(
|
|||||||
'file_name': 'Test message'
|
'file_name': 'Test message'
|
||||||
}
|
}
|
||||||
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
|
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
|
||||||
|
mocker.patch('app.statistics_api_client.get_statistics_for_service_for_day', return_value=None)
|
||||||
|
|
||||||
with app_.test_request_context():
|
with app_.test_request_context():
|
||||||
with app_.test_client() as client:
|
with app_.test_client() as client:
|
||||||
@@ -284,7 +286,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
|
|||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
|
|
||||||
@@ -316,6 +318,9 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
|
|||||||
assert '07700 900750' not in content
|
assert '07700 900750' not in content
|
||||||
assert 'Only showing the first 50 rows' in content
|
assert 'Only showing the first 50 rows' in content
|
||||||
|
|
||||||
|
today = datetime.today().date().strftime('%Y-%m-%d')
|
||||||
|
mock_get_service_statistics_for_day.assert_called_once_with(fake_uuid, today)
|
||||||
|
|
||||||
|
|
||||||
def test_create_job_should_call_api(
|
def test_create_job_should_call_api(
|
||||||
app_,
|
app_,
|
||||||
@@ -360,7 +365,7 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
|
|||||||
mock_s3_upload,
|
mock_s3_upload,
|
||||||
mocker,
|
mocker,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
mock_get_users_by_service,
|
mock_get_users_by_service,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
@@ -601,7 +606,7 @@ def test_check_messages_back_link_with_help(app_,
|
|||||||
mock_get_service,
|
mock_get_service,
|
||||||
mock_get_service_template_with_placeholders,
|
mock_get_service_template_with_placeholders,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
mock_s3_download,
|
mock_s3_download,
|
||||||
fake_uuid):
|
fake_uuid):
|
||||||
with app_.test_request_context():
|
with app_.test_request_context():
|
||||||
@@ -633,7 +638,7 @@ def test_check_messages_back_link_without_help(app_,
|
|||||||
mock_get_service,
|
mock_get_service,
|
||||||
mock_get_service_template_with_placeholders,
|
mock_get_service_template_with_placeholders,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
mock_get_service_statistics,
|
mock_get_service_statistics_for_day,
|
||||||
mock_s3_download,
|
mock_s3_download,
|
||||||
fake_uuid):
|
fake_uuid):
|
||||||
with app_.test_request_context():
|
with app_.test_request_context():
|
||||||
@@ -654,3 +659,48 @@ def test_check_messages_back_link_without_help(app_,
|
|||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
content = response.get_data(as_text=True)
|
content = response.get_data(as_text=True)
|
||||||
assert url_for('.send_test', service_id=fake_uuid, template_id=fake_uuid) in content
|
assert url_for('.send_test', service_id=fake_uuid, template_id=fake_uuid) in content
|
||||||
|
|
||||||
|
|
||||||
|
def test_send_and_check_page_renders_if_no_statistics(
|
||||||
|
app_,
|
||||||
|
mocker,
|
||||||
|
api_user_active,
|
||||||
|
mock_login,
|
||||||
|
mock_get_service,
|
||||||
|
mock_get_service_template,
|
||||||
|
mock_s3_upload,
|
||||||
|
mock_has_permissions,
|
||||||
|
mock_get_users_by_service,
|
||||||
|
fake_uuid
|
||||||
|
):
|
||||||
|
|
||||||
|
mock_get_stats = mocker.patch('app.statistics_api_client.get_statistics_for_service_for_day', return_value=None)
|
||||||
|
|
||||||
|
mocker.patch(
|
||||||
|
'app.main.views.send.s3download',
|
||||||
|
return_value="""
|
||||||
|
phone number
|
||||||
|
+447700900986
|
||||||
|
"""
|
||||||
|
)
|
||||||
|
|
||||||
|
with app_.test_request_context():
|
||||||
|
with app_.test_client() as client:
|
||||||
|
client.login(api_user_active)
|
||||||
|
with client.session_transaction() as session:
|
||||||
|
session['upload_data'] = {'original_file_name': 'some.csv',
|
||||||
|
'template_id': fake_uuid,
|
||||||
|
'notification_count': 0,
|
||||||
|
'valid': True}
|
||||||
|
resp = client.post(
|
||||||
|
url_for('main.check_messages', service_id=fake_uuid, template_type='sms', upload_id='abc123'),
|
||||||
|
data={'file': (BytesIO(''.encode('utf-8')), 'some.csv')},
|
||||||
|
content_type='multipart/form-data',
|
||||||
|
follow_redirects=True
|
||||||
|
)
|
||||||
|
|
||||||
|
today = datetime.today().date().strftime('%Y-%m-%d')
|
||||||
|
assert resp.status_code == 200
|
||||||
|
page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser')
|
||||||
|
assert page.h1.text.strip() == 'Check and confirm'
|
||||||
|
mock_get_stats.assert_called_once_with(fake_uuid, today)
|
||||||
|
|||||||
@@ -193,6 +193,26 @@ def mock_get_service_statistics(mocker):
|
|||||||
'app.statistics_api_client.get_statistics_for_service', side_effect=_create)
|
'app.statistics_api_client.get_statistics_for_service', side_effect=_create)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(scope='function')
|
||||||
|
def mock_get_service_statistics_for_day(mocker):
|
||||||
|
|
||||||
|
stats = {'day': datetime.today().date().strftime('%Y-%m-%d'),
|
||||||
|
'emails_delivered': 0,
|
||||||
|
'sms_requested': 0,
|
||||||
|
'sms_delivered': 0,
|
||||||
|
'sms_failed': 0,
|
||||||
|
'emails_requested': 0,
|
||||||
|
'emails_failed': 0,
|
||||||
|
'service': fake_uuid,
|
||||||
|
'id': fake_uuid}
|
||||||
|
|
||||||
|
def _stats(service_id, day):
|
||||||
|
return {'data': stats}
|
||||||
|
|
||||||
|
return mocker.patch(
|
||||||
|
'app.statistics_api_client.get_statistics_for_service_for_day', side_effect=_stats)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(scope='function')
|
@pytest.fixture(scope='function')
|
||||||
def mock_get_aggregate_service_statistics(mocker):
|
def mock_get_aggregate_service_statistics(mocker):
|
||||||
def _create(service_id, limit_days=None):
|
def _create(service_id, limit_days=None):
|
||||||
|
|||||||
Reference in New Issue
Block a user