feat(certification): Optionally email certifier #628
Conversation
dleard
commented
May 8, 2020
dleard
commented
- Add checkbox to optionally notify certifier by email
- Updates certification_url table with boolean to indicate whether to send an email
- Updates trigger function to not send email on boolean = false
- Create pgTap tests to test trigger function
- Update jest/cypress tests
dleard
force-pushed
the
feature/optional-certifier-email
branch
from
72f9aad to
e9e04c7
Compare
| @@ -18,7 +18,8 @@ create table ggircs_portal.certification_url ( | |||
| updated_by int references ggircs_portal.ciip_user, | |||
| deleted_at timestamp with time zone, | |||
| deleted_by int references ggircs_portal.ciip_user, | |||
| certification_request_sent_to varchar(1000), | |||
| send_certification_request boolean not null default false, | |||
There was a problem hiding this comment.
UX, maybe already addressed: if this is false by default how do we ensure users know that this critical email won't be sent unless the toggle is clicked?
Out of scope on this PR but worth closing the loop with @rajpersram maybe?
There was a problem hiding this comment.
Maybe the default should be true and the toggle should begin as clicked? I'll make a task in youtrack to visit the idea with @rajpersram
There was a problem hiding this comment.
| -- timestamp when the email was sent | ||
| new.certification_request_sent_at = now(); | ||
| return new; | ||
| if (new.send_certification_request = true) then |
There was a problem hiding this comment.
I'd love to see ggircs_portal_private.run_graphile_worker_job split so instead of switching when tg_argv[0] = 'certification_request' each of those blocks is contained within a function. This function is really scheduling jobs rather than running them, right? Tech debt for later I think (should add to tracker)
There was a problem hiding this comment.
I'll make an unscheduled tech debt task for this in youtrack
There was a problem hiding this comment.
…ification_url table & update trigger to use it
…er behaves appropriately
dleard
force-pushed
the
feature/optional-certifier-email
branch
from
e9e04c7 to
1d8a0f3
Compare
