feat(frontend): add trial banner with eye-tracking and sparkle CTA#1663
feat(frontend): add trial banner with eye-tracking and sparkle CTA#1663
Conversation
- Add TrialBanner component with emoji-style eyes that follow the cursor - Pupils scale up when cursor approaches the CTA button - Always-on sparkle particles and shimmer effect on View Plans button - Add i18n keys for trial banner message and CTA - Mount banner on dashboard page for trial users - Hide Expert-as-a-Service CTA for non-paying orgs
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughAdds a new TrialBanner Vue component and global registration, two English translation keys, non-nullable Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cf1d89617
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
🧹 Nitpick comments (3)
src/components/dashboard/TrialBanner.vue (3)
92-100: MissingrequestAnimationFramethrottling — claimed in the PR description but not implemented.The
mousemoveevent can fire at 200+ Hz on high-refresh displays. Each call triggers two Vue reactive ref assignments and, when the banner is visible, agetBoundingClientRect()read (forces a layout flush). RAF throttling would cap updates to one per animation frame:♻️ Suggested RAF throttle
+let rafId: number | null = null +let pendingEvent: MouseEvent | null = null + function handleMouseMove(e: MouseEvent) { + if (!showBanner.value) return + pendingEvent = e + if (rafId !== null) return + rafId = requestAnimationFrame(() => { + rafId = null + if (!pendingEvent) return + const ev = pendingEvent + pendingEvent = null - leftPupil.value = calcOffset(leftEye.value, e) - rightPupil.value = calcOffset(rightEye.value, e) + leftPupil.value = calcOffset(leftEye.value, ev) + rightPupil.value = calcOffset(rightEye.value, ev) if (ctaRef.value) { const el = ctaRef.value.$el ?? ctaRef.value - const ctaRect = (el as HTMLElement).getBoundingClientRect() - excited.value = distToRect(e.clientX, e.clientY, ctaRect) < EXCITE_DISTANCE + const ctaRect = (el as HTMLElement).getBoundingClientRect() + excited.value = distToRect(ev.clientX, ev.clientY, ctaRect) < EXCITE_DISTANCE } + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/TrialBanner.vue` around lines 92 - 100, handleMouseMove currently runs on every mousemove and does two reactive assignments plus a getBoundingClientRect, causing layout thrashing; implement RAF throttling by queuing the latest MouseEvent and scheduling a single requestAnimationFrame if one isn’t pending, then in the RAF callback call calcOffset for leftEye/rightEye, set leftPupil/rightPupil, and compute the ctaRect/getBoundingClientRect and set excited based on distToRect and EXCITE_DISTANCE; use a module-scoped rafId (or boolean rafPending) and a latestEvent variable so handleMouseMove just updates latestEvent and schedules RAF.
227-265: CTA button uses fully custom scoped CSS rather than a DaisyUI base.The interactive CTA is rendered as a
<router-link>styled entirely with scoped.cta-button/.cta-sparkleclasses. The complex shimmer/sparkle animations do require custom CSS, but the structural button styles (padding, border-radius, font weight, colours) could be composed from ad-btnbase, keeping animation overrides in scoped styles. As per coding guidelines, DaisyUI components should be used for interactive elements in Vue components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/TrialBanner.vue` around lines 227 - 265, Replace the fully custom structural styling on the CTA with DaisyUI base classes: add the d-btn (and variant like d-btn-primary) to the <router-link> that currently uses the .cta-button class, remove structural rules from .cta-button (padding, border-radius, font-size, font-weight, background-color, color, box-shadow) so those come from DaisyUI, and keep only the scoped animation/visual overrides (the shimmer/sparkle rules for .cta-sparkle and any transform/overflow/transition tweaks) in TrialBanner.vue; ensure hover/focus tweaks are implemented as overrides that target .cta-button.d-btn and preserve the dark .cta-button:focus override logic.
95-99:$el ?? ctaRef.valuefallback is type-unsafe dead code.
$elis always set on a mounted component instance, so the nullish-coalescing fallback?? ctaRef.valueis never reached. If it were, passing aComponentPublicInstanceas anHTMLElementwould still throw at runtime. The cast is safe to simplify:🔧 Proposed cleanup
- const el = ctaRef.value.$el ?? ctaRef.value - const ctaRect = (el as HTMLElement).getBoundingClientRect() + const ctaRect = (ctaRef.value.$el as HTMLElement).getBoundingClientRect()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/TrialBanner.vue` around lines 95 - 99, The current nullish-coalescing fallback "$el ?? ctaRef.value" is dead/unsafe; replace it by using the mounted component's element directly and a proper HTMLElement cast: grab ctaRef.value.$el and cast it to HTMLElement (e.g., treat it as HTMLElement) before calling getBoundingClientRect, remove the ?? ctaRef.value fallback and the broad cast on el, and keep the existing logic using distToRect, EXCITE_DISTANCE and excited so the computed distance uses the actual HTMLElement returned from the component instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/dashboard/TrialBanner.vue`:
- Around line 102-108: The component currently adds a global window mousemove
listener unconditionally in onMounted and removes it in onUnmounted which causes
unnecessary work for non-trial users; update the mount/unmount logic so the
listener is only added when the banner is actually shown (check the reactive
showBanner) and removed when it becomes hidden, and also add an early-return at
the start of handleMouseMove if leftEye/rightEye/ctaRef are null to avoid
allocations; ensure you reference the existing functions/refs (onMounted,
onUnmounted, handleMouseMove, showBanner, leftEye, rightEye, ctaRef) and keep
cleanup logic to remove the listener whenever showBanner toggles or the
component unmounts.
- Around line 40-47: The computed isAccountOldEnough currently only recalculates
when currentOrg changes, so Date.now() is frozen for the session; change the
logic so age is evaluated continuously by introducing a reactive time tick
(e.g., a ref updated on an interval in onMounted and cleared in onUnmounted) and
reference that tick inside isAccountOldEnough so the computed re-evaluates as
time passes; also explicitly handle org.subscription_start === null (don’t treat
it the same as undefined) by returning a distinct result or boolean (e.g., false
or an “unknown” flag) so the UI can distinguish null subscription_start cases
from “too new” accounts—update references to isAccountOldEnough, currentOrg and
subscription_start accordingly.
---
Nitpick comments:
In `@src/components/dashboard/TrialBanner.vue`:
- Around line 92-100: handleMouseMove currently runs on every mousemove and does
two reactive assignments plus a getBoundingClientRect, causing layout thrashing;
implement RAF throttling by queuing the latest MouseEvent and scheduling a
single requestAnimationFrame if one isn’t pending, then in the RAF callback call
calcOffset for leftEye/rightEye, set leftPupil/rightPupil, and compute the
ctaRect/getBoundingClientRect and set excited based on distToRect and
EXCITE_DISTANCE; use a module-scoped rafId (or boolean rafPending) and a
latestEvent variable so handleMouseMove just updates latestEvent and schedules
RAF.
- Around line 227-265: Replace the fully custom structural styling on the CTA
with DaisyUI base classes: add the d-btn (and variant like d-btn-primary) to the
<router-link> that currently uses the .cta-button class, remove structural rules
from .cta-button (padding, border-radius, font-size, font-weight,
background-color, color, box-shadow) so those come from DaisyUI, and keep only
the scoped animation/visual overrides (the shimmer/sparkle rules for
.cta-sparkle and any transform/overflow/transition tweaks) in TrialBanner.vue;
ensure hover/focus tweaks are implemented as overrides that target
.cta-button.d-btn and preserve the dark .cta-button:focus override logic.
- Around line 95-99: The current nullish-coalescing fallback "$el ??
ctaRef.value" is dead/unsafe; replace it by using the mounted component's
element directly and a proper HTMLElement cast: grab ctaRef.value.$el and cast
it to HTMLElement (e.g., treat it as HTMLElement) before calling
getBoundingClientRect, remove the ?? ctaRef.value fallback and the broad cast on
el, and keep the existing logic using distToRect, EXCITE_DISTANCE and excited so
the computed distance uses the actual HTMLElement returned from the component
instance.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
messages/en.jsonsrc/components.d.tssrc/components/dashboard/TrialBanner.vuesrc/pages/dashboard.vuesrc/pages/settings/organization/Plans.vue
…ak & time reactivity - Use org.created_at instead of subscription_start for the 3-hour age gate. subscription_start was the billing-cycle anchor (bc.cycle_start), which defaults to the start of the current month for trial orgs without Stripe subscriptions — causing the banner to show immediately after signup. - Add created_at to get_orgs_v7 return type via new migration. - Only attach the global mousemove listener when the banner is actually visible (via watch on showBanner). Previously it fired at ~60 Hz for all dashboard users including non-trial/paying users. - Add a reactive time tick (nowTick, updated every 60s) so isAccountOldEnough re-evaluates during long sessions without requiring a page reload.
WcaleNieWolny
force-pushed
the
feat/trial-banner-eyes-sparkle
branch
from
ccaa212 to
3ab393d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
supabase/functions/_backend/utils/supabase.types.ts (1)
3201-3234:⚠️ Potential issue | 🟡 MinorConfirm
created_attype safety inget_orgs_v7function.The
orgs.created_atcolumn is nullable (defined astimestamp with time zone DEFAULT "now" ()with noNOT NULLconstraint), butget_orgs_v7declares it as non-nullable (created_at timestamptz). The function selects the column without a COALESCE, so if any org row has NULL, callers will receive null values despite the type declaring the field as always present. Either add aNOT NULLconstraint to the column, coalesce in the SQL, or mark the type as nullable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/supabase.types.ts` around lines 3201 - 3234, The get_orgs_v7 return type declares created_at as non-nullable but the orgs.created_at column is nullable; update the code to ensure runtime/type alignment by either (A) marking created_at as nullable in the get_orgs_v7 return type (i.e., allow string | null) so callers can handle null, or (B) change the SQL that populates get_orgs_v7 to coalesce(created_at, '1970-01-01T00:00:00Z') (or another safe default) before returning so the field is guaranteed non-null; locate references to get_orgs_v7 and the created_at field in supabase.types.ts and apply one of these two fixes consistently across the function signature and its SQL selection.src/types/supabase.types.ts (1)
3201-3234:⚠️ Potential issue | 🟡 MinorMake
created_atnullable in TypeScript types to match the database column definition.The
created_atcolumn in theorgstable is nullable (noNOT NULLconstraint), but the function returns it unwrapped without null handling. The TypeScript types currently showcreated_at: string(non-nullable), which will mislead UI code if a null value is returned. Update the type tocreated_at: string | nullto align with the database contract.Type correction for both function overloads
- created_at: string + created_at: string | nullAlso applies to: 3237-3269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/supabase.types.ts` around lines 3201 - 3234, The generated TypeScript return types for the RPC that include the org record have the created_at field typed as non-nullable string; update the Returns object(s) in src/types/supabase.types.ts so that the created_at property is typed as string | null (apply this change to both overloaded Returns blocks that define the org shape) to match the database column; locate the Returns {...} arrays that include "created_at" and replace its type accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/dashboard/TrialBanner.vue`:
- Around line 131-167: The CTA router-link in TrialBanner.vue (ref="ctaRef",
currently class="cta-button cta-sparkle") is missing the DaisyUI base button
class; update the router-link's class list to include the d-btn base (and any
appropriate variant like d-btn-primary or sizing classes as needed) so the CTA
uses DaisyUI button styling while preserving existing sparkle classes (e.g., add
d-btn alongside cta-button and cta-sparkle).
- Around line 63-107: The constants MAX_TRAVEL and EXCITE_DISTANCE should be
renamed to camelCase (e.g., maxTravel and exciteDistance) and all references
updated; update the definition of MAX_TRAVEL used in calcOffset to maxTravel and
update EXCITE_DISTANCE used in handleMouseMove (and the comparison with
distToRect) to exciteDistance, ensuring calcOffset, handleMouseMove and any
other function (distToRect usage) reference the new names consistently.
In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql`:
- Around line 115-130: The boolean result columns paying, can_use_more, and
is_canceled can become NULL when the LEFT JOINed stripe_info (si) is absent;
update each CASE (the ones producing aliases paying, can_use_more, is_canceled)
to wrap their ELSE boolean expression with COALESCE(..., false) (or wrap the
whole CASE result in COALESCE(..., false)) so that when si is NULL these columns
return false instead of NULL; reference the CASEs that inspect si.status,
si.is_good_plan, si.trial_at and the redaction flags tfa.should_redact_2fa /
ppa.should_redact_password to locate the exact expressions to change.
- Line 42: The get_orgs_v7(uuid) function is declared STABLE but calls VOLATILE
helpers (has_2fa_enabled, user_meets_password_policy, get_next_cron_time);
update the function declaration in get_orgs_v7 to use VOLATILE instead of STABLE
so the stability contract matches the callees and avoids runtime/cache errors.
Locate the function header containing "get_orgs_v7(uuid) ... STABLE SECURITY
DEFINER" and replace STABLE with VOLATILE, leaving the rest of the signature and
body intact.
---
Outside diff comments:
In `@src/types/supabase.types.ts`:
- Around line 3201-3234: The generated TypeScript return types for the RPC that
include the org record have the created_at field typed as non-nullable string;
update the Returns object(s) in src/types/supabase.types.ts so that the
created_at property is typed as string | null (apply this change to both
overloaded Returns blocks that define the org shape) to match the database
column; locate the Returns {...} arrays that include "created_at" and replace
its type accordingly.
In `@supabase/functions/_backend/utils/supabase.types.ts`:
- Around line 3201-3234: The get_orgs_v7 return type declares created_at as
non-nullable but the orgs.created_at column is nullable; update the code to
ensure runtime/type alignment by either (A) marking created_at as nullable in
the get_orgs_v7 return type (i.e., allow string | null) so callers can handle
null, or (B) change the SQL that populates get_orgs_v7 to coalesce(created_at,
'1970-01-01T00:00:00Z') (or another safe default) before returning so the field
is guaranteed non-null; locate references to get_orgs_v7 and the created_at
field in supabase.types.ts and apply one of these two fixes consistently across
the function signature and its SQL selection.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/dashboard/TrialBanner.vuesrc/types/supabase.types.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql
supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql
Outdated
Show resolved
Hide resolved
…rval - Wrap created_at in the same 2FA/password-policy redaction CASE as all other sensitive fields in get_orgs_v7, returning NULL when the user lacks compliance. - Move the 60s nowTick interval into the showBanner watcher so it only runs when the banner is visible, consistent with the mousemove listener optimization.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/components/dashboard/TrialBanner.vue (2)
160-166:⚠️ Potential issue | 🟡 MinorApply DaisyUI base button class to the CTA.
This is an interactive element and should use the DaisyUI
d-btnbase class.Minimal fix
- class="cta-button cta-sparkle" + class="d-btn cta-button cta-sparkle"As per coding guidelines, "Use DaisyUI components (
d-btn,d-input,d-card) for interactive elements in Vue components".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/TrialBanner.vue` around lines 160 - 166, The CTA router-link (ref="ctaRef", currently class="cta-button cta-sparkle") must include the DaisyUI base button class; update the router-link's class attribute to add "d-btn" (e.g., class="d-btn cta-button cta-sparkle") so the interactive element uses the required DaisyUI button component styling.
63-106:⚠️ Potential issue | 🟡 MinorRename MAX_TRAVEL/EXCITE_DISTANCE to camelCase.
Uppercase constants violate the camelCase rule for TS/JS variables.
Suggested update
-const MAX_TRAVEL = 4 // How far the pupil can move from center (px) +const maxTravel = 4 // How far the pupil can move from center (px) ... - const easedDist = Math.min(dist * 0.1, MAX_TRAVEL) + const easedDist = Math.min(dist * 0.1, maxTravel) ... -const EXCITE_DISTANCE = 80 // px from CTA edge to trigger excitement +const exciteDistance = 80 // px from CTA edge to trigger excitement ... - excited.value = distToRect(e.clientX, e.clientY, ctaRect) < EXCITE_DISTANCE + excited.value = distToRect(e.clientX, e.clientY, ctaRect) < exciteDistanceAs per coding guidelines, "Use camelCase for variable names in TypeScript and JavaScript".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/TrialBanner.vue` around lines 63 - 106, The constants MAX_TRAVEL and EXCITE_DISTANCE are using UPPER_SNAKE_CASE but must follow camelCase; rename them to maxTravel and exciteDistance and update all usages (change the declaration const MAX_TRAVEL = 4 to const maxTravel = 4 and const EXCITE_DISTANCE = 80 to const exciteDistance = 80), then update references inside calcOffset (where MAX_TRAVEL is used) and inside handleMouseMove (where EXCITE_DISTANCE is compared) so the identifiers match the new camelCase names.supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql (2)
44-44:get_orgs_v7(uuid)is still declaredSTABLEwhile callingVOLATILEhelpersThis was flagged on a previous commit and remains unresolved. The function calls
has_2fa_enabled,user_meets_password_policy, andget_next_cron_time, all of which areVOLATILE. ASTABLEfunction "cannot modify the database and is guaranteed to return the same results given the same arguments for all rows within a single statement." CallingVOLATILEhelpers breaks this contract and can produce stale cached results.Change
STABLEtoVOLATILEon line 44.🐛 Proposed fix
-) LANGUAGE plpgsql STABLE SECURITY DEFINER +) LANGUAGE plpgsql VOLATILE SECURITY DEFINER🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql` at line 44, The function declaration for get_orgs_v7(uuid) is incorrectly marked STABLE while it calls VOLATILE helpers (has_2fa_enabled, user_meets_password_policy, get_next_cron_time); update the function declaration keyword from STABLE to VOLATILE so the planner knows the function can return varying results within a statement and avoid caching/staleness issues.
149-166:paying,can_use_more, andis_canceledstill returnNULLinstead offalsewhen no stripe row existsThis was flagged on a previous commit and remains unresolved. The
stripe_infojoin is aLEFT JOIN(line 221), so when no billing record exists theELSEbranches on lines 151, 159–161, and 165 all evaluate toNULL. Wrap eachELSEexpression withCOALESCE(..., false)as suggested in the prior review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql` around lines 149 - 166, The CASE branches for paying, can_use_more, and is_canceled currently return NULL when no stripe_info (si) row exists; update each ELSE expression in the CASE for fields paying, can_use_more, and is_canceled to wrap the boolean result with COALESCE(..., false) (e.g., COALESCE((si.status = 'succeeded'), false)), so that when the LEFT JOIN yields NULL you get false; reference the aliases si, tfa, ppa, and ucb in the same CASEs to locate and modify the expressions.
🧹 Nitpick comments (3)
supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql (3)
324-326:GRANT ALLon the wrapper is inconsistent withGRANT EXECUTEused above
GRANT ALL ON FUNCTIONis functionally identical toGRANT EXECUTE ON FUNCTIONfor PostgreSQL functions (EXECUTE is the only applicable privilege), but the inconsistency with lines 238–239 reduces clarity.♻️ Proposed fix
-GRANT ALL ON FUNCTION public.get_orgs_v7() TO anon; -GRANT ALL ON FUNCTION public.get_orgs_v7() TO authenticated; -GRANT ALL ON FUNCTION public.get_orgs_v7() TO service_role; +GRANT EXECUTE ON FUNCTION public.get_orgs_v7() TO anon; +GRANT EXECUTE ON FUNCTION public.get_orgs_v7() TO authenticated; +GRANT EXECUTE ON FUNCTION public.get_orgs_v7() TO service_role;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql` around lines 324 - 326, Replace the three inconsistent GRANT statements for the wrapper function with explicit EXECUTE privileges: instead of "GRANT ALL ON FUNCTION public.get_orgs_v7() TO anon/authenticated/service_role", change each to "GRANT EXECUTE ON FUNCTION public.get_orgs_v7() TO <role>" so they match the earlier GRANT EXECUTE usage for get_orgs_v7 and improve clarity.
87-113: System-wide scans inpaying_orgs_orderedandbilling_cyclesCTEs scale with total org count
paying_orgs_orderedusesROW_NUMBER() OVER (ORDER BY o.id ASC)over the entireorgs+stripe_infojoin — window functions prevent predicate pushdown, so PostgreSQL must always read every paying org in the system, regardless of how many orgs the calling user actually belongs to.billing_cyclessimilarly scans all oforgsandstripe_info.At scale, this becomes a full-table read on every dashboard load. Consider:
- Pre-computing and storing
preceding_countas a materialised column onorgs(updated by the stats job) so the per-user query avoids the window function entirely.- Adding a covering index on
stripe_info(customer_id, status, canceled_at, subscription_anchor_end, trial_at)if one does not already exist, to make thepaying_orgs_orderedjoin cheaper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql` around lines 87 - 113, The paying_orgs_ordered CTE (ROW_NUMBER() OVER ... as preceding_count) and the billing_cycles CTE currently scan public.orgs JOIN public.stripe_info for the whole table; replace the window-based full scan by either using a precomputed/materialized preceding_count column on orgs (update via your periodic stats job and reference that column instead of computing ROW_NUMBER in paying_orgs_ordered) or restrict the CTE to only the current user's org set (apply a WHERE org_id IN (...) or join to the caller's membership table before the window) and remove the window function; also add a covering index on public.stripe_info(customer_id, status, canceled_at, subscription_anchor_end, trial_at) if it doesn't exist to speed the join used by paying_orgs_ordered and billing_cycles.
120-134: Each ofhas_2fa_enabledanduser_meets_password_policyis called twice per rowIn
two_fa_access,public.has_2fa_enabled(userid)appears on both line 122 and line 124. Inpassword_policy_access,public.user_meets_password_policy(userid, o.id)appears on both lines 133 and 134. Because these areVOLATILEfunctions, PostgreSQL evaluates each occurrence independently — doubling the call count per org row.Both
should_redact_2faandshould_redact_passwordare simply the logical negations of their corresponding_has_accesscolumns; they don't need to be materialised in the CTE.♻️ Proposed refactor
-- Calculate 2FA access status for user/org combinations + user_2fa_result AS ( + SELECT public.has_2fa_enabled(userid) AS result + ), two_fa_access AS ( SELECT o.id AS org_id, o.enforcing_2fa, CASE WHEN o.enforcing_2fa = false THEN true - ELSE public.has_2fa_enabled(userid) + ELSE u.result END AS "2fa_has_access", - (o.enforcing_2fa = true AND NOT public.has_2fa_enabled(userid)) AS should_redact_2fa + (o.enforcing_2fa = true AND NOT (CASE WHEN o.enforcing_2fa = false THEN true ELSE u.result END)) AS should_redact_2fa FROM public.orgs o JOIN user_orgs uo ON uo.org_id = o.id + CROSS JOIN user_2fa_result u ), -- Calculate password policy access status for user/org combinations password_policy_access AS ( SELECT o.id AS org_id, o.password_policy_config, - public.user_meets_password_policy(userid, o.id) AS password_has_access, - NOT public.user_meets_password_policy(userid, o.id) AS should_redact_password + public.user_meets_password_policy(userid, o.id) AS password_has_access FROM public.orgs o JOIN user_orgs uo ON uo.org_id = o.id )Then replace all
ppa.should_redact_passwordreferences in the mainSELECTwithNOT ppa.password_has_access, andtfa.should_redact_2fawithNOT tfa."2fa_has_access".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql` around lines 120 - 134, The CTEs two_fa_access and password_policy_access call the VOLATILE functions public.has_2fa_enabled(userid) and public.user_meets_password_policy(userid, o.id) twice per row (once for *_has_access and again for should_redact_*); remove the redundant function calls by computing only the *_has_access columns in two_fa_access and password_policy_access (using public.has_2fa_enabled and public.user_meets_password_policy once each) and drop should_redact_2fa / should_redact_password from those CTEs, then update the main SELECT to use NOT tfa."2fa_has_access" and NOT ppa.password_has_access wherever should_redact_2fa / should_redact_password were referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/dashboard/TrialBanner.vue`:
- Around line 137-179: The .eyes-container layout/spacing CSS should be moved to
Tailwind utilities: update the template's div with class="eyes-container" to
also include utility classes like "flex gap-3 items-center justify-center
flex-shrink-0 px-1" (or equivalent spacing utilities you prefer) and remove the
corresponding properties (display, gap, align-items, justify-content,
flex-shrink, padding) from the <style scoped> block, leaving only the transition
rule (and any animation-related styles) in .eyes-container; ensure the reactive
class binding ( :class="{ 'eyes-excited': excited }" ) and the .eyes-container
selector remain in place so the existing transition and excited styling continue
to work.
---
Duplicate comments:
In `@src/components/dashboard/TrialBanner.vue`:
- Around line 160-166: The CTA router-link (ref="ctaRef", currently
class="cta-button cta-sparkle") must include the DaisyUI base button class;
update the router-link's class attribute to add "d-btn" (e.g., class="d-btn
cta-button cta-sparkle") so the interactive element uses the required DaisyUI
button component styling.
- Around line 63-106: The constants MAX_TRAVEL and EXCITE_DISTANCE are using
UPPER_SNAKE_CASE but must follow camelCase; rename them to maxTravel and
exciteDistance and update all usages (change the declaration const MAX_TRAVEL =
4 to const maxTravel = 4 and const EXCITE_DISTANCE = 80 to const exciteDistance
= 80), then update references inside calcOffset (where MAX_TRAVEL is used) and
inside handleMouseMove (where EXCITE_DISTANCE is compared) so the identifiers
match the new camelCase names.
In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql`:
- Line 44: The function declaration for get_orgs_v7(uuid) is incorrectly marked
STABLE while it calls VOLATILE helpers (has_2fa_enabled,
user_meets_password_policy, get_next_cron_time); update the function declaration
keyword from STABLE to VOLATILE so the planner knows the function can return
varying results within a statement and avoid caching/staleness issues.
- Around line 149-166: The CASE branches for paying, can_use_more, and
is_canceled currently return NULL when no stripe_info (si) row exists; update
each ELSE expression in the CASE for fields paying, can_use_more, and
is_canceled to wrap the boolean result with COALESCE(..., false) (e.g.,
COALESCE((si.status = 'succeeded'), false)), so that when the LEFT JOIN yields
NULL you get false; reference the aliases si, tfa, ppa, and ucb in the same
CASEs to locate and modify the expressions.
---
Nitpick comments:
In `@supabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql`:
- Around line 324-326: Replace the three inconsistent GRANT statements for the
wrapper function with explicit EXECUTE privileges: instead of "GRANT ALL ON
FUNCTION public.get_orgs_v7() TO anon/authenticated/service_role", change each
to "GRANT EXECUTE ON FUNCTION public.get_orgs_v7() TO <role>" so they match the
earlier GRANT EXECUTE usage for get_orgs_v7 and improve clarity.
- Around line 87-113: The paying_orgs_ordered CTE (ROW_NUMBER() OVER ... as
preceding_count) and the billing_cycles CTE currently scan public.orgs JOIN
public.stripe_info for the whole table; replace the window-based full scan by
either using a precomputed/materialized preceding_count column on orgs (update
via your periodic stats job and reference that column instead of computing
ROW_NUMBER in paying_orgs_ordered) or restrict the CTE to only the current
user's org set (apply a WHERE org_id IN (...) or join to the caller's membership
table before the window) and remove the window function; also add a covering
index on public.stripe_info(customer_id, status, canceled_at,
subscription_anchor_end, trial_at) if it doesn't exist to speed the join used by
paying_orgs_ordered and billing_cycles.
- Around line 120-134: The CTEs two_fa_access and password_policy_access call
the VOLATILE functions public.has_2fa_enabled(userid) and
public.user_meets_password_policy(userid, o.id) twice per row (once for
*_has_access and again for should_redact_*); remove the redundant function calls
by computing only the *_has_access columns in two_fa_access and
password_policy_access (using public.has_2fa_enabled and
public.user_meets_password_policy once each) and drop should_redact_2fa /
should_redact_password from those CTEs, then update the main SELECT to use NOT
tfa."2fa_has_access" and NOT ppa.password_has_access wherever should_redact_2fa
/ should_redact_password were referenced.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/dashboard/TrialBanner.vuesrc/types/supabase.types.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260224120000_add_created_at_to_get_orgs_v7.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/supabase.types.ts
- supabase/functions/_backend/utils/supabase.types.ts
- Rename MAX_TRAVEL/EXCITE_DISTANCE to camelCase (maxTravel/exciteDistance) - Add DaisyUI d-btn class to CTA button - Change get_orgs_v7(uuid) from STABLE to VOLATILE (calls VOLATILE helpers) - Add COALESCE wrappers to paying/can_use_more/is_canceled for NULL safety
Translate trial-banner-cta and trial-banner-message keys to de, es, fr, hi, id, it, ja, ko, pl, pt-br, ru, tr, vi, zh-cn using DeepL API.
|
2 participants
Summary
TrialBannercomponent with 👀-style eyes that follow the user's cursor in real-timetrial-banner-messageandtrial-banner-ctakeysDetails
TrialBanner Component (
src/components/dashboard/TrialBanner.vue)mousemovelistener withrequestAnimationFramethrottlingtransformstring to avoid CSS specificity issuessparkle-floatanimations.darkprefix stylesaria-hidden="true"on decorative eye elementsOther Changes
dashboard.vue: Mount<TrialBanner />in the dashboard layoutmessages/en.json: Add trial banner i18n stringscomponents.d.ts: Auto-generated component registrationPlans.vue: Conditionally hide Expert-as-a-Service CTA for non-paying orgsVerification
bun lint:fix✅bun typecheck✅Summary by CodeRabbit
New Features
UI Changes