Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reinstate bulk sms limit #1834

Merged
merged 14 commits into from
May 22, 2024
Merged

Reinstate bulk sms limit #1834

merged 14 commits into from
May 22, 2024

Conversation

whabanks
Copy link
Contributor

@whabanks whabanks commented May 3, 2024

Summary | Résumé

This PR reinstates bulk sms validation code that was reverted in this PR due to issues with the added validation firing on bulk emails as well.

Related Issues | Cartes liées

cds-snc/notification-utils#294
cds-snc/notification-api#2169

Test instructions | Instructions pour tester la modification

Set up

  • SMS template with 1 variable and some content.
  • SMS template with 2 (or more) variables and some content
  • Create CSVs with some rows where the variables do not exceed 612 characters, and some rows that do.

Test 1

  1. Try to send using the CSV's that go over the limit for both templates
  2. Note that an error message is displayed at the top of the Edit your spreadsheet screen
  3. Note that inline error messages are displayed on the cells with variable content indicating the length limit.

Test 2

  1. Change the CSV's so there are no more errors
  2. Re-upload and verify that normal sending still functions as expected.

- Bumped boto3 so we have compatibility with latest utils
- Added a check to utils.py to ensure we only return validation errors for SMS bulk sends, not email
Copy link

github-actions bot commented May 3, 2024

@whabanks whabanks merged commit 23196f1 into main May 22, 2024
9 checks passed
@whabanks whabanks deleted the reinstate-bulk-sms-limit branch May 22, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants