integrated the support section using plain#313
Open
kshitij2040 wants to merge 2 commits into
Open
Conversation
Contributor
WalkthroughAdds a Plain chat provider (injects styles and external script) and mounts it in LayoutProviders; replaces the sidebar hover support popover with a direct “Chat with Support” action in the user menu; adds an email verification resend flow in the user menu; and applies minor formatting/selector edits to globals.css. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (NavUser)
participant Nav as NavUser Component
participant Firebase as Firebase Auth
participant Backend as Backend API
participant Toast as Toast Notification
User->>Nav: Click "Resend Verification"
Nav->>Firebase: requestIdToken()
Firebase-->>Nav: idToken
Nav->>Backend: POST /api/resend-verification (idToken, email/githubId)
Backend-->>Nav: success / error
Nav->>Toast: show success/error toast
Nav->>Nav: reset isSendingVerification
sequenceDiagram
participant Browser as Browser Client
participant Provider as PlainChatProvider
participant CDN as Plain CDN (chat.cdn-plain.com)
participant Window as window.Plain
Browser->>Provider: Mount (LayoutProviders)
Provider->>Provider: useEffect — inject CSS variables & styles
Provider->>CDN: append script (index.js)
CDN-->>Window: script loaded
Provider->>Window: initialize Plain (appId, hideLauncher, theme=light, links)
Window-->>Browser: chat widget ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
providers/LayoutProviders.tsx (1)
46-46: 🛠️ Refactor suggestion | 🟠 MajorPre-existing:
new QueryClient()is instantiated on every render.This isn't introduced by this PR, but
new QueryClient()inside the render body creates a fresh client each render, defeating React Query's caching. It should be hoisted to module scope or wrapped inuseState/useRef.Suggested fix
+const queryClient = new QueryClient(); + const LayoutProviders = ({ children }: { children: React.ReactNode }) => { return ( <PostHogProvider client={posthog}> - <QueryClientProvider client={new QueryClient()}> + <QueryClientProvider client={queryClient}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@providers/LayoutProviders.tsx` at line 46, The QueryClient is being re-created on every render because new QueryClient() is passed directly into the JSX; move its instantiation out of the render so React Query can preserve cache. Fix by creating a single QueryClient instance (hoist to module scope or initialize once with useRef/useState) and pass that stable instance into QueryClientProvider (referencing QueryClientProvider and QueryClient in LayoutProviders.tsx); ensure the provider uses that persistent variable instead of calling new QueryClient() inside the component body.
🧹 Nitpick comments (5)
app/plain-chat.tsx (3)
64-64: HardcodedappId— move to an environment variable.Embedding the Plain app ID directly in source makes it harder to manage across environments (staging vs. production) and exposes it in the client bundle regardless. Use
process.env.NEXT_PUBLIC_PLAIN_APP_IDinstead.- appId: "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5", + appId: process.env.NEXT_PUBLIC_PLAIN_APP_ID!,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` at line 64, Replace the hardcoded Plain appId with a public environment variable: use process.env.NEXT_PUBLIC_PLAIN_APP_ID wherever the appId property is set in plain-chat.tsx (the object with the appId key) and remove the literal string; also add a clear fallback or runtime check that throws/logs a helpful error if NEXT_PUBLIC_PLAIN_APP_ID is undefined so deployments fail fast in missing-config scenarios.
55-89:script.async = falseblocks HTML parsing — preferasync = truefor a chat widget.Setting
async = falsemakes the browser download and execute the script synchronously relative to other scripts, which can delay page interactivity. A chat widget is non-critical and should load asynchronously.Also, inconsistent DOM API: line 53 uses
document.head.appendChild(style)but line 88 usesd.getElementsByTagName("head")[0].appendChild(script). Simplify for consistency.Proposed fix
const script = d.createElement("script"); script.id = "plain-chat-script"; - script.async = false; + script.async = true; script.onload = function () { ... }; script.src = "https://chat.cdn-plain.com/index.js"; - d.getElementsByTagName("head")[0].appendChild(script); + d.head.appendChild(script);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` around lines 55 - 89, Change the chat script to load asynchronously by setting script.async = true instead of false, and append it using the same DOM API used elsewhere for consistency (use document.head.appendChild(script) rather than d.getElementsByTagName("head")[0].appendChild(script)). Keep the existing onload handler that checks window.Plain and calls window.Plain.init with the current options; only modify the async flag and the append call to match document.head usage.
60-63: Replace@ts-ignorewith a proper type declaration.Multiple
@ts-ignorecomments suppress type checking entirely. A minimal ambient declaration is cleaner and catches actual type errors:// In a global.d.ts or at the top of this file: declare global { interface Window { Plain?: { init: (config: Record<string, unknown>) => void; open?: () => void; }; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` around lines 60 - 63, Remove the two // `@ts-ignore` lines and add an ambient type declaration for window.Plain (so TypeScript knows about Plain.init and Plain.open) — e.g., add a declare global { interface Window { Plain?: { init: (config: Record<string, unknown>) => void; open?: () => void; }; } } either in a global.d.ts or at the top of this file, then call window.Plain.init(...) and window.Plain.open() without using // `@ts-ignore`.components/Layouts/minors/nav-user.tsx (2)
76-85: API call lacks a timeout — could hang indefinitely on a slow/unreachable backend.The
fetchcall to/api/v1/account/resend-verificationhas noAbortController/ timeout. If the backend is slow or unreachable, the user sees an indefinite loading state (isSendingVerificationstaystrue).Suggested fix
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + const response = await fetch( `${process.env.NEXT_PUBLIC_BASE_URL || 'http://localhost:8000'}/api/v1/account/resend-verification`, { method: 'POST', headers: { 'Authorization': `Bearer ${token}`, 'Content-Type': 'application/json', }, + signal: controller.signal, } ); + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/minors/nav-user.tsx` around lines 76 - 85, The fetch call that assigns to response when posting to `/api/v1/account/resend-verification` can hang; wrap this request in an AbortController with a timeout (e.g., 5s) so the request is aborted if it exceeds the limit, pass controller.signal into fetch, and clear the timeout on success; update the error handling around the response/token logic (and ensure isSendingVerification is set to false in both the catch and abort branches) so the UI doesn't remain loading after an aborted or timed-out request.
174-180: Chat with Support integration looks good.The
handleOpenPlainChatcorrectly checks forwindow.Plainbefore callingopen?.()with optional chaining. This gracefully handles the case where the Plain script hasn't loaded yet, though the user gets no feedback in that scenario.Consider adding a fallback toast when
window.Plainis unavailable:Suggested improvement
const handleOpenPlainChat = () => { // `@ts-ignore` if (window.Plain) { // `@ts-ignore` window.Plain.open?.(); + } else { + toast.error("Chat support is loading. Please try again in a moment."); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/minors/nav-user.tsx` around lines 174 - 180, The dropdown item calls handleOpenPlainChat which currently checks window.Plain and optionally invokes open?.() but provides no user feedback if Plain isn't loaded; update handleOpenPlainChat to show a fallback toast/notification when window.Plain is undefined (e.g., use your existing toast/snackbar utility) so users know support chat isn't available, and ensure the DropdownMenuItem still calls handleOpenPlainChat; reference the handleOpenPlainChat function and window.Plain check and add a brief user-facing message via the toast path when Plain is missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/plain-chat.tsx`:
- Around line 28-32: Remove the overly broad attribute selector
`[class*="plain"], [class*="plain"] * { ... }` from the CSS in plain-chat.tsx;
instead target the chat UI with the existing specific selectors such as
`[data-plain-chat]` and `.plain-chat-container` (or add a `.plain-chat-*` class
prefix) and keep the background-color/color rules scoped there without
`!important`, ensuring you only affect elements intended to be plain chat.
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 8-11: The component imports CheckCircle2, AlertCircle, and Mail
and defines handleResendVerification and an emailVerified prop that are unused
because Sidebar.tsx doesn't pass emailVerified; remove dead code or wire it up:
either delete the unused imports (CheckCircle2, AlertCircle, Mail), remove the
entire handleResendVerification function and the emailVerified prop usage in
this component, or alternatively add a menu item that calls
handleResendVerification and update the parent (Sidebar.tsx) to pass a real
emailVerified prop; ensure you update any prop types or interfaces referencing
emailVerified and remove any unreachable logic that checks emailVerified if you
choose deletion.
---
Outside diff comments:
In `@providers/LayoutProviders.tsx`:
- Line 46: The QueryClient is being re-created on every render because new
QueryClient() is passed directly into the JSX; move its instantiation out of the
render so React Query can preserve cache. Fix by creating a single QueryClient
instance (hoist to module scope or initialize once with useRef/useState) and
pass that stable instance into QueryClientProvider (referencing
QueryClientProvider and QueryClient in LayoutProviders.tsx); ensure the provider
uses that persistent variable instead of calling new QueryClient() inside the
component body.
---
Nitpick comments:
In `@app/plain-chat.tsx`:
- Line 64: Replace the hardcoded Plain appId with a public environment variable:
use process.env.NEXT_PUBLIC_PLAIN_APP_ID wherever the appId property is set in
plain-chat.tsx (the object with the appId key) and remove the literal string;
also add a clear fallback or runtime check that throws/logs a helpful error if
NEXT_PUBLIC_PLAIN_APP_ID is undefined so deployments fail fast in missing-config
scenarios.
- Around line 55-89: Change the chat script to load asynchronously by setting
script.async = true instead of false, and append it using the same DOM API used
elsewhere for consistency (use document.head.appendChild(script) rather than
d.getElementsByTagName("head")[0].appendChild(script)). Keep the existing onload
handler that checks window.Plain and calls window.Plain.init with the current
options; only modify the async flag and the append call to match document.head
usage.
- Around line 60-63: Remove the two // `@ts-ignore` lines and add an ambient type
declaration for window.Plain (so TypeScript knows about Plain.init and
Plain.open) — e.g., add a declare global { interface Window { Plain?: { init:
(config: Record<string, unknown>) => void; open?: () => void; }; } } either in a
global.d.ts or at the top of this file, then call window.Plain.init(...) and
window.Plain.open() without using // `@ts-ignore`.
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 76-85: The fetch call that assigns to response when posting to
`/api/v1/account/resend-verification` can hang; wrap this request in an
AbortController with a timeout (e.g., 5s) so the request is aborted if it
exceeds the limit, pass controller.signal into fetch, and clear the timeout on
success; update the error handling around the response/token logic (and ensure
isSendingVerification is set to false in both the catch and abort branches) so
the UI doesn't remain loading after an aborted or timed-out request.
- Around line 174-180: The dropdown item calls handleOpenPlainChat which
currently checks window.Plain and optionally invokes open?.() but provides no
user feedback if Plain isn't loaded; update handleOpenPlainChat to show a
fallback toast/notification when window.Plain is undefined (e.g., use your
existing toast/snackbar utility) so users know support chat isn't available, and
ensure the DropdownMenuItem still calls handleOpenPlainChat; reference the
handleOpenPlainChat function and window.Plain check and add a brief user-facing
message via the toast path when Plain is missing.
|
Cloud Run service deployed: https://pr-update-plain-support-cj6r7x3fpa-uc.a.run.app |
kshitij2040
force-pushed
the
update/plain_support
branch
from
03bb923 to
5b45bdc
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/Layouts/minors/nav-user.tsx (1)
151-151:⚠️ Potential issue | 🟡 MinorHardcoded fallback "CN" — should use user initials like the trigger avatar does.
Line 126 uses
user.name?.charAt(0) || "U"for the fallback, but the dropdown label avatar on line 151 is hardcoded to"CN". This looks like a copy-paste oversight.Suggested fix
- <AvatarFallback className="rounded-lg">CN</AvatarFallback> + <AvatarFallback className="rounded-lg"> + {user.name?.charAt(0) || "U"} + </AvatarFallback>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/minors/nav-user.tsx` at line 151, Replace the hardcoded AvatarFallback text "CN" with the same user-initials logic used by the trigger avatar (e.g. use user.name?.charAt(0) || "U" or a shared getInitials(user.name) helper); update the AvatarFallback element (symbol: AvatarFallback) inside the dropdown so it derives initials from user.name rather than the literal "CN" to keep both avatars consistent.
🧹 Nitpick comments (1)
components/Layouts/minors/nav-user.tsx (1)
34-35: Import order: React imports should come first.Per coding guidelines, React/Next imports should be ordered before everything else.
useStatefrom"react"(line 35) anduseRouterfrom"next/navigation"(line 31) should be grouped at the top, beforelucide-reactand component imports.Suggested ordering
+"use client"; + +import { useState } from "react"; +import { useRouter } from "next/navigation"; +import Link from "next/link"; + import { ChevronRight, CreditCard, LogOut, Receipt, MessageCircle, } from "lucide-react"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; ... import { signOut } from "firebase/auth"; import { auth } from "@/configs/Firebase-config"; -import { useRouter } from "next/navigation"; -import Link from "next/link"; import posthog from "posthog-js"; import { toast } from "sonner"; -import { useState } from "react";As per coding guidelines: "Order imports: React/Next imports first, then components, then utilities."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/minors/nav-user.tsx` around lines 34 - 35, Reorder imports in nav-user.tsx so React/Next imports appear first: move useState (from "react") and useRouter (from "next/navigation") to the top of the import list, followed by component imports (e.g., Avatar, AvatarImage, AvatarFallback, DropdownMenu, DropdownMenuTrigger, DropdownMenuContent, DropdownMenuItem) and then utilities like toast (from "sonner") and icons from "lucide-react"; ensure the import grouping keeps the existing named symbols unchanged (useState, useRouter, toast, Avatar, DropdownMenu, etc.) and preserves existing functionality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 50-56: The handler handleOpenPlainChat currently silently no-ops
when window.Plain is undefined; update it to detect absence of the Plain SDK and
provide a fallback (e.g., show a toast/notification or open a support URL).
Specifically, in handleOpenPlainChat check if window.Plain exists and call
window.Plain.open(), otherwise call your app's toast/notification utility (or
window.open to a support URL) and log/report the missing SDK so users receive
immediate feedback. Ensure TypeScript ignores remain limited to the SDK access
and keep the original successful path unchanged.
---
Outside diff comments:
In `@components/Layouts/minors/nav-user.tsx`:
- Line 151: Replace the hardcoded AvatarFallback text "CN" with the same
user-initials logic used by the trigger avatar (e.g. use user.name?.charAt(0) ||
"U" or a shared getInitials(user.name) helper); update the AvatarFallback
element (symbol: AvatarFallback) inside the dropdown so it derives initials from
user.name rather than the literal "CN" to keep both avatars consistent.
---
Duplicate comments:
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 44-111: The block defining the emailVerified prop,
isSendingVerification state and handleResendVerification is dead because
handleResendVerification is never used; either wire it into the UI (e.g., call
handleResendVerification from the appropriate menu item or button in
nav-user.tsx and show a sending state using isSendingVerification, and use the
emailVerified prop to conditionally render the verification menu entry), or
remove the unused items (remove emailVerified from the component props, delete
the isSendingVerification useState and the handleResendVerification function)
and also remove the now-unused imports CheckCircle2, AlertCircle, Mail, toast
and useState if they become unused elsewhere. Ensure references to
auth.currentUser/getIdToken and NEXT_PUBLIC_BASE_URL are removed if you delete
the function to avoid leftover unused variables.
- Around line 8-10: The imports CheckCircle2, AlertCircle, and Mail in the
NavUser component (nav-user.tsx) are unused; remove these three symbols from the
import list so they are no longer imported into the NavUser module to prevent
dead code and reduce bundle size. Locate the import statement that includes
CheckCircle2, AlertCircle, and Mail and delete those identifiers (leave any
other icons in that same import intact), then run the build/linters to confirm
no remaining references.
---
Nitpick comments:
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 34-35: Reorder imports in nav-user.tsx so React/Next imports
appear first: move useState (from "react") and useRouter (from
"next/navigation") to the top of the import list, followed by component imports
(e.g., Avatar, AvatarImage, AvatarFallback, DropdownMenu, DropdownMenuTrigger,
DropdownMenuContent, DropdownMenuItem) and then utilities like toast (from
"sonner") and icons from "lucide-react"; ensure the import grouping keeps the
existing named symbols unchanged (useState, useRouter, toast, Avatar,
DropdownMenu, etc.) and preserves existing functionality.
Comment on lines
+50
to
+56
| const handleOpenPlainChat = () => { | ||
| // @ts-ignore | ||
| if (window.Plain) { | ||
| // @ts-ignore | ||
| window.Plain.open?.(); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Silent no-op when Plain SDK isn't loaded — user gets no feedback.
If window.Plain is undefined (SDK not yet loaded or failed to load), clicking "Chat with Support" does nothing with no indication to the user. Consider showing a fallback toast or opening a support URL.
Suggested improvement
const handleOpenPlainChat = () => {
// `@ts-ignore`
if (window.Plain) {
// `@ts-ignore`
window.Plain.open?.();
+ } else {
+ toast.error("Support chat is temporarily unavailable. Please try again shortly.");
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/Layouts/minors/nav-user.tsx` around lines 50 - 56, The handler
handleOpenPlainChat currently silently no-ops when window.Plain is undefined;
update it to detect absence of the Plain SDK and provide a fallback (e.g., show
a toast/notification or open a support URL). Specifically, in
handleOpenPlainChat check if window.Plain exists and call window.Plain.open(),
otherwise call your app's toast/notification utility (or window.open to a
support URL) and log/report the missing SDK so users receive immediate feedback.
Ensure TypeScript ignores remain limited to the SDK access and keep the original
successful path unchanged.
|
Cloud Run service deployed: https://pr-update-plain-support-cj6r7x3fpa-uc.a.run.app |
kshitij2040
force-pushed
the
update/plain_support
branch
from
5b45bdc to
9bd6954
Compare
|
Cloud Run service deployed: https://pr-update-plain-support-cj6r7x3fpa-uc.a.run.app |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/Layouts/Sidebar.tsx (1)
231-249:⚠️ Potential issue | 🟠 MajorNested
<button>inside<button>— invalid HTML andhandleClickfires twice.When
link.showProModal || isWorkflowsLinkistrue:
- Line 232:
asChild={false}→SidebarMenuButtonrenders as a<button>withonClick={handleClick}(line 235).- Lines 238–241: a second
<button onClick={handleClick}>is rendered inside it.This is invalid HTML (browsers promote or suppress nested interactive elements), and every click dispatches
handleClicktwice — once from the inner button and once as the event bubbles to the outer button. For the workflows path theisCheckingBackendguard does not protect against the double-call because both handlers fire synchronously in the same tick, before any React state update.🐛 Proposed fix — use `asChild` consistently and let each child own its handler
<SidebarMenuButton - asChild={!link.showProModal && !isWorkflowsLink} + asChild isActive={isActive} disabled={link.disabled || (isWorkflowsLink && isCheckingBackend)} - onClick={link.showProModal || isWorkflowsLink ? handleClick : link.handleTrack ? handleTrack : undefined} > {link.showProModal || isWorkflowsLink ? ( <button className="flex gap-2 items-center w-full" onClick={handleClick} > ... </button> ) : ( <Link href={link.href} className="flex gap-2 items-center w-full" + onClick={link.handleTrack ? handleTrack : undefined} > ... </Link> )} </SidebarMenuButton>With
asChildalways enabled,SidebarMenuButtonuses RadixSlotto forward its styling/active state to the single child element; each branch owns its own click handler and there is no nested interactive element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Layouts/Sidebar.tsx` around lines 231 - 249, The current JSX nests a button inside SidebarMenuButton when link.showProModal || isWorkflowsLink is true, causing invalid HTML and double-firing handleClick; fix by always using asChild={true} on <SidebarMenuButton> so it renders a Slot and let the single child element own its onClick handler (remove the outer onClick on SidebarMenuButton), and ensure branches for link.showProModal or isWorkflowsLink attach handleClick only on the child button; keep existing disabled logic (link.disabled || (isWorkflowsLink && isCheckingBackend)) on the child so the guard still applies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 103-104: Remove the PII-leaking console.log that prints the user's
email in the success flow: locate the toast.success call in the handler (the
lines that use toast.success(`Verification email sent to ${data.email ||
user.email}...`)) and delete the console.log("Verification email sent
successfully to work email:", data.email || user.email); statement, or replace
it with a non-PII debug message (e.g., a generic "verification email sent"
message) if an internal log is needed.
- Around line 76-85: Remove the hardcoded 'http://localhost:8000' fallback in
the fetch call so a missing NEXT_PUBLIC_BASE_URL fails fast: update the fetch
invocation in components/Layouts/minors/nav-user.tsx to require
process.env.NEXT_PUBLIC_BASE_URL (no default), and add a quick runtime check
that throws or logs a clear error if NEXT_PUBLIC_BASE_URL is undefined before
calling fetch (reference the fetch call that posts to
/api/v1/account/resend-verification and the NEXT_PUBLIC_BASE_URL env var).
---
Outside diff comments:
In `@components/Layouts/Sidebar.tsx`:
- Around line 231-249: The current JSX nests a button inside SidebarMenuButton
when link.showProModal || isWorkflowsLink is true, causing invalid HTML and
double-firing handleClick; fix by always using asChild={true} on
<SidebarMenuButton> so it renders a Slot and let the single child element own
its onClick handler (remove the outer onClick on SidebarMenuButton), and ensure
branches for link.showProModal or isWorkflowsLink attach handleClick only on the
child button; keep existing disabled logic (link.disabled || (isWorkflowsLink &&
isCheckingBackend)) on the child so the guard still applies.
---
Duplicate comments:
In `@components/Layouts/minors/nav-user.tsx`:
- Around line 50-56: handleOpenPlainChat currently silently no-ops if
window.Plain is missing; update it to provide a user-facing fallback by
detecting absence of window.Plain (in handleOpenPlainChat) and then either
showing a toast/error notification or redirecting to a fallback support
URL/modal so the user gets feedback; keep the existing path that calls
window.Plain.open() when available, but add an explicit branch that triggers a
UI fallback (e.g., call showToast('Support is unavailable, please try again
later') or openSupportPage()) so clicks never do nothing.
- Around line 8-11: The file has unused icon imports (CheckCircle2, AlertCircle,
Mail) and dead/unwired logic (handleResendVerification function and
emailVerified prop). Fix by either: A) remove the unused imports from
nav-user.tsx and delete the handleResendVerification function and the
emailVerified prop from the component signature, or B) wire them properly by
passing emailVerified from Sidebar.tsx into the nav-user component and attach
handleResendVerification to the appropriate menu item (e.g., the verification
menu action) so the guard that checks emailVerified is reachable; update any JSX
to reference the used icons only (MessageCircle) and ensure no unused symbols
remain.
Comment on lines
+76
to
+85
| const response = await fetch( | ||
| `${process.env.NEXT_PUBLIC_BASE_URL || 'http://localhost:8000'}/api/v1/account/resend-verification`, | ||
| { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Authorization': `Bearer ${token}`, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); |
Contributor
There was a problem hiding this comment.
Hardcoded localhost:8000 fallback will silently fail in production.
If NEXT_PUBLIC_BASE_URL is unset in a deployed environment, the fetch targets http://localhost:8000, which causes a network error rather than surfacing the missing env-var as a configuration mistake. Remove the fallback so a missing env var fails fast and loudly (ideally validated at build time).
🐛 Proposed fix
- const response = await fetch(
- `${process.env.NEXT_PUBLIC_BASE_URL || 'http://localhost:8000'}/api/v1/account/resend-verification`,
+ if (!process.env.NEXT_PUBLIC_BASE_URL) {
+ throw new Error("NEXT_PUBLIC_BASE_URL is not configured");
+ }
+ const response = await fetch(
+ `${process.env.NEXT_PUBLIC_BASE_URL}/api/v1/account/resend-verification`,📝 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.
Suggested change
| const response = await fetch( | |
| `${process.env.NEXT_PUBLIC_BASE_URL || 'http://localhost:8000'}/api/v1/account/resend-verification`, | |
| { | |
| method: 'POST', | |
| headers: { | |
| 'Authorization': `Bearer ${token}`, | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| if (!process.env.NEXT_PUBLIC_BASE_URL) { | |
| throw new Error("NEXT_PUBLIC_BASE_URL is not configured"); | |
| } | |
| const response = await fetch( | |
| `${process.env.NEXT_PUBLIC_BASE_URL}/api/v1/account/resend-verification`, | |
| { | |
| method: 'POST', | |
| headers: { | |
| 'Authorization': `Bearer ${token}`, | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/Layouts/minors/nav-user.tsx` around lines 76 - 85, Remove the
hardcoded 'http://localhost:8000' fallback in the fetch call so a missing
NEXT_PUBLIC_BASE_URL fails fast: update the fetch invocation in
components/Layouts/minors/nav-user.tsx to require
process.env.NEXT_PUBLIC_BASE_URL (no default), and add a quick runtime check
that throws or logs a clear error if NEXT_PUBLIC_BASE_URL is undefined before
calling fetch (reference the fetch call that posts to
/api/v1/account/resend-verification and the NEXT_PUBLIC_BASE_URL env var).
Comment on lines
+103
to
+104
| toast.success(`Verification email sent to ${data.email || user.email}! Please check your inbox (and spam folder).`); | ||
| console.log("Verification email sent successfully to work email:", data.email || user.email); |
Contributor
There was a problem hiding this comment.
console.log leaks PII (email address) to the browser console.
The toast.success call on line 103 already conveys success to the user. The console.log on line 104 is redundant and exposes the user's email address in plaintext in any browser devtools session, including those of browser extensions.
🐛 Proposed fix
toast.success(`Verification email sent to ${data.email || user.email}! Please check your inbox (and spam folder).`);
- console.log("Verification email sent successfully to work email:", data.email || user.email);📝 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.
Suggested change
| toast.success(`Verification email sent to ${data.email || user.email}! Please check your inbox (and spam folder).`); | |
| console.log("Verification email sent successfully to work email:", data.email || user.email); | |
| toast.success(`Verification email sent to ${data.email || user.email}! Please check your inbox (and spam folder).`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/Layouts/minors/nav-user.tsx` around lines 103 - 104, Remove the
PII-leaking console.log that prints the user's email in the success flow: locate
the toast.success call in the handler (the lines that use
toast.success(`Verification email sent to ${data.email || user.email}...`)) and
delete the console.log("Verification email sent successfully to work email:",
data.email || user.email); statement, or replace it with a non-PII debug message
(e.g., a generic "verification email sent" message) if an internal log is
needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/plain-chat.tsx (4)
49-83: IIFE wrapper is unnecessary;documentis already in scope.The
(function(d) { ... })(document)pattern is a copy-paste artifact from Plain's standalone embed snippet. Inside a React module,documentis already directly accessible.♻️ Proposed simplification
- (function (d) { - const script = d.createElement("script"); + const script = document.createElement("script"); script.id = "plain-chat-script"; script.async = false; script.onload = function () { ... }; script.src = "https://chat.cdn-plain.com/index.js"; - d.getElementsByTagName("head")[0].appendChild(script); - })(document); + document.head.appendChild(script);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` around lines 49 - 83, The IIFE wrapper around the embed is unnecessary in this React module; remove the self-invoking function (the (function(d){ ... })(document) wrapper) and its parameter 'd', and use document directly when creating and appending the script element in app/plain-chat.tsx; keep the existing script id "plain-chat-script", script.async, script.onload handler (including the window.Plain.init call and its config), script.src, and the appendChild to document.getElementsByTagName("head")[0] unchanged.
54-57: Replace@ts-ignorewith a properWindowinterface augmentation.Using
@ts-ignoresilently bypasses type checking on thewindow.Plain.init(...)call. Any typo inappId,hideLauncher, or thelinksshape will go undetected. Declare the shape in a*.d.tsfile or inline:♻️ Proposed type augmentation
+"use client"; + +interface PlainLinkItem { + icon: string; + text: string; + url: string; +} + +interface PlainInitOptions { + appId: string; + hideLauncher?: boolean; + theme?: "light" | "dark"; + links?: PlainLinkItem[]; +} + +declare global { + interface Window { + Plain?: { + init: (options: PlainInitOptions) => void; + }; + } +}Then in the
onloadhandler:- // `@ts-ignore` if (window.Plain) { - // `@ts-ignore` window.Plain.init({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` around lines 54 - 57, Replace the two uses of "@ts-ignore" around window.Plain.init by adding a proper Window augmentation that declares the Plain object and its init(options) signature: create a .d.ts (or inline declaration) that defines interface PlainInitOptions { appId: string; hideLauncher?: boolean; links?: Array<{ label: string; url: string }>; /* add other known fields */ } and interface PlainWindow { Plain?: { init: (opts: PlainInitOptions) => void } } and merge it into global Window; then remove the `@ts-ignore` lines in app/plain-chat.tsx so TypeScript validates the call to window.Plain.init and the shapes of appId, hideLauncher, and links.
5-5: AddFCtype annotation to the component.The component is currently untyped. As per coding guidelines, functional components should use TypeScript
FCtypes.-export default function PlainChatProvider() { +import type { FC } from "react"; + +const PlainChatProvider: FC = () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` at line 5, The PlainChatProvider component is untyped; import React and the FC type from 'react' and change the declaration from a plain function to a typed functional component (e.g., declare PlainChatProvider as a const with the FC type: PlainChatProvider: FC = () => { ... }) and then export it as the default export (export default PlainChatProvider) so the component uses the TypeScript FC annotation; reference the PlainChatProvider symbol when making these changes.
58-58: Consider externalizingappIdto an environment variable.The app ID is currently hardcoded. While it's not a secret, using
process.env.NEXT_PUBLIC_PLAIN_APP_IDmakes it easy to use a different widget instance across environments (staging, production).♻️ Proposed change
- appId: "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5", + appId: process.env.NEXT_PUBLIC_PLAIN_APP_ID ?? "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/plain-chat.tsx` at line 58, Replace the hardcoded appId with an environment-backed value: read process.env.NEXT_PUBLIC_PLAIN_APP_ID where the appId property is set (the appId symbol in the Plain chat widget config) and use a sensible fallback or runtime warning if the env var is missing; also ensure NEXT_PUBLIC_PLAIN_APP_ID is defined in your Next/hosting env so the widget can target different instances per environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/plain-chat.tsx`:
- Around line 53-80: The script insertion currently only sets script.onload and
has no error handling, so if loading "https://chat.cdn-plain.com/index.js" fails
the chat silently disappears; add a script.onerror handler alongside
script.onload that logs the failure (include the URL and the error/event) using
the app logger or console.error and optionally sets a fallback state/flag so the
UI can show a degraded message; locate the script creation block that assigns
script.onload and the window.Plain.init call and add the onerror handler there
to surface network/CSP/load errors and avoid silent failures.
- Around line 50-82: The injected Plain chat script (created as script with id
"plain-chat-script" and referenced via window.Plain.init) lacks crossOrigin and
no Content Security Policy is declared; add crossOrigin="anonymous" when
creating the script element to enable CORS-safe error reporting, and ensure your
app serves a strict CSP that allowlists the Plain CDN (e.g., include
https://chat.cdn-plain.com in script-src) so third‑party scripts are
restricted—update the code that creates the script element and your server/meta
headers to apply these changes.
---
Nitpick comments:
In `@app/plain-chat.tsx`:
- Around line 49-83: The IIFE wrapper around the embed is unnecessary in this
React module; remove the self-invoking function (the (function(d){ ...
})(document) wrapper) and its parameter 'd', and use document directly when
creating and appending the script element in app/plain-chat.tsx; keep the
existing script id "plain-chat-script", script.async, script.onload handler
(including the window.Plain.init call and its config), script.src, and the
appendChild to document.getElementsByTagName("head")[0] unchanged.
- Around line 54-57: Replace the two uses of "@ts-ignore" around
window.Plain.init by adding a proper Window augmentation that declares the Plain
object and its init(options) signature: create a .d.ts (or inline declaration)
that defines interface PlainInitOptions { appId: string; hideLauncher?: boolean;
links?: Array<{ label: string; url: string }>; /* add other known fields */ }
and interface PlainWindow { Plain?: { init: (opts: PlainInitOptions) => void } }
and merge it into global Window; then remove the `@ts-ignore` lines in
app/plain-chat.tsx so TypeScript validates the call to window.Plain.init and the
shapes of appId, hideLauncher, and links.
- Line 5: The PlainChatProvider component is untyped; import React and the FC
type from 'react' and change the declaration from a plain function to a typed
functional component (e.g., declare PlainChatProvider as a const with the FC
type: PlainChatProvider: FC = () => { ... }) and then export it as the default
export (export default PlainChatProvider) so the component uses the TypeScript
FC annotation; reference the PlainChatProvider symbol when making these changes.
- Line 58: Replace the hardcoded appId with an environment-backed value: read
process.env.NEXT_PUBLIC_PLAIN_APP_ID where the appId property is set (the appId
symbol in the Plain chat widget config) and use a sensible fallback or runtime
warning if the env var is missing; also ensure NEXT_PUBLIC_PLAIN_APP_ID is
defined in your Next/hosting env so the widget can target different instances
per environment.
Comment on lines
+50
to
+82
| const script = d.createElement("script"); | ||
| script.id = "plain-chat-script"; | ||
| script.async = false; | ||
| script.onload = function () { | ||
| // @ts-ignore | ||
| if (window.Plain) { | ||
| // @ts-ignore | ||
| window.Plain.init({ | ||
| appId: "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5", | ||
| hideLauncher: true, | ||
| theme: 'light', | ||
| links: [ | ||
| { | ||
| icon: 'book', | ||
| text: 'Documentation', | ||
| url: 'https://docs.potpie.ai/introduction', | ||
| }, | ||
| { | ||
| icon: 'link', | ||
| text: 'Open Source', | ||
| url: 'https://github.com/potpie-ai/potpie', | ||
| }, | ||
| { | ||
| icon: 'discord', | ||
| text: 'Join our Discord', | ||
| url: 'https://discord.gg/ryk5CMD5v6', | ||
| }, | ||
| ], | ||
| }); | ||
| } | ||
| }; | ||
| script.src = "https://chat.cdn-plain.com/index.js"; | ||
| d.getElementsByTagName("head")[0].appendChild(script); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
curl -sI "https://chat.cdn-plain.com/index.js" | grep -iE 'cache-control|etag|last-modified|content-type|x-version'Repository: potpie-ai/potpie-ui
Length of output: 259
Add crossOrigin="anonymous" and implement Content Security Policy — third-party script lacks integrity verification.
The Plain CDN serves a dynamically updated script (4-hour cache, ETags indicate frequent updates), making Subresource Integrity (SRI) impractical without continuous CI/CD hash updates. However, crossOrigin="anonymous" should still be added for consistency and CORS-safe error reporting.
Primary mitigation: Implement a strict Content Security Policy that explicitly allowlists https://chat.cdn-plain.com in the script-src directive. This prevents injection of unauthorized scripts from compromised or misconfigured CDNs.
Without CSP or SRI, a compromised chat.cdn-plain.com would have full DOM access and could exfiltrate user data, modify page content, or inject malicious payloads into every session.
📋 Recommended changes
const script = d.createElement("script");
script.id = "plain-chat-script";
script.async = false;
+script.crossOrigin = "anonymous";
script.onload = function () {Add to CSP header or <meta> tag:
Content-Security-Policy: script-src 'self' https://chat.cdn-plain.com;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/plain-chat.tsx` around lines 50 - 82, The injected Plain chat script
(created as script with id "plain-chat-script" and referenced via
window.Plain.init) lacks crossOrigin and no Content Security Policy is declared;
add crossOrigin="anonymous" when creating the script element to enable CORS-safe
error reporting, and ensure your app serves a strict CSP that allowlists the
Plain CDN (e.g., include https://chat.cdn-plain.com in script-src) so
third‑party scripts are restricted—update the code that creates the script
element and your server/meta headers to apply these changes.
Comment on lines
+53
to
+80
| script.onload = function () { | ||
| // @ts-ignore | ||
| if (window.Plain) { | ||
| // @ts-ignore | ||
| window.Plain.init({ | ||
| appId: "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5", | ||
| hideLauncher: true, | ||
| theme: 'light', | ||
| links: [ | ||
| { | ||
| icon: 'book', | ||
| text: 'Documentation', | ||
| url: 'https://docs.potpie.ai/introduction', | ||
| }, | ||
| { | ||
| icon: 'link', | ||
| text: 'Open Source', | ||
| url: 'https://github.com/potpie-ai/potpie', | ||
| }, | ||
| { | ||
| icon: 'discord', | ||
| text: 'Join our Discord', | ||
| url: 'https://discord.gg/ryk5CMD5v6', | ||
| }, | ||
| ], | ||
| }); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
No onerror handler — script load failure is silent.
If https://chat.cdn-plain.com/index.js fails to load (CDN outage, network error, CSP block), onload never fires and the chat widget is silently unavailable with no logging.
🛡️ Proposed fix
script.onload = function () {
if (window.Plain) {
window.Plain.init({ ... });
}
};
+script.onerror = function () {
+ console.error("[PlainChat] Failed to load Plain chat script from CDN.");
+};📝 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.
Suggested change
| script.onload = function () { | |
| // @ts-ignore | |
| if (window.Plain) { | |
| // @ts-ignore | |
| window.Plain.init({ | |
| appId: "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5", | |
| hideLauncher: true, | |
| theme: 'light', | |
| links: [ | |
| { | |
| icon: 'book', | |
| text: 'Documentation', | |
| url: 'https://docs.potpie.ai/introduction', | |
| }, | |
| { | |
| icon: 'link', | |
| text: 'Open Source', | |
| url: 'https://github.com/potpie-ai/potpie', | |
| }, | |
| { | |
| icon: 'discord', | |
| text: 'Join our Discord', | |
| url: 'https://discord.gg/ryk5CMD5v6', | |
| }, | |
| ], | |
| }); | |
| } | |
| }; | |
| script.onload = function () { | |
| // `@ts-ignore` | |
| if (window.Plain) { | |
| // `@ts-ignore` | |
| window.Plain.init({ | |
| appId: "liveChatApp_01KHRDQSB5WJHKCW4ZE3GFH7K5", | |
| hideLauncher: true, | |
| theme: 'light', | |
| links: [ | |
| { | |
| icon: 'book', | |
| text: 'Documentation', | |
| url: 'https://docs.potpie.ai/introduction', | |
| }, | |
| { | |
| icon: 'link', | |
| text: 'Open Source', | |
| url: 'https://github.com/potpie-ai/potpie', | |
| }, | |
| { | |
| icon: 'discord', | |
| text: 'Join our Discord', | |
| url: 'https://discord.gg/ryk5CMD5v6', | |
| }, | |
| ], | |
| }); | |
| } | |
| }; | |
| script.onerror = function () { | |
| console.error("[PlainChat] Failed to load Plain chat script from CDN."); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/plain-chat.tsx` around lines 53 - 80, The script insertion currently only
sets script.onload and has no error handling, so if loading
"https://chat.cdn-plain.com/index.js" fails the chat silently disappears; add a
script.onerror handler alongside script.onload that logs the failure (include
the URL and the error/event) using the app logger or console.error and
optionally sets a fallback state/flag so the UI can show a degraded message;
locate the script creation block that assigns script.onload and the
window.Plain.init call and add the onerror handler there to surface
network/CSP/load errors and avoid silent failures.
|
Cloud Run service deployed: https://pr-update-plain-support-cj6r7x3fpa-uc.a.run.app |
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
New Features
UI Improvements
Style