From f664568d2f6656551daa0d7040d062d175e475da Mon Sep 17 00:00:00 2001 From: Mike Pond <32133001+P0NDER0SA@users.noreply.github.com> Date: Thu, 18 Apr 2024 15:08:08 -0400 Subject: [PATCH 1/7] Minor changes to Readme and make file to simplify steps for running app in dev (#2157) --- Makefile | 8 +++--- README.md | 73 +------------------------------------------------------ 2 files changed, 5 insertions(+), 76 deletions(-) diff --git a/Makefile b/Makefile index 3f98e79566..debab9d61e 100644 --- a/Makefile +++ b/Makefile @@ -48,19 +48,19 @@ smoke-test: .PHONY: run run: ## Run the web app - flask run -p 6011 --host=0.0.0.0 + poetry run flask run -p 6011 --host=0.0.0.0 .PHONY: run-celery-local run-celery-local: ## Run the celery workers with all the queues - ./scripts/run_celery_local.sh + poetry run ./scripts/run_celery_local.sh .PHONY: run-celery-local-filtered run-celery-local-filtered: ## Run the celery workers with all queues but filter out common scheduled tasks - ./scripts/run_celery_local.sh 2>&1 >/dev/null | grep -iEv 'beat|in-flight-to-inbox|run-scheduled-jobs|check-job-status' + poetry run ./scripts/run_celery_local.sh 2>&1 >/dev/null | grep -iEv 'beat|in-flight-to-inbox|run-scheduled-jobs|check-job-status' .PHONY: run-celery-purge run-celery-purge: ## Purge the celery queues - ./scripts/run_celery_purge.sh + poetry run ./scripts/run_celery_purge.sh .PHONY: run-db run-db: ## psql to access dev database diff --git a/README.md b/README.md index d2c8a6bf01..e36ade9a6e 100644 --- a/README.md +++ b/README.md @@ -17,78 +17,7 @@ Contains: For any issues during the following instructions, make sure to review the **Frequent problems** section toward the end of the document. -### Local installation instruction - -#### On OS X: - -1. Install PyEnv with Homebrew. This will preserve your sanity. - -`brew install pyenv` - -2. Install Python 3.10.8 or whatever is the latest - -`pyenv install 3.10.8` - -3. If you expect no conflicts, set `3.10.8` as you default - -`pyenv global 3.10.8` - -4. Ensure it installed by running - -`python --version` - -if it did not, take a look here: https://github.com/pyenv/pyenv/issues/660 - -5. Install `poetry`: - -`pip install poetry==1.3.2` - -6. Restart your terminal and make your virtual environtment: - -`poetry env use $(which python)` - -8. Verify that the environment was created and activated by poetry - -`poetry env list` - -9. Install [Postgres.app](http://postgresapp.com/). - -10. Create the database for the application - -`createdb --user=postgres notification_api` - -11. Install the required environment variables via our LastPast Vault - -Within the team's *LastPass Vault*, you should find corresponding folders for this -project containing the `.env` content that you should copy in your project root folder. This -will grant the application necessary access to our internal infrastructure. - -If you don't have access to our *LastPass Vault* (as you evaluate our notification -platform for example), you will find a sane set of defaults exists in the `.env.example` -file. Copy that file to `.env` and customize it to your needs. - -12. Install all dependencies - -`poetry install` - -1. Generate the version file ?!? - -`make generate-version-file` - -14. Run all DB migrations - -`flask db upgrade` - -15. Run the service - -`make run` - -15a. To test - -`poetry install --with test` - -`make test` - +### Local installation instruction (Use Dev Containers) #### In a [VS Code devcontainer](https://code.visualstudio.com/docs/remote/containers-tutorial) 1. Install VS Code From 522451e4b0151acf088ae49a0304786b224c68fc Mon Sep 17 00:00:00 2001 From: Jimmy Royer Date: Thu, 18 Apr 2024 15:33:17 -0400 Subject: [PATCH 2/7] Retry callbacks on 429 'too many requests' code (#2155) * Handle 429 'too many requests' differently with callbacks by retrying later * Added comments on code intent. * Run formatting on test file --- app/celery/service_callback_tasks.py | 20 ++++++------------- .../app/celery/test_service_callback_tasks.py | 7 ++++--- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index c3a560b175..af3d51e3b2 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -65,31 +65,23 @@ def _send_data_to_service_callback_api(self, data, service_callback_url, token, data=json.dumps(data), headers={ "Content-Type": "application/json", - "Authorization": "Bearer {}".format(token), + "Authorization": f"Bearer {token}", }, timeout=5, ) current_app.logger.info( - "{} sending {} to {}, response {}".format( - function_name, - notification_id, - service_callback_url, - response.status_code, - ) + f"{function_name} sending {notification_id} to {service_callback_url}, response {response.status_code}" ) response.raise_for_status() except RequestException as e: current_app.logger.warning( - "{} request failed for notification_id: {} and url: {}. exc: {}".format( - function_name, notification_id, service_callback_url, e - ) + f"{function_name} request failed for notification_id: {notification_id} and url: {service_callback_url}. exc: {e}" ) - if not isinstance(e, HTTPError) or e.response.status_code >= 500: + # Retry if the response status code is server-side or 429 (too many requests). + if not isinstance(e, HTTPError) or e.response.status_code >= 500 or e.response.status_code == 429: try: self.retry(queue=QueueNames.CALLBACKS_RETRY) except self.MaxRetriesExceededError: current_app.logger.warning( - "Retry: {} has retried the max num of times for callback url {} and notification_id: {}".format( - function_name, service_callback_url, notification_id - ) + "Retry: {function_name} has retried the max num of times for callback url {service_callback_url} and notification_id: {notification_id}" ) diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index a8259ccd29..eda0e212d8 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -90,8 +90,9 @@ def test_send_complaint_to_service_posts_https_request_to_service_with_signed_da @pytest.mark.parametrize("notification_type", ["email", "letter", "sms"]) -def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_signed_data( - notify_db_session, mocker, notification_type +@pytest.mark.parametrize("status_code", [429, 500, 503]) +def test__send_data_to_service_callback_api_retries_if_request_returns_error_code_with_signed_data( + notify_db_session, mocker, notification_type, status_code ): callback_api, template = _set_up_test_data(notification_type, "delivery_status") datestr = datetime(2017, 6, 20) @@ -107,7 +108,7 @@ def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_ signed_data = _set_up_data_for_status_update(callback_api, notification) mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.retry") with requests_mock.Mocker() as request_mock: - request_mock.post(callback_api.url, json={}, status_code=500) + request_mock.post(callback_api.url, json={}, status_code=status_code) send_delivery_status_to_service(notification.id, signed_status_update=signed_data) assert mocked.call_count == 1 From a049ee26c354bbd831aaed6e84d8c30a41cde33b Mon Sep 17 00:00:00 2001 From: Jumana B Date: Wed, 24 Apr 2024 09:52:55 -0400 Subject: [PATCH 3/7] Added alt text for email_branding table (#2159) * Added ialt text for email_branding table * test fix * fix --- app/models.py | 4 +++ migrations/versions/0446_add_alt_text.py | 34 ++++++++++++++++++++++++ tests/app/email_branding/test_rest.py | 8 +++++- 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 migrations/versions/0446_add_alt_text.py diff --git a/app/models.py b/app/models.py index 81767406a1..215ad47372 100644 --- a/app/models.py +++ b/app/models.py @@ -280,6 +280,8 @@ class EmailBranding(BaseModel): UUID(as_uuid=True), db.ForeignKey("organisation.id", ondelete="SET NULL"), index=True, nullable=True ) organisation = db.relationship("Organisation", back_populates="email_branding", foreign_keys=[organisation_id]) + alt_text_en = db.Column(db.String(), nullable=True) + alt_text_fr = db.Column(db.String(), nullable=True) def serialize(self) -> dict: serialized = { @@ -290,6 +292,8 @@ def serialize(self) -> dict: "text": self.text, "brand_type": self.brand_type, "organisation_id": str(self.organisation_id) if self.organisation_id else "", + "alt_text_en": self.alt_text_en, + "alt_text_fr": self.alt_text_fr, } return serialized diff --git a/migrations/versions/0446_add_alt_text.py b/migrations/versions/0446_add_alt_text.py new file mode 100644 index 0000000000..868ce33db7 --- /dev/null +++ b/migrations/versions/0446_add_alt_text.py @@ -0,0 +1,34 @@ +""" +Revision ID: 0446_add_alt_text.py +Revises: 0445_add_org_id_branding.py +Create Date: 2024-04-23 +""" +import sqlalchemy as sa +from alembic import op +from sqlalchemy import text + +revision = "0446_add_alt_text" +down_revision = "0445_add_org_id_branding" + + +def upgrade(): + table_description = op.get_bind().execute( + text("SELECT * FROM information_schema.columns WHERE table_name = 'email_branding'") + ) + + # Check if the column exists + if "alt_text_en" not in [column["column_name"] for column in table_description]: + op.add_column( + "email_branding", + sa.Column("alt_text_en", sa.String(), nullable=True), + ) + if "alt_text_fr" not in [column["column_name"] for column in table_description]: + op.add_column( + "email_branding", + sa.Column("alt_text_fr", sa.String(), nullable=True), + ) + + +def downgrade(): + op.drop_column("email_branding", "alt_text_fr") + op.drop_column("email_branding", "alt_text_en") diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 05e0b5a48e..5cee670554 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -39,7 +39,9 @@ def test_get_email_branding_options_filter_org(admin_request, notify_db, notify_ def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): - email_branding = EmailBranding(colour="#FFFFFF", logo="/path/image.png", name="Some Org", text="My Org") + email_branding = EmailBranding( + colour="#FFFFFF", logo="/path/image.png", name="Some Org", text="My Org", alt_text_en="hello world" + ) notify_db.session.add(email_branding) notify_db.session.commit() @@ -57,6 +59,8 @@ def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): "text", "brand_type", "organisation_id", + "alt_text_en", + "alt_text_fr", } assert response["email_branding"]["colour"] == "#FFFFFF" assert response["email_branding"]["logo"] == "/path/image.png" @@ -64,6 +68,8 @@ def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): assert response["email_branding"]["text"] == "My Org" assert response["email_branding"]["id"] == str(email_branding.id) assert response["email_branding"]["brand_type"] == str(email_branding.brand_type) + assert response["email_branding"]["alt_text_en"] == "hello world" + assert response["email_branding"]["alt_text_fr"] is None def test_post_create_email_branding(admin_request, notify_db_session): From 6b24be4dc32984d77926b5d9702d198be040c459 Mon Sep 17 00:00:00 2001 From: William B <7444334+whabanks@users.noreply.github.com> Date: Wed, 24 Apr 2024 15:25:43 -0400 Subject: [PATCH 4/7] Add error message for rows_with_combined_variable_content_too_long (#2160) * Add error message for rows_with_combined_variable_content_too_long * Fix tests - Bump Waffles version - Bump Utils version --- .github/workflows/test.yaml | 2 +- app/v2/notifications/post_notifications.py | 7 +++++++ poetry.lock | 8 ++++---- pyproject.toml | 2 +- tests/app/celery/test_tasks.py | 14 +++++++++----- .../v2/notifications/test_post_notifications.py | 4 ++-- 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 8d5c023e27..d5d607856c 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@06a40db6286f525fe3551e029418458d33342592 # 52.1.0 + uses: cds-snc/notification-utils/.github/actions/waffles@4e204b1ec688961ae32c3a6646246c58a1c1bc4f # 52.1.10 with: app-loc: '/github/workspace' app-libs: '/github/workspace/env/site-packages' diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 9a1640b9a2..ee69366f9d 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -7,6 +7,7 @@ import werkzeug from flask import abort, current_app, jsonify, request +from notifications_utils import SMS_CHAR_COUNT_LIMIT from notifications_utils.recipients import ( RecipientCSV, try_validate_and_format_phone_number, @@ -699,6 +700,12 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_messages): message=f"You cannot send to these recipients {explanation}", status_code=400, ) + if any(recipient_csv.rows_with_combined_variable_content_too_long): + raise BadRequestError( + message=f"Row {next(recipient_csv.rows_with_combined_variable_content_too_long).index + 1} - has a character count greater than {SMS_CHAR_COUNT_LIMIT} characters. Some messages may be too long due to custom content.", + status_code=400, + ) + if recipient_csv.rows_with_errors: def row_error(row): diff --git a/poetry.lock b/poetry.lock index 1ab0346e40..3417ce4eb3 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2444,7 +2444,7 @@ requests = ">=2.0.0" [[package]] name = "notifications-utils" -version = "52.1.5" +version = "52.1.10" description = "Shared python code for Notification - Provides logging utils etc." optional = false python-versions = "~3.10.9" @@ -2479,8 +2479,8 @@ werkzeug = "2.3.7" [package.source] type = "git" url = "https://github.com/cds-snc/notifier-utils.git" -reference = "52.1.5" -resolved_reference = "9d9e8c7c32e3608f4dd8f320eaba4bb67edfcbf5" +reference = "52.1.10" +resolved_reference = "4e204b1ec688961ae32c3a6646246c58a1c1bc4f" [[package]] name = "ordered-set" @@ -4255,4 +4255,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "f00992b7f47d8434a76d0be08135eace31315c696e20a222a10e2bf926e8a561" +content-hash = "c4c46eaf3c731be2eb3e27d29405e0c79ab17d53eaf3e7728f80145abc39c88e" diff --git a/pyproject.toml b/pyproject.toml index db3ae6fff3..4b73ba0a7d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,7 +64,7 @@ Werkzeug = "2.3.7" MarkupSafe = "2.1.4" # 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", tag = "52.1.5" } +notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.1.10" } # 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/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 437d59e7ec..0bbaff06ba 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -2,7 +2,7 @@ import uuid from datetime import datetime, timedelta from unittest import mock -from unittest.mock import Mock, call +from unittest.mock import MagicMock, Mock, call import pytest import requests_mock @@ -891,9 +891,10 @@ def test_process_rows_sends_save_task( mocker.patch("app.celery.tasks.create_uuid", return_value="noti_uuid") task_mock = mocker.patch("app.celery.tasks.{}".format(expected_function)) signer_mock = mocker.patch("app.celery.tasks.signer_notification.sign") - template = Mock(id="template_id", template_type=template_type, process_type=NORMAL) + template = MagicMock(id="template_id", template_type=template_type, process_type=NORMAL) job = Mock(id="job_id", template_version="temp_vers", notification_count=1, api_key_id=api_key_id, sender_id=sender_id) service = Mock(id="service_id", research_mode=research_mode) + template.__len__.return_value = 1 process_rows( [ @@ -950,10 +951,11 @@ def test_should_redirect_email_job_to_queue_depending_on_csv_threshold( ): mock_save_email = mocker.patch("app.celery.tasks.save_emails") - template = Mock(id=1, template_type=EMAIL_TYPE, process_type=template_process_type) + template = MagicMock(id=1, template_type=EMAIL_TYPE, process_type=template_process_type) api_key = Mock(id=1, key_type=KEY_TYPE_NORMAL) job = Mock(id=1, template_version="temp_vers", notification_count=1, api_key=api_key) service = Mock(id=1, research_mode=False) + template.__len__.return_value = 1 row = next( RecipientCSV( @@ -994,10 +996,11 @@ def test_should_redirect_sms_job_to_queue_depending_on_csv_threshold( ): mock_save_sms = mocker.patch("app.celery.tasks.save_smss") - template = Mock(id=1, template_type=SMS_TYPE, process_type=template_process_type) + template = MagicMock(id=1, template_type=SMS_TYPE, process_type=template_process_type) api_key = Mock(id=1, key_type=KEY_TYPE_NORMAL) job = Mock(id=1, template_version="temp_vers", notification_count=1, api_key=api_key) service = Mock(id=1, research_mode=False) + template.__len__.return_value = 1 row = next( RecipientCSV( @@ -1066,7 +1069,8 @@ def test_process_rows_works_without_key_type( mocker.patch("app.celery.tasks.create_uuid", return_value="noti_uuid") task_mock = mocker.patch("app.celery.tasks.{}".format(expected_function)) signer_mock = mocker.patch("app.celery.tasks.signer_notification.sign") - template = Mock(id="template_id", template_type=template_type, process_type=NORMAL) + template = MagicMock(id="template_id", template_type=template_type, process_type=NORMAL) + template.__len__.return_value = 1 api_key = {} job = Mock( id="job_id", diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 2e3d1d36b2..a5b3da2fc0 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -2495,7 +2495,7 @@ def test_post_bulk_with_too_large_sms_fails(self, client, notify_db, notify_db_s mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4())) service = create_service(sms_daily_limit=10, message_limit=100) - template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms", content="a" * 612) + template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms", content="a" * 613) data = { "name": "job_name", "template_id": template.id, @@ -2546,7 +2546,7 @@ def test_post_bulk_with_too_large_sms_fail_and_shows_correct_row( ) assert response.status_code == 400 assert "has a character count greater than" in str(response.data) - assert "row #{}".format(failure_row) in str(response.data) + assert "Row {}".format(failure_row) in str(response.data) class TestBatchPriorityLanes: From 1083dc39688bd2eab13db687fe53beaa1cb5e9eb Mon Sep 17 00:00:00 2001 From: Andrew Date: Thu, 25 Apr 2024 15:03:15 -0300 Subject: [PATCH 5/7] a11y(2fa email template): update template to make 2FA code easier to select (#2161) --- .../0447_update_verify_code_template.py | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 migrations/versions/0447_update_verify_code_template.py diff --git a/migrations/versions/0447_update_verify_code_template.py b/migrations/versions/0447_update_verify_code_template.py new file mode 100644 index 0000000000..9db7e8f1c8 --- /dev/null +++ b/migrations/versions/0447_update_verify_code_template.py @@ -0,0 +1,97 @@ +""" + +Revision ID: 0447_update_verify_code_template +Revises: 0446_add_alt_text +Create Date: 2023-10-05 00:00:00 + +""" +from datetime import datetime + +from alembic import op +from flask import current_app + +revision = "0447_update_verify_code_template" +down_revision = "0446_add_alt_text" + +near_content = "\n".join( + [ + "[[en]]", + "Hi ((name)),", + "", + "Here is your security code to log in to GC Notify:", + "", + "^ **((verify_code))**", + "[[/en]]", + "", + "---", + "", + "[[fr]]", + "Bonjour ((name)),", + "", + "Voici votre code de sécurité pour vous connecter à Notification GC:", + "", + "^ **((verify_code))**", + "[[/fr]]", + ] +) + + +templates = [ + { + "id": current_app.config["EMAIL_2FA_TEMPLATE_ID"], + "template_type": "email", + "subject": "Sign in | Connectez-vous", + "content": near_content, + "process_type": "priority", + }, +] + + +def upgrade(): + conn = op.get_bind() + + for template in templates: + current_version = conn.execute("select version from templates where id='{}'".format(template["id"])).fetchone() + name = conn.execute("select name from templates where id='{}'".format(template["id"])).fetchone() + template["version"] = current_version[0] + 1 + template["name"] = name[0] + + template_update = """ + UPDATE templates SET content = '{}', subject = '{}', version = '{}', updated_at = '{}' + WHERE id = '{}' + """ + template_history_insert = """ + INSERT INTO templates_history (id, name, template_type, created_at, content, archived, service_id, subject, + created_by_id, version, process_type, hidden) + VALUES ('{}', '{}', '{}', '{}', '{}', False, '{}', '{}', '{}', {}, '{}', false) + """ + + for template in templates: + op.execute( + template_update.format( + template["content"], + template["subject"], + template["version"], + datetime.utcnow(), + template["id"], + ) + ) + + op.execute( + template_history_insert.format( + template["id"], + template["name"], + template["template_type"], + datetime.utcnow(), + template["content"], + current_app.config["NOTIFY_SERVICE_ID"], + template["subject"], + current_app.config["NOTIFY_USER_ID"], + template["version"], + template["process_type"], + ) + ) + + +def downgrade(): + pass From eb16fee56b9260c3614f1e62fb98d21729ecd42c Mon Sep 17 00:00:00 2001 From: Ben Larabie Date: Fri, 26 Apr 2024 08:01:36 -0400 Subject: [PATCH 6/7] upping perf test to 10 mins (#2039) Co-authored-by: Jumana B --- tests-perf/locust/locust.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests-perf/locust/locust.conf b/tests-perf/locust/locust.conf index c1eba3b220..76aa3d2273 100644 --- a/tests-perf/locust/locust.conf +++ b/tests-perf/locust/locust.conf @@ -3,7 +3,7 @@ locustfile = tests-perf/locust/locust-notifications.py host = https://api.staging.notification.cdssandbox.xyz users = 3000 spawn-rate = 20 -run-time = 5m +run-time = 10m # headless = true # master = true From 5b29366eeb563c81f7306c68ad41b9a0e7802bf7 Mon Sep 17 00:00:00 2001 From: Jumana B Date: Fri, 26 Apr 2024 10:49:57 -0400 Subject: [PATCH 7/7] Revert "Add error message for rows_with_combined_variable_content_too_long" (#2164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Revert "Add error message for rows_with_combined_variable_content_too_long (#…" This reverts commit 6b24be4dc32984d77926b5d9702d198be040c459. * Bump utils to 52.2.0 --------- Co-authored-by: wbanks --- .github/workflows/test.yaml | 2 +- app/v2/notifications/post_notifications.py | 7 ------- poetry.lock | 8 ++++---- pyproject.toml | 2 +- tests/app/celery/test_tasks.py | 14 +++++--------- .../v2/notifications/test_post_notifications.py | 4 ++-- 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index d5d607856c..8d5c023e27 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@4e204b1ec688961ae32c3a6646246c58a1c1bc4f # 52.1.10 + uses: cds-snc/notification-utils/.github/actions/waffles@06a40db6286f525fe3551e029418458d33342592 # 52.1.0 with: app-loc: '/github/workspace' app-libs: '/github/workspace/env/site-packages' diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index ee69366f9d..9a1640b9a2 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -7,7 +7,6 @@ import werkzeug from flask import abort, current_app, jsonify, request -from notifications_utils import SMS_CHAR_COUNT_LIMIT from notifications_utils.recipients import ( RecipientCSV, try_validate_and_format_phone_number, @@ -700,12 +699,6 @@ def check_for_csv_errors(recipient_csv, max_rows, remaining_messages): message=f"You cannot send to these recipients {explanation}", status_code=400, ) - if any(recipient_csv.rows_with_combined_variable_content_too_long): - raise BadRequestError( - message=f"Row {next(recipient_csv.rows_with_combined_variable_content_too_long).index + 1} - has a character count greater than {SMS_CHAR_COUNT_LIMIT} characters. Some messages may be too long due to custom content.", - status_code=400, - ) - if recipient_csv.rows_with_errors: def row_error(row): diff --git a/poetry.lock b/poetry.lock index 3417ce4eb3..5002212cb4 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2444,7 +2444,7 @@ requests = ">=2.0.0" [[package]] name = "notifications-utils" -version = "52.1.10" +version = "52.2.0" description = "Shared python code for Notification - Provides logging utils etc." optional = false python-versions = "~3.10.9" @@ -2479,8 +2479,8 @@ werkzeug = "2.3.7" [package.source] type = "git" url = "https://github.com/cds-snc/notifier-utils.git" -reference = "52.1.10" -resolved_reference = "4e204b1ec688961ae32c3a6646246c58a1c1bc4f" +reference = "52.2.0" +resolved_reference = "0146718a423b0144566392ab3082d15779f4a73f" [[package]] name = "ordered-set" @@ -4255,4 +4255,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "c4c46eaf3c731be2eb3e27d29405e0c79ab17d53eaf3e7728f80145abc39c88e" +content-hash = "18ed30bed79c84db2cc559fecdba223a88c8d8546f4fb951f85ef1d76142a714" diff --git a/pyproject.toml b/pyproject.toml index 4b73ba0a7d..973ced94f6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,7 +64,7 @@ Werkzeug = "2.3.7" MarkupSafe = "2.1.4" # 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", tag = "52.1.10" } +notifications-utils = { git = "https://github.com/cds-snc/notifier-utils.git", tag = "52.2.0" } # 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/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 0bbaff06ba..437d59e7ec 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -2,7 +2,7 @@ import uuid from datetime import datetime, timedelta from unittest import mock -from unittest.mock import MagicMock, Mock, call +from unittest.mock import Mock, call import pytest import requests_mock @@ -891,10 +891,9 @@ def test_process_rows_sends_save_task( mocker.patch("app.celery.tasks.create_uuid", return_value="noti_uuid") task_mock = mocker.patch("app.celery.tasks.{}".format(expected_function)) signer_mock = mocker.patch("app.celery.tasks.signer_notification.sign") - template = MagicMock(id="template_id", template_type=template_type, process_type=NORMAL) + template = Mock(id="template_id", template_type=template_type, process_type=NORMAL) job = Mock(id="job_id", template_version="temp_vers", notification_count=1, api_key_id=api_key_id, sender_id=sender_id) service = Mock(id="service_id", research_mode=research_mode) - template.__len__.return_value = 1 process_rows( [ @@ -951,11 +950,10 @@ def test_should_redirect_email_job_to_queue_depending_on_csv_threshold( ): mock_save_email = mocker.patch("app.celery.tasks.save_emails") - template = MagicMock(id=1, template_type=EMAIL_TYPE, process_type=template_process_type) + template = Mock(id=1, template_type=EMAIL_TYPE, process_type=template_process_type) api_key = Mock(id=1, key_type=KEY_TYPE_NORMAL) job = Mock(id=1, template_version="temp_vers", notification_count=1, api_key=api_key) service = Mock(id=1, research_mode=False) - template.__len__.return_value = 1 row = next( RecipientCSV( @@ -996,11 +994,10 @@ def test_should_redirect_sms_job_to_queue_depending_on_csv_threshold( ): mock_save_sms = mocker.patch("app.celery.tasks.save_smss") - template = MagicMock(id=1, template_type=SMS_TYPE, process_type=template_process_type) + template = Mock(id=1, template_type=SMS_TYPE, process_type=template_process_type) api_key = Mock(id=1, key_type=KEY_TYPE_NORMAL) job = Mock(id=1, template_version="temp_vers", notification_count=1, api_key=api_key) service = Mock(id=1, research_mode=False) - template.__len__.return_value = 1 row = next( RecipientCSV( @@ -1069,8 +1066,7 @@ def test_process_rows_works_without_key_type( mocker.patch("app.celery.tasks.create_uuid", return_value="noti_uuid") task_mock = mocker.patch("app.celery.tasks.{}".format(expected_function)) signer_mock = mocker.patch("app.celery.tasks.signer_notification.sign") - template = MagicMock(id="template_id", template_type=template_type, process_type=NORMAL) - template.__len__.return_value = 1 + template = Mock(id="template_id", template_type=template_type, process_type=NORMAL) api_key = {} job = Mock( id="job_id", diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index a5b3da2fc0..2e3d1d36b2 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -2495,7 +2495,7 @@ def test_post_bulk_with_too_large_sms_fails(self, client, notify_db, notify_db_s mocker.patch("app.v2.notifications.post_notifications.create_bulk_job", return_value=str(uuid.uuid4())) service = create_service(sms_daily_limit=10, message_limit=100) - template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms", content="a" * 613) + template = create_sample_template(notify_db, notify_db_session, service=service, template_type="sms", content="a" * 612) data = { "name": "job_name", "template_id": template.id, @@ -2546,7 +2546,7 @@ def test_post_bulk_with_too_large_sms_fail_and_shows_correct_row( ) assert response.status_code == 400 assert "has a character count greater than" in str(response.data) - assert "Row {}".format(failure_row) in str(response.data) + assert "row #{}".format(failure_row) in str(response.data) class TestBatchPriorityLanes: