[Grida WEST] UX Improvements #306
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Skipped Deployments
|
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a variety of enhancements and refactorings across the codebase. Key changes include the addition of new React hooks for unsaved changes warnings and CSV export, enriched API responses for invitation-related endpoints, and improved UI components for campaign management such as tabbed settings forms and event analytics with loading/empty states. The platform's public campaign type is extended with new properties, and the sharing logic is centralized to rely on server-generated data. Internationalization support is added to certain pages, and a new permanent redirect is configured. Several database function declarations related to TimescaleDB are removed, and the migration script is updated to conditionally enable the extension. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant useUnsavedChangesWarning
participant Browser
User->>UI_Component: Interact with form (make changes)
UI_Component->>useUnsavedChangesWarning: Register isDirty callback
User->>Browser: Attempt to navigate away or reload
Browser->>useUnsavedChangesWarning: Trigger beforeunload/click event
useUnsavedChangesWarning->>User: Show unsaved changes warning
User-->>Browser: Confirm or cancel navigation
sequenceDiagram
participant Client
participant API (invite/refresh route)
participant renderSharable
participant Database
Client->>API (invite/refresh route): POST request
API (invite/refresh route)->>Database: Fetch invitation, campaign, templates, referrer
API (invite/refresh route)->>renderSharable: Generate sharable content
renderSharable-->>API (invite/refresh route): Sharable { title, text, url }
API (invite/refresh route)-->>Client: Respond with { code, sharable }
sequenceDiagram
participant User
participant ReferrersTable
participant useExportCSV
participant Supabase
User->>ReferrersTable: Click "Export CSV"
ReferrersTable->>useExportCSV: Call exportToCSV()
useExportCSV->>Supabase: Fetch paginated data
Supabase-->>useExportCSV: Return data pages
useExportCSV->>ReferrersTable: Update progress
useExportCSV->>User: Trigger CSV download
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
editor/lib/platform/index.ts (1)
685-717: 🛠️ Refactor suggestionImplementation not aligned with new return-type contract
invite/refreshpromise types were narrowed to a success payload, but the methods still blindly returnres.json(). If the server answers with{ error }the caller will mis-interpret the response and crash on missing fields. Add anif (!res.ok)guard or widen the type to include the error branch.return fetch(url).then(async (res) => { const body = await res.json(); if (!res.ok) { throw new Error(body?.error ?? "invite request failed"); } return body as { data: { code: string; sharable: { title: string; text: string; url: string } }; }; });editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/settings.tsx (1)
124-131:⚠️ Potential issueWrong variable referenced –
updatedis ignored after mutationAfter the
update()call, you correctly destructure the response intoupdated.
However, the success branch checks and returns based on the parameterdata, which is always truthy, instead of the newly-fetchedupdatedresult. This can mask failed updates (e.g. constraint errors that returnnulldata but noerror).- if (data) { - setCampaign(updated as unknown as Platform.WEST.Referral.Campaign); - return true; - } + if (updated) { + setCampaign(updated as Platform.WEST.Referral.Campaign); + return true; + }Without this fix the UI will optimistically claim success while the DB row was not changed.
🧹 Nitpick comments (14)
editor/app/(tenant)/~/[tenant]/api/west/t/_utils/render-sharable.ts (1)
13-15: Consider adding internationalization support for the title.The title is hardcoded in Korean, which contrasts with the internationalization approach implemented in other files like the NotFound component.
For consistency, consider implementing a similar locale-based approach:
+ // Import getLocale or pass locale as a parameter + const dictionary = { + en: { + title: "Get a gift when you test drive Polestar 🎁", + textTemplate: "{referrer_name} invites you to the Polestar test drive event!" + }, + ko: { + title: "Polestar 시승하고 경품 받아가세요 🎁", + textTemplate: "{referrer_name} 님 께서 Polestar 시승 이벤트에 초대합니다!" + } + }; export function renderSharable({ - template, context: { baseUrl, referrer_name, invitation_code, locale = "ko" }, }: { - template: unknown; context: { baseUrl: string | URL; referrer_name: string; invitation_code: string; + locale?: "en" | "ko"; }; }) { + const t = dictionary[locale]; return { - // FIXME: content template - title: "Polestar 시승하고 경품 받아가세요 🎁", - text: `${referrer_name} 님 께서 Polestar 시승 이벤트에 초대합니다!`, + title: t.title, + text: t.textTemplate.replace("{referrer_name}", referrer_name), url: `${baseUrl.toString()}/t/${invitation_code}`, }; }editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/rewards/page.tsx (1)
52-57: Consider using dedicated components for the feature lists.The feature lists are implemented as direct bullet points in the UI. For better maintainability and potential reuse, these could be extracted into dedicated components.
// Example implementation for a FeatureList component function FeatureList({ items }: { items: string[] }) { return ( <ul className="space-y-2 text-sm text-muted-foreground"> {items.map((item, index) => ( <li key={index}>• {item}</li> ))} </ul> ); } // Usage: <FeatureList items={[ "Discount codes and coupons", "Gift cards and vouchers", "Points and credits", "Custom rewards" ]} />Also applies to: 69-74
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/referrers-table.tsx (2)
243-254: Consider adding success feedback for export completion.While the export button shows progress during export, there's no success notification when the export completes successfully.
Consider adding a toast notification for successful export completion, similar to the clipboard copy action on line 68:
const { exportToCSV, isExporting, progress, error } = useExportCSV<ReferrerWithCustomer>({ fetchData: async (page, pageSize) => { // ... existing code }, transformToCSV: (referrer) => [ // ... existing code ], headers: [ // ... existing code ], pageSize: 100, + onSuccess: () => { + toast.success("Export completed successfully"); + } }); // Then modify the useExportCSV hook to support the onSuccess callback
255-255: Enhance error display for better visibility.The error display is minimal with just text color styling.
Consider using a more visible error component for better user experience:
- {error && <div className="text-red-500 text-sm mb-4">{error}</div>} + {error && ( + <div className="bg-red-50 border border-red-200 text-red-700 px-4 py-3 rounded mb-4 flex items-center"> + <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 20 20"> + <path fillRule="evenodd" d="M18 10a8 8 0 11-16 0 8 8 0 0116 0zm-7-4a1 1 0 11-2 0 1 1 0 012 0zM9 9a1 1 0 000 2v3a1 1 0 001 1h1a1 1 0 100-2h-1V9z" clipRule="evenodd" /> + </svg> + <span>{error}</span> + </div> + )}editor/hooks/use-unsaved-changes-warning.ts (1)
31-34: Consider handling route changes more efficientlyWhen preventing navigation, you're currently pushing the current URL back to the router which might create an unnecessary history entry.
if (link && isDirty() && !window.confirm(message)) { e.preventDefault(); - router.push(window.location.href); + // Optional: You could use e.stopPropagation() instead of re-pushing the URL + // to avoid creating an unnecessary history entry }editor/app/(tenant)/~/[tenant]/api/west/t/invite/route.ts (1)
42-49: Avoid unsafe type castingThe current type casting approach using
as unknown as ...is error-prone. Consider using proper type validation or a type guard.- const invitation = _data as unknown as Platform.WEST.Referral.Invitation & { + // Use a type guard function or zod for validation + const invitation = validateInvitationData(_data) as Platform.WEST.Referral.Invitation & { referrer: Omit<Platform.WEST.Referral.ReferrerPublicRead, "type">; campaign: Platform.WEST.Referral.CampaignPublic; templates: { invitation_email_template: unknown; invitation_share_template: unknown; }; };editor/scaffolds/platform/data/use-export-csv.ts (2)
131-132: Improve progress reporting accuracyThe current progress reporting is based on page numbers rather than actual processed items, which could be misleading.
for (let page = 2; page <= totalPages; page++) { const { data } = await config.fetchData(page, pageSize); allData = [...allData, ...data]; - setProgress((page - 1) * pageSize); + setProgress(allData.length); }
145-145: Improve CSV filename with a safer date formatThe current filename uses ISO string which includes characters that might be problematic in some contexts.
- link.setAttribute("download", `export-${new Date().toISOString()}.csv`); + const formattedDate = new Date().toISOString().replace(/:/g, '-').replace(/\..+/, ''); + link.setAttribute("download", `export-${formattedDate}.csv`);editor/app/(site)/sign-in/page.tsx (2)
31-33: Avoid re-awaitingsearchParams
searchParamsis awaited twice. Resolve it once and reuse to avoid redundant work and accidental divergence if the object is ever mutated.-const { next, redirect_uri } = await searchParams; -const q = new URLSearchParams(await searchParams).toString(); +const params = await searchParams; +const { next, redirect_uri } = params; +const q = new URLSearchParams(params).toString();
34-40: Expose Supabase auth errors
client.auth.getUser()can return{ error, data }. Swallowingerrormakes mis-configuration harder to diagnose.const { data: { user }, error: authErr } = await client.auth.getUser(); if (authErr) { console.error("Supabase getUser failed", authErr); } if (user) { return redirect("/"); }editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/overview.tsx (2)
170-195: Avoid repeated heavy computations
generateChartConfig(data)is executed multiple times per render (totals, card colour, chart). Cache the result withuseMemoto save unnecessary array scans.const chartConfig = useMemo(() => generateChartConfig(data.data), [data.data]);
135-144: Handle fetch errors from SWR
If/events/analyzereturns a non-200 status,isLoadingbecomesfalsebutdatais undefined, silently showing the empty state. Provide anonErroror validateres.okbeforeres.json()to surface issues to the user.editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/settings.tsx (2)
71-74: Inconsistent title length constraints (256 vs 40 characters)The Zod schema allows up to 256 characters, while the helper text says “1-40 characters”.
Please align these two sources of truth to avoid user confusion and accidental validation failures downstream.- title: z.string().min(1).max(256, { + // Pick one: 40 or 256 – example below assumes 40 + title: z.string().min(1).max(40, { message: "Campaign title must be between 1 and 40 characters", }),Also applies to: 280-283
590-610: Editing JSON feels broken – textarea never shows invalid draftWhen the user types malformed JSON you ignore the change, so the textarea immediately re-renders with the last valid value, effectively discarding everything the user just typed.
Consider keeping a local
jsonDraftstate that always reflects the textarea contents, and only propagate tofield.onChangeafter a successfulJSON.parse(e.g. on blur or via debounce). Example sketch:+const [jsonDraft, setJsonDraft] = useState( + JSON.stringify(field.value ?? {}, null, 2) +); + <Textarea - className="font-mono h-32 resize-none" - placeholder="{}" - value={JSON.stringify(field.value, null, 2)} - onChange={(e) => { - try { - field.onChange(JSON.parse(e.target.value)); - } catch { - // Allow invalid JSON during editing - } - }} + className="font-mono h-32 resize-none" + placeholder="{}" + value={jsonDraft} + onChange={(e) => setJsonDraft(e.target.value)} + onBlur={() => { + try { + field.onChange(JSON.parse(jsonDraft)); + } catch {/* keep draft, show error via FormMessage if desired */} + }} />This preserves user input and enables proper validation messaging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
editor/app/(dev)/canvas/editor.tsx(2 hunks)editor/app/(dev)/canvas/examples/with-templates/002/page.tsx(2 hunks)editor/app/(site)/sign-in/page.tsx(3 hunks)editor/app/(tenant)/~/[tenant]/(r)/r/[slug]/t/not-found.tsx(1 hunks)editor/app/(tenant)/~/[tenant]/api/west/t/_utils/render-sharable.ts(1 hunks)editor/app/(tenant)/~/[tenant]/api/west/t/invite/route.ts(2 hunks)editor/app/(tenant)/~/[tenant]/api/west/t/refresh/route.ts(2 hunks)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/overview.tsx(5 hunks)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/referrers-table.tsx(3 hunks)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/settings.tsx(5 hunks)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/page.tsx(1 hunks)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/rewards/page.tsx(1 hunks)editor/database-generated.types.ts(1 hunks)editor/hooks/use-unsaved-changes-warning.ts(1 hunks)editor/lib/platform/index.ts(3 hunks)editor/next.config.mjs(1 hunks)editor/scaffolds/platform/data/use-export-csv.ts(1 hunks)editor/theme/templates/west-referral/referrer/page.tsx(2 hunks)package.json(1 hunks)supabase/migrations/20250323084830_grida_west.sql(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
editor/app/(dev)/canvas/editor.tsx (1)
editor/hooks/use-unsaved-changes-warning.ts (1)
useUnsavedChangesWarning(9-44)
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/rewards/page.tsx (4)
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/observability/page.tsx (1)
Page(3-14)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/quests/page.tsx (1)
Page(3-14)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/participants/page.tsx (1)
Page(5-28)editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/settings/page.tsx (1)
Page(3-9)
editor/app/(tenant)/~/[tenant]/api/west/t/refresh/route.ts (2)
editor/lib/platform/index.ts (3)
Invitation(612-621)ReferrerPublicRead(630-645)CampaignPublic(578-592)editor/app/(tenant)/~/[tenant]/api/west/t/_utils/render-sharable.ts (1)
renderSharable(1-18)
editor/app/(tenant)/~/[tenant]/api/west/t/invite/route.ts (2)
editor/lib/platform/index.ts (3)
Invitation(612-621)ReferrerPublicRead(630-645)CampaignPublic(578-592)editor/app/(tenant)/~/[tenant]/api/west/t/_utils/render-sharable.ts (1)
renderSharable(1-18)
editor/app/(site)/sign-in/page.tsx (1)
editor/lib/supabase/server.ts (1)
createClient(41-43)
🔇 Additional comments (18)
supabase/migrations/20250323084830_grida_west.sql (1)
23-23: Improved TimescaleDB extension installationThe change adds conditional installation and explicitly specifies the schema for the TimescaleDB extension. This is a safer approach that prevents errors if the extension already exists and ensures it's installed in the extensions schema rather than the default schema.
editor/database-generated.types.ts (2)
4290-4290: Improved type declaration for the citext functionThe function signature has been updated to more accurately reflect the possible argument types that citext can accept: unknown, string, or boolean. This makes the type definition more flexible and accurate for function callers.
4293-4466:Details
✅ Verification successful
Verify removal of TimescaleDB function types
I notice that TimescaleDB-related function declarations have been removed from the type definitions. This aligns with the migration changes to install the extension in the extensions schema rather than the public schema.
Please verify that these removed function types won't affect any existing code that might be importing or referencing them.
🏁 Script executed:
#!/bin/bash # Search for imports or references to TimescaleDB functions in the codebase echo "Searching for potential references to removed TimescaleDB functions..." grep -r "time_bucket\|hypertable\|compression_policy\|continuous_aggregate\|chunks" --include="*.ts" --include="*.tsx" .Length of output: 273
Let’s re-run the search using ripgrep for more reliable results:
🏁 Script executed:
#!/bin/bash echo "Re-checking for references to TimescaleDB functions with ripgrep..." rg "time_bucket|hypertable|compression_policy|continuous_aggregate|chunks" -t ts -t tsx .Length of output: 264
Let’s try one more time using explicit glob patterns for both
.tsand.tsxfiles:
🏁 Script executed:
#!/bin/bash echo "Re-running search with explicit globs for .ts and .tsx..." rg "time_bucket|hypertable|compression_policy|continuous_aggregate|chunks" --glob '*.ts' --glob '*.tsx' .Length of output: 232
No references to TimescaleDB functions detected – removal is safe
I’ve searched all TypeScript files (
.ts,.tsx) for calls totime_bucket,hypertable,compression_policy,continuous_aggregate, andchunksand found none. Removing these function types will not impact existing code.editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/design/page.tsx (1)
164-165: LGTM: Campaign object updated with new properties.The addition of
www_nameandwww_route_pathproperties to the campaign object aligns with the type extensions in the platform definition. These properties will support URL construction for invitation sharing.editor/app/(dev)/canvas/examples/with-templates/002/page.tsx (2)
3-3: LGTM: Consistent import style.Moving the React import to the top of the file follows good code organization practices.
286-287: LGTM: Consistent campaign object structure.The addition of
www_nameandwww_route_pathproperties with dummy values ensures the example component matches the updated campaign data structure used in production, maintaining consistency across the codebase.editor/next.config.mjs (1)
46-51: LGTM: Appropriate redirect configuration.Adding a permanent redirect from
/loginto/sign-inis a clean approach to standardizing authentication URLs. Thepermanent: trueflag ensures browsers and search engines understand this is a long-term change (301 redirect).editor/app/(dev)/canvas/editor.tsx (2)
3-5: LGTM: Clean import management.Removing unused imports and adding the new hook import improves code cleanliness.
17-17: Improved unsaved changes warning implementation.Replacing the manual event listener with the reusable
useUnsavedChangesWarninghook is a good refactoring that centralizes this functionality. The hook handles both browser unload events and Next.js App Router navigation more robustly.Note that this implementation will always show warnings since the isDirty function simply returns
true. Ensure this is the intended behavior for this editor component.editor/app/(tenant)/~/[tenant]/(r)/r/[slug]/t/not-found.tsx (2)
1-20: LGTM! Good internationalization implementation.The approach of using async/await with getLocale for dynamic language detection and implementing a dictionary for translations is a clean implementation that enhances user experience.
24-25: Well-implemented text localization.The component properly renders localized title and description based on the detected locale.
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/rewards/page.tsx (1)
17-78: Well-structured UI implementation with clear information hierarchy.The card-based layout with distinct sections for settings, types, and features provides a clear organization that aligns well with the design patterns used across other campaign pages.
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/referrers-table.tsx (1)
178-217: LGTM! Well-implemented CSV export functionality.The CSV export implementation with the useExportCSV hook is well-structured with proper pagination, data transformation, progress tracking, and error handling.
editor/hooks/use-unsaved-changes-warning.ts (1)
1-44: Well-implemented unsaved changes warning hook with good UX considerationsThis hook provides a complete solution for warning users about unsaved changes in both browser navigation (beforeunload) and app router navigation scenarios. The implementation is clean with proper event handling and cleanup.
editor/app/(tenant)/~/[tenant]/api/west/t/invite/route.ts (1)
4-5: Good refactoring to centralize sharable content generationMoving the sharable content generation logic to the server side with the
renderSharableutility creates a more maintainable architecture.editor/scaffolds/platform/data/use-export-csv.ts (1)
1-174: Excellent reusable CSV export hook with comprehensive documentationThis hook provides a well-structured solution for client-side CSV export with pagination, progress tracking, and proper error handling. The documentation is thorough with clear examples.
editor/theme/templates/west-referral/referrer/page.tsx (2)
51-51: Good refactoring to use server-provided sharable contentThe simplified approach of directly using the server-provided sharable object improves code maintainability and ensures consistency between server and client.
69-69: Good refactoring to use server-provided sharable contentThe simplified approach of directly using the server-provided sharable object improves code maintainability and ensures consistency between server and client.
| "test": "pnpm -r --filter 'packages/*' run test" | ||
| }, | ||
| "packageManager": "pnpm@9.15.4", | ||
| "packageManager": "pnpm@10.10.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify upgrade to pnpm v10.10.0
Upgrading the packageManager field to pnpm@10.10.0 may introduce breaking changes in lockfile formats and workspace commands. Please ensure:
- CI/CD pipelines and local development environments are updated to pnpm v10.
- The
pnpm-lock.yamlis regenerated viapnpm installand committed. - Any scripts or configuration files referencing pnpm are updated.
🏁 Script executed:
#!/bin/bash
# 1. Search for pnpm version references across the repo
rg -n "pnpm@" --glob "*.*"
# 2. Regenerate lockfile and verify changes
pnpm install
if git diff --quiet -- pnpm-lock.yaml; then
echo "Warning: pnpm-lock.yaml unchanged. Please run 'pnpm install' to regenerate the lockfile."
else
echo "Success: pnpm-lock.yaml has been updated."
fiLength of output: 8306
Verify pnpm v10.10.0 upgrade across all workspaces
- Root
package.jsonhas been updated to"packageManager": "pnpm@10.10.0". - desktop/package.json still declares
"packageManager": "pnpm@9.15.4"– align this to10.10.0. - The
editor/package.json(and any other nested package.json) contains a top-levelpnpm.overridesfield, which only takes effect at the workspace root. Please move those override entries into the rootpackage.json. - Running
pnpm installwith pnpm v10.10.0 leftpnpm-lock.yamlunchanged (no dep changes detected). If any lockfile updates do occur, commit the updatedpnpm-lock.yaml. - Ensure CI/CD pipelines and all local environments are running pnpm v10.10.0.
🤖 Prompt for AI Agents (early access)
In package.json at line 8, the packageManager field is updated to "pnpm@10.10.0". To fix the issues, update all nested package.json files, such as desktop/package.json, to also use "pnpm@10.10.0" for consistency. Move any pnpm.overrides fields from nested package.json files like editor/package.json to the root package.json. Run pnpm install with pnpm v10.10.0 to regenerate the pnpm-lock.yaml if needed and commit any changes. Finally, ensure all CI/CD pipelines and local environments use pnpm v10.10.0.
| export function renderSharable({ | ||
| template, | ||
| context: { baseUrl, referrer_name, invitation_code }, | ||
| }: { | ||
| template: unknown; | ||
| context: { | ||
| baseUrl: string | URL; | ||
| referrer_name: string; | ||
| invitation_code: string; | ||
| }; | ||
| }) { | ||
| return { | ||
| // FIXME: content template | ||
| title: "Polestar 시승하고 경품 받아가세요 🎁", | ||
| text: `${referrer_name} 님 께서 Polestar 시승 이벤트에 초대합니다!`, | ||
| url: `${baseUrl.toString()}/t/${invitation_code}`, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address the FIXME comment and clarify template usage.
The function is well-structured, but there are two issues to address:
- The FIXME comment indicates unfinished work
- The
templateparameter is typed asunknownbut not used in the implementation
Consider either implementing the templating functionality or removing the unused parameter:
export function renderSharable({
- template,
context: { baseUrl, referrer_name, invitation_code },
}: {
- template: unknown;
context: {
baseUrl: string | URL;
referrer_name: string;
invitation_code: string;
};
}) {
return {
- // FIXME: content template
title: "Polestar 시승하고 경품 받아가세요 🎁",
text: `${referrer_name} 님 께서 Polestar 시승 이벤트에 초대합니다!`,
url: `${baseUrl.toString()}/t/${invitation_code}`,
};
}Alternatively, if templating is planned for future implementation, keep the parameter but document its intended use with a more specific TODO comment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function renderSharable({ | |
| template, | |
| context: { baseUrl, referrer_name, invitation_code }, | |
| }: { | |
| template: unknown; | |
| context: { | |
| baseUrl: string | URL; | |
| referrer_name: string; | |
| invitation_code: string; | |
| }; | |
| }) { | |
| return { | |
| // FIXME: content template | |
| title: "Polestar 시승하고 경품 받아가세요 🎁", | |
| text: `${referrer_name} 님 께서 Polestar 시승 이벤트에 초대합니다!`, | |
| url: `${baseUrl.toString()}/t/${invitation_code}`, | |
| }; | |
| } | |
| export function renderSharable({ | |
| context: { baseUrl, referrer_name, invitation_code }, | |
| }: { | |
| context: { | |
| baseUrl: string | URL; | |
| referrer_name: string; | |
| invitation_code: string; | |
| }; | |
| }) { | |
| return { | |
| title: "Polestar 시승하고 경품 받아가세요 🎁", | |
| text: `${referrer_name} 님 께서 Polestar 시승 이벤트에 초대합니다!`, | |
| url: `${baseUrl.toString()}/t/${invitation_code}`, | |
| }; | |
| } |
🤖 Prompt for AI Agents (early access)
In editor/app/(tenant)/~/[tenant]/api/west/t/_utils/render-sharable.ts lines 1 to 18, the FIXME comment indicates unfinished work related to the content template, and the template parameter is typed as unknown but unused. To fix this, either implement the templating logic using the template parameter to generate the title and text dynamically, or if templating is not yet ready, remove the template parameter entirely to avoid confusion. If you plan to implement templating later, keep the parameter but replace the FIXME comment with a clear TODO explaining the intended use of the template parameter.
| const client = useMemo(() => createBrowserWestReferralClient(), []); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider reusing the existing Supabase client.
A new client is created for export operations when one already exists from line 148. Reusing the existing client would avoid redundant instantiation.
export function ReferrersTable() {
const campaign = useCampaign();
const importCustomersDialog = useDialogState("import-customers", {
refreshkey: true,
});
const { tokens } = useReferrers(campaign.id);
- const client = useMemo(() => createBrowserWestReferralClient(), []);
+ // Reuse the existing client from useReferrers
+ const client = useMemo(() => createBrowserWestReferralClient(), []);
const { exportToCSV, isExporting, progress, error } =
useExportCSV<ReferrerWithCustomer>({🏁 Script executed:
#!/bin/bash
# Check if useReferrers function exposes its client for reuse
grep -A 30 "function useReferrers" . | grep -B 10 -A 20 "return"Length of output: 88
🏁 Script executed:
#!/bin/bash
# Locate the definition of useReferrers and inspect its return signature
rg "function useReferrers" -C20 -n .
# Check for any instantiation or exposure of createBrowserWestReferralClient within useReferrers
rg "createBrowserWestReferralClient" -n .Length of output: 7820
Refactor shared Supabase client instantiation
To avoid creating multiple Supabase clients in the same component, lift client creation into a single, reusable instance and consume that in both your hook and the export logic.
Suggested changes:
-
In
editor/lib/supabase/client.ts, export a singleton client instead of a factory:
-export const createBrowserWestReferralClient = () =>
- createBrowserClient(...)
+export const supabaseClient = createBrowserClient(...)
- Update your `useReferrers` hook to use (and return) the shared client:
```ts
import { supabaseClient } from "@/lib/supabase/client";
export function useReferrers(campaignId: string) {
const [participants, setParticipants] = useState<ReferrerWithCustomer[] | null>(null);
- const client = useMemo(() => createBrowserWestReferralClient(), []);
+ const client = useMemo(() => supabaseClient, []);
useEffect(() => {
client
.from("referrer")
… // fetch logic
}, [campaignId]);
return { participants, client };
}
-
In your
ReferrersTablecomponent, consume the hook’s client instead of calling the factory again:export function ReferrersTable() { const campaign = useCampaign(); -
const { tokens } = useReferrers(campaign.id);
- const { participants, client } = useReferrers(campaign.id);
- const client = useMemo(() => createBrowserWestReferralClient(), []);
const { exportToCSV, isExporting, progress, error } =
useExportCSV({ client, data: participants });
These changes ensure there’s only one Supabase client instance per page, reducing redundant instantiation.
<details>
<summary>🤖 Prompt for AI Agents (early access)</summary>
In editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/referrers-table.tsx around lines 176-177, avoid creating a new Supabase client with createBrowserWestReferralClient(). Instead, refactor to use a single shared Supabase client instance exported from editor/lib/supabase/client.ts. Update the useReferrers hook to import and use this shared client and return it, then in ReferrersTable consume the client from the hook rather than creating a new one. This eliminates redundant client instantiation and ensures a single client per page.
</details>
<!-- This is an auto-generated comment by CodeRabbit -->
| baseUrl: new URL( | ||
| www_route_path ?? "", | ||
| IS_HOSTED | ||
| ? `https://${www_name}.grida.site/` | ||
| : `http://${www_name}.localhost:3000/` | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null safety checks for URL construction
The current implementation might fail if www_name is null, and the empty fallback for www_route_path could lead to invalid URLs.
baseUrl: new URL(
- www_route_path ?? "",
+ www_route_path ?? "/",
IS_HOSTED
- ? `https://${www_name}.grida.site/`
- : `http://${www_name}.localhost:3000/`
+ ? `https://${www_name || 'default'}.grida.site/`
+ : `http://${www_name || 'default'}.localhost:3000/`
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| baseUrl: new URL( | |
| www_route_path ?? "", | |
| IS_HOSTED | |
| ? `https://${www_name}.grida.site/` | |
| : `http://${www_name}.localhost:3000/` | |
| ), | |
| baseUrl: new URL( | |
| www_route_path ?? "/", | |
| IS_HOSTED | |
| ? `https://${www_name || 'default'}.grida.site/` | |
| : `http://${www_name || 'default'}.localhost:3000/` | |
| ), |
🤖 Prompt for AI Agents (early access)
In editor/app/(tenant)/~/[tenant]/api/west/t/invite/route.ts around lines 67 to 72, the URL construction lacks null safety checks for www_name and www_route_path, which can cause runtime errors or invalid URLs. Add explicit checks to ensure www_name is not null or undefined before using it in the URL template, and provide a valid default or handle the case when www_route_path is empty or null to avoid constructing invalid URLs.
| const { www_name, www_route_path } = invitation.campaign; | ||
| const { invitation_share_template } = invitation.templates; | ||
|
|
||
| return NextResponse.json({ | ||
| data: next, | ||
| data: { | ||
| code: invitation.code, | ||
| sharable: renderSharable({ | ||
| template: invitation_share_template, | ||
| context: { | ||
| referrer_name: invitation.referrer.referrer_name ?? "", | ||
| invitation_code: invitation.code, | ||
| baseUrl: new URL( | ||
| www_route_path ?? "", | ||
| IS_HOSTED | ||
| ? `https://${www_name}.grida.site/` | ||
| : `http://${www_name}.localhost:3000/` | ||
| ), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against missing www_name
If campaign.www_name is null, the generated URL becomes https://null.grida.site/, yielding broken share links. Assert or default the field before URL construction.
assert(invitation.campaign.www_name, "campaign.www_name is required");To confirm whether campaign.www_name can ever be null or undefined, let’s locate its type definition and usage:
🏁 Script executed:
#!/bin/bash
set -e
# Find the Campaign type definition
rg "interface Campaign" -A5
rg "type Campaign" -A5
# See how www_name is declared and whether it’s optional
rg "www_name" -A3Length of output: 17533
Require non-null www_name before URL construction
The Campaign type defines www_name: string | null (see editor/lib/platform/index.ts), so it can be null. Without a guard, you’ll emit URLs like https://null.grida.site/. Add an explicit check or default before using it:
• File: editor/app/(tenant)/~/[tenant]/api/west/t/refresh/route.ts
• File: editor/app/(tenant)/~/[tenant]/api/west/t/invite/route.ts
Suggested change:
// At top of your handler(s)
const wwwName = invitation.campaign.www_name;
if (!wwwName) {
throw new Error("campaign.www_name is required for share links");
}
const baseUrl = new URL(
www_route_path ?? "",
IS_HOSTED
? `https://${wwwName}.grida.site/`
: `http://${wwwName}.localhost:3000/`
);
// …
return NextResponse.json({
data: {
// …
sharable: renderSharable({
template: invitation_share_template,
context: {
// …
baseUrl,
},
}),
},
});This ensures you never construct a URL with null or undefined.
🤖 Prompt for AI Agents (early access)
In editor/app/(tenant)/~/[tenant]/api/west/t/refresh/route.ts around lines 58 to 74, the code constructs a URL using invitation.campaign.www_name which can be null, causing invalid URLs like https://null.grida.site/. To fix this, add an explicit check before URL construction to ensure www_name is not null or undefined. If it is missing, throw an error indicating "campaign.www_name is required for share links". Then use the validated www_name variable for building the baseUrl to prevent broken share links.
| const { data: _data, error: refresh_err } = await service_role.west_referral | ||
| .rpc("refresh", { | ||
| p_campaign_id: campaign_id, | ||
| p_invitation_id: invitation_id, | ||
| }); | ||
| }) | ||
| .select( | ||
| ` | ||
| *, | ||
| referrer:referrer_public_secure( | ||
| * | ||
| ), | ||
| campaign:campaign_public(*), | ||
| templates:campaign( | ||
| invitation_email_template, | ||
| invitation_share_template | ||
| ) | ||
| ` | ||
| ) | ||
| .single(); | ||
|
|
There was a problem hiding this comment.
Check error & null data before casting
_data is cast to invitation before confirming success, risking runtime crashes if the RPC fails and returns null.
-const { data: _data, error: refresh_err } = await service_role.west_referral
- ... .single();
-
-const invitation = _data as ...;
-
-if (refresh_err) {
+const { data: _data, error: refresh_err } = await service_role.west_referral
+ ... .single();
+
+if (refresh_err || !_data) {
console.error("error while refreshing", refresh_err);
return NextResponse.json({ error: refresh_err }, { status: 400 });
}
+
+const invitation = _data as Platform.WEST.Referral.Invitation & { ... };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data: _data, error: refresh_err } = await service_role.west_referral | |
| .rpc("refresh", { | |
| p_campaign_id: campaign_id, | |
| p_invitation_id: invitation_id, | |
| }); | |
| }) | |
| .select( | |
| ` | |
| *, | |
| referrer:referrer_public_secure( | |
| * | |
| ), | |
| campaign:campaign_public(*), | |
| templates:campaign( | |
| invitation_email_template, | |
| invitation_share_template | |
| ) | |
| ` | |
| ) | |
| .single(); | |
| const { data: _data, error: refresh_err } = await service_role.west_referral | |
| .rpc("refresh", { | |
| p_campaign_id: campaign_id, | |
| p_invitation_id: invitation_id, | |
| }) | |
| .select( | |
| ` | |
| *, | |
| referrer:referrer_public_secure( | |
| * | |
| ), | |
| campaign:campaign_public(*), | |
| templates:campaign( | |
| invitation_email_template, | |
| invitation_share_template | |
| ) | |
| ` | |
| ) | |
| .single(); | |
| if (refresh_err || !_data) { | |
| console.error("error while refreshing", refresh_err); | |
| return NextResponse.json({ error: refresh_err }, { status: 400 }); | |
| } | |
| const invitation = _data as Platform.WEST.Referral.Invitation & { | |
| templates: { | |
| invitation_email_template: string; | |
| invitation_share_template: string; | |
| }; | |
| }; |
🤖 Prompt for AI Agents (early access)
In editor/app/(tenant)/~/[tenant]/api/west/t/refresh/route.ts between lines 24 and 43, the code casts _data to invitation without verifying if the RPC call succeeded or if _data is non-null. To fix this, add a check for refresh_err and ensure _data is not null before casting or using it. Handle the error or null case appropriately to prevent runtime crashes.
| {Object.keys(generateChartConfig(data)).map((name, index) => ( | ||
| <Line | ||
| key={index} | ||
| type="linear" | ||
| dataKey={name} | ||
| stroke={`var(--color-${name})`} | ||
| strokeWidth={2} | ||
| activeDot={{ r: 6, strokeWidth: 2 }} | ||
| dot={false} | ||
| /> | ||
| ))} |
There was a problem hiding this comment.
Line colours will not render as intended
stroke={"var(--color-" + name + ")"} assumes a CSS variable for every event name, which rarely exists. Use the colour computed in generateChartConfig instead.
-<Line ... stroke={`var(--color-${name})`} ... />
+const cfg = chartConfig[name];
+<Line ... stroke={cfg.color} ... />Create const chartConfig = useMemo(() => generateChartConfig(data), [data]); once to reuse throughout the component.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/overview.tsx around lines 289 to 299, the stroke color for each Line component is incorrectly set using CSS variables based on event names, which likely do not exist. To fix this, first create a memoized constant chartConfig using useMemo to call generateChartConfig(data) once and reuse it. Then, update the stroke prop to use the color value from chartConfig for each corresponding name instead of the CSS variable string.
.../(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/_components/settings.tsx
Show resolved
Hide resolved
1 participant
Summary by CodeRabbit
New Features
/loginto/sign-in.Improvements
Bug Fixes
Chores
pnpm@10.10.0.