Feature: Contact Segmentation (#310)#385
Open
BenoitPrmt wants to merge 3 commits into
Open
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
@BenoitPrmt is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR implements contact segmentation, allowing users to create segments based on custom contact fields and send campaigns to those segments. The changes introduce a new Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/web/src/app/(dashboard)/contacts/[contactBookId]/page.tsx (1)
47-47: Use the~/alias for this new src import.
contact-segments-managerlives underapps/web/src, so this new import should follow the repo alias convention instead of adding another relative path.As per coding guidelines,
apps/web/src/**/*.{ts,tsx}should "Use the~/alias for imports from src in apps/web (e.g.,import { x } from "~/utils/x")".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(dashboard)/contacts/[contactBookId]/page.tsx at line 47, The import of ContactSegmentsManager in page.tsx uses a relative path; update the import to use the apps/web src alias (~/...) per convention. Locate the import statement for ContactSegmentsManager in page.tsx and replace the relative path with the aliased path (e.g., import ContactSegmentsManager from "~/...") so it imports from apps/web/src via the ~/ alias.apps/web/src/lib/contact-segments.unit.test.ts (1)
8-77: Add one regression test for the server-side filter path.These tests only cover the in-memory helpers. The new audience counts and contact filtering now depend on
buildContactSegmentWhere(), so a mismatch in the Prisma translation can still ship unnoticed. Please add at least one server-side test that exercisesisSet/isNotSetthrough the query path.As per coding guidelines,
apps/web/**/*.{unit,trpc,api,integration}.test.{ts,tsx}should "Add tests when changes impact logic, APIs, or behavior in apps/web".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/contact-segments.unit.test.ts` around lines 8 - 77, Add a regression test that exercises the server-side filter translation by calling buildContactSegmentWhere with a segment containing isSet and isNotSet conditions and asserting the produced Prisma "where" shape (or the actual query result against a test Prisma client) matches the expected behavior; specifically construct a segment with conditions like { field: "company", operator: "isSet" } and { field: "email", operator: "isNotSet" }, call buildContactSegmentWhere(...) and assert the returned object contains the correct null/not-null checks (or run a small in-memory/test DB query using that where to verify matching rows). Use the existing test patterns in contact-segments.unit.test.ts for structure and reference the buildContactSegmentWhere symbol to locate the code to test.apps/web/src/server/service/contact-segment-service.ts (1)
9-9: Use the app src alias for this import.♻️ Suggested change
-import { buildContactSegmentWhere } from "./contact-segment-filter"; +import { buildContactSegmentWhere } from "~/server/service/contact-segment-filter";As per coding guidelines,
apps/web/src/**/*.{ts,tsx}: Use the~/alias for imports from src in apps/web.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/server/service/contact-segment-service.ts` at line 9, Replace the relative import of buildContactSegmentWhere with the app src alias import (use the ~/ alias) so the module is resolved from the app's src; specifically update the import in contact-segment-service.ts to import buildContactSegmentWhere from the aliased path (e.g., "~/server/service/contact-segment-filter") instead of "./contact-segment-filter".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/`(dashboard)/contacts/[contactBookId]/contact-list.tsx:
- Around line 86-89: The URL-stored segmentId must be cleared when the selected
segment no longer exists: after the segmentsQuery
(api.contacts.listSegments.useQuery) returns, check whether the current
segmentId from useUrlState("segment") exists in segmentsQuery.data; if it does
not, call setSegmentId("") (or undefined per useUrlState expectation) to remove
the invalid filter so the contacts/export queries stop sending the deleted id;
implement this check in a useEffect that depends on segmentsQuery.data and
segmentId and reference segmentId, setSegmentId, and segmentsQuery to locate the
code to update.
In
`@apps/web/src/app/`(dashboard)/contacts/[contactBookId]/contact-segments-manager.tsx:
- Around line 168-182: The early return when contactBookVariables.length === 0
hides the segments list; remove that return and instead keep rendering the
segments management UI (the list/rendering code for existing segments) while
showing the informational Card (the existing
Card/CardHeader/CardTitle/CardContent snippet) as a warning/notice; only disable
or hide the segment-creation controls (e.g., the Create/Manage button or
createSegment handler) when contactBookVariables is empty so saved segments
(referenced as segments or whatever list variable the component renders) remain
visible and deletable. Ensure you update any create-segment button or form
(CreateSegmentButton, onCreateSegment, or similar) to be disabled when
contactBookVariables.length === 0 rather than preventing the entire component
from rendering.
In `@apps/web/src/server/service/contact-segment-filter.ts`:
- Around line 44-58: The isSet/isNotSet Prisma filters currently use
Prisma.JsonNull which only matches explicit JSON nulls; update the case handling
in contact-segment-filter (the "isSet" and "isNotSet" branches that build the
properties filter for path) to use Prisma.DbNull instead (for isNotSet use
equals: Prisma.DbNull or for isSet use NOT: { properties: { path, equals:
Prisma.DbNull } } / for the negated case use not: Prisma.DbNull) so missing JSON
keys (SQL NULL) are matched the same way the in-memory matcher does; adjust the
NOT/negation logic accordingly in the code that constructs the properties filter
for path.
---
Nitpick comments:
In `@apps/web/src/app/`(dashboard)/contacts/[contactBookId]/page.tsx:
- Line 47: The import of ContactSegmentsManager in page.tsx uses a relative
path; update the import to use the apps/web src alias (~/...) per convention.
Locate the import statement for ContactSegmentsManager in page.tsx and replace
the relative path with the aliased path (e.g., import ContactSegmentsManager
from "~/...") so it imports from apps/web/src via the ~/ alias.
In `@apps/web/src/lib/contact-segments.unit.test.ts`:
- Around line 8-77: Add a regression test that exercises the server-side filter
translation by calling buildContactSegmentWhere with a segment containing isSet
and isNotSet conditions and asserting the produced Prisma "where" shape (or the
actual query result against a test Prisma client) matches the expected behavior;
specifically construct a segment with conditions like { field: "company",
operator: "isSet" } and { field: "email", operator: "isNotSet" }, call
buildContactSegmentWhere(...) and assert the returned object contains the
correct null/not-null checks (or run a small in-memory/test DB query using that
where to verify matching rows). Use the existing test patterns in
contact-segments.unit.test.ts for structure and reference the
buildContactSegmentWhere symbol to locate the code to test.
In `@apps/web/src/server/service/contact-segment-service.ts`:
- Line 9: Replace the relative import of buildContactSegmentWhere with the app
src alias import (use the ~/ alias) so the module is resolved from the app's
src; specifically update the import in contact-segment-service.ts to import
buildContactSegmentWhere from the aliased path (e.g.,
"~/server/service/contact-segment-filter") instead of
"./contact-segment-filter".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ae99c22-9183-423c-8b95-b957d36834bf
📒 Files selected for processing (15)
apps/web/prisma/migrations/20260330193000_add_contact_segments/migration.sqlapps/web/prisma/schema.prismaapps/web/src/app/(dashboard)/campaigns/[campaignId]/edit/page.tsxapps/web/src/app/(dashboard)/campaigns/[campaignId]/page.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/contact-list.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/contact-segments-manager.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/page.tsxapps/web/src/lib/contact-segments.tsapps/web/src/lib/contact-segments.unit.test.tsapps/web/src/server/api/routers/campaign.tsapps/web/src/server/api/routers/contacts.tsapps/web/src/server/service/campaign-service.tsapps/web/src/server/service/contact-segment-filter.tsapps/web/src/server/service/contact-segment-service.tsapps/web/src/server/service/webhook-service.unit.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added availability to create segments on contacts lists and make campaigns over it
Closes #310
Summary by cubic
Add contact segmentation so teams can target subsets of a contact book and see exact audience counts in campaigns.
New Features
getCampaignnow includescontactSegment.listSegments,createSegment,updateSegment,deleteSegment,getCampaignAudience; contacts APIs acceptsegmentId; campaign updates acceptcontactSegmentIdwith contact book validation; send/schedule/batching compute audience and totals using segment filters.Migration
ContactSegmenttable andCampaign.contactSegmentId. Run Prisma migrations to apply the new schema and indexes.Written for commit a1cd608. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit