Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds UTM plumbing across site and blog: new utm libs, client-only UtmPersistence components that persist/propagate UTM query params and rewrite/reroute qualifying links, NavigationWrapper components that resolve defaults vs. URL UTMs, WebNavigation prop changes, and Console CTA components/updates. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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. 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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![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: 3
🤖 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/site/src/components/navigation-wrapper.tsx`:
- Around line 60-79: The initial useState for storedUtmParams only sets
utm_source, causing other UTM fields to be undefined on first render; change the
initial state to synchronously read persisted values by calling
readStoredUtmParams() (and merging fallbacks from utm.source/utm.medium) so
storedUtmParams is fully populated immediately, then keep the existing useEffect
to update/write via getUtmParams and writeStoredUtmParams; apply the same
synchronous initialization fix to the other storedUtmParams instance referenced
later (the one updated in the second block using setStoredUtmParams).
In `@apps/site/src/lib/utm.ts`:
- Around line 34-50: readStoredUtmParams currently reads
window.sessionStorage.getItem outside a try/catch and casts parsed JSON to
UtmParams without validation; update readStoredUtmParams to wrap sessionStorage
access (getItem) and JSON.parse in try/catch, and validate/filter the parsed
object so it only returns keys that match the utm_* pattern with string values
before returning {}; for writeStoredUtmParams wrap sessionStorage.setItem in
try/catch so failures in restricted environments are swallowed gracefully, and
ensure you only stringify and store a validated object (use hasUtmParams as a
guard) to avoid persisting malformed data.
In `@packages/ui/src/components/web-navigation.tsx`:
- Around line 40-45: The utm prop in WebNavigation (and the forwarding helper
around it) currently narrows the persisted UTM payload to only source, medium,
campaign, content, and term which drops any other utm_* keys; change the
prop/type and forwarding logic to accept a generic record of arbitrary utm_*
entries (e.g., Record<string,string | undefined> or a generic object) and
forward all keys through, and only synthesize/default utm_campaign when that
specific key is absent (preserving other keys like utm_id or
utm_source_platform); update the component/utility signatures that reference utm
(the utm prop and the helper that builds the login/get-started URL) to use the
broader type and pass through the entire map.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d96bc111-8286-4eef-bbec-fe1b9aafb530
📒 Files selected for processing (5)
apps/site/src/app/layout.tsxapps/site/src/components/navigation-wrapper.tsxapps/site/src/components/utm-persistence.tsxapps/site/src/lib/utm.tspackages/ui/src/components/web-navigation.tsx
Preserve arbitrary persisted UTM fields across site and console navigation while making storage access safer and available on first render. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/site/src/components/navigation-wrapper.tsx`:
- Around line 7-12: The current logic in UtmPersistence (and in the consumers in
navigation-wrapper.tsx) only treats payloads with utm_source as valid, causing
stored single-field params like utm_campaign or utm_content to be dropped;
update the acceptance/normalization logic to consider any non-empty UtmParams
(i.e., any utm_* field present) as valid rather than requiring utm_source, and
ensure getUtmParams/readStoredUtmParams/writeStoredUtmParams and the
WebNavigation prop forwarding preserve all present optional fields (UtmParams)
when merging or persisting. Ensure checks use a generic "hasAnyUtmField"
predicate and update places referencing UtmPersistence, getUtmParams, and
WebNavigation to use that predicate so partial UTM objects are kept.
In `@apps/site/src/components/utm-persistence.tsx`:
- Around line 45-91: The click handler mutates the DOM href which Next.js Link
ignores; update handleClick (inside the useEffect) to, after computing targetUrl
and merging UTM params via mergeUtmParams/readStoredUtmParams, prevent the
default navigation and perform client-side navigation using Next's router (e.g.,
call router.push with
`${targetUrl.pathname}${targetUrl.search}${targetUrl.hash}`) for same-origin
internal links instead of setting anchor.setAttribute, while preserving existing
guards for blank/modified/new-tab clicks and download/mailto/tel; ensure you
import/use Next's useRouter or next/router and only call router.push when
anchor.target is not "_blank" and no modifier keys are pressed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68430cc9-93ea-4362-8458-246c74aa941f
📒 Files selected for processing (2)
apps/site/src/components/navigation-wrapper.tsxapps/site/src/components/utm-persistence.tsx
Extend the site UTM persistence handler to rewrite console.prisma.io links and simplify nav UTM resolution so page-level CTAs inherit the stored campaign parameters. Made-with: Cursor
Use the incoming UTM set exactly when one is present, fall back to defaults only when no UTM exists, and bring the same persistence behavior to the blog shell. Made-with: Cursor
Avoid nav hydration mismatches by deferring exact UTM resolution until after mount and apply the same exact-or-default UTM handling to the homepage create database CTAs. Made-with: Cursor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/blog/src/components/navigation-wrapper.tsx (1)
6-20: Consider extracting sharedLinkinterface to reduce duplication.This interface is duplicated from
packages/ui/src/components/web-navigation.tsx. While keeping it local avoids cross-package type dependencies, it creates a maintenance burden where changes must be synchronized manually.Consider exporting the
Linktype from@prisma-docs/uialongsideWebNavigationto ensure type consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blog/src/components/navigation-wrapper.tsx` around lines 6 - 20, The local Link interface in navigation-wrapper.tsx duplicates the Link type defined in WebNavigation (packages/ui/src/components/web-navigation.tsx); export the shared Link type from `@prisma-docs/ui` and import it here instead of redefining it. Update the UI package to export the Link type alongside the WebNavigation component, then replace the local interface in navigation-wrapper.tsx with an import of Link from '@prisma-docs/ui' and remove the duplicated declaration so both packages share a single source of truth.apps/site/src/lib/utm.ts (1)
1-101: Consider extracting this shared UTM helper instead of maintaining a second copy.This is almost a verbatim copy of
apps/blog/src/lib/utm.tsaside from the storage key. Any fix to sanitization, sync behavior, or storage semantics now has to land twice, and the two apps can drift silently. Pulling the shared logic into one helper and injecting the key only where it truly differs would make future changes safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/lib/utm.ts` around lines 1 - 101, Duplicate UTM logic exists (UTM_STORAGE_KEY, sanitizeUtmParams, getUtmParams, syncUtmParams, readStoredUtmParams, writeStoredUtmParams, clearStoredUtmParams) and should be extracted into a shared helper module to avoid drift; create a single shared utm utility (exporting sanitizeUtmParams, getUtmParams, hasUtmParams, syncUtmParams, readStoredUtmParams, writeStoredUtmParams, clearStoredUtmParams) in a common package or lib and update this file to import those functions, keeping only the app-specific storage key injection (UTM_STORAGE_KEY) or a small wrapper that passes the key into the shared write/read functions so storage key remains configurable without duplicating logic.
🤖 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/site/src/components/utm-persistence.tsx`:
- Around line 18-29: The effect currently clears stored UTMs whenever the
current route lacks utm_* which loses the original campaign; instead, when
getUtmParams(...) yields no utm params, call readStoredUtmParams() and if that
returns a stored value, keep/use it (do not call clearStoredUtmParams()); only
clear stored UTMs when explicitly needed (e.g., when a new campaign is detected
or on session end). Update the logic around getUtmParams, hasUtmParams,
writeStoredUtmParams, clearStoredUtmParams and the other block that currently
returns without reading readStoredUtmParams() so it falls back to
readStoredUtmParams() before deciding to drop UTMs.
---
Nitpick comments:
In `@apps/blog/src/components/navigation-wrapper.tsx`:
- Around line 6-20: The local Link interface in navigation-wrapper.tsx
duplicates the Link type defined in WebNavigation
(packages/ui/src/components/web-navigation.tsx); export the shared Link type
from `@prisma-docs/ui` and import it here instead of redefining it. Update the UI
package to export the Link type alongside the WebNavigation component, then
replace the local interface in navigation-wrapper.tsx with an import of Link
from '@prisma-docs/ui' and remove the duplicated declaration so both packages
share a single source of truth.
In `@apps/site/src/lib/utm.ts`:
- Around line 1-101: Duplicate UTM logic exists (UTM_STORAGE_KEY,
sanitizeUtmParams, getUtmParams, syncUtmParams, readStoredUtmParams,
writeStoredUtmParams, clearStoredUtmParams) and should be extracted into a
shared helper module to avoid drift; create a single shared utm utility
(exporting sanitizeUtmParams, getUtmParams, hasUtmParams, syncUtmParams,
readStoredUtmParams, writeStoredUtmParams, clearStoredUtmParams) in a common
package or lib and update this file to import those functions, keeping only the
app-specific storage key injection (UTM_STORAGE_KEY) or a small wrapper that
passes the key into the shared write/read functions so storage key remains
configurable without duplicating logic.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85815350-a1ad-4e02-a806-630a507b1303
📒 Files selected for processing (8)
apps/blog/src/app/(blog)/layout.tsxapps/blog/src/components/navigation-wrapper.tsxapps/blog/src/components/utm-persistence.tsxapps/blog/src/lib/utm.tsapps/site/src/components/navigation-wrapper.tsxapps/site/src/components/utm-persistence.tsxapps/site/src/lib/utm.tspackages/ui/src/components/web-navigation.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/blog/src/app/(blog)/layout.tsx
- apps/site/src/components/navigation-wrapper.tsx
Comment on lines
+18
to
+29
| useEffect(() => { | ||
| const currentUtmParams = getUtmParams( | ||
| new URLSearchParams(window.location.search), | ||
| ); | ||
|
|
||
| if (hasUtmParams(currentUtmParams)) { | ||
| writeStoredUtmParams(currentUtmParams); | ||
| return; | ||
| } | ||
|
|
||
| clearStoredUtmParams(); | ||
| }, [pathname]); |
Contributor
There was a problem hiding this comment.
Fall back to stored UTMs instead of clearing them on the first clean URL.
Lines 18-29 clear the session as soon as the next route lacks utm_*, and Lines 57-63 return without reading readStoredUtmParams(). A redirect, router.push(), or any internal navigation this listener doesn't rewrite will drop the original campaign for the rest of the session, so this behaves like link propagation rather than persistence.
Also applies to: 57-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/site/src/components/utm-persistence.tsx` around lines 18 - 29, The
effect currently clears stored UTMs whenever the current route lacks utm_* which
loses the original campaign; instead, when getUtmParams(...) yields no utm
params, call readStoredUtmParams() and if that returns a stored value, keep/use
it (do not call clearStoredUtmParams()); only clear stored UTMs when explicitly
needed (e.g., when a new campaign is detected or on session end). Update the
logic around getUtmParams, hasUtmParams, writeStoredUtmParams,
clearStoredUtmParams and the other block that currently returns without reading
readStoredUtmParams() so it falls back to readStoredUtmParams() before deciding
to drop UTMs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/site/src/app/(index)/page.tsx (1)
121-125: Extract repeated default UTM config to a shared constant.The same
defaultUtmobject appears twice. Pulling it into one constant reduces drift and keeps campaign edits one-touch.Proposed refactor
+const INDEX_CTA_DEFAULT_UTM = { + utm_source: "website", + utm_medium: "index", + utm_campaign: "cta", +} as const; + export default function SiteHome() { return ( @@ - defaultUtm={{ - utm_source: "website", - utm_medium: "index", - utm_campaign: "cta", - }} + defaultUtm={INDEX_CTA_DEFAULT_UTM} @@ - defaultUtm={{ - utm_source: "website", - utm_medium: "index", - utm_campaign: "cta", - }} + defaultUtm={INDEX_CTA_DEFAULT_UTM}Also applies to: 259-263
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/site/src/app/`(index)/page.tsx around lines 121 - 125, Duplicate inline defaultUtm objects are used; extract them into a single shared constant (e.g. const DEFAULT_UTM = { utm_source: "website", utm_medium: "index", utm_campaign: "cta" }) and replace both inline usages of defaultUtm with DEFAULT_UTM; update any components or props that currently pass the inline object (the places where defaultUtm: {...} is provided) to reference DEFAULT_UTM so edits to campaign/medium/source are centralized.
🤖 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/site/src/components/console-cta-button.tsx`:
- Around line 38-42: The code currently replaces defaultUtm with
currentUtmParams when hasUtmParams(...) is true, causing missing keys; instead
merge defaults with incoming UTMs: create a mergedUtm object that starts from
defaultUtm and overlays any non-empty properties from currentUtmParams, then
pass mergedUtm to buildConsoleHref (change call sites around
setHref/buildConsoleHref and reference hasUtmParams, currentUtmParams,
defaultUtm). Ensure you only override when the URL value is present (non-empty)
so unspecified defaults are preserved.
---
Nitpick comments:
In `@apps/site/src/app/`(index)/page.tsx:
- Around line 121-125: Duplicate inline defaultUtm objects are used; extract
them into a single shared constant (e.g. const DEFAULT_UTM = { utm_source:
"website", utm_medium: "index", utm_campaign: "cta" }) and replace both inline
usages of defaultUtm with DEFAULT_UTM; update any components or props that
currently pass the inline object (the places where defaultUtm: {...} is
provided) to reference DEFAULT_UTM so edits to campaign/medium/source are
centralized.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58bbda39-8891-4c9b-af6d-86846e75bd4a
📒 Files selected for processing (4)
apps/blog/src/components/navigation-wrapper.tsxapps/site/src/app/(index)/page.tsxapps/site/src/components/console-cta-button.tsxapps/site/src/components/navigation-wrapper.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/site/src/components/navigation-wrapper.tsx
Use the anchor variant of the shared Button props so homepage console CTAs accept link-only attributes like target and rel during type checking. Made-with: Cursor
Extract the shared homepage CTA UTM payload into a single constant so the create database buttons stay aligned when campaign defaults change. Made-with: Cursor
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.
Summary by CodeRabbit