Null Text Message Sender causing 500 error on dashboard

sastels commented 4 months ago

Describe the bug

Got a 500 error going to the dashboard of the JLR - Short code test | Test petit code service

Bug Severity

See examples in the documentation

~SEV-2 : not affecting production but we have to fix this before we can rely on being able to use the short code~

Update: SEV-4: it wasn't the shortcode

To Reproduce

Steps to reproduce the behavior:

Go to https://notification.canada.ca/services/9b37c2b1-e96a-4fcc-8e76-a095be97845b

Note that doing this 5 times will cause an OpsGenie page

Expected behavior

no 500s

Impact

~Can't reliably use short code~ Actually no impact for normal use cases (you have to manually edit the database to cause the bug)

Additional context

Slack discussion

QA

Unfortunately, we click-ops fixed the issue by setting the "Text message senders" in the admin settings to "CANADA.CA". We have tried and cannot re-break it.

sastels commented 4 months ago

IT turned out that the problem was that the Text Message Senders was set to None

However the code that is crashing is supposed to not crash even if the input is invalid, so we should fix this in any case.

sastels commented 3 months ago

https://github.com/cds-snc/notification-utils/pull/279

ben851 commented 3 months ago

@ben851 to review today

ben851 commented 3 months ago

Approved and merged!

ben851 commented 3 months ago

@ben851 to QA

sastels commented 3 months ago

Two PRs to review: use new utils in admin use new utils in api

ben851 commented 3 months ago

Ben to review

jimleroyer commented 3 months ago

All PRs are approved. We can move this to QA.

sastels commented 3 months ago

opps, may as well upgrade document-download-api to the latest utls while we're at it https://github.com/cds-snc/notification-document-download-api/pull/159

cds-snc / notification-planning-core