fix: prevent change in defaults if user unsets in template edit (#10793)

Emyrk · web-flow · commit 7060069034ba · 2023-11-20T18:14:30.000-06:00
* fix: template edit not change defaults if user unset
diff --git a/cli/templateedit.go b/cli/templateedit.go
@@ -120,16 +120,24 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
 				autostopRequirementDaysOfWeek = []string{}
 			}
 
-			// Only pass explicitly set deprecated values since the empty string
-			// removes the deprecated message. By default if we pass a nil,
-			// there is no change to this field.
+			// Default values
+			if !userSetOption(inv, "description") {
+				description = template.Description
+			}
+
+			if !userSetOption(inv, "icon") {
+				icon = template.Icon
+			}
+
+			if !userSetOption(inv, "display-name") {
+				displayName = template.DisplayName
+			}
+
 			var deprecated *string
-			opt := inv.Command.Options.ByName(deprecatedFlagName)
-			if !(opt.ValueSource == "" || opt.ValueSource == clibase.ValueSourceDefault) {
+			if !userSetOption(inv, "deprecated") {
 				deprecated = &deprecationMessage
 			}
 
-			// NOTE: coderd will ignore empty fields.
 			req := codersdk.UpdateTemplateMeta{
 				Name:             name,
 				DisplayName:      displayName,
diff --git a/cli/templateedit_test.go b/cli/templateedit_test.go
@@ -228,6 +228,9 @@ func TestTemplateEdit(t *testing.T) {
 			"templates",
 			"edit",
 			template.Name,
+			"--description", "",
+			"--display-name", "",
+			"--icon", "",
 		}
 		inv, root := clitest.New(t, cmdArgs...)
 		clitest.SetupConfig(t, templateAdmin, root)
@@ -1047,4 +1050,41 @@ func TestTemplateEdit(t *testing.T) {
 		require.Error(t, err)
 		require.ErrorContains(t, err, "appears to be an AGPL deployment")
 	})
+	t.Run("DefaultValues", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		owner := coderdtest.CreateFirstUser(t, client)
+
+		version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, nil)
+		_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
+		template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
+			ctr.Name = "random"
+			ctr.Icon = "/icon/foobar.png"
+			ctr.DisplayName = "Foobar"
+			ctr.Description = "Some description"
+		})
+
+		// We need to change some field to get a db write.
+		cmdArgs := []string{
+			"templates",
+			"edit",
+			template.Name,
+			"--name", "something-new",
+		}
+		inv, root := clitest.New(t, cmdArgs...)
+		//nolint
+		clitest.SetupConfig(t, client, root)
+
+		ctx := testutil.Context(t, testutil.WaitLong)
+		err := inv.WithContext(ctx).Run()
+		require.NoError(t, err)
+
+		updated, err := client.Template(context.Background(), template.ID)
+		require.NoError(t, err)
+		assert.Equal(t, "something-new", updated.Name)
+		assert.Equal(t, template.Icon, updated.Icon)
+		assert.Equal(t, template.DisplayName, updated.DisplayName)
+		assert.Equal(t, template.Description, updated.Description)
+		assert.Equal(t, template.DeprecationMessage, updated.DeprecationMessage)
+	})
 }
diff --git a/cli/util.go b/cli/util.go
@@ -8,6 +8,7 @@ import (
 
 	"golang.org/x/xerrors"
 
+	"github.com/coder/coder/v2/cli/clibase"
 	"github.com/coder/coder/v2/coderd/schedule/cron"
 	"github.com/coder/coder/v2/coderd/util/tz"
 )
@@ -18,6 +19,19 @@ var (
 	errUnsupportedTimezone   = xerrors.New("The location you provided looks like a timezone. Check https://ipinfo.io for your location.")
 )
 
+// userSetOption returns true if the option was set by the user.
+// This is helpful if the zero value of a flag is meaningful, and you need
+// to distinguish between the user setting the flag to the zero value and
+// the user not setting the flag at all.
+func userSetOption(inv *clibase.Invocation, flagName string) bool {
+	for _, opt := range inv.Command.Options {
+		if opt.Name == flagName {
+			return !(opt.ValueSource == clibase.ValueSourceNone || opt.ValueSource == clibase.ValueSourceDefault)
+		}
+	}
+	return false
+}
+
 // durationDisplay formats a duration for easier display:
 //   - Durations of 24 hours or greater are displays as Xd
 //   - Durations less than 1 minute are displayed as <1m
