diff --git a/notification/twilio/client.go b/notification/twilio/client.go index bb5218bd7a..b13d830022 100644 --- 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 new file mode 100644 index 0000000000..5eecd845b1 --- /dev/null +++ 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: ") + }) +} + +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 index cdf9857412..0c95de6431 100644 --- 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 +} diff --git a/notification/twilio/voice_test.go b/notification/twilio/voice_test.go index 4d42245bf2..2491c52f75 100644 --- a/notification/twilio/voice_test.go +++ b/notification/twilio/voice_test.go @@ -1,9 +1,11 @@ package twilio import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + "github.com/target/goalert/notification" ) func TestSpellNumber(t *testing.T) { @@ -11,6 +13,114 @@ func TestSpellNumber(t *testing.T) { assert.Equal(t, "1. 2. 3. 4. 5. 6", spellNumber(123456)) } +func TestBuildMessage(t *testing.T) { + prefix := "This is GoAlert" + + // Test Notification + result, err := buildMessage( + prefix, + notification.Test{}, + ) + assert.Equal(t, fmt.Sprintf("%s with a test message.", prefix), result) + assert.NoError(t, err) + + // AlertBundle Notification + result, err = buildMessage( + prefix, + notification.AlertBundle{ + CallbackID: "2", + ServiceID: "3", + ServiceName: "Widget", + Count: 5, + }, + ) + assert.Equal(t, fmt.Sprintf("%s with alert notifications. Service 'Widget' has 5 unacknowledged alerts.", prefix), result) + assert.NoError(t, err) + + // Alert Notification + result, err = buildMessage( + prefix, + notification.Alert{ + CallbackID: "2", + AlertID: 3, + Summary: "Widget is Broken", + Details: "Oh No!", + }, + ) + assert.Equal(t, fmt.Sprintf("%s with an alert notification. Widget is Broken.", prefix), result) + assert.NoError(t, err) + + // AlertStatus Notification + result, err = buildMessage( + prefix, + notification.AlertStatus{ + CallbackID: "2", + AlertID: 3, + Summary: "Widget is Broken", + Details: "Oh No!", + LogEntry: "Something is Wrong", + }, + ) + assert.Equal(t, fmt.Sprintf("%s with a status update for alert 'Widget is Broken'. Something is Wrong", prefix), result) + assert.NoError(t, err) + + // Verification Notification + result, err = buildMessage( + prefix, + notification.Verification{ + CallbackID: "2", + Code: 1234, + }, + ) + assert.Equal(t, fmt.Sprintf("%s with your 4-digit verification code. The code is: %s. Again, your 4-digit verification code is: %s.", prefix, spellNumber(1234), spellNumber(1234)), result) + assert.NoError(t, err) + + // Bad Type + result, err = buildMessage( + prefix, + notification.ScheduleOnCallUsers{ + CallbackID: "2", + ScheduleID: "3", + ScheduleName: "4", + ScheduleURL: "5", + }, + ) + assert.Empty(t, result) + assert.Error(t, err) + + // Missing prefix + result, err = buildMessage( + "", + notification.Test{}, + ) + assert.Empty(t, result) + assert.Error(t, err) + + // no input + result, err = buildMessage( + prefix, + nil, + ) + assert.Empty(t, result) + assert.Error(t, err) +} + +func BenchmarkBuildMessage(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = buildMessage( + fmt.Sprintf("%d", i), + notification.Test{ + Dest: notification.Dest{ + ID: "1", + Type: notification.DestTypeVoice, + Value: fmt.Sprintf("+1612555123%d", i), + }, + CallbackID: "2", + }, + ) + } +} + func BenchmarkSpellNumber(b *testing.B) { for i := 0; i < b.N; i++ { _ = spellNumber(i)