Merge pull request #503 from typeid/limited_support_ref

Minor UX refactor to cluster support post

cmd/cluster/support/post.go

@@ -45,9 +45,13 @@ func newCmdpost() *cobra.Command {
 		Long: `Sends limited support reason to a given cluster, along with an internal service log detailing why the cluster was placed into limited support.
 The caller will be prompted to continue before sending the limited support reason.`,
 		Example: `# Post a limited support reason for a cluster misconfiguration
-osdctl cluster support post 1a2B3c4DefghIjkLMNOpQrSTUV5 --misconfiguration cluster --problem="the cluster has a second failing ingress controller, which is not supported and can cause issues with SLA" \
---resolution="remove the additional ingress controller 'my-custom-ingresscontroller'. 'oc get ingresscontroller -n openshift-ingress-operator' should yield only 'default'" \
---evidence="See OHSS-1234"`,
+osdctl cluster support post 1a2B3c4DefghIjkLMNOpQrSTUV5 --misconfiguration cluster --problem="The cluster has a second failing ingress controller, which is not supported and can cause issues with SLA." \
+--resolution="Remove the additional ingress controller 'my-custom-ingresscontroller'. 'oc get ingresscontroller -n openshift-ingress-operator' should yield only 'default'" \
+--evidence="See OHSS-1234"
+
+Will result in the following limited-support text sent to the customer:
+The cluster has a second failing ingress controller, which is not supported and can cause issues with SLA. Remove the additional ingress controller 'my-custom-ingresscontroller'. 'oc get ingresscontroller -n openshift-ingress-operator' should yield only 'default'.
+`,
 		Args:              cobra.ExactArgs(1),
 		DisableAutoGenTag: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -72,21 +76,15 @@ osdctl cluster support post 1a2B3c4DefghIjkLMNOpQrSTUV5 --misconfiguration clust
 	return postCmd
 }
 
-func (p *Post) setup() error {
-	switch p.Problem[len(p.Problem)-1:] {
-	case ".", "?", "!":
-		return errors.New("--problem should not end in punctuation")
-	}
-	switch p.Resolution[len(p.Resolution)-1:] {
-	case ".", "?", "!":
-		return errors.New("--resolution should not end in punctuation")
+func validateResolutionString(res string) error {
+	if res[len(res)-1:] == "." {
+		return errors.New("resolution string should not end with a `.` as it is already added in the email template")
 	}
-
 	return nil
 }
 
 func (p *Post) Run(clusterID string) error {
-	if err := p.setup(); err != nil {
+	if err := validateResolutionString(p.Resolution); err != nil {
 		return err
 	}
 
@@ -159,7 +157,7 @@ func (p *Post) Run(clusterID string) error {
 
 func (p *Post) buildLimitedSupport() (*cmv1.LimitedSupportReason, error) {
 	limitedSupportBuilder := cmv1.NewLimitedSupportReason().
-		Details(fmt.Sprintf("%v. %v", p.Problem, p.Resolution)).
+		Details(fmt.Sprintf("%s %s", p.Problem, p.Resolution)).
 		DetectionType(cmv1.DetectionTypeManual)
 	switch p.Misconfiguration {
 	case cloud:

cmd/cluster/support/post_test.go

@@ -2,45 +2,25 @@ package support
 
 import (
 	"fmt"
-	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 	"testing"
+
+	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 )
 
-func Test_setup(t *testing.T) {
+func TestValidateResolutionString(t *testing.T) {
 	tests := []struct {
-		name      string
-		post      *Post
-		expectErr bool
+		input         string
+		errorExpected bool
 	}{
-		{
-			name: "Error - Ends in period",
-			post: &Post{
-				Problem:    "A problem sentence.",
-				Resolution: "A resolution sentence.",
-			},
-			expectErr: true,
-		},
-		{
-			name: "No error",
-			post: &Post{
-				Problem:    "A problem sentence",
-				Resolution: "A resolution sentence",
-			},
-			expectErr: false,
-		},
+		{"resolution.", true},
+		{"no-dot-at-end", false},
 	}
 
 	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			err := test.post.setup()
-			if err != nil {
-				if !test.expectErr {
-					t.Errorf("expected no err, got %v", err)
-				}
-			} else {
-				if test.expectErr {
-					t.Error("expected err, got nil")
-				}
+		t.Run(test.input, func(t *testing.T) {
+			err := validateResolutionString(test.input)
+			if (err == nil) == test.errorExpected {
+				t.Errorf("For input '%s', expected an error: %t, but got: %v", test.input, test.errorExpected, err)
 			}
 		})
 	}
@@ -84,8 +64,8 @@ func Test_buildLimitedSupport(t *testing.T) {
 			if detectionType := got.DetectionType(); detectionType != cmv1.DetectionTypeManual {
 				t.Errorf("buildLimitedSupport() got detectionType = %v, want %v", detectionType, cmv1.DetectionTypeManual)
 			}
-			if details := got.Details(); details != fmt.Sprintf("%v. %v", tt.post.Problem, tt.post.Resolution) {
-				t.Errorf("buildLimitedSupport() got details = %v, want %v", details, fmt.Sprintf("%v. %v", tt.post.Problem, tt.post.Resolution))
+			if details := got.Details(); details != fmt.Sprintf("%s %s", tt.post.Problem, tt.post.Resolution) {
+				t.Errorf("buildLimitedSupport() got details = %s, want %s", details, fmt.Sprintf("%s %s", tt.post.Problem, tt.post.Resolution))
 			}
 		})
 	}