fix: fix dormancy notifications #14029
Conversation
Dormancy templates have been updated for better readability
|
|
||
| var bodyTmpl string | ||
| err := sql. | ||
| QueryRow("SELECT body_template FROM notification_templates WHERE id = $1 LIMIT 1", tc.id). |
There was a problem hiding this comment.
I've inlined this query because it's only used for tests now. I don't think the overhead of adding a new SQLC query would be justifiable. I actually tried that, and I encountered a few accounting errors because the query wasn't being used in the non-test codebase.
|
|
||
| if _, err := s.NotificationsEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.TemplateWorkspaceAutobuildFailed, | ||
| map[string]string{ | ||
| "name": workspace.Name, | ||
| "initiator": initiator, |
There was a problem hiding this comment.
I found out that TemplateWorkspaceAutobuildFailed was not using the initiator label in the template body so I just removed it.
|
Unfortunately, those tests don't prevent wrong labels from being used or required labels from being missed. To make that happen, the notifications tests would have to use the database. I think it would be valid but not in the scope for this PR. |
coderd/database/migrations/000232_update_dormancy_notification_template.down.sql
Outdated
Show resolved
Hide resolved
coderd/database/migrations/000232_update_dormancy_notification_template.down.sql
Outdated
Show resolved
Hide resolved
coderd/database/migrations/000232_update_dormancy_notification_template.up.sql
Show resolved
Hide resolved
Co-authored-by: Danny Kopping <danny@coder.com>
Co-authored-by: Danny Kopping <danny@coder.com>
Co-authored-by: Danny Kopping <danny@coder.com>
Co-authored-by: Danny Kopping <danny@coder.com>
…mancy-template
…oder into bq/fix-dormancy-template
|
Issue created for tests: coder/internal#32 |
There was a problem hiding this comment.
Almost there!
I don't need to re-review once you've address the one remaining comment
Thanks @BrunoQuaresma
Uh oh!
There was an error while loading. Please reload this page.