Skip to content

Commit

Permalink
Merge pull request #122 from nyaruka/less_urn_priority
Browse files Browse the repository at this point in the history 
Don't encode priority in URN strings
  • Loading branch information
@rowanseymour
rowanseymour authored Sep 11, 2023
2 parents 4d6ec09 + 6454283 commit dc13ded
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 114 deletions.
29 changes: 7 additions & 22 deletions core/models/contacts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,22 +151,20 @@ func (c *Contact) UpdatePreferredURN(ctx context.Context, db DBorTx, oa *OrgAsse
topURN := urns.NilURN
newURNs := make([]urns.URN, 0, len(c.urns))

priority := topURNPriority - 1
for _, urn := range c.urns {
id := URNID(GetURNInt(urn, "id"))
if id == urnID {
updated, err := updateURNChannelPriority(urn, channel, topURNPriority)
updated, err := updateURNChannel(urn, channel)
if err != nil {
return errors.Wrapf(err, "error updating channel on urn")
}
topURN = updated
} else {
updated, err := updateURNChannelPriority(urn, nil, priority)
updated, err := updateURNChannel(urn, nil)
if err != nil {
return errors.Wrapf(err, "error updating priority on urn")
}
newURNs = append(newURNs, updated)
priority--
}
}

Expand Down Expand Up @@ -422,7 +420,7 @@ type ContactURN struct {
ID URNID `json:"id" db:"id"`
OrgID OrgID ` db:"org_id"`
ContactID ContactID ` db:"contact_id"`
Priority int `json:"priority" db:"priority"`
Priority int ` db:"priority"`
Identity urns.URN `json:"identity" db:"identity"`
Scheme string `json:"scheme" db:"scheme"`
Path string `json:"path" db:"path"`
Expand All @@ -434,10 +432,7 @@ type ContactURN struct {
// AsURN returns a full URN representation including the query parameters needed by goflow and mailroom
func (u *ContactURN) AsURN(oa *OrgAssets) (urns.URN, error) {
// id needed to turn msg_created events into database messages
query := url.Values{
"id": []string{fmt.Sprintf("%d", u.ID)},
"priority": []string{fmt.Sprintf("%d", u.Priority)},
}
query := url.Values{"id": []string{fmt.Sprintf("%d", u.ID)}}

// channel needed by goflow URN/channel selection
if u.ChannelID != NilChannelID {
Expand All @@ -447,7 +442,7 @@ func (u *ContactURN) AsURN(oa *OrgAssets) (urns.URN, error) {
}
}

// create our URN
// re-encode our URN
urn, err := urns.NewURNFromParts(u.Scheme, u.Path, query.Encode(), string(u.Display))
if err != nil {
return urns.NilURN, errors.Wrapf(err, "invalid URN %s:%s", u.Scheme, u.Path)
Expand Down Expand Up @@ -521,15 +516,7 @@ LEFT JOIN (
SELECT
contact_id,
array_agg(
json_build_object(
'id', u.id,
'scheme', u.scheme,
'path', path,
'display', display,
'auth', auth,
'channel_id', channel_id,
'priority', priority
) ORDER BY priority DESC, id ASC
json_build_object('id', u.id, 'scheme', u.scheme, 'path', path, 'display', display, 'channel_id', channel_id) ORDER BY priority DESC, id ASC
) as urns
FROM
contacts_contacturn u
Expand Down Expand Up @@ -1170,16 +1157,14 @@ func GetURNID(urn urns.URN) URNID {
return URNID(urnID)
}

func updateURNChannelPriority(urn urns.URN, channel *Channel, priority int) (urns.URN, error) {
func updateURNChannel(urn urns.URN, channel *Channel) (urns.URN, error) {
query, err := urn.Query()
if err != nil {
return urns.NilURN, errors.Errorf("error parsing query from URN: %s", urn)
}
if channel != nil {
query["channel"] = []string{string(channel.UUID())}
}
query["priority"] = []string{strconv.FormatInt(int64(priority), 10)}

urn, err = urns.NewURNFromParts(urn.Scheme(), urn.Path(), query.Encode(), urn.Display())
if err != nil {
return urns.NilURN, errors.Wrap(err, "unable to create new urn")
Expand Down
50 changes: 25 additions & 25 deletions core/models/contacts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestContacts(t *testing.T) {

assert.Equal(t, "Cathy", cathy.Name())
assert.Equal(t, len(cathy.URNs()), 1)
assert.Equal(t, cathy.URNs()[0].String(), "tel:+16055741111?id=10000&priority=1000")
assert.Equal(t, cathy.URNs()[0].String(), "tel:+16055741111?id=10000")
assert.Equal(t, 1, cathy.Groups().Count())
assert.NotNil(t, cathy.Ticket())

Expand All @@ -79,8 +79,8 @@ func TestContacts(t *testing.T) {
assert.Equal(t, "Bob", bob.Name())
assert.NotNil(t, bob.Fields()["joined"].QueryValue())
assert.Equal(t, 2, len(bob.URNs()))
assert.Equal(t, "tel:+16055742222?id=10001&priority=1000", bob.URNs()[0].String())
assert.Equal(t, "whatsapp:250788373373?id=30000&priority=999", bob.URNs()[1].String())
assert.Equal(t, "tel:+16055742222?id=10001", bob.URNs()[0].String())
assert.Equal(t, "whatsapp:250788373373?id=30000", bob.URNs()[1].String())
assert.Equal(t, 0, bob.Groups().Count())
assert.Nil(t, bob.Ticket()) // because ticketer no longer exists

Expand All @@ -97,8 +97,8 @@ func TestContacts(t *testing.T) {

bob, err = modelContacts[1].FlowContact(org)
assert.NoError(t, err)
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001&priority=1000", bob.URNs()[0].String())
assert.Equal(t, "whatsapp:250788373373?id=30000&priority=999", bob.URNs()[1].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001", bob.URNs()[0].String())
assert.Equal(t, "whatsapp:250788373373?id=30000", bob.URNs()[1].String())

// add another tel urn to bob
testdata.InsertContactURN(rt, testdata.Org1, testdata.Bob, urns.URN("tel:+250788373373"), 10)
Expand All @@ -113,29 +113,29 @@ func TestContacts(t *testing.T) {

bob, err = modelContacts[0].FlowContact(org)
assert.NoError(t, err)
assert.Equal(t, "tel:+250788373373?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30001&priority=1000", bob.URNs()[0].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001&priority=999", bob.URNs()[1].String())
assert.Equal(t, "whatsapp:250788373373?id=30000&priority=998", bob.URNs()[2].String())
assert.Equal(t, "tel:+250788373373?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30001", bob.URNs()[0].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001", bob.URNs()[1].String())
assert.Equal(t, "whatsapp:250788373373?id=30000", bob.URNs()[2].String())

// no op this time
err = modelContacts[0].UpdatePreferredURN(ctx, rt.DB, org, models.URNID(30001), channel)
assert.NoError(t, err)

bob, err = modelContacts[0].FlowContact(org)
assert.NoError(t, err)
assert.Equal(t, "tel:+250788373373?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30001&priority=1000", bob.URNs()[0].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001&priority=999", bob.URNs()[1].String())
assert.Equal(t, "whatsapp:250788373373?id=30000&priority=998", bob.URNs()[2].String())
assert.Equal(t, "tel:+250788373373?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30001", bob.URNs()[0].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001", bob.URNs()[1].String())
assert.Equal(t, "whatsapp:250788373373?id=30000", bob.URNs()[2].String())

// calling with no channel is a noop on the channel
err = modelContacts[0].UpdatePreferredURN(ctx, rt.DB, org, models.URNID(30001), nil)
assert.NoError(t, err)

bob, err = modelContacts[0].FlowContact(org)
assert.NoError(t, err)
assert.Equal(t, "tel:+250788373373?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30001&priority=1000", bob.URNs()[0].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001&priority=999", bob.URNs()[1].String())
assert.Equal(t, "whatsapp:250788373373?id=30000&priority=998", bob.URNs()[2].String())
assert.Equal(t, "tel:+250788373373?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30001", bob.URNs()[0].String())
assert.Equal(t, "tel:+16055742222?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=10001", bob.URNs()[1].String())
assert.Equal(t, "whatsapp:250788373373?id=30000", bob.URNs()[2].String())
}

func TestCreateContact(t *testing.T) {
Expand All @@ -156,11 +156,11 @@ func TestCreateContact(t *testing.T) {

assert.Equal(t, "Rich", contact.Name())
assert.Equal(t, i18n.Language(`kin`), contact.Language())
assert.Equal(t, []urns.URN{"telegram:200001?id=30001&priority=1000", "telegram:200002?id=30000&priority=999"}, contact.URNs())
assert.Equal(t, []urns.URN{"telegram:200001?id=30001", "telegram:200002?id=30000"}, contact.URNs())

assert.Equal(t, "Rich", flowContact.Name())
assert.Equal(t, i18n.Language(`kin`), flowContact.Language())
assert.Equal(t, []urns.URN{"telegram:200001?id=30001&priority=1000", "telegram:200002?id=30000&priority=999"}, flowContact.URNs().RawURNs())
assert.Equal(t, []urns.URN{"telegram:200001?id=30001", "telegram:200002?id=30000"}, flowContact.URNs().RawURNs())
assert.Len(t, flowContact.Groups().All(), 1)
assert.Equal(t, assets.GroupUUID("d636c966-79c1-4417-9f1c-82ad629773a2"), flowContact.Groups().All()[0].UUID())

Expand Down Expand Up @@ -229,7 +229,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{testdata.Cathy.URN},
testdata.Cathy.ID,
false,
[]urns.URN{"tel:+16055741111?id=10000&priority=1000"},
[]urns.URN{"tel:+16055741111?id=10000"},
models.NilChannelID,
[]assets.GroupUUID{testdata.DoctorsGroup.UUID},
},
Expand All @@ -238,7 +238,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN(testdata.Cathy.URN.String() + "?foo=bar")},
testdata.Cathy.ID, // only URN identity is considered
false,
[]urns.URN{"tel:+16055741111?id=10000&priority=1000"},
[]urns.URN{"tel:+16055741111?id=10000"},
models.NilChannelID,
[]assets.GroupUUID{testdata.DoctorsGroup.UUID},
},
Expand All @@ -247,7 +247,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:100001")},
newContact(), // creates new contact
true,
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002&priority=1000"},
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002"},
testdata.TwilioChannel.ID,
[]assets.GroupUUID{"dcc16d85-8274-4d19-a3c2-152d4ee99380"},
},
Expand All @@ -256,7 +256,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:100001")},
prevContact(), // returns the same created contact
false,
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002&priority=1000"},
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002"},
models.NilChannelID,
[]assets.GroupUUID{"dcc16d85-8274-4d19-a3c2-152d4ee99380"},
},
Expand All @@ -265,7 +265,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:100001"), urns.URN("telegram:100002")},
prevContact(), // same again as other URNs don't exist
false,
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002&priority=1000"},
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002"},
models.NilChannelID,
[]assets.GroupUUID{"dcc16d85-8274-4d19-a3c2-152d4ee99380"},
},
Expand All @@ -274,7 +274,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:100002"), urns.URN("telegram:100001")},
prevContact(), // same again as other URNs don't exist
false,
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002&priority=1000"},
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002"},
models.NilChannelID,
[]assets.GroupUUID{"dcc16d85-8274-4d19-a3c2-152d4ee99380"},
},
Expand All @@ -283,7 +283,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:200001"), urns.URN("telegram:100001")},
prevContact(), // same again as other URNs are orphaned
false,
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002&priority=1000"},
[]urns.URN{"telegram:100001?channel=74729f45-7f29-4868-9dc4-90e491e3c7d8&id=30002"},
models.NilChannelID,
[]assets.GroupUUID{"dcc16d85-8274-4d19-a3c2-152d4ee99380"},
},
Expand All @@ -292,7 +292,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:100003"), urns.URN("telegram:100004")}, // 2 new URNs
newContact(),
true,
[]urns.URN{"telegram:100003?id=30003&priority=1000", "telegram:100004?id=30004&priority=999"},
[]urns.URN{"telegram:100003?id=30003", "telegram:100004?id=30004"},
models.NilChannelID,
[]assets.GroupUUID{},
},
Expand All @@ -301,7 +301,7 @@ func TestGetOrCreateContact(t *testing.T) {
[]urns.URN{urns.URN("telegram:100005"), urns.URN("telegram:200002")}, // 1 new, 1 orphaned
newContact(),
true,
[]urns.URN{"telegram:100005?id=30005&priority=1000", "telegram:200002?id=30001&priority=999"},
[]urns.URN{"telegram:100005?id=30005", "telegram:200002?id=30001"},
models.NilChannelID,
[]assets.GroupUUID{},
},
Expand Down
4 changes: 2 additions & 2 deletions core/models/msgs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,15 +513,15 @@ func TestNewMsgOut(t *testing.T) {

out, ch := models.NewMsgOut(oa, cathy, "hello", nil, nil, `eng-US`)
assert.Equal(t, "hello", out.Text())
assert.Equal(t, urns.URN("tel:+16055741111?id=10000&priority=1000"), out.URN())
assert.Equal(t, urns.URN("tel:+16055741111?id=10000"), out.URN())
assert.Equal(t, assets.NewChannelReference("74729f45-7f29-4868-9dc4-90e491e3c7d8", "Twilio"), out.Channel())
assert.Equal(t, i18n.Locale(`eng-US`), out.Locale())
assert.Equal(t, "Twilio", ch.Name())

cathy.SetStatus(flows.ContactStatusBlocked)

out, ch = models.NewMsgOut(oa, cathy, "hello", nil, nil, `eng-US`)
assert.Equal(t, urns.URN("tel:+16055741111?id=10000&priority=1000"), out.URN())
assert.Equal(t, urns.URN("tel:+16055741111?id=10000"), out.URN())
assert.Equal(t, assets.NewChannelReference("74729f45-7f29-4868-9dc4-90e491e3c7d8", "Twilio"), out.Channel())
assert.Equal(t, "Twilio", ch.Name())
assert.Equal(t, flows.UnsendableReasonContactStatus, out.UnsendableReason())
Expand Down
Loading

0 comments on commit dc13ded

Please sign in to comment.