cds-snc / cds-snc/notification-planning
Improve the handling of notifications sent using TEST keys
- Dominant language
- No language data
- Stars
- 5
- Forks
- 0
- Avg merge
- 19m
- Merged PRs (30d)
- 1
Description
## Issue
Currently, notifications sent using test keys via the `/email`, `/sms` and `/bulk/` endpoints go through our entire sending process (i.e. API -> redis -> SQS -> db -> celery send workers) just to never actually send.
Keeping in line with the documentation surrounding test keys, we should be able to simplify this code and reduce other bugs and issues:
> Messages sent using a test key:
> - generate realistic responses
> - result in a delivered status
> - are not actually delivered to a recipient
> - do not appear on your dashboard
> - do not count against your email and text message allowances
### Improvement
Instead of short circuiting at the very end of the send process, we could simplify this process (and our code) to return a `201 created` as soon as possible in our code.
### Acceptance criteria
#### SMS
- **Notification api call succeeds and no db entry is written for:**
- [ ] a phone number in the safelist
- [ ] a phone number in the team list
- [ ] a phone number not in the team list or safelist
#### Emails
- **Notification api call succeeds and no db entry is written for:**
- [ ] an email address in the safelist
- [ ] and email address in the team list
- [ ] an email address not in the team list or safelist
#### Bulk
- **Notification api call succeeds and no db entry is written for:**
- [ ] 2 email addresses in the safelist
- [ ] 2 email email addresses in the team list
- [ ] 1 email email address in the team list and 1 email address in the safe list
- [ ] 2 email addresses not in the safelist or team list
- [ ] 1 email addresses in the safe list, 1 in the team list and 1 not in either
## Implementation notes
### Endpoints affected
There are three endpoints we will need to short circuit for test keys:
1. Sending one-off SMS: `POST /v2/notifications/sms` -> [code is here](https://github.com/cds-snc/notification-api/blob/main/app/v2/notifications/post_notifications.py#L177)
1. Sending one-off emails: `POST /v2/notifications/email` -> [code is here](https://github.com/cds-snc/notification-api/blob/main/app/v2/notifications/post_notifications.py#L177)
1. Sending bulk notifications: `POST /v2/notifications/bulk` -> [code is here](https://github.com/cds-snc/notification-api/blob/main/app/v2/notifications/post_notifications.py#L132)
Relates to #805
Contributor guide
Research direction
Start with the one-off SMS and email handlers in app/v2/notifications/post_notifications.py around line 177, then inspect the bulk handler around line 132 and the test-key flow. The change is done when test-key requests return 201 without database entries for every listed safelist, team-list, non-listed, and bulk combination.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, backend, databases
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100