Merge pull request #2768 from AlaricWhitney/feat--breaking-out-voice-…

…calls-into-a-testable-function feat: breaking out voice calls into a testable function
target · May 1, 2023 · 19dace4 · 19dace4
2 parents 7cae5eb + 31124a9
commit 19dace4
Show file tree

Hide file tree

Showing 4 changed files with 379 additions and 40 deletions.
diff --git a/notification/twilio/client.go b/notification/twilio/client.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/target/goalert/config"
+	"github.com/target/goalert/notification"
 	"github.com/target/goalert/util/log"
 )
 
@@ -191,6 +192,51 @@ func (voice *VoiceOptions) CallbackURL(cfg config.Config) (string, error) {
 	return cfg.CallbackURL("/api/v2/twilio/call?type="+url.QueryEscape(string(voice.CallType)), voice.CallbackParams, voice.Params), nil
 }
 
+// setMsgBody will encode and set/update the message body parameter.
+func (voice *VoiceOptions) setMsgBody(body string) {
+	if voice.Params == nil {
+		voice.Params = make(url.Values)
+	}
+
+	// Encode the body, so we don't need to worry about
+	// buggy apps not escaping URL params properly.
+	voice.Params.Set(msgParamBody, b64enc.EncodeToString([]byte(body)))
+}
+
+// setMsgParams will set parameters for the provided message.
+func (voice *VoiceOptions) setMsgParams(msg notification.Message) (err error) {
+	if voice.CallbackParams == nil {
+		voice.CallbackParams = make(url.Values)
+	}
+	if voice.Params == nil {
+		voice.Params = make(url.Values)
+	}
+
+	subID := -1
+	switch t := msg.(type) {
+	case notification.AlertBundle:
+		voice.Params.Set(msgParamBundle, "1")
+		voice.CallType = CallTypeAlert
+	case notification.Alert:
+		voice.CallType = CallTypeAlert
+		subID = t.AlertID
+	case notification.AlertStatus:
+		voice.CallType = CallTypeAlertStatus
+		subID = t.AlertID
+	case notification.Test:
+		voice.CallType = CallTypeTest
+	case notification.Verification:
+		voice.CallType = CallTypeVerify
+	default:
+		return errors.Errorf("unhandled message type: %T", t)
+	}
+
+	voice.Params.Set(msgParamSubID, strconv.Itoa(subID))
+	voice.CallbackParams.Set(msgParamID, msg.ID())
+
+	return nil
+}
+
 // StatusCallbackURL will return the status callback url for the given configuration.
 func (voice *VoiceOptions) StatusCallbackURL(cfg config.Config) (string, error) {
 	if voice == nil {

diff --git a/notification/twilio/client_test.go b/notification/twilio/client_test.go
@@ -0,0 +1,184 @@
+package twilio
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/target/goalert/notification"
+)
+
+func TestSetMsgParams(t *testing.T) {
+	t.Run("Test Notification", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(
+			notification.Test{
+				Dest: notification.Dest{
+					ID:    "1",
+					Type:  notification.DestTypeVoice,
+					Value: "+16125551234",
+				},
+				CallbackID: "2",
+			},
+		)
+		expected := VoiceOptions{
+			CallType:       "test",
+			CallbackParams: url.Values{"msgID": []string{"2"}},
+			Params:         url.Values{"msgSubjectID": []string{"-1"}},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.NoError(t, err)
+	})
+	t.Run("AlertBundle Notification", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(
+			notification.AlertBundle{
+				Dest: notification.Dest{
+					ID:    "1",
+					Type:  notification.DestTypeVoice,
+					Value: "+16125551234",
+				},
+				CallbackID:  "2",
+				ServiceID:   "3",
+				ServiceName: "Widget",
+				Count:       5,
+			},
+		)
+		expected := VoiceOptions{
+			CallType:       "alert",
+			CallbackParams: url.Values{"msgID": []string{"2"}},
+			Params: url.Values{
+				"msgBundle":    []string{"1"},
+				"msgSubjectID": []string{"-1"},
+			},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.NoError(t, err)
+	})
+	t.Run("Alert Notification", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(
+			notification.Alert{
+				Dest: notification.Dest{
+					ID:    "1",
+					Type:  notification.DestTypeVoice,
+					Value: "+16125551234",
+				},
+				CallbackID: "2",
+				AlertID:    3,
+				Summary:    "Widget is Broken",
+				Details:    "Oh No!",
+			},
+		)
+		expected := VoiceOptions{
+			CallType:       "alert",
+			CallbackParams: url.Values{"msgID": []string{"2"}},
+			Params:         url.Values{"msgSubjectID": []string{"3"}},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.NoError(t, err)
+	})
+	t.Run("AlertStatus Notification", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(
+			notification.AlertStatus{
+				Dest: notification.Dest{
+					ID:    "1",
+					Type:  notification.DestTypeVoice,
+					Value: "+16125551234",
+				},
+				CallbackID: "2",
+				AlertID:    3,
+				Summary:    "Widget is Broken",
+				Details:    "Oh No!",
+				LogEntry:   "Something is Wrong",
+			},
+		)
+		expected := VoiceOptions{
+			CallType:       "alert-status",
+			CallbackParams: url.Values{"msgID": []string{"2"}},
+			Params:         url.Values{"msgSubjectID": []string{"3"}},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.NoError(t, err)
+	})
+	t.Run("Verification Notification", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(
+			notification.Verification{
+				Dest: notification.Dest{
+					ID:    "1",
+					Type:  notification.DestTypeVoice,
+					Value: "+16125551234",
+				},
+				CallbackID: "2",
+				Code:       1234,
+			},
+		)
+		expected := VoiceOptions{
+			CallType:       "verify",
+			CallbackParams: url.Values{"msgID": []string{"2"}},
+			Params:         url.Values{"msgSubjectID": []string{"-1"}},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.NoError(t, err)
+	})
+	t.Run("Bad Type", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(
+			notification.ScheduleOnCallUsers{
+				Dest: notification.Dest{
+					ID:    "1",
+					Type:  notification.DestTypeVoice,
+					Value: "+16125551234",
+				},
+				CallbackID:   "2",
+				ScheduleID:   "3",
+				ScheduleName: "4",
+				ScheduleURL:  "5",
+			},
+		)
+		expected := VoiceOptions{
+			CallbackParams: url.Values{},
+			Params:         url.Values{},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.Equal(t, err.Error(), "unhandled message type: notification.ScheduleOnCallUsers")
+	})
+	t.Run("no input", func(t *testing.T) {
+		result := &VoiceOptions{}
+		err := result.setMsgParams(nil)
+		expected := VoiceOptions{
+			CallbackParams: url.Values{},
+			Params:         url.Values{},
+		}
+
+		assert.Equal(t, expected, *result)
+		assert.Equal(t, err.Error(), "unhandled message type: <nil>")
+	})
+}
+
+func TestSetMsgBody(t *testing.T) {
+	t.Run("Test Notification", func(t *testing.T) {
+		result := &VoiceOptions{}
+		result.setMsgBody("This is GoAlert with a test message.")
+		expected := &VoiceOptions{
+			Params: url.Values{"msgBody": []string{b64enc.EncodeToString([]byte("This is GoAlert with a test message."))}},
+		}
+		assert.Equal(t, expected, result)
+	})
+	t.Run("no input", func(t *testing.T) {
+		result := &VoiceOptions{}
+		result.setMsgBody("")
+		expected := &VoiceOptions{
+			Params: url.Values{"msgBody": []string{b64enc.EncodeToString([]byte(""))}},
+		}
+		assert.Equal(t, expected, result)
+	})
+}
diff --git a/notification/twilio/voice.go b/notification/twilio/voice.go
@@ -186,50 +186,17 @@ func (v *Voice) Send(ctx context.Context, msg notification.Message) (*notificati
 
 	opts := &VoiceOptions{
 		ValidityPeriod: time.Second * 10,
-		CallbackParams: make(url.Values),
-		Params:         make(url.Values),
 	}
 
-	prefix := fmt.Sprintf("Hello! This is %s", cfg.ApplicationName())
-
-	var message string
-	subID := -1
-	switch t := msg.(type) {
-	case notification.AlertBundle:
-		message = fmt.Sprintf("%s with alert notifications. Service '%s' has %d unacknowledged alerts.", prefix, t.ServiceName, t.Count)
-		opts.Params.Set(msgParamBundle, "1")
-		opts.CallType = CallTypeAlert
-	case notification.Alert:
-		if t.Summary == "" {
-			t.Summary = "No summary provided"
-		}
-		message = fmt.Sprintf("%s with an alert notification. %s.", prefix, t.Summary)
-		opts.CallType = CallTypeAlert
-		subID = t.AlertID
-	case notification.AlertStatus:
-		message = rmParen.ReplaceAllString(t.LogEntry, "")
-		message = fmt.Sprintf("%s with a status update for alert '%s'. %s", prefix, t.Summary, message)
-		opts.CallType = CallTypeAlertStatus
-		subID = t.AlertID
-	case notification.Test:
-		message = fmt.Sprintf("%s with a test message.", prefix)
-		opts.CallType = CallTypeTest
-	case notification.Verification:
-		count := int(math.Log10(float64(t.Code)) + 1)
-		message = fmt.Sprintf(
-			"%s with your %d-digit verification code. The code is: %s. Again, your  %d-digit verification code is: %s.",
-			prefix, count, spellNumber(t.Code), count, spellNumber(t.Code),
-		)
-		opts.CallType = CallTypeVerify
-	default:
-		return nil, errors.Errorf("unhandled message type: %T", t)
+	if err := opts.setMsgParams(msg); err != nil {
+		return nil, err
 	}
 
-	opts.Params.Set(msgParamSubID, strconv.Itoa(subID))
-	opts.CallbackParams.Set(msgParamID, msg.ID())
-	// Encode the body so we don't need to worry about
-	// buggy apps not escaping url params properly.
-	opts.Params.Set(msgParamBody, b64enc.EncodeToString([]byte(message)))
+	msgBody, err := buildMessage(fmt.Sprintf("Hello! This is %s", cfg.ApplicationName()), msg)
+	if err != nil {
+		return nil, err
+	}
+	opts.setMsgBody(msgBody)
 
 	voiceResponse, err := v.c.StartVoice(ctx, toNumber, opts)
 	if err != nil {
@@ -644,3 +611,35 @@ func (v *Voice) FriendlyValue(ctx context.Context, value string) (string, error)
 	}
 	return libphonenumber.Format(num, libphonenumber.INTERNATIONAL), nil
 }
+
+// buildMessage is a function that will build the VoiceOptions object with the proper message contents
+func buildMessage(prefix string, msg notification.Message) (message string, err error) {
+	if prefix == "" {
+		return "", errors.New("buildMessage error: no prefix provided")
+	}
+
+	switch t := msg.(type) {
+	case notification.AlertBundle:
+		message = fmt.Sprintf("%s with alert notifications. Service '%s' has %d unacknowledged alerts.", prefix, t.ServiceName, t.Count)
+	case notification.Alert:
+		if t.Summary == "" {
+			t.Summary = "No summary provided"
+		}
+		message = fmt.Sprintf("%s with an alert notification. %s.", prefix, t.Summary)
+	case notification.AlertStatus:
+		message = rmParen.ReplaceAllString(t.LogEntry, "")
+		message = fmt.Sprintf("%s with a status update for alert '%s'. %s", prefix, t.Summary, message)
+	case notification.Test:
+		message = fmt.Sprintf("%s with a test message.", prefix)
+	case notification.Verification:
+		count := int(math.Log10(float64(t.Code)) + 1)
+		message = fmt.Sprintf(
+			"%s with your %d-digit verification code. The code is: %s. Again, your %d-digit verification code is: %s.",
+			prefix, count, spellNumber(t.Code), count, spellNumber(t.Code),
+		)
+	default:
+		return "", errors.Errorf("unhandled message type: %T", t)
+	}
+
+	return
+}