From d2fd5ab07d4dcbc3af3a37822ff5456fda9d5c1b Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Mon, 15 May 2023 14:37:05 -0600 Subject: [PATCH 1/6] Added display of daily limit and remaining amount after csv upload --- app/main/views/send.py | 6 +++++- app/templates/service_navigation.html | 1 + app/templates/views/check/ok.html | 7 ++++++- app/templates/views/send.html | 7 +++++++ tests/app/main/views/accounts/test_choose_accounts.py | 2 +- tests/app/main/views/test_send.py | 2 +- 6 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 764b3daee..53e8eaa9d 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -80,6 +80,9 @@ def get_example_csv_rows(template, use_example_as_example=True, submitted_fields @main.route("/services//send//csv", methods=['GET', 'POST']) @user_has_permissions('send_messages', restrict_admin_usage=True) def send_messages(service_id, template_id): + notification_count = service_api_client.get_notification_count(service_id) + remaining_messages = (current_service.message_limit - notification_count) + db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user) email_reply_to = None @@ -154,7 +157,8 @@ def send_messages(service_id, template_id): column_headings=list(ascii_uppercase[:len(column_headings)]), example=[column_headings, get_example_csv_rows(template)], form=form, - allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS + allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS, + remaining_messages=remaining_messages ) diff --git a/app/templates/service_navigation.html b/app/templates/service_navigation.html index 3271603f6..d5f0ce807 100644 --- a/app/templates/service_navigation.html +++ b/app/templates/service_navigation.html @@ -1,6 +1,7 @@ {% macro navigation_service_name(service) %}
{{ service.name }} +

Daily Message Limit: {{ service.message_limit }}

{% if not service.active %} Suspended {% endif %} diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index b584b6930..33f491db0 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -93,5 +93,10 @@ Only showing the first {{ count_of_displayed_recipients }} rows

{% endif %} - +
+

Daily messages remaining:

+

{{ remaining_messages }}

+

Remaining messages if sent:

+

{{ remaining_messages - count_of_recipients }}

+
{% endblock %} diff --git a/app/templates/views/send.html b/app/templates/views/send.html index aa9481f1f..ee716c23f 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -56,4 +56,11 @@

Your file will populate this template ({{ template.name }})

{{ template|string }} +
+

Daily message limit for this service:

+

{{ current_service.name }} - {{ current_service.message_limit }}

+

Daily remaining messages for this service:

+

{{ remaining_messages }}

+
+ {% endblock %} diff --git a/tests/app/main/views/accounts/test_choose_accounts.py b/tests/app/main/views/accounts/test_choose_accounts.py index e789f7f5e..59344a691 100644 --- a/tests/app/main/views/accounts/test_choose_accounts.py +++ b/tests/app/main/views/accounts/test_choose_accounts.py @@ -322,7 +322,7 @@ def test_should_show_back_to_service_if_user_belongs_to_service( ): mock_get_service.return_value = service_one expected_page_text = ( - 'Test Service Switch service ' + 'Test Service Daily Message Limit: 50 Switch service ' '' 'Dashboard ' 'Send messages ' diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 467a5c149..aeb6c950c 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_once_with(service_one['id']) + mock_get_notification_count.assert_called_with(service_one['id']) @pytest.mark.parametrize('international_sms_permission, should_allow_international', [ From 3abb61614c96949cf1dc8a48ee6af8ba542d9e54 Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Tue, 30 May 2023 08:01:29 -0600 Subject: [PATCH 2/6] Added context_process to init.py for remaining daily messages --- app/__init__.py | 8 ++++++++ app/templates/main_nav.html | 6 ++++++ app/templates/views/send.html | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index 590f0f595..65a08337b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -311,6 +311,11 @@ def init_app(application): def _nav_selected(): return navigation + @application.context_processor + def _attach_current_remaining_messages(): + request + return {'side_nav_remaining_messages': current_service.message_limit - service_api_client.get_notification_count(service_id=current_service.id)} + @application.before_request def record_start_time(): g.start = monotonic() @@ -395,6 +400,9 @@ def load_organisation_before_request(): raise +def load_current_daily_messages_per_service(): + service = request.args.get('service') + def save_service_or_org_after_request(response): # Only save the current session if the request is 200 service_id = request.view_args.get('service_id', None) if request.view_args else None diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 757616832..2ddd198d9 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -27,4 +27,10 @@ {% endif %} +
+

Messages Left / Daily Limit

+
    +
  • {{ side_nav_remaining_messages }} / {{ current_service.message_limit }}
    • +
+
{% endif %} diff --git a/app/templates/views/send.html b/app/templates/views/send.html index ee716c23f..88c71eb6a 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -60,7 +60,7 @@

Daily message limit for this service:

{{ current_service.name }} - {{ current_service.message_limit }}

Daily remaining messages for this service:

-

{{ remaining_messages }}

+

{{ side_nav_remaining_messages }}

{% endblock %} From bd798e78b6ca98324954f89862680bad11a1991a Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Thu, 1 Jun 2023 10:44:13 -0600 Subject: [PATCH 3/6] Added global remaining daily messages across services to ui --- app/__init__.py | 24 ++++++++++++++----- app/config.py | 2 ++ app/notify_client/service_api_client.py | 12 +++++++++- app/templates/main_nav.html | 10 +++++--- app/templates/service_navigation.html | 1 - app/templates/views/check/ok.html | 6 ++--- app/templates/views/send.html | 6 ++--- .../views/accounts/test_choose_accounts.py | 2 +- 8 files changed, 43 insertions(+), 20 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 65a08337b..c108d6a8c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -312,9 +312,24 @@ def init_app(application): return navigation @application.context_processor - def _attach_current_remaining_messages(): - request - return {'side_nav_remaining_messages': current_service.message_limit - service_api_client.get_notification_count(service_id=current_service.id)} + 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 + + if current_app: + global_limit = current_app.config['GLOBAL_SERVICE_MESSAGE_LIMIT'] + global_messages_count = service_api_client.get_global_notification_count() + remaining_global_messages = global_limit - global_messages_count + return {'daily_global_messages_remaining': remaining_global_messages} @application.before_request def record_start_time(): @@ -400,9 +415,6 @@ def load_organisation_before_request(): raise -def load_current_daily_messages_per_service(): - service = request.args.get('service') - def save_service_or_org_after_request(response): # Only save the current session if the request is 200 service_id = request.view_args.get('service_id', None) if request.view_args else None diff --git a/app/config.py b/app/config.py index 34da3b1a8..18e2f6f4a 100644 --- a/app/config.py +++ b/app/config.py @@ -45,6 +45,8 @@ class Config(object): DEFAULT_SERVICE_LIMIT = 50 + GLOBAL_SERVICE_MESSAGE_LIMIT = 5000 + EMAIL_EXPIRY_SECONDS = 3600 # 1 hour INVITATION_EXPIRY_SECONDS = 3600 * 24 * 2 # 2 days - also set on api EMAIL_2FA_EXPIRY_SECONDS = 1800 # 30 Minutes diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 449a5e4b0..4e7488246 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,6 +1,9 @@ from datetime import datetime -from notifications_utils.clients.redis import daily_limit_cache_key +from notifications_utils.clients.redis import ( + daily_limit_cache_key, + daily_total_cache_key, +) from app.extensions import redis_client from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache @@ -532,5 +535,12 @@ class ServiceAPIClient(NotifyAdminAPIClient): return int(count) + def get_global_notification_count(self): + # if cache is not set, or not enabled, return 0 + + count = redis_client.get(daily_total_cache_key()) or 0 + + return int(count) + service_api_client = ServiceAPIClient() diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 2ddd198d9..133f93861 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -26,11 +26,15 @@
  • Team members
    • {% endif %} - +
    -

    Messages Left / Daily Limit

    +

    Messages Left / Daily Limit

      -
    • {{ side_nav_remaining_messages }} / {{ current_service.message_limit }}
      • +
    • {{ daily_remaining_messages }} / {{ current_service.message_limit }}
      • +

    +

    Messages Left Across Services

    +
      +
    • {{ daily_global_messages_remaining }}
    {% endif %} diff --git a/app/templates/service_navigation.html b/app/templates/service_navigation.html index d5f0ce807..3271603f6 100644 --- a/app/templates/service_navigation.html +++ b/app/templates/service_navigation.html @@ -1,7 +1,6 @@ {% macro navigation_service_name(service) %}
    {{ service.name }} -

    Daily Message Limit: {{ service.message_limit }}

    {% if not service.active %} Suspended {% endif %} diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index 33f491db0..c45ebc528 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -94,9 +94,7 @@

    {% endif %}
    -

    Daily messages remaining:

    -

    {{ remaining_messages }}

    -

    Remaining messages if sent:

    -

    {{ remaining_messages - count_of_recipients }}

    +

    Messages remaining today / Messages left if list is sent

    +

    {{ remaining_messages }} / {{ remaining_messages - count_of_recipients }}

    {% endblock %} diff --git a/app/templates/views/send.html b/app/templates/views/send.html index 88c71eb6a..93de9bd39 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -57,10 +57,8 @@ {{ template|string }}
    -

    Daily message limit for this service:

    -

    {{ current_service.name }} - {{ current_service.message_limit }}

    -

    Daily remaining messages for this service:

    -

    {{ side_nav_remaining_messages }}

    +

    Messages Remaining Today / Daily Message Limit

    +

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

    {% endblock %} diff --git a/tests/app/main/views/accounts/test_choose_accounts.py b/tests/app/main/views/accounts/test_choose_accounts.py index 59344a691..e7dae5bc1 100644 --- a/tests/app/main/views/accounts/test_choose_accounts.py +++ b/tests/app/main/views/accounts/test_choose_accounts.py @@ -322,7 +322,7 @@ def test_should_show_back_to_service_if_user_belongs_to_service( ): mock_get_service.return_value = service_one expected_page_text = ( - 'Test Service Daily Message Limit: 50 Switch service ' + 'Test Service Switch service ' '' 'Dashboard ' 'Send messages ' From c97b412afe4f7038c2536b7751df16fb256ee22c Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Thu, 1 Jun 2023 12:33:57 -0600 Subject: [PATCH 4/6] Fix assert function signature in test --- tests/app/main/views/test_send.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index a3f323ec2..3b25c42fe 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_one['id']) + mock_get_notification_count.assert_called_with(service_id=service_one['id']) @pytest.mark.parametrize('international_sms_permission, should_allow_international', [ From a1484c62bba1f266f9e7f10a20c135b638d2ee4b Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Thu, 1 Jun 2023 15:40:50 -0600 Subject: [PATCH 5/6] Requested changes in PR review --- app/__init__.py | 2 +- app/main/views/send.py | 2 +- app/notify_client/service_api_client.py | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index c108d6a8c..a8cc7e930 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -315,7 +315,7 @@ def init_app(application): def _attach_current_daily_remaining_messages_per_service(): remaining_messages = 0 - if hasattr(current_service, 'message_limit'): + if current_service: remaining_messages = current_service.message_limit - service_api_client.get_notification_count( service_id=current_service.id) diff --git a/app/main/views/send.py b/app/main/views/send.py index 53e8eaa9d..b96479d58 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -81,7 +81,7 @@ def get_example_csv_rows(template, use_example_as_example=True, submitted_fields @user_has_permissions('send_messages', restrict_admin_usage=True) def send_messages(service_id, template_id): notification_count = service_api_client.get_notification_count(service_id) - remaining_messages = (current_service.message_limit - notification_count) + remaining_messages = current_service.message_limit - notification_count db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 4e7488246..8c55d5157 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -530,14 +530,12 @@ class ServiceAPIClient(NotifyAdminAPIClient): 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 return int(count) def get_global_notification_count(self): # if cache is not set, or not enabled, return 0 - count = redis_client.get(daily_total_cache_key()) or 0 return int(count) From 21f9e80c9ed3851eb4247671abfbd0ccf4682bd9 Mon Sep 17 00:00:00 2001 From: Andrew Shumway Date: Fri, 2 Jun 2023 08:32:44 -0600 Subject: [PATCH 6/6] Added hasattr() back for potential NoneType error --- app/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index a8cc7e930..c108d6a8c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -315,7 +315,7 @@ def init_app(application): def _attach_current_daily_remaining_messages_per_service(): remaining_messages = 0 - if current_service: + if hasattr(current_service, 'message_limit'): remaining_messages = current_service.message_limit - service_api_client.get_notification_count( service_id=current_service.id)