feat: save history of notification sends #557
Conversation
adityathebe
force-pushed
the
feat/notification-send-history
branch
from
8f73b72
to
f9fe64d
Compare
jobs/jobs.go
Outdated
| @@ -63,6 +66,16 @@ func Start() { | |||
| logger.Errorf("Failed to schedule job for cleaning up event queue table: %v", err) | |||
| } | |||
|
|
|||
| if _, err := ScheduleFunc(CleanupNotificationSendHistorySchedule, func() { | |||
| if count, err := duty.DeleteNotificationSendHistory(api.NewContext(db.Gorm, nil), 30); err != nil { | |||
There was a problem hiding this comment.
nit: The logging of the results should probably be in the function itself, and we should be passing through a DbContext rather than db.Gorm
There was a problem hiding this comment.
The logging of the results should probably be in the function itself
I am not sure if the duty func should be doing the logging.
we should be passing through a DbContext rather than db.Gorm
All of the jobs use db.Gorm directly and there's no DBContext at this stage yet. I guess we could pass gorm directly to the jobs starting function
func Start() {}
to
func Start(db *gorm.DB) {}
There was a problem hiding this comment.
We should be using the DBContext exported by duty
I don't see why the logging shouldn't be done in duty, if we ever modify this function maybe to delete by type, or delete by errors, etc.. We would need to update the logic and the function return, publish and then update in incident commander.
We schedule the job from incident commander, just be cause duty is a pure library without global context, that doesn't mean the job details should leak into incident commander
Resolves: #553