From 3b703a5ac3782d852b1d1cb0f3a5c3893810a9a1 Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Wed, 29 Nov 2023 11:38:08 -0500 Subject: [PATCH] Revert "Revert "Add reverted code (#2045)" (#2046)" (#2048) This reverts commit 8494ce491179ca9e185077a65d1dfca3d339fdad. --- .github/workflows/test.yaml | 2 +- app/dao/services_dao.py | 4 +- app/job/rest.py | 4 +- app/service/send_notification.py | 4 +- app/sms_fragment_utils.py | 10 +- app/v2/notifications/post_notifications.py | 28 +- poetry.lock | 58 ++-- pyproject.toml | 2 +- tests/app/dao/test_services_dao.py | 7 - .../notifications/test_post_notifications.py | 313 +----------------- 10 files changed, 64 insertions(+), 368 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index f7676deaa6..bf503e7453 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -67,7 +67,7 @@ jobs: run: | cp -f .env.example .env - name: Checks for new endpoints against AWS WAF rules - uses: cds-snc/notification-utils/.github/actions/waffles@b50a26d4a2c1844369640aa9de399d0b02e896b5 # 52.0.12 + uses: cds-snc/notification-utils/.github/actions/waffles@4a4ba9d51af71d493f873b977bc0561b121ae51e # 52.0.15 with: app-loc: '/github/workspace' app-libs: '/github/workspace/env/site-packages' diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 3183cbdef9..80d1f25c35 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -443,7 +443,7 @@ def fetch_todays_total_message_count(service_id): def fetch_todays_total_sms_count(service_id): midnight = get_midnight(datetime.now(tz=pytz.utc)) result = ( - db.session.query(func.sum(Notification.billable_units).label("sum_billable_units")) + db.session.query(func.count(Notification.id).label("total_sms_notifications")) .filter( Notification.service_id == service_id, Notification.key_type != KEY_TYPE_TEST, @@ -452,7 +452,7 @@ def fetch_todays_total_sms_count(service_id): ) .first() ) - return 0 if result is None or result.sum_billable_units is None else result.sum_billable_units + return 0 if result is None or result.total_sms_notifications is None else result.total_sms_notifications def fetch_service_email_limit(service_id: uuid.UUID) -> int: diff --git a/app/job/rest.py b/app/job/rest.py index 45f07840c4..3c443ec4ba 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -169,7 +169,7 @@ def create_job(service_id): is_test_notification = len(list(recipient_csv.get_rows())) == numberOfSimulated if not is_test_notification: - check_sms_daily_limit(service, recipient_csv.sms_fragment_count) + check_sms_daily_limit(service, len(recipient_csv)) if template.template_type == EMAIL_TYPE: check_email_daily_limit(service, len(list(recipient_csv.get_rows()))) @@ -183,7 +183,7 @@ def create_job(service_id): raise InvalidRequest(errors, status_code=400) if template.template_type == SMS_TYPE and not is_test_notification: - increment_sms_daily_count_send_warnings_if_needed(service, recipient_csv.sms_fragment_count) + increment_sms_daily_count_send_warnings_if_needed(service, len(recipient_csv)) elif template.template_type == EMAIL_TYPE: increment_email_daily_count_send_warnings_if_needed(service, len(list(recipient_csv.get_rows()))) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index cfae1129a4..813ce2f138 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -72,7 +72,7 @@ def send_one_off_notification(service_id, post_data): if template.template_type == SMS_TYPE: is_test_notification = simulated_recipient(post_data["to"], template.template_type) if not is_test_notification: - check_sms_daily_limit(service, template_with_content.fragment_count) + check_sms_daily_limit(service, 1) elif template.template_type == EMAIL_TYPE and current_app.config["FF_EMAIL_DAILY_LIMIT"]: check_email_daily_limit(service, 1) # 1 email @@ -91,7 +91,7 @@ def send_one_off_notification(service_id, post_data): if template.template_type == SMS_TYPE: is_test_notification = simulated_recipient(post_data["to"], template.template_type) if not is_test_notification: - increment_sms_daily_count_send_warnings_if_needed(service, template_with_content.fragment_count) + increment_sms_daily_count_send_warnings_if_needed(service, 1) elif template.template_type == EMAIL_TYPE and current_app.config["FF_EMAIL_DAILY_LIMIT"]: increment_email_daily_count_send_warnings_if_needed(service, 1) # 1 email diff --git a/app/sms_fragment_utils.py b/app/sms_fragment_utils.py index 1398a98d97..0b7bdb152f 100644 --- a/app/sms_fragment_utils.py +++ b/app/sms_fragment_utils.py @@ -13,11 +13,11 @@ def fetch_todays_requested_sms_count(service_id: UUID) -> int: return fetch_todays_total_sms_count(service_id) cache_key = sms_daily_count_cache_key(service_id) - fragment_count = redis_store.get(cache_key) - if fragment_count is None: - fragment_count = fetch_todays_total_sms_count(service_id) - redis_store.set(cache_key, fragment_count, ex=int(timedelta(hours=2).total_seconds())) - return int(fragment_count) + sms_count = redis_store.get(cache_key) + if sms_count is None: + sms_count = fetch_todays_total_sms_count(service_id) + redis_store.set(cache_key, sms_count, ex=int(timedelta(hours=2).total_seconds())) + return int(sms_count) def increment_todays_requested_sms_count(service_id: UUID, increment_by: int): diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 0280df058b..d9d333c0e0 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -61,6 +61,7 @@ Notification, NotificationType, Service, + TemplateType, ) from app.notifications.process_letter_notifications import create_letter_notification from app.notifications.process_notifications import ( @@ -238,8 +239,8 @@ def post_bulk(): is_test_notification = api_user.key_type == KEY_TYPE_TEST or len(list(recipient_csv.get_rows())) == numberOfSimulated if not is_test_notification: - check_sms_daily_limit(authenticated_service, recipient_csv.sms_fragment_count) - increment_sms_daily_count_send_warnings_if_needed(authenticated_service, recipient_csv.sms_fragment_count) + check_sms_daily_limit(authenticated_service, len(recipient_csv)) + increment_sms_daily_count_send_warnings_if_needed(authenticated_service, len(recipient_csv)) job = create_bulk_job(authenticated_service, api_user, template, form, recipient_csv) @@ -296,7 +297,7 @@ def post_notification(notification_type: NotificationType): if template.template_type == SMS_TYPE: is_test_notification = api_user.key_type == KEY_TYPE_TEST or simulated_recipient(form["phone_number"], notification_type) if not is_test_notification: - check_sms_daily_limit(authenticated_service, template_with_content.fragment_count) + check_sms_daily_limit(authenticated_service, 1) current_app.logger.info(f"Trying to send notification for Template ID: {template.id}") @@ -327,7 +328,7 @@ def post_notification(notification_type: NotificationType): if template.template_type == SMS_TYPE: is_test_notification = api_user.key_type == KEY_TYPE_TEST or simulated_recipient(form["phone_number"], notification_type) if not is_test_notification: - increment_sms_daily_count_send_warnings_if_needed(authenticated_service, template_with_content.fragment_count) + increment_sms_daily_count_send_warnings_if_needed(authenticated_service, 1) if notification_type == SMS_TYPE: create_resp_partial = functools.partial(create_post_sms_response_from_notification, from_number=reply_to) @@ -667,16 +668,17 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_messages): message=f"Duplicate column headers: {', '.join(sorted(recipient_csv.duplicate_recipient_column_headers))}", status_code=400, ) - if recipient_csv.more_sms_rows_than_can_send: - raise BadRequestError( - message=f"You only have {remaining_messages} remaining sms message parts before you reach your daily limit. You've tried to send {recipient_csv.sms_fragment_count} message parts.", - status_code=400, - ) if recipient_csv.more_rows_than_can_send: - raise BadRequestError( - message=f"You only have {remaining_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.", - status_code=400, - ) + if recipient_csv.template_type == SMS_TYPE: + raise BadRequestError( + message=f"You only have {remaining_messages} remaining sms messages before you reach your daily limit. You've tried to send {len(recipient_csv)} sms messages.", + status_code=400, + ) + else: + raise BadRequestError( + message=f"You only have {remaining_messages} remaining messages before you reach your daily limit. You've tried to send {nb_rows} messages.", + status_code=400, + ) if recipient_csv.too_many_rows: raise BadRequestError( diff --git a/poetry.lock b/poetry.lock index 0fc903d853..35765d7072 100644 --- a/poetry.lock +++ b/poetry.lock @@ -219,23 +219,23 @@ aiohttp = "*" [[package]] name = "awscli" -version = "1.29.39" +version = "1.29.84" description = "Universal Command Line Environment for AWS." category = "main" optional = false python-versions = ">= 3.7" files = [ - {file = "awscli-1.29.39-py3-none-any.whl", hash = "sha256:917a4e6d63cca96bfaafd8c680b2d0c5b046ffd2921d1b34b0ab5ab7b0006d2f"}, - {file = "awscli-1.29.39.tar.gz", hash = "sha256:e027120aea449b34dad91773caf3357a8dae52abf54c406f062dd90158c97c14"}, + {file = "awscli-1.29.84-py3-none-any.whl", hash = "sha256:bc8c86d6bc6c086b5db47b848f7261118571a38aba993cb62cf3a6ddcbb70a09"}, + {file = "awscli-1.29.84.tar.gz", hash = "sha256:68245c32a6fce891d14b3fa4ce50c75a394db3a5ba7bab938ba704dadb9ae17d"}, ] [package.dependencies] -botocore = "1.31.39" +botocore = "1.31.84" colorama = ">=0.2.5,<0.4.5" docutils = ">=0.10,<0.17" PyYAML = ">=3.10,<6.1" rsa = ">=3.1.2,<4.8" -s3transfer = ">=0.6.0,<0.7.0" +s3transfer = ">=0.7.0,<0.8.0" [[package]] name = "awscli-cwlogs" @@ -392,43 +392,43 @@ files = [ [[package]] name = "boto3" -version = "1.28.39" +version = "1.28.84" description = "The AWS SDK for Python" category = "main" optional = false python-versions = ">= 3.7" files = [ - {file = "boto3-1.28.39-py3-none-any.whl", hash = "sha256:48d1ea0918088df0e59a37a88ce53de7f4500108638c81255f5b1cb8edea28f4"}, - {file = "boto3-1.28.39.tar.gz", hash = "sha256:3ac38ad8afafc6ed6c8dd6cc58ddd22b6352c6a413b969aef928c6aacf555c56"}, + {file = "boto3-1.28.84-py3-none-any.whl", hash = "sha256:98b01bbea27740720a06f7c7bc0132ae4ce902e640aab090cfb99ad3278449c3"}, + {file = "boto3-1.28.84.tar.gz", hash = "sha256:adfb915958d7b54d876891ea1599dd83189e35a2442eb41ca52b04ea716180b6"}, ] [package.dependencies] -botocore = ">=1.31.39,<1.32.0" +botocore = ">=1.31.84,<1.32.0" jmespath = ">=0.7.1,<2.0.0" -s3transfer = ">=0.6.0,<0.7.0" +s3transfer = ">=0.7.0,<0.8.0" [package.extras] crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.31.39" +version = "1.31.84" description = "Low-level, data-driven core of boto 3." category = "main" optional = false python-versions = ">= 3.7" files = [ - {file = "botocore-1.31.39-py3-none-any.whl", hash = "sha256:8ce716925284c1c0d04c796016a1e0e9c29ca3e196afefacacc16bc4e80c978f"}, - {file = "botocore-1.31.39.tar.gz", hash = "sha256:61aefac8b44f86a4581d4128cce30806f633357e8d8efc4f73367a8e62009e70"}, + {file = "botocore-1.31.84-py3-none-any.whl", hash = "sha256:d65bc05793d1a8a8c191a739f742876b4b403c5c713dc76beef262d18f7984a2"}, + {file = "botocore-1.31.84.tar.gz", hash = "sha256:8913bedb96ad0427660dee083aeaa675466eb662bbf1a47781956b5882aadcc5"}, ] [package.dependencies] jmespath = ">=0.7.1,<2.0.0" python-dateutil = ">=2.1,<3.0.0" -urllib3 = ">=1.25.4,<1.27" +urllib3 = {version = ">=1.25.4,<2.1", markers = "python_version >= \"3.10\""} [package.extras] -crt = ["awscrt (==0.16.26)"] +crt = ["awscrt (==0.19.10)"] [[package]] name = "brotli" @@ -2487,7 +2487,7 @@ requests = ">=2.0.0" [[package]] name = "notifications-utils" -version = "52.0.12" +version = "52.0.15" description = "Shared python code for Notification - Provides logging utils etc." category = "main" optional = false @@ -2496,10 +2496,12 @@ files = [] develop = false [package.dependencies] -awscli = "1.29.39" +awscli = "1.29.84" bleach = "6.0.0" -boto3 = "1.28.39" +boto3 = "1.28.84" cachetools = "4.2.4" +certifi = "^2023.7.22" +cryptography = "^41.0.2" Flask = "2.3.3" Flask-Redis = "0.4.0" itsdangerous = "2.1.2" @@ -2507,7 +2509,7 @@ Jinja2 = "^3.0.0" markupsafe = "2.1.3" mistune = "0.8.4" ordered-set = "4.1.0" -phonenumbers = "8.13.19" +phonenumbers = "8.13.24" py_w3c = "0.3.1" pypdf2 = "1.28.6" python-json-logger = "2.0.7" @@ -2521,8 +2523,8 @@ werkzeug = "2.3.7" [package.source] type = "git" url = "https://github.com/cds-snc/notifier-utils.git" -reference = "52.0.12" -resolved_reference = "b50a26d4a2c1844369640aa9de399d0b02e896b5" +reference = "52.0.15" +resolved_reference = "4a4ba9d51af71d493f873b977bc0561b121ae51e" [[package]] name = "ordered-set" @@ -2565,14 +2567,14 @@ files = [ [[package]] name = "phonenumbers" -version = "8.13.19" +version = "8.13.24" description = "Python version of Google's common library for parsing, formatting, storing and validating international phone numbers." category = "main" optional = false python-versions = "*" files = [ - {file = "phonenumbers-8.13.19-py2.py3-none-any.whl", hash = "sha256:ba542f20f6dc83be8f127f240f9b5b7e7c1dec42aceff1879400d4dc0c781d81"}, - {file = "phonenumbers-8.13.19.tar.gz", hash = "sha256:38180247697240ccedd74dec4bfbdbc22bb108b9c5f991f270ca3e41395e6f96"}, + {file = "phonenumbers-8.13.24-py2.py3-none-any.whl", hash = "sha256:7dd66c57da00c0f373de83074e78d66a0801381cface4d010cfe07be2fa77fe5"}, + {file = "phonenumbers-8.13.24.tar.gz", hash = "sha256:7abc66f38d92c3b9e827d597b5d590283ca3b05288d9fadea8bc0d6c8ad73c06"}, ] [[package]] @@ -3494,14 +3496,14 @@ pyasn1 = ">=0.1.3" [[package]] name = "s3transfer" -version = "0.6.1" +version = "0.7.0" description = "An Amazon S3 Transfer Manager" category = "main" optional = false python-versions = ">= 3.7" files = [ - {file = "s3transfer-0.6.1-py3-none-any.whl", hash = "sha256:3c0da2d074bf35d6870ef157158641178a4204a6e689e82546083e31e0311346"}, - {file = "s3transfer-0.6.1.tar.gz", hash = "sha256:640bb492711f4c0c0905e1f62b6aaeb771881935ad27884852411f8e9cacbca9"}, + {file = "s3transfer-0.7.0-py3-none-any.whl", hash = "sha256:10d6923c6359175f264811ef4bf6161a3156ce8e350e705396a7557d6293c33a"}, + {file = "s3transfer-0.7.0.tar.gz", hash = "sha256:fd3889a66f5fe17299fe75b82eae6cf722554edca744ca5d5fe308b104883d2e"}, ] [package.dependencies] @@ -4212,4 +4214,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "64a5bf5874d3e17eba4acb5290f4a54dbf30c83a3f8109b4e1c5ef4344e70e15" +content-hash = "ccef13745076d6498c196e2aeb9b05f28b60a827e4fb13a231b49819655f86bc" diff --git a/pyproject.toml b/pyproject.toml index 7642e1fc32..d4860e58b7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,7 +64,7 @@ Werkzeug = "2.3.7" MarkupSafe = "2.1.3" # REVIEW: v2 is using sha512 instead of sha1 by default (in v1) itsdangerous = "2.1.2" -notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", rev = "52.0.12" } +notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", rev = "52.0.15" } # rsa = "4.9 # awscli 1.22.38 depends on rsa<4.8 typing-extensions = "4.7.1" greenlet = "2.0.2" diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index ecdb17f634..4a81ea4f86 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1090,13 +1090,6 @@ def test_only_counts_sms(self): save_notification(create_notification(template=email_template)) assert fetch_todays_total_sms_count(service.id) == 2 - def test_sums_billable_units(self): - service = create_service() - sms_template = create_template(service=service, template_type=SMS_TYPE) - save_notification(create_notification(template=sms_template, billable_units=3)) - save_notification(create_notification(template=sms_template, billable_units=10)) - assert fetch_todays_total_sms_count(service.id) == 13 - def test_returns_0_when_no_messages_for_today(self): assert fetch_todays_total_sms_count(uuid.uuid4()) == 0 diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index e86c405f64..84b868168d 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1450,7 +1450,7 @@ def test_post_notification_without_document_upload_permission(self, client, noti class TestSMSSendFragments: - def test_post_sms_enough_fragments_left(self, notify_api, client, notify_db, notify_db_session, mocker): + def test_post_sms_enough_messages_left(self, notify_api, client, notify_db, notify_db_session, mocker): mocker.patch("app.sms_normal_publish.publish") service = create_service(sms_daily_limit=10, message_limit=100) template = create_sample_template(notify_db, notify_db_session, content=500 * "a", service=service, template_type="sms") @@ -1459,7 +1459,7 @@ def test_post_sms_enough_fragments_left(self, notify_api, client, notify_db, not "template_id": str(template.id), "personalisation": {" Name": "Jo"}, } - for x in range(2): + for x in range(6): create_sample_notification(notify_db, notify_db_session, service=service) auth_header = create_authorization_header(service_id=template.service_id) @@ -1471,7 +1471,7 @@ def test_post_sms_enough_fragments_left(self, notify_api, client, notify_db, not ) assert response.status_code == 201 - def test_post_sms_not_enough_fragments_left(self, notify_api, client, notify_db, notify_db_session, mocker): + def test_post_sms_not_enough_messages_left(self, notify_api, client, notify_db, notify_db_session, mocker): mocker.patch("app.sms_normal_publish.publish") service = create_service(sms_daily_limit=10, message_limit=100) template = create_sample_template(notify_db, notify_db_session, content=500 * "a", service=service, template_type="sms") @@ -1480,7 +1480,7 @@ def test_post_sms_not_enough_fragments_left(self, notify_api, client, notify_db, "template_id": str(template.id), "personalisation": {" Name": "Jo"}, } - for x in range(7): + for x in range(10): create_sample_notification(notify_db, notify_db_session, service=service) auth_header = create_authorization_header(service_id=template.service_id) @@ -1493,7 +1493,7 @@ def test_post_sms_not_enough_fragments_left(self, notify_api, client, notify_db, assert response.status_code == 429 -class TestSMSFragmentCounter: +class TestSMSMessageCounter: # Testing API one-off: # - Sending using TEST, NORMAL, and TEAM API keys with a simulated phone number should not count towards limits # TODO: update these params when we fix https://github.com/cds-snc/notification-planning/issues/855 and remove the xfao; @@ -1772,40 +1772,6 @@ def __send_sms(): response = __send_sms() # send the 11th fragment assert response.status_code == 429 # Ensure send is blocked - def test_API_ONEOFF_cant_hop_over_limit_using_3_fragment_sms(self, notify_api, client, notify_db, notify_db_session, mocker): - # test setup - mocker.patch("app.sms_normal_publish.publish") - send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email") - - def __send_sms(): - auth_header = create_authorization_header(service_id=template.service_id) - with set_config_values(notify_api, {"REDIS_ENABLED": True}): - response = client.post( - path="/v2/notifications/sms", - data=json.dumps(data), - headers=[("Content-Type", "application/json"), auth_header], - ) - return response - - # Create a service, Set limit to 10 fragments - service = create_service(sms_daily_limit=10, message_limit=100) - - # Create 5 notifications in the db - template = create_sample_template(notify_db, notify_db_session, content="a" * 400, service=service, template_type="sms") - data = { - "phone_number": "+16502532222", - "template_id": str(template.id), - "personalisation": {" Name": "Jo"}, - } - for x in range(5): - create_sample_notification(notify_db, notify_db_session, service=service) - - __send_sms() # send 1 sms (3 fragments) should be at 80% - assert send_warning_email.called - - response = __send_sms() # send one more, puts us at 11/10 fragments - assert response.status_code == 429 # Ensure send is blocked - def test_API_BULK_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker): # test setup mocker.patch("app.sms_normal_publish.publish") @@ -1849,94 +1815,6 @@ def __send_sms(): response = __send_sms() # send the 11th fragment assert response.status_code == 400 # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else - def test_API_BULK_cant_hop_over_limit_1_fragment(self, notify_api, client, notify_db, notify_db_session, mocker): - # test setup - mocker.patch("app.sms_normal_publish.publish") - mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4())) - send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email") - send_limit_reached_email = mocker.patch("app.notifications.validators.send_sms_limit_reached_email") - - def __send_sms(number_to_send=1): - with set_config_values(notify_api, {"REDIS_ENABLED": True}): - numbers = [["9025551234"]] * number_to_send - data = { - "name": "job_name", - "template_id": str(template.id), - "rows": [["phone number"], *numbers], - } - - response = client.post( - "/v2/notifications/bulk", - data=json.dumps(data), - headers=[ - ("Content-Type", "application/json"), - create_authorization_header(service_id=template.service_id), - ], - ) - return response - - # Create a service, Set limit to 10 fragments - service = create_service(sms_daily_limit=10, message_limit=100) - - # Create 7 notifications in the db - template = create_sample_template(notify_db, notify_db_session, content="Hello", service=service, template_type="sms") - for x in range(7): - create_sample_notification(notify_db, notify_db_session, service=service) - - __send_sms(1) # send 1 sms (1 fragment) should be at 80% - assert send_warning_email.called - - response = __send_sms(3) # attempt to send over limit (11 with max 10) - assert response.status_code == 400 - - __send_sms(2) # attempt to send at limit (10 with max 10) - assert send_limit_reached_email.called - - response = __send_sms(1) # attempt to send over limit (11 with max 10)1 - assert response.status_code == 400 # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else - - def test_API_BULK_cant_hop_over_limit_2_fragment(self, notify_api, client, notify_db, notify_db_session, mocker): - # test setup - mocker.patch("app.sms_normal_publish.publish") - mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4())) - send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email") - - def __send_sms(number_to_send=1): - with set_config_values(notify_api, {"REDIS_ENABLED": True}): - numbers = [["9025551234"]] * number_to_send - data = { - "name": "job_name", - "template_id": str(template.id), - "rows": [["phone number"], *numbers], - } - - response = client.post( - "/v2/notifications/bulk", - data=json.dumps(data), - headers=[ - ("Content-Type", "application/json"), - create_authorization_header(service_id=template.service_id), - ], - ) - return response - - # Create a service, Set limit to 10 fragments - service = create_service(sms_daily_limit=10, message_limit=100) - - # Create notifications in the db - template = create_sample_template(notify_db, notify_db_session, content="A" * 400, service=service, template_type="sms") - for x in range(5): - create_sample_notification(notify_db, notify_db_session, service=service) - - __send_sms(1) # 8/10 fragments used - assert send_warning_email.called - - response = __send_sms(3) # attempt to send over limit - assert response.status_code == 400 - - response = __send_sms(2) # attempt to send over limit - assert response.status_code == 400 - # ADMIN def test_ADMIN_ONEOFF_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker): # test setup @@ -1981,46 +1859,6 @@ def __send_sms(): response = __send_sms() # 11/10 fragments assert response.status_code == 429 # Ensure send is blocked - def test_ADMIN_ONEOFF_cant_hop_over_limit_using_3_fragment_sms( - self, notify_api, client, notify_db, notify_db_session, mocker - ): - # test setup - mocker.patch("app.sms_normal_publish.publish") - - mocker.patch("app.service.send_notification.send_notification_to_queue") - send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email") - - def __send_sms(): - with set_config_values(notify_api, {"REDIS_ENABLED": True}): - token = create_jwt_token( - current_app.config["ADMIN_CLIENT_SECRET"], client_id=current_app.config["ADMIN_CLIENT_USER_NAME"] - ) - response = client.post( - f"/service/{template.service_id}/send-notification", - json={ - "to": "9025551234", - "template_id": str(template.id), - "created_by": service.users[0].id, - "personalisation": {"var": "var"}, - }, - headers={"Authorization": f"Bearer {token}"}, - ) - return response - - # Create a service, Set limit to 10 fragments - service = create_service(sms_daily_limit=10, message_limit=100) - - # Create 5 notifications in the db - template = create_sample_template(notify_db, notify_db_session, content="a" * 400, service=service, template_type="sms") - for x in range(5): - create_sample_notification(notify_db, notify_db_session, service=service) - - __send_sms() # 8/10 fragments - assert send_warning_email.called - - response = __send_sms() # 11/10 fragments - assert response.status_code == 429 # Ensure send is blocked - def test_ADMIN_CSV_sends_warning_emails_and_blocks_sends(self, notify_api, client, notify_db, notify_db_session, mocker): # test setup mocker.patch("app.sms_normal_publish.publish") @@ -2076,118 +1914,6 @@ def __send_sms(): response = __send_sms() # 11/10 fragments assert response.status_code == 429 # Ensure send is blocked - def test_ADMIN_CSV_cant_hop_over_limit_using_1_fragment_sms(self, notify_api, client, notify_db, notify_db_session, mocker): - # test setup - mocker.patch("app.sms_normal_publish.publish") - mocker.patch("app.service.send_notification.send_notification_to_queue") - mocker.patch("app.celery.tasks.process_job.apply_async") - - send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email") - send_limit_reached_email = mocker.patch("app.notifications.validators.send_sms_limit_reached_email") - - def __send_sms(number_to_send=1): - with set_config_values(notify_api, {"REDIS_ENABLED": True}): - phone_numbers = "\r\n6502532222" * number_to_send - mocker.patch("app.job.rest.get_job_from_s3", return_value=f"phone number{phone_numbers}") - mocker.patch( - "app.job.rest.get_job_metadata_from_s3", - return_value={ - "template_id": str(template.id), - "original_file_name": "thisisatest.csv", - "notification_count": f"{number_to_send}", - "valid": "True", - }, - ) - - token = create_jwt_token( - current_app.config["ADMIN_CLIENT_SECRET"], client_id=current_app.config["ADMIN_CLIENT_USER_NAME"] - ) - response = client.post( - f"/service/{template.service_id}/job", - json={ - "id": str(uuid.uuid4()), - "created_by": service.users[0].id, - }, - headers={"Authorization": f"Bearer {token}"}, - ) - return response - - # Create a service, Set limit to 10 fragments - service = create_service(sms_daily_limit=10, message_limit=100) - - # Create 7 notifications in the db - template = create_sample_template(notify_db, notify_db_session, content="Hello", service=service, template_type="sms") - for x in range(7): - create_sample_notification(notify_db, notify_db_session, service=service) - - __send_sms(1) # 8/10 fragments - assert send_warning_email.called - - response = __send_sms(3) # 11/10 fragments - assert response.status_code == 429 - - __send_sms(2) # 10/10 fragments - assert send_limit_reached_email.called - - response = __send_sms(1) # 11/10 fragments - assert response.status_code == 429 # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else - - def test_ADMIN_CSV_cant_hop_over_limit_using_2_fragment_sms(self, notify_api, client, notify_db, notify_db_session, mocker): - # test setup - mocker.patch("app.sms_normal_publish.publish") - mocker.patch("app.service.send_notification.send_notification_to_queue") - mocker.patch("app.celery.tasks.process_job.apply_async") - - send_warning_email = mocker.patch("app.notifications.validators.send_near_sms_limit_email") - send_limit_reached_email = mocker.patch("app.notifications.validators.send_sms_limit_reached_email") - - def __send_sms(number_to_send=1): - with set_config_values(notify_api, {"REDIS_ENABLED": True}): - phone_numbers = "\r\n6502532222" * number_to_send - mocker.patch("app.job.rest.get_job_from_s3", return_value=f"phone number{phone_numbers}") - mocker.patch( - "app.job.rest.get_job_metadata_from_s3", - return_value={ - "template_id": str(template.id), - "original_file_name": "thisisatest.csv", - "notification_count": f"{number_to_send}", - "valid": "True", - }, - ) - - token = create_jwt_token( - current_app.config["ADMIN_CLIENT_SECRET"], client_id=current_app.config["ADMIN_CLIENT_USER_NAME"] - ) - response = client.post( - f"/service/{template.service_id}/job", - json={ - "id": str(uuid.uuid4()), - "created_by": service.users[0].id, - }, - headers={"Authorization": f"Bearer {token}"}, - ) - return response - - # Create a service, Set limit to 10 fragments - service = create_service(sms_daily_limit=10, message_limit=100) - - # Create 6 notifications in the db - template = create_sample_template(notify_db, notify_db_session, content="A" * 200, service=service, template_type="sms") - for x in range(6): - create_sample_notification(notify_db, notify_db_session, service=service) - - __send_sms(1) # 8/10 fragments - assert send_warning_email.called - - response = __send_sms(2) # 12/10 fragments - assert response.status_code == 429 - - __send_sms(1) # 10/10 fragments - assert send_limit_reached_email.called - - response = __send_sms(1) # 11/10 fragments - assert response.status_code == 429 # Ensure send is blocked - not sure why we send a 400 here and a 429 everywhere else - class TestBulkSend: @pytest.mark.parametrize("args", [{}, {"rows": [1, 2], "csv": "foo"}], ids=["no args", "both args"]) @@ -2623,38 +2349,11 @@ def test_post_bulk_flags_not_enough_remaining_sms_messages(self, notify_api, cli assert error_json["errors"] == [ { "error": "BadRequestError", - "message": "You only have 1 remaining sms message parts before you reach your daily limit. You've tried to send 2 message parts.", + "message": "You only have 1 remaining sms messages before you reach your daily limit. You've tried to send 2 sms messages.", } ] messages_count_mock.assert_called_once() - def test_post_bulk_flags_not_enough_remaining_sms_message_parts( - self, notify_api, client, notify_db, notify_db_session, mocker - ): - service = create_service(sms_daily_limit=10, message_limit=100) - template = create_sample_template(notify_db, notify_db_session, content=500 * "a", service=service, template_type="sms") - mocker.patch("app.v2.notifications.post_notifications.fetch_todays_total_message_count", return_value=9) - mocker.patch("app.v2.notifications.post_notifications.fetch_todays_requested_sms_count", return_value=9) - data = { - "name": "job_name", - "template_id": template.id, - "csv": rows_to_csv([["phone number"], ["6135551234"]]), - } - response = client.post( - "/v2/notifications/bulk", - data=json.dumps(data), - headers=[("Content-Type", "application/json"), create_authorization_header(service_id=template.service_id)], - ) - - assert response.status_code == 400 - error_json = json.loads(response.get_data(as_text=True)) - assert error_json["errors"] == [ - { - "error": "BadRequestError", - "message": "You only have 1 remaining sms message parts before you reach your daily limit. You've tried to send 4 message parts.", - } - ] - @pytest.mark.parametrize("data_type", ["rows", "csv"]) def test_post_bulk_flags_rows_with_errors(self, client, notify_db, notify_db_session, data_type): template = create_sample_template(notify_db, notify_db_session, template_type="email", content="Hello ((name))")