From 57717a23afa2917e543d833ceb04634a26c01fd6 Mon Sep 17 00:00:00 2001
From: Ben Schumacher <ben.schumacher@mattermost.com>
Date: Wed, 5 Feb 2020 10:46:58 +0100
Subject: [PATCH] Allow placeholder for types number and longtext (#13809)

---
 model/manifest.go      |   8 +++-
 model/manifest_test.go | 104 +++++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 32 deletions(-)

diff --git a/model/manifest.go b/model/manifest.go
index d6f7e09b34..7c09830a61 100644
--- a/model/manifest.go
+++ b/model/manifest.go
@@ -76,7 +76,7 @@ type PluginSetting struct {
 	// The help text to display alongside the "Regenerate" button for settings of the "generated" type.
 	RegenerateHelpText string `json:"regenerate_help_text,omitempty" yaml:"regenerate_help_text,omitempty"`
 
-	// The placeholder to display for "text", "generated" and "username" types when blank.
+	// The placeholder to display for "generated", "text", "longtext", "number" and "username" types when blank.
 	Placeholder string `json:"placeholder" yaml:"placeholder"`
 
 	// The default value of the setting.
@@ -386,7 +386,11 @@ func (s *PluginSetting) isValid() error {
 		return errors.New("should not set RegenerateHelpText for setting type that is not generated")
 	}
 
-	if s.Placeholder != "" && !(pluginSettingType == Text || pluginSettingType == Generated || pluginSettingType == Username) {
+	if s.Placeholder != "" && !(pluginSettingType == Generated ||
+		pluginSettingType == Text ||
+		pluginSettingType == LongText ||
+		pluginSettingType == Number ||
+		pluginSettingType == Username) {
 		return errors.New("should not set Placeholder for setting type not in text, generated or username")
 	}
 
diff --git a/model/manifest_test.go b/model/manifest_test.go
index e86179a01e..c4137ac5ee 100644
--- a/model/manifest_test.go
+++ b/model/manifest_test.go
@@ -105,46 +105,90 @@ func TestIsValidSettingsSchema(t *testing.T) {
 }
 
 func TestSettingIsValid(t *testing.T) {
-	testCases := []struct {
-		Title       string
-		setting     *PluginSetting
+	for name, test := range map[string]struct {
+		Setting     PluginSetting
 		ExpectError bool
 	}{
-		{"Invalid setting type", &PluginSetting{Type: "invalid"}, true},
-		{"RegenerateHelpText error", &PluginSetting{Type: "text", RegenerateHelpText: "some text"}, true},
-		{"Placeholder error", &PluginSetting{Type: "bool", Placeholder: "some text"}, true},
-		{"Nil Options", &PluginSetting{Type: "bool"}, false},
-		{"Options error", &PluginSetting{Type: "generated", Options: []*PluginOption{}}, true},
-		{"Options displayName error", &PluginSetting{Type: "radio", Options: []*PluginOption{
-			{
-				Value: "some value",
+		"Invalid setting type": {
+			PluginSetting{Type: "invalid"},
+			true,
+		},
+		"RegenerateHelpText error": {
+			PluginSetting{Type: "text", RegenerateHelpText: "some text"},
+			true,
+		},
+		"Placeholder error": {
+			PluginSetting{Type: "bool", Placeholder: "some text"},
+			true,
+		},
+		"Nil Options": {
+			PluginSetting{Type: "bool"},
+			false,
+		},
+		"Options error": {
+			PluginSetting{Type: "generated", Options: []*PluginOption{}},
+			true,
+		},
+		"Options displayName error": {
+			PluginSetting{
+				Type: "radio",
+				Options: []*PluginOption{{
+					Value: "some value",
+				}},
 			},
-		}}, true},
-		{"Options value error", &PluginSetting{Type: "radio", Options: []*PluginOption{
-			{
-				DisplayName: "some name",
+			true,
+		},
+		"Options value error": {
+			PluginSetting{
+				Type: "radio",
+				Options: []*PluginOption{{
+					DisplayName: "some name",
+				}},
 			},
-		}}, true},
-		{"Happy case", &PluginSetting{Type: "radio", Options: []*PluginOption{
-			{
-				DisplayName: "Name",
-				Value:       "value",
+			true,
+		},
+		"Happy case": {
+			PluginSetting{
+				Type: "radio",
+				Options: []*PluginOption{{
+					DisplayName: "Name",
+					Value:       "value",
+				}},
 			},
-		}}, false},
-		{
-			"Valid number setting",
-			&PluginSetting{
+			false,
+		},
+		"Valid number setting": {
+			PluginSetting{
 				Type:    "number",
 				Default: 10,
 			},
 			false,
 		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.Title, func(t *testing.T) {
-			err := tc.setting.isValid()
-			if tc.ExpectError {
+		"Placeholder is disallowed for bool settings": {
+			PluginSetting{
+				Type:        "bool",
+				Placeholder: "some Text",
+			},
+			true,
+		},
+		"Placeholder is allowed for text settings": {
+			PluginSetting{
+				Type:        "text",
+				Placeholder: "some Text",
+			},
+			false,
+		},
+		"Placeholder is allowed for long text settings": {
+			PluginSetting{
+				Type:        "longtext",
+				Placeholder: "some Text",
+			},
+			false,
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			err := test.Setting.isValid()
+			if test.ExpectError {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)