From 17753216c2708dc83b73d991e2767022ac372b87 Mon Sep 17 00:00:00 2001 From: Rafael Rodriguez Date: Fri, 8 Aug 2025 15:37:15 +0000 Subject: [PATCH 1/5] fix: Add support for empty/default template fields --- cli/templateedit.go | 4 +- coderd/templates.go | 11 ++- coderd/templates_test.go | 109 ++++++++++++++++++++++------ codersdk/templates.go | 10 +-- enterprise/cli/templateedit_test.go | 4 +- enterprise/coderd/templates_test.go | 28 +++---- 6 files changed, 116 insertions(+), 50 deletions(-) diff --git a/cli/templateedit.go b/cli/templateedit.go index b115350ab4437..1f3d03f4021b3 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -170,8 +170,8 @@ func (r *RootCmd) templateEdit() *serpent.Command { req := codersdk.UpdateTemplateMeta{ Name: name, DisplayName: displayName, - Description: description, - Icon: icon, + Description: &description, + Icon: &icon, DefaultTTLMillis: defaultTTL.Milliseconds(), ActivityBumpMillis: activityBump.Milliseconds(), AutostopRequirement: &codersdk.TemplateAutostopRequirement{ diff --git a/coderd/templates.go b/coderd/templates.go index 694bb90b86a4d..4cd25b413198c 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -770,12 +770,15 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { classicTemplateFlow = *req.UseClassicParameterFlow } + description := ptr.NilToDefault(req.Description, template.Description) + icon := ptr.NilToDefault(req.Icon, template.Icon) + var updated database.Template err = api.Database.InTx(func(tx database.Store) error { if req.Name == template.Name && - req.Description == template.Description && + description == template.Description && req.DisplayName == template.DisplayName && - req.Icon == template.Icon && + icon == template.Icon && req.AllowUserAutostart == template.AllowUserAutostart && req.AllowUserAutostop == template.AllowUserAutostop && req.AllowUserCancelWorkspaceJobs == template.AllowUserCancelWorkspaceJobs && @@ -827,8 +830,8 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { UpdatedAt: dbtime.Now(), Name: name, DisplayName: req.DisplayName, - Description: req.Description, - Icon: req.Icon, + Description: description, + Icon: icon, AllowUserCancelWorkspaceJobs: req.AllowUserCancelWorkspaceJobs, GroupACL: groupACL, MaxPortSharingLevel: maxPortShareLevel, diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 0858ce83325cc..ec8aa43ebe54a 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -862,8 +862,8 @@ func TestPatchTemplateMeta(t *testing.T) { req := codersdk.UpdateTemplateMeta{ Name: "new-template-name", DisplayName: "Displayed Name 456", - Description: "lorem ipsum dolor sit amet et cetera", - Icon: "/icon/new-icon.png", + Description: ptr.Ref("lorem ipsum dolor sit amet et cetera"), + Icon: ptr.Ref("/icon/new-icon.png"), DefaultTTLMillis: 12 * time.Hour.Milliseconds(), ActivityBumpMillis: 3 * time.Hour.Milliseconds(), AllowUserCancelWorkspaceJobs: false, @@ -879,8 +879,8 @@ func TestPatchTemplateMeta(t *testing.T) { assert.Greater(t, updated.UpdatedAt, template.UpdatedAt) assert.Equal(t, req.Name, updated.Name) assert.Equal(t, req.DisplayName, updated.DisplayName) - assert.Equal(t, req.Description, updated.Description) - assert.Equal(t, req.Icon, updated.Icon) + assert.Equal(t, *req.Description, updated.Description) + assert.Equal(t, *req.Icon, updated.Icon) assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis) assert.Equal(t, req.ActivityBumpMillis, updated.ActivityBumpMillis) assert.False(t, req.AllowUserCancelWorkspaceJobs) @@ -891,8 +891,8 @@ func TestPatchTemplateMeta(t *testing.T) { assert.Greater(t, updated.UpdatedAt, template.UpdatedAt) assert.Equal(t, req.Name, updated.Name) assert.Equal(t, req.DisplayName, updated.DisplayName) - assert.Equal(t, req.Description, updated.Description) - assert.Equal(t, req.Icon, updated.Icon) + assert.Equal(t, *req.Description, updated.Description) + assert.Equal(t, *req.Icon, updated.Icon) assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis) assert.Equal(t, req.ActivityBumpMillis, updated.ActivityBumpMillis) assert.False(t, req.AllowUserCancelWorkspaceJobs) @@ -1128,8 +1128,8 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, DefaultTTLMillis: 0, AutostopRequirement: &template.AutostopRequirement, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1163,8 +1163,8 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, DefaultTTLMillis: template.DefaultTTLMillis, AutostopRequirement: &template.AutostopRequirement, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1224,8 +1224,8 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, DefaultTTLMillis: template.DefaultTTLMillis, AutostopRequirement: &template.AutostopRequirement, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1255,8 +1255,8 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, // Increase the default TTL to avoid error "not modified". DefaultTTLMillis: template.DefaultTTLMillis + 1, AutostopRequirement: &template.AutostopRequirement, @@ -1286,8 +1286,8 @@ func TestPatchTemplateMeta(t *testing.T) { req := codersdk.UpdateTemplateMeta{ Name: template.Name, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, DefaultTTLMillis: template.DefaultTTLMillis, ActivityBumpMillis: template.ActivityBumpMillis, AutostopRequirement: nil, @@ -1347,7 +1347,7 @@ func TestPatchTemplateMeta(t *testing.T) { ctr.Icon = "/icon/code.png" }) req := codersdk.UpdateTemplateMeta{ - Icon: "", + Icon: ptr.Ref(""), } ctx := testutil.Context(t, testutil.WaitLong) @@ -1403,8 +1403,8 @@ func TestPatchTemplateMeta(t *testing.T) { req := codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, DefaultTTLMillis: time.Hour.Milliseconds(), AutostopRequirement: &codersdk.TemplateAutostopRequirement{ @@ -1480,8 +1480,8 @@ func TestPatchTemplateMeta(t *testing.T) { req := codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, DefaultTTLMillis: time.Hour.Milliseconds(), AutostopRequirement: &codersdk.TemplateAutostopRequirement{ @@ -1517,8 +1517,8 @@ func TestPatchTemplateMeta(t *testing.T) { req := codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, DefaultTTLMillis: time.Hour.Milliseconds(), AutostopRequirement: &codersdk.TemplateAutostopRequirement{ @@ -1578,6 +1578,69 @@ func TestPatchTemplateMeta(t *testing.T) { require.NoError(t, err) assert.False(t, updated.UseClassicParameterFlow, "expected false") }) + + t.Run("SupportEmptyOrDefault", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + + description := "test-description" + icon := "/icon/icon.png" + defaultTTLMillis := 10 * time.Hour.Milliseconds() + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.Description = description + ctr.Icon = icon + ctr.DefaultTTLMillis = ptr.Ref(defaultTTLMillis) + }) + + restoreReq := codersdk.UpdateTemplateMeta{ + Description: &description, + Icon: &icon, + DefaultTTLMillis: defaultTTLMillis, + } + + type expected struct { + description string + icon string + defaultTTLMillis int64 + } + + type testCase struct { + name string + req codersdk.UpdateTemplateMeta + expected expected + } + + tests := []testCase{ + {name: "Only update default_ttl_ms", req: codersdk.UpdateTemplateMeta{DefaultTTLMillis: 99 * time.Hour.Milliseconds()}, expected: expected{description: template.Description, icon: template.Icon, defaultTTLMillis: 99 * time.Hour.Milliseconds()}}, + {name: "Clear description", req: codersdk.UpdateTemplateMeta{Description: ptr.Ref("")}, expected: expected{description: "", icon: template.Icon, defaultTTLMillis: 0}}, + {name: "Clear icon", req: codersdk.UpdateTemplateMeta{Icon: ptr.Ref("")}, expected: expected{description: template.Description, icon: "", defaultTTLMillis: 0}}, + {name: "Nil description defaults to template description", req: codersdk.UpdateTemplateMeta{Description: nil}, expected: expected{description: template.Description, icon: template.Icon, defaultTTLMillis: 0}}, + {name: "Nil icon defaults to template icon", req: codersdk.UpdateTemplateMeta{Icon: nil}, expected: expected{description: template.Description, icon: template.Icon, defaultTTLMillis: 0}}, + } + + // It is unfortunate we need to sleep, but the test can fail if the + // updatedAt is too close together. + time.Sleep(time.Millisecond * 5) + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := testutil.Context(t, testutil.WaitLong) + updated, err := client.UpdateTemplateMeta(ctx, template.ID, tc.req) + require.NoError(t, err) + assert.Equal(t, tc.expected.description, updated.Description) + assert.Equal(t, tc.expected.icon, updated.Icon) + assert.Equal(t, tc.expected.defaultTTLMillis, updated.DefaultTTLMillis) + + // Restore template after each test case + _, err = client.UpdateTemplateMeta(ctx, template.ID, restoreReq) + require.NoError(t, err) + }) + } + }) } func TestDeleteTemplate(t *testing.T) { diff --git a/codersdk/templates.go b/codersdk/templates.go index 2e77d999003ed..b30e106e33dde 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -208,11 +208,11 @@ type ACLAvailable struct { } type UpdateTemplateMeta struct { - Name string `json:"name,omitempty" validate:"omitempty,template_name"` - DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` - Description string `json:"description,omitempty"` - Icon string `json:"icon,omitempty"` - DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"` + Name string `json:"name,omitempty" validate:"omitempty,template_name"` + DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` + Description *string `json:"description,omitempty"` + Icon *string `json:"icon,omitempty"` + DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"` // ActivityBumpMillis allows optionally specifying the activity bump // duration for all workspaces created from this template. Defaults to 1h // but can be set to 0 to disable activity bumping. diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go index fbff3e75dffcf..6244043884a24 100644 --- a/enterprise/cli/templateedit_test.go +++ b/enterprise/cli/templateedit_test.go @@ -178,8 +178,8 @@ func TestTemplateEdit(t *testing.T) { var ( expectedName = "template" expectedDisplayName = "template_display" - expectedDescription = "My description" - expectedIcon = "icon.pjg" + expectedDescription = ptr.Ref("My description") + expectedIcon = ptr.Ref("icon.pjg") expectedDefaultTTLMillis = time.Hour.Milliseconds() expectedAllowAutostart = false expectedAllowAutostop = false diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 6c7a20f85a642..de1c3b7adb2e2 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -262,8 +262,8 @@ func TestTemplates(t *testing.T) { updated, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AutostartRequirement: &codersdk.TemplateAutostartRequirement{ DaysOfWeek: []string{"monday", "saturday"}, }, @@ -279,8 +279,8 @@ func TestTemplates(t *testing.T) { updated, err = anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon + "something", + Description: &template.Description, + Icon: ptr.Ref(template.Icon + "something"), }) require.NoError(t, err) require.Equal(t, []string{"monday", "saturday"}, updated.AutostartRequirement.DaysOfWeek) @@ -316,8 +316,8 @@ func TestTemplates(t *testing.T) { _, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AutostartRequirement: &codersdk.TemplateAutostartRequirement{ DaysOfWeek: []string{"foobar", "saturday"}, }, @@ -352,8 +352,8 @@ func TestTemplates(t *testing.T) { updated, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, DefaultTTLMillis: time.Hour.Milliseconds(), AutostopRequirement: &codersdk.TemplateAutostopRequirement{ @@ -406,8 +406,8 @@ func TestTemplates(t *testing.T) { updated, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, TimeTilDormantMillis: inactivityTTL.Milliseconds(), FailureTTLMillis: failureTTL.Milliseconds(), @@ -475,8 +475,8 @@ func TestTemplates(t *testing.T) { _, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, TimeTilDormantMillis: c.TimeTilDormantMS, FailureTTLMillis: c.FailureTTLMS, @@ -1014,8 +1014,8 @@ func TestTemplateACL(t *testing.T) { _, err = client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, DisplayName: template.DisplayName, - Description: template.Description, - Icon: template.Icon, + Description: &template.Description, + Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, DisableEveryoneGroupAccess: true, }) From 0665980611c9b236a1ab18dd429f9c1d3ad6cbb4 Mon Sep 17 00:00:00 2001 From: Rafael Rodriguez Date: Fri, 8 Aug 2025 16:43:04 +0000 Subject: [PATCH 2/5] Fix pointer issues in tests --- enterprise/cli/templateedit_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go index 6244043884a24..74613627295b4 100644 --- a/enterprise/cli/templateedit_test.go +++ b/enterprise/cli/templateedit_test.go @@ -178,8 +178,8 @@ func TestTemplateEdit(t *testing.T) { var ( expectedName = "template" expectedDisplayName = "template_display" - expectedDescription = ptr.Ref("My description") - expectedIcon = ptr.Ref("icon.pjg") + expectedDescription = "My description" + expectedIcon = "icon.pjg" expectedDefaultTTLMillis = time.Hour.Milliseconds() expectedAllowAutostart = false expectedAllowAutostop = false @@ -220,8 +220,8 @@ func TestTemplateEdit(t *testing.T) { template, err := ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{ Name: expectedName, DisplayName: expectedDisplayName, - Description: expectedDescription, - Icon: expectedIcon, + Description: &expectedDescription, + Icon: &expectedIcon, DefaultTTLMillis: expectedDefaultTTLMillis, AllowUserAutostop: expectedAllowAutostop, AllowUserAutostart: expectedAllowAutostart, @@ -268,8 +268,8 @@ func TestTemplateEdit(t *testing.T) { template, err = ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{ Name: expectedName, DisplayName: expectedDisplayName, - Description: expectedDescription, - Icon: expectedIcon, + Description: &expectedDescription, + Icon: &expectedIcon, DefaultTTLMillis: expectedDefaultTTLMillis, AllowUserAutostop: expectedAllowAutostop, AllowUserAutostart: expectedAllowAutostart, From 7c1317efb050ef967286ce6b1c7302a1d37ae8de Mon Sep 17 00:00:00 2001 From: Rafael Rodriguez Date: Fri, 8 Aug 2025 19:04:18 +0000 Subject: [PATCH 3/5] Test updates based on PR comments --- coderd/templates_test.go | 53 ++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 4f688b135d097..03823d195475b 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -1619,7 +1619,7 @@ func TestPatchTemplateMeta(t *testing.T) { assert.False(t, updated.UseClassicParameterFlow, "expected false") }) - t.Run("SupportEmptyOrDefault", func(t *testing.T) { + t.Run("SupportEmptyOrDefaultFields", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) @@ -1630,11 +1630,13 @@ func TestPatchTemplateMeta(t *testing.T) { icon := "/icon/icon.png" defaultTTLMillis := 10 * time.Hour.Milliseconds() - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + reference := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { ctr.Description = description ctr.Icon = icon ctr.DefaultTTLMillis = ptr.Ref(defaultTTLMillis) }) + require.Equal(t, description, reference.Description) + require.Equal(t, icon, reference.Icon) restoreReq := codersdk.UpdateTemplateMeta{ Description: &description, @@ -1655,29 +1657,48 @@ func TestPatchTemplateMeta(t *testing.T) { } tests := []testCase{ - {name: "Only update default_ttl_ms", req: codersdk.UpdateTemplateMeta{DefaultTTLMillis: 99 * time.Hour.Milliseconds()}, expected: expected{description: template.Description, icon: template.Icon, defaultTTLMillis: 99 * time.Hour.Milliseconds()}}, - {name: "Clear description", req: codersdk.UpdateTemplateMeta{Description: ptr.Ref("")}, expected: expected{description: "", icon: template.Icon, defaultTTLMillis: 0}}, - {name: "Clear icon", req: codersdk.UpdateTemplateMeta{Icon: ptr.Ref("")}, expected: expected{description: template.Description, icon: "", defaultTTLMillis: 0}}, - {name: "Nil description defaults to template description", req: codersdk.UpdateTemplateMeta{Description: nil}, expected: expected{description: template.Description, icon: template.Icon, defaultTTLMillis: 0}}, - {name: "Nil icon defaults to template icon", req: codersdk.UpdateTemplateMeta{Icon: nil}, expected: expected{description: template.Description, icon: template.Icon, defaultTTLMillis: 0}}, + { + name: "Only update default_ttl_ms", + req: codersdk.UpdateTemplateMeta{DefaultTTLMillis: 99 * time.Hour.Milliseconds()}, + expected: expected{description: reference.Description, icon: reference.Icon, defaultTTLMillis: 99 * time.Hour.Milliseconds()}, + }, + { + name: "Clear description", + req: codersdk.UpdateTemplateMeta{Description: ptr.Ref("")}, + expected: expected{description: "", icon: reference.Icon, defaultTTLMillis: 0}, + }, + { + name: "Clear icon", + req: codersdk.UpdateTemplateMeta{Icon: ptr.Ref("")}, + expected: expected{description: reference.Description, icon: "", defaultTTLMillis: 0}, + }, + { + name: "Nil description defaults to reference description", + req: codersdk.UpdateTemplateMeta{Description: nil}, + expected: expected{description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, + }, + { + name: "Nil icon defaults to reference icon", + req: codersdk.UpdateTemplateMeta{Icon: nil}, + expected: expected{description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, + }, } - // It is unfortunate we need to sleep, but the test can fail if the - // updatedAt is too close together. - time.Sleep(time.Millisecond * 5) - for _, tc := range tests { + //nolint:tparallel,paralleltest t.Run(tc.name, func(t *testing.T) { + defer func() { + ctx := testutil.Context(t, testutil.WaitLong) + // Restore reference after each test case + _, err := client.UpdateTemplateMeta(ctx, reference.ID, restoreReq) + require.NoError(t, err) + }() ctx := testutil.Context(t, testutil.WaitLong) - updated, err := client.UpdateTemplateMeta(ctx, template.ID, tc.req) + updated, err := client.UpdateTemplateMeta(ctx, reference.ID, tc.req) require.NoError(t, err) assert.Equal(t, tc.expected.description, updated.Description) assert.Equal(t, tc.expected.icon, updated.Icon) assert.Equal(t, tc.expected.defaultTTLMillis, updated.DefaultTTLMillis) - - // Restore template after each test case - _, err = client.UpdateTemplateMeta(ctx, template.ID, restoreReq) - require.NoError(t, err) }) } }) From 38bdabf47de59b4d7cfe287219058014194b14b3 Mon Sep 17 00:00:00 2001 From: Rafael Rodriguez Date: Fri, 8 Aug 2025 20:11:51 +0000 Subject: [PATCH 4/5] fix: Add nil/default support to template displayName --- cli/templateedit.go | 2 +- coderd/templates.go | 5 ++-- coderd/templates_test.go | 42 ++++++++++++++++++++--------- codersdk/templates.go | 2 +- enterprise/cli/templateedit_test.go | 4 +-- enterprise/coderd/templates_test.go | 14 +++++----- 6 files changed, 43 insertions(+), 26 deletions(-) diff --git a/cli/templateedit.go b/cli/templateedit.go index 1f3d03f4021b3..fe0323449c9be 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -169,7 +169,7 @@ func (r *RootCmd) templateEdit() *serpent.Command { req := codersdk.UpdateTemplateMeta{ Name: name, - DisplayName: displayName, + DisplayName: &displayName, Description: &description, Icon: &icon, DefaultTTLMillis: defaultTTL.Milliseconds(), diff --git a/coderd/templates.go b/coderd/templates.go index 3daedf074c28d..16ab5b3fa37a5 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -771,6 +771,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { classicTemplateFlow = *req.UseClassicParameterFlow } + displayName := ptr.NilToDefault(req.DisplayName, template.DisplayName) description := ptr.NilToDefault(req.Description, template.Description) icon := ptr.NilToDefault(req.Icon, template.Icon) @@ -778,7 +779,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { err = api.Database.InTx(func(tx database.Store) error { if req.Name == template.Name && description == template.Description && - req.DisplayName == template.DisplayName && + displayName == template.DisplayName && icon == template.Icon && req.AllowUserAutostart == template.AllowUserAutostart && req.AllowUserAutostop == template.AllowUserAutostop && @@ -830,7 +831,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { ID: template.ID, UpdatedAt: dbtime.Now(), Name: name, - DisplayName: req.DisplayName, + DisplayName: displayName, Description: description, Icon: icon, AllowUserCancelWorkspaceJobs: req.AllowUserCancelWorkspaceJobs, diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 03823d195475b..01467cebca3fe 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -901,7 +901,7 @@ func TestPatchTemplateMeta(t *testing.T) { req := codersdk.UpdateTemplateMeta{ Name: "new-template-name", - DisplayName: "Displayed Name 456", + DisplayName: ptr.Ref("Displayed Name 456"), Description: ptr.Ref("lorem ipsum dolor sit amet et cetera"), Icon: ptr.Ref("/icon/new-icon.png"), DefaultTTLMillis: 12 * time.Hour.Milliseconds(), @@ -1167,7 +1167,7 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, DefaultTTLMillis: 0, @@ -1202,7 +1202,7 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, DefaultTTLMillis: template.DefaultTTLMillis, @@ -1263,7 +1263,7 @@ func TestPatchTemplateMeta(t *testing.T) { allowAutostop.Store(false) got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, DefaultTTLMillis: template.DefaultTTLMillis, @@ -1294,7 +1294,7 @@ func TestPatchTemplateMeta(t *testing.T) { got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, // Increase the default TTL to avoid error "not modified". @@ -1442,7 +1442,7 @@ func TestPatchTemplateMeta(t *testing.T) { require.EqualValues(t, 1, template.AutostopRequirement.Weeks) req := codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1519,7 +1519,7 @@ func TestPatchTemplateMeta(t *testing.T) { require.EqualValues(t, 2, template.AutostopRequirement.Weeks) req := codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1556,7 +1556,7 @@ func TestPatchTemplateMeta(t *testing.T) { require.EqualValues(t, 1, template.AutostopRequirement.Weeks) req := codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1626,25 +1626,30 @@ func TestPatchTemplateMeta(t *testing.T) { user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + displayName := "Test Display Name" description := "test-description" icon := "/icon/icon.png" defaultTTLMillis := 10 * time.Hour.Milliseconds() reference := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.DisplayName = displayName ctr.Description = description ctr.Icon = icon ctr.DefaultTTLMillis = ptr.Ref(defaultTTLMillis) }) + require.Equal(t, displayName, reference.DisplayName) require.Equal(t, description, reference.Description) require.Equal(t, icon, reference.Icon) restoreReq := codersdk.UpdateTemplateMeta{ + DisplayName: &displayName, Description: &description, Icon: &icon, DefaultTTLMillis: defaultTTLMillis, } type expected struct { + displayName string description string icon string defaultTTLMillis int64 @@ -1660,27 +1665,37 @@ func TestPatchTemplateMeta(t *testing.T) { { name: "Only update default_ttl_ms", req: codersdk.UpdateTemplateMeta{DefaultTTLMillis: 99 * time.Hour.Milliseconds()}, - expected: expected{description: reference.Description, icon: reference.Icon, defaultTTLMillis: 99 * time.Hour.Milliseconds()}, + expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 99 * time.Hour.Milliseconds()}, + }, + { + name: "Clear display name", + req: codersdk.UpdateTemplateMeta{DisplayName: ptr.Ref("")}, + expected: expected{displayName: "", description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, }, { name: "Clear description", req: codersdk.UpdateTemplateMeta{Description: ptr.Ref("")}, - expected: expected{description: "", icon: reference.Icon, defaultTTLMillis: 0}, + expected: expected{displayName: reference.DisplayName, description: "", icon: reference.Icon, defaultTTLMillis: 0}, }, { name: "Clear icon", req: codersdk.UpdateTemplateMeta{Icon: ptr.Ref("")}, - expected: expected{description: reference.Description, icon: "", defaultTTLMillis: 0}, + expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: "", defaultTTLMillis: 0}, + }, + { + name: "Nil display name defaults to reference display name", + req: codersdk.UpdateTemplateMeta{DisplayName: nil}, + expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, }, { name: "Nil description defaults to reference description", req: codersdk.UpdateTemplateMeta{Description: nil}, - expected: expected{description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, + expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, }, { name: "Nil icon defaults to reference icon", req: codersdk.UpdateTemplateMeta{Icon: nil}, - expected: expected{description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, + expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0}, }, } @@ -1696,6 +1711,7 @@ func TestPatchTemplateMeta(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) updated, err := client.UpdateTemplateMeta(ctx, reference.ID, tc.req) require.NoError(t, err) + assert.Equal(t, tc.expected.displayName, updated.DisplayName) assert.Equal(t, tc.expected.description, updated.Description) assert.Equal(t, tc.expected.icon, updated.Icon) assert.Equal(t, tc.expected.defaultTTLMillis, updated.DefaultTTLMillis) diff --git a/codersdk/templates.go b/codersdk/templates.go index b30e106e33dde..cc9314e44794d 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -209,7 +209,7 @@ type ACLAvailable struct { type UpdateTemplateMeta struct { Name string `json:"name,omitempty" validate:"omitempty,template_name"` - DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` + DisplayName *string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` Description *string `json:"description,omitempty"` Icon *string `json:"icon,omitempty"` DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"` diff --git a/enterprise/cli/templateedit_test.go b/enterprise/cli/templateedit_test.go index 74613627295b4..01d4784fd3c1e 100644 --- a/enterprise/cli/templateedit_test.go +++ b/enterprise/cli/templateedit_test.go @@ -219,7 +219,7 @@ func TestTemplateEdit(t *testing.T) { template, err := ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{ Name: expectedName, - DisplayName: expectedDisplayName, + DisplayName: &expectedDisplayName, Description: &expectedDescription, Icon: &expectedIcon, DefaultTTLMillis: expectedDefaultTTLMillis, @@ -267,7 +267,7 @@ func TestTemplateEdit(t *testing.T) { template, err = ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{ Name: expectedName, - DisplayName: expectedDisplayName, + DisplayName: &expectedDisplayName, Description: &expectedDescription, Icon: &expectedIcon, DefaultTTLMillis: expectedDefaultTTLMillis, diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index e7f7c598fc32a..e5eafa82f8d1c 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -259,7 +259,7 @@ func TestTemplates(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) updated, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AutostartRequirement: &codersdk.TemplateAutostartRequirement{ @@ -276,7 +276,7 @@ func TestTemplates(t *testing.T) { // Ensure a missing field is a noop updated, err = anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: ptr.Ref(template.Icon + "something"), }) @@ -313,7 +313,7 @@ func TestTemplates(t *testing.T) { ctx := testutil.Context(t, testutil.WaitLong) _, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AutostartRequirement: &codersdk.TemplateAutostartRequirement{ @@ -349,7 +349,7 @@ func TestTemplates(t *testing.T) { ctx := context.Background() updated, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -403,7 +403,7 @@ func TestTemplates(t *testing.T) { updated, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -472,7 +472,7 @@ func TestTemplates(t *testing.T) { t.Run(c.Name, func(t *testing.T) { _, err := anotherClient.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, @@ -1004,7 +1004,7 @@ func TestTemplateACL(t *testing.T) { require.Equal(t, 1, len(acl.Groups)) _, err = client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{ Name: template.Name, - DisplayName: template.DisplayName, + DisplayName: &template.DisplayName, Description: &template.Description, Icon: &template.Icon, AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, From aaab655060ef61442598257b69e65a4485b72a9c Mon Sep 17 00:00:00 2001 From: Rafael Rodriguez Date: Fri, 8 Aug 2025 20:24:17 +0000 Subject: [PATCH 5/5] Fix pointer issues in tests --- coderd/templates_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 01467cebca3fe..325de6a18c8e3 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -918,7 +918,7 @@ func TestPatchTemplateMeta(t *testing.T) { require.NoError(t, err) assert.Greater(t, updated.UpdatedAt, template.UpdatedAt) assert.Equal(t, req.Name, updated.Name) - assert.Equal(t, req.DisplayName, updated.DisplayName) + assert.Equal(t, *req.DisplayName, updated.DisplayName) assert.Equal(t, *req.Description, updated.Description) assert.Equal(t, *req.Icon, updated.Icon) assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis) @@ -930,7 +930,7 @@ func TestPatchTemplateMeta(t *testing.T) { require.NoError(t, err) assert.Greater(t, updated.UpdatedAt, template.UpdatedAt) assert.Equal(t, req.Name, updated.Name) - assert.Equal(t, req.DisplayName, updated.DisplayName) + assert.Equal(t, *req.DisplayName, updated.DisplayName) assert.Equal(t, *req.Description, updated.Description) assert.Equal(t, *req.Icon, updated.Icon) assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis)