-
Notifications
You must be signed in to change notification settings - Fork 0
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
- Forward Update #999
base: main
Are you sure you want to change the base?
- Forward Update #999
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a message forwarding feature that spans both server and client components. A new mutation, forwardMessage, is added to handle message forwarding with authentication and chat membership checks. On the client side, new components and state handling (ForwardDialog and ForwardHandler) enable users to select recipients and submit forwarding actions. Additionally, a Chats type is now exported for external use, and a new Checkbox component is added for UI interactions. Package dependencies and devDependencies are updated, with several packages being removed or upgraded. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as Message Component
participant D as ForwardDialog
participant S as Convex Server
U->>M: Initiate forward action
M->>D: Set forwardedMessageId & display dialog
U->>D: Select recipients & submit forwarding
D->>S: Call forwardMessage mutation with forwardObjects & messageId
S-->>D: Return response (success/error)
D-->>M: Notify update to UI
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…essages to the my notes chats
Things that need to be done before merge:
|
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: 3
♻️ Duplicate comments (2)
convex/messages.ts (2)
289-289
:⚠️ Potential issueChange type from string to Id<"messages">.
As per PR objectives and past review comments, the type for
messageId
should be changed fromstring
toId<"messages">
.Apply this diff:
- messageId: v.string(), + messageId: v.id("messages"),
295-297
: 🛠️ Refactor suggestionRemove console.log in production.
Console.log statements should not be present in production code.
Apply this diff:
- console.error("Unauthenticated call to mutation"); - return null; + throw new ConvexError("Unauthenticated call to mutation");
🧹 Nitpick comments (6)
src/components/forwardMessage.tsx (3)
73-73
: Fix typo in dialog title.There's a typo in the dialog title: "Fordward" should be "Forward".
Apply this diff:
- <p className="mt-1">Fordward</p>{" "} + <p className="mt-1">Forward</p>{" "}
84-86
: Enhance loading state UI.The loading state could be improved to provide better user feedback.
Consider applying this diff for a more polished loading state:
- <LoaderCircle className="mr-1.5 animate-spin justify-center p-0.5" /> + <div className="flex flex-col items-center justify-center space-y-2"> + <LoaderCircle className="h-8 w-8 animate-spin" /> + <p className="text-sm text-muted-foreground">Loading chats...</p> + </div>
128-133
: Simplify conditional className.The conditional className logic can be simplified.
Apply this diff:
- className={cn( - "mt-4 flex cursor-pointer rounded-xl bg-secondary p-5", - user.username == userInfos[0]?.username - ? "h-0 p-0" - : null, - )} + className={cn("mt-4 flex cursor-pointer rounded-xl bg-secondary p-5", { + "h-0 p-0": user.username == userInfos[0]?.username + })}convex/messages.ts (1)
314-324
: Remove redundant message existence check.The message existence check is redundant since
getX
will throw if the message doesn't exist.Apply this diff:
- const parsedMessageId = ctx.table("messages").normalizeId(args.messageId); - - if (!parsedMessageId) { - throw new ConvexError("messageId was invalid"); - } - - const message = await ctx.table("messages").getX(parsedMessageId); - - if (!message) { - throw new ConvexError("Message does not exist"); - } + const message = await ctx.table("messages").getX(args.messageId);src/components/message.tsx (2)
275-279
: Follow React's naming convention for state variables.The state variable and setter function should follow camelCase naming convention:
-const [ForwardedMessageId, setForwardedMessageId] = useQueryState( +const [forwardedMessageId, setForwardedMessageId] = useQueryState(
281-283
: Follow React's naming convention for event handlers.The handler function should follow camelCase naming convention:
-const ForwardHandler = () => { +const handleForward = () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
convex/messages.ts
(1 hunks)package.json
(1 hunks)src/components/chat-overview.tsx
(1 hunks)src/components/forwardMessage.tsx
(1 hunks)src/components/message.tsx
(6 hunks)src/components/ui/checkbox.tsx
(1 hunks)
🔇 Additional comments (3)
src/components/ui/checkbox.tsx (1)
1-29
: LGTM! Well-structured Checkbox component.The component is well-implemented with proper accessibility support through Radix UI, correct usage of forward ref pattern, and clean styling implementation.
src/components/chat-overview.tsx (1)
16-16
: LGTM! Appropriate type export.The Chats type is now correctly exported for use in other components.
package.json (1)
18-92
: 🌐 Web query:What is the latest version of Next.js?
💡 Result:
The latest stable version of Next.js as of February 2025 is **Next.js 15**, with its most recent patch being **15.1.6** released on January 22, 2025[1][3]. This version requires React 19 and introduces several improvements including core upgrades, new APIs, and enhanced developer experience[2][3]. Key details about current releases: - **Stable channel**: v15.x (security-supported) - **Latest patch**: v15.1.6 - **Previous supported version**: v14.2.23 (security updates until January 2025)[1] The development team has also been working on Next.js 15.2 in canary releases (pre-release versions), with multiple test builds like v15.2.0-canary.47 published in February 2025[4]. However, these canary releases are not recommended for production use[1]. To check your current version: ```bash npx next --version ``` To upgrade an existing project: ```bash npm i next@latest react@latest react-dom@latest eslint-config-next@latest ``` Citations: - 1: https://endoflife.date/nextjs - 2: https://nextjs.org/blog - 3: https://nextjs.org/docs/app/building-your-application/upgrading/version-15 - 4: https://github.com/vercel/next.js/releases - 5: https://nextjs.org - 6: https://www.npmjs.com/package/next
🌐 Web query:
What is the latest version of React?
💡 Result:
The latest version of React is **19.0**, released on December 5, 2024[1][2][4][7]. This major update introduces significant features like: - **Actions**: Simplified form handling with automatic state management. - **New hooks**: `useActionState`, `useFormStatus`, and `useOptimistic` for enhanced state control. - **Server Components**: Native support for server-rendered components with async capabilities[2][7]. - **Improved hydration**: Better error reporting and reduced client-server mismatch errors[2]. - **Ref handling**: Can now pass refs as normal props instead of using `forwardRef`[2]. Key development recommendations: - Upgrade path: First migrate to React 18.3.x to identify deprecated API usage before moving to v19[2]. - Concurrent features: Enabled by default in the new root API (`ReactDOM.createRoot`)[6]. - Support status: Active security updates and non-critical bug fixes are ongoing for v19[4]. Previous major versions like React 18 (March 2022) introduced concurrent rendering and automatic batching[3][5], while React 17 (October 2020) focused on gradual upgrade capabilities[3][4]. The npm package `react@latest` currently points to v19.0.0[10]. Citations: - 1: https://react.dev/versions - 2: https://github.com/facebook/react/releases - 3: https://en.wikipedia.org/wiki/React_(JavaScript_library) - 4: https://endoflife.date/react - 5: https://www.scalablepath.com/react/react-18-release-features - 6: https://www.w3schools.com/react/react_upgrade.asp - 7: https://react.dev/blog/2024/12/05/react-19 - 8: https://www.simplilearn.com/tutorials/reactjs-tutorial/react-eighteen-new-features - 9: https://legacy.reactjs.org/versions - 10: https://www.npmjs.com/package/react
interface ForwardDialogProps { | ||
ForwardedMessageId: string; |
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.
Change type from string to Id<"messages">.
As per PR objectives, the type for ForwardedMessageId
should be changed from string
to Id<"messages">
.
Apply this diff:
- ForwardedMessageId: string;
+ ForwardedMessageId: Id<"messages">;
📝 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.
interface ForwardDialogProps { | |
ForwardedMessageId: string; | |
interface ForwardDialogProps { | |
ForwardedMessageId: Id<"messages">; |
<div | ||
onClick={() => { | ||
ForwardHandler(); | ||
}} | ||
className="flex w-full cursor-pointer border-b-2 border-secondary-foreground p-2 pr-8" | ||
> | ||
<Forward /> | ||
<p className="ml-1">Forward</p> | ||
</div> | ||
<ForwardDialog | ||
ForwardedMessageId={ForwardedMessageId} | ||
setForwardedMessageId={setForwardedMessageId} | ||
chats={existingChats} | ||
userInfos={userInfos} | ||
/> |
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
Use a button element for better accessibility.
Replace the div
with a button
element to improve accessibility:
-<div
+<button
onClick={() => {
- ForwardHandler();
+ handleForward();
}}
className="flex w-full cursor-pointer border-b-2 border-secondary-foreground p-2 pr-8"
->
+>
<Forward />
<p className="ml-1">Forward</p>
-</div>
+</button>
Committable suggestion skipped: line range outside the PR's diff.
<div | ||
className="flex w-full cursor-pointer border-t-2 border-secondary-foreground p-2 pr-8" | ||
onClick={() => { | ||
ForwardHandler(); | ||
}} | ||
> | ||
<Forward /> | ||
<p className="ml-1">Forward</p> | ||
</div> | ||
<ForwardDialog | ||
ForwardedMessageId={ForwardedMessageId} | ||
setForwardedMessageId={setForwardedMessageId} | ||
chats={existingChats} | ||
userInfos={userInfos} | ||
/> |
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
Use a button element for better accessibility.
Replace the div
with a button
element to improve accessibility:
-<div
+<button
className="flex w-full cursor-pointer border-t-2 border-secondary-foreground p-2 pr-8"
onClick={() => {
- ForwardHandler();
+ handleForward();
}}
->
+>
<Forward />
<p className="ml-1">Forward</p>
-</div>
+</button>
📝 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.
<div | |
className="flex w-full cursor-pointer border-t-2 border-secondary-foreground p-2 pr-8" | |
onClick={() => { | |
ForwardHandler(); | |
}} | |
> | |
<Forward /> | |
<p className="ml-1">Forward</p> | |
</div> | |
<ForwardDialog | |
ForwardedMessageId={ForwardedMessageId} | |
setForwardedMessageId={setForwardedMessageId} | |
chats={existingChats} | |
userInfos={userInfos} | |
/> | |
<button | |
className="flex w-full cursor-pointer border-t-2 border-secondary-foreground p-2 pr-8" | |
onClick={() => { | |
handleForward(); | |
}} | |
> | |
<Forward /> | |
<p className="ml-1">Forward</p> | |
</button> | |
<ForwardDialog | |
ForwardedMessageId={ForwardedMessageId} | |
setForwardedMessageId={setForwardedMessageId} | |
chats={existingChats} | |
userInfos={userInfos} | |
/> |
Summary by CodeRabbit
New Features
Chores