Display referral and group moves changelog#3427
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds workspaceId to workflow identity payloads across multiple conversion/webhook/workflow flows; introduces activity-log schemas/types, API, SWR hook and a full frontend activity-log UI; makes ActivityLog.user optional in Prisma; updates partner/group move and partner-link generation logic; adds new icon assets. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client (UI)
participant Hook as useActivityLogs (frontend)
participant API as Server /api/activity-logs
participant DB as Database (ActivityLog)
Client->>Hook: mount & request logs (query)
Hook->>API: GET /api/activity-logs?programId=...&resourceType=...&resourceId=...&action=...
API->>DB: query ActivityLog (filters: programId, resourceType, resourceId, action)
DB-->>API: return logs
API-->>Hook: JSON logs
Hook-->>Client: activityLogs (render)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/lib/api/groups/move-partners-to-group.ts`:
- Around line 147-158: Wrap the call to getWorkspaceUsers in a try/catch so any
thrown error does not abort the rest of the function: attempt to fetch users to
populate workspaceUserId (when userId is falsy) inside a try block, on success
keep the existing logic (assign users[0].id if present), and on failure catch
the error, log it, and leave workspaceUserId as null so downstream tasks
(Promise.allSettled calling recordLink, notifyPartnerGroupChange,
trackActivityLog, etc.) still run; ensure you reference the existing variables
and functions (workspaceUserId, userId, getWorkspaceUsers, Promise.allSettled,
recordLink, notifyPartnerGroupChange, trackActivityLog) when making this change.
In `@packages/ui/src/icons/user-clock.tsx`:
- Around line 1-32: The UserClock SVG hardcodes color values (`#525252`) on path
elements causing inconsistency with the icon set; update the component (function
UserClock) to use "currentColor" for all stroke and fill attributes (replace any
stroke="#525252" and fill="#525252" with stroke="currentColor" and
fill="currentColor") so parent CSS can control the icon color, ensuring all path
elements inside the returned <svg> use currentColor.
🧹 Nitpick comments (12)
apps/web/lib/integrations/shopify/create-sale.ts (1)
164-178: Nit: use property shorthand.
workspaceId: workspaceIdcan be simplified toworkspaceId.Proposed diff
identity: { - workspaceId: workspaceId, + workspaceId, programId: link.programId,apps/web/app/api/activity-logs/route.ts (1)
28-31: No pagination support — hardcodedtake: 100.There's no cursor/offset-based pagination. If a resource accumulates more than 100 activity logs, older entries won't be retrievable. This may be acceptable for the current use case but could become limiting.
apps/web/ui/activity-logs/partner-group-activity-section.tsx (1)
24-32: Error state says "Please try again" but offers no retry action.SWR will auto-revalidate on window focus, but consider adding a retry button using
mutatefromuseActivityLogsto make the error state actionable — or simplify the text to just "Failed to load history."apps/web/ui/activity-logs/action-renderers/partner-group-changed-renderer.tsx (1)
7-10: Theoldgroup inGroupChangeSetis declared but never used.The interface defines both
oldandnew, but onlynewis read (line 22). The UI shows "Moved to [new group]" without indicating which group the partner was moved from. If this is intentional (e.g., the old group is shown elsewhere or isn't needed), consider removingoldfrom the interface to avoid confusion. Otherwise, consider displaying the origin group for richer context:Moved from [old group] to [new group]Also applies to: 19-28
apps/web/lib/swr/use-activity-logs.ts (1)
14-20:URLSearchParamsmay stringifyundefinedvalues.If
queryis provided butresourceTypeorresourceIdisundefined,new URLSearchParams(...)will produce keys likeresourceType=undefined. The SWR key guard on Lines 22–26 prevents the fetch in this scenario, so it's harmless at runtime, but it's slightly misleading if you ever log or inspectsearchParamsduring debugging.Optional: guard URLSearchParams construction
const searchParams = query - ? new URLSearchParams({ - resourceType: query.resourceType, - resourceId: query.resourceId, - ...(query.action && { action: query.action }), - }).toString() + && query.resourceType + && query.resourceId + ? new URLSearchParams({ + resourceType: query.resourceType, + resourceId: query.resourceId, + ...(query.action && { action: query.action }), + }).toString() : "";apps/web/ui/activity-logs/referral-activity-item.tsx (2)
41-48: Consider adding adateTimeattribute to the<time>element.For semantic HTML and accessibility, the
<time>element should include a machine-readabledateTimeattribute (e.g., the ISO string fromlog.createdAt).Proposed fix
- <time className="hidden shrink-0 text-xs text-neutral-500 sm:block"> + <time + dateTime={new Date(log.createdAt).toISOString()} + className="hidden shrink-0 text-xs text-neutral-500 sm:block" + >
13-53: Significant structural overlap withPartnerGroupActivityItem.This component shares the same pattern as
PartnerGroupActivityItem(icon + renderer + timestamp in a timeline layout). The differences are minor: responsive date visibility,ActivityLogDescription, and spacing. Consider extracting a sharedActivityTimelineItembase component that accepts children/slots for the content area and an optional description, to reduce duplication if more activity item types are introduced.apps/web/ui/activity-logs/partner-group-activity-item.tsx (1)
39-46: SamedateTimeattribute suggestion asreferral-activity-item.tsx.The
<time>element here also lacks adateTimeattribute for semantic correctness.Proposed fix
- <time className="text-xs text-neutral-500"> + <time + dateTime={new Date(log.createdAt).toISOString()} + className="text-xs text-neutral-500" + >apps/web/lib/api/activity-log/track-activity-log.ts (1)
27-31: Filter logic now has action-specific bypasses — consider a more extensible approach.The filter was extended to allow
referral.createdto bypass the non-emptychangeSetrequirement. This works, but if more actions without changeSets are added later, this condition will need to be updated each time.A minor suggestion would be to use a set of actions that don't require changeSets:
Optional: use a set for exempted actions
+const ACTIONS_WITHOUT_CHANGESET: Set<ActivityLogAction> = new Set([ + "referral.created", +]); + export const trackActivityLog = async ( input: TrackActivityLogInput | TrackActivityLogInput[], ) => { let inputs = Array.isArray(input) ? input : [input]; inputs = inputs.filter( (i) => - i.action === "referral.created" || + ACTIONS_WITHOUT_CHANGESET.has(i.action) || (i.changeSet && Object.keys(i.changeSet).length > 0), );apps/web/ui/partners/partner-info-cards.tsx (1)
85-89: Unnecessary activity-log fetch for non-enrolled (network) partners.
usePartnerGroupHistorySheetis invoked unconditionally, so it fires an API request for network partners too — even though the "View history" button and sheet are only used whenisEnrolled. Consider gating the hook'spartnerargument:- } = usePartnerGroupHistorySheet({ partner: partner || null }); + } = usePartnerGroupHistorySheet({ + partner: isEnrolled ? partner || null : null, + });This avoids a wasted network round-trip for the network-partner view.
apps/web/ui/activity-logs/partner-group-history-sheet.tsx (1)
14-36:PartnerGroupHistorySheetContentreceivessetIsOpenbut never uses it.The type
Omit<PartnerGroupHistorySheetProps, "isOpen">includessetIsOpen, but the component only usespartner. Since the close button relies onSheet.Close, the extra prop is harmless but imprecise.🔧 Optional: tighten the prop type
-function PartnerGroupHistorySheetContent({ - partner, -}: Omit<PartnerGroupHistorySheetProps, "isOpen">) { +function PartnerGroupHistorySheetContent({ + partner, +}: Pick<PartnerGroupHistorySheetProps, "partner">) {apps/web/ui/activity-logs/activity-entry-chips.tsx (1)
63-74: Minor: Inconsistent fallback text betweenaltand display.The
altattribute falls back to"User"(line 68) while the visible text falls back to"Unknown user"(line 71). Consider aligning them for consistency.Proposed fix
- alt={`${user.name || user.email || "User"}`} + alt={user.name || user.email || "Unknown user"}
- Activity log userId optional; workspaceId in workflow identity - Activity logs API, registry, renderers (action-renderers) - Partner group history sheet and activity section - Referral activity section and status change renderers - New icons: FilePen, FileSend, UserClock - Activity log types in lib/types, zod schema updates - Aligns branch with #3423 (group-move-changelog) Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/lib/api/groups/move-partners-to-group.ts`:
- Around line 162-171: The cron job body currently always sets userId to
workspaceUserId which can be null and will fail the route's z.string()
validation; update the qstash.publishJSON call in move-partners-to-group.ts so
the body only includes userId when workspaceUserId is non-null (e.g., build the
body object then conditionally add userId if workspaceUserId != null) or else
provide a guaranteed non-null fallback (like 'system'); alternatively, if you
prefer schema-side changes, update the cron route (route.ts) to accept userId as
optional (z.string().optional()) and ensure generatePartnerLink handles a
missing/undefined userId.
In `@apps/web/ui/activity-logs/activity-log-description.tsx`:
- Line 31: The paragraph element renders even when the nullable prop description
is null, leaving a vertical gap; update the ActivityLogDescription component to
only render the <p> element when description is non-null/non-empty (e.g., check
description != null and description !== '' or use a truthy check) so that the
element with className "text-content-subtle mt-2 text-sm" is not output when
description is absent; locate the JSX where description is used (the <p
className="text-content-subtle mt-2 text-sm">{description}</p>) and wrap or
replace it with a conditional render that returns nothing when description is
null/empty.
🧹 Nitpick comments (3)
apps/web/ui/activity-logs/action-renderers/referral-created-renderer.tsx (1)
8-12: Consider extracting theLabelcomponent to a shared module.The
Labelhelper is duplicated identically across three renderer files (referral-created-renderer.tsx,partner-group-changed-renderer.tsx, andreferral-status-changed-renderer.tsx). Extracting it toactivity-entry-chips.tsxor a dedicated shared utilities file would eliminate this duplication.apps/web/lib/swr/use-activity-logs.ts (1)
15-22:searchParamsis built eagerly withworkspaceId!before the SWR key gates on it.The
workspaceId!non-null assertion on line 17 will produce"workspaceId=undefined"in the URLSearchParams string whenworkspaceIdis null/undefined. This is harmless because the SWR key (lines 25–28) short-circuits to falsy whenworkspaceIdis missing, so the string is never used as a fetch URL. Still, computing the params lazily (or guarding the construction) would be slightly cleaner.♻️ Optional: guard searchParams construction
- const searchParams = query - ? new URLSearchParams({ - workspaceId: workspaceId!, - resourceType: query.resourceType, - resourceId: query.resourceId, - ...(query.action && { action: query.action }), - }).toString() - : ""; + const searchParams = + query && workspaceId + ? new URLSearchParams({ + workspaceId, + resourceType: query.resourceType, + resourceId: query.resourceId, + ...(query.action && { action: query.action }), + }).toString() + : "";Based on learnings, workspace authentication patterns in the Dub codebase prevent issues when workspaceId is missing, so the current code is safe in practice.
apps/web/ui/activity-logs/partner-group-history-sheet.tsx (1)
16-18:setIsOpenis passed toPartnerGroupHistorySheetContentbut never used.The
Omit<…, "isOpen">type includessetIsOpen, and the...restspread inPartnerGroupHistorySheetforwards it to the content component. However, the content component only usespartner— closing is handled bySheet.Close. Consider narrowing the content component's props to just what it needs.Proposed fix
-function PartnerGroupHistorySheetContent({ - partner, -}: Omit<PartnerGroupHistorySheetProps, "isOpen">) { +function PartnerGroupHistorySheetContent({ + partner, +}: Pick<PartnerGroupHistorySheetProps, "partner">) {And in
PartnerGroupHistorySheet:export function PartnerGroupHistorySheet({ isOpen, - ...rest + setIsOpen, + partner, }: PartnerGroupHistorySheetProps) { return ( - <Sheet open={isOpen} onOpenChange={rest.setIsOpen}> - <PartnerGroupHistorySheetContent {...rest} /> + <Sheet open={isOpen} onOpenChange={setIsOpen}> + <PartnerGroupHistorySheetContent partner={partner} /> </Sheet> ); }
|
@coderabbitai full review, any breaking changes? |
|
🔴 Breaking Changes1. Database Schema Migration RequiredFile:
2. WorkflowIdentity Interface - Required Field AddedFile:
3. API Parameter Type ChangesFile:
4. Zod Schema ChangesFile:
5. Type Export RelocationsFiles:
6. API Behavior ChangeFile:
✅ MitigationMost breaking changes are internal and already handled in the PR. The main external concerns are:
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/prisma/schema/activity.prisma (1)
1-16:⚠️ Potential issue | 🟠 MajorAdd indexes on
programIdandcreatedAtfor query performance.The activity-logs API filters by
programIdand orders results bycreatedAtin descending order. Without indexes on these columns, queries will result in full table scans as the table grows. A composite index onprogramIdandcreatedAtwould significantly improve query performance.Suggested index additions
@@index([resourceType, resourceId]) + @@index([programId, createdAt]) }
🤖 Fix all issues with AI agents
In `@apps/web/ui/activity-logs/partner-group-activity-item.tsx`:
- Around line 44-51: formatDate is being called without the day option so
timestamps omit the day; update the formatDate calls that pass log.createdAt in
PartnerGroupActivityItem (partner-group-activity-item.tsx) and
ReferralActivityItem (referral-activity-item.tsx) to include day: "numeric" in
the options object so dates render like "Feb 6, 2026, 2:30 PM"; search for
formatDate(log.createdAt, {...}) in those components and add day: "numeric" to
the options.
🧹 Nitpick comments (10)
apps/web/lib/integrations/shopify/create-sale.ts (1)
164-178: Nit: use shorthand property.
workspaceId: workspaceIdcan be simplified toworkspaceId, consistent with howprogramIdandpartnerIdare used elsewhere in this codebase (e.g., line 394 increate-partner-commission.tsdoesn't repeatprogramId: programId).♻️ Suggested diff
identity: { - workspaceId: workspaceId, + workspaceId, programId: link.programId, partnerId: link.partnerId, },apps/web/app/api/activity-logs/route.ts (1)
20-23: Passing optionalactiondirectly into thewhereclause works but is implicit.When
actionisundefined, Prisma silently ignores it. This is correct behavior, but for clarity you could use a conditional spread:♻️ Optional: explicit conditional spread
where: { programId, resourceType, resourceId, - action, + ...(action && { action }), },apps/web/ui/activity-logs/action-renderers/partner-group-changed-renderer.tsx (1)
7-10:oldfield inGroupChangeSetis defined but never read.The
oldgroup is part of the type but the renderer only usesnew. If this is intentional (for future use or other renderers), that's fine — just noting it for awareness.apps/web/app/(ee)/api/cron/groups/remap-default-links/route.ts (1)
251-258:error.messagemay throw iferroris not anErrorinstance.If a non-Error value is thrown (e.g., a string or object), accessing
.messagewill returnundefined. Consider using a safer pattern.🛡️ Safer error message extraction
- message: `Error creating default links for the partners: ${error.message}.`, + message: `Error creating default links for the partners: ${error instanceof Error ? error.message : String(error)}.`,apps/web/ui/partners/partner-info-cards.tsx (1)
85-89:usePartnerGroupHistorySheetfires for network partners too.The hook is called unconditionally, so the underlying
useActivityLogsSWR request will fire even fortype: "network"partners (who never see the "View history" button). This is harmless since SWR is lightweight, but you could passpartner: isEnrolled ? (partner || null) : nullto skip the fetch entirely for network partners.♻️ Optional: skip fetch for non-enrolled partners
const { partnerGroupHistorySheet, setIsOpen: setGroupHistoryOpen, hasActivityLogs, - } = usePartnerGroupHistorySheet({ partner: partner || null }); + } = usePartnerGroupHistorySheet({ + partner: isEnrolled ? (partner || null) : null, + });apps/web/ui/activity-logs/referral-activity-item.tsx (1)
46-53: Consider adding adateTimeattribute to the<time>element.The
<time>element renders a human-readable date but lacks the machine-readabledateTimeattribute. This is a minor semantic HTML / accessibility improvement.♻️ Add dateTime attribute
- <time className="hidden shrink-0 text-xs text-neutral-500 sm:block"> + <time + dateTime={log.createdAt} + className="hidden shrink-0 text-xs text-neutral-500 sm:block" + >apps/web/lib/swr/use-activity-logs.ts (1)
15-22:workspaceId!non-null assertion runs before the SWR guard.
searchParamsis built every render withworkspaceId!, even whenworkspaceIdis stillundefined(workspace loading). The SWR key guard on line 26 prevents an actual fetch, so this isn't a runtime bug, but it produces a transient"workspaceId=undefined"string. Moving the construction inside the SWR key callback (or guarding it) would be cleaner.Proposed fix
- const searchParams = query - ? new URLSearchParams({ - workspaceId: workspaceId!, - resourceType: query.resourceType, - resourceId: query.resourceId, - ...(query.action && { action: query.action }), - }).toString() - : ""; - const { data, error, isLoading, mutate } = useSWR<ActivityLog[]>( enabled && workspaceId && query?.resourceType && - query?.resourceId && - `/api/activity-logs?${searchParams}`, + query?.resourceId && + `/api/activity-logs?${new URLSearchParams({ + workspaceId, + resourceType: query.resourceType, + resourceId: query.resourceId, + ...(query.action && { action: query.action }), + }).toString()}`, fetcher,apps/web/ui/activity-logs/activity-log-registry.tsx (1)
16-27:referral.updatedandreward.*actions have no icon or renderer entries.These actions exist in
activityLogActionSchemabut aren't mapped here. If they can appear in the activity feed, they'll render the fallbackCircleInfoicon andgetActivityLogRendererwill returnnull. Just confirming this is intentional for now (perhaps future work).apps/web/ui/activity-logs/partner-group-history-sheet.tsx (1)
16-39:setIsOpenis passed but unused inPartnerGroupHistorySheetContent.The
Omit<PartnerGroupHistorySheetProps, "isOpen">type includessetIsOpen, which gets spread into this component but is never consumed — the close button works throughSheet.Closecontext instead. Not a bug, just a minor type surface nit.Tighter type for the content component
function PartnerGroupHistorySheetContent({ partner, -}: Omit<PartnerGroupHistorySheetProps, "isOpen">) { +}: Pick<PartnerGroupHistorySheetProps, "partner">) {apps/web/ui/activity-logs/activity-entry-chips.tsx (1)
71-82: Consider addingreferrerPolicyto the avatar<img>to avoid leaking origin.The avatar loads from a user-controlled URL (
user.image) or the OG fallback. Ifuser.imagepoints to an external domain, the browser may send the fullRefererheader.Minor hardening
<img src={user.image || `${OG_AVATAR_URL}${user.id}`} alt={`${user.name || user.email || "User"}`} className="size-4 shrink-0 rounded-full" + referrerPolicy="no-referrer" />
1 participant
Summary by CodeRabbit
New Features
Bug Fixes / Improvements