diff --git a/app/main/views/send.py b/app/main/views/send.py index 6dd1c83e2..083961977 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -612,8 +612,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ if e.status_code != 404: raise - statistics = service_api_client.get_service_statistics(service_id, today_only=True) - remaining_messages = (current_service.message_limit - sum(stat['requested'] for stat in statistics.values())) + notification_count = service_api_client.get_notification_count(service_id) + remaining_messages = (current_service.message_limit - notification_count) contents = s3download(service_id, upload_id) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 9c90e5ce5..81eafa4f2 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,5 +1,7 @@ from datetime import datetime +from notifications_utils.clients.redis import daily_limit_cache_key + from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache @@ -42,18 +44,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): return self.get('/service/{0}'.format(service_id)) def get_service_statistics(self, service_id, today_only, limit_days=None): - # FIXME: The statistics request is taking more than 30s on some accounts, - # so return an empty set of statistics for the upload check page. - # We still do the request for when `today_only=False` so that reporting - # pages in the admin are still showing the correct values for other services. - if today_only: - return { - 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, - 'letter': {'requested': 0, 'delivered': 0, 'failed': 0} - } - else: - return self.get('/service/{0}/statistics'.format(service_id), params={ + return self.get('/service/{0}/statistics'.format(service_id), params={ 'today_only': today_only, 'limit_days': limit_days })['data'] @@ -644,5 +635,10 @@ class ServiceAPIClient(NotifyAdminAPIClient): return self.post("/service/{}/set-as-broadcast-service".format(service_id), data) + def get_notification_count(self, service_id): + # if cache is not set return 0 + count = redis_client.get(daily_limit_cache_key(service_id)) or 0 + return int(count) + service_api_client = ServiceAPIClient() diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index ec9dacc33..ad9634195 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2453,7 +2453,6 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( logged_in_client, mock_get_service_template, mock_get_users_by_service, - mock_get_service_statistics, mock_get_live_service, mock_get_job_doesnt_exist, mock_get_jobs, @@ -2471,7 +2470,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( '07700 9007{0:02d}'.format(final_two) for final_two in range(0, 53) ]) ) - + mock_get_notification_count = mocker.patch('app.service_api_client.get_notification_count', return_value=0) response = logged_in_client.post( url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), data={'file': (BytesIO(''.encode('utf-8')), 'example.csv')}, @@ -2503,7 +2502,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( assert '07700 900750' not in content assert 'Only showing the first 50 rows' in content - mock_get_service_statistics.assert_called_once_with(service_one['id'], today_only=True) + mock_get_notification_count.assert_called_once_with(service_one['id']) @pytest.mark.parametrize('international_sms_permission, should_allow_international', [ @@ -3101,9 +3100,10 @@ def test_go_to_dashboard_after_tour_link( @pytest.mark.parametrize('num_requested,expected_msg', [ - (0, '‘example.csv’ contains 1,234 phone numbers.'), - (1, 'You can still send 49 messages today, but ‘example.csv’ contains 1,234 phone numbers.') -], ids=['none_sent', 'some_sent']) + (None, '‘example.csv’ contains 1,234 phone numbers.'), + ("0", '‘example.csv’ contains 1,234 phone numbers.'), + ("1", 'You can still send 49 messages today, but ‘example.csv’ contains 1,234 phone numbers.') +], ids=['none_sent', 'none_sent', 'some_sent']) def test_check_messages_shows_too_many_messages_errors( mocker, client_request, @@ -3121,10 +3121,7 @@ def test_check_messages_shows_too_many_messages_errors( mocker.patch('app.main.views.send.s3download', return_value=',\n'.join( ['phone number'] + ([mock_get_users_by_service(None)[0]['mobile_number']] * 1234) )) - mocker.patch('app.service_api_client.get_service_statistics', return_value={ - 'sms': {'requested': num_requested, 'delivered': 0, 'failed': 0}, - 'email': {'requested': 0, 'delivered': 0, 'failed': 0} - }) + mocker.patch('app.extensions.redis_client.get', return_value=num_requested) with client_request.session_transaction() as session: session['file_uploads'] = { diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index ddaea8ed9..3cc128a1b 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -40,7 +40,6 @@ def test_client_gets_service(mocker): @pytest.mark.parametrize('today_only, limit_days', [ (False, None), (False, 30), - pytest.param(True, None, marks=pytest.mark.xfail(raises=AssertionError)), ]) def test_client_gets_service_statistics(mocker, today_only, limit_days): client = ServiceAPIClient()