Skip to content

Commit aab2ccd

Browse files
rafrdzrafrdz
authored
fix!: support empty or default fields when updating templates (#19256)
Breaking change: Field types in `codersdk.UpdateTemplateMeta` for `Icon`, `Description`, and `DisplayName` moved to `*string` ## Summary In this pull request we're updating the `UpdateTemplateMeta` struct to allow `DisplayName`, `Description`, and `Icon` to be set as empty `""` or default to the value from the template if not provided in an update call. Fixes #19036 ### The bug The reported bug occurred when clients were attempting to update a metadata field in a template via an edit call. When the request was decoded into an `UpdateTemplateMeta` struct the default values for fields in the struct were used to update the template even if they weren't provided. This led to fields like `Icon` being set to `""` (the default value). ### Changes To allow for specific fields to be set to `""` these fields were updated to be `*string` as opposed to `string`. This allows for clients to set these fields as `""` in an update request or they will default to the template value if they are not provided in the update request (will be `nil`). Added tests to confirm empty and nil values and updated other tests that use these fields.
1 parent 064436a commit aab2ccd

File tree

6 files changed

+178
-74
lines changed

6 files changed

+178
-74
lines changed

‎cli/templateedit.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,9 @@ func (r *RootCmd) templateEdit() *serpent.Command {
169169

170170
req := codersdk.UpdateTemplateMeta{
171171
Name: name,
172-
DisplayName: displayName,
173-
Description: description,
174-
Icon: icon,
172+
DisplayName: &displayName,
173+
Description: &description,
174+
Icon: &icon,
175175
DefaultTTLMillis: defaultTTL.Milliseconds(),
176176
ActivityBumpMillis: activityBump.Milliseconds(),
177177
AutostopRequirement: &codersdk.TemplateAutostopRequirement{

‎coderd/templates.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -771,12 +771,16 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
771771
classicTemplateFlow = *req.UseClassicParameterFlow
772772
}
773773

774+
displayName := ptr.NilToDefault(req.DisplayName, template.DisplayName)
775+
description := ptr.NilToDefault(req.Description, template.Description)
776+
icon := ptr.NilToDefault(req.Icon, template.Icon)
777+
774778
var updated database.Template
775779
err = api.Database.InTx(func(tx database.Store) error {
776780
if req.Name == template.Name &&
777-
req.Description == template.Description &&
778-
req.DisplayName == template.DisplayName &&
779-
req.Icon == template.Icon &&
781+
description == template.Description &&
782+
displayName == template.DisplayName &&
783+
icon == template.Icon &&
780784
req.AllowUserAutostart == template.AllowUserAutostart &&
781785
req.AllowUserAutostop == template.AllowUserAutostop &&
782786
req.AllowUserCancelWorkspaceJobs == template.AllowUserCancelWorkspaceJobs &&
@@ -827,9 +831,9 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
827831
ID: template.ID,
828832
UpdatedAt: dbtime.Now(),
829833
Name: name,
830-
DisplayName: req.DisplayName,
831-
Description: req.Description,
832-
Icon: req.Icon,
834+
DisplayName: displayName,
835+
Description: description,
836+
Icon: icon,
833837
AllowUserCancelWorkspaceJobs: req.AllowUserCancelWorkspaceJobs,
834838
GroupACL: groupACL,
835839
MaxPortSharingLevel: maxPortShareLevel,

‎coderd/templates_test.go

Lines changed: 133 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -901,9 +901,9 @@ func TestPatchTemplateMeta(t *testing.T) {
901901

902902
req := codersdk.UpdateTemplateMeta{
903903
Name: "new-template-name",
904-
DisplayName: "Displayed Name 456",
905-
Description: "lorem ipsum dolor sit amet et cetera",
906-
Icon: "/icon/new-icon.png",
904+
DisplayName: ptr.Ref("Displayed Name 456"),
905+
Description: ptr.Ref("lorem ipsum dolor sit amet et cetera"),
906+
Icon: ptr.Ref("/icon/new-icon.png"),
907907
DefaultTTLMillis: 12 * time.Hour.Milliseconds(),
908908
ActivityBumpMillis: 3 * time.Hour.Milliseconds(),
909909
AllowUserCancelWorkspaceJobs: false,
@@ -918,9 +918,9 @@ func TestPatchTemplateMeta(t *testing.T) {
918918
require.NoError(t, err)
919919
assert.Greater(t, updated.UpdatedAt, template.UpdatedAt)
920920
assert.Equal(t, req.Name, updated.Name)
921-
assert.Equal(t, req.DisplayName, updated.DisplayName)
922-
assert.Equal(t, req.Description, updated.Description)
923-
assert.Equal(t, req.Icon, updated.Icon)
921+
assert.Equal(t, *req.DisplayName, updated.DisplayName)
922+
assert.Equal(t, *req.Description, updated.Description)
923+
assert.Equal(t, *req.Icon, updated.Icon)
924924
assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis)
925925
assert.Equal(t, req.ActivityBumpMillis, updated.ActivityBumpMillis)
926926
assert.False(t, req.AllowUserCancelWorkspaceJobs)
@@ -930,9 +930,9 @@ func TestPatchTemplateMeta(t *testing.T) {
930930
require.NoError(t, err)
931931
assert.Greater(t, updated.UpdatedAt, template.UpdatedAt)
932932
assert.Equal(t, req.Name, updated.Name)
933-
assert.Equal(t, req.DisplayName, updated.DisplayName)
934-
assert.Equal(t, req.Description, updated.Description)
935-
assert.Equal(t, req.Icon, updated.Icon)
933+
assert.Equal(t, *req.DisplayName, updated.DisplayName)
934+
assert.Equal(t, *req.Description, updated.Description)
935+
assert.Equal(t, *req.Icon, updated.Icon)
936936
assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis)
937937
assert.Equal(t, req.ActivityBumpMillis, updated.ActivityBumpMillis)
938938
assert.False(t, req.AllowUserCancelWorkspaceJobs)
@@ -1167,9 +1167,9 @@ func TestPatchTemplateMeta(t *testing.T) {
11671167

11681168
got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{
11691169
Name: template.Name,
1170-
DisplayName: template.DisplayName,
1171-
Description: template.Description,
1172-
Icon: template.Icon,
1170+
DisplayName: &template.DisplayName,
1171+
Description: &template.Description,
1172+
Icon: &template.Icon,
11731173
DefaultTTLMillis: 0,
11741174
AutostopRequirement: &template.AutostopRequirement,
11751175
AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs,
@@ -1202,9 +1202,9 @@ func TestPatchTemplateMeta(t *testing.T) {
12021202

12031203
got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{
12041204
Name: template.Name,
1205-
DisplayName: template.DisplayName,
1206-
Description: template.Description,
1207-
Icon: template.Icon,
1205+
DisplayName: &template.DisplayName,
1206+
Description: &template.Description,
1207+
Icon: &template.Icon,
12081208
DefaultTTLMillis: template.DefaultTTLMillis,
12091209
AutostopRequirement: &template.AutostopRequirement,
12101210
AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs,
@@ -1263,9 +1263,9 @@ func TestPatchTemplateMeta(t *testing.T) {
12631263
allowAutostop.Store(false)
12641264
got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{
12651265
Name: template.Name,
1266-
DisplayName: template.DisplayName,
1267-
Description: template.Description,
1268-
Icon: template.Icon,
1266+
DisplayName: &template.DisplayName,
1267+
Description: &template.Description,
1268+
Icon: &template.Icon,
12691269
DefaultTTLMillis: template.DefaultTTLMillis,
12701270
AutostopRequirement: &template.AutostopRequirement,
12711271
AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs,
@@ -1294,9 +1294,9 @@ func TestPatchTemplateMeta(t *testing.T) {
12941294

12951295
got, err := client.UpdateTemplateMeta(ctx, template.ID, codersdk.UpdateTemplateMeta{
12961296
Name: template.Name,
1297-
DisplayName: template.DisplayName,
1298-
Description: template.Description,
1299-
Icon: template.Icon,
1297+
DisplayName: &template.DisplayName,
1298+
Description: &template.Description,
1299+
Icon: &template.Icon,
13001300
// Increase the default TTL to avoid error "not modified".
13011301
DefaultTTLMillis: template.DefaultTTLMillis + 1,
13021302
AutostopRequirement: &template.AutostopRequirement,
@@ -1326,8 +1326,8 @@ func TestPatchTemplateMeta(t *testing.T) {
13261326

13271327
req := codersdk.UpdateTemplateMeta{
13281328
Name: template.Name,
1329-
Description: template.Description,
1330-
Icon: template.Icon,
1329+
Description: &template.Description,
1330+
Icon: &template.Icon,
13311331
DefaultTTLMillis: template.DefaultTTLMillis,
13321332
ActivityBumpMillis: template.ActivityBumpMillis,
13331333
AutostopRequirement: nil,
@@ -1387,7 +1387,7 @@ func TestPatchTemplateMeta(t *testing.T) {
13871387
ctr.Icon = "/icon/code.png"
13881388
})
13891389
req := codersdk.UpdateTemplateMeta{
1390-
Icon: "",
1390+
Icon: ptr.Ref(""),
13911391
}
13921392

13931393
ctx := testutil.Context(t, testutil.WaitLong)
@@ -1442,9 +1442,9 @@ func TestPatchTemplateMeta(t *testing.T) {
14421442
require.EqualValues(t, 1, template.AutostopRequirement.Weeks)
14431443
req := codersdk.UpdateTemplateMeta{
14441444
Name: template.Name,
1445-
DisplayName: template.DisplayName,
1446-
Description: template.Description,
1447-
Icon: template.Icon,
1445+
DisplayName: &template.DisplayName,
1446+
Description: &template.Description,
1447+
Icon: &template.Icon,
14481448
AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs,
14491449
DefaultTTLMillis: time.Hour.Milliseconds(),
14501450
AutostopRequirement: &codersdk.TemplateAutostopRequirement{
@@ -1519,9 +1519,9 @@ func TestPatchTemplateMeta(t *testing.T) {
15191519
require.EqualValues(t, 2, template.AutostopRequirement.Weeks)
15201520
req := codersdk.UpdateTemplateMeta{
15211521
Name: template.Name,
1522-
DisplayName: template.DisplayName,
1523-
Description: template.Description,
1524-
Icon: template.Icon,
1522+
DisplayName: &template.DisplayName,
1523+
Description: &template.Description,
1524+
Icon: &template.Icon,
15251525
AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs,
15261526
DefaultTTLMillis: time.Hour.Milliseconds(),
15271527
AutostopRequirement: &codersdk.TemplateAutostopRequirement{
@@ -1556,9 +1556,9 @@ func TestPatchTemplateMeta(t *testing.T) {
15561556
require.EqualValues(t, 1, template.AutostopRequirement.Weeks)
15571557
req := codersdk.UpdateTemplateMeta{
15581558
Name: template.Name,
1559-
DisplayName: template.DisplayName,
1560-
Description: template.Description,
1561-
Icon: template.Icon,
1559+
DisplayName: &template.DisplayName,
1560+
Description: &template.Description,
1561+
Icon: &template.Icon,
15621562
AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs,
15631563
DefaultTTLMillis: time.Hour.Milliseconds(),
15641564
AutostopRequirement: &codersdk.TemplateAutostopRequirement{
@@ -1618,6 +1618,106 @@ func TestPatchTemplateMeta(t *testing.T) {
16181618
require.NoError(t, err)
16191619
assert.False(t, updated.UseClassicParameterFlow, "expected false")
16201620
})
1621+
1622+
t.Run("SupportEmptyOrDefaultFields", func(t *testing.T) {
1623+
t.Parallel()
1624+
1625+
client := coderdtest.New(t, nil)
1626+
user := coderdtest.CreateFirstUser(t, client)
1627+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
1628+
1629+
displayName := "Test Display Name"
1630+
description := "test-description"
1631+
icon := "/icon/icon.png"
1632+
defaultTTLMillis := 10 * time.Hour.Milliseconds()
1633+
1634+
reference := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
1635+
ctr.DisplayName = displayName
1636+
ctr.Description = description
1637+
ctr.Icon = icon
1638+
ctr.DefaultTTLMillis = ptr.Ref(defaultTTLMillis)
1639+
})
1640+
require.Equal(t, displayName, reference.DisplayName)
1641+
require.Equal(t, description, reference.Description)
1642+
require.Equal(t, icon, reference.Icon)
1643+
1644+
restoreReq := codersdk.UpdateTemplateMeta{
1645+
DisplayName: &displayName,
1646+
Description: &description,
1647+
Icon: &icon,
1648+
DefaultTTLMillis: defaultTTLMillis,
1649+
}
1650+
1651+
type expected struct {
1652+
displayName string
1653+
description string
1654+
icon string
1655+
defaultTTLMillis int64
1656+
}
1657+
1658+
type testCase struct {
1659+
name string
1660+
req codersdk.UpdateTemplateMeta
1661+
expected expected
1662+
}
1663+
1664+
tests := []testCase{
1665+
{
1666+
name: "Only update default_ttl_ms",
1667+
req: codersdk.UpdateTemplateMeta{DefaultTTLMillis: 99 * time.Hour.Milliseconds()},
1668+
expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 99 * time.Hour.Milliseconds()},
1669+
},
1670+
{
1671+
name: "Clear display name",
1672+
req: codersdk.UpdateTemplateMeta{DisplayName: ptr.Ref("")},
1673+
expected: expected{displayName: "", description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0},
1674+
},
1675+
{
1676+
name: "Clear description",
1677+
req: codersdk.UpdateTemplateMeta{Description: ptr.Ref("")},
1678+
expected: expected{displayName: reference.DisplayName, description: "", icon: reference.Icon, defaultTTLMillis: 0},
1679+
},
1680+
{
1681+
name: "Clear icon",
1682+
req: codersdk.UpdateTemplateMeta{Icon: ptr.Ref("")},
1683+
expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: "", defaultTTLMillis: 0},
1684+
},
1685+
{
1686+
name: "Nil display name defaults to reference display name",
1687+
req: codersdk.UpdateTemplateMeta{DisplayName: nil},
1688+
expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0},
1689+
},
1690+
{
1691+
name: "Nil description defaults to reference description",
1692+
req: codersdk.UpdateTemplateMeta{Description: nil},
1693+
expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0},
1694+
},
1695+
{
1696+
name: "Nil icon defaults to reference icon",
1697+
req: codersdk.UpdateTemplateMeta{Icon: nil},
1698+
expected: expected{displayName: reference.DisplayName, description: reference.Description, icon: reference.Icon, defaultTTLMillis: 0},
1699+
},
1700+
}
1701+
1702+
for _, tc := range tests {
1703+
//nolint:tparallel,paralleltest
1704+
t.Run(tc.name, func(t *testing.T) {
1705+
defer func() {
1706+
ctx := testutil.Context(t, testutil.WaitLong)
1707+
// Restore reference after each test case
1708+
_, err := client.UpdateTemplateMeta(ctx, reference.ID, restoreReq)
1709+
require.NoError(t, err)
1710+
}()
1711+
ctx := testutil.Context(t, testutil.WaitLong)
1712+
updated, err := client.UpdateTemplateMeta(ctx, reference.ID, tc.req)
1713+
require.NoError(t, err)
1714+
assert.Equal(t, tc.expected.displayName, updated.DisplayName)
1715+
assert.Equal(t, tc.expected.description, updated.Description)
1716+
assert.Equal(t, tc.expected.icon, updated.Icon)
1717+
assert.Equal(t, tc.expected.defaultTTLMillis, updated.DefaultTTLMillis)
1718+
})
1719+
}
1720+
})
16211721
}
16221722

16231723
func TestDeleteTemplate(t *testing.T) {

‎codersdk/templates.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,11 @@ type ACLAvailable struct {
208208
}
209209

210210
type UpdateTemplateMeta struct {
211-
Name string `json:"name,omitempty" validate:"omitempty,template_name"`
212-
DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"`
213-
Description string `json:"description,omitempty"`
214-
Icon string `json:"icon,omitempty"`
215-
DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"`
211+
Name string `json:"name,omitempty" validate:"omitempty,template_name"`
212+
DisplayName *string `json:"display_name,omitempty" validate:"omitempty,template_display_name"`
213+
Description *string `json:"description,omitempty"`
214+
Icon *string `json:"icon,omitempty"`
215+
DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"`
216216
// ActivityBumpMillis allows optionally specifying the activity bump
217217
// duration for all workspaces created from this template. Defaults to 1h
218218
// but can be set to 0 to disable activity bumping.

‎enterprise/cli/templateedit_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,9 +219,9 @@ func TestTemplateEdit(t *testing.T) {
219219

220220
template, err := ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{
221221
Name: expectedName,
222-
DisplayName: expectedDisplayName,
223-
Description: expectedDescription,
224-
Icon: expectedIcon,
222+
DisplayName: &expectedDisplayName,
223+
Description: &expectedDescription,
224+
Icon: &expectedIcon,
225225
DefaultTTLMillis: expectedDefaultTTLMillis,
226226
AllowUserAutostop: expectedAllowAutostop,
227227
AllowUserAutostart: expectedAllowAutostart,
@@ -267,9 +267,9 @@ func TestTemplateEdit(t *testing.T) {
267267

268268
template, err = ownerClient.UpdateTemplateMeta(ctx, dbtemplate.ID, codersdk.UpdateTemplateMeta{
269269
Name: expectedName,
270-
DisplayName: expectedDisplayName,
271-
Description: expectedDescription,
272-
Icon: expectedIcon,
270+
DisplayName: &expectedDisplayName,
271+
Description: &expectedDescription,
272+
Icon: &expectedIcon,
273273
DefaultTTLMillis: expectedDefaultTTLMillis,
274274
AllowUserAutostop: expectedAllowAutostop,
275275
AllowUserAutostart: expectedAllowAutostart,

0 commit comments

Comments
 (0)