Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitaab2ccd

Browse files
author
Rafael Rodriguez
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`## SummaryIn this pull request we're updating the `UpdateTemplateMeta` struct toallow `DisplayName`, `Description`, and `Icon` to be set as empty `""`or default to the value from the template if not provided in an updatecall.Fixes#19036### The bugThe reported bug occurred when clients were attempting to update ametadata field in a template via an edit call. When the request wasdecoded into an `UpdateTemplateMeta` struct the default values forfields in the struct were used to update the template even if theyweren't provided. This led to fields like `Icon` being set to `""` (thedefault value).### ChangesTo allow for specific fields to be set to `""` these fields were updatedto be `*string` as opposed to `string`. This allows for clients to setthese fields as `""` in an update request or they will default to thetemplate 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 thatuse these fields.
1 parent064436a commitaab2ccd

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
varupdated database.Template
775779
err=api.Database.InTx(func(tx database.Store)error {
776780
ifreq.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+
typeexpectedstruct {
1652+
displayNamestring
1653+
descriptionstring
1654+
iconstring
1655+
defaultTTLMillisint64
1656+
}
1657+
1658+
typetestCasestruct {
1659+
namestring
1660+
req codersdk.UpdateTemplateMeta
1661+
expectedexpected
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:=rangetests {
1703+
//nolint:tparallel,paralleltest
1704+
t.Run(tc.name,func(t*testing.T) {
1705+
deferfunc() {
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
funcTestDeleteTemplate(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
typeUpdateTemplateMetastruct {
211-
Namestring`json:"name,omitempty" validate:"omitempty,template_name"`
212-
DisplayNamestring`json:"display_name,omitempty" validate:"omitempty,template_display_name"`
213-
Descriptionstring`json:"description,omitempty"`
214-
Iconstring`json:"icon,omitempty"`
215-
DefaultTTLMillisint64`json:"default_ttl_ms,omitempty"`
211+
Namestring`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+
DefaultTTLMillisint64`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)
[8]ページ先頭
©2009-2025 Movatter.jp