Revert PR #278 which added additional length checks to bulk SMS sends (#293)

* Revert PR #278

* Bump utils and waffles versions

Author: whabanks

.github/actions/waffles/requirements.txt

@@ -1,4 +1,4 @@
 docopt==0.6.2
 Flask==2.3.3
 markupsafe==2.1.4
-git+https://github.com/cds-snc/notifier-utils.git@52.1.9#egg=notifications-utils
+git+https://github.com/cds-snc/notifier-utils.git@52.2.0#egg=notifications-utils

notifications_utils/columns.py

@@ -63,12 +63,7 @@ def __init__(
             template.values = row_dict
             self.message_too_long = template.is_message_too_long()
 
-        super().__init__(
-            OrderedDict(
-                (key, Cell(key, value, error_fn, self.placeholders, len(template.content) if template else None))
-                for key, value in row_dict.items()
-            )
-        )
+        super().__init__(OrderedDict((key, Cell(key, value, error_fn, self.placeholders)) for key, value in row_dict.items()))
 
     def __getitem__(self, key):
         return super().__getitem__(key) or Cell()
@@ -126,7 +121,7 @@ def recipient_and_personalisation(self):
 class Cell:
     missing_field_error = "Missing"
 
-    def __init__(self, key=None, value=None, error_fn=None, placeholders=None, template_content_length=None):
+    def __init__(self, key=None, value=None, error_fn=None, placeholders=None):
         self.data = value
         self.error = error_fn(key, value) if error_fn else None
         self.ignore = Columns.make_key(key) not in (placeholders or [])

notifications_utils/recipients.py

@@ -10,9 +10,7 @@
 from collections import OrderedDict, namedtuple
 from ordered_set import OrderedSet
 from typing import Callable, Dict, List
-from notifications_utils import SMS_CHAR_COUNT_LIMIT
 from flask import current_app
-from notifications_utils.sanitise_text import SanitiseSMS
 from . import EMAIL_REGEX_PATTERN, hostname_part, tld_part
 from notifications_utils.formatters import strip_and_remove_obscure_whitespace, strip_whitespace
 from notifications_utils.template import SMSMessageTemplate, Template
@@ -276,23 +274,6 @@ def rows_with_missing_data(self):
     def rows_with_message_too_long(self):
         return self._filter_rows("message_too_long")
 
-    @property
-    def rows_with_combined_variable_content_too_long(self):
-        """
-        Checks if the length of all variable, plus the length of the template content,
-        exceeds the SMS limit of 612 characters. Counts non-GSM characters as 2.
-        """
-        if self.rows_as_list:
-            for row in self.rows_as_list:
-                if row.personalisation and self.template:
-                    variable_length = 0
-                    for variable in row.personalisation.as_dict_with_keys(self.template.placeholders).values():
-                        if variable:
-                            variable_length += sum(1 if char in SanitiseSMS.ALLOWED_CHARACTERS else 2 for char in str(variable))
-                    total_length = variable_length + (len(self.template.content) if self.template else 0)
-                    if total_length > SMS_CHAR_COUNT_LIMIT:
-                        yield row
-
     @property
     def initial_rows_with_errors(self):
         return islice(self.rows_with_errors, self.max_errors_shown)
@@ -378,9 +359,7 @@ def _get_error_for_field(self, key, value):  # noqa: C901
         if self.is_optional_address_column(key):
             return
 
-        formatted_key = Columns.make_key(key)
-
-        if formatted_key in self.recipient_column_headers_as_column_keys:
+        if Columns.make_key(key) in self.recipient_column_headers_as_column_keys:
             if value in [None, ""] or isinstance(value, list):
                 if self.duplicate_recipient_column_headers:
                     return None
@@ -391,19 +370,12 @@ def _get_error_for_field(self, key, value):  # noqa: C901
             except (InvalidEmailError, InvalidPhoneError, InvalidAddressError) as error:
                 return str(error)
 
-        if formatted_key not in self.placeholders_as_column_keys:
+        if Columns.make_key(key) not in self.placeholders_as_column_keys:
             return
 
         if value in [None, ""]:
             return Cell.missing_field_error
 
-        if self.template:
-            if formatted_key in self.placeholders_as_column_keys and self.template.template_type == "sms":
-                try:
-                    validate_sms_message_length(value, self.template.content)
-                except ValueError as error:
-                    return str(error)
-
 
 class InvalidEmailError(Exception):
     def __init__(self, message=None):
@@ -579,14 +551,6 @@ def validate_recipient(recipient, template_type: str, column=None, international
     return validators[template_type](recipient, column)
 
 
-def validate_sms_message_length(variable_content, template_content_length):
-    variable_length = sum(1 if char in SanitiseSMS.ALLOWED_CHARACTERS else 2 for char in variable_content)
-
-    if variable_length + len(template_content_length) > SMS_CHAR_COUNT_LIMIT:
-        raise ValueError(f"Maximum {SMS_CHAR_COUNT_LIMIT} characters. Some messages may be too long due to custom content.")
-    return
-
-
 @lru_cache(maxsize=32, typed=False)
 def format_recipient(recipient):
     if not isinstance(recipient, str):

pyproject.toml

@@ -5,7 +5,7 @@ include = '(notifications_utils|tests)/.*\.pyi?$'
 
 [tool.poetry]
 name = "notifications-utils"
-version = "52.1.9"
+version = "52.2.0"
 description = "Shared python code for Notification - Provides logging utils etc."
 authors = ["Canadian Digital Service"]
 license = "MIT license"

tests/test_recipient_csv.py

@@ -1,4 +1,3 @@
-from math import floor
 import pytest
 import itertools
 import unicodedata
@@ -746,57 +745,29 @@ def test_recipient_safelist(file_contents, template_type, safelist, count_of_row
     assert recipients.allowed_to_send_to
 
 
-@pytest.mark.parametrize(
-    "template_content, csv, error_rows",
-    [
-        (
-            "((placeholder))",
-            f"""
-                phone number,placeholder
-                6502532222,1
-                6502532222,{"a" * (SMS_CHAR_COUNT_LIMIT - 1)}
-                6502532223,{"a" * SMS_CHAR_COUNT_LIMIT}
-                6502532224,{"a" * (SMS_CHAR_COUNT_LIMIT + 1)}
-            """,
-            {3},
-        ),
-        (  # Placeholder + content length should not exceed SMS_CHAR_COUNT_LIMIT. 72 = content length - placeholder text
-            "((placeholder)) This is template content. Believe it or not there are character limits.",
-            f"""
-            phone number,placeholder
-            6502532222,1
-            6502532222,{'a' * (SMS_CHAR_COUNT_LIMIT + 1)}
-            6502532222,{'a'* (SMS_CHAR_COUNT_LIMIT - 72)}
-            6502532222,{'a' * (SMS_CHAR_COUNT_LIMIT - 73) }
-            """,
-            {1},
-        ),
-        (
-            "((placeholder1)) This is template content.((placeholder2)) Believe it or not there are character limits.",
-            f"""
-            phone number,placeholder1,placeholder2
-            6502532222,1
-            6502532222,{'a' * (floor(SMS_CHAR_COUNT_LIMIT / 2))},{'a' * (floor(SMS_CHAR_COUNT_LIMIT / 2) + 1)}
-            6502532222,{'a'* (floor(SMS_CHAR_COUNT_LIMIT / 2))},{'a' * (floor(SMS_CHAR_COUNT_LIMIT / 2) - 72)}
-            6502532222,{'a' * (floor(SMS_CHAR_COUNT_LIMIT / 2)) },{'a' * (floor(SMS_CHAR_COUNT_LIMIT / 2) - 73)}
-            """,
-            {1},
-        ),
-    ],
-)
-def test_detects_rows_which_result_in_overly_long_messages(template_content, csv, error_rows):
+def test_detects_rows_which_result_in_overly_long_messages():
     template = SMSMessageTemplate(
-        {"content": template_content, "template_type": "sms"},
+        {"content": "((placeholder))", "template_type": "sms"},
         sender=None,
         prefix=None,
     )
     recipients = RecipientCSV(
-        csv,
+        """
+            phone number,placeholder
+            6502532222,1
+            6502532222,{one_under}
+            6502532223,{exactly}
+            6502532224,{one_over}
+        """.format(
+            one_under="a" * (SMS_CHAR_COUNT_LIMIT - 1),
+            exactly="a" * SMS_CHAR_COUNT_LIMIT,
+            one_over="a" * (SMS_CHAR_COUNT_LIMIT + 1),
+        ),
         template_type=template.template_type,
         template=template,
     )
-    assert _index_rows(recipients.rows_with_errors) == error_rows
-    assert _index_rows(recipients.rows_with_message_too_long) == error_rows
+    assert _index_rows(recipients.rows_with_errors) == {3}
+    assert _index_rows(recipients.rows_with_message_too_long) == {3}
     assert recipients.has_errors