Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
badd15b
validate alert log on ack test
mastercactapus Mar 18, 2025
7cdab04
config: Add RCS sender ID configuration and update harness for RCS me…
mastercactapus Mar 18, 2025
6a380dc
feat(mocktwilio): add RCS support to messaging service and update SMS…
mastercactapus Mar 18, 2025
4ca7a11
test: add RCS acknowledgment processing test for Twilio SMS
mastercactapus Mar 18, 2025
671844d
feat(notification): add support for message read status and update pr…
mastercactapus Mar 18, 2025
346843f
feat(twilio): enhance SMS handling for RCS sender ID and improve vali…
mastercactapus Mar 18, 2025
58b11c5
fix form casing
mastercactapus Mar 18, 2025
291fdc0
regen
mastercactapus Mar 18, 2025
dc13fdc
feat(mocktwilio): improve RCS ID handling and update SMS processing l…
mastercactapus Mar 18, 2025
38915ff
feat(twilio): add support for message read status and update SMS proc…
mastercactapus Mar 18, 2025
133b415
test(mailpit): increase timeout for mailpit listening check
mastercactapus Mar 18, 2025
00d16f1
Merge branch 'master' into base-rcs-support
mastercactapus Mar 20, 2025
cee99bf
fix(twilio): correct sender ID check in ServeStatusCallback
mastercactapus Mar 20, 2025
3bbf8bf
feat(alert): add support for 'Read' notification state
mastercactapus Mar 21, 2025
8749bb4
fix(twilio): rename RCSSenderID to RCSSenderSID and update related va…
mastercactapus Mar 21, 2025
a127654
Revert "fix(twilio): rename RCSSenderID to RCSSenderSID and update re…
mastercactapus Mar 21, 2025
d881a9b
fix(twilio): update RCSSenderID validation and clarify dependency on …
mastercactapus Mar 21, 2025
f793630
fix(twilio): update RCS sender ID description to clarify dependency o…
mastercactapus Mar 21, 2025
607dbde
fix(twilio): reorder configuration setting for RCSSenderID to ensure …
mastercactapus Mar 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ type Config struct {

MessagingServiceSID string `public:"true" info:"If set, replaces the use of From Number for SMS notifications."`

RCSSenderID string `info:"The sender ID for RCS messages. Required if RCS is enabled for the MessagingServiceSID."`

DisableTwoWaySMS bool `info:"Disables SMS reply codes for alert messages."`
SMSCarrierLookup bool `info:"Perform carrier lookup of SMS contact methods (required for SMSFromNumberOverride). Extra charges may apply."`
SMSFromNumberOverride []string `info:"List of 'carrier=number' pairs, SMS messages to numbers of the provided carrier string (exact match) will use the alternate From Number."`
Expand Down Expand Up @@ -508,6 +510,13 @@ func (cfg Config) Validate() error {
if cfg.Twilio.MessagingServiceSID != "" {
err = validate.Many(err, validate.TwilioSID("Twilio.MessagingServiceSID", "MG", cfg.Twilio.MessagingServiceSID))
}
if cfg.Twilio.RCSSenderID != "" {
err = validate.Many(err, validate.ASCII("Twilio.RCSSenderSID", cfg.Twilio.RCSSenderID, 1, 255))
if cfg.Twilio.MessagingServiceSID == "" {
err = validate.Many(err, validation.NewFieldError("Twilio.MessagingServiceSID", "required when Twilio.RCSSenderID is set"))
}
}

if cfg.Mailgun.EmailDomain != "" {
err = validate.Many(err, validate.Email("Mailgun.EmailDomain", "example@"+cfg.Mailgun.EmailDomain))
}
Expand Down
20 changes: 20 additions & 0 deletions devtools/mocktwilio/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type Server struct {
messages map[string]*SMS
calls map[string]*VoiceCall
msgSvc map[string][]string
rcs map[string]string

mux *http.ServeMux

Expand Down Expand Up @@ -80,6 +81,7 @@ func NewServer(cfg Config) *Server {
errs: make(chan error, 10000),
shutdown: make(chan struct{}),
carrierInfo: make(map[string]twilio.CarrierInfo),
rcs: make(map[string]string),
}

base := "/2010-04-01/Accounts/" + cfg.AccountSID
Expand Down Expand Up @@ -237,6 +239,24 @@ func (s *Server) NewMessagingService(url string, numbers ...string) (string, err
return svcID, nil
}

// EnableRCS enables RCS for the given messaging service ID, returning the RCS sender ID.
func (s *Server) EnableRCS(msgSvcID string) (string, error) {
s.mx.Lock()
defer s.mx.Unlock()

if _, ok := s.msgSvc[msgSvcID]; !ok {
return "", errors.New("messaging service not found")
}
seq := atomic.AddUint64(&s.sidSeq, 1)
rcsID := fmt.Sprintf("test_%04d_agent", seq)
s.rcs[msgSvcID] = rcsID

url := s.msgSvc[msgSvcID][0]
s.callbacks["SMS:"+rcsID] = url

return rcsID, nil
}

// RegisterSMSCallback will set/update a callback URL for SMS calls made to the given number.
func (s *Server) RegisterSMSCallback(number, url string) error {
err := validate.URL("URL", url)
Expand Down
19 changes: 18 additions & 1 deletion devtools/mocktwilio/sms.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,22 @@ func (s *Server) sendSMS(fromValue, to, body, statusURL, destURL string) (*SMS,
}
}

// Use the RCS ID for the rest of the processing
s.mx.RLock()
rcsID := s.rcs[fromValue]
if rcsID != "" {
fromNumber = "rcs:" + rcsID
}
s.mx.RUnlock()

if strings.HasPrefix(fromNumber, "rcs:") {
to = "rcs:" + to
}
sms := &SMS{
s: s,
msg: twilio.Message{
To: to,
From: fromNumber,
Status: twilio.MessageStatusAccepted,
SID: s.id("SM"),
},
Expand Down Expand Up @@ -157,6 +169,7 @@ func (sms *SMS) updateStatus(stat twilio.MessageStatus) {
sms.s.errs <- err
}
}

func (sms *SMS) cloneMessage() *twilio.Message {
sms.mx.Lock()
defer sms.mx.Unlock()
Expand Down Expand Up @@ -251,7 +264,11 @@ func (sms *SMS) process() {
return
case accepted := <-sms.acceptCh:
if accepted {
sms.updateStatus(twilio.MessageStatusDelivered)
if strings.HasPrefix(sms.To(), "rcs:") {
sms.updateStatus(twilio.MessageStatusRead)
} else {
sms.updateStatus(twilio.MessageStatusDelivered)
}
} else {
sms.updateStatus(twilio.MessageStatusFailed)
}
Expand Down
4 changes: 3 additions & 1 deletion engine/message/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ type DB struct {
func NewDB(ctx context.Context, db *sql.DB, a *alertlog.Store, pausable lifecycle.Pausable) (*DB, error) {
lock, err := processinglock.NewLock(ctx, db, processinglock.Config{
Type: processinglock.TypeMessage,
Version: 10,
Version: 11,
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -465,6 +465,8 @@ func (db *DB) _UpdateMessageStatus(ctx context.Context, status *notification.Sen
s = StatusSent
case notification.StateDelivered:
s = StatusDelivered
case notification.StateRead:
s = StatusRead
}

var srcValue sql.NullString
Expand Down
5 changes: 4 additions & 1 deletion engine/message/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ const (
// StatusDelivered will be set on delivery if the upstream supports delivery confirmation.
StatusDelivered = Status("delivered")

// StatusRead will be set on read if the upstream supports read confirmation.
StatusRead = Status("read")

// StatusFailed means the message failed to send.
StatusFailed = Status("failed")

Expand All @@ -30,7 +33,7 @@ const (
// IsSent returns true if the message has been successfully sent to the downstream server.
func (s Status) IsSent() bool {
switch s {
case StatusQueuedRemotely, StatusDelivered, StatusSent:
case StatusQueuedRemotely, StatusDelivered, StatusSent, StatusRead:
return true
}
return false
Expand Down
1 change: 1 addition & 0 deletions gadb/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions graphql2/graphqlapp/alert.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ func notificationStateFromSendResult(s notification.Status, formattedSrc string)
prefix = "Sent"
case notification.StateDelivered:
prefix = "Delivered"
case notification.StateRead:
prefix = "Read"
case notification.StateFailedTemp, notification.StateFailedPerm:
prefix = "Failed"
default:
Expand Down
3 changes: 3 additions & 0 deletions graphql2/mapconfig.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions migrate/migrations/20250318133045-om-status-read.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-- +migrate Up notransaction
ALTER TYPE enum_outgoing_messages_status
ADD VALUE IF NOT EXISTS 'read';

-- +migrate Down
16 changes: 16 additions & 0 deletions migrate/migrations/20250318133231-inc-msg-module.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-- +migrate Up
UPDATE
engine_processing_versions
SET
"version" = 11
WHERE
type_id = 'message';

-- +migrate Down
UPDATE
engine_processing_versions
SET
"version" = 10
WHERE
type_id = 'message';

7 changes: 4 additions & 3 deletions migrate/schema.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
-- This file is auto-generated by "make db-schema"; DO NOT EDIT
-- DATA=0bb97428c654543fa28c6f9c86d39f59c6bd7d60de96b18fd7de8da7ced9b271 -
-- DISK=a8c9bd1468f8c14f428ef3317831db62ce978f94b804286ffea19963f701ea4a -
-- PSQL=a8c9bd1468f8c14f428ef3317831db62ce978f94b804286ffea19963f701ea4a -
-- DATA=aaee9fbdb7ed73f3fd484738aa252e094be5f358c45dc7136dd11c753c7e57f4 -
-- DISK=7cd140a81d4302dc932be2d3ad49f3d1649b9253b3665511bf91121c3bf347ea -
-- PSQL=7cd140a81d4302dc932be2d3ad49f3d1649b9253b3665511bf91121c3bf347ea -
--
-- pgdump-lite database dump
--
Expand Down Expand Up @@ -111,6 +111,7 @@ CREATE TYPE enum_outgoing_messages_status AS ENUM (
'failed',
'pending',
'queued_remotely',
'read',
'sending',
'sent'
);
Expand Down
1 change: 1 addition & 0 deletions notification/compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
StateFailedPerm = nfymsg.StateFailedPerm
StateFailedTemp = nfymsg.StateFailedTemp
StateDelivered = nfymsg.StateDelivered
StateRead = nfymsg.StateRead
StateSent = nfymsg.StateSent
StateBundled = nfymsg.StateBundled
StateUnknown = nfymsg.StateUnknown
Expand Down
9 changes: 7 additions & 2 deletions notification/nfymsg/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package nfymsg
// State represents the current state of an outgoing message.
type State int

// IsOK returns true if the message has passed successfully to a remote system (StateSending, StateSent, or StateDelivered).
func (s State) IsOK() bool { return s == StateSending || s == StateSent || s == StateDelivered }
// IsOK returns true if the message has passed successfully to a remote system (StateSending, StateSent, StateDelivered, or StateRead).
func (s State) IsOK() bool {
return s == StateSending || s == StateSent || s == StateDelivered || s == StateRead
}

const (
// StateUnknown is returned when the message has not yet been sent.
Expand All @@ -28,6 +30,9 @@ const (
// completed (including if it was voice mail).
StateDelivered

// StateRead means the message was read by the recipient.
StateRead

// StateFailedTemp should be set when a message was not sent (no SMS or ringing phone)
// but a subsequent try later may succeed. (e.g. voice call with busy signal).
StateFailedTemp
Expand Down
2 changes: 2 additions & 0 deletions notification/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,8 @@ func messageStateFromStatus(lastStatus gadb.EnumOutgoingMessagesStatus, hasNextR
return StateSending, nil
case gadb.EnumOutgoingMessagesStatusPending:
return StatePending, nil
case gadb.EnumOutgoingMessagesStatusRead:
return StateRead, nil
case gadb.EnumOutgoingMessagesStatusSent:
return StateSent, nil
case gadb.EnumOutgoingMessagesStatusDelivered:
Expand Down
4 changes: 4 additions & 0 deletions notification/twilio/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const (
MessageStatusReceiving = MessageStatus("receiving")
MessageStatusReceived = MessageStatus("received")
MessageStatusDelivered = MessageStatus("delivered")
MessageStatusRead = MessageStatus("read")
MessageStatusUndelivered = MessageStatus("undelivered")
MessageStatusFailed = MessageStatus("failed")
)
Expand Down Expand Up @@ -81,6 +82,7 @@ func (msg *Message) sentMessage() *notification.SentMessage {
SrcValue: msg.From,
}
}

func (msg *Message) messageStatus() *notification.Status {
if msg == nil {
return nil
Expand All @@ -102,6 +104,8 @@ func (msg *Message) messageStatus() *notification.Status {
status.State = notification.StateFailedPerm
case MessageStatusDelivered:
status.State = notification.StateDelivered
case MessageStatusRead:
status.State = notification.StateRead
case MessageStatusSent, MessageStatusUndelivered:
status.State = notification.StateSent
default:
Expand Down
16 changes: 11 additions & 5 deletions notification/twilio/sms.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,15 @@ func (s *SMS) ServeStatusCallback(w http.ResponseWriter, req *http.Request) {
return
}
ctx := req.Context()
cfg := config.FromContext(ctx)
status := MessageStatus(req.FormValue("MessageStatus"))
sid := validSID(req.FormValue("MessageSid"))
number := validPhone(req.FormValue("To"))
var number string
if cfg.Twilio.RCSSenderID != "" && req.FormValue("From") == "rcs:"+cfg.Twilio.RCSSenderID {
number = req.FormValue("To")
} else {
number = validPhone(req.FormValue("To"))
}
if status == "" || sid == "" || number == "" {
http.Error(w, "", http.StatusBadRequest)
return
Expand All @@ -182,7 +188,7 @@ func (s *SMS) ServeStatusCallback(w http.ResponseWriter, req *http.Request) {
"Phone": number,
"Type": "TwilioSMS",
})
msg := Message{SID: sid, Status: status, From: req.FormValue("From")}
msg := Message{SID: sid, Status: status, From: strings.TrimPrefix(req.FormValue("From"), "rcs:")}

log.Debugf(ctx, "Got Twilio SMS status callback.")

Expand Down Expand Up @@ -221,8 +227,8 @@ func (s *SMS) ServeMessage(w http.ResponseWriter, req *http.Request) {
}
ctx := req.Context()
cfg := config.FromContext(ctx)
from := validPhone(req.FormValue("From"))
if from == "" || from == cfg.Twilio.FromNumber {
from := validPhone(strings.TrimPrefix(req.FormValue("From"), "rcs:"))
if from == "" || from == cfg.Twilio.FromNumber || from == cfg.Twilio.RCSSenderID {
http.Error(w, "", http.StatusBadRequest)
return
}
Expand Down Expand Up @@ -253,7 +259,7 @@ func (s *SMS) ServeMessage(w http.ResponseWriter, req *http.Request) {
}
s.limit.RecordPassiveReply(from)
}
smsFrom := req.FormValue("to")
smsFrom := req.FormValue("To")
if cfg.Twilio.MessagingServiceSID != "" {
smsFrom = cfg.Twilio.MessagingServiceSID
}
Expand Down
4 changes: 2 additions & 2 deletions test/smoke/harness/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/target/goalert/auth"
"github.com/target/goalert/limit"
"github.com/target/goalert/permission"
Expand Down Expand Up @@ -63,7 +63,7 @@ func (h *Harness) GraphQLQuery2(query string) *QLResponse {
func (h *Harness) SetConfigValue(id, value string) {
h.t.Helper()
res := h.GraphQLQuery2(fmt.Sprintf(`mutation{setConfig(input:[{id: %s, value: %s}])}`, strconv.Quote(id), strconv.Quote(value)))
assert.Empty(h.t, res.Errors)
require.Empty(h.t, res.Errors)

// wait for engine cycle to complete to ensure next action
// uses new config only
Expand Down
29 changes: 27 additions & 2 deletions test/smoke/harness/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,10 @@ type Harness struct {

appPool *pgxpool.Pool

msgSvcID string
expFlags expflag.FlagSet
rcsSenderID string
rcsMsgSvcID string
msgSvcID string
expFlags expflag.FlagSet

tw *twilioAssertionAPI
twS *httptest.Server
Expand Down Expand Up @@ -753,6 +755,29 @@ func (h *Harness) TwilioMessagingService() string {
return newID
}

// TwilioMessagingServiceRCS will return the id and phone numbers for the mock messaging service with RCS enabled.
func (h *Harness) TwilioMessagingServiceRCS() (rcs, msg string) {
h.mx.Lock()
defer h.mx.Unlock()
if h.rcsSenderID != "" {
return h.rcsSenderID, h.rcsMsgSvcID
}

nums := []string{h.phoneCCG.Get("twilio:rcs:sid1"), h.phoneCCG.Get("twilio:rcs:sid2"), h.phoneCCG.Get("twilio:rcs:sid3")}
newID, err := h.tw.NewMessagingService(h.URL()+"/v1/twilio/sms/messages", nums...)
if err != nil {
panic(err)
}

h.rcsMsgSvcID = newID

rcsID, err := h.tw.EnableRCS(newID)
require.NoError(h.t, err)
h.rcsSenderID = rcsID

return rcsID, newID
}

// CreateUser generates a random user.
func (h *Harness) CreateUser() (u *user.User) {
h.t.Helper()
Expand Down
Loading
Loading