From b0df567beec3c5c6c4a34694f465e2bc9ab9cf75 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 16 Aug 2024 17:24:40 +0000 Subject: [PATCH 1/4] fix(coderd): humanize duration on notifications --- coderd/autobuild/lifecycle_executor.go | 3 +- coderd/notifications/notifications_test.go | 4 +- coderd/util/time/duration.go | 44 +++++++++++++++++ coderd/util/time/duration_test.go | 57 ++++++++++++++++++++++ coderd/workspaces.go | 3 +- enterprise/coderd/schedule/template.go | 3 +- 6 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 coderd/util/time/duration.go create mode 100644 coderd/util/time/duration_test.go diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 8da233d6d610f..28b6d265a82f4 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -22,6 +22,7 @@ import ( "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" + duration "github.com/coder/coder/v2/coderd/util/time" "github.com/coder/coder/v2/coderd/wsbuilder" ) @@ -330,7 +331,7 @@ func (e *Executor) runOnce(t time.Time) Stats { map[string]string{ "name": ws.Name, "reason": "inactivity exceeded the dormancy threshold", - "timeTilDormant": time.Duration(tmpl.TimeTilDormant).String(), + "timeTilDormant": duration.Humanize(time.Duration(tmpl.TimeTilDormant)), }, "lifecycle_executor", ws.ID, diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index e21dce4246f20..a62bee417b787 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -691,7 +691,7 @@ func TestNotificationTemplatesCanRender(t *testing.T) { "reason": "breached the template's threshold for inactivity", "initiator": "autobuild", "dormancyHours": "24", - "timeTilDormant": "24h", + "timeTilDormant": "24 hours", }, }, }, @@ -716,7 +716,7 @@ func TestNotificationTemplatesCanRender(t *testing.T) { "name": "bobby-workspace", "reason": "template updated to new dormancy policy", "dormancyHours": "24", - "timeTilDormant": "24h", + "timeTilDormant": "24 hours", }, }, }, diff --git a/coderd/util/time/duration.go b/coderd/util/time/duration.go new file mode 100644 index 0000000000000..3577aa353df73 --- /dev/null +++ b/coderd/util/time/duration.go @@ -0,0 +1,44 @@ +package duration + +import ( + "fmt" + "time" +) + +type Unit struct { + value int + unit string +} + +func Humanize(d time.Duration) string { + units := []Unit{ + {int(d.Hours() / 24), "day"}, + {int(d.Hours()) % 24, "hour"}, + {int(d.Minutes()) % 60, "minute"}, + {int(d.Seconds()) % 60, "second"}, + } + nonZeroUnits := []Unit{} + for _, unit := range units { + if unit.value > 0 { + nonZeroUnits = append(nonZeroUnits, unit) + } + } + if len(nonZeroUnits) == 0 { + return "0 seconds" + } + var result string + for i, unit := range nonZeroUnits { + if i > 0 { + if i == len(nonZeroUnits)-1 { + result += " and " + } else { + result += ", " + } + } + result += fmt.Sprintf("%d %s", unit.value, unit.unit) + if unit.value > 1 { + result += "s" + } + } + return result +} diff --git a/coderd/util/time/duration_test.go b/coderd/util/time/duration_test.go new file mode 100644 index 0000000000000..12980a80bc827 --- /dev/null +++ b/coderd/util/time/duration_test.go @@ -0,0 +1,57 @@ +package duration_test + +import ( + "testing" + "time" + + duration "github.com/coder/coder/v2/coderd/util/time" +) + +func TestHumanize(t *testing.T) { + t.Parallel() + + testCases := []struct { + duration time.Duration + expected string + }{ + { + duration: time.Duration(0), + expected: "0 seconds", + }, + { + duration: time.Duration(1 * time.Second), + expected: "1 second", + }, + { + duration: time.Duration(45 * time.Second), + expected: "45 seconds", + }, + { + duration: time.Duration(30 * time.Minute), + expected: "30 minutes", + }, + { + duration: time.Duration(30*time.Minute + 10*time.Second), + expected: "30 minutes and 10 seconds", + }, + { + duration: time.Duration(2*time.Hour + 25*time.Minute + 10*time.Second), + expected: "2 hours, 25 minutes and 10 seconds", + }, + { + duration: time.Duration(2400 * time.Hour), + expected: "100 days", + }, + } + + for _, tc := range testCases { + t.Run(tc.expected, func(t *testing.T) { + t.Parallel() + + actual := duration.Humanize(tc.duration) + if actual != tc.expected { + t.Errorf("expected: %s, got: %s", tc.expected, actual) + } + }) + } +} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 901e3723964bd..bea024b4d6d0e 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -32,6 +32,7 @@ import ( "github.com/coder/coder/v2/coderd/searchquery" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/util/ptr" + duration "github.com/coder/coder/v2/coderd/util/time" "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" @@ -1062,7 +1063,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { map[string]string{ "name": workspace.Name, "reason": "a " + initiator.Username + " request", - "timeTilDormant": time.Duration(tmpl.TimeTilDormant).String(), + "timeTilDormant": duration.Humanize(time.Duration(tmpl.TimeTilDormant)), }, "api", workspace.ID, diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index c38b8f509b5c3..b5e511483aa6e 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -19,6 +19,7 @@ import ( "github.com/coder/coder/v2/coderd/notifications" agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" + duration "github.com/coder/coder/v2/coderd/util/time" "github.com/coder/coder/v2/codersdk" ) @@ -212,7 +213,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S map[string]string{ "name": ws.Name, "reason": "an update to the template's dormancy", - "timeTilDormant": opts.TimeTilDormantAutoDelete.String(), + "timeTilDormant": duration.Humanize(opts.TimeTilDormantAutoDelete), }, "scheduletemplate", // Associate this notification with all the related entities. From 193c57817f43bdbffb85310d9dacaceefc9cbdfb Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 19 Aug 2024 14:44:46 +0000 Subject: [PATCH 2/4] Use go-humanize --- coderd/util/time/duration.go | 40 ++++-------------------------------- 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/coderd/util/time/duration.go b/coderd/util/time/duration.go index 3577aa353df73..6036dbfe84e5f 100644 --- a/coderd/util/time/duration.go +++ b/coderd/util/time/duration.go @@ -1,44 +1,12 @@ package duration import ( - "fmt" "time" -) -type Unit struct { - value int - unit string -} + "github.com/dustin/go-humanize" +) func Humanize(d time.Duration) string { - units := []Unit{ - {int(d.Hours() / 24), "day"}, - {int(d.Hours()) % 24, "hour"}, - {int(d.Minutes()) % 60, "minute"}, - {int(d.Seconds()) % 60, "second"}, - } - nonZeroUnits := []Unit{} - for _, unit := range units { - if unit.value > 0 { - nonZeroUnits = append(nonZeroUnits, unit) - } - } - if len(nonZeroUnits) == 0 { - return "0 seconds" - } - var result string - for i, unit := range nonZeroUnits { - if i > 0 { - if i == len(nonZeroUnits)-1 { - result += " and " - } else { - result += ", " - } - } - result += fmt.Sprintf("%d %s", unit.value, unit.unit) - if unit.value > 1 { - result += "s" - } - } - return result + endTime := time.Now().Add(d) + return humanize.Time(endTime) } From ad1209673da064a1cb2fb77190819702a4f5a210 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 19 Aug 2024 16:27:27 +0000 Subject: [PATCH 3/4] Remove bad abstraction --- coderd/autobuild/lifecycle_executor.go | 5 ++- coderd/util/time/duration.go | 12 ------ coderd/util/time/duration_test.go | 57 -------------------------- coderd/workspaces.go | 5 ++- enterprise/coderd/schedule/template.go | 5 ++- 5 files changed, 9 insertions(+), 75 deletions(-) delete mode 100644 coderd/util/time/duration.go delete mode 100644 coderd/util/time/duration_test.go diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 28b6d265a82f4..32d6e6d4b6e17 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -8,6 +8,7 @@ import ( "sync/atomic" "time" + "github.com/dustin/go-humanize" "github.com/google/uuid" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" @@ -22,7 +23,6 @@ import ( "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" - duration "github.com/coder/coder/v2/coderd/util/time" "github.com/coder/coder/v2/coderd/wsbuilder" ) @@ -324,6 +324,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } } if shouldNotifyDormancy { + dormantTime := time.Now().Add(time.Duration(tmpl.TimeTilDormant)) _, err = e.notificationsEnqueuer.Enqueue( e.ctx, ws.OwnerID, @@ -331,7 +332,7 @@ func (e *Executor) runOnce(t time.Time) Stats { map[string]string{ "name": ws.Name, "reason": "inactivity exceeded the dormancy threshold", - "timeTilDormant": duration.Humanize(time.Duration(tmpl.TimeTilDormant)), + "timeTilDormant": humanize.Time(dormantTime), }, "lifecycle_executor", ws.ID, diff --git a/coderd/util/time/duration.go b/coderd/util/time/duration.go deleted file mode 100644 index 6036dbfe84e5f..0000000000000 --- a/coderd/util/time/duration.go +++ /dev/null @@ -1,12 +0,0 @@ -package duration - -import ( - "time" - - "github.com/dustin/go-humanize" -) - -func Humanize(d time.Duration) string { - endTime := time.Now().Add(d) - return humanize.Time(endTime) -} diff --git a/coderd/util/time/duration_test.go b/coderd/util/time/duration_test.go deleted file mode 100644 index 12980a80bc827..0000000000000 --- a/coderd/util/time/duration_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package duration_test - -import ( - "testing" - "time" - - duration "github.com/coder/coder/v2/coderd/util/time" -) - -func TestHumanize(t *testing.T) { - t.Parallel() - - testCases := []struct { - duration time.Duration - expected string - }{ - { - duration: time.Duration(0), - expected: "0 seconds", - }, - { - duration: time.Duration(1 * time.Second), - expected: "1 second", - }, - { - duration: time.Duration(45 * time.Second), - expected: "45 seconds", - }, - { - duration: time.Duration(30 * time.Minute), - expected: "30 minutes", - }, - { - duration: time.Duration(30*time.Minute + 10*time.Second), - expected: "30 minutes and 10 seconds", - }, - { - duration: time.Duration(2*time.Hour + 25*time.Minute + 10*time.Second), - expected: "2 hours, 25 minutes and 10 seconds", - }, - { - duration: time.Duration(2400 * time.Hour), - expected: "100 days", - }, - } - - for _, tc := range testCases { - t.Run(tc.expected, func(t *testing.T) { - t.Parallel() - - actual := duration.Humanize(tc.duration) - if actual != tc.expected { - t.Errorf("expected: %s, got: %s", tc.expected, actual) - } - }) - } -} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index bea024b4d6d0e..0307a325b24e7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -11,6 +11,7 @@ import ( "strconv" "time" + "github.com/dustin/go-humanize" "github.com/go-chi/chi/v5" "github.com/google/uuid" "golang.org/x/xerrors" @@ -32,7 +33,6 @@ import ( "github.com/coder/coder/v2/coderd/searchquery" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/util/ptr" - duration "github.com/coder/coder/v2/coderd/util/time" "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" @@ -1056,6 +1056,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { } if initiatorErr == nil && tmplErr == nil { + dormantTime := time.Now().Add(time.Duration(tmpl.TimeTilDormant)) _, err = api.NotificationsEnqueuer.Enqueue( ctx, workspace.OwnerID, @@ -1063,7 +1064,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { map[string]string{ "name": workspace.Name, "reason": "a " + initiator.Username + " request", - "timeTilDormant": duration.Humanize(time.Duration(tmpl.TimeTilDormant)), + "timeTilDormant": humanize.Time(dormantTime), }, "api", workspace.ID, diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index b5e511483aa6e..52789e752ba8e 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -8,6 +8,7 @@ import ( "cdr.dev/slog" + "github.com/dustin/go-humanize" "github.com/google/uuid" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -19,7 +20,6 @@ import ( "github.com/coder/coder/v2/coderd/notifications" agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" - duration "github.com/coder/coder/v2/coderd/util/time" "github.com/coder/coder/v2/codersdk" ) @@ -206,6 +206,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S } for _, ws := range markedForDeletion { + dormantTime := time.Now().Add(opts.TimeTilDormantAutoDelete) _, err = s.enqueuer.Enqueue( ctx, ws.OwnerID, @@ -213,7 +214,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S map[string]string{ "name": ws.Name, "reason": "an update to the template's dormancy", - "timeTilDormant": duration.Humanize(opts.TimeTilDormantAutoDelete), + "timeTilDormant": humanize.Time(dormantTime), }, "scheduletemplate", // Associate this notification with all the related entities. From 8d3acc93048b431ada8621b7a0aaeacf5f2f0129 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 19 Aug 2024 17:14:08 +0000 Subject: [PATCH 4/4] Use dbtime --- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/notifications/notifications_test.go | 13 +++++++++++++ coderd/workspaces.go | 2 +- enterprise/coderd/schedule/template.go | 2 +- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 32d6e6d4b6e17..5bd8efe2b9fcf 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -324,7 +324,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } } if shouldNotifyDormancy { - dormantTime := time.Now().Add(time.Duration(tmpl.TimeTilDormant)) + dormantTime := dbtime.Now().Add(time.Duration(tmpl.TimeTilDormant)) _, err = e.notificationsEnqueuer.Enqueue( e.ctx, ws.OwnerID, diff --git a/coderd/notifications/notifications_test.go b/coderd/notifications/notifications_test.go index a62bee417b787..95b3a84026fad 100644 --- a/coderd/notifications/notifications_test.go +++ b/coderd/notifications/notifications_test.go @@ -720,6 +720,19 @@ func TestNotificationTemplatesCanRender(t *testing.T) { }, }, }, + { + name: "TemplateWorkspaceMarkedForDeletionInOneWeek", + id: notifications.TemplateWorkspaceMarkedForDeletion, + payload: types.MessagePayload{ + UserName: "bobby", + Labels: map[string]string{ + "name": "bobby-workspace", + "reason": "template updated to new dormancy policy", + "dormancyHours": "168", // 168 hours = 7 days = 1 week + "timeTilDormant": "1 week", + }, + }, + }, { name: "TemplateUserAccountCreated", id: notifications.TemplateUserAccountCreated, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 0307a325b24e7..bde8f1c917d39 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1056,7 +1056,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { } if initiatorErr == nil && tmplErr == nil { - dormantTime := time.Now().Add(time.Duration(tmpl.TimeTilDormant)) + dormantTime := dbtime.Now().Add(time.Duration(tmpl.TimeTilDormant)) _, err = api.NotificationsEnqueuer.Enqueue( ctx, workspace.OwnerID, diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 52789e752ba8e..6b148e8ef4708 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -206,7 +206,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S } for _, ws := range markedForDeletion { - dormantTime := time.Now().Add(opts.TimeTilDormantAutoDelete) + dormantTime := dbtime.Now().Add(opts.TimeTilDormantAutoDelete) _, err = s.enqueuer.Enqueue( ctx, ws.OwnerID,