To check the daily limit get the daily notification_count from redis.

The daily limit cache is set by the api when a notification is created. There is one cache key per service per day and it expires after 24 hours.
This commit is contained in:
Rebecca Law
2021-06-23 15:56:05 +01:00
parent 7930a53a58
commit 44f02f2e30
4 changed files with 17 additions and 25 deletions

View File

@@ -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)

View File

@@ -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()

View File

@@ -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'] = {

View File

@@ -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()