Merge pull request #1860 from ksylvan/kayvan/fix-for-setup-reset-required-value-now-does-not-show-validation-error

fix: allow resetting required settings without validation errors

cmd/generate_changelog/incoming/1860.txt

@@ -0,0 +1,7 @@
+### PR [#1860](https://github.com/danielmiessler/Fabric/pull/1860) by [ksylvan](https://github.com/ksylvan): fix: allow resetting required settings without validation errors
+
+- Fix: allow resetting required settings without validation errors
+- Update `Ask` to detect reset command and bypass validation
+- Refactor `OnAnswer` to support new `isReset` parameter logic
+- Invoke `ConfigureCustom` in `Setup` to avoid redundant re-validation
+- Add unit tests ensuring required fields can be reset

internal/plugins/plugin.go

@@ -92,7 +92,11 @@ func (o *PluginBase) Setup() (err error) {
 		return
 	}
 
-	err = o.Configure()
+	// After Setup, run ConfigureCustom if present, but skip re-validation
+	// since Ask() already validated user input (or allowed explicit reset)
+	if o.ConfigureCustom != nil {
+		err = o.ConfigureCustom()
+	}
 	return
 }
 
@@ -198,16 +202,21 @@ func (o *SetupQuestion) Ask(label string) (err error) {
 	var answer string
 	fmt.Scanln(&answer)
 	answer = strings.TrimRight(answer, "\n")
+	isReset := strings.ToLower(answer) == AnswerReset
 	if answer == "" {
 		answer = o.Value
-	} else if strings.ToLower(answer) == AnswerReset {
+	} else if isReset {
 		answer = ""
 	}
-	err = o.OnAnswer(answer)
+	err = o.OnAnswerWithReset(answer, isReset)
 	return
 }
 
 func (o *SetupQuestion) OnAnswer(answer string) (err error) {
+	return o.OnAnswerWithReset(answer, false)
+}
+
+func (o *SetupQuestion) OnAnswerWithReset(answer string, isReset bool) (err error) {
 	if o.Type == SettingTypeBool {
 		if answer == "" {
 			o.Value = ""
@@ -226,6 +235,11 @@ func (o *SetupQuestion) OnAnswer(answer string) (err error) {
 			return
 		}
 	}
+	// Skip validation when explicitly resetting a value - the user intentionally
+	// wants to clear the value even if it's required
+	if isReset {
+		return nil
+	}
 	err = o.IsValidErr()
 	return
 }

internal/plugins/plugin_test.go

@@ -116,6 +116,91 @@ func TestSetupQuestion_Ask(t *testing.T) {
 	assert.Equal(t, "user_value", setting.Value)
 }
 
+func TestSetupQuestion_Ask_Reset(t *testing.T) {
+	// Test that resetting a required field doesn't produce an error
+	setting := &Setting{
+		EnvVariable: "TEST_RESET_SETTING",
+		Value:       "existing_value",
+		Required:    true,
+	}
+	question := &SetupQuestion{
+		Setting:  setting,
+		Question: "Enter test setting:",
+	}
+	input := "reset\n"
+	fmtInput := captureInput(input)
+	defer fmtInput()
+	err := question.Ask("TestConfigurable")
+	// Should NOT return an error even though the field is required
+	assert.NoError(t, err)
+	// Value should be cleared
+	assert.Equal(t, "", setting.Value)
+}
+
+func TestSetupQuestion_OnAnswerWithReset(t *testing.T) {
+	tests := []struct {
+		name        string
+		setting     *Setting
+		answer      string
+		isReset     bool
+		expectError bool
+		expectValue string
+	}{
+		{
+			name: "reset required field should not error",
+			setting: &Setting{
+				EnvVariable: "TEST_SETTING",
+				Value:       "old_value",
+				Required:    true,
+			},
+			answer:      "",
+			isReset:     true,
+			expectError: false,
+			expectValue: "",
+		},
+		{
+			name: "empty answer on required field should error",
+			setting: &Setting{
+				EnvVariable: "TEST_SETTING",
+				Value:       "",
+				Required:    true,
+			},
+			answer:      "",
+			isReset:     false,
+			expectError: true,
+			expectValue: "",
+		},
+		{
+			name: "valid answer on required field should not error",
+			setting: &Setting{
+				EnvVariable: "TEST_SETTING",
+				Value:       "",
+				Required:    true,
+			},
+			answer:      "new_value",
+			isReset:     false,
+			expectError: false,
+			expectValue: "new_value",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			question := &SetupQuestion{
+				Setting:  tt.setting,
+				Question: "Test question",
+			}
+			err := question.OnAnswerWithReset(tt.answer, tt.isReset)
+			if tt.expectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, tt.expectValue, tt.setting.Value)
+		})
+	}
+}
+
 func TestSettings_IsConfigured(t *testing.T) {
 	settings := Settings{
 		{EnvVariable: "TEST_SETTING1", Value: "value1", Required: true},