-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(ee): add publishable q&a conversations #1847
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a full FAQ system: DB schema and migration; visitor-facing app API for published FAQs; team-scoped CRUD API and page proxies; admin UI (publish/edit/delete) and viewer FAQ components; message selection UI updates; navigation/badge tweaks; and removal of a legacy set-cookie API. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Viewer
participant Client as Viewer UI (FAQSection)
participant AppAPI as App Route /api/faqs
participant Session as verifyDataroomSession
participant DB as Prisma/DB
Viewer->>Client: Load sidebar
Client->>AppAPI: GET /api/faqs?dataroomId&linkId[&documentId]
AppAPI->>Session: verifyDataroomSession()
Session-->>AppAPI: Valid/Invalid
alt Session invalid
AppAPI-->>Client: 401
else Session valid
AppAPI->>DB: findMany DataroomFaqItem (status=PUBLISHED + visibility filters)
DB-->>AppAPI: FAQ list
AppAPI-->>Client: 200 JSON VisitorFAQResponse[]
Client->>Viewer: Render Accordion with count
end
rect rgba(230,245,255,0.7)
note right of Client: On FAQ open -> view tracking
Client->>AppAPI: POST /api/faqs/{faqId}?dataroomId&linkId (mark viewed)
AppAPI->>DB: increment view or record view
DB-->>AppAPI: 2xx/4xx
AppAPI-->>Client: 2xx/4xx
end
sequenceDiagram
autonumber
actor Admin
participant UI as Admin UI (Publish/Edit Modals)
participant TeamAPI as /api/teams/:teamId/datarooms/:id/faqs[/:faqId]
participant Auth as NextAuth
participant DB as Prisma/DB
Admin->>UI: Submit publish/edit
UI->>TeamAPI: POST/PUT /faqs[/:faqId] (payload)
TeamAPI->>Auth: getSession()
Auth-->>TeamAPI: Session (user)
TeamAPI->>DB: validate refs & permissions
TeamAPI->>DB: create/update DataroomFaqItem
DB-->>TeamAPI: Created/Updated item
TeamAPI-->>UI: 201/200 with item
UI-->>Admin: Success toast & refresh
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ee/features/conversations/pages/conversation-overview.tsx (1)
191-193
: Avoid router.push during render.Navigate in an effect to prevent render-time side effects and repeated pushes.
- if (!limits?.conversationsInDataroom) { - router.push(`/datarooms/${dataroom?.id}/documents`); - } + useEffect(() => { + if (!limits?.conversationsInDataroom && dataroom?.id) { + router.push(`/datarooms/${dataroom.id}/documents`); + } + }, [limits?.conversationsInDataroom, dataroom?.id, router]);ee/features/conversations/components/viewer/conversation-view-sidebar.tsx (1)
89-100
: Gate the SWR fetch and enforce link-level filtering in the API
Front-end: change the
useSWR
key toconst conversationsKey = dataroomId && viewerId ? `/api/conversations?dataroomId=${dataroomId}&viewerId=${viewerId}&linkId=${linkId}` : null; const { data, error } = useSWR<Conversation[]>(conversationsKey, fetcher, { revalidateOnFocus: true, dedupingInterval: 5000, keepPreviousData: true, onError: (err) => { console.error("Error fetching conversations:", err); toast.error("Failed to load conversations"); }, });to avoid firing requests with
undefined
IDs.Back-end: in
ee/features/conversations/api/conversations-route.ts
update the GET handler toconst { dataroomId, viewerId, linkId } = req.query as { dataroomId?: string; viewerId?: string; linkId?: string; }; // … const conversations = await prisma.conversation.findMany({ where: { dataroomId, ...(linkId ? { linkId } : {}), participants: { some: { viewerId } }, }, // … });so the query filters by
linkId
and guarantees link-scoped isolation.
🧹 Nitpick comments (34)
components/layouts/breadcrumb.tsx (1)
168-172
: Use plural/explicit label and cover potential subroutes.
- Consider “Published FAQs” (or at least “FAQs”) to align with the new tab label.
- If detail routes like
/datarooms/[id]/conversations/faqs/[faqId]
exist (now or soon), add them here to avoid showing the dataroom name fallback.- case "/datarooms/[id]/conversations/faqs": - return "FAQ"; + case "/datarooms/[id]/conversations/faqs": + return "Published FAQs"; + // Optional future-proofing + case "/datarooms/[id]/conversations/faqs/[faqId]": + return "Published FAQs";components/ui/badge.tsx (1)
30-31
: Avoid conflicting radius classes.Base has
rounded-full
; this variant setsrounded-sm
. It renders fine (later class wins) but is brittle. Consider moving radius to a “shape” variant or set radius conditionally in the component.-const badgeVariants = cva( - "inline-flex items-center rounded-full border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2", +const badgeVariants = cva( + "inline-flex items-center border px-2.5 py-0.5 text-xs font-semibold transition-colors focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2", { variants: { - variant: { + variant: { default: - "border-transparent bg-primary text-primary-foreground hover:bg-primary/80", + "rounded-full border-transparent bg-primary text-primary-foreground hover:bg-primary/80", secondary: - "border-transparent bg-secondary text-secondary-foreground hover:bg-secondary/80", + "rounded-full border-transparent bg-secondary text-secondary-foreground hover:bg-secondary/80", @@ - notification: - "px-1.5 rounded-sm bg-secondary text-secondary-foreground border-transparent", + notification: + "rounded-sm px-1.5 bg-secondary text-secondary-foreground border-transparent",ee/features/conversations/components/shared/conversation-message.tsx (2)
31-42
: Make selection accessible and avoid misleading cursor.Add keyboard support and role only when selectable; don’t show pointer when not selectable.
- <div className="group relative"> + <div className="group relative"> <div - className={`flex w-max max-w-[80%] cursor-pointer flex-col rounded-lg px-4 py-2 transition-all ${ + className={`flex w-max max-w-[80%] flex-col rounded-lg px-4 py-2 transition-all ${ isAuthor ? "ml-auto bg-primary text-primary-foreground" : "bg-muted" } ${ isSelected ? "ring-2 ring-blue-500 ring-offset-2" : canBeSelected - ? "hover:ring-1 hover:ring-gray-300" + ? "cursor-pointer hover:ring-1 hover:ring-gray-300" : "" }`} - onClick={() => canBeSelected && onSelect?.(message.id, isVisitor)} + role={canBeSelected ? "button" : undefined} + tabIndex={canBeSelected ? 0 : undefined} + aria-pressed={canBeSelected ? isSelected : undefined} + onClick={() => canBeSelected && onSelect?.(message.id, isVisitor)} + onKeyDown={(e) => { + if (!canBeSelected) return; + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onSelect?.(message.id, isVisitor); + } + }} >
10-11
: Unused prop and source-of-truth for labels.
selectionType
isn’t used. Either remove it or use it to drive the “Q/A” label instead of inferring fromisVisitor
.- selectionType?: "question" | "answer"; + selectionType?: "question" | "answer"; @@ - {isVisitor ? ( + {(selectionType ?? (isVisitor ? "question" : "answer")) === + "question" ? ( <> <HelpCircle className="mr-1 h-3 w-3" /> {isSelected ? "Question" : "Q"} </> ) : ( <> <MessageSquareReply className="mr-1 h-3 w-3" /> {isSelected ? "Answer" : "A"} </> )}Also applies to: 60-71
ee/features/conversations/pages/conversation-overview.tsx (2)
76-87
: LocalStorage access: add SSR guard.This runs during render (via useMemo). It’s currently safe if
dataroom?.id
is falsy on SSR, but add an explicit window check to avoid accidental SSR reads.- const isBannerDismissed = useMemo(() => { - if (!dataroom?.id) return false; + const isBannerDismissed = useMemo(() => { + if (!dataroom?.id || typeof window === "undefined") return false; return ( localStorage.getItem( `dataroom-${dataroom.id}-conversations-banner-dismissed`, ) === "true" ); }, [dataroom?.id]);
95-112
: Revalidate after delete to refresh the list/count.Call SWR mutate after successful deletion.
- const { data: conversations = [], isLoading: isLoadingConversations } = + const { + data: conversations = [], + isLoading: isLoadingConversations, + mutate: mutateConversations, + } = useSWR<ConversationSummary[]>( @@ const handleDeleteConversation = async () => { @@ - toast.success("Conversation deleted successfully"); + toast.success("Conversation deleted successfully"); + // Refresh conversation list + mutateConversations();Also applies to: 143-174
prisma/schema/schema.prisma (1)
66-67
: Name and align the FAQ relation on both modelsIn
schema.prisma
(User model):- publishedFaqItems DataroomFaqItem[] + publishedFaqItems DataroomFaqItem[] @relation("PublishedFaqItems")In
conversation.prisma
(DataroomFaqItem model):- publishedByUser User @relation(fields: [publishedByUserId], references: [id], onDelete: Cascade) + publishedByUser User @relation("PublishedFaqItems", fields: [publishedByUserId], references: [id], onDelete: Cascade)Ensure the onDelete behavior remains
Cascade
(or switch toSetNull
if you’d rather preserve FAQs when a user is deleted).pages/api/teams/[teamId]/datarooms/[id]/faq/index.ts (1)
5-10
: Drop unnecessaryreturn await
in the wrapperNo need to await when returning a Promise directly.
export default async function handler( req: NextApiRequest, res: NextApiResponse, ) { - return await publishFAQRoute(req, res); + return publishFAQRoute(req, res); }components/view/nav.tsx (1)
294-300
: Accessibility: reflect toggle state on the “View FAQ” buttonExpose sidebar state with ARIA for screen readers; optionally wire to an element id via
aria-controls
.- <Button - onClick={() => setShowConversations(!showConversations)} - className="bg-gray-900 text-white hover:bg-gray-900/80" - > + <Button + onClick={() => setShowConversations(!showConversations)} + aria-expanded={showConversations} + aria-controls="conversation-sidebar" + className="bg-gray-900 text-white hover:bg-gray-900/80" + > View FAQ </Button>Also consider renaming state to
showFAQ
for clarity in a follow-up.components/view/dataroom/nav-dataroom.tsx (1)
175-177
: Accessibility: add ARIA to the FAQ toggleMirror open/closed state and control association.
- <Button onClick={() => setShowConversations(!showConversations)}> + <Button + onClick={() => setShowConversations(!showConversations)} + aria-expanded={showConversations} + aria-controls="conversation-sidebar" + > View FAQ </Button>Optional: rename
showConversations
→showFAQ
in a follow-up for semantic clarity.app/(ee)/api/faqs/route.ts (3)
42-47
: Strengthen typing and use Prisma enums instead of strings/anyAvoid
any
and stringly-typed enums for safer refactors and IDE help.+import { Prisma, FaqStatus, FaqVisibility } from "@prisma/client"; @@ - const whereClause: any = { - dataroomId, - status: "PUBLISHED", - }; + const whereClause: Prisma.DataroomFaqItemWhereInput = { + dataroomId, + status: FaqStatus.PUBLISHED, + }; @@ - const visibilityFilters: any[] = [ - { visibilityMode: "PUBLIC_DATAROOM" }, // Always include dataroom-wide FAQs - ]; + const visibilityFilters: Prisma.DataroomFaqItemWhereInput[] = [ + { visibilityMode: FaqVisibility.PUBLIC_DATAROOM }, // Always include dataroom-wide FAQs + ]; @@ - if (linkId) { + if (linkId) { visibilityFilters.push({ - visibilityMode: "PUBLIC_LINK", + visibilityMode: FaqVisibility.PUBLIC_LINK, linkId: linkId, }); } @@ - if (documentId) { + if (dataroomDocumentId) { visibilityFilters.push({ - visibilityMode: "PUBLIC_DOCUMENT", - dataroomDocumentId: documentId, + visibilityMode: FaqVisibility.PUBLIC_DOCUMENT, + dataroomDocumentId, }); }Also applies to: 48-68
70-90
: Optional: cap result size to prevent large payloadsIf a dataroom accumulates many FAQs, consider a sensible
take
(e.g., 200) and pagination later.- const faqs = await prisma.dataroomFaqItem.findMany({ + const faqs = await prisma.dataroomFaqItem.findMany({ where: whereClause, select: { id: true, @@ }, - orderBy: { createdAt: "desc" }, + orderBy: { createdAt: "desc" }, + take: 200, });
1-5
: Consider forcing dynamic rendering due to session-bound responsesThis route depends on auth/session; make it explicit to avoid accidental static optimization.
import { NextRequest, NextResponse } from "next/server"; +export const dynamic = "force-dynamic";
ee/features/conversations/components/viewer/conversation-view-sidebar.tsx (4)
219-226
: FAQ integration looks goodPlugs in cleanly at the top of the content area and keeps the rest of the layout intact. Minor: you pass viewerId to FAQSection but it isn’t used in that component. Safe to drop.
122-126
: Harden error handling: JSON-only parsing can throwFalling back to response.text() prevents secondary errors when the API returns non-JSON.
Apply:
- if (!response.ok) { - const errorData = await response.json(); - toast.error(errorData.error); - return; - } + if (!response.ok) { + let message = "Failed to create conversation"; + try { + const err = await response.json(); + message = err.error || message; + } catch { + message = (await response.text()) || message; + } + toast.error(message); + return; + }- if (!response.ok) { - const errorData = await response.json(); - toast.error(errorData.error); - return; - } + if (!response.ok) { + let message = "Failed to send message"; + try { + const err = await response.json(); + message = err.error || message; + } catch { + message = (await response.text()) || message; + } + toast.error(message); + return; + }Also applies to: 165-169
140-141
: Copy: align with “Question” terminologyUse “Question posted” for consistency with the new UI.
- toast.success("Conversation created successfully"); + toast.success("Question posted");
336-351
: Prevent double-submits on slow networksOptional: add an isSending flag to disable the Send button during the POST.
Add state near Line 79:
+ const [isSending, setIsSending] = useState(false);
Wrap the handler:
- const handleSendMessage = async (e: React.FormEvent) => { + const handleSendMessage = async (e: React.FormEvent) => { e.preventDefault(); if (!newMessage.trim() || !activeConversation) return; - - try { + setIsSending(true); + try { const response = await fetch(`/api/conversations/messages`, { method: "POST", headers: { "Content-Type": "application/json", },And the footer:
- <Button type="submit" disabled={!newMessage.trim()}> + <Button type="submit" disabled={!newMessage.trim() || isSending}> Send </Button>Finally, after Line 200:
+ } finally { + setIsSending(false); + }ee/features/conversations/components/dashboard/publish-faq-modal.tsx (1)
175-178
: Harden error handling: support non-JSON responsesPrevents secondary exceptions when the API sends plain text.
- if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || "Failed to publish FAQ"); - } + if (!response.ok) { + let message = "Failed to publish FAQ"; + try { + const err = await response.json(); + message = err.error || message; + } catch { + message = (await response.text()) || message; + } + throw new Error(message); + }ee/features/conversations/components/dashboard/edit-faq-modal.tsx (3)
25-26
: Import type-only to avoid pulling a page module into the client bundleThis keeps runtime bundles lean and avoids accidental circulars.
-import { PublishedFAQ } from "../../pages/faq-overview"; +import type { PublishedFAQ } from "../../pages/faq-overview";
130-133
: Harden error handling: support non-JSON responsesSame rationale as in publish modal.
- if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || "Failed to update FAQ"); - } + if (!response.ok) { + let message = "Failed to update FAQ"; + try { + const err = await response.json(); + message = err.error || message; + } catch { + message = (await response.text()) || message; + } + throw new Error(message); + }
42-61
: Deduplicate visibilityOptionsPublish and Edit modals duplicate the same options. Extract to a shared module (e.g., ee/features/conversations/constants.ts) and import in both to keep UI consistent.
ee/features/conversations/pages/faq-overview.tsx (2)
457-463
: Clear selection when closing the delete dialogPrevents stale faqToDelete if the dialog is dismissed.
- <Dialog - open={isDeleteModalOpen} - onOpenChange={setIsDeleteModalOpen} - > + <Dialog + open={isDeleteModalOpen} + onOpenChange={(open) => { + setIsDeleteModalOpen(open); + if (!open) setFaqToDelete(null); + }} + >
489-505
: Avoid non-null assertions when passing IDsRender the modal only when context exists and pass non-asserted values.
- {faqToEdit && ( + {faqToEdit && dataroom?.id && teamId && ( <EditFAQModal faq={faqToEdit} - dataroomId={dataroom?.id!} - teamId={teamId!} + dataroomId={dataroom.id} + teamId={teamId} isOpen={isEditModalOpen} onClose={() => { setIsEditModalOpen(false); setFaqToEdit(null); }} onSuccess={() => { - mutate(`/api/teams/${teamId}/datarooms/${dataroom?.id}/faq`); + mutate(`/api/teams/${teamId}/datarooms/${dataroom.id}/faq`); setIsEditModalOpen(false); setFaqToEdit(null); }} /> )}ee/features/conversations/components/viewer/faq-section.tsx (3)
49-61
: Don’t mask errors as “0 FAQs”Expose a small error state instead of falling through to the empty list UI.
- const { data: faqs = [], isLoading } = useSWR<FAQ[]>( + const { data: faqs = [], isLoading, error } = useSWR<FAQ[]>( `/api/faqs?${new URLSearchParams({ ...(dataroomId && { dataroomId }), ...(linkId && { linkId }), ...(documentId && { documentId }), }).toString()}`, fetcher, { revalidateOnFocus: false, dedupingInterval: 30000, // 30 seconds keepPreviousData: true, }, );And above Line 152:
+ if (error) { + return ( + <> + <div className="bg-gray-50 p-4 dark:bg-gray-900"> + <div className="mb-1 font-medium text-primary">Frequently Asked Questions</div> + <div className="text-sm text-destructive">Failed to load FAQs.</div> + </div> + <Separator /> + </> + ); + }
69-76
: Track views with documentId too (when available)Improves attribution and deduplication on the backend.
- const params = new URLSearchParams({ - ...(dataroomId && { dataroomId }), - ...(linkId && { linkId }), - }); + const params = new URLSearchParams({ + ...(dataroomId && { dataroomId }), + ...(linkId && { linkId }), + ...(documentId && { documentId }), + });
37-43
: viewerId prop is unusedDrop it here and at the call-site to avoid confusion.
-export function FAQSection({ - dataroomId, - linkId, - documentId, - viewerId, -}: FAQSectionProps) { +export function FAQSection({ dataroomId, linkId, documentId }: FAQSectionProps) {ee/features/conversations/pages/conversation-detail.tsx (3)
126-145
: Select truly latest Q/A by createdAt instead of array orderIf
conversation.messages
isn’t guaranteed sorted, using.pop()
logic by position can pick the wrong pair. Select bycreatedAt
.- const visitorMessages = conversation.messages.filter( - (msg) => msg.viewerId !== null, - ); - const latestQuestion = visitorMessages[visitorMessages.length - 1]; + const latestQuestion = conversation.messages + .filter((m) => m.viewerId !== null) + .reduce((acc, m) => (acc && new Date(acc.createdAt) > new Date(m.createdAt) ? acc : m), null as any); ... - const adminMessages = conversation.messages.filter( - (msg) => msg.userId !== null, - ); - const latestAnswer = adminMessages[adminMessages.length - 1]; + const latestAnswer = conversation.messages + .filter((m) => m.userId !== null) + .reduce((acc, m) => (acc && new Date(acc.createdAt) > new Date(m.createdAt) ? acc : m), null as any);
53-53
: Use a type-only import for PublishedFAQ to avoid bundling page codePrevents unnecessary client bundle impact from importing a page module.
-import { PublishedFAQ } from "./faq-overview"; +import type { PublishedFAQ } from "./faq-overview";
355-357
: Ensure dataroomId is string when constructing the URL
router.query.id
can bestring | string[]
. Coerce safely.- router.push(`/datarooms/${dataroomId}/conversations/${id}`); + router.push(`/datarooms/${String(dataroomId)}/conversations/${id}`);ee/features/conversations/api/team-faqs-route.ts (2)
151-169
: Return consistent relations on create responseInclude
sourceConversation
,questionMessage
, andanswerMessage
to match the GET payload and frontend typing expectations.include: { dataroom: { select: { name: true }, }, link: { select: { name: true }, }, dataroomDocument: { include: { document: { select: { name: true }, }, }, }, publishedByUser: { select: { name: true, email: true }, }, + sourceConversation: { select: { id: true } }, + questionMessage: { select: { id: true, content: true } }, + answerMessage: { select: { id: true, content: true } }, },
295-309
: Optional: allow validated changes to link/document on updatePUT currently can’t move an FAQ between link/document scopes. If needed, add
linkId
/dataroomDocumentId
updates with the same validations used in POST.prisma/migrations/20250830000000_dataroom_faq_items_model/migration.sql (2)
61-61
: Add a composite index to speed common filtersPublic/overview queries often filter by dataroom + status + visibility. Add a composite index.
+-- CreateIndex (composite for common filters) +CREATE INDEX "DataroomFaqItem_dataroom_status_visibility_idx" + ON "DataroomFaqItem"("dataroomId" ASC, "status" ASC, "visibilityMode" ASC);
61-61
: Optional: prevent duplicate Q/A pairs within a dataroomIf duplicates are undesirable, add a partial unique index (Postgres) guarding non-null pairs.
+-- Unique partial index to avoid duplicate pairs in same dataroom +CREATE UNIQUE INDEX "DataroomFaqItem_unique_pair_per_dataroom" + ON "DataroomFaqItem"("dataroomId", "questionMessageId", "answerMessageId") + WHERE "questionMessageId" IS NOT NULL AND "answerMessageId" IS NOT NULL;prisma/schema/conversation.prisma (1)
150-213
: Schema-level composite index for common FAQ filtersMirror the migration by adding a composite index to guide future migrations and keep Prisma schema authoritative.
model DataroomFaqItem { ... @@index([dataroomId]) + @@index([dataroomId, status, visibilityMode]) @@index([linkId]) @@index([dataroomDocumentId]) @@index([sourceConversationId]) @@index([teamId]) @@index([publishedByUserId]) @@index([status]) @@index([visibilityMode]) @@index([createdAt]) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (26)
app/(ee)/api/faqs/route.ts
(1 hunks)components/datarooms/dataroom-navigation.tsx
(1 hunks)components/layouts/breadcrumb.tsx
(1 hunks)components/ui/badge.tsx
(1 hunks)components/view/dataroom/nav-dataroom.tsx
(2 hunks)components/view/nav.tsx
(1 hunks)ee/features/conversations/api/team-faqs-route.ts
(1 hunks)ee/features/conversations/components/conversation-document-context.tsx
(0 hunks)ee/features/conversations/components/dashboard/edit-faq-modal.tsx
(1 hunks)ee/features/conversations/components/dashboard/publish-faq-modal.tsx
(1 hunks)ee/features/conversations/components/shared/conversation-message.tsx
(1 hunks)ee/features/conversations/components/viewer/conversation-view-sidebar.tsx
(8 hunks)ee/features/conversations/components/viewer/faq-section.tsx
(1 hunks)ee/features/conversations/pages/conversation-detail.tsx
(7 hunks)ee/features/conversations/pages/conversation-overview.tsx
(6 hunks)ee/features/conversations/pages/faq-overview.tsx
(1 hunks)pages/api/auth-plus/set-cookie.ts
(0 hunks)pages/api/teams/[teamId]/datarooms/[id]/faq/[faqId].ts
(1 hunks)pages/api/teams/[teamId]/datarooms/[id]/faq/index.ts
(1 hunks)pages/datarooms/[id]/conversations/faqs/index.tsx
(1 hunks)prisma/migrations/20250830000000_dataroom_faq_items_model/migration.sql
(1 hunks)prisma/schema/conversation.prisma
(3 hunks)prisma/schema/dataroom.prisma
(2 hunks)prisma/schema/link.prisma
(1 hunks)prisma/schema/schema.prisma
(1 hunks)prisma/schema/team.prisma
(1 hunks)
💤 Files with no reviewable changes (2)
- pages/api/auth-plus/set-cookie.ts
- ee/features/conversations/components/conversation-document-context.tsx
🧰 Additional context used
🧬 Code graph analysis (13)
pages/api/teams/[teamId]/datarooms/[id]/faq/index.ts (1)
pages/api/teams/[teamId]/datarooms/[id]/faq/[faqId].ts (1)
handler
(5-10)
components/view/nav.tsx (1)
components/ui/button.tsx (1)
Button
(71-71)
pages/api/teams/[teamId]/datarooms/[id]/faq/[faqId].ts (1)
pages/api/teams/[teamId]/datarooms/[id]/faq/index.ts (1)
handler
(5-10)
ee/features/conversations/components/dashboard/publish-faq-modal.tsx (4)
components/ui/dialog.tsx (6)
Dialog
(149-149)DialogContent
(151-151)DialogHeader
(152-152)DialogTitle
(154-154)DialogDescription
(155-155)DialogFooter
(153-153)components/ui/textarea.tsx (1)
Textarea
(25-25)components/ui/select.tsx (5)
Select
(151-151)SelectTrigger
(154-154)SelectValue
(153-153)SelectContent
(155-155)SelectItem
(157-157)components/ui/button.tsx (1)
Button
(71-71)
ee/features/conversations/pages/faq-overview.tsx (5)
lib/swr/use-dataroom.ts (1)
useDataroom
(21-55)context/team-context.tsx (1)
useTeam
(85-85)lib/utils.ts (2)
fetcher
(48-62)timeAgo
(165-183)ee/features/conversations/pages/conversation-overview.tsx (1)
ConversationSummary
(41-60)ee/features/conversations/components/dashboard/edit-faq-modal.tsx (1)
EditFAQModal
(63-283)
components/view/dataroom/nav-dataroom.tsx (1)
components/ui/button.tsx (1)
Button
(71-71)
ee/features/conversations/components/dashboard/edit-faq-modal.tsx (1)
ee/features/conversations/pages/faq-overview.tsx (1)
PublishedFAQ
(60-95)
ee/features/conversations/components/viewer/faq-section.tsx (1)
lib/utils.ts (1)
fetcher
(48-62)
ee/features/conversations/components/viewer/conversation-view-sidebar.tsx (1)
ee/features/conversations/components/viewer/faq-section.tsx (1)
FAQSection
(37-247)
ee/features/conversations/components/shared/conversation-message.tsx (1)
components/conversations/index.tsx (1)
ConversationMessage
(10-12)
ee/features/conversations/api/team-faqs-route.ts (5)
pages/api/auth/[...nextauth].ts (1)
authOptions
(35-204)lib/types.ts (1)
CustomUser
(17-17)lib/utils/sanitize-html.ts (1)
validateContent
(3-17)pages/api/teams/[teamId]/datarooms/[id]/faq/[faqId].ts (1)
handler
(5-10)pages/api/teams/[teamId]/datarooms/[id]/faq/index.ts (1)
handler
(5-10)
ee/features/conversations/pages/conversation-detail.tsx (6)
ee/features/conversations/pages/faq-overview.tsx (1)
PublishedFAQ
(60-95)lib/utils.ts (1)
fetcher
(48-62)ee/features/conversations/components/dashboard/conversation-list-item.tsx (1)
ConversationListItem
(7-80)ee/features/conversations/components/shared/conversation-document-context.tsx (1)
ConversationDocumentContext
(16-57)ee/features/conversations/components/shared/conversation-message.tsx (1)
ConversationMessage
(4-82)ee/features/conversations/components/dashboard/publish-faq-modal.tsx (1)
PublishFAQModal
(90-350)
ee/features/conversations/pages/conversation-overview.tsx (5)
context/team-context.tsx (1)
useTeam
(85-85)ee/features/conversations/pages/faq-overview.tsx (1)
PublishedFAQ
(60-95)lib/utils.ts (1)
fetcher
(48-62)components/conversations/index.tsx (1)
ConversationListItem
(6-8)ee/features/conversations/components/dashboard/conversation-list-item.tsx (1)
ConversationListItem
(7-80)
🪛 GitHub Check: CodeQL
ee/features/conversations/components/dashboard/publish-faq-modal.tsx
[failure] 164-173: Server-side request forgery
The URL of this request depends on a user-provided value.
The URL of this request depends on a user-provided value.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
components/datarooms/dataroom-navigation.tsx (1)
29-33
: Microcopy consistency: “Q&A” vs “Conversations”.Breadcrumbs use “Conversations” while the nav shows “Q&A”. If intentional, ignore; otherwise standardize to a single term to reduce user confusion.
components/ui/badge.tsx (1)
30-31
: LGTM: notification variant.Variant fits the count-badge use case and the typing surface updates correctly.
ee/features/conversations/pages/conversation-overview.tsx (2)
9-15
: Type import source may be wrong.You import
PublishedFAQ
fromcomponents/dashboard/published-faq-table
, while the type appears defined inpages/faq-overview.tsx
in this PR’s context. Ensure the actual export exists to avoid TS errors.
218-242
: Tabs + Link: confirm intended UX.Using TabsTrigger asChild Link navigates away instead of switching local tab state. If that’s the design (split routes), fine. If you want client-side tabbing on this page, remove Link and render the FAQs panel here.
prisma/schema/team.prisma (1)
22-22
: LGTM: adds Team → FAQ items inverse relation.No issues spotted with the back-reference.
pages/datarooms/[id]/conversations/faqs/index.tsx (1)
1-1
: Route alias re-export looks good.Keeps the page path stable while centralizing implementation.
prisma/schema/link.prisma (1)
68-70
: Confirm desired FK behavior for link-scoped FAQs.If a Link is deleted, should its FAQs be deleted or preserved for dataroom-level visibility? Ensure DataroomFaqItem.linkId uses onDelete: SetNull (likely preferable) rather than Cascade.
prisma/schema/dataroom.prisma (1)
22-24
: No changes required – theDataroomFaqItem
model already definesdataroom Dataroom @relation(fields: [dataroomId], references: [id], onDelete: Cascade)
so deleting a dataroom will cascade-delete its FAQs.
app/(ee)/api/faqs/route.ts (1)
93-106
: LGTM on response mappingShape matches the declared
VisitorFAQResponse
and guards optional document name.ee/features/conversations/components/dashboard/publish-faq-modal.tsx (2)
164-173
: CodeQL SSRF: false positive for client-side same-origin fetchThis is a browser fetch to a relative path; SSRF doesn’t apply. Keep, but ensure the server route validates teamId/dataroomId authorization.
290-326
: Server-side guard recommended for visibility/document mismatchIf visibilityMode is PUBLIC_DOCUMENT while dataroomDocumentId is null, the API should reject the request. Please confirm backend validation exists.
prisma/schema/conversation.prisma (1)
122-125
: Reverse relations on Message look goodThe
faqAsQuestion
/faqAsAnswer
relations are correctly named and scoped.
ee/features/conversations/components/shared/conversation-message.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
ee/features/conversations/pages/conversation-overview.tsx (1)
190-194
: Avoid calling router.push during render; redirect in an effect instead.Calling router.push in render can cause double renders and hydration glitches. Move it to useEffect and return null while redirecting.
// Initialize local state from dataroom useEffect(() => { if (dataroom) { setLocalConversationsEnabled(dataroom.conversationsEnabled); } }, [dataroom]); + + // Redirect to documents if conversations feature is not available + useEffect(() => { + if (!dataroom) return; + if (!limits?.conversationsInDataroom) { + router.replace(`/datarooms/${dataroom.id}/documents`); + } + }, [router, dataroom?.id, limits?.conversationsInDataroom]);- if (!limits?.conversationsInDataroom) { - // Redirect to documents page if conversations are not enabled - router.push(`/datarooms/${dataroom?.id}/documents`); - } + if (!limits?.conversationsInDataroom) return null;Also applies to: 95-95
♻️ Duplicate comments (7)
ee/features/conversations/api/team-faqs-route.ts (3)
190-216
: Good: message IDs are verified against conversation + dataroom.
124-136
: Use findFirst (findUnique + relational filters is invalid in Prisma).Relational filters under team aren’t allowed with findUnique. Switch to findFirst.
- const dataroom = await prisma.dataroom.findUnique({ + const dataroom = await prisma.dataroom.findFirst({ where: { id: dataroomId, team: { id: teamId, users: { some: { userId } }, }, }, select: { id: true, teamId: true, }, });
290-301
: Same: replace findUnique with findFirst in GET path.- const dataroom = await prisma.dataroom.findUnique({ + const dataroom = await prisma.dataroom.findFirst({ where: { id: dataroomId, team: { id: teamId, users: { some: { userId } }, }, }, select: { id: true, }, });ee/features/conversations/pages/conversation-detail.tsx (3)
681-691
: Nice: revalidates FAQs after publish.
468-483
: Guard against empty participants array.Avoid accessing participants[0] blindly.
- {conversation.participants[0].email - ? conversation.participants[0].email + {conversation.participants?.[0]?.email + ? conversation.participants?.[0]?.email .charAt(0) .toUpperCase() : "?"} ... - {conversation.participants[0].email || + {conversation.participants?.[0]?.email || "Anonymous Viewer"}
599-605
: Session can be undefined: null-safe isAuthor check.- isAuthor={ - message.userId === - (session?.user as CustomUser).id - } + isAuthor={ + message.userId === + ((session?.user as CustomUser | undefined)?.id ?? + null) + } ... - senderEmail={ - conversation.participants[0].email || "Viewer" - } + senderEmail={ + conversation.participants?.[0]?.email || + "Viewer" + }ee/features/conversations/pages/conversation-overview.tsx (1)
113-124
: Endpoint fixed to “/faqs” — looks good.The SWR key now hits the pluralized route; this resolves the prior 404 concern.
🧹 Nitpick comments (4)
ee/features/conversations/api/team-faqs-route.ts (1)
351-445
: Extend update validation for visibility-mode-specific fields.When switching to PUBLIC_DOCUMENT or PUBLIC_LINK, enforce presence/validity of dataroomDocumentId/linkId (and optionally clear the other). Consider Zod refinements.
ee/features/conversations/pages/conversation-overview.tsx (3)
95-112
: Optimistically update the conversations list after delete (SWR mutate).Right now the UI stays stale until the next revalidation. Use SWR’s mutate to remove the item immediately.
- const { data: conversations = [], isLoading: isLoadingConversations } = + const { + data: conversations = [], + isLoading: isLoadingConversations, + mutate: mutateConversations, + } = useSWR<ConversationSummary[]>(if (!response.ok) throw new Error("Failed to delete conversation"); + // Optimistically remove from cache; skip immediate revalidation + await mutateConversations( + (prev) => + (prev ?? []).filter( + (c) => c.id !== conversationToDeleteParsed, + ), + { revalidate: false }, + ); toast.success("Conversation deleted successfully");Also applies to: 143-174
9-9
: Unify PublishedFAQ type source (avoid cross-import from a table component).Importing a core API type from a UI table component is brittle. Prefer a single shared types module (e.g., ee/features/conversations/types.ts) and re-use across pages/components.
Proposed extraction:
// ee/features/conversations/types.ts export interface PublishedFAQ { id: string; editedQuestion: string; originalQuestion?: string; answer: string; visibilityMode: "PUBLIC_DATAROOM" | "PUBLIC_LINK" | "PUBLIC_DOCUMENT"; status: "DRAFT" | "PUBLISHED" | "ARCHIVED"; createdAt: string; updatedAt: string; dataroom: { name: string }; link?: { name: string }; dataroomDocument?: { document: { name: string } }; publishedByUser: { name: string; email: string }; sourceConversation?: { id: string }; questionMessage?: { id: string; content: string }; answerMessage?: { id: string; content: string }; }Then update this file:
-import { PublishedFAQ } from "@/ee/features/conversations/components/dashboard/published-faq-table"; +import { PublishedFAQ } from "@/ee/features/conversations/types";I can scan the repo for other PublishedFAQ definitions and consolidate them if you’d like.
218-242
: Minor: prevent transient tab state when navigating to FAQs.Clicking the FAQs tab both switches Tabs value and navigates via Link. To avoid a brief empty state, intercept value change and push the route, or keep the trigger as a plain Link without changing Tabs value.
Example minimal tweak:
- <Tabs - value={activeTab} - onValueChange={setActiveTab} - className="space-y-6" - > + <Tabs + value={activeTab} + onValueChange={(v) => { + if (v === "faqs") { + router.push(`/datarooms/${dataroom.id}/conversations/faqs`); + return; + } + setActiveTab(v); + }} + className="space-y-6" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
app/(ee)/api/faqs/route.ts
(1 hunks)ee/features/conversations/api/team-faqs-route.ts
(1 hunks)ee/features/conversations/components/dashboard/edit-faq-modal.tsx
(1 hunks)ee/features/conversations/components/dashboard/publish-faq-modal.tsx
(1 hunks)ee/features/conversations/components/shared/conversation-message.tsx
(1 hunks)ee/features/conversations/pages/conversation-detail.tsx
(7 hunks)ee/features/conversations/pages/conversation-overview.tsx
(6 hunks)ee/features/conversations/pages/faq-overview.tsx
(1 hunks)pages/api/teams/[teamId]/datarooms/[id]/faqs/[faqId].ts
(1 hunks)pages/api/teams/[teamId]/datarooms/[id]/faqs/index.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/(ee)/api/faqs/route.ts
- ee/features/conversations/components/dashboard/edit-faq-modal.tsx
- ee/features/conversations/components/shared/conversation-message.tsx
- ee/features/conversations/pages/faq-overview.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
pages/api/teams/[teamId]/datarooms/[id]/faqs/[faqId].ts (1)
pages/api/teams/[teamId]/datarooms/[id]/faqs/index.ts (1)
handler
(5-10)
pages/api/teams/[teamId]/datarooms/[id]/faqs/index.ts (1)
pages/api/teams/[teamId]/datarooms/[id]/faqs/[faqId].ts (1)
handler
(5-10)
ee/features/conversations/pages/conversation-detail.tsx (6)
ee/features/conversations/pages/faq-overview.tsx (1)
PublishedFAQ
(67-102)lib/utils.ts (1)
fetcher
(48-62)ee/features/conversations/components/dashboard/conversation-list-item.tsx (1)
ConversationListItem
(7-80)ee/features/conversations/components/shared/conversation-document-context.tsx (1)
ConversationDocumentContext
(16-57)ee/features/conversations/components/shared/conversation-message.tsx (1)
ConversationMessage
(4-82)ee/features/conversations/components/dashboard/publish-faq-modal.tsx (1)
PublishFAQModal
(111-390)
ee/features/conversations/api/team-faqs-route.ts (5)
pages/api/auth/[...nextauth].ts (1)
authOptions
(35-204)lib/types.ts (1)
CustomUser
(17-17)lib/utils/sanitize-html.ts (1)
validateContent
(3-17)pages/api/teams/[teamId]/datarooms/[id]/faqs/[faqId].ts (1)
handler
(5-10)pages/api/teams/[teamId]/datarooms/[id]/faqs/index.ts (1)
handler
(5-10)
ee/features/conversations/components/dashboard/publish-faq-modal.tsx (4)
components/ui/dialog.tsx (6)
Dialog
(149-149)DialogContent
(151-151)DialogHeader
(152-152)DialogTitle
(154-154)DialogDescription
(155-155)DialogFooter
(153-153)components/ui/textarea.tsx (1)
Textarea
(25-25)components/ui/select.tsx (5)
Select
(151-151)SelectTrigger
(154-154)SelectValue
(153-153)SelectContent
(155-155)SelectItem
(157-157)components/ui/button.tsx (1)
Button
(71-71)
ee/features/conversations/pages/conversation-overview.tsx (6)
context/team-context.tsx (1)
useTeam
(85-85)lib/webstorage.ts (1)
localStorage
(6-33)ee/features/conversations/pages/faq-overview.tsx (1)
PublishedFAQ
(67-102)lib/utils.ts (1)
fetcher
(48-62)components/conversations/index.tsx (1)
ConversationListItem
(6-8)ee/features/conversations/components/dashboard/conversation-list-item.tsx (1)
ConversationListItem
(7-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
ee/features/conversations/api/team-faqs-route.ts (1)
218-238
: Ignore the status field concern
Thestatus
column onDataroomFaqItem
is already defined with@default(PUBLISHED)
in the Prisma schema, so the create call will automatically set it toPUBLISHED
.Likely an incorrect or invalid review comment.
pages/api/teams/[teamId]/datarooms/[id]/faqs/index.ts (1)
5-10
: Thin proxy looks good.pages/api/teams/[teamId]/datarooms/[id]/faqs/[faqId].ts (1)
5-10
: Wrapper delegation is correct and consistent.
ee/features/conversations/components/dashboard/publish-faq-modal.tsx
Outdated
Show resolved
Hide resolved
ee/features/conversations/components/dashboard/publish-faq-modal.tsx
Outdated
Show resolved
Hide resolved
// Memoize banner dismissed state to avoid localStorage reads on every render | ||
const isBannerDismissed = useMemo(() => { | ||
if (!dataroom?.id) return false; | ||
return ( | ||
localStorage.getItem( | ||
`dataroom-${dataroom.id}-conversations-banner-dismissed`, | ||
) === "true" | ||
); | ||
}, [dataroom?.id]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix banner dismissal (state never updates) and make localStorage access SSR-safe.
- Parent reads localStorage via useMemo and never re-checks after dismissal, so the banner keeps showing until a reload.
- Direct window.localStorage access can throw during SSR or restricted storage contexts. Use the project’s safe wrapper.
Apply the diff to replace the memo with state + effect, and to gate rendering on isConversationsEnabled as well:
- import { useEffect, useMemo, useState } from "react";
+ import { useEffect, useState } from "react";
+ import { localStorage as safeLocalStorage } from "@/lib/webstorage";
- const [activeTab, setActiveTab] = useState("conversations");
-
- // Memoize banner dismissed state to avoid localStorage reads on every render
- const isBannerDismissed = useMemo(() => {
- if (!dataroom?.id) return false;
- return (
- localStorage.getItem(
- `dataroom-${dataroom.id}-conversations-banner-dismissed`,
- ) === "true"
- );
- }, [dataroom?.id]);
+ const [activeTab, setActiveTab] = useState("conversations");
+ const [isBannerDismissed, setIsBannerDismissed] = useState(false);
+ useEffect(() => {
+ if (!dataroom?.id) return;
+ // Safe read; tolerates SSR and restricted storage
+ setIsBannerDismissed(
+ safeLocalStorage.getItem(
+ `dataroom-${dataroom.id}-conversations-banner-dismissed`,
+ ) === "true",
+ );
+ }, [dataroom?.id]);
- {/* Show banner unless it's been dismissed */}
- {!isBannerDismissed && (
+ {/* Show banner only if not dismissed and conversations are disabled */}
+ {!isBannerDismissed && !isConversationsEnabled && (
<ConversationsNotEnabledBanner
dataroomId={dataroom.id}
teamId={teamId as string}
isConversationsEnabled={isConversationsEnabled}
onConversationsToggled={handleConversationsToggled}
+ // If the banner component exposes an onDismiss, wire it up:
+ onDismiss={() => setIsBannerDismissed(true)}
/>
)}
If ConversationsNotEnabledBanner doesn’t expose onDismiss, I can add it (and have it persist the flag). Want me to open a quick follow-up?
Also applies to: 208-217, 4-4
🏁 Script executed:
#!/bin/bash
# Locate the conversation overview page
fd -e tsx ee/features/conversations/pages/conversation-overview.tsx
Length of output: 69
🏁 Script executed:
#!/bin/bash
# Find the ConversationOverview component path
fd conversation-overview.tsx
Length of output: 88
🏁 Script executed:
#!/bin/bash
# 1. Inspect imports and initial code in conversation-overview
sed -n '1,50p' ee/features/conversations/pages/conversation-overview.tsx
# 2. Inspect the banner logic around lines 60-130
sed -n '60,130p' ee/features/conversations/pages/conversation-overview.tsx
# 3. List webstorage utilities
fd lib/webstorage
# 4. Search for safeLocalStorage export or definition
rg -n 'export.*localStorage' -C2 lib/webstorage
# 5. Locate ConversationsNotEnabledBanner usage and definition
rg -n 'ConversationsNotEnabledBanner' -n ee/features/conversations -C1
rg -n 'ConversationsNotEnabledBanner' -g '*banner*.tsx'
Length of output: 6624
🏁 Script executed:
#!/bin/bash
# 1. List top-level directories to locate lib folder
find . -maxdepth 2 -type d
# 2. Search for any files or directories related to storage utilities
fd storage -t f
fd webstorage -t f -t d
# 3. Search for safeLocalStorage references across the repo
rg -n "safeLocalStorage" -C2 .
# 4. Inspect ConversationsNotEnabledBanner props and implementation
sed -n '1,100p' ee/features/conversations/components/dashboard/conversations-not-enabled-banner.tsx
Length of output: 4500
🏁 Script executed:
#!/bin/bash
# Print the safeLocalStorage implementation
sed -n '1,200p' lib/webstorage.ts
Length of output: 1130
🏁 Script executed:
# Inspect where the banner handles dismissal
rg -n "setIsDismissed" -C2 ee/features/conversations/components/dashboard/conversations-not-enabled-banner.tsx
rg -n "onClick" -C2 ee/features/conversations/components/dashboard/conversations-not-enabled-banner.tsx
Length of output: 1474
Revise localStorage usage to be SSR-safe and fix banner dismissal
-
ee/features/conversations/pages/conversation-overview.tsx
• Remove theuseMemo
block (lines ~78–87) and instead:
– ImportsafeLocalStorage
fromlib/webstorage.ts
and dropuseMemo
from the import.
– InitializeisBannerDismissed
viauseState(false)
and in auseEffect([dataroom?.id])
callsafeLocalStorage.getItem
to set it.
– Change the banner render condition to!isBannerDismissed && !isConversationsEnabled
and passonDismiss={() => setIsBannerDismissed(true)}
into<ConversationsNotEnabledBanner>
. -
ee/features/conversations/components/dashboard/conversations-not-enabled-banner.tsx
• ImportsafeLocalStorage
instead of usingwindow.localStorage
.
• Update alllocalStorage.getItem
/setItem
calls tosafeLocalStorage
.
• ExtendConversationsNotEnabledBannerProps
withonDismiss?: () => void
and inhandleDismiss
after setting the flag callonDismiss?.()
.
These changes ensure no SSR errors, persist dismissal immediately, and gate the banner when conversations are already enabled.
🤖 Prompt for AI Agents
In ee/features/conversations/pages/conversation-overview.tsx around lines 78–87
and
ee/features/conversations/components/dashboard/conversations-not-enabled-banner.tsx
update storage usage to be SSR-safe: remove the useMemo block in
conversation-overview, import safeLocalStorage from lib/webstorage.ts and drop
useMemo import, create const [isBannerDismissed, setIsBannerDismissed] =
useState(false) and in a useEffect listening on [dataroom?.id] call
safeLocalStorage.getItem(`dataroom-${dataroom.id}-conversations-banner-dismissed`)
to setIsBannerDismissed(Boolean value), change the banner render to
!isBannerDismissed && !isConversationsEnabled and pass onDismiss={() =>
setIsBannerDismissed(true)} to <ConversationsNotEnabledBanner>; in
conversations-not-enabled-banner.tsx import safeLocalStorage instead of
window.localStorage, replace all localStorage.getItem/setItem calls with
safeLocalStorage equivalents, extend ConversationsNotEnabledBannerProps with
optional onDismiss?: () => void and call onDismiss?.() from handleDismiss after
persisting the dismissed flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
ee/features/conversations/pages/conversation-overview.tsx (1)
131-139
: Fix optional-chaining bugs in search filter (runtime TypeErrors).Calling .includes on potentially undefined strings will throw. Chain through includes as well, and guard the document path.
- if (conversation.viewerEmail?.toLowerCase().includes(query)) return true; + if (conversation.viewerEmail?.toLowerCase()?.includes(query)) return true; ... - return ( - conversation.title?.toLowerCase().includes(query) || - conversation.lastMessage?.content.toLowerCase().includes(query) || - conversation.dataroomDocument?.document.name.toLowerCase().includes(query) - ); + return ( + conversation.title?.toLowerCase()?.includes(query) || + conversation.lastMessage?.content.toLowerCase().includes(query) || + conversation.dataroomDocument?.document?.name + ?.toLowerCase() + ?.includes(query) + );
♻️ Duplicate comments (2)
ee/features/conversations/pages/conversation-overview.tsx (2)
112-116
: Endpoint pluralization looks correct now (“/faqs”).Matches the team-scoped API route; good catch and fix.
4-4
: Make banner dismissal SSR-safe and actually dismissible; also gate when conversations are enabled.Replace the localStorage useMemo with state + effect via the project’s safe webstorage helper, and don’t show the banner if conversations are already enabled. Optionally wire an onDismiss callback if the banner supports it.
- import { useEffect, useMemo, useState } from "react"; + import { useEffect, useState } from "react"; + import { localStorage as safeLocalStorage } from "@/lib/webstorage";- // Memoize banner dismissed state to avoid localStorage reads on every render - const isBannerDismissed = useMemo(() => { - if (!dataroom?.id) return false; - return ( - localStorage.getItem( - `dataroom-${dataroom.id}-conversations-banner-dismissed`, - ) === "true" - ); - }, [dataroom?.id]); + const [isBannerDismissed, setIsBannerDismissed] = useState(false); + useEffect(() => { + if (!dataroom?.id) return; + setIsBannerDismissed( + safeLocalStorage.getItem( + `dataroom-${dataroom.id}-conversations-banner-dismissed`, + ) === "true", + ); + }, [dataroom?.id]);- {/* Show banner unless it's been dismissed */} - {!isBannerDismissed && ( + {/* Show banner only if not dismissed and conversations are disabled */} + {!isBannerDismissed && !isConversationsEnabled && ( <ConversationsNotEnabledBanner dataroomId={dataroom.id} teamId={teamId as string} isConversationsEnabled={isConversationsEnabled} onConversationsToggled={handleConversationsToggled} + // If supported by the component: + onDismiss={() => setIsBannerDismissed(true)} /> )}Also applies to: 77-86, 207-215
🧹 Nitpick comments (4)
ee/features/conversations/pages/conversation-overview.tsx (4)
189-193
: Avoid router.push during render; redirect in an effect and return null.Prevents side-effects in render and double renders.
- if (!limits?.conversationsInDataroom) { - // Redirect to documents page if conversations are not enabled - router.push(`/datarooms/${dataroom?.id}/documents`); - } + useEffect(() => { + if (!limits?.conversationsInDataroom && dataroom?.id) { + router.replace(`/datarooms/${dataroom.id}/documents`); + } + }, [limits?.conversationsInDataroom, dataroom?.id, router]);Optionally add:
+ if (!limits?.conversationsInDataroom) return null;
94-111
: Revalidate conversations list after delete (SWR mutate).Without a mutate, the deleted item may remain until a focus/revalidate.
- const { data: conversations = [], isLoading: isLoadingConversations } = + const { + data: conversations = [], + isLoading: isLoadingConversations, + mutate: mutateConversations, + } = useSWR<ConversationSummary[]>(toast.success("Conversation deleted successfully"); + await mutateConversations();
Also applies to: 145-173
118-122
: Add onError handler to FAQs SWR for parity and UX.Surface failures like conversations do.
{ revalidateOnFocus: true, dedupingInterval: 10000, keepPreviousData: true, + onError: (err) => { + console.error("Error fetching FAQs:", err); + toast.error("Failed to load FAQs"); + }, },
217-241
: Minor: Tab/link mismatch can make activeTab misleading.Clicking “Published FAQs” navigates away, so activeTab may show “conversations” until route changes. Either derive the tab value from the route or drop activeTab for the link trigger.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ee/features/conversations/pages/conversation-overview.tsx
(6 hunks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ee/features/conversations/pages/conversation-overview.tsx (2)
192-196
: Redirect during render and premature redirect when limits is undefinedThis executes on initial render (and when limits is undefined), causing unintended navigation. Move to effect and gate on resolved limits.
-if (!limits?.conversationsInDataroom) { - // Redirect to documents page if conversations are not enabled - router.push(`/datarooms/${dataroom?.id}/documents`); -} +useEffect(() => { + if (!dataroom?.id) return; + if (limits && limits.conversationsInDataroom === false) { + router.replace(`/datarooms/${dataroom.id}/documents`); + } +}, [limits?.conversationsInDataroom, dataroom?.id, router]);
210-218
: Extend banner component and update rendering logic
- In ee/features/conversations/components/dashboard/conversations-not-enabled-banner.tsx, add
onDismiss?: () => void
toConversationsNotEnabledBannerProps
and invoke it when the user dismisses the banner.- In ee/features/conversations/pages/conversation-overview.tsx, change the render condition to
{!isBannerDismissed && !isConversationsEnabled && (…)}
and passonDismiss={() => setIsBannerDismissed(true)}
to<ConversationsNotEnabledBanner />
.
♻️ Duplicate comments (3)
ee/features/conversations/pages/conversation-overview.tsx (1)
80-89
: Banner dismissal never updates; switch to state + effect and gate by feature flagMemoized read won’t re-run after child persists dismissal. Also only show when conversations are disabled.
-// Memoize banner dismissed state to avoid localStorage reads on every render -const isBannerDismissed = useMemo(() => { - if (!dataroom?.id) return false; - return ( - safeLocalStorage.getItem( - `dataroom-${dataroom.id}-conversations-banner-dismissed`, - ) === "true" - ); -}, [dataroom?.id]); +const [isBannerDismissed, setIsBannerDismissed] = useState(false); +useEffect(() => { + if (!dataroom?.id) return; + setIsBannerDismissed( + safeLocalStorage.getItem( + `dataroom-${dataroom.id}-conversations-banner-dismissed`, + ) === "true", + ); +}, [dataroom?.id]);ee/features/conversations/api/team-faqs-route.ts (2)
59-71
: Prisma: findUnique with relational filters is invalid; use findFirstfindUnique only accepts unique fields; relational team filters will be ignored/fail. Use findFirst with the same where.
- const dataroom = await prisma.dataroom.findUnique({ + const dataroom = await prisma.dataroom.findFirst({ where: { id: dataroomId, team: { id: teamId, users: { some: { userId } }, }, }, select: { id: true, teamId: true, }, });
225-236
: Repeat: findUnique → findFirst with relational filters in GETSame issue as POST.
- const dataroom = await prisma.dataroom.findUnique({ + const dataroom = await prisma.dataroom.findFirst({ where: { id: dataroomId, team: { id: teamId, users: { some: { userId } }, }, }, select: { id: true, }, });
🧹 Nitpick comments (4)
ee/features/conversations/lib/schemas/faq.ts (2)
57-60
: Form-level safeguard: require sourceConversationId when message IDs are presentPrevents dangling/foreign message refs from the form.
-export const publishFAQFormSchema = baseFAQSchema.extend({ - questionMessageId: z.string().cuid("Invalid question message ID"), - answerMessageId: z.string().cuid("Invalid answer message ID"), -}); +export const publishFAQFormSchema = baseFAQSchema + .extend({ + questionMessageId: z.string().cuid("Invalid question message ID"), + answerMessageId: z.string().cuid("Invalid answer message ID"), + sourceConversationId: z.string().cuid("Invalid conversation ID"), + });
65-69
: Harden Next.js param parsing (string | string[])Handle array params gracefully to avoid 400s when Next.js supplies arrays.
-export const faqParamSchema = z.object({ - teamId: z.string().cuid("Invalid team ID format"), - id: z.string().cuid("Invalid dataroom ID format"), - faqId: z.string().cuid("Invalid FAQ ID format").optional(), -}); +export const faqParamSchema = z.object({ + teamId: z.preprocess((v) => (Array.isArray(v) ? v[0] : v), z.string().cuid("Invalid team ID format")), + id: z.preprocess((v) => (Array.isArray(v) ? v[0] : v), z.string().cuid("Invalid dataroom ID format")), + faqId: z + .preprocess((v) => (Array.isArray(v) ? v[0] : v), z.string().cuid("Invalid FAQ ID format")) + .optional(), +});ee/features/conversations/pages/conversation-overview.tsx (1)
4-4
: Drop unused useMemo importNot needed after switching to effect/state for banner.
-import { useEffect, useMemo, useState } from "react"; +import { useEffect, useState } from "react";ee/features/conversations/api/team-faqs-route.ts (1)
174-191
: Return a consistent shape from POST (include message refs like GET)Keeps client types consistent between create and list.
include: { dataroom: { select: { name: true }, }, link: { select: { name: true }, }, dataroomDocument: { include: { document: { select: { name: true }, }, }, }, publishedByUser: { select: { name: true, email: true }, }, + sourceConversation: { + select: { id: true }, + }, + questionMessage: { + select: { id: true, content: true }, + }, + answerMessage: { + select: { id: true, content: true }, + }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
ee/features/conversations/api/team-faqs-route.ts
(1 hunks)ee/features/conversations/components/dashboard/edit-faq-modal.tsx
(1 hunks)ee/features/conversations/components/dashboard/publish-faq-modal.tsx
(1 hunks)ee/features/conversations/lib/schemas/faq.ts
(1 hunks)ee/features/conversations/pages/conversation-overview.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- ee/features/conversations/components/dashboard/edit-faq-modal.tsx
- ee/features/conversations/components/dashboard/publish-faq-modal.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
ee/features/conversations/pages/conversation-overview.tsx (5)
context/team-context.tsx (1)
useTeam
(85-85)ee/features/conversations/pages/faq-overview.tsx (1)
PublishedFAQ
(67-102)lib/utils.ts (1)
fetcher
(48-62)components/conversations/index.tsx (1)
ConversationListItem
(6-8)ee/features/conversations/components/dashboard/conversation-list-item.tsx (1)
ConversationListItem
(7-80)
ee/features/conversations/api/team-faqs-route.ts (6)
pages/api/auth/[...nextauth].ts (1)
authOptions
(35-204)ee/features/conversations/lib/schemas/faq.ts (3)
faqParamSchema
(65-69)publishFAQSchema
(17-36)updateFAQSchema
(39-54)lib/types.ts (1)
CustomUser
(17-17)lib/utils/sanitize-html.ts (1)
validateContent
(3-17)pages/api/teams/[teamId]/datarooms/[id]/faqs/[faqId].ts (1)
handler
(5-10)pages/api/teams/[teamId]/datarooms/[id]/faqs/index.ts (1)
handler
(5-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
// Validate visibility mode and related fields | ||
if (data.visibilityMode === "PUBLIC_LINK" && !data.linkId) { | ||
return res.status(400).json({ | ||
error: "Link ID is required for link visibility", | ||
}); | ||
} | ||
|
||
if ( | ||
data.visibilityMode === "PUBLIC_DOCUMENT" && | ||
!data.dataroomDocumentId | ||
) { | ||
return res.status(400).json({ | ||
error: "Document ID is required for document visibility", | ||
}); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate page/version fields require a document
Prevent inconsistent records like page/version without a document.
// Validate visibility mode and related fields
if (data.visibilityMode === "PUBLIC_LINK" && !data.linkId) {
return res.status(400).json({
error: "Link ID is required for link visibility",
});
}
if (
data.visibilityMode === "PUBLIC_DOCUMENT" &&
!data.dataroomDocumentId
) {
return res.status(400).json({
error: "Document ID is required for document visibility",
});
}
+ if ((data.documentPageNumber != null || data.documentVersionNumber != null) && !data.dataroomDocumentId) {
+ return res.status(400).json({
+ error: "Document page/version requires a document",
+ });
+ }
📝 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.
// Validate visibility mode and related fields | |
if (data.visibilityMode === "PUBLIC_LINK" && !data.linkId) { | |
return res.status(400).json({ | |
error: "Link ID is required for link visibility", | |
}); | |
} | |
if ( | |
data.visibilityMode === "PUBLIC_DOCUMENT" && | |
!data.dataroomDocumentId | |
) { | |
return res.status(400).json({ | |
error: "Document ID is required for document visibility", | |
}); | |
} | |
// Validate visibility mode and related fields | |
if (data.visibilityMode === "PUBLIC_LINK" && !data.linkId) { | |
return res.status(400).json({ | |
error: "Link ID is required for link visibility", | |
}); | |
} | |
if ( | |
data.visibilityMode === "PUBLIC_DOCUMENT" && | |
!data.dataroomDocumentId | |
) { | |
return res.status(400).json({ | |
error: "Document ID is required for document visibility", | |
}); | |
} | |
if ( | |
(data.documentPageNumber != null || data.documentVersionNumber != null) && | |
!data.dataroomDocumentId | |
) { | |
return res.status(400).json({ | |
error: "Document page/version requires a document", | |
}); | |
} |
🤖 Prompt for AI Agents
In ee/features/conversations/api/team-faqs-route.ts around lines 77 to 92, add
validation to ensure that any provided page or version fields require a
dataroomDocumentId; specifically, if data.pageId or data.pageVersion (or both)
are present but data.dataroomDocumentId is missing, return a 400 response with
an error message like "Document ID is required when page or version is
specified". Implement the check alongside the existing visibility validations so
inconsistent records (page/version without a document) are rejected before
proceeding.
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
package.json (2)
23-27
: Align AWS SDK v3 packages to the same version to avoid duplicate transitive trees.Most AWS v3 clients are at ^3.879.0 but cloudfront-signer is ^3.873.0. Keeping them aligned reduces duplication and surprises during tree-shaking.
Apply:
- "@aws-sdk/cloudfront-signer": "^3.873.0", + "@aws-sdk/cloudfront-signer": "^3.879.0",
34-34
: No CSP changes needed:connect-src 'self' https:
already allows Jitsu/PostHog endpoints
Verify bundle size impact before merging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (3)
ee/features/conversations/api/team-faqs-route.ts
(1 hunks)ee/features/conversations/lib/schemas/faq.ts
(1 hunks)package.json
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- ee/features/conversations/lib/schemas/faq.ts
- ee/features/conversations/api/team-faqs-route.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
package.json (4)
90-90
: LGTM on Next + ESLint + TS ranges.Patch bumps look safe and consistent with the current Node engine (>=18.18.0). No action needed.
Also applies to: 102-102, 157-165
96-96
: Verify lucide-react icon imports: lucide-react may rename icons even in patch bumps—manually confirm all imported icon names in your .ts/.tsx files still exist to prevent undefined exports at runtime.
36-36
: Peer dependency compatibility confirmed: @next/[email protected] requires next^13.0.0 || ^14.0.0 || ^15.0.0
, covering Next 14.2.32; dry-run produced no peer warnings.
129-129
: No bundling/SSR regressions in Shiki 3.12.0
v3.12.0 changelog lists only vitepress-twoslash UX improvements and a remove-notation-escape bug fix—no changes to createHighlighter, codeToHtml, ESM or WASM resolution (newreleases.io)
Summary by CodeRabbit
New Features
UI
Chores