From c165589d4c6a4f94b1f9e34fe2482505e14743a5 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 11 Jul 2023 12:37:27 -0700 Subject: [PATCH 1/2] notify-api-340 remove daily limit --- app/__init__.py | 10 ---------- app/config.py | 3 +-- app/notify_client/service_api_client.py | 8 ++------ app/templates/main_nav.html | 2 -- tests/app/main/views/test_send.py | 5 +++-- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index bfd3da0f4..3be6d811b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -311,16 +311,6 @@ def init_app(application): def _nav_selected(): return navigation - @application.context_processor - def _attach_current_daily_remaining_messages_per_service(): - remaining_messages = 0 - - if hasattr(current_service, 'message_limit'): - remaining_messages = current_service.message_limit - service_api_client.get_notification_count( - service_id=current_service.id) - - return {'daily_remaining_messages': remaining_messages} - @application.context_processor def _attach_current_global_daily_messages(): remaining_global_messages = 0 diff --git a/app/config.py b/app/config.py index 568c51be7..fbf88d38e 100644 --- a/app/config.py +++ b/app/config.py @@ -2,7 +2,6 @@ import json from os import getenv import newrelic.agent -from notifications_utils import DAILY_MESSAGE_LIMIT from app.cloudfoundry_config import cloud_config @@ -45,7 +44,7 @@ class Config(object): DEFAULT_SERVICE_LIMIT = 50 - GLOBAL_SERVICE_MESSAGE_LIMIT = DAILY_MESSAGE_LIMIT + GLOBAL_SERVICE_MESSAGE_LIMIT = 250000 EMAIL_EXPIRY_SECONDS = 3600 # 1 hour INVITATION_EXPIRY_SECONDS = 3600 * 24 * 2 # 2 days - also set on api diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 0fdcc8b85..bddd62cc6 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,9 +1,6 @@ from datetime import datetime -from notifications_utils.clients.redis import ( - daily_limit_cache_key, - daily_total_cache_key, -) +from notifications_utils.clients.redis import daily_total_cache_key from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache @@ -528,8 +525,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): return self.get("/service/{}/data-retention".format(service_id)) def get_notification_count(self, service_id): - # if cache is not set, or not enabled, return 0 - count = redis_client.get(daily_limit_cache_key(service_id)) or 0 + count = 0 return int(count) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 732331941..a57d221db 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -28,8 +28,6 @@
-

Messages Left / Daily Limit

-

{{ daily_remaining_messages }} / {{ current_service.message_limit }}

Messages Left Across Services

{{ daily_global_messages_remaining }}

diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index ff23e617d..2bee0f6fe 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1751,7 +1751,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( assert '202 867 0750' not in page.text assert 'Only showing the first 50 rows' in page.text - mock_get_notification_count.assert_called_with(service_id=service_one['id']) + mock_get_notification_count.assert_called_with(service_one['id']) @pytest.mark.parametrize('international_sms_permission, should_allow_international', [ @@ -2084,7 +2084,8 @@ def test_check_messages_back_link( @pytest.mark.parametrize('num_requested,expected_msg', [ (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.') + # This used to trigger the too many messages errors but we removed the daily limit + ("1", '‘example.csv’ contains 1,234 phone numbers.') ], ids=['none_sent', 'none_sent', 'some_sent']) def test_check_messages_shows_too_many_messages_errors( mocker, From 4b3ccfeec367bb55d37527d0fd3c6faa50dd3a89 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Thu, 13 Jul 2023 08:28:46 -0700 Subject: [PATCH 2/2] fix tests --- app/config.py | 3 ++- tests/app/main/views/test_email_preview.py | 15 --------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/app/config.py b/app/config.py index fbf88d38e..568c51be7 100644 --- a/app/config.py +++ b/app/config.py @@ -2,6 +2,7 @@ import json from os import getenv import newrelic.agent +from notifications_utils import DAILY_MESSAGE_LIMIT from app.cloudfoundry_config import cloud_config @@ -44,7 +45,7 @@ class Config(object): DEFAULT_SERVICE_LIMIT = 50 - GLOBAL_SERVICE_MESSAGE_LIMIT = 250000 + GLOBAL_SERVICE_MESSAGE_LIMIT = DAILY_MESSAGE_LIMIT EMAIL_EXPIRY_SECONDS = 3600 # 1 hour INVITATION_EXPIRY_SECONDS = 3600 * 24 * 2 # 2 days - also set on api diff --git a/tests/app/main/views/test_email_preview.py b/tests/app/main/views/test_email_preview.py index ef0080471..180f1c7c8 100644 --- a/tests/app/main/views/test_email_preview.py +++ b/tests/app/main/views/test_email_preview.py @@ -18,27 +18,12 @@ def test_renders(client_request, mocker, query_args, result): assert response.get_data(as_text=True) == 'rendered' -def test_displays_govuk_branding_by_default(client_request): - - page = client_request.get('main.email_template', _test_page_title=False) - - assert page.find("a", attrs={"href": "https://www.gov.uk"}) - - -def test_displays_govuk_branding(client_request, mock_get_email_branding_with_govuk_brand_type): - - page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) - - assert page.find("a", attrs={"href": "https://www.gov.uk"}) - - def test_displays_both_branding(client_request, mock_get_email_branding_with_both_brand_type): page = client_request.get('main.email_template', branding_style="1", _test_page_title=False) mock_get_email_branding_with_both_brand_type.assert_called_once_with('1') - assert page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png$")}) assert page.select("body > table:nth-of-type(3) table > tr:nth-of-type(1) > td:nth-of-type(2)")[0]\ .get_text().strip() == 'Organization text' # brand text is set