From 0146718a423b0144566392ab3082d15779f4a73f Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Fri, 26 Apr 2024 10:22:03 -0400 Subject: [PATCH] Revert PR #278 which added additional length checks to bulk SMS sends (#293) * Revert PR #278 * Bump utils and waffles versions --- .github/actions/waffles/requirements.txt | 2 +- notifications_utils/columns.py | 9 +--- notifications_utils/recipients.py | 40 +--------------- pyproject.toml | 2 +- tests/test_recipient_csv.py | 59 ++++++------------------ 5 files changed, 21 insertions(+), 91 deletions(-) diff --git a/.github/actions/waffles/requirements.txt b/.github/actions/waffles/requirements.txt index ed93c11a..6a0690b8 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.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 diff --git a/notifications_utils/columns.py b/notifications_utils/columns.py index 26e6dd13..5f42dd35 100644 --- a/notifications_utils/columns.py +++ b/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 []) diff --git a/notifications_utils/recipients.py b/notifications_utils/recipients.py index b59c6a21..958537a1 100644 --- a/notifications_utils/recipients.py +++ b/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): diff --git a/pyproject.toml b/pyproject.toml index dea99cc9..6784239f 100644 --- a/pyproject.toml +++ b/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" diff --git a/tests/test_recipient_csv.py b/tests/test_recipient_csv.py index bdc0a5fb..d9ffc306 100644 --- a/tests/test_recipient_csv.py +++ b/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