From f5b97f3875cfa0b2253944b466c9f70ab953a7e6 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:10 -0700 Subject: [PATCH 01/11] feat: resolve a contact import's segment targets through one shared path that a recurring source can ask to be lenient about, expose it as ValidateSegmentTargets so a saved sheet-sync source is refused when it is written rather than on its next run, and report segments_pinned on the import result so a caller never claims a membership write that did not land --- internal/app/contact/import.go | 28 +++++++++++++++++++++++++--- internal/app/contact/service.go | 4 ++++ internal/models/contact_import.go | 10 ++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/internal/app/contact/import.go b/internal/app/contact/import.go index 436ee472..b59f48a3 100644 --- a/internal/app/contact/import.go +++ b/internal/app/contact/import.go @@ -224,7 +224,7 @@ func (s *contactService) ImportCommit( } // Segment targets are resolved up front: each must exist in the org. // Membership is written after the rows exist, as an include override. - segmentIDs, xerr := s.parseSegmentIDs(ctx, orgID, opts.SegmentIDs) + segmentIDs, xerr := s.resolveSegmentIDs(ctx, orgID, opts.SegmentIDs, opts.SkipMissingSegments) if xerr != nil { return nil, xerr } @@ -542,10 +542,13 @@ func (s *contactService) ImportCommit( } // Segment membership last, once every touched row exists. A failed write - // is a note, not a failed import: the contacts themselves are in. - if len(touched) > 0 { + // is a note, not a failed import: the contacts themselves are in, and the + // result says the pin did not land so the UI does not claim it did. + if len(segmentIDs) > 0 && len(touched) > 0 { + res.SegmentsPinned = true for _, segID := range segmentIDs { if _, xerr := s.segmentLinker.SetMembers(ctx, orgID, segID, touched, models.SegmentMemberInclude); xerr != nil { + res.SegmentsPinned = false warn(0, "", nil, "imported contacts could not be added to a segment: "+xerr.Message) } } @@ -564,9 +567,22 @@ func (s *contactService) ImportCommit( return res, nil } +// ValidateSegmentTargets exposes resolveSegmentIDs' verdict without running an +// import, so a saved Google Sheets source is rejected at save time. +func (s *contactService) ValidateSegmentTargets(ctx context.Context, orgID uuid.UUID, ids []string) *errx.Error { + _, xerr := s.resolveSegmentIDs(ctx, orgID, ids, false) + return xerr +} + // parseSegmentIDs validates the import's target segments: well-formed ids // that exist in the org, deduplicated. func (s *contactService) parseSegmentIDs(ctx context.Context, orgID uuid.UUID, raw []string) ([]uuid.UUID, *errx.Error) { + return s.resolveSegmentIDs(ctx, orgID, raw, false) +} + +// resolveSegmentIDs is parseSegmentIDs with the recurring-source relaxation: +// skipMissing drops an id whose segment is gone instead of failing the run. +func (s *contactService) resolveSegmentIDs(ctx context.Context, orgID uuid.UUID, raw []string, skipMissing bool) ([]uuid.UUID, *errx.Error) { if len(raw) == 0 { return nil, nil } @@ -578,6 +594,9 @@ func (s *contactService) parseSegmentIDs(ctx context.Context, orgID uuid.UUID, r for _, r := range raw { id, err := uuid.Parse(strings.TrimSpace(r)) if err != nil { + if skipMissing { + continue + } return nil, errx.New(errx.BadRequest, "invalid segment id") } if seen[id] { @@ -586,6 +605,9 @@ func (s *contactService) parseSegmentIDs(ctx context.Context, orgID uuid.UUID, r seen[id] = true if _, xerr := s.segmentLinker.Get(ctx, orgID, id); xerr != nil { if xerr.Code == errx.NotFound { + if skipMissing { + continue + } return nil, errx.New(errx.BadRequest, "a selected segment does not exist") } return nil, xerr diff --git a/internal/app/contact/service.go b/internal/app/contact/service.go index e634db57..7bf62c4e 100644 --- a/internal/app/contact/service.go +++ b/internal/app/contact/service.go @@ -40,6 +40,10 @@ type ContactService interface { // that persist a mapping for later (the Google Sheets sync sources) use // it so a bad mapping is caught when it is saved, not on the next sync. ValidateImportMapping(mapping []models.ContactImportColumnMapping) *errx.Error + // ValidateSegmentTargets reports whether every id names a segment in the + // organization, so a saved source is refused when it is written rather + // than on its next run. + ValidateSegmentTargets(ctx context.Context, orgID uuid.UUID, ids []string) *errx.Error // ImportCommit re-parses the uploaded file with the chosen mapping // and performs the upsert / skip / dedup work. Returns per-row diff --git a/internal/models/contact_import.go b/internal/models/contact_import.go index 10103aad..909cae65 100644 --- a/internal/models/contact_import.go +++ b/internal/models/contact_import.go @@ -109,6 +109,11 @@ type ContactImportCommit struct { // SegmentIDs pins every imported row into these segments as a manual // include override, the same write the "Add to segment" bulk action does. SegmentIDs []string `json:"segment_ids,omitempty"` + // SkipMissingSegments drops a target segment that no longer exists instead + // of refusing the import. Set by saved recurring sources (the Google Sheets + // sync), where a segment deleted months later must not stop every run; an + // interactive import still gets a 400 on a segment the user just picked. + SkipMissingSegments bool `json:"-"` // SubscribedDefault is what new contacts inherit when no // subscribed column was mapped. Defaults to true server-side. @@ -151,6 +156,11 @@ type ContactImportResult struct { // "showing the first N of M" instead of implying it listed everything. ErrorsTruncated bool `json:"errors_truncated,omitempty"` + // SegmentsPinned is true when the import had segment targets and every + // membership write landed. False with targets set means the reason is in + // Errors, so the UI never claims a pin that did not happen. + SegmentsPinned bool `json:"segments_pinned,omitempty"` + // Quality is what the uploaded addresses look like, measured at import. // Advisory: a bad list is reported here and stopped at launch, never // refused here, because these are the customer's own records. From 2818cfa9383695926a79b0c8a0f8b7b40c0c1bc9 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:10 -0700 Subject: [PATCH 02/11] feat: encode a segment's empty condition list as [] rather than null in segmentRepository Create and Update, so creating the condition-less segment a static imported list wants no longer trips the jsonb array CHECK and answers 500 --- internal/repository/pg_segment.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/internal/repository/pg_segment.go b/internal/repository/pg_segment.go index 214e819e..cdf1cf9e 100644 --- a/internal/repository/pg_segment.go +++ b/internal/repository/pg_segment.go @@ -127,6 +127,19 @@ func (r *segmentRepository) Get(ctx context.Context, orgID, id uuid.UUID) (*mode return s, nil } +// marshalConditions encodes a condition list for the jsonb column. A nil slice +// encodes as `null`, which the array CHECK rejects, so it becomes an empty list. +func marshalConditions(conds []models.SegmentCondition) []byte { + if len(conds) == 0 { + return []byte("[]") + } + b, err := json.Marshal(conds) + if err != nil { + return []byte("[]") + } + return b +} + func (r *segmentRepository) Create(ctx context.Context, orgID uuid.UUID, createdBy *uuid.UUID, seg *models.Segment) (*models.Segment, *errx.Error) { var total int if err := r.DB.QueryRow(ctx, `SELECT COUNT(*) FROM segments WHERE organization_id = $1`, orgID).Scan(&total); err != nil { @@ -136,7 +149,7 @@ func (r *segmentRepository) Create(ctx context.Context, orgID uuid.UUID, created if total >= models.SegmentsPerOrgMax { return nil, errx.New(errx.BadRequest, fmt.Sprintf("a workspace can have at most %d segments", models.SegmentsPerOrgMax)) } - conds, _ := json.Marshal(seg.Conditions) + conds := marshalConditions(seg.Conditions) var id uuid.UUID err := r.DB.QueryRow(ctx, ` INSERT INTO segments (organization_id, created_by, name, description, color, match, conditions) @@ -153,7 +166,7 @@ func (r *segmentRepository) Create(ctx context.Context, orgID uuid.UUID, created } func (r *segmentRepository) Update(ctx context.Context, orgID uuid.UUID, seg *models.Segment) (*models.Segment, *errx.Error) { - conds, _ := json.Marshal(seg.Conditions) + conds := marshalConditions(seg.Conditions) tag, err := r.DB.Exec(ctx, ` UPDATE segments SET name = $3, description = $4, color = $5, match = $6, conditions = $7, updated_at = now() WHERE organization_id = $1 AND id = $2`, orgID, seg.ID, seg.Name, seg.Description, seg.Color, seg.Match, conds) From 6284eb27ec694f23cf9afb7673d91f41c1015261 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:18 -0700 Subject: [PATCH 03/11] feat: give a saved Google Sheets sync source segment targets (migration 000136 adds lead_sync_sources.segment_ids) that pin every synced row into those segments on each run, validated against the organization when the source is written and dropped from the run when a segment is deleted later, and filter GET /lead-sync/sources by ?segment_id= through a jsonb containment test so a segment can list the sources feeding it --- internal/api/handler/lead_sync.go | 13 +++++- internal/app/leadsync/service.go | 40 ++++++++++++----- .../000136_lead_sync_segments.down.sql | 2 + .../000136_lead_sync_segments.up.sql | 10 +++++ internal/models/lead_sync.go | 12 ++++-- internal/repository/pg_lead_sync.go | 43 +++++++++++++------ 6 files changed, 91 insertions(+), 29 deletions(-) create mode 100644 internal/infrastructure/db/migrations/000136_lead_sync_segments.down.sql create mode 100644 internal/infrastructure/db/migrations/000136_lead_sync_segments.up.sql diff --git a/internal/api/handler/lead_sync.go b/internal/api/handler/lead_sync.go index c7a28627..b664e3e5 100644 --- a/internal/api/handler/lead_sync.go +++ b/internal/api/handler/lead_sync.go @@ -133,7 +133,7 @@ func (h *Handler) PreviewLeadSync(c *gin.Context) { } // ListLeadSyncSources lists this org's saved sources, optionally filtered to a -// campaign via ?campaign_id=. +// campaign via ?campaign_id= or to a segment via ?segment_id=. func (h *Handler) ListLeadSyncSources(c *gin.Context) { orgID, ok := requireOrgID(c) if !ok { @@ -148,7 +148,16 @@ func (h *Handler) ListLeadSyncSources(c *gin.Context) { } campaignID = &id } - sources, err := h.LeadSyncService.List(c.Request.Context(), orgID, campaignID) + var segmentID *uuid.UUID + if raw := strings.TrimSpace(c.Query("segment_id")); raw != "" { + id, err := uuid.Parse(raw) + if err != nil { + errx.JSON(c, errx.New(errx.BadRequest, "invalid segment_id")) + return + } + segmentID = &id + } + sources, err := h.LeadSyncService.List(c.Request.Context(), orgID, campaignID, segmentID) if err != nil { errx.JSON(c, errx.New(errx.Internal, "failed to list sync sources")) return diff --git a/internal/app/leadsync/service.go b/internal/app/leadsync/service.go index 410f846a..23303128 100644 --- a/internal/app/leadsync/service.go +++ b/internal/app/leadsync/service.go @@ -47,7 +47,7 @@ type Service interface { Preview(ctx context.Context, orgID, connID uuid.UUID, sheetID, tabTitle string) (*models.ContactImportPreview, *errx.Error) // Source CRUD. - List(ctx context.Context, orgID uuid.UUID, campaignID *uuid.UUID) ([]models.LeadSyncSource, error) + List(ctx context.Context, orgID uuid.UUID, campaignID, segmentID *uuid.UUID) ([]models.LeadSyncSource, error) Get(ctx context.Context, orgID, id uuid.UUID) (*models.LeadSyncSource, *errx.Error) Create(ctx context.Context, orgID, userID uuid.UUID, in *models.CreateLeadSyncSource) (*models.LeadSyncSource, *errx.Error) Update(ctx context.Context, orgID, id uuid.UUID, in *models.UpdateLeadSyncSource) (*models.LeadSyncSource, *errx.Error) @@ -114,8 +114,8 @@ func (s *service) Preview(ctx context.Context, orgID, connID uuid.UUID, sheetID, }, nil } -func (s *service) List(ctx context.Context, orgID uuid.UUID, campaignID *uuid.UUID) ([]models.LeadSyncSource, error) { - return s.repo.List(ctx, orgID, campaignID) +func (s *service) List(ctx context.Context, orgID uuid.UUID, campaignID, segmentID *uuid.UUID) ([]models.LeadSyncSource, error) { + return s.repo.List(ctx, orgID, campaignID, segmentID) } func (s *service) Get(ctx context.Context, orgID, id uuid.UUID) (*models.LeadSyncSource, *errx.Error) { @@ -166,6 +166,13 @@ func (s *service) Create(ctx context.Context, orgID, userID uuid.UUID, in *model if cats == nil { cats = []string{} } + segs := in.SegmentIDs + if segs == nil { + segs = []string{} + } + if xerr := s.contacts.ValidateSegmentTargets(ctx, orgID, segs); xerr != nil { + return nil, xerr + } src := &models.LeadSyncSource{ OrganizationID: orgID, @@ -181,6 +188,7 @@ func (s *service) Create(ctx context.Context, orgID, userID uuid.UUID, in *model Dedup: in.Dedup, TargetCampaignID: in.TargetCampaignID, CategoryIDs: cats, + SegmentIDs: segs, SubscribedDefault: subscribed, Label: strings.TrimSpace(in.Label), Status: models.LeadSyncStatusIdle, @@ -240,6 +248,12 @@ func (s *service) Update(ctx context.Context, orgID, id uuid.UUID, in *models.Up if in.CategoryIDs != nil { src.CategoryIDs = *in.CategoryIDs } + if in.SegmentIDs != nil { + if xerr := s.contacts.ValidateSegmentTargets(ctx, orgID, *in.SegmentIDs); xerr != nil { + return nil, xerr + } + src.SegmentIDs = *in.SegmentIDs + } if in.SubscribedDefault != nil { src.SubscribedDefault = *in.SubscribedDefault } @@ -297,14 +311,18 @@ func (s *service) SyncNow(ctx context.Context, triggeringUserID, orgID, sourceID } subscribed := src.SubscribedDefault opts := &models.ContactImportCommit{ - Mapping: src.ColumnMapping, - Dedup: src.Dedup, - HasHeader: src.HasHeader, - CategoryIDs: src.CategoryIDs, - CampaignIDs: campaignIDs, - SubscribedDefault: &subscribed, - Source: models.ContactSourceSheetSync, - SourceDetail: src.SheetTitle, + Mapping: src.ColumnMapping, + Dedup: src.Dedup, + HasHeader: src.HasHeader, + CategoryIDs: src.CategoryIDs, + CampaignIDs: campaignIDs, + SegmentIDs: src.SegmentIDs, + // A segment deleted after the source was saved is dropped from the run; + // a recurring sync must not stop importing because a target is gone. + SkipMissingSegments: true, + SubscribedDefault: &subscribed, + Source: models.ContactSourceSheetSync, + SourceDetail: src.SheetTitle, } result, ierr := s.contacts.ImportCommit(ctx, triggeringUserID.String(), orgID, bytes.NewReader(csvBytes), "google-sheets-sync.csv", opts) diff --git a/internal/infrastructure/db/migrations/000136_lead_sync_segments.down.sql b/internal/infrastructure/db/migrations/000136_lead_sync_segments.down.sql new file mode 100644 index 00000000..a8248921 --- /dev/null +++ b/internal/infrastructure/db/migrations/000136_lead_sync_segments.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE lead_sync_sources + DROP COLUMN IF EXISTS segment_ids; diff --git a/internal/infrastructure/db/migrations/000136_lead_sync_segments.up.sql b/internal/infrastructure/db/migrations/000136_lead_sync_segments.up.sql new file mode 100644 index 00000000..f6bd30f0 --- /dev/null +++ b/internal/infrastructure/db/migrations/000136_lead_sync_segments.up.sql @@ -0,0 +1,10 @@ +-- Segment targets for a saved Google Sheets sync source. +-- +-- A file import can pin every row into a segment (ContactImportCommit.segment_ids); +-- a sheet sync committed through the same importer could not, so a sync created +-- from a segment's member list produced contacts that were nowhere in it. +-- jsonb for the same reason category_ids is: it is a list handed straight to +-- the importer, never filtered in SQL beyond a containment lookup. + +ALTER TABLE lead_sync_sources + ADD COLUMN segment_ids jsonb NOT NULL DEFAULT '[]'; diff --git a/internal/models/lead_sync.go b/internal/models/lead_sync.go index ce92c7d2..49376ee0 100644 --- a/internal/models/lead_sync.go +++ b/internal/models/lead_sync.go @@ -48,9 +48,13 @@ type LeadSyncSource struct { // TargetCampaignID, when set, enrols every new/updated lead into that // campaign on each sync. - TargetCampaignID *uuid.UUID `json:"target_campaign_id,omitempty"` - CategoryIDs []string `json:"category_ids"` - SubscribedDefault bool `json:"subscribed_default"` + TargetCampaignID *uuid.UUID `json:"target_campaign_id,omitempty"` + CategoryIDs []string `json:"category_ids"` + // SegmentIDs pins every synced row into these segments as an include + // override, the same write the file import's segment targets do. A segment + // deleted later is dropped from the run rather than failing it. + SegmentIDs []string `json:"segment_ids"` + SubscribedDefault bool `json:"subscribed_default"` Label string `json:"label,omitempty"` Status LeadSyncStatus `json:"status"` @@ -75,6 +79,7 @@ type CreateLeadSyncSource struct { Dedup ContactImportDedupStrategy `json:"dedup"` TargetCampaignID *uuid.UUID `json:"target_campaign_id"` CategoryIDs []string `json:"category_ids"` + SegmentIDs []string `json:"segment_ids"` SubscribedDefault *bool `json:"subscribed_default"` Label string `json:"label"` } @@ -92,6 +97,7 @@ type UpdateLeadSyncSource struct { TargetCampaignID *uuid.UUID `json:"target_campaign_id"` ClearCampaign bool `json:"clear_campaign"` CategoryIDs *[]string `json:"category_ids"` + SegmentIDs *[]string `json:"segment_ids"` SubscribedDefault *bool `json:"subscribed_default"` Label *string `json:"label"` } diff --git a/internal/repository/pg_lead_sync.go b/internal/repository/pg_lead_sync.go index 69db1343..459e68ce 100644 --- a/internal/repository/pg_lead_sync.go +++ b/internal/repository/pg_lead_sync.go @@ -16,7 +16,9 @@ import ( // reachable by the org that created it. type LeadSyncRepository interface { Create(ctx context.Context, src *models.LeadSyncSource) error - List(ctx context.Context, orgID uuid.UUID, campaignID *uuid.UUID) ([]models.LeadSyncSource, error) + // List filters to the sources feeding one campaign and/or one segment; + // both nil lists every source in the organization. + List(ctx context.Context, orgID uuid.UUID, campaignID, segmentID *uuid.UUID) ([]models.LeadSyncSource, error) Get(ctx context.Context, orgID, id uuid.UUID) (*models.LeadSyncSource, error) Update(ctx context.Context, src *models.LeadSyncSource) error Delete(ctx context.Context, orgID, id uuid.UUID) error @@ -36,7 +38,7 @@ func NewLeadSyncRepository(db *pgxpool.Pool) LeadSyncRepository { const leadSyncCols = ` id, organization_id, created_by_user_id, provider, connection_id, sheet_id, COALESCE(sheet_title, ''), COALESCE(tab_title, ''), COALESCE(a1_range, ''), - has_header, column_mapping, dedup, target_campaign_id, category_ids, + has_header, column_mapping, dedup, target_campaign_id, category_ids, segment_ids, subscribed_default, COALESCE(label, ''), status, last_synced_at, last_result, COALESCE(last_error, ''), created_at, updated_at` @@ -56,32 +58,41 @@ func (r *leadSyncRepository) Create(ctx context.Context, src *models.LeadSyncSou mapping := marshalJSONDefault(src.ColumnMapping, "[]") cats := marshalJSONDefault(src.CategoryIDs, "[]") + segs := marshalJSONDefault(src.SegmentIDs, "[]") _, err := r.db.Exec(ctx, ` INSERT INTO lead_sync_sources ( id, organization_id, created_by_user_id, provider, connection_id, sheet_id, sheet_title, tab_title, a1_range, has_header, - column_mapping, dedup, target_campaign_id, category_ids, + column_mapping, dedup, target_campaign_id, category_ids, segment_ids, subscribed_default, label, status, created_at, updated_at ) VALUES ( $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, - $11, $12, $13, $14, - $15, $16, $17, $18, $18 + $11, $12, $13, $14, $15, + $16, $17, $18, $19, $19 )`, src.ID, src.OrganizationID, src.CreatedByUserID, src.Provider, src.ConnectionID, src.SheetID, nullIfEmptyStr(src.SheetTitle), nullIfEmptyStr(src.TabTitle), nullIfEmptyStr(src.A1Range), src.HasHeader, - mapping, string(src.Dedup), src.TargetCampaignID, cats, + mapping, string(src.Dedup), src.TargetCampaignID, cats, segs, src.SubscribedDefault, nullIfEmptyStr(src.Label), string(src.Status), now, ) return err } -func (r *leadSyncRepository) List(ctx context.Context, orgID uuid.UUID, campaignID *uuid.UUID) ([]models.LeadSyncSource, error) { +func (r *leadSyncRepository) List(ctx context.Context, orgID uuid.UUID, campaignID, segmentID *uuid.UUID) ([]models.LeadSyncSource, error) { + // The segment filter is a jsonb containment test against the id list the + // importer reads, so it needs no extra column. + var wantSegment []byte + if segmentID != nil { + wantSegment = []byte(`["` + segmentID.String() + `"]`) + } rows, err := r.db.Query(ctx, `SELECT `+leadSyncCols+` FROM lead_sync_sources - WHERE organization_id = $1 AND ($2::uuid IS NULL OR target_campaign_id = $2) - ORDER BY created_at DESC`, orgID, campaignID) + WHERE organization_id = $1 + AND ($2::uuid IS NULL OR target_campaign_id = $2) + AND ($3::jsonb IS NULL OR segment_ids @> $3::jsonb) + ORDER BY created_at DESC`, orgID, campaignID, wantSegment) if err != nil { return nil, err } @@ -116,16 +127,17 @@ func (r *leadSyncRepository) Update(ctx context.Context, src *models.LeadSyncSou src.UpdatedAt = now mapping := marshalJSONDefault(src.ColumnMapping, "[]") cats := marshalJSONDefault(src.CategoryIDs, "[]") + segs := marshalJSONDefault(src.SegmentIDs, "[]") _, err := r.db.Exec(ctx, ` UPDATE lead_sync_sources SET sheet_id = $1, sheet_title = $2, tab_title = $3, a1_range = $4, has_header = $5, column_mapping = $6, dedup = $7, target_campaign_id = $8, - category_ids = $9, subscribed_default = $10, label = $11, updated_at = $12 - WHERE organization_id = $13 AND id = $14`, + category_ids = $9, segment_ids = $10, subscribed_default = $11, label = $12, updated_at = $13 + WHERE organization_id = $14 AND id = $15`, src.SheetID, nullIfEmptyStr(src.SheetTitle), nullIfEmptyStr(src.TabTitle), nullIfEmptyStr(src.A1Range), src.HasHeader, mapping, string(src.Dedup), src.TargetCampaignID, - cats, src.SubscribedDefault, nullIfEmptyStr(src.Label), now, + cats, segs, src.SubscribedDefault, nullIfEmptyStr(src.Label), now, src.OrganizationID, src.ID, ) return err @@ -165,6 +177,7 @@ func scanLeadSyncInto(row scanner, s *models.LeadSyncSource) error { var ( mapping []byte cats []byte + segs []byte lastResult []byte status string dedup string @@ -172,7 +185,7 @@ func scanLeadSyncInto(row scanner, s *models.LeadSyncSource) error { if err := row.Scan( &s.ID, &s.OrganizationID, &s.CreatedByUserID, &s.Provider, &s.ConnectionID, &s.SheetID, &s.SheetTitle, &s.TabTitle, &s.A1Range, - &s.HasHeader, &mapping, &dedup, &s.TargetCampaignID, &cats, + &s.HasHeader, &mapping, &dedup, &s.TargetCampaignID, &cats, &segs, &s.SubscribedDefault, &s.Label, &status, &s.LastSyncedAt, &lastResult, &s.LastError, &s.CreatedAt, &s.UpdatedAt, ); err != nil { @@ -189,6 +202,10 @@ func scanLeadSyncInto(row scanner, s *models.LeadSyncSource) error { if len(cats) > 0 { _ = json.Unmarshal(cats, &s.CategoryIDs) } + s.SegmentIDs = []string{} + if len(segs) > 0 { + _ = json.Unmarshal(segs, &s.SegmentIDs) + } if len(lastResult) > 0 { var res models.ContactImportResult if err := json.Unmarshal(lastResult, &res); err == nil { From c9781a1c578d24a4dd20915dcc88c377370d105a Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:18 -0700 Subject: [PATCH 04/11] feat: surface the sheet-sync segment targets in the dashboard: an Add to segments picker on the sync wizard's options step and in the source edit drawer, a lockedSegment the wizard applies on every run and shows as a fixed row, and a SyncSourcesPanel that scopes its list and its New sync to the segment it was opened from --- .../app/contacts/SheetSyncWizard.tsx | 51 ++++++++++++++++++- .../app/contacts/SyncSourceEditDrawer.tsx | 16 +++++- .../app/contacts/SyncSourcesPanel.tsx | 28 ++++++---- .../api/client/app/leadsync/listSources.ts | 16 ++++-- .../hooks/app/leadsync/useLeadSyncSources.ts | 11 ++-- .../lib/api/models/app/leadsync/LeadSync.ts | 4 ++ 6 files changed, 103 insertions(+), 23 deletions(-) diff --git a/web/src/components/app/contacts/SheetSyncWizard.tsx b/web/src/components/app/contacts/SheetSyncWizard.tsx index f3ba46a0..c5d2ddaa 100644 --- a/web/src/components/app/contacts/SheetSyncWizard.tsx +++ b/web/src/components/app/contacts/SheetSyncWizard.tsx @@ -10,7 +10,8 @@ // EXISTING integration OAuth popup (provider "google_sheets"). // 2. sheet — paste a Sheet ID, fetch its tabs, pick a tab. // 3. map — preview first rows + map columns (reused MapStep). -// 4. options — dedup strategy, optional target campaign, optional categories. +// 4. options — dedup strategy, optional target campaign, optional categories, +// optional target segments. // 5. save — POST /lead-sync/sources, then optionally Sync now → result. import React from "react"; @@ -47,6 +48,7 @@ import { } from "@/lib/api/hooks/app/integrations/useIntegrationOAuth"; import { openOAuthPopup } from "@/lib/integrations/oauthPopup"; import CampaignPicker from "@/components/app/campaigns/CampaignPicker"; +import { SegmentMultiPicker } from "@/components/app/segments/SegmentPickers"; import useGoogleConnection from "@/lib/api/hooks/app/leadsync/useGoogleConnection"; import { useGetSpreadsheet, @@ -72,13 +74,16 @@ interface Props { // When set, the source is pre-targeted to this campaign and the campaign // picker is hidden — used by the per-campaign "Connect a Google Sheet". lockedCampaign?: { id: string; name: string }; + // When set (a segment's member list), every synced row is pinned into this + // segment on each run, the same way the file importer's target works. + lockedSegment?: { id: string; name: string; color?: string }; // Notified after a source is saved so callers can refresh their list. onSaved?: (source: LeadSyncSource) => void; } const STEP_ORDER: Step[] = ["connect", "sheet", "map", "options", "result"]; -export default function SheetSyncWizard({ open, onClose, lockedCampaign, onSaved }: Props) { +export default function SheetSyncWizard({ open, onClose, lockedCampaign, lockedSegment, onSaved }: Props) { const connection = useGoogleConnection(); const connectionId = connection.data?.connection?.id ?? null; const connected = !!connection.data?.connected && !!connectionId; @@ -94,6 +99,7 @@ export default function SheetSyncWizard({ open, onClose, lockedCampaign, onSaved const [campaignId, setCampaignId] = React.useState(lockedCampaign?.id ?? null); const [campaignName, setCampaignName] = React.useState(lockedCampaign?.name ?? ""); const [categoryIds, setCategoryIds] = React.useState([]); + const [segmentIds, setSegmentIds] = React.useState([]); const [label, setLabel] = React.useState(""); const [result, setResult] = React.useState(null); const [busy, setBusy] = React.useState(false); @@ -118,6 +124,7 @@ export default function SheetSyncWizard({ open, onClose, lockedCampaign, onSaved setCampaignId(lockedCampaign?.id ?? null); setCampaignName(lockedCampaign?.name ?? ""); setCategoryIds([]); + setSegmentIds([]); setLabel(""); setResult(null); setBusy(false); @@ -203,6 +210,12 @@ export default function SheetSyncWizard({ open, onClose, lockedCampaign, onSaved // field name is rejected by the API, so catch it on the mapping screen. const mapProblem = mappingProblem(mapping); + // The segment the wizard was opened inside always travels with the source. + const targetSegmentIds = React.useMemo( + () => (lockedSegment ? [lockedSegment.id, ...segmentIds.filter((id) => id !== lockedSegment.id)] : segmentIds), + [lockedSegment, segmentIds], + ); + async function save(runSync: boolean) { if (!connectionId || !meta) return; setBusy(true); @@ -217,6 +230,7 @@ export default function SheetSyncWizard({ open, onClose, lockedCampaign, onSaved dedup, target_campaign_id: campaignId ?? undefined, category_ids: categoryIds, + segment_ids: targetSegmentIds, subscribed_default: true, label: label.trim() || meta.title, }); @@ -333,6 +347,9 @@ export default function SheetSyncWizard({ open, onClose, lockedCampaign, onSaved lockedCampaign={lockedCampaign} categoryIds={categoryIds} setCategoryIds={setCategoryIds} + segmentIds={segmentIds} + setSegmentIds={setSegmentIds} + lockedSegment={lockedSegment} /> )} {step === "result" && result && ( @@ -575,6 +592,9 @@ function OptionsStep({ lockedCampaign, categoryIds, setCategoryIds, + segmentIds, + setSegmentIds, + lockedSegment, }: { dedup: ImportDedupStrategy; setDedup: (v: ImportDedupStrategy) => void; @@ -586,6 +606,9 @@ function OptionsStep({ lockedCampaign?: { id: string; name: string }; categoryIds: string[]; setCategoryIds: (v: string[]) => void; + segmentIds: string[]; + setSegmentIds: (v: string[]) => void; + lockedSegment?: { id: string; name: string; color?: string }; }) { return (
@@ -672,6 +695,30 @@ function OptionsStep({

+ +
+

+ Add to segments +

+

+ {lockedSegment + ? "Every synced contact is pinned into this segment on each run, whether or not it matches the segment's conditions. Add more below." + : "Every synced contact is pinned into these segments on each run. A segment linked to a campaign enrols them there automatically."} +

+ {lockedSegment && ( +
+ + {lockedSegment.name} + + Always + +
+ )} + +
); } diff --git a/web/src/components/app/contacts/SyncSourceEditDrawer.tsx b/web/src/components/app/contacts/SyncSourceEditDrawer.tsx index e4618847..1a0b30ed 100644 --- a/web/src/components/app/contacts/SyncSourceEditDrawer.tsx +++ b/web/src/components/app/contacts/SyncSourceEditDrawer.tsx @@ -1,7 +1,8 @@ // SyncSourceEditDrawer — edit a saved sync source's options without re-running // the column mapper. Editing the sheet/tab/mapping is a "make a new source" // operation conceptually, so here we only expose the safe, common edits: -// label, dedup, target campaign (with detach), categories, and the header flag. +// label, dedup, target campaign (with detach), categories, segments, and the +// header flag. // Sheet/tab/mapping are shown read-only for context. import React from "react"; @@ -11,6 +12,7 @@ import toast from "react-hot-toast"; import { DEDUP_OPTIONS, describeError } from "./importShared"; import CategoryPicker from "./CategoryPicker"; +import { SegmentMultiPicker } from "@/components/app/segments/SegmentPickers"; import { Label, TextInput } from "@/components/ui/field"; import { PopoverMenu, @@ -45,6 +47,7 @@ export default function SyncSourceEditDrawer({ source.target_campaign_id ?? null, ); const [categoryIds, setCategoryIds] = React.useState(source.category_ids ?? []); + const [segmentIds, setSegmentIds] = React.useState(source.segment_ids ?? []); const [busy, setBusy] = React.useState(false); const campaignName = @@ -59,6 +62,7 @@ export default function SyncSourceEditDrawer({ dedup, has_header: hasHeader, category_ids: categoryIds, + segment_ids: segmentIds, }; // A nil pointer can't express "clear", so detach explicitly. if (campaignId) { @@ -224,6 +228,16 @@ export default function SyncSourceEditDrawer({ + +
+

+ Add to segments +

+

+ Every synced contact is pinned into these segments on each run. +

+ +
diff --git a/web/src/components/app/contacts/SyncSourcesPanel.tsx b/web/src/components/app/contacts/SyncSourcesPanel.tsx index 544ecdd7..4a0707ad 100644 --- a/web/src/components/app/contacts/SyncSourcesPanel.tsx +++ b/web/src/components/app/contacts/SyncSourcesPanel.tsx @@ -1,10 +1,12 @@ // SyncSourcesPanel — the management surface for on-demand Google-Sheet → leads // "sync sources". Lists saved sources with their last-sync result + status, and // offers per-row Sync now / Edit / Delete plus a "New sync" entry that opens the -// SheetSyncWizard. Works in two placements: -// - global Contacts page (no campaignId): lists every source. -// - per-campaign leads view (campaignId set): lists that campaign's sources +// SheetSyncWizard. Works in three placements: +// - global Contacts page (no scope): lists every source. +// - per-campaign leads view (campaign set): lists that campaign's sources // and pre-targets the wizard to it. +// - a segment's member list (segment set): the same, for the segment every +// synced row is pinned into. // // Rendered as a centered modal mirroring ImportWizard's dialog shell + theme. @@ -36,13 +38,17 @@ export default function SyncSourcesPanel({ open, onClose, campaign, + segment, }: { open: boolean; onClose: () => void; // When set, scopes the list + pre-targets new sources to this campaign. campaign?: { id: string; name: string }; + // When set, the same for a segment: only the sources that feed it, and a + // new source pins every synced row into it. + segment?: { id: string; name: string; color?: string }; }) { - const sources = useLeadSyncSources(campaign?.id); + const sources = useLeadSyncSources(campaign?.id, segment?.id); const deleteSource = useDeleteLeadSyncSource(); const syncSource = useSyncLeadSyncSource(); const confirm = useConfirm(); @@ -108,23 +114,23 @@ export default function SyncSourcesPanel({ Sync sources - {campaign && ( + {(campaign || segment) && ( <>
- {campaign.name} + {campaign?.name ?? segment?.name} )} @@ -178,7 +184,8 @@ export default function SyncSourcesPanel({

Connect a Google Sheet and re-run it on demand to pull new and - updated leads into Warmbly{campaign ? ` and into ${campaign.name}` : ""}. + updated leads into Warmbly + {campaign ? ` and into ${campaign.name}` : segment ? ` and into ${segment.name}` : ""}.

) : ( @@ -213,6 +220,7 @@ export default function SyncSourcesPanel({ open={wizardOpen} onClose={() => setWizardOpen(false)} lockedCampaign={campaign} + lockedSegment={segment} onSaved={() => sources.refetch()} /> {editing && ( diff --git a/web/src/lib/api/client/app/leadsync/listSources.ts b/web/src/lib/api/client/app/leadsync/listSources.ts index 58ce00f6..473cc0d4 100644 --- a/web/src/lib/api/client/app/leadsync/listSources.ts +++ b/web/src/lib/api/client/app/leadsync/listSources.ts @@ -1,14 +1,20 @@ import type { LeadSyncSource } from "@/lib/api/models/app/leadsync/LeadSync"; import Request from "../../Request"; -// Lists this org's saved sync sources, optionally filtered to one campaign -// (powers both the global Contacts > Sync sources area and the per-campaign -// "Connect a Google Sheet" list). -export default async function listSources(campaignId?: string): Promise<{ data: LeadSyncSource[] }> { +// Lists this org's saved sync sources, optionally filtered to one campaign or +// one segment (powers the global Contacts > Sync sources area, the per-campaign +// "Connect a Google Sheet" list and a segment's own sources). +export default async function listSources( + campaignId?: string, + segmentId?: string, +): Promise<{ data: LeadSyncSource[] }> { + const params: Record = {}; + if (campaignId) params.campaign_id = campaignId; + if (segmentId) params.segment_id = segmentId; return await Request<{ data: LeadSyncSource[] }>({ method: "GET", url: "/lead-sync/sources", - params: campaignId ? { campaign_id: campaignId } : undefined, + params: Object.keys(params).length > 0 ? params : undefined, authorization: true, }); } diff --git a/web/src/lib/api/hooks/app/leadsync/useLeadSyncSources.ts b/web/src/lib/api/hooks/app/leadsync/useLeadSyncSources.ts index 9f003713..2432f63c 100644 --- a/web/src/lib/api/hooks/app/leadsync/useLeadSyncSources.ts +++ b/web/src/lib/api/hooks/app/leadsync/useLeadSyncSources.ts @@ -1,12 +1,13 @@ import { useQuery } from "@tanstack/react-query"; import listSources from "@/lib/api/client/app/leadsync/listSources"; -// Lists saved sync sources, optionally filtered to one campaign. Used by both -// the global Contacts > Sync sources area and the per-campaign list. -export default function useLeadSyncSources(campaignId?: string) { +// Lists saved sync sources, optionally filtered to one campaign or one segment. +// Used by the global Contacts > Sync sources area, the per-campaign list and a +// segment's own sources. +export default function useLeadSyncSources(campaignId?: string, segmentId?: string) { return useQuery({ - queryKey: ["lead-sync", "sources", campaignId ?? null], - queryFn: () => listSources(campaignId), + queryKey: ["lead-sync", "sources", campaignId ?? null, segmentId ?? null], + queryFn: () => listSources(campaignId, segmentId), staleTime: 10_000, }); } diff --git a/web/src/lib/api/models/app/leadsync/LeadSync.ts b/web/src/lib/api/models/app/leadsync/LeadSync.ts index a7372248..558a22d6 100644 --- a/web/src/lib/api/models/app/leadsync/LeadSync.ts +++ b/web/src/lib/api/models/app/leadsync/LeadSync.ts @@ -43,6 +43,8 @@ export interface LeadSyncSource { dedup: ImportDedupStrategy; target_campaign_id?: string; category_ids: string[]; + // Segments every synced row is pinned into on each run. + segment_ids: string[]; subscribed_default: boolean; label?: string; status: LeadSyncStatus; @@ -94,6 +96,7 @@ export interface CreateLeadSyncSource { dedup: ImportDedupStrategy; target_campaign_id?: string; category_ids: string[]; + segment_ids: string[]; subscribed_default?: boolean; label: string; } @@ -111,6 +114,7 @@ export interface UpdateLeadSyncSource { target_campaign_id?: string; clear_campaign?: boolean; category_ids?: string[]; + segment_ids?: string[]; subscribed_default?: boolean; label?: string; } From 5f665a32087f0317b791bd4eb7764c7c6f7a5506 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:25 -0700 Subject: [PATCH 05/11] feat: teach the contact import wizard a lockedSegment that always travels with the upload, named in the header and as a fixed always-applied row above the segment picker, confirmed on the result step from the API's segments_pinned flag, and refresh the segments query alongside contacts so the page behind the wizard is right before the realtime spine event lands --- .../components/app/contacts/ImportWizard.tsx | 92 +++++++++++++++++-- .../api/client/app/contacts/importContacts.ts | 4 + 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/web/src/components/app/contacts/ImportWizard.tsx b/web/src/components/app/contacts/ImportWizard.tsx index 8f30670e..6bbf5d7c 100644 --- a/web/src/components/app/contacts/ImportWizard.tsx +++ b/web/src/components/app/contacts/ImportWizard.tsx @@ -54,6 +54,7 @@ import { import { Label, TextInput } from "@/components/ui/field"; import CategoryPicker from "./CategoryPicker"; import { CampaignMultiPicker, SegmentMultiPicker } from "@/components/app/segments/SegmentPickers"; +import { useSegments } from "@/lib/api/hooks/app/segments"; import { downloadBlob } from "@/lib/api/client/app/contacts/exportContacts"; import { CUSTOM_KEY_RULES, @@ -73,11 +74,16 @@ interface Props { // When set (the campaign Leads tab), imported contacts are attached to this // campaign and the wizard shows a read-only "Adding to …" indicator. lockedCampaign?: { id: string; name: string }; + // When set (a segment's member list), every imported row is pinned into + // this segment, the same way the campaign target works. Without it an + // import started from inside a segment created contacts that were nowhere + // in it (issue #381). + lockedSegment?: { id: string; name: string; color?: string }; } type Step = "upload" | "map" | "options" | "result"; -export default function ImportWizard({ open, onClose, lockedCampaign }: Props) { +export default function ImportWizard({ open, onClose, lockedCampaign, lockedSegment }: Props) { const [step, setStep] = React.useState("upload"); const [file, setFile] = React.useState(null); const [preview, setPreview] = React.useState(null); @@ -91,6 +97,7 @@ export default function ImportWizard({ open, onClose, lockedCampaign }: Props) { const [commitBusy, setCommitBusy] = React.useState(false); const [result, setResult] = React.useState(null); const queryClient = useQueryClient(); + const segments = useSegments(open); function reset() { setStep("upload"); @@ -127,6 +134,20 @@ export default function ImportWizard({ open, onClose, lockedCampaign }: Props) { } } + // The segment the wizard was opened inside always travels with the + // import, whether or not the user opened the options step. + const targetSegmentIds = React.useMemo( + () => (lockedSegment ? [lockedSegment.id, ...segmentIds.filter((id) => id !== lockedSegment.id)] : segmentIds), + [lockedSegment, segmentIds], + ); + + // What the result step names back. A segment the list has not loaded + // falls back to the locked one's name rather than showing an id. + const pinnedSegmentNames = React.useMemo(() => { + const byId = new Map((segments.data ?? []).map((seg) => [seg.id, seg.name])); + return targetSegmentIds.map((id) => byId.get(id) ?? (id === lockedSegment?.id ? lockedSegment.name : "a segment")); + }, [segments.data, targetSegmentIds, lockedSegment]); + async function commit() { if (!file || !preview) return; setCommitBusy(true); @@ -137,11 +158,16 @@ export default function ImportWizard({ open, onClose, lockedCampaign }: Props) { has_header: hasHeader, category_ids: categoryIds.length > 0 ? categoryIds : undefined, campaign_ids: lockedCampaign ? [lockedCampaign.id] : campaignIds.length > 0 ? campaignIds : undefined, - segment_ids: segmentIds.length > 0 ? segmentIds : undefined, + segment_ids: targetSegmentIds.length > 0 ? targetSegmentIds : undefined, }); setResult(res); setStep("result"); - await queryClient.invalidateQueries({ queryKey: ["contacts"] }); + // Segment counts and pinned-member lists move with the import, so + // the page behind the wizard is right before the spine event lands. + await Promise.all([ + queryClient.invalidateQueries({ queryKey: ["contacts"] }), + queryClient.invalidateQueries({ queryKey: ["segments"] }), + ]); if (res.failed === 0) { toast.success(`Imported ${res.imported} · updated ${res.updated} · skipped ${res.skipped}`); } else { @@ -196,6 +222,15 @@ export default function ImportWizard({ open, onClose, lockedCampaign }: Props) { → {lockedCampaign.name} )} + {lockedSegment && ( + + → + {lockedSegment.color && ( + + )} + {lockedSegment.name} + + )}
@@ -673,6 +713,7 @@ function OptionsStep({ segmentIds, setSegmentIds, campaignLocked, + lockedSegment, }: { dedup: ImportDedupStrategy; setDedup: (v: ImportDedupStrategy) => void; @@ -685,6 +726,9 @@ function OptionsStep({ // From a campaign's Leads tab the target campaign is fixed, so the // campaign picker is hidden and the header chip shows the target instead. campaignLocked: boolean; + // From a segment's member list the segment is fixed and always applied; + // the picker stays so more segments can be added alongside it. + lockedSegment?: { id: string; name: string; color?: string }; }) { return (
@@ -755,10 +799,23 @@ function OptionsStep({ Add to segments

- Every imported contact is pinned into these segments. A segment linked to a campaign enrols them - there automatically. + {lockedSegment + ? "Every imported contact is pinned into this segment, whether or not it matches the segment's conditions. Add more below." + : "Every imported contact is pinned into these segments. A segment linked to a campaign enrols them there automatically."}

- + {lockedSegment && ( +
+ + {lockedSegment.name} + + Always + +
+ )} +
@@ -774,7 +831,17 @@ function OptionsStep({ // ----- Result step ---------------------------------------------- -export function ResultStep({ result, filename }: { result: ImportResult; filename: string }) { +export function ResultStep({ + result, + filename, + pinnedSegments, +}: { + result: ImportResult; + filename: string; + // Names of the segments every imported, updated and skipped row was + // pinned into, so the wizard confirms the membership it just wrote. + pinnedSegments?: string[]; +}) { function downloadErrors() { if (!result.errors || result.errors.length === 0) return; const rows = [["line", "email", "reason"]]; @@ -807,6 +874,9 @@ export function ResultStep({ result, filename }: { result: ImportResult; filenam

Processed {result.total.toLocaleString()} rows in{" "} {durationText(result.started_at, result.ended_at)}. + {result.segments_pinned && pinnedSegments && pinnedSegments.length > 0 && ( + <> Pinned into {pinnedSegments.join(", ")}. + )}

@@ -835,7 +905,7 @@ export function ResultStep({ result, filename }: { result: ImportResult; filenam
- Errors + {result.failed === 0 ? "Notes" : "Errors"} {result.errors_truncated @@ -863,7 +933,9 @@ export function ResultStep({ result, filename }: { result: ImportResult; filenam {result.errors.slice(0, 200).map((e, i) => ( - {e.line} + + {e.line > 0 ? e.line : —} + {e.email || —} diff --git a/web/src/lib/api/client/app/contacts/importContacts.ts b/web/src/lib/api/client/app/contacts/importContacts.ts index 5e2a5247..7bfdeb01 100644 --- a/web/src/lib/api/client/app/contacts/importContacts.ts +++ b/web/src/lib/api/client/app/contacts/importContacts.ts @@ -69,6 +69,10 @@ export interface ImportResult { // Set when more rows failed than the API reports back; `errors` then holds // the first slice of them and `failed` is the true count. errors_truncated?: boolean; + // True when the import had segment targets and every membership write + // landed. Absent when it had none; false when one failed, with the reason + // in `errors`. + segments_pinned?: boolean; // What the uploaded addresses look like. Advisory: a bad list is reported // here and stopped at launch, never refused here. quality?: ImportQuality; From 16349852cc4511a32bdfd708be260d709de36a26 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:25 -0700 Subject: [PATCH 06/11] feat: fix issue #381 by scoping the import and sheet-sync entry points on a segment's member list to that segment, so a CSV uploaded from inside a segment pins its rows in instead of creating contacts that are nowhere in it, and round the page out with an empty state offering Add contacts, Import file and New contact plus a pinned-contacts panel that says when it is showing only the newest slice of a large import --- .../app/app/contacts/segments/[id]/page.tsx | 12 +++++++- .../components/app/contacts/ContactsTable.tsx | 30 +++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/web/src/app/app/contacts/segments/[id]/page.tsx b/web/src/app/app/contacts/segments/[id]/page.tsx index 2e41856a..2f312085 100644 --- a/web/src/app/app/contacts/segments/[id]/page.tsx +++ b/web/src/app/app/contacts/segments/[id]/page.tsx @@ -127,7 +127,7 @@ function SegmentDetail() { {(s.included_count > 0 || s.excluded_count > 0) && } - + setEditorOpen(false)} segment={s} /> setCampaignOpen(false)} segment={s} /> @@ -190,6 +190,10 @@ function OverridesPanel({ segment }: { segment: Segment }) { } const list = overrides.data ?? []; + // The API caps one listing, so a segment a big import pinned into shows a + // slice of its overrides. Say so rather than implying this is all of them. + const pinned = segment.included_count + segment.excluded_count; + const truncated = pinned > list.length; return (
diff --git a/web/src/components/app/contacts/ContactsTable.tsx b/web/src/components/app/contacts/ContactsTable.tsx index bb35be32..0f85edc2 100644 --- a/web/src/components/app/contacts/ContactsTable.tsx +++ b/web/src/components/app/contacts/ContactsTable.tsx @@ -117,7 +117,7 @@ export default function ContactsTable({ }: { current_campaign?: MiniCampaign; // Scope the list to one segment's members (the segment detail page). - segment?: { id: string; name: string }; + segment?: { id: string; name: string; color?: string }; }) { const confirm = useConfirm(); const segmentMembers = useSetSegmentMembers(); @@ -487,7 +487,7 @@ export default function ContactsTable({ ? "Pick people from your contacts, import a file, or add one by hand. The linked segments could not be loaded." : "Pick people from your contacts, link a segment, import a file, or add one by hand." : segment - ? "Nobody matches its conditions yet. Adjust them or pin contacts in." + ? "Nothing matches it yet. Pin people in from your contacts, import a file, or add one by hand." : "Add or upload contacts to get started." } emptyCta={ @@ -532,6 +532,29 @@ export default function ContactsTable({ Import file
+ ) : segment ? ( +
+ } + onClick={() => setFromContactsOpen(true)} + > + Add contacts + + } + onClick={() => setImportOpen(true)} + > + Import file + + } + onClick={() => setNewOpen(true)} + > + New contact + +
) : ( } @@ -931,8 +954,9 @@ export default function ContactsTable({ setImportOpen(false)} + lockedSegment={segment} /> - setSyncOpen(false)} /> + setSyncOpen(false)} segment={segment} /> ); } From 7bac16aab0c6963b60d3a2d67862c6228e6fc783 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:31 -0700 Subject: [PATCH 07/11] feat: cover the segment import path with live tests: an import pins its inserted and skipped-as-duplicate rows into the target segment and the segment's own contact query returns them, an unknown segment is refused before any row is written while a recurring source skips one that is gone, and a sheet-sync source round-trips its segment targets through the new column and the ?segment_id= filter --- internal/app/contact/import_live_test.go | 99 ++++++++++++++++ .../lead_sync_segments_live_test.go | 110 ++++++++++++++++++ 2 files changed, 209 insertions(+) create mode 100644 internal/repository/lead_sync_segments_live_test.go diff --git a/internal/app/contact/import_live_test.go b/internal/app/contact/import_live_test.go index ff6d2572..a573b12b 100644 --- a/internal/app/contact/import_live_test.go +++ b/internal/app/contact/import_live_test.go @@ -33,6 +33,7 @@ type importFixture struct { campaign uuid.UUID svc ContactService repo repository.ContactRepository + segments repository.SegmentRepository pool *pgxpool.Pool } @@ -79,6 +80,8 @@ func newImportFixture(t *testing.T) *importFixture { {`DELETE FROM campaign_leads WHERE campaign_id IN (SELECT id FROM campaigns WHERE organization_id = $1)`, f.org}, {`DELETE FROM campaigns WHERE organization_id = $1`, f.org}, {`DELETE FROM contact_categories WHERE contact_id IN (SELECT id FROM contacts WHERE organization_id = $1)`, f.org}, + {`DELETE FROM segment_members WHERE segment_id IN (SELECT id FROM segments WHERE organization_id = $1)`, f.org}, + {`DELETE FROM segments WHERE organization_id = $1`, f.org}, {`DELETE FROM contacts WHERE organization_id = $1`, f.org}, {`DELETE FROM categories WHERE user_id = $1`, f.user}, {`DELETE FROM organization_members WHERE organization_id = $1`, f.org}, @@ -92,8 +95,12 @@ func newImportFixture(t *testing.T) *importFixture { }) f.repo = repository.NewContactRepostory(handle) + f.segments = repository.NewSegmentRepository(handle) // nil sub/plan repos: the plan cap is skipped, which is what we want here. f.svc = NewService(f.repo, nil, nil) + // Segment targets on an import are written through the same repository the + // backend wires in; no syncer, so nothing enrols campaigns here. + f.svc.(SegmentAware).WireSegments(f.segments, nil) return f } @@ -721,3 +728,95 @@ func TestLiveImportQualityFoldsInALegacyFinding(t *testing.T) { t.Errorf("the finding survived enough good data to outweigh it: %v", risk.Signals) } } + +// Issue #381: a CSV imported with segment targets must land in those segments, +// including rows the dedup strategy skipped, and the segment's own contact +// list (the query the dashboard runs) must return them. +func TestLiveImportPinsRowsIntoSegments(t *testing.T) { + f := newImportFixture(t) + ctx := context.Background() + + // Conditions nothing in the file matches, so membership can only come + // from the import's pin. + seg, xerr := f.segments.Create(ctx, f.org, &f.user, &models.Segment{ + Name: "Import target", Color: "#0284c7", Match: models.SegmentMatchAll, + Conditions: []models.SegmentCondition{{Field: "company", Operator: "equals", Value: "nobody-here"}}, + }) + if xerr != nil { + t.Fatalf("create segment: %v", xerr) + } + + tag := uuid.New().String()[:6] + emails := []string{"seg1-" + tag + "@i381.test", "seg2-" + tag + "@i381.test"} + opts := &models.ContactImportCommit{ + Mapping: emailOnlyMapping(), + Dedup: models.ContactImportDedupSkip, + HasHeader: true, + SegmentIDs: []string{seg.ID.String()}, + } + res, msg := f.commit(t, simpleCSV(emails), opts) + if msg != "" { + t.Fatalf("commit: %s", msg) + } + if res.Imported != 2 { + t.Fatalf("imported = %d, want 2", res.Imported) + } + if !res.SegmentsPinned { + t.Fatalf("result does not report the pin: %+v", res.Errors) + } + + members := func() []string { + t.Helper() + page, xerr := f.repo.Search(ctx, f.org.String(), nil, nil, + models.SearchContacts{SegmentIDs: []string{seg.ID.String()}}, 50) + if xerr != nil { + t.Fatalf("search: %v", xerr) + } + out := make([]string, 0, len(page.Data)) + for _, c := range page.Data { + out = append(out, c.Email) + } + return out + } + if got := members(); len(got) != 2 { + t.Fatalf("segment holds %v, want both imported rows", got) + } + + // The same file again: every row is skipped as a duplicate, and a third + // row is new. All three have to be in the segment afterwards, because + // "skip" means "leave their fields alone", not "leave them out". + emails = append(emails, "seg3-"+tag+"@i381.test") + res, msg = f.commit(t, simpleCSV(emails), opts) + if msg != "" { + t.Fatalf("second commit: %s", msg) + } + if res.Skipped != 2 || res.Imported != 1 { + t.Fatalf("second run: imported=%d skipped=%d, want 1/2", res.Imported, res.Skipped) + } + if got := members(); len(got) != 3 { + t.Fatalf("segment holds %v, want all three rows", got) + } + + // A segment id from another organization is refused up front rather than + // dropping the pin silently. + gone := uuid.NewString() + opts.SegmentIDs = []string{gone} + if _, msg := f.commit(t, simpleCSV([]string{"ghost-" + tag + "@i381.test"}), opts); msg == "" { + t.Fatalf("import with an unknown segment succeeded") + } + + // A recurring source (the Google Sheets sync) keeps importing when one of + // its saved targets is gone, and says the pin did not fully land. + opts.SegmentIDs = []string{seg.ID.String(), gone} + opts.SkipMissingSegments = true + res, msg = f.commit(t, simpleCSV([]string{"lenient-" + tag + "@i381.test"}), opts) + if msg != "" { + t.Fatalf("lenient commit: %s", msg) + } + if res.Imported != 1 || !res.SegmentsPinned { + t.Fatalf("lenient run: imported=%d pinned=%v", res.Imported, res.SegmentsPinned) + } + if got := members(); len(got) != 4 { + t.Fatalf("segment holds %v after the lenient run, want four rows", got) + } +} diff --git a/internal/repository/lead_sync_segments_live_test.go b/internal/repository/lead_sync_segments_live_test.go new file mode 100644 index 00000000..9c641994 --- /dev/null +++ b/internal/repository/lead_sync_segments_live_test.go @@ -0,0 +1,110 @@ +package repository + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/warmbly/warmbly/internal/models" +) + +// Issue #381: a saved Google Sheets source carries segment targets so a sync +// started from a segment's member list pins its rows into that segment, the +// same way a file import does. These prove the column round-trips and that the +// segment filter finds the sources feeding one segment. +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \ +// go test ./internal/repository/ -run LiveLeadSyncSegment -v + +func TestLiveLeadSyncSegmentTargets(t *testing.T) { + handle, pool := liveContactDB(t) + f := newSharedOrgFixture(t, pool) + ctx := context.Background() + repo := NewLeadSyncRepository(pool) + segments := NewSegmentRepository(handle) + + t.Cleanup(func() { + if _, err := pool.Exec(context.Background(), `DELETE FROM lead_sync_sources WHERE organization_id = $1`, f.org); err != nil { + t.Errorf("cleanup sources: %v", err) + } + if _, err := pool.Exec(context.Background(), `DELETE FROM segments WHERE organization_id = $1`, f.org); err != nil { + t.Errorf("cleanup segments: %v", err) + } + }) + + newSeg := func(name string) uuid.UUID { + t.Helper() + seg, xerr := segments.Create(ctx, f.org, &f.owner, &models.Segment{ + Name: name, Color: "#0284c7", Match: models.SegmentMatchAll, + }) + if xerr != nil { + t.Fatalf("create segment: %v", xerr) + } + return seg.ID + } + target := newSeg("Sheet target " + uuid.New().String()[:6]) + other := newSeg("Elsewhere " + uuid.New().String()[:6]) + + pinned := &models.LeadSyncSource{ + OrganizationID: f.org, CreatedByUserID: f.owner, ConnectionID: uuid.New(), + SheetID: "sheet-pinned", HasHeader: true, + ColumnMapping: []models.ContactImportColumnMapping{{Index: 0, Target: models.ContactImportTargetEmail}}, + Dedup: models.ContactImportDedupUpdate, + SegmentIDs: []string{target.String()}, + } + loose := &models.LeadSyncSource{ + OrganizationID: f.org, CreatedByUserID: f.owner, ConnectionID: uuid.New(), + SheetID: "sheet-loose", HasHeader: true, + ColumnMapping: []models.ContactImportColumnMapping{{Index: 0, Target: models.ContactImportTargetEmail}}, + Dedup: models.ContactImportDedupUpdate, + } + for _, src := range []*models.LeadSyncSource{pinned, loose} { + if err := repo.Create(ctx, src); err != nil { + t.Fatalf("create source: %v", err) + } + } + + got, err := repo.Get(ctx, f.org, pinned.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if len(got.SegmentIDs) != 1 || got.SegmentIDs[0] != target.String() { + t.Fatalf("segment targets = %v, want [%s]", got.SegmentIDs, target) + } + // A source with no targets reads back as an empty list, never nil, so the + // importer never sees a null segment list. + back, err := repo.Get(ctx, f.org, loose.ID) + if err != nil { + t.Fatalf("get loose: %v", err) + } + if back.SegmentIDs == nil || len(back.SegmentIDs) != 0 { + t.Fatalf("untargeted source segment ids = %v, want []", back.SegmentIDs) + } + + list, err := repo.List(ctx, f.org, nil, &target) + if err != nil { + t.Fatalf("list by segment: %v", err) + } + if len(list) != 1 || list[0].ID != pinned.ID { + t.Fatalf("segment filter returned %d sources, want only the pinned one", len(list)) + } + if list, err = repo.List(ctx, f.org, nil, &other); err != nil || len(list) != 0 { + t.Fatalf("filter on an unrelated segment returned %d sources (err %v)", len(list), err) + } + if list, err = repo.List(ctx, f.org, nil, nil); err != nil || len(list) != 2 { + t.Fatalf("unfiltered list returned %d sources (err %v), want 2", len(list), err) + } + + // The targets survive an edit that does not mention them. + got.Label = "renamed" + if err := repo.Update(ctx, got); err != nil { + t.Fatalf("update: %v", err) + } + again, err := repo.Get(ctx, f.org, pinned.ID) + if err != nil { + t.Fatalf("get after update: %v", err) + } + if len(again.SegmentIDs) != 1 || again.SegmentIDs[0] != target.String() { + t.Fatalf("segment targets after update = %v", again.SegmentIDs) + } +} From b5fc5b297427f4b2957bece8b6f8a34250754207 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 04:59:31 -0700 Subject: [PATCH 08/11] feat: document importing into a segment: the file wizard and a Google Sheets source both inherit the segment they were started from, a condition-less segment is the shortest way to turn a file into an audience, and the API reference gains segment_ids on lead-sync create and update, the ?segment_id= list filter and segments_pinned on the import result --- docs/content/docs/api/reference/contacts.mdx | 5 ++++- docs/content/docs/api/reference/integrations.mdx | 8 +++++++- docs/content/docs/guides/contacts-crm.mdx | 8 +++++--- docs/content/docs/guides/segments.mdx | 6 ++++-- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/docs/content/docs/api/reference/contacts.mdx b/docs/content/docs/api/reference/contacts.mdx index f69031b0..4342c1c9 100644 --- a/docs/content/docs/api/reference/contacts.mdx +++ b/docs/content/docs/api/reference/contacts.mdx @@ -367,7 +367,7 @@ Send `multipart/form-data` with a `file` field and an `options` field containing | `has_header` | boolean | Yes | Whether the first row is a header. | | `category_ids` | string[] | No | Categories to assign to imported contacts. | | `campaign_ids` | string[] | No | Campaigns to add imported contacts to. | -| `segment_ids` | string[] | No | Segments to pin imported contacts into, as a manual include override. Applies to imported, updated, and skipped-but-linked contacts alike. | +| `segment_ids` | string[] | No | Segments to pin imported contacts into, as a manual include override. Applies to imported, updated, and skipped-but-linked contacts alike. An id that is not a valid UUID, or that names no segment in the organization, is rejected with `400` before any row is written. | | `subscribed_default` | boolean | No | Subscription state for new contacts when no subscribed column is mapped. Defaults to true. | ```json @@ -398,6 +398,7 @@ Send `multipart/form-data` with a `file` field and an `options` field containing "errors": [ { "line": 57, "email": "not-an-email", "reason": "invalid email" } ], + "segments_pinned": true, "quality": { "malformed": 4, "disposable": 0, @@ -412,6 +413,8 @@ Send `multipart/form-data` with a `file` field and an `options` field containing Imports are capped at 50,000 rows. `errors` carries at most the first 1,000 entries; past that `errors_truncated` is `true` and the counters, not the list, are the real totals. Every row lands in exactly one of `imported`, `updated`, `skipped`, or `failed`, so those four always sum to `total`. +`errors` also carries notes about rows that were not failures, so an entry there does not always mean a lost row; a note about the import as a whole rather than one row carries `line: 0`. `segments_pinned` is present only when `segment_ids` was set: `true` when every membership write landed, `false` when one did not, with the reason among the notes. + A custom-field name may use letters, numbers, underscores, spaces, and dashes (`Company Mobile`, `first-name`, `plan_tier`). Anything else is a `400` on the whole request, raised before any row is written, along with a mapping that names no `email` column or a `custom` column with no `custom_key`. Per-row `errors` are reserved for problems with the data itself. ## Verification overview diff --git a/docs/content/docs/api/reference/integrations.mdx b/docs/content/docs/api/reference/integrations.mdx index 6bcd70fd..71377744 100644 --- a/docs/content/docs/api/reference/integrations.mdx +++ b/docs/content/docs/api/reference/integrations.mdx @@ -1061,13 +1061,14 @@ Auth: **Scope** `WRITE_CONTACTS` · **Org permission** `manage_contacts`. `GET /lead-sync/sources` -Lists this org's saved sync sources, optionally filtered to a campaign. +Lists this org's saved sync sources, optionally filtered to a campaign or a segment. Auth: **Scope** `WRITE_CONTACTS` · **Org permission** `manage_contacts`. | Parameter | In | Type | Description | |-----------|-----|------|-------------| | `campaign_id` | query | uuid | Optional. Restricts to sources targeting that campaign. Invalid ids return `400`. | +| `segment_id` | query | uuid | Optional. Restricts to sources that pin their rows into that segment. Invalid ids return `400`. Combines with `campaign_id`. | ### Response @@ -1090,6 +1091,7 @@ A `data` array (no pagination envelope on this list). ], "dedup": "update", "category_ids": [], + "segment_ids": [], "subscribed_default": true, "status": "idle", "last_synced_at": "2026-06-10T12:00:00Z", @@ -1121,6 +1123,7 @@ Auth: **Scope** `WRITE_CONTACTS` · **Org permission** `manage_contacts`. | `dedup` | string | No | Collision strategy: `skip`, `update`, or `create_duplicate`. | | `target_campaign_id` | uuid | No | Enrol new/updated leads into this campaign on each sync. | | `category_ids` | string[] | No | Categories to assign to synced leads. | +| `segment_ids` | string[] | No | Segments every synced row is pinned into as a manual include override, on every run. Each id must name a segment in the organization or the save is a `400`; a segment deleted later is dropped from the run instead of failing it. | | `subscribed_default` | boolean | No | Default subscription state for new contacts. | | `label` | string | No | Friendly name. | @@ -1138,6 +1141,7 @@ Auth: **Scope** `WRITE_CONTACTS` · **Org permission** `manage_contacts`. "dedup": "update", "target_campaign_id": null, "category_ids": [], + "segment_ids": [], "subscribed_default": true, "label": "Q2 inbound leads" } @@ -1162,6 +1166,7 @@ Auth: **Scope** `WRITE_CONTACTS` · **Org permission** `manage_contacts`. ], "dedup": "update", "category_ids": [], + "segment_ids": [], "subscribed_default": true, "status": "idle", "created_at": "2026-06-11T15:00:00Z", @@ -1210,6 +1215,7 @@ Auth: **Scope** `WRITE_CONTACTS` · **Org permission** `manage_contacts`. | `target_campaign_id` | uuid | No | New target campaign. | | `clear_campaign` | boolean | No | When `true`, unsets the target campaign. | | `category_ids` | string[] | No | Replacement category set. | +| `segment_ids` | string[] | No | Replacement segment target set. An unknown id is a `400`; an empty array clears the targets. | | `subscribed_default` | boolean | No | Default subscription state. | | `label` | string | No | New label. | diff --git a/docs/content/docs/guides/contacts-crm.mdx b/docs/content/docs/guides/contacts-crm.mdx index 8f1a53c1..3ebb9bdd 100644 --- a/docs/content/docs/guides/contacts-crm.mdx +++ b/docs/content/docs/guides/contacts-crm.mdx @@ -9,7 +9,7 @@ Contacts are the people you reach out to; the CRM tracks what happens after they Two routes share the same column-mapping screen: a file upload and an on-demand Google Sheets sync. -**From a file** (CSV, TSV, or XLSX, up to 50 MB and 50,000 rows), the wizard runs **Upload**, **Map**, **Options**, **Result**. Parsing happens server-side. Start it from Contacts, or from a campaign's Leads tab to attach the imports to that campaign automatically. The result step reports imported, updated, skipped, and failed counts, and failed rows download as an error CSV you can fix and re-import. +**From a file** (CSV, TSV, or XLSX, up to 50 MB and 50,000 rows), the wizard runs **Upload**, **Map**, **Options**, **Result**. Parsing happens server-side. Start it from Contacts, or from a campaign's Leads tab to attach the imports to that campaign automatically, or from a [segment](/guides/segments/)'s member list to pin every row into that segment. The wizard names the target it inherited in its header and on the Options step, and the result step confirms it. The result step reports imported, updated, skipped, and failed counts, and failed rows download as an error CSV you can fix and re-import. You must map at least one column to **Email**. @@ -31,9 +31,11 @@ Rows with a missing or invalid email are reported with a line number and reason Problems with the mapping itself, an unnamed custom field or a name Warmbly cannot use, are reported once before anything is written, so a single typo can never fail every row. -The **Options** step also decides where the contacts land: **Add to campaigns** enrols everyone in the file as leads of the campaigns you pick (hidden when the wizard was started from a campaign's Leads tab, since that campaign is already the target), and **Add to segments** pins them into the [segments](/guides/segments/) you pick as manual includes, so they stay members whatever the segment's conditions say. +The **Options** step also decides where the contacts land: **Add to campaigns** enrols everyone in the file as leads of the campaigns you pick (hidden when the wizard was started from a campaign's Leads tab, since that campaign is already the target), and **Add to segments** pins them into the [segments](/guides/segments/) you pick as manual includes, so they stay members whatever the segment's conditions say. Started from a segment, that segment is always applied and shown as a fixed row; the picker below it adds more. -**From Google Sheets**, a sheet is a reusable **sync source** rather than a one-time upload. Connect it once (Warmbly reads only the tab you choose and never writes back), paste the spreadsheet ID from the URL between `/d/` and `/edit`, pick the tab, map columns, then set duplicate handling, a label, and optionally a campaign to enroll into and categories to apply. **Nothing syncs automatically**: press **Sync now**, or save and sync immediately. +**From Google Sheets**, a sheet is a reusable **sync source** rather than a one-time upload. Connect it once (Warmbly reads only the tab you choose and never writes back), paste the spreadsheet ID from the URL between `/d/` and `/edit`, pick the tab, map columns, then set duplicate handling, a label, and optionally a campaign to enroll into, categories to apply and segments to pin into. **Nothing syncs automatically**: press **Sync now**, or save and sync immediately. + +Segment targets apply on every run, so a sheet you keep adding rows to keeps feeding the same audience. Opening **Sync sources** from a segment's member list lists only the sources feeding it and pre-targets a new one to it. A segment deleted later is dropped from the run rather than stopping it. Saved sources live in your Sync sources list to re-run, edit, or remove. Each sync dedupes on lowercased email using your chosen duplicate handling, so re-syncing a sheet with new rows is safe. diff --git a/docs/content/docs/guides/segments.mdx b/docs/content/docs/guides/segments.mdx index d34e1fd4..8cc4bf70 100644 --- a/docs/content/docs/guides/segments.mdx +++ b/docs/content/docs/guides/segments.mdx @@ -42,9 +42,11 @@ Conditions decide membership, and two overrides sit on top of them: The segment header shows how many contacts are pinned in or out, and a **Pinned contacts** panel below it lists them; **Back to automatic** clears an override so the conditions decide again. A contact's own drawer has a **Segments** section that shows every segment, whether the contact is in it, and the same pin in, pin out and back-to-automatic controls. -**New contact** on a segment page pins as well: a contact created from inside a segment joins that segment, so it appears in the list you created it from even when the conditions do not describe it yet. +**New contact** and **Import** on a segment page pin as well: a contact created or a file uploaded from inside a segment joins that segment, so it appears in the list you started from even when the conditions do not describe it yet. The import wizard names the segment in its header and confirms it on the result step, and rows it skipped as duplicates join too. -Sequences can pin too: the **Add to segment** and **Remove from segment** action steps apply the override to a contact as it moves through a campaign flow, so a positive reply can drop someone into a "warm" segment automatically. So can an import: the file import wizard's **Add to segments** picker on its Options step pins every contact in the file into the segments you choose, so a fresh list can land straight in the audience it was collected for. +Sequences can pin too: the **Add to segment** and **Remove from segment** action steps apply the override to a contact as it moves through a campaign flow, so a positive reply can drop someone into a "warm" segment automatically. So can an import: the file import wizard's **Add to segments** picker on its Options step pins every contact in the file into the segments you choose, so a fresh list can land straight in the audience it was collected for. A [Google Sheets sync source](/guides/contacts-crm/#importing) carries the same targets and re-applies them on every run, which is how a sheet you keep appending to keeps feeding one audience. + +A segment with no conditions at all is a plain list: it holds exactly the contacts you pin in, so creating one and importing into it is the shortest way to turn a file into an audience. ## Using a segment From 99c4ec9e21538e3a31743f77cc8bb20be9347958 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 05:19:03 -0700 Subject: [PATCH 09/11] feat: answer the CodeRabbit review on #386 by making ContactImportResult.SegmentsPinned a *bool so omitempty stops collapsing a failed membership write into the same absent value as an import that asked for none, warning in the result step when a pin did not land instead of leaving it to a note under a green Import complete, holding the pinned-contacts truncation notice until the overrides listing has actually arrived so it cannot read the newest 0 of 5,000 while it loads, and pointing that notice at the contact drawer, since a pinned-out contact can never be reached from the member list it used to name --- internal/app/contact/import.go | 5 +++-- internal/app/contact/import_live_test.go | 15 +++++++++++++-- internal/models/contact_import.go | 10 ++++++---- web/src/app/app/contacts/segments/[id]/page.tsx | 13 ++++++++----- web/src/components/app/contacts/ImportWizard.tsx | 16 ++++++++++++++++ .../api/client/app/contacts/importContacts.ts | 6 +++--- 6 files changed, 49 insertions(+), 16 deletions(-) diff --git a/internal/app/contact/import.go b/internal/app/contact/import.go index b59f48a3..b167b858 100644 --- a/internal/app/contact/import.go +++ b/internal/app/contact/import.go @@ -545,13 +545,14 @@ func (s *contactService) ImportCommit( // is a note, not a failed import: the contacts themselves are in, and the // result says the pin did not land so the UI does not claim it did. if len(segmentIDs) > 0 && len(touched) > 0 { - res.SegmentsPinned = true + pinned := true for _, segID := range segmentIDs { if _, xerr := s.segmentLinker.SetMembers(ctx, orgID, segID, touched, models.SegmentMemberInclude); xerr != nil { - res.SegmentsPinned = false + pinned = false warn(0, "", nil, "imported contacts could not be added to a segment: "+xerr.Message) } } + res.SegmentsPinned = &pinned } res.EndedAt = time.Now().UTC() diff --git a/internal/app/contact/import_live_test.go b/internal/app/contact/import_live_test.go index a573b12b..a2e4ebf2 100644 --- a/internal/app/contact/import_live_test.go +++ b/internal/app/contact/import_live_test.go @@ -761,7 +761,7 @@ func TestLiveImportPinsRowsIntoSegments(t *testing.T) { if res.Imported != 2 { t.Fatalf("imported = %d, want 2", res.Imported) } - if !res.SegmentsPinned { + if res.SegmentsPinned == nil || !*res.SegmentsPinned { t.Fatalf("result does not report the pin: %+v", res.Errors) } @@ -797,6 +797,17 @@ func TestLiveImportPinsRowsIntoSegments(t *testing.T) { t.Fatalf("segment holds %v, want all three rows", got) } + // No targets at all leaves the flag absent, so a caller can tell "nothing + // to pin" apart from "the pin failed". + res, msg = f.commit(t, simpleCSV([]string{"untargeted-" + tag + "@i381.test"}), + &models.ContactImportCommit{Mapping: emailOnlyMapping(), Dedup: models.ContactImportDedupSkip, HasHeader: true}) + if msg != "" { + t.Fatalf("untargeted commit: %s", msg) + } + if res.SegmentsPinned != nil { + t.Fatalf("import with no segment targets reported segments_pinned=%v", *res.SegmentsPinned) + } + // A segment id from another organization is refused up front rather than // dropping the pin silently. gone := uuid.NewString() @@ -813,7 +824,7 @@ func TestLiveImportPinsRowsIntoSegments(t *testing.T) { if msg != "" { t.Fatalf("lenient commit: %s", msg) } - if res.Imported != 1 || !res.SegmentsPinned { + if res.Imported != 1 || res.SegmentsPinned == nil || !*res.SegmentsPinned { t.Fatalf("lenient run: imported=%d pinned=%v", res.Imported, res.SegmentsPinned) } if got := members(); len(got) != 4 { diff --git a/internal/models/contact_import.go b/internal/models/contact_import.go index 909cae65..87f98871 100644 --- a/internal/models/contact_import.go +++ b/internal/models/contact_import.go @@ -156,10 +156,12 @@ type ContactImportResult struct { // "showing the first N of M" instead of implying it listed everything. ErrorsTruncated bool `json:"errors_truncated,omitempty"` - // SegmentsPinned is true when the import had segment targets and every - // membership write landed. False with targets set means the reason is in - // Errors, so the UI never claims a pin that did not happen. - SegmentsPinned bool `json:"segments_pinned,omitempty"` + // SegmentsPinned is nil when the import had no segment targets to write. + // With targets it is true when every membership write landed and false + // when one did not, with the reason among the notes. A plain bool could + // not carry that third state: omitempty drops false, so a failed pin + // looked exactly like an import that never asked for one. + SegmentsPinned *bool `json:"segments_pinned,omitempty"` // Quality is what the uploaded addresses look like, measured at import. // Advisory: a bad list is reported here and stopped at launch, never diff --git a/web/src/app/app/contacts/segments/[id]/page.tsx b/web/src/app/app/contacts/segments/[id]/page.tsx index 2f312085..38073e89 100644 --- a/web/src/app/app/contacts/segments/[id]/page.tsx +++ b/web/src/app/app/contacts/segments/[id]/page.tsx @@ -191,9 +191,11 @@ function OverridesPanel({ segment }: { segment: Segment }) { const list = overrides.data ?? []; // The API caps one listing, so a segment a big import pinned into shows a - // slice of its overrides. Say so rather than implying this is all of them. + // slice of its overrides. Say so rather than implying this is all of them, + // but only once the listing actually arrived: while it is pending `list` + // is empty, and the notice would read "the newest 0 of 5,000". const pinned = segment.included_count + segment.excluded_count; - const truncated = pinned > list.length; + const truncated = overrides.isSuccess && pinned > list.length; return (
+ {result.segments_pinned === false && ( +
+ +
+

+ The contacts are in, but not in the segment +

+

+ The rows imported, and the membership write did not land. The reason is in the notes below. + Select them in your contact list and use Segment to add + them, or run the import again. +

+
+
+ )} + {result.quality?.flagged && (
diff --git a/web/src/lib/api/client/app/contacts/importContacts.ts b/web/src/lib/api/client/app/contacts/importContacts.ts index 7bfdeb01..516400e8 100644 --- a/web/src/lib/api/client/app/contacts/importContacts.ts +++ b/web/src/lib/api/client/app/contacts/importContacts.ts @@ -69,9 +69,9 @@ export interface ImportResult { // Set when more rows failed than the API reports back; `errors` then holds // the first slice of them and `failed` is the true count. errors_truncated?: boolean; - // True when the import had segment targets and every membership write - // landed. Absent when it had none; false when one failed, with the reason - // in `errors`. + // Absent when the import had no segment targets. With targets, true when + // every membership write landed and false when one did not, with the + // reason among the notes in `errors`. segments_pinned?: boolean; // What the uploaded addresses look like. Advisory: a bad list is reported // here and stopped at launch, never refused here. From 03b6188364313ffb2f4fc98d23b334e516140631 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 06:36:34 -0700 Subject: [PATCH 10/11] feat: tighten the import result's failed-pin warning so it contrasts the two outcomes instead of joining them with an and that reads as if both succeeded --- web/src/components/app/contacts/ImportWizard.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/components/app/contacts/ImportWizard.tsx b/web/src/components/app/contacts/ImportWizard.tsx index a0677bd7..75e53aed 100644 --- a/web/src/components/app/contacts/ImportWizard.tsx +++ b/web/src/components/app/contacts/ImportWizard.tsx @@ -896,9 +896,9 @@ export function ResultStep({ The contacts are in, but not in the segment

- The rows imported, and the membership write did not land. The reason is in the notes below. - Select them in your contact list and use Segment to add - them, or run the import again. + The rows imported; the membership write did not. The reason is in the notes below. Select + them in your contact list and use Segment to add them, + or run the import again.

From 0f8332993672aaf18056c1eed81d7489b0e44032 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 8 Sep 2026 06:54:36 -0700 Subject: [PATCH 11/11] feat: answer the CodeRabbit re-review by guaranteeing the promise the failed-pin warning makes: an import-level note now goes to the front of the error list and is never dropped by the per-row cap, so a file full of bad addresses cannot bury or evict the one note explaining why the rows that did import are not in the segment they were imported into, with a single note however many segments were targeted and a live test that forces the write to fail behind a capped error list --- docs/content/docs/api/reference/contacts.mdx | 2 +- internal/app/contact/import.go | 27 ++++++++-- internal/app/contact/import_live_test.go | 54 ++++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/docs/content/docs/api/reference/contacts.mdx b/docs/content/docs/api/reference/contacts.mdx index 4342c1c9..1f006b17 100644 --- a/docs/content/docs/api/reference/contacts.mdx +++ b/docs/content/docs/api/reference/contacts.mdx @@ -413,7 +413,7 @@ Send `multipart/form-data` with a `file` field and an `options` field containing Imports are capped at 50,000 rows. `errors` carries at most the first 1,000 entries; past that `errors_truncated` is `true` and the counters, not the list, are the real totals. Every row lands in exactly one of `imported`, `updated`, `skipped`, or `failed`, so those four always sum to `total`. -`errors` also carries notes about rows that were not failures, so an entry there does not always mean a lost row; a note about the import as a whole rather than one row carries `line: 0`. `segments_pinned` is present only when `segment_ids` was set: `true` when every membership write landed, `false` when one did not, with the reason among the notes. +`errors` also carries notes about rows that were not failures, so an entry there does not always mean a lost row. A note about the import as a whole rather than one row carries `line: 0` and is listed first, so a file full of bad addresses cannot push it out of a truncated list. `segments_pinned` is present only when `segment_ids` was set: `true` when every membership write landed, `false` when one did not, with the reason among the notes. A custom-field name may use letters, numbers, underscores, spaces, and dashes (`Company Mobile`, `first-name`, `plan_tier`). Anything else is a `400` on the whole request, raised before any row is written, along with a mapping that names no `email` column or a `custom` column with no `custom_key`. Per-row `errors` are reserved for problems with the data itself. diff --git a/internal/app/contact/import.go b/internal/app/contact/import.go index b167b858..c25255df 100644 --- a/internal/app/contact/import.go +++ b/internal/app/contact/import.go @@ -379,6 +379,18 @@ func (s *contactService) ImportCommit( res.Failed++ warn(line, addr, values, reason) } + // noteImport records a message about the whole import rather than one row. + // It goes to the front and is never dropped by the per-row cap: a file full + // of bad addresses must not push out the one note explaining why the rows + // that DID import are not in the segment they were imported into, nor bury + // it past the entries the dashboard renders. + noteImport := func(reason string) { + res.Errors = append([]models.ContactImportRowError{{Reason: reason}}, res.Errors...) + if len(res.Errors) > models.MaxContactImportReportedErrors { + res.Errors = res.Errors[:models.MaxContactImportReportedErrors] + res.ErrorsTruncated = true + } + } // Bucket rows by target action. We send fresh inserts through // contactRepository.Add in batches and fall back to per-row @@ -545,13 +557,22 @@ func (s *contactService) ImportCommit( // is a note, not a failed import: the contacts themselves are in, and the // result says the pin did not land so the UI does not claim it did. if len(segmentIDs) > 0 && len(touched) > 0 { - pinned := true + pinned, failedPins, firstReason := true, 0, "" for _, segID := range segmentIDs { if _, xerr := s.segmentLinker.SetMembers(ctx, orgID, segID, touched, models.SegmentMemberInclude); xerr != nil { - pinned = false - warn(0, "", nil, "imported contacts could not be added to a segment: "+xerr.Message) + pinned, failedPins = false, failedPins+1 + if firstReason == "" { + firstReason = xerr.Message + } } } + // One note for the whole pin, however many segments were targeted: + // the same failure repeated per segment is noise, not information. + if failedPins == 1 { + noteImport("imported contacts could not be added to a segment: " + firstReason) + } else if failedPins > 1 { + noteImport(fmt.Sprintf("imported contacts could not be added to %d segments: %s", failedPins, firstReason)) + } res.SegmentsPinned = &pinned } diff --git a/internal/app/contact/import_live_test.go b/internal/app/contact/import_live_test.go index a2e4ebf2..14e3f079 100644 --- a/internal/app/contact/import_live_test.go +++ b/internal/app/contact/import_live_test.go @@ -11,6 +11,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "github.com/warmbly/warmbly/internal/app/orgrisk" + "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/infrastructure/db" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/repository" @@ -831,3 +832,56 @@ func TestLiveImportPinsRowsIntoSegments(t *testing.T) { t.Fatalf("segment holds %v after the lenient run, want four rows", got) } } + +// failingLinker validates segment targets through the real repository but +// refuses every membership write, which is the only way to reach the pin +// failure path from a test. +type failingLinker struct{ repository.SegmentRepository } + +func (failingLinker) SetMembers(context.Context, uuid.UUID, uuid.UUID, []uuid.UUID, models.SegmentMemberMode) (int, *errx.Error) { + return 0, errx.New(errx.Internal, "segment store unavailable") +} + +// A pin that does not land is reported as segments_pinned=false with one note +// saying why, and that note survives a file that produced more row errors than +// the payload cap: it explains the rows that DID import, so it must not be the +// entry that gets dropped. +func TestLiveImportReportsAPinThatDidNotLand(t *testing.T) { + f := newImportFixture(t) + ctx := context.Background() + + seg, xerr := f.segments.Create(ctx, f.org, &f.user, &models.Segment{ + Name: "Unreachable " + uuid.New().String()[:6], Color: "#0284c7", Match: models.SegmentMatchAll, + }) + if xerr != nil { + t.Fatalf("create segment: %v", xerr) + } + f.svc.(SegmentAware).WireSegments(failingLinker{f.segments}, nil) + + tag := uuid.New().String()[:6] + rows := []string{"good-" + tag + "@i381.test"} + for i := 0; i < models.MaxContactImportReportedErrors+5; i++ { + rows = append(rows, fmt.Sprintf("not-an-email-%d", i)) + } + res, msg := f.commit(t, simpleCSV(rows), &models.ContactImportCommit{ + Mapping: emailOnlyMapping(), Dedup: models.ContactImportDedupSkip, HasHeader: true, + SegmentIDs: []string{seg.ID.String()}, + }) + if msg != "" { + t.Fatalf("commit: %s", msg) + } + if res.Imported != 1 { + t.Fatalf("imported = %d, want the one valid row", res.Imported) + } + if res.SegmentsPinned == nil || *res.SegmentsPinned { + t.Fatalf("a refused membership write reported segments_pinned=%v", res.SegmentsPinned) + } + if !res.ErrorsTruncated || len(res.Errors) != models.MaxContactImportReportedErrors { + t.Fatalf("errors = %d truncated=%v, want the cap", len(res.Errors), res.ErrorsTruncated) + } + // First, so the dashboard renders it however many row errors came with it. + first := res.Errors[0] + if first.Line != 0 || !strings.Contains(first.Reason, "could not be added to a segment") { + t.Fatalf("first note = %+v, want the segment-pin reason", first) + } +}