cds-snc / notification-planning

Project planning for GC Notify Team
4 stars 0 forks source link

"email address" is allowed as a conditional variable, and causes a 500 error #1511

Open amazingphilippe opened 4 months ago

amazingphilippe commented 4 months ago

Describe the bug

When setting ((email address?? conditional content)) Notify will not know be able to generate the email template and throw a generic 500 error.

Its unlikely to happen, because it makes no sense to do this, but we could handle the error a bit more gracefully, or not allow "email address" or "phone number" to be conditionals. And throw a validation error on template edit.

Bug Severity

See examples in the documentation

SEV-4

To Reproduce

Steps to reproduce the behavior:

  1. Go to a template
  2. Add this conditional variable ((email address?? conditional content)) or ((phone number?? conditional content))
  3. Try to send yourself a test
  4. Get a 500

Expected behavior

To know what broke.

Impact

Some confusion, though again this is an unlikely scenario that I encountered because I was testing random stuff.

yaelberger-commits commented 2 months ago

Should be handled better than 500 error and through some sort of validation with error message

yaelberger-commits commented 2 months ago

email address variable gets a added at some point, conversion happens when we process the CSV. Figure out where the conditional check is happening, add the at an earlier point.