refactor: 'feedback' command trace and error handling (#68)

* feat: update 'feedback' command to include feedback about the Slack CLI

* fix: update survey output to feedback output

* refactor: rename PlatformSurvey to SlackPlatformFeedback

* feat: support skipping query params for feedback urls

* test: add feedback name to trace

* test: fix failing tests

* chore: tweak comment

* refactor: 'feedback' command always traces with slacktrace.FeedbackMessage

* refactor: add slackerrors for 'feedback' error handling

* refactor: clarify the ErrFeedbackNameRequired message

* Update internal/slackerror/errors.go

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

* fix: update error code messages to be consistent

---------

Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>

Author: mwbrooks

cmd/feedback/feedback.go

@@ -51,8 +51,6 @@ type SlackSurvey struct {
 	// Info prints additional information about the survey; displayed when the option is selected in `feedback`
 	// Info is optional
 	Info func(ctx context.Context, clients *shared.ClientFactory)
-	// Trace is a string consumed by tests to confirm that the Info text was displayed
-	Trace string
 	// Ask either prints text or prompts the user to complete the survey
 	// Potentially displayed after `run`/`deploy`/`doctor` (or other places where ShowSurveyMessages is called)
 	Ask func(ctx context.Context, clients *shared.ClientFactory) (bool, error)
@@ -98,7 +96,6 @@ var SurveyStore = map[string]SlackSurvey{
 				style.Secondary(style.Highlight("https://github.com/slackapi/slack-cli/issues")),
 			))
 		},
-		Trace: slacktrace.FeedbackMessage,
 		Ask: func(ctx context.Context, clients *shared.ClientFactory) (bool, error) {
 			clients.IO.PrintInfo(ctx, false, style.Sectionf(style.TextSection{
 				Emoji: "love_letter",
@@ -127,7 +124,6 @@ var SurveyStore = map[string]SlackSurvey{
 				style.Secondary("Or, share your experiences at "+style.Highlight("https://docs.slack.dev/developer-support")),
 			))
 		},
-		Trace: slacktrace.FeedbackMessage,
 		Ask: func(ctx context.Context, clients *shared.ClientFactory) (bool, error) {
 			clients.IO.PrintInfo(ctx, false, style.Sectionf(style.TextSection{
 				Emoji: "love_letter",
@@ -249,15 +245,12 @@ func runFeedbackCommand(ctx context.Context, clients *shared.ClientFactory, cmd
 	surveyNames, surveyPromptOptions := initSurveyOpts(ctx, clients, SurveyStore)
 
 	if _, ok := SurveyStore[surveyNameFlag]; !ok && surveyNameFlag != "" {
-		return slackerror.New("invalid_survey_name").
-			WithMessage("Invalid feedback name provided: %s", surveyNameFlag).
-			WithRemediation("View the feedback options with %s", style.Commandf("feedback --help", false))
+		return slackerror.New(slackerror.ErrFeedbackNameInvalid).
+			WithMessage("Invalid feedback name provided: %s", surveyNameFlag)
 	}
 
 	if surveyNameFlag == "" && noPromptFlag {
-		return slackerror.New("survey_name_required").
-			WithMessage("Please provide a feedback name or remove the --no-prompt flag").
-			WithRemediation("View feedback options with %s", style.Commandf("feedback --help", false))
+		return slackerror.New(slackerror.ErrFeedbackNameRequired)
 	}
 
 	clients.IO.PrintInfo(ctx, false, style.Sectionf(style.TextSection{
@@ -338,7 +331,8 @@ func executeSurvey(ctx context.Context, clients *shared.ClientFactory, s SlackSu
 	if s.Info != nil {
 		s.Info(ctx, clients)
 	}
-	clients.IO.PrintTrace(ctx, s.Trace, s.Name)
+
+	clients.IO.PrintTrace(ctx, slacktrace.FeedbackMessage, s.Name)
 
 	var err error
 	var ok bool

cmd/feedback/feedback_test.go

@@ -26,15 +26,14 @@ import (
 	"github.com/slackapi/slack-cli/internal/shared"
 	"github.com/slackapi/slack-cli/internal/slackcontext"
 	"github.com/slackapi/slack-cli/internal/slackerror"
+	"github.com/slackapi/slack-cli/internal/slacktrace"
 	"github.com/slackapi/slack-cli/internal/style"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
 
 func TestFeedbackCommand(t *testing.T) {
-
 	t.Run("when there is only one survey option", func(t *testing.T) {
-
 		surveys := map[string]SlackSurvey{
 			SlackPlatformFeedback: {
 				Name:              SlackPlatformFeedback,
@@ -72,12 +71,14 @@ func TestFeedbackCommand(t *testing.T) {
 		// Execute test
 		cmd := NewFeedbackCommand(clients)
 		err := runFeedbackCommand(ctx, clients, cmd)
+
+		// Assertions
 		assert.NoError(t, err)
 		clientsMock.Browser.AssertCalled(t, "OpenURL", "https://survey.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli")
+		clientsMock.IO.AssertCalled(t, "PrintTrace", mock.Anything, slacktrace.FeedbackMessage, []string{SlackPlatformFeedback})
 	})
 
 	t.Run("when there are multiple survey options", func(t *testing.T) {
-
 		surveys := map[string]SlackSurvey{
 			"A_test": {
 				Name:              "A_test",
@@ -139,12 +140,14 @@ func TestFeedbackCommand(t *testing.T) {
 		cmd := NewFeedbackCommand(clients)
 		err := runFeedbackCommand(ctx, clients, cmd)
 		assert.NoError(t, err)
+
+		// Assertions
 		clientsMock.Browser.AssertCalled(t, "OpenURL", "https://survey.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli")
+		clientsMock.IO.AssertCalled(t, "PrintTrace", mock.Anything, slacktrace.FeedbackMessage, []string{SlackPlatformFeedback})
 	})
 }
 
 func TestShowSurveyMessages(t *testing.T) {
-
 	t.Run("surveys asked or not asked based on the stored config", func(t *testing.T) {
 		surveys := map[string]SlackSurvey{
 			// Should be asked once; already asked
@@ -248,5 +251,4 @@ func TestShowSurveyMessages(t *testing.T) {
 		clientsMock.Browser.AssertCalled(t, "OpenURL", "https://B.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli")
 		clientsMock.Browser.AssertCalled(t, "OpenURL", "https://C.com?project_id=projectID&system_id=systemID&utm_medium=cli&utm_source=cli")
 	})
-
 }

internal/slackerror/errors.go

@@ -103,6 +103,8 @@ const (
 	ErrFailedToGetUser                               = "failed_to_get_user"
 	ErrFailedToSaveExtensionLogs                     = "failed_to_save_extension_logs"
 	ErrFailToGetTeamsForRestrictedUser               = "fail_to_get_teams_for_restricted_user"
+	ErrFeedbackNameInvalid                           = "feedback_name_invalid"
+	ErrFeedbackNameRequired                          = "feedback_name_required"
 	ErrFileRejected                                  = "file_rejected"
 	ErrForbiddenTeam                                 = "forbidden_team"
 	ErrFreeTeamNotAllowed                            = "free_team_not_allowed"
@@ -711,6 +713,21 @@ Otherwise start your app for local development with: %s`,
 		Message: "Failed to get teams for restricted user",
 	},
 
+	ErrFeedbackNameInvalid: {
+		Code:        ErrFeedbackNameInvalid,
+		Message:     "The name of the feedback is invalid",
+		Remediation: fmt.Sprintf("View the feedback options with %s", style.Commandf("feedback --help", false)),
+	},
+
+	ErrFeedbackNameRequired: {
+		Code:    ErrFeedbackNameRequired,
+		Message: "The name of the feedback is required",
+		Remediation: strings.Join([]string{
+			"Please provide a --name <string> flag or remove the --no-prompt flag",
+			fmt.Sprintf("View feedback options with %s", style.Commandf("feedback --help", false)),
+		}, "\n"),
+	},
+
 	ErrFileRejected: {
 		Code:    ErrFileRejected,
 		Message: "Not an acceptable S3 file",