From 29e82a422723c9ae0ad894077a8dcaedbf0bfe24 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Mon, 12 Sep 2022 13:23:07 -0500 Subject: [PATCH] Use MsgOut.UnsendableReason instead of checking contact status --- core/models/msgs.go | 27 +++++++++++++++++---------- core/models/msgs_test.go | 25 ++++++++++++++++--------- go.mod | 2 +- go.sum | 4 ++-- services/ivr/twiml/service_test.go | 17 ++++++++--------- services/ivr/vonage/service_test.go | 17 ++++++++--------- testsuite/testdata/msgs.go | 2 +- 7 files changed, 53 insertions(+), 41 deletions(-) diff --git a/core/models/msgs.go b/core/models/msgs.go index 0e8cd8dee..5dc1159a8 100644 --- a/core/models/msgs.go +++ b/core/models/msgs.go @@ -87,6 +87,11 @@ const ( MsgFailedNoDestination = MsgFailedReason("D") ) +var unsendableToFailedReason = map[flows.UnsendableReason]MsgFailedReason{ + flows.UnsendableReasonContactStatus: MsgFailedContact, + flows.UnsendableReasonNoDestination: MsgFailedNoDestination, +} + // BroadcastID is our internal type for broadcast ids, which can be null/0 type BroadcastID null.Int @@ -361,18 +366,13 @@ func newOutgoingMsg(rt *runtime.Runtime, org *Org, channel *Channel, contact *fl msg.SetChannel(channel) msg.SetURN(out.URN()) - if org.Suspended() { + if out.UnsendableReason() != flows.NilUnsendableReason { + m.Status = MsgStatusFailed + m.FailedReason = unsendableToFailedReason[out.UnsendableReason()] + } else if org.Suspended() { // we fail messages for suspended orgs right away m.Status = MsgStatusFailed m.FailedReason = MsgFailedSuspended - } else if contact.Status() != flows.ContactStatusActive { - // and blocked, stopped or archived contacts - m.Status = MsgStatusFailed - m.FailedReason = MsgFailedContact - } else if msg.URN() == urns.NilURN || channel == nil { - // if msg is missing the URN or channel, we also fail it - m.Status = MsgStatusFailed - m.FailedReason = MsgFailedNoDestination } else { // also fail right away if this looks like a loop repetitions, err := GetMsgRepetitions(rt.RP, contact, out) @@ -1071,8 +1071,15 @@ func (b *BroadcastBatch) CreateMessages(ctx context.Context, rt *runtime.Runtime return nil, nil } + unsendableReason := flows.NilUnsendableReason + if contact.Status() != flows.ContactStatusActive { + unsendableReason = flows.UnsendableReasonContactStatus + } else if urn == urns.NilURN || channel == nil { + unsendableReason = flows.UnsendableReasonNoDestination + } + // create our outgoing message - out := flows.NewMsgOut(urn, channel.ChannelReference(), text, t.Attachments, t.QuickReplies, nil, flows.NilMsgTopic) + out := flows.NewMsgOut(urn, channel.ChannelReference(), text, t.Attachments, t.QuickReplies, nil, flows.NilMsgTopic, unsendableReason) msg, err := NewOutgoingBroadcastMsg(rt, oa.Org(), channel, contact, out, time.Now(), b.BroadcastID) if err != nil { return nil, errors.Wrapf(err, "error creating outgoing message") diff --git a/core/models/msgs_test.go b/core/models/msgs_test.go index 68dbb4f6c..bc64a807d 100644 --- a/core/models/msgs_test.go +++ b/core/models/msgs_test.go @@ -44,6 +44,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { Attachments []utils.Attachment QuickReplies []string Topic flows.MsgTopic + Unsendable flows.UnsendableReason Flow *testdata.Flow ResponseTo models.MsgID SuspendedOrg bool @@ -120,6 +121,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { Contact: testdata.Cathy, URN: urns.NilURN, URNID: models.URNID(0), + Unsendable: flows.UnsendableReasonNoDestination, Flow: testdata.Favorites, ExpectedStatus: models.MsgStatusFailed, ExpectedFailedReason: models.MsgFailedNoDestination, @@ -133,6 +135,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { Contact: testdata.Cathy, URN: urns.URN(fmt.Sprintf("tel:+250700000001?id=%d", testdata.Cathy.URNID)), URNID: testdata.Cathy.URNID, + Unsendable: flows.UnsendableReasonNoDestination, Flow: testdata.Favorites, ExpectedStatus: models.MsgStatusFailed, ExpectedFailedReason: models.MsgFailedNoDestination, @@ -146,6 +149,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { Contact: blake, URN: urns.URN(fmt.Sprintf("tel:+250700000007?id=%d", blakeURNID)), URNID: blakeURNID, + Unsendable: flows.UnsendableReasonContactStatus, Flow: testdata.Favorites, ExpectedStatus: models.MsgStatusFailed, ExpectedFailedReason: models.MsgFailedContact, @@ -158,6 +162,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { now := time.Now() for _, tc := range tcs { + desc := fmt.Sprintf("text='%s'", tc.Text) db.MustExec(`UPDATE orgs_org SET is_suspended = $1 WHERE id = $2`, tc.SuspendedOrg, testdata.Org1.ID) oa, err := models.GetOrgAssetsWithRefresh(ctx, rt, testdata.Org1.ID, models.RefreshOrg) @@ -171,7 +176,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { session.SetIncomingMsg(flows.MsgID(tc.ResponseTo), null.NullString) } - flowMsg := flows.NewMsgOut(tc.URN, assets.NewChannelReference(tc.ChannelUUID, "Test Channel"), tc.Text, tc.Attachments, tc.QuickReplies, nil, tc.Topic) + flowMsg := flows.NewMsgOut(tc.URN, assets.NewChannelReference(tc.ChannelUUID, "Test Channel"), tc.Text, tc.Attachments, tc.QuickReplies, nil, tc.Topic, tc.Unsendable) msg, err := models.NewOutgoingFlowMsg(rt, oa.Org(), channel, session, flow, flowMsg, now) assert.NoError(t, err) @@ -191,8 +196,8 @@ func TestNewOutgoingFlowMsg(t *testing.T) { } assert.Equal(t, tc.Flow.ID, msg.FlowID()) - assert.Equal(t, tc.ExpectedStatus, msg.Status()) - assert.Equal(t, tc.ExpectedFailedReason, msg.FailedReason()) + assert.Equal(t, tc.ExpectedStatus, msg.Status(), "status mismatch for %s", desc) + assert.Equal(t, tc.ExpectedFailedReason, msg.FailedReason(), "failed reason mismatch for %s", desc) assert.Equal(t, tc.ExpectedMetadata, msg.Metadata()) assert.Equal(t, tc.ExpectedMsgCount, msg.MsgCount()) assert.Equal(t, now, msg.CreatedOn()) @@ -216,7 +221,7 @@ func TestNewOutgoingFlowMsg(t *testing.T) { // check that msg loop detection triggers after 20 repeats of the same text newOutgoing := func(text string) *models.Msg { - flowMsg := flows.NewMsgOut(urns.URN(fmt.Sprintf("tel:+250700000001?id=%d", testdata.Cathy.URNID)), assets.NewChannelReference(testdata.TwilioChannel.UUID, "Twilio"), text, nil, nil, nil, flows.NilMsgTopic) + flowMsg := flows.NewMsgOut(urns.URN(fmt.Sprintf("tel:+250700000001?id=%d", testdata.Cathy.URNID)), assets.NewChannelReference(testdata.TwilioChannel.UUID, "Twilio"), text, nil, nil, nil, flows.NilMsgTopic, flows.NilUnsendableReason) msg, err := models.NewOutgoingFlowMsg(rt, oa.Org(), channel, session, flow, flowMsg, now) require.NoError(t, err) return msg @@ -261,6 +266,7 @@ func TestMarshalMsg(t *testing.T) { []string{"yes", "no"}, nil, flows.MsgTopicPurchase, + flows.NilUnsendableReason, ) // create a non-priority flow message.. i.e. the session isn't responding to an incoming message @@ -318,6 +324,7 @@ func TestMarshalMsg(t *testing.T) { "Hi there", nil, nil, nil, flows.NilMsgTopic, + flows.NilUnsendableReason, ) in1 := testdata.InsertIncomingMsg(db, testdata.Org1, testdata.TwilioChannel, testdata.Cathy, "test", models.MsgStatusHandled) session.SetIncomingMsg(flows.MsgID(in1.ID()), null.String("EX123")) @@ -359,7 +366,7 @@ func TestMarshalMsg(t *testing.T) { // try a broadcast message which won't have session and flow fields set bcastID := testdata.InsertBroadcast(db, testdata.Org1, `eng`, map[envs.Language]string{`eng`: "Blast"}, models.NilScheduleID, []*testdata.Contact{testdata.Cathy}, nil) - bcastMsg1 := flows.NewMsgOut(urn, assets.NewChannelReference(testdata.TwilioChannel.UUID, "Test Channel"), "Blast", nil, nil, nil, flows.NilMsgTopic) + bcastMsg1 := flows.NewMsgOut(urn, assets.NewChannelReference(testdata.TwilioChannel.UUID, "Test Channel"), "Blast", nil, nil, nil, flows.NilMsgTopic, flows.NilUnsendableReason) msg3, err := models.NewOutgoingBroadcastMsg(rt, oa.Org(), channel, cathy, bcastMsg1, time.Date(2021, 11, 9, 14, 3, 30, 0, time.UTC), bcastID) require.NoError(t, err) @@ -492,8 +499,8 @@ func TestGetMsgRepetitions(t *testing.T) { oa := testdata.Org1.Load(rt) _, cathy := testdata.Cathy.Load(db, oa) - msg1 := flows.NewMsgOut(testdata.Cathy.URN, nil, "foo", nil, nil, nil, flows.NilMsgTopic) - msg2 := flows.NewMsgOut(testdata.Cathy.URN, nil, "bar", nil, nil, nil, flows.NilMsgTopic) + msg1 := flows.NewMsgOut(testdata.Cathy.URN, nil, "foo", nil, nil, nil, flows.NilMsgTopic, flows.NilUnsendableReason) + msg2 := flows.NewMsgOut(testdata.Cathy.URN, nil, "bar", nil, nil, nil, flows.NilMsgTopic, flows.NilUnsendableReason) assertRepetitions := func(m *flows.MsgOut, expected int) { count, err := models.GetMsgRepetitions(rp, cathy, m) @@ -660,12 +667,12 @@ func TestNewOutgoingIVR(t *testing.T) { createdOn := time.Date(2021, 7, 26, 12, 6, 30, 0, time.UTC) - flowMsg := flows.NewMsgOut(testdata.Cathy.URN, vonage.ChannelReference(), "Hello", []utils.Attachment{"audio/mp3:http://example.com/hi.mp3"}, nil, nil, flows.NilMsgTopic) + flowMsg := flows.NewIVRMsgOut(testdata.Cathy.URN, vonage.ChannelReference(), "Hello", "eng", "http://example.com/hi.mp3") dbMsg := models.NewOutgoingIVR(rt.Config, testdata.Org1.ID, conn, flowMsg, createdOn) assert.Equal(t, flowMsg.UUID(), dbMsg.UUID()) assert.Equal(t, "Hello", dbMsg.Text()) - assert.Equal(t, []utils.Attachment{"audio/mp3:http://example.com/hi.mp3"}, dbMsg.Attachments()) + assert.Equal(t, []utils.Attachment{"audio:http://example.com/hi.mp3"}, dbMsg.Attachments()) assert.Equal(t, createdOn, dbMsg.CreatedOn()) assert.Equal(t, &createdOn, dbMsg.SentOn()) diff --git a/go.mod b/go.mod index d8780bd15..1dcea6fcb 100644 --- a/go.mod +++ b/go.mod @@ -16,7 +16,7 @@ require ( github.com/lib/pq v1.10.6 github.com/nyaruka/ezconf v0.2.1 github.com/nyaruka/gocommon v1.30.0 - github.com/nyaruka/goflow v0.169.0 + github.com/nyaruka/goflow v0.170.0 github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d github.com/nyaruka/null v1.2.0 github.com/nyaruka/redisx v0.2.2 diff --git a/go.sum b/go.sum index 2a56b77a3..4d8486a38 100644 --- a/go.sum +++ b/go.sum @@ -220,8 +220,8 @@ github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0= github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw= github.com/nyaruka/gocommon v1.30.0 h1:394CU1fxGXSs+mrd5x8nXnqw2epT9P0ui/9MtSE2ycQ= github.com/nyaruka/gocommon v1.30.0/go.mod h1:PApT/06fP5Tzs4/kbkJ+rVoyOc9Lbqm1lR0ow8Vqzp0= -github.com/nyaruka/goflow v0.169.0 h1:7njRUHK2vBcw0tZbbMQV81r7H6yf6Ycmh/Om/08OBRg= -github.com/nyaruka/goflow v0.169.0/go.mod h1:BELqxxpx4dqpBvaqM2TSXyq3rdJvX4RPlSAyuevjL+4= +github.com/nyaruka/goflow v0.170.0 h1:D3k6EXzvdrxze0Uz1Ze700WY7ByeoxXmAspKz/VBllM= +github.com/nyaruka/goflow v0.170.0/go.mod h1:BELqxxpx4dqpBvaqM2TSXyq3rdJvX4RPlSAyuevjL+4= github.com/nyaruka/librato v1.0.0 h1:Vznj9WCeC1yZXbBYyYp40KnbmXLbEkjKmHesV/v2SR0= github.com/nyaruka/librato v1.0.0/go.mod h1:pkRNLFhFurOz0QqBz6/DuTFhHHxAubWxs4Jx+J7yUgg= github.com/nyaruka/logrus_sentry v0.8.2-0.20190129182604-c2962b80ba7d h1:hyp9u36KIwbTCo2JAJ+TuJcJBc+UZzEig7RI/S5Dvkc= diff --git a/services/ivr/twiml/service_test.go b/services/ivr/twiml/service_test.go index 447872114..4ff23e430 100644 --- a/services/ivr/twiml/service_test.go +++ b/services/ivr/twiml/service_test.go @@ -13,7 +13,6 @@ import ( "github.com/nyaruka/goflow/assets" "github.com/nyaruka/goflow/flows/events" "github.com/nyaruka/goflow/flows/routers/waits/hints" - "github.com/nyaruka/goflow/utils" "github.com/nyaruka/mailroom/services/ivr/twiml" "github.com/nyaruka/mailroom/testsuite" @@ -40,7 +39,7 @@ func TestResponseForSprint(t *testing.T) { }{ { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "")), }, `hello world`, }, @@ -58,40 +57,40 @@ func TestResponseForSprint(t *testing.T) { }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", []utils.Attachment{utils.Attachment("audio:/recordings/foo.wav")}, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "eng", "/recordings/foo.wav")), }, `https://mailroom.io/recordings/foo.wav`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", []utils.Attachment{utils.Attachment("audio:https://temba.io/recordings/foo.wav")}, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "https://temba.io/recordings/foo.wav")), }, `https://temba.io/recordings/foo.wav`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", nil, nil, nil, flows.NilMsgTopic)), - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "goodbye", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "")), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "goodbye", "", "")), }, `hello worldgoodbye`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "enter a number", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "enter a number", "", "")), events.NewMsgWait(nil, nil, hints.NewFixedDigitsHint(1)), }, `enter a numberhttp://temba.io/resume?session=1&wait_type=gather&timeout=true`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "enter a number, then press #", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "enter a number, then press #", "", "")), events.NewMsgWait(nil, nil, hints.NewTerminatedDigitsHint("#")), }, `enter a number, then press #http://temba.io/resume?session=1&wait_type=gather&timeout=true`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "say something", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "say something", "", "")), events.NewMsgWait(nil, nil, hints.NewAudioHint()), }, `say somethinghttp://temba.io/resume?session=1&wait_type=record&empty=true`, diff --git a/services/ivr/vonage/service_test.go b/services/ivr/vonage/service_test.go index e29f88a1d..be2cb6444 100644 --- a/services/ivr/vonage/service_test.go +++ b/services/ivr/vonage/service_test.go @@ -12,7 +12,6 @@ import ( "github.com/nyaruka/goflow/flows" "github.com/nyaruka/goflow/flows/events" "github.com/nyaruka/goflow/flows/routers/waits/hints" - "github.com/nyaruka/goflow/utils" "github.com/nyaruka/mailroom/core/models" "github.com/nyaruka/mailroom/testsuite" "github.com/nyaruka/mailroom/testsuite/testdata" @@ -72,46 +71,46 @@ func TestResponseForSprint(t *testing.T) { }{ { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "")), }, `[{"action":"talk","text":"hello world"}]`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", []utils.Attachment{utils.Attachment("audio:/recordings/foo.wav")}, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "/recordings/foo.wav")), }, `[{"action":"stream","streamUrl":["/recordings/foo.wav"]}]`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", []utils.Attachment{utils.Attachment("audio:https://temba.io/recordings/foo.wav")}, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "https://temba.io/recordings/foo.wav")), }, `[{"action":"stream","streamUrl":["https://temba.io/recordings/foo.wav"]}]`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "hello world", nil, nil, nil, flows.NilMsgTopic)), - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "goodbye", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "hello world", "", "")), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "goodbye", "", "")), }, `[{"action":"talk","text":"hello world"},{"action":"talk","text":"goodbye"}]`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "enter a number", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "enter a number", "", "")), events.NewMsgWait(nil, nil, hints.NewFixedDigitsHint(1)), }, `[{"action":"talk","text":"enter a number","bargeIn":true},{"action":"input","maxDigits":1,"submitOnHash":true,"timeOut":30,"eventUrl":["http://temba.io/resume?session=1\u0026wait_type=gather\u0026sig=OjsMUDhaBTUVLq1e6I4cM0SKYpk%3D"],"eventMethod":"POST"}]`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "enter a number, then press #", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "enter a number, then press #", "", "")), events.NewMsgWait(nil, nil, hints.NewTerminatedDigitsHint("#")), }, `[{"action":"talk","text":"enter a number, then press #","bargeIn":true},{"action":"input","maxDigits":20,"submitOnHash":true,"timeOut":30,"eventUrl":["http://temba.io/resume?session=1\u0026wait_type=gather\u0026sig=OjsMUDhaBTUVLq1e6I4cM0SKYpk%3D"],"eventMethod":"POST"}]`, }, { []flows.Event{ - events.NewIVRCreated(flows.NewMsgOut(urn, channelRef, "say something", nil, nil, nil, flows.NilMsgTopic)), + events.NewIVRCreated(flows.NewIVRMsgOut(urn, channelRef, "say something", "", "")), events.NewMsgWait(nil, nil, hints.NewAudioHint()), }, `[{"action":"talk","text":"say something"},{"action":"record","endOnKey":"#","timeOut":600,"endOnSilence":5,"eventUrl":["http://temba.io/resume?session=1\u0026wait_type=recording_url\u0026recording_uuid=f3ede2d6-becc-4ea3-ae5e-88526a9f4a57\u0026sig=Am9z7fXyU3SPCZagkSpddZSi6xY%3D"],"eventMethod":"POST"},{"action":"input","submitOnHash":true,"timeOut":1,"eventUrl":["http://temba.io/resume?session=1\u0026wait_type=record\u0026recording_uuid=f3ede2d6-becc-4ea3-ae5e-88526a9f4a57\u0026sig=fX1RhjcJNN4xYaiojVYakaz5F%2Fk%3D"],"eventMethod":"POST"}]`, diff --git a/testsuite/testdata/msgs.go b/testsuite/testdata/msgs.go index 1a3f1c8a0..bc29d1f83 100644 --- a/testsuite/testdata/msgs.go +++ b/testsuite/testdata/msgs.go @@ -54,7 +54,7 @@ func insertOutgoingMsg(db *sqlx.DB, org *Org, channel *Channel, contact *Contact channelID = channel.ID } - msg := flows.NewMsgOut(contact.URN, channelRef, text, attachments, nil, nil, flows.NilMsgTopic) + msg := flows.NewMsgOut(contact.URN, channelRef, text, attachments, nil, nil, flows.NilMsgTopic, flows.NilUnsendableReason) var sentOn *time.Time if status == models.MsgStatusWired || status == models.MsgStatusSent || status == models.MsgStatusDelivered {