feat: close the last silent-success window on the segment pin: the in-transaction segment_members insert now compares RowsAffected against the deduplicated segment list and rolls the create back with a 400 when a segment was deleted between the service's existence check and the write, plus a live repository test that a contact created with a segment is pinned as an include override while a sibling naming none stays out, and that a create naming a segment the organization does not have fails and leaves no contact behind

This commit is contained in:
Matthew Meszaros
2026-09-01 03:15:08 -07:00
parent 103145e970
commit c2b0f93583
2 changed files with 67 additions and 2 deletions
+10 -2
View File
@@ -512,16 +512,24 @@ func (r *contactRepository) Add(ctx context.Context, userID string, orgID uuid.U
if len(segs) == 0 {
continue
}
if _, err := tx.Exec(ctx, `
tag, err := tx.Exec(ctx, `
INSERT INTO segment_members (segment_id, contact_id, mode)
SELECT s.id, $1, 'include'
FROM segments s
WHERE s.id = ANY($2) AND s.organization_id = $3
ON CONFLICT (segment_id, contact_id) DO UPDATE SET mode = EXCLUDED.mode, created_at = now()
`, ncontacts[i].ID, segs, orgID); err != nil {
`, ncontacts[i].ID, segs, orgID)
if err != nil {
db.CaptureError(err, "", nil, "segment_members insert")
return nil, errx.InternalError()
}
// One row per requested segment, since `segs` is deduplicated and a
// conflicting row still counts as affected. Fewer means a segment was
// deleted between the service's check and this statement, so the
// create is rolled back rather than answered without the membership.
if int(tag.RowsAffected()) != len(segs) {
return nil, errx.New(errx.BadRequest, "a selected segment does not exist")
}
}
if err := logContactCreated(ctx, tx, orgID, actor, createdIDs); err != nil {
+57
View File
@@ -345,3 +345,60 @@ func TestLiveSegmentCampaignLinks(t *testing.T) {
t.Errorf("leads after unlink = %d, want 3", got)
}
}
// Issue #285: a contact created inside a segment joins it, and the include
// override is written in the same transaction as the contact. A segment that
// vanishes between the service's existence check and this insert must roll the
// whole create back rather than answer with a membership that never happened.
func TestLiveContactAddPinsSegments(t *testing.T) {
f, repo := newSegmentFixture(t)
ctx := context.Background()
handle, pool := liveContactDB(t)
contacts := NewContactRepostory(handle)
acme, xerr := repo.Create(ctx, f.org, &f.owner, &models.Segment{
Name: "Pin target", Color: "#0284c7", Match: models.SegmentMatchAll,
Conditions: []models.SegmentCondition{{Field: "company", Operator: "equals", Value: "acme"}},
})
if xerr != nil {
t.Fatalf("create segment: %v", xerr)
}
// A company the conditions do not match, so membership can only come from
// the pin. The second contact names no segment and must stay out.
tag := uuid.New().String()[:6]
made, xerr := contacts.Add(ctx, f.owner.String(), f.org, []models.AddContact{
{Email: "pinned-" + tag + "@initech.test", FirstName: "Pinned", Company: "initech", Segments: []string{acme.ID.String(), acme.ID.String()}},
{Email: "loose-" + tag + "@initech.test", FirstName: "Loose", Company: "initech"},
})
if xerr != nil || len(made) != 2 {
t.Fatalf("add: %+v, %v", made, xerr)
}
modes, xerr := repo.MemberModes(ctx, acme.ID, []uuid.UUID{made[0].ID, made[1].ID})
if xerr != nil {
t.Fatalf("modes: %v", xerr)
}
if modes[made[0].ID] != models.SegmentMemberInclude {
t.Errorf("pinned contact mode = %q, want include", modes[made[0].ID])
}
if _, ok := modes[made[1].ID]; ok {
t.Errorf("contact that named no segment was pinned")
}
// A segment id that is not in the organization (the state the race leaves
// behind) fails the create instead of dropping the membership silently.
gone := uuid.New().String()
email := "vanished-" + tag + "@initech.test"
if _, xerr := contacts.Add(ctx, f.owner.String(), f.org, []models.AddContact{
{Email: email, FirstName: "Vanished", Company: "initech", Segments: []string{gone}},
}); xerr == nil {
t.Fatalf("create with a vanished segment succeeded")
}
var stranded int
if err := pool.QueryRow(ctx, `SELECT COUNT(*) FROM contacts WHERE organization_id = $1 AND email = $2`, f.org, email).Scan(&stranded); err != nil {
t.Fatalf("count: %v", err)
}
if stranded != 0 {
t.Errorf("rolled-back create left %d contacts behind", stranded)
}
}