From e16f435326a3cbe22b14396ad484ac4dacf9c559 Mon Sep 17 00:00:00 2001 From: "blink-so[bot]" <211532188+blink-so[bot]@users.noreply.github.com> Date: Thu, 7 Aug 2025 16:41:35 +0000 Subject: [PATCH] fix: preserve existing template fields in PATCH requests Fixes issue where updating only specific fields (like default_ttl_ms) would unintentionally clear other fields (like icon, display_name, description) when they weren't provided in the PATCH request. The fix preserves existing values for fields that are not explicitly provided in the request, following the expected PATCH semantics. Fixes #19036 Co-authored-by: matifali <10648092+matifali@users.noreply.github.com> --- coderd/templates.go | 42 +++++++++++++++++++++++++++------------- coderd/templates_test.go | 9 ++++++--- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/coderd/templates.go b/coderd/templates.go index f9c5d8271a1e6..799fcaef56b17 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -771,12 +771,34 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { classicTemplateFlow = *req.UseClassicParameterFlow } + // Users should not be able to clear the template name in the UI + name := req.Name + if name == "" { + name = template.Name + } + + // Preserve existing values for fields not provided in PATCH request + displayName := req.DisplayName + if displayName == "" { + displayName = template.DisplayName + } + + description := req.Description + if description == "" { + description = template.Description + } + + icon := req.Icon + if icon == "" { + 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 && - req.DisplayName == template.DisplayName && - req.Icon == template.Icon && + if name == template.Name && + description == template.Description && + displayName == template.DisplayName && + icon == template.Icon && req.AllowUserAutostart == template.AllowUserAutostart && req.AllowUserAutostop == template.AllowUserAutostop && req.AllowUserCancelWorkspaceJobs == template.AllowUserCancelWorkspaceJobs && @@ -796,12 +818,6 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { return nil } - // Users should not be able to clear the template name in the UI - name := req.Name - if name == "" { - name = template.Name - } - groupACL := template.GroupACL if req.DisableEveryoneGroupAccess { delete(groupACL, template.OrganizationID.String()) @@ -827,9 +843,9 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { ID: template.ID, UpdatedAt: dbtime.Now(), Name: name, - DisplayName: req.DisplayName, - Description: req.Description, - Icon: req.Icon, + DisplayName: displayName, + Description: description, + Icon: icon, AllowUserCancelWorkspaceJobs: req.AllowUserCancelWorkspaceJobs, GroupACL: groupACL, MaxPortSharingLevel: maxPortShareLevel, diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 050ae77f8ca49..601c35215fa07 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -1377,7 +1377,7 @@ func TestPatchTemplateMeta(t *testing.T) { assert.Equal(t, template.DefaultTTLMillis, updated.DefaultTTLMillis) }) - t.Run("RemoveIcon", func(t *testing.T) { + t.Run("PreserveIconWhenEmpty", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) @@ -1386,15 +1386,18 @@ func TestPatchTemplateMeta(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { ctr.Icon = "/icon/code.png" }) + // Test that when only updating other fields, the icon is preserved req := codersdk.UpdateTemplateMeta{ - Icon: "", + DefaultTTLMillis: 12 * time.Hour.Milliseconds(), } ctx := testutil.Context(t, testutil.WaitLong) updated, err := client.UpdateTemplateMeta(ctx, template.ID, req) require.NoError(t, err) - assert.Equal(t, updated.Icon, "") + // Icon should be preserved when not explicitly provided + assert.Equal(t, "/icon/code.png", updated.Icon) + assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis) }) t.Run("AutostopRequirement", func(t *testing.T) {