Skip to content

Commit 36644b2

Browse files
mtojekmtojek
committed
expanding
1 parent f7826d6 commit 36644b2

File tree

3 files changed

+36
-13
lines changed

3 files changed

+36
-13
lines changed

‎coderd/database/dbmem/dbmem.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1715,7 +1715,7 @@ func (q *FakeQuerier) DeleteOldReportGeneratorLogs(_ context.Context, params dat
17151715

17161716
var validLogs []database.ReportGeneratorLog
17171717
for _, record := range q.reportGeneratorLogs {
1718-
if record.NotificationTemplateID != params.NotificationTemplateID || record.LastGeneratedAt.Before(params.Before) {
1718+
if record.NotificationTemplateID != params.NotificationTemplateID || record.LastGeneratedAt.After(params.Before) {
17191719
validLogs = append(validLogs, record)
17201720
}
17211721
}

‎coderd/notifications/reports/generator.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,11 @@ func (i *reportGenerator) Close() error {
101101
const failedWorkspaceBuildsReportFrequencyDays = 7
102102

103103
func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db database.Store, enqueuer notifications.Enqueuer, clk quartz.Clock) error {
104-
statsRows, err := db.GetWorkspaceBuildStatsByTemplates(ctx, dbtime.Time(clk.Now().Add(-failedWorkspaceBuildsReportFrequencyDays*24*time.Hour)).UTC())
104+
now := clk.Now()
105+
since := now.Add(-failedWorkspaceBuildsReportFrequencyDays * 24 * time.Hour)
106+
107+
// TODO skip new templates
108+
statsRows, err := db.GetWorkspaceBuildStatsByTemplates(ctx, dbtime.Time(since).UTC())
105109
if err != nil {
106110
return xerrors.Errorf("unable to fetch failed workspace builds: %w", err)
107111
}
@@ -116,7 +120,7 @@ func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db dat
116120
if stats.FailedBuilds > 0 {
117121
failedBuilds, err = db.GetFailedWorkspaceBuildsByTemplateID(ctx, database.GetFailedWorkspaceBuildsByTemplateIDParams{
118122
TemplateID: stats.TemplateID,
119-
Since: dbtime.Time(clk.Now()).UTC(),
123+
Since: dbtime.Time(now).UTC(),
120124
})
121125
if err != nil {
122126
logger.Error(ctx, "unable to fetch failed workspace builds", slog.F("template_id", stats.TemplateID), slog.Error(err))
@@ -142,25 +146,25 @@ func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db dat
142146
return xerrors.Errorf("unable to get recent report generator log for user: %w", err)
143147
}
144148

145-
if !reportLog.LastGeneratedAt.IsZero() && reportLog.LastGeneratedAt.Add(failedWorkspaceBuildsReportFrequencyDays*24*time.Hour).After(clk.Now()) {
149+
if !reportLog.LastGeneratedAt.IsZero() && reportLog.LastGeneratedAt.Add(failedWorkspaceBuildsReportFrequencyDays*24*time.Hour).After(now) {
146150
// report generated recently, no need to send it now
147151
err = db.UpsertReportGeneratorLog(ctx, database.UpsertReportGeneratorLogParams{
148152
UserID: templateAdmin.ID,
149153
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
150-
LastGeneratedAt: dbtime.Time(clk.Now()).UTC(),
154+
LastGeneratedAt: dbtime.Time(now).UTC(),
151155
})
152156
if err != nil {
153157
logger.Error(ctx, "unable to update report generator logs", slog.F("template_id", stats.TemplateID), slog.F("user_id", templateAdmin.ID), slog.F("failed_builds", len(failedBuilds)), slog.Error(err))
154-
continue
155158
}
159+
continue
156160
}
157161

158162
if len(failedBuilds) == 0 {
159163
// no failed workspace builds, no need to send the report
160164
err = db.UpsertReportGeneratorLog(ctx, database.UpsertReportGeneratorLogParams{
161165
UserID: templateAdmin.ID,
162166
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
163-
LastGeneratedAt: dbtime.Time(clk.Now()).UTC(),
167+
LastGeneratedAt: dbtime.Time(now).UTC(),
164168
})
165169
if err != nil {
166170
logger.Error(ctx, "unable to update report generator logs", slog.F("template_id", stats.TemplateID), slog.F("user_id", templateAdmin.ID), slog.F("failed_builds", len(failedBuilds)), slog.Error(err))
@@ -188,7 +192,7 @@ func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db dat
188192
err = db.UpsertReportGeneratorLog(ctx, database.UpsertReportGeneratorLogParams{
189193
UserID: templateAdmin.ID,
190194
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
191-
LastGeneratedAt: dbtime.Time(clk.Now()).UTC(),
195+
LastGeneratedAt: dbtime.Time(now).UTC(),
192196
})
193197
if err != nil {
194198
logger.Error(ctx, "unable to update report generator logs", slog.F("template_id", stats.TemplateID), slog.F("user_id", templateAdmin.ID), slog.F("failed_builds", len(failedBuilds)), slog.Error(err))
@@ -199,7 +203,7 @@ func reportFailedWorkspaceBuilds(ctx context.Context, logger slog.Logger, db dat
199203

200204
err = db.DeleteOldReportGeneratorLogs(ctx, database.DeleteOldReportGeneratorLogsParams{
201205
NotificationTemplateID: notifications.TemplateWorkspaceBuildsFailedReport,
202-
Before: dbtime.Time(clk.Now().Add(-failedWorkspaceBuildsReportFrequencyDays*24*time.Hour - time.Hour)).UTC(),
206+
Before: dbtime.Time(now.Add(-failedWorkspaceBuildsReportFrequencyDays*24*time.Hour - time.Hour)).UTC(),
203207
})
204208
if err != nil {
205209
return xerrors.Errorf("unable to delete old report generator logs: %w", err)

‎coderd/notifications/reports/generator_internal_test.go

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ import (
77
"time"
88

99
"github.com/google/uuid"
10+
"github.com/prometheus/client_golang/prometheus"
1011
"github.com/stretchr/testify/require"
1112

1213
"cdr.dev/slog"
1314
"cdr.dev/slog/sloggers/slogtest"
1415
"github.com/coder/quartz"
1516

17+
"github.com/coder/coder/v2/coderd/coderdtest"
1618
"github.com/coder/coder/v2/coderd/database"
1719
"github.com/coder/coder/v2/coderd/database/dbauthz"
1820
"github.com/coder/coder/v2/coderd/database/dbgen"
@@ -39,11 +41,15 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) {
3941
// Setup
4042
ctx, logger, db, _, notifEnq, clk := setup(t)
4143

44+
// Database is ready, so we can clear notifications queue
45+
notifEnq.Clear()
46+
4247
// When
4348
err := reportFailedWorkspaceBuilds(ctx, logger, db, notifEnq, clk)
4449

4550
// Then
4651
require.NoError(t, err)
52+
require.Empty(t, notifEnq.Sent)
4753
})
4854

4955
t.Run("FailedBuilds_TemplateAdminOptIn_FirstRun_Report_SecondRunTooEarly_NoReport_ThirdRun_Report", func(t *testing.T) {
@@ -117,7 +123,7 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) {
117123
notifEnq.Clear()
118124

119125
// When
120-
err := reportFailedWorkspaceBuilds(ctx, logger, db, notifEnq, clk)
126+
err := reportFailedWorkspaceBuilds(ctx, logger, authedDB(db, logger), notifEnq, clk)
121127

122128
// Then
123129
require.NoError(t, err)
@@ -158,6 +164,17 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) {
158164
require.Equal(t, notifEnq.Sent[3].Data["total_builds"], int64(5))
159165
require.Equal(t, notifEnq.Sent[3].Data["report_frequency"], "week")
160166
// require.Contains(t, notifEnq.Sent[0].Data["template_versions"], "?")
167+
168+
// Given: 6 days later (less than report frequency)
169+
clk.Advance(6 * dayDuration).MustWait(context.Background())
170+
notifEnq.Clear()
171+
172+
// When
173+
err = reportFailedWorkspaceBuilds(ctx, logger, authedDB(db, logger), notifEnq, clk)
174+
require.NoError(t, err)
175+
176+
// Then
177+
require.Empty(t, notifEnq.Sent) // no notifications as it is too early.
161178
})
162179

163180
t.Run("NoFailedBuilds_TemplateAdminIn_NoReport", func(t *testing.T) {
@@ -176,16 +193,18 @@ func TestReportFailedWorkspaceBuilds(t *testing.T) {
176193
})
177194
}
178195

179-
func setup(t *testing.T) (context.Context, slog.Logger, database.Store, pubsub.Pubsub, *testutil.FakeNotificationsEnqueuer, quartz.Clock) {
196+
func setup(t *testing.T) (context.Context, slog.Logger, database.Store, pubsub.Pubsub, *testutil.FakeNotificationsEnqueuer, *quartz.Mock) {
180197
t.Helper()
181198

182199
// nolint:gocritic // reportFailedWorkspaceBuilds is called by system.
183200
ctx := dbauthz.AsSystemRestricted(context.Background())
184201
logger := slogtest.Make(t, &slogtest.Options{})
185202
db, ps := dbtestutil.NewDB(t)
186-
// does not work with works
187-
// db := dbauthz.New(rdb, rbac.NewAuthorizer(prometheus.NewRegistry()), logger, coderdtest.AccessControlStorePointer())
188203
notifyEnq := &testutil.FakeNotificationsEnqueuer{}
189204
clk := quartz.NewMock(t)
190205
return ctx, logger, db, ps, notifyEnq, clk
191206
}
207+
208+
func authedDB(db database.Store, logger slog.Logger) database.Store {
209+
return dbauthz.New(db, rbac.NewAuthorizer(prometheus.NewRegistry()), logger, coderdtest.AccessControlStorePointer())
210+
}

0 commit comments

Comments
 (0)