feat: fix sending; write tests; 🙏

app/main/views/send.py

@@ -793,6 +793,8 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
         remaining_email_this_year = current_service.email_annual_limit - stats_ytd["email"]
 
         # Show annual limit validation over the daily one (even if both are true)
+        data["send_exceeds_annual_limit"] = False
+        data["send_exceeds_daily_limit"] = False
         if data["template"].template_type == "email":
             if remaining_email_this_year < data["count_of_recipients"]:
                 data["recipients_remaining_messages"] = remaining_email_this_year
@@ -801,10 +803,11 @@ def check_messages(service_id, template_id, upload_id, row_index=2):
             if remaining_sms_this_year < data["count_of_recipients"]:
                 data["recipients_remaining_messages"] = remaining_sms_this_year
                 data["send_exceeds_annual_limit"] = True
+            else:
+                data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"]
 
-        data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"]
     else:
-         data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"]
+        data["send_exceeds_daily_limit"] = data["recipients"].sms_fragment_count > data["sms_parts_remaining"]
 
     if (
         data["recipients"].too_many_rows

app/templates/partials/check/too-many-email-messages.html

@@ -6,18 +6,17 @@
   {% else %}
     {% if send_exceeds_annual_limit %}
 
-      <p>{{ _('<strong>{}</strong> can only send <strong>{}</strong> more email messages until annual limit resets'.format(current_service.name, recipients_remaining_messages)) }}</p>
+      <p data-testid="exceeds-annual">{{ _('<strong>{}</strong> can only send <strong>{}</strong> more email messages until annual limit resets'.format(current_service.name, recipients_remaining_messages)) }}</p>
       <p>
         {{ _('To send some of these messages now, edit the spreadsheet to <strong>{}</strong> recipients maximum. '.format(recipients_remaining_messages)) }}
         {{ _('To send to recipients you removed, wait until <strong>April 1, {}</strong> or contact them some other way.'.format(now().year)) }}
       </p>
 
-    {% endif %}
-    {% if send_exceeds_daily_limit %}
+    {% elif send_exceeds_daily_limit or recipients.more_rows_than_can_send %}
       {{ _("To request a daily limit above {} emails, {}").format(current_service.message_limit, content_link(_("contact us"), url_for('main.contact'), is_external_link=true)) }}
 
       <h2 class="heading-medium">{{ _('You cannot send all these email messages today') }}</h2>
-      <p>
+      <p data-testid="exceeds-daily">
         {{ _("You can try sending these messages after {} Eastern Time. Check {}.").format(time_to_reset[current_lang],
         content_link(_("your current local time"), _('https://nrc.canada.ca/en/web-clock/'), is_external_link=true))}}
       </p>

app/templates/partials/check/too-many-sms-message-parts.html

@@ -1,6 +1,6 @@
 {% from "components/links.html" import content_link %}
 
-<p>
+<p data-testid="exceeds-daily">
   {%- if current_service.trial_mode %}
     {{ _("Your service is in trial mode. To send more messages, <a href='{}'>request to go live</a>").format(url_for('main.request_to_go_live', service_id=current_service.id)) }}
   {% else %}

app/templates/views/check/column-errors.html

@@ -10,10 +10,12 @@
 {% set prefix_txt = _('a column called') %}
 {% set prefix_plural_txt = _('columns called') %}
 
-{% if send_exceeds_daily_limit %}
-  {% set page_title = _('These messages exceed your daily limit') %}
-{% elif send_exceeds_annual_limit %}
+{% if send_exceeds_annual_limit %}
   {% set page_title = _('These messages exceed the annual limit') %}
+{% elif send_exceeds_daily_limit and (sms_parts_remaining <= 0) %}
+  {% set page_title = _('These messages exceed your daily limit') %}
+{% elif send_exceeds_daily_limit or recipients.more_rows_than_can_send %}
+  {% set page_title = _('These messages exceed your daily limit') %}
 {% else %}
   {% set page_title = _('Check there’s a column for each variable') %}
 {% endif %}
@@ -168,7 +170,7 @@ <h2 class="heading-medium">{{ _('You cannot send all these text messages today')
         {% call banner_wrapper(type='dangerous') %}
           {% include "partials/check/too-many-email-messages.html" %}
         {% endcall %}
-      {% elif recipients.more_rows_than_can_send %}
+      {% elif recipients.more_rows_than_can_send or  send_exceeds_annual_limit %}
           {% call banner_wrapper(type='dangerous') %}
             {% include "partials/check/too-many-email-messages.html" %}
           {% endcall %}           

tests/app/main/views/test_send.py

@@ -6,6 +6,7 @@
 from io import BytesIO
 from itertools import repeat
 from os import path
+from unittest.mock import patch
 from uuid import uuid4
 from zipfile import BadZipFile
 
@@ -41,6 +42,7 @@
     mock_get_service_letter_template,
     mock_get_service_template,
     normalize_spaces,
+    set_config,
 )
 
 template_types = ["email", "sms"]
@@ -2543,6 +2545,7 @@ def test_check_messages_shows_too_many_sms_messages_errors(
     mock_get_jobs,
     mock_s3_download,
     mock_s3_set_metadata,
+    mock_notification_counts_client,
     fake_uuid,
     num_requested,
     expected_msg,
@@ -2560,6 +2563,10 @@ def test_check_messages_shows_too_many_sms_messages_errors(
         },
     )
 
+    mock_notification_counts_client.get_all_notification_counts_for_year.return_value = {
+        "sms": 0,
+        "email": 0,
+    }
     with client_request.session_transaction() as session:
         session["file_uploads"] = {
             fake_uuid: {
@@ -2584,6 +2591,30 @@ def test_check_messages_shows_too_many_sms_messages_errors(
     assert details == expected_msg
 
 
+@pytest.fixture
+def mock_notification_counts_client():
+    with patch("app.main.views.send.notification_counts_client") as mock:
+        yield mock
+
+
+@pytest.fixture
+def mock_daily_sms_fragment_count():
+    with patch("app.main.views.send.daily_sms_fragment_count") as mock:
+        yield mock
+
+
+@pytest.fixture
+def mock_daily_email_count():
+    with patch("app.main.views.send.daily_email_count") as mock:
+        yield mock
+
+
+@pytest.fixture
+def mock_get_service_template_annual_limits():
+    with patch("app.service_api_client.get_service_template") as mock:
+        yield mock
+
+
 @pytest.mark.parametrize(
     "num_requested,expected_msg",
     [
@@ -2601,6 +2632,7 @@ def test_check_messages_shows_too_many_email_messages_errors(
     mock_get_template_statistics,
     mock_get_job_doesnt_exist,
     mock_get_jobs,
+    mock_notification_counts_client,
     fake_uuid,
     num_requested,
     expected_msg,
@@ -2617,7 +2649,10 @@ def test_check_messages_shows_too_many_email_messages_errors(
             "email": {"requested": num_requested, "delivered": 0, "failed": 0},
         },
     )
-
+    mock_notification_counts_client.get_all_notification_counts_for_year.return_value = {
+        "sms": 0,
+        "email": 0,
+    }
     with client_request.session_transaction() as session:
         session["file_uploads"] = {
             fake_uuid: {
@@ -3401,3 +3436,181 @@ class Object(object):
     multiple_choise_options = [x.text.strip() for x in options]
 
     assert multiple_choise_options == expected_filenames
+
+
+class TestAnnualLimitsSend:
+    @pytest.mark.parametrize(
+        "num_being_sent, num_sent_today, num_sent_this_year, expect_to_see_annual_limit_msg, expect_to_see_daily_limit_msg",
+        [
+            # annual limit for mock_get_live_service is 10,000email/10,000sms
+            # daily limit for mock_get_live_service is 1,000email/1,000sms
+            # 1000 have already been sent today, trying to send 100 more [over both limits]
+            (100, 1000, 10000, True, False),
+            # No sent yet today or this year, trying to send 1001 [over both limits]
+            (10001, 0, 0, True, False),
+            # 600 have already been sent this year, trying to send 500 more [over annual limit but not daily]
+            (500, 0, 9600, True, False),
+            # No sent yet today or this year, trying to send 1001 [over daily limit but not annual]
+            (1001, 0, 0, False, True),
+            # No sent yet today or this year, trying to send 100 [over neither limit]
+            (100, 0, 0, False, False),
+        ],
+        ids=[
+            "email_over_both_limits",
+            "email_over_both_limits2",
+            "email_over_annual_but_not_daily",
+            "email_over_daily_but_not_annual",
+            "email_over_neither",
+        ],
+    )
+    def test_email_send_fails_approrpiately_when_over_limits(
+        self,
+        mocker,
+        client_request,
+        mock_get_live_service,  # set email_annual_limit and sms_annual_limit to 1000
+        mock_get_users_by_service,
+        mock_get_service_email_template_without_placeholders,
+        mock_get_template_statistics,
+        mock_get_job_doesnt_exist,
+        mock_get_jobs,
+        mock_s3_set_metadata,
+        mock_notification_counts_client,
+        mock_daily_sms_fragment_count,
+        mock_daily_email_count,
+        fake_uuid,
+        num_being_sent,
+        num_sent_today,
+        num_sent_this_year,
+        expect_to_see_annual_limit_msg,
+        expect_to_see_daily_limit_msg,
+    ):
+        mocker.patch(
+            "app.main.views.send.s3download",
+            return_value=",\n".join(["email address"] + ([mock_get_users_by_service(None)[0]["email_address"]] * num_being_sent)),
+        )
+
+        # mock that `num_sent_this_year` have already been sent this year
+        mock_notification_counts_client.get_all_notification_counts_for_year.return_value = {
+            "sms": 1000,  # not used in test but needs a value
+            "email": num_sent_this_year,
+        }
+
+        # mock that we've already sent `emails_sent_today` emails today
+        mock_daily_email_count.return_value = num_sent_today
+        mock_daily_sms_fragment_count.return_value = 900  # not used in test but needs a value
+
+        with client_request.session_transaction() as session:
+            session["file_uploads"] = {
+                fake_uuid: {
+                    "template_id": fake_uuid,
+                    "notification_count": 1,
+                    "valid": True,
+                }
+            }
+
+        page = client_request.get(
+            "main.check_messages",
+            service_id=SERVICE_ONE_ID,
+            template_id=fake_uuid,
+            upload_id=fake_uuid,
+            original_file_name="valid.csv",
+            _test_page_title=False,
+        )
+
+        if expect_to_see_annual_limit_msg:
+            assert page.find(attrs={"data-testid": "exceeds-annual"}) is not None
+        else:
+            assert page.find(attrs={"data-testid": "exceeds-annual"}) is None
+
+        if expect_to_see_daily_limit_msg:
+            assert page.find(attrs={"data-testid": "exceeds-daily"}) is not None
+        else:
+            assert page.find(attrs={"data-testid": "exceeds-daily"}) is None
+
+    @pytest.mark.parametrize(
+        "num_being_sent, num_sent_today, num_sent_this_year, expect_to_see_annual_limit_msg, expect_to_see_daily_limit_msg",
+        [
+            # annual limit for mock_get_live_service is 10,000email/10,000sms
+            # daily limit for mock_get_live_service is 1,000email/1,000sms
+            # 1000 have already been sent today, trying to send 100 more [over both limits]
+            (100, 1000, 10000, True, False),
+            # No sent yet today or this year, trying to send 1001 [over both limits]
+            (10001, 0, 0, True, False),
+            # 600 have already been sent this year, trying to send 500 more [over annual limit but not daily]
+            (500, 0, 9600, True, False),
+            # No sent yet today or this year, trying to send 1001 [over daily limit but not annual]
+            (1001, 0, 0, False, True),
+            # No sent yet today or this year, trying to send 100 [over neither limit]
+            (100, 0, 0, False, False),
+        ],
+        ids=[
+            "sms_over_both_limits",
+            "sms_over_both_limits2",
+            "sms_over_annual_but_not_daily",
+            "sms_over_daily_but_not_annual",
+            "sms_over_neither",
+        ],
+    )
+    def test_sms_send_fails_approrpiately_when_over_limits(
+        self,
+        mocker,
+        client_request,
+        mock_get_live_service,  # set email_annual_limit and sms_annual_limit to 1000
+        mock_get_users_by_service,
+        mock_get_service_sms_template_without_placeholders,
+        mock_get_template_statistics,
+        mock_get_job_doesnt_exist,
+        mock_get_jobs,
+        mock_s3_set_metadata,
+        mock_notification_counts_client,
+        mock_daily_sms_fragment_count,
+        mock_daily_email_count,
+        fake_uuid,
+        num_being_sent,
+        num_sent_today,
+        num_sent_this_year,
+        expect_to_see_annual_limit_msg,
+        expect_to_see_daily_limit_msg,
+    ):
+        mocker.patch(
+            "app.main.views.send.s3download",
+            return_value=",\n".join(["phone number"] + ([mock_get_users_by_service(None)[0]["mobile_number"]] * num_being_sent)),
+        )
+
+        # mock that `num_sent_this_year` have already been sent this year
+        mock_notification_counts_client.get_all_notification_counts_for_year.return_value = {
+            "sms": num_sent_this_year,
+            "email": 1000,  # not used in test but needs a value
+        }
+
+        # mock that we've already sent `emails_sent_today` emails today
+        mock_daily_email_count.return_value = 900  # not used in test but needs a value
+        mock_daily_sms_fragment_count.return_value = num_sent_today
+
+        with client_request.session_transaction() as session:
+            session["file_uploads"] = {
+                fake_uuid: {
+                    "template_id": fake_uuid,
+                    "notification_count": 1,
+                    "valid": True,
+                }
+            }
+
+        page = client_request.get(
+            "main.check_messages",
+            service_id=SERVICE_ONE_ID,
+            template_id=fake_uuid,
+            upload_id=fake_uuid,
+            original_file_name="valid.csv",
+            _test_page_title=False,
+        )
+
+        if expect_to_see_annual_limit_msg:
+            assert page.find(attrs={"data-testid": "exceeds-annual"}) is not None
+        else:
+            assert page.find(attrs={"data-testid": "exceeds-annual"}) is None
+
+        if expect_to_see_daily_limit_msg:
+            assert page.find(attrs={"data-testid": "exceeds-daily"}) is not None
+        else:
+            assert page.find(attrs={"data-testid": "exceeds-daily"}) is None

tests/conftest.py

@@ -550,7 +550,14 @@ def fake_uuid():
 @pytest.fixture(scope="function")
 def mock_get_service(mocker, api_user_active):
     def _get(service_id):
-        service = service_json(service_id, users=[api_user_active["id"]], message_limit=50, sms_daily_limit=20)
+        service = service_json(
+            service_id,
+            users=[api_user_active["id"]],
+            message_limit=50,
+            sms_daily_limit=20,
+            email_annual_limit=1000,
+            sms_annual_limit=1000,
+        )
         return {"data": service}
 
     return mocker.patch("app.service_api_client.get_service", side_effect=_get)
@@ -675,7 +682,9 @@ def mock_service_email_from_is_unique(mocker):
 @pytest.fixture(scope="function")
 def mock_get_live_service(mocker, api_user_active):
     def _get(service_id):
-        service = service_json(service_id, users=[api_user_active["id"]], restricted=False)
+        service = service_json(
+            service_id, users=[api_user_active["id"]], restricted=False, sms_annual_limit=10000, email_annual_limit=10000
+        )
         return {"data": service}
 
     return mocker.patch("app.service_api_client.get_service", side_effect=_get)
@@ -971,6 +980,21 @@ def _get(service_id, template_id, version=None):
     return mocker.patch("app.service_api_client.get_service_template", side_effect=_get)
 
 
+@pytest.fixture(scope="function")
+def mock_get_service_sms_template_without_placeholders(mocker):
+    def _get(service_id, template_id, version=None):
+        template = template_json(
+            service_id,
+            template_id,
+            "Two week reminder",
+            "sms",
+            "Yo.",
+        )
+        return {"data": template}
+
+    return mocker.patch("app.service_api_client.get_service_template", side_effect=_get)
+
+
 @pytest.fixture(scope="function")
 def mock_get_service_letter_template(mocker, content=None, subject=None, postage="second"):
     def _get(service_id, template_id, version=None, postage=postage):