[BACK-3478] Add email notifications work system processors. #896
Conversation
…sers on specific events.
| @@ -0,0 +1,129 @@ | |||
| package claimaccount | |||
There was a problem hiding this comment.
In order to fit in with the other platform and work implementations, how about we move this to notifications/work/users/claim. And, for the others, notifications/work/users/connections/new and notifications/work/users/connections/issues.
There was a problem hiding this comment.
I went with a middle ground of
notifications/work/claims
*notifications/work/connections/requestsnotifications/work/connections/issues
Not a huge fan of packages where the parent packages are empty. IMO some package named connectionissues gives me pretty good idea of what it deals with, in the same way package stringutil does. When it gets to connections => subcategory is where things can start getting hairy if then sub-subcategories are added.
…itionalnotifications => notifications.
darinkrauss
left a comment
darinkrauss
left a comment
There was a problem hiding this comment.
A number of quick, minor changes. Also, I didn't highlight every instance, but please replace all fmt.Errorf with errors.Newf or errors.Wrapf, as appropriate. Otherwise, looks good.
|
|
||
| type Metadata struct { | ||
| ClinicID string `json:"clinicId,omitempty"` | ||
| UserId string `json:"userId,omitempty"` |
There was a problem hiding this comment.
Uppercase ID per Golang convention:
| UserId string `json:"userId,omitempty"` | |
| UserID string `json:"userId,omitempty"` |
|
Also, it looks like the build is failing. Try running |
darinkrauss
left a comment
darinkrauss
left a comment
There was a problem hiding this comment.
LGTM, just one question out of curiosity.
| script: | ||
| - export TIMING_CMD='time -p' | ||
| - make plugins-visibility-${PLUGINS_VISIBILITY} && make ci | ||
| - make "plugins-visibility-${PLUGINS_VISIBILITY}" && make ci |
There was a problem hiding this comment.
Was this to silence some linter warning or another?
No description provided.