-
Notifications
You must be signed in to change notification settings - Fork 51
Revert PR #83: Chat Sharing Feature #85
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
- Add share_id and share_date fields to chats schema with index - Implement shareChat, updateShareDate, and unshareChat mutations - Add getSharedChat and getUserSharedChats queries - Implement frozen content filtering in getSharedMessages - Strip sensitive data (user_id, file URLs) from shared messages - Allow public access to /share routes in middleware
- Create ShareDialog with create/update/unshare functionality - Add SharedLinksTab for managing all shared chats - Build public share view with frozen content display - Add UUID validation for share links - Replace file/image placeholders with "Uploaded a file" text - Match shared conversation UI to main chat interface - Add date-fns dependency for relative time formatting
- Add share button to ChatHeader with tooltip - Add share option to ChatItem dropdown menu - Pass share_id and share_date through component hierarchy - Enable ShareDialog to recognize already-shared chats
- Fix ShareDialog state persistence after unsharing - Reset all loading states when dialog opens - Clear shareUrl and shareDate when chat is not shared - Properly reset isUnsharing, isUpdating, isGenerating states - Relax UUID validation regex to accept all UUID versions - Changed from strict v4 validation to general UUID format - Fixes "Invalid share link" error for valid share IDs
- Remove unnecessary 'as any' type casts in ChatHeader for better type safety - Fix race condition in shareChat mutation by re-fetching persisted values - Fix linting errors: escape apostrophes, use Link component, extract complex dependency
This reverts commit 550fe8e.
… issues" This reverts commit f92b460.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR removes the chat-sharing feature across frontend, backend, schema, and routing, and changes the Stripe client apiVersion from "2025-10-29.clover" to "2025-09-30.clover". Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Backend
participant DB
rect `#E8F3FF`
Note over User,Frontend: Old flow (with sharing)
User->>Frontend: Click "Share"
Frontend->>Backend: mutation shareChat(...)
Backend->>DB: insert share record (share_id, share_date)
Backend-->>Frontend: share_id / share_url
Frontend->>User: show ShareDialog with URL
end
rect `#FFF4E6`
Note over User,Frontend: New flow (sharing removed)
User->>Frontend: Click "Share" (button removed)
Frontend-->>User: No share UI / option
end
sequenceDiagram
participant Public
participant Middleware
participant App
rect `#E8F8F0`
Note over Public,App: Old public access to /share/:id
Public->>App: GET /share/{id}
App->>Backend: query getSharedChat / getSharedMessages (public)
Backend->>App: shared chat data
App->>Public: render shared view
end
rect `#FFF0F0`
Note over Public,App: New behavior (protected)
Public->>Middleware: GET /share/{id}
Middleware->>Public: require authentication / block
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/page.tsx (1)
89-110: The useMemo dependency may not trigger updates when URL parameters change.The dependency
typeof window !== "undefined" ? window.location.search : ""has two issues:
- The ternary expression itself is re-evaluated on every render, potentially causing the dependency check to behave unexpectedly
window.location.searchis not a reactive value tracked by React—URL changes won't automatically re-trigger this memoIf you need the memo to recompute when URL parameters change, use Next.js's
useSearchParams()hook:+"use client"; + import React from "react"; +import { useSearchParams } from "next/navigation"; import { Authenticated, Unauthenticated } from "convex/react"; // ... other imports export default function Page() { // ... other code + const searchParams = useSearchParams(); const { initialSeats, initialPlan } = React.useMemo(() => { - if (typeof window === "undefined") { - return { initialSeats: 5, initialPlan: "monthly" as const }; - } - const urlParams = new URLSearchParams(window.location.search); + const urlSeats = searchParams.get("numSeats"); + const urlPlan = searchParams.get("selectedPlan"); - const urlSeats = urlParams.get("numSeats"); - const urlPlan = urlParams.get("selectedPlan"); let seats = 5; if (urlSeats) { const parsed = parseInt(urlSeats, 10); if (!isNaN(parsed) && parsed >= 1) { seats = parsed; } } const plan = (urlPlan === "yearly" ? "yearly" : "monthly") as | "monthly" | "yearly"; return { initialSeats: seats, initialPlan: plan }; - }, [typeof window !== "undefined" ? window.location.search : ""]); + }, [searchParams]);If this is intentional (to read URL params only once on mount), clarify with a comment explaining the one-time read behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
app/api/stripe.ts(1 hunks)app/components/ChatHeader.tsx(5 hunks)app/components/ChatItem.tsx(1 hunks)app/components/DataControlsTab.tsx(0 hunks)app/components/ShareDialog.tsx(0 hunks)app/components/SharedLinksTab.tsx(0 hunks)app/components/SidebarHistory.tsx(0 hunks)app/page.tsx(2 hunks)app/share/[shareId]/SharedChatView.tsx(0 hunks)app/share/[shareId]/SharedMessages.tsx(0 hunks)app/share/[shareId]/page.tsx(0 hunks)convex/chats.ts(0 hunks)convex/messages.ts(0 hunks)convex/schema.ts(0 hunks)middleware.ts(0 hunks)
💤 Files with no reviewable changes (11)
- middleware.ts
- convex/messages.ts
- app/components/ShareDialog.tsx
- app/components/DataControlsTab.tsx
- convex/schema.ts
- app/components/SharedLinksTab.tsx
- app/share/[shareId]/SharedChatView.tsx
- convex/chats.ts
- app/share/[shareId]/SharedMessages.tsx
- app/components/SidebarHistory.tsx
- app/share/[shareId]/page.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from ./_generated/dataModel for document IDs (e.g., Id<'users'>) instead of string
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) -> Record<Id<'users'>, string>)
Use as const on string literals in discriminated unions
When using Array type, explicitly declare arrays with const array: Array = [...]
When using Record type, explicitly declare with const record: Record<KeyType, ValueType> = {...}
Files:
app/api/stripe.tsapp/components/ChatItem.tsxapp/components/ChatHeader.tsxapp/page.tsx
🪛 GitHub Actions: Run Tests
app/api/stripe.ts
[error] 4-4: Type '"2025-09-30.clover"' is not assignable to type '"2025-10-29.clover"'.
🪛 GitHub Check: test (20.x)
app/api/stripe.ts
[failure] 4-4:
Type '"2025-09-30.clover"' is not assignable to type '"2025-10-29.clover"'.
🔇 Additional comments (5)
app/components/ChatHeader.tsx (3)
3-3: LGTM! Clean removal of share-related imports.The removal of ShareDialog and Share2 icon imports correctly reflects the revert of the sharing feature.
Also applies to: 6-6
23-26: LGTM! Type definition correctly narrowed.The chatData type properly removes share_id and share_date fields, aligning with the feature revert.
267-267: LGTM! Render logic correctly updated.The removal of ShareDialog from the render tree is appropriate for this revert.
Also applies to: 339-339
app/components/ChatItem.tsx (2)
29-29: LGTM! Share icon import correctly removed.The removal of the Share2 icon import aligns with the revert of the sharing feature.
34-39: LGTM! Props interface correctly cleaned up.The removal of
shareIdandshareDateprops fromChatItemPropsis appropriate for this revert, leaving only the essential chat item properties.
|
|
||
| const stripe = new Stripe(process.env.STRIPE_SECRET_KEY as string, { | ||
| apiVersion: "2025-10-29.clover", | ||
| apiVersion: "2025-09-30.clover", |
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.
🧩 Analysis chain
Critical: TypeScript error indicates incomplete revert of Stripe package.
The pipeline is failing because the Stripe types expect "2025-10-29.clover" but the code uses "2025-09-30.clover". This suggests that while the API version string is being reverted, the Stripe npm package itself may not have been reverted to its pre-PR-#83 version.
Run the following script to verify whether the Stripe package version needs to be reverted:
If PR #83 updated the Stripe package, you'll need to revert that change in package.json as well and run npm install (or your package manager equivalent) to update the lock file.
🏁 Script executed:
#!/bin/bash
# Check the current Stripe package version and what versions are available
echo "Current Stripe package version:"
cat package.json | jq '.dependencies.stripe // .devDependencies.stripe'
echo -e "\nChecking git history for Stripe package changes in PR #83:"
git log --all --oneline --grep="issue-79" -- package.json
echo -e "\nShowing Stripe package changes in the commits being reverted:"
git show 324116c:package.json | jq '.dependencies.stripe // .devDependencies.stripe' 2>/dev/null || echo "Not found in 324116c"
git show 550fe8e:package.json | jq '.dependencies.stripe // .devDependencies.stripe' 2>/dev/null || echo "Not found in 550fe8e"
git show fb9c1e4:package.json | jq '.dependencies.stripe // .devDependencies.stripe' 2>/dev/null || echo "Not found in fb9c1e4"
git show f92b460:package.json | jq '.dependencies.stripe // .devDependencies.stripe' 2>/dev/null || echo "Not found in f92b460"
git show 7671911:package.json | jq '.dependencies.stripe // .devDependencies.stripe' 2>/dev/null || echo "Not found in 7671911"Length of output: 214
Critical: Revert of Stripe package is incomplete—package.json still has the upgraded version.
PR #83 upgraded Stripe from ^19.1.0 to ^19.3.0, which introduced support for the "2025-10-29.clover" API version. While this PR reverts the API version string in app/api/stripe.ts to "2025-09-30.clover", the package.json still references Stripe ^19.3.0. This version mismatch causes the TypeScript error because the Stripe types no longer recognize the older API version.
Fix: Revert package.json to use Stripe ^19.1.0 and run npm install to update the lock file.
🧰 Tools
🪛 GitHub Actions: Run Tests
[error] 4-4: Type '"2025-09-30.clover"' is not assignable to type '"2025-10-29.clover"'.
🪛 GitHub Check: test (20.x)
[failure] 4-4:
Type '"2025-09-30.clover"' is not assignable to type '"2025-10-29.clover"'.
🤖 Prompt for AI Agents
In app/api/stripe.ts around line 4 the API version was reverted to
"2025-09-30.clover" but package.json still pins Stripe to ^19.3.0 which causes a
types mismatch; update package.json to revert the Stripe dependency to ^19.1.0
(matching the older API), then run npm install (or npm ci) to update the
lockfile and node_modules so TypeScript uses the correct Stripe types; verify
build/typecheck succeeds and commit the updated package.json and lockfile.
|
|
||
| // Check if this is a branched chat | ||
| const isBranchedChat = !!chatData?.branched_from_chat_id; | ||
| const isBranchedChat = !!(chatData as any)?.branched_from_chat_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.
🛠️ Refactor suggestion | 🟠 Major
Remove unnecessary type cast to any.
The type cast (chatData as any) is unnecessary and undermines type safety. The chatData type at lines 23-26 already includes branched_from_chat_id?: string, so you can access it directly.
Apply this diff:
- const isBranchedChat = !!(chatData as any)?.branched_from_chat_id;
+ const isBranchedChat = !!chatData?.branched_from_chat_id;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isBranchedChat = !!(chatData as any)?.branched_from_chat_id; | |
| const isBranchedChat = !!chatData?.branched_from_chat_id; |
🤖 Prompt for AI Agents
In app/components/ChatHeader.tsx around line 66, remove the unnecessary
(chatData as any) cast and access the optional property directly; replace the
expression with a safe optional-chain check like chatData?.branched_from_chat_id
(e.g. const isBranchedChat = !!chatData?.branched_from_chat_id) so you preserve
type safety and still coerce to boolean.
- Delete package-lock.json (using pnpm-lock.yaml instead) - Fix React Hook exhaustive-deps warning in app/page.tsx
Emergency Revert
This PR reverts the chat sharing feature that was accidentally merged in PR #83.
Reverted Commits
This PR reverts the following commits:
7671911- fix(issue-79): address PR review comments and linting issuesf92b460- fix(issue-79): resolve share dialog state and UUID validation issuesfb9c1e4- feat(issue-79): integrate sharing into chat interface550fe8e- feat(issue-79): add chat sharing UI components324116c- feat(issue-79): add backend support for chat sharingChanges
Summary by CodeRabbit