From 5fdb75321ed00a33ddb2676222d33084d6612d0c Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 3 Dec 2024 14:01:20 -0500 Subject: [PATCH 1/2] Delineate between daily / annual limit csv validation --- .github/actions/waffles/requirements.txt | 2 +- notifications_utils/recipients.py | 17 +++++--- pyproject.toml | 2 +- tests/test_recipient_csv.py | 51 ++++++++++++++++++++---- 4 files changed, 57 insertions(+), 15 deletions(-) diff --git a/.github/actions/waffles/requirements.txt b/.github/actions/waffles/requirements.txt index 665564fc..9400985b 100644 --- a/.github/actions/waffles/requirements.txt +++ b/.github/actions/waffles/requirements.txt @@ -1,4 +1,4 @@ docopt==0.6.2 Flask==2.3.3 markupsafe==2.1.5 -git+https://github.com/cds-snc/notifier-utils.git@52.4.0#egg=notifications-utils +git+https://github.com/cds-snc/notifier-utils.git@52.4.1#egg=notifications-utils diff --git a/notifications_utils/recipients.py b/notifications_utils/recipients.py index 0fc4ff0b..f8e85faf 100644 --- a/notifications_utils/recipients.py +++ b/notifications_utils/recipients.py @@ -89,7 +89,8 @@ def __init__( max_initial_rows_shown=10, safelist=None, template=None, - remaining_messages=sys.maxsize, + remaining_daily_messages=sys.maxsize, + remaining_annual_messages=sys.maxsize, international_sms=False, max_rows=50000, user_language="en", @@ -103,7 +104,8 @@ def __init__( self.safelist = safelist self.template = template if isinstance(template, Template) else None self.international_sms = international_sms - self.remaining_messages = remaining_messages + self.remaining_daily_messages = remaining_daily_messages + self.remaining_annual_messages = remaining_annual_messages self.rows_as_list = None self.max_rows = max_rows @@ -162,7 +164,8 @@ def has_errors(self): return bool( self.missing_column_headers or self.duplicate_recipient_column_headers - or self.more_rows_than_can_send + or self.more_rows_than_can_send_this_year + or self.more_rows_than_can_send_today or self.too_many_rows or (not self.allowed_to_send_to) or any(self.rows_with_errors) @@ -230,8 +233,12 @@ def get_rows(self): yield None @property - def more_rows_than_can_send(self): - return len(self) > self.remaining_messages + def more_rows_than_can_send_today(self): + return len(self) > self.remaining_daily_messages + + @property + def more_rows_than_can_send_this_year(self): + return len(self) > self.remaining_annual_messages @property def sms_fragment_count(self): diff --git a/pyproject.toml b/pyproject.toml index ca4e565a..9c128241 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "notifications-utils" -version = "52.4.0" +version = "52.4.1" description = "Shared python code for Notification - Provides logging utils etc." authors = ["Canadian Digital Service"] license = "MIT license" diff --git a/tests/test_recipient_csv.py b/tests/test_recipient_csv.py index 06442f56..66abb311 100644 --- a/tests/test_recipient_csv.py +++ b/tests/test_recipient_csv.py @@ -873,18 +873,18 @@ def test_ignores_leading_whitespace_in_file(character, name): assert not recipients.has_errors -def test_error_if_too_many_email_recipients(): +def test_error_if_too_many_email_recipients_for_year(): recipients = RecipientCSV( "email address,\ntest@test.com,\ntest@test.com,\ntest@test.com,", placeholders=["email_address"], template_type="email", - remaining_messages=2, + remaining_annual_messages=2, ) assert recipients.has_errors - assert recipients.more_rows_than_can_send + assert recipients.more_rows_than_can_send_this_year -def test_error_if_too_many_sms_recipients(): +def test_error_if_too_many_sms_recipients_for_year(): recipients = RecipientCSV( "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], @@ -894,18 +894,53 @@ def test_error_if_too_many_sms_recipients(): sender=None, prefix=None, ), - remaining_messages=2, + remaining_annual_messages=2, ) assert recipients.has_errors - assert recipients.more_rows_than_can_send + assert recipients.more_rows_than_can_send_this_year -def test_dont_error_if_too_many_recipients_not_specified(): +def test_dont_error_if_too_many_recipients_not_specified_for_year(): recipients = RecipientCSV( "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], template_type="sms" ) assert not recipients.has_errors - assert not recipients.more_rows_than_can_send + assert not recipients.more_rows_than_can_send_this_year + + +def test_error_if_too_many_email_recipients_for_today(): + recipients = RecipientCSV( + "email address,\ntest@test.com,\ntest@test.com,\ntest@test.com,", + placeholders=["email_address"], + template_type="email", + remaining_daily_messages=2, + ) + assert recipients.has_errors + assert recipients.more_rows_than_can_send_today + + +def test_error_if_too_many_sms_recipients_for_today(): + recipients = RecipientCSV( + "phone number,\n6502532222,\n6502532222,\n6502532222,", + placeholders=["phone_number"], + template_type="sms", + template=SMSMessageTemplate( + {"content": "test message", "template_type": "sms"}, + sender=None, + prefix=None, + ), + remaining_daily_messages=2, + ) + assert recipients.has_errors + assert recipients.more_rows_than_can_send_today + + +def test_dont_error_if_too_many_recipients_not_specified_for_today(): + recipients = RecipientCSV( + "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], template_type="sms" + ) + assert not recipients.has_errors + assert not recipients.more_rows_than_can_send_today @pytest.mark.parametrize( From f76351fbb28fd832c87016c7cc7ce69276ee88d5 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 3 Dec 2024 16:04:06 -0500 Subject: [PATCH 2/2] Reinstate remaining_messages and related props --- notifications_utils/recipients.py | 8 ++ tests/conftest.py | 11 +++ tests/test_recipient_csv.py | 123 +++++++++++++++++++++--------- 3 files changed, 106 insertions(+), 36 deletions(-) diff --git a/notifications_utils/recipients.py b/notifications_utils/recipients.py index f8e85faf..193a1e17 100644 --- a/notifications_utils/recipients.py +++ b/notifications_utils/recipients.py @@ -89,6 +89,7 @@ def __init__( max_initial_rows_shown=10, safelist=None, template=None, + remaining_messages=sys.maxsize, # TODO FF_ANNUAL_LIMIT removal - remove this param remaining_daily_messages=sys.maxsize, remaining_annual_messages=sys.maxsize, international_sms=False, @@ -104,6 +105,7 @@ def __init__( self.safelist = safelist self.template = template if isinstance(template, Template) else None self.international_sms = international_sms + self.remaining_messages = remaining_messages self.remaining_daily_messages = remaining_daily_messages self.remaining_annual_messages = remaining_annual_messages self.rows_as_list = None @@ -164,6 +166,7 @@ def has_errors(self): return bool( self.missing_column_headers or self.duplicate_recipient_column_headers + or self.more_rows_than_can_send # TODO FF_ANNUAL_LIMIT removal - Remove this check or self.more_rows_than_can_send_this_year or self.more_rows_than_can_send_today or self.too_many_rows @@ -232,6 +235,11 @@ def get_rows(self): else: yield None + # TODO FF_ANNUAL_LIMIT removal - remove this property + @property + def more_rows_than_can_send(self): + return len(self) > self.remaining_messages + @property def more_rows_than_can_send_today(self): return len(self) > self.remaining_daily_messages diff --git a/tests/conftest.py b/tests/conftest.py index 8963c50a..2a5965d9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager from unittest.mock import Mock import pytest @@ -9,6 +10,16 @@ class FakeService: id = "1234" +@contextmanager +def set_config(app, name, value): + old_val = app.config.get(name) + app.config[name] = value + try: + yield + finally: + app.config[name] = old_val + + @pytest.fixture def app(): flask_app = Flask(__name__) diff --git a/tests/test_recipient_csv.py b/tests/test_recipient_csv.py index 66abb311..1f832009 100644 --- a/tests/test_recipient_csv.py +++ b/tests/test_recipient_csv.py @@ -9,6 +9,8 @@ from notifications_utils.template import SMSMessageTemplate from ordered_set import OrderedSet +from tests.conftest import set_config + def _index_rows(rows): return set(row.index for row in rows) @@ -873,18 +875,84 @@ def test_ignores_leading_whitespace_in_file(character, name): assert not recipients.has_errors -def test_error_if_too_many_email_recipients_for_year(): - recipients = RecipientCSV( - "email address,\ntest@test.com,\ntest@test.com,\ntest@test.com,", - placeholders=["email_address"], - template_type="email", - remaining_annual_messages=2, - ) - assert recipients.has_errors - assert recipients.more_rows_than_can_send_this_year +def test_error_if_too_many_email_recipients_for_year(app): + with set_config(app, "FF_ANNUAL_LIMIT", True): + recipients = RecipientCSV( + "email address,\ntest@test.com,\ntest@test.com,\ntest@test.com,", + placeholders=["email_address"], + template_type="email", + remaining_annual_messages=2, + ) + assert recipients.has_errors + assert recipients.more_rows_than_can_send_this_year + + +def test_error_if_too_many_sms_recipients_for_year(app): + with set_config(app, "FF_ANNUAL_LIMIT", True): + recipients = RecipientCSV( + "phone number,\n6502532222,\n6502532222,\n6502532222,", + placeholders=["phone_number"], + template_type="sms", + template=SMSMessageTemplate( + {"content": "test message", "template_type": "sms"}, + sender=None, + prefix=None, + ), + remaining_annual_messages=2, + ) + assert recipients.has_errors + assert recipients.more_rows_than_can_send_this_year -def test_error_if_too_many_sms_recipients_for_year(): +def test_dont_error_if_too_many_recipients_not_specified_for_year(app): + with set_config(app, "FF_ANNUAL_LIMIT", True): + recipients = RecipientCSV( + "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], template_type="sms" + ) + assert not recipients.has_errors + assert not recipients.more_rows_than_can_send_this_year + + +def test_error_if_too_many_email_recipients_for_today(app): + with set_config(app, "FF_ANNUAL_LIMIT", True): + recipients = RecipientCSV( + "email address,\ntest@test.com,\ntest@test.com,\ntest@test.com,", + placeholders=["email_address"], + template_type="email", + remaining_daily_messages=2, + ) + assert recipients.has_errors + assert recipients.more_rows_than_can_send_today + + +def test_error_if_too_many_sms_recipients_for_today(app): + with set_config(app, "FF_ANNUAL_LIMIT", True): + recipients = RecipientCSV( + "phone number,\n6502532222,\n6502532222,\n6502532222,", + placeholders=["phone_number"], + template_type="sms", + template=SMSMessageTemplate( + {"content": "test message", "template_type": "sms"}, + sender=None, + prefix=None, + ), + remaining_daily_messages=2, + ) + assert recipients.has_errors + assert recipients.more_rows_than_can_send_today + + +def test_dont_error_if_too_many_recipients_not_specified_for_today(app): + with set_config(app, "FF_ANNUAL_LIMIT", True): + recipients = RecipientCSV( + "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], template_type="sms" + ) + assert not recipients.has_errors + assert not recipients.more_rows_than_can_send_today + + +# TODO: FF_ANNUAL_LIMIT removal - remove this test +def test_error_if_too_many_email_recipients(app): recipients = RecipientCSV( "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], @@ -894,32 +962,14 @@ def test_error_if_too_many_sms_recipients_for_year(): sender=None, prefix=None, ), - remaining_annual_messages=2, - ) - assert recipients.has_errors - assert recipients.more_rows_than_can_send_this_year - - -def test_dont_error_if_too_many_recipients_not_specified_for_year(): - recipients = RecipientCSV( - "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], template_type="sms" - ) - assert not recipients.has_errors - assert not recipients.more_rows_than_can_send_this_year - - -def test_error_if_too_many_email_recipients_for_today(): - recipients = RecipientCSV( - "email address,\ntest@test.com,\ntest@test.com,\ntest@test.com,", - placeholders=["email_address"], - template_type="email", - remaining_daily_messages=2, + remaining_messages=2, ) assert recipients.has_errors - assert recipients.more_rows_than_can_send_today + assert recipients.more_rows_than_can_send -def test_error_if_too_many_sms_recipients_for_today(): +# TODO: FF_ANNUAL_LIMIT removal - remove this test +def test_error_if_too_many_sms_recipients(): recipients = RecipientCSV( "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], @@ -929,18 +979,19 @@ def test_error_if_too_many_sms_recipients_for_today(): sender=None, prefix=None, ), - remaining_daily_messages=2, + remaining_messages=2, ) assert recipients.has_errors - assert recipients.more_rows_than_can_send_today + assert recipients.more_rows_than_can_send -def test_dont_error_if_too_many_recipients_not_specified_for_today(): +# TODO: FF_ANNUAL_LIMIT removal - remove this test +def test_dont_error_if_too_many_recipients_not_specified(): recipients = RecipientCSV( "phone number,\n6502532222,\n6502532222,\n6502532222,", placeholders=["phone_number"], template_type="sms" ) assert not recipients.has_errors - assert not recipients.more_rows_than_can_send_today + assert not recipients.more_rows_than_can_send @pytest.mark.parametrize(