-
Notifications
You must be signed in to change notification settings - Fork 90
Conversation Sharing #214
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
base: main
Are you sure you want to change the base?
Conversation Sharing #214
Conversation
WalkthroughThis pull request introduces a conversation sharing feature to the chat interface. A new share button in the Chat component triggers an asynchronous function that validates conversation data, sends it to a new API endpoint, and generates a shareable URL copied to the clipboard. A dedicated share page displays shared conversations by their share ID. Additionally, new TypeScript interfaces, UI components (for chat messages, markdown rendering, and icons), utility functions, and database schema changes have been implemented along with added dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Chat Component
participant A as /api/share Endpoint
participant CB as Clipboard
participant AL as Alert (Swal.fire)
U->>C: Clicks share button
C->>C: Validates conversationId and messages
C->>A: Sends POST request with conversation data
A-->>C: Returns share ID or error response
C->>C: Generates shareable URL
C->>CB: Copies URL to clipboard
C->>AL: Displays success (or error) alert
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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
🧹 Nitpick comments (9)
app/types.ts (2)
1-7: Consider addressing potential redundancy in Message interfaceThe
Messageinterface has both aroleproperty (which can be 'user' or 'assistant') and anisUserboolean property. These serve similar purposes and could potentially become inconsistent if not managed carefully.Consider either:
- Removing the
isUserproperty and deriving it fromrole === 'user'when needed- Making
isUsera computed property based onrole- Adding validation to ensure consistency between these properties
This will help prevent maintenance issues and potential bugs in the future.
14-25: Consider simplifying the SharedConversation content structureThe
contentproperty uses a nested structure (content: { messages: Message[] }) which adds an unnecessary level of nesting. This could be simplified to justmessages: Message[]directly at the root level for cleaner data access.Additionally, note that date fields (
created_at,expires_at) are typed as strings rather than Date objects. This is common for serialized data but might require parsing before use in date calculations.utils/supabase/actions.ts (1)
13-28: Good development bypass for Stripe integrationThis addition provides a convenient way to bypass Stripe subscription logic during development, which is a good practice. The implementation returns a complete dummy subscription object with all the necessary fields.
Consider extracting the date calculation into a named constant for better readability:
+ const ONE_YEAR_MS = 365 * 24 * 60 * 60 * 1000; return { id: 'dev_subscription', user_id: userId, status: 'trialing', metadata: { freeMessages: 999999 }, price_id: null, quantity: 1, cancel_at_period_end: false, created: new Date().toISOString(), current_period_start: new Date().toISOString(), - current_period_end: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), - trial_end: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000).toISOString(), + current_period_end: new Date(Date.now() + ONE_YEAR_MS).toISOString(), + trial_end: new Date(Date.now() + ONE_YEAR_MS).toISOString(), };components/chat/ChatMessage.tsx (1)
28-61: Consider extracting the assistant icon to a separate componentFor better maintainability and consistency with how the user icon is handled, consider extracting the assistant SVG icon into its own component file.
Create a new file components/icons/IconAssistant.tsx:
export function IconAssistant() { return ( <svg width="25" height="25" viewBox="0 0 25 25" fill="none" xmlns="http://www.w3.org/2000/svg" > <path d="M12.5 2L2 7.5L12.5 13L23 7.5L12.5 2Z" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" /> <path d="M2 17.5L12.5 23L23 17.5" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" /> <path d="M2 12.5L12.5 18L23 12.5" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round" /> </svg> ) }Then update the import and usage:
import { IconUser } from '@/components/icons' +import { IconAssistant } from '@/components/icons' // Then in the render function: {isUser ? ( <IconUser /> ) : ( - <svg - width="25" - height="25" - viewBox="0 0 25 25" - fill="none" - xmlns="http://www.w3.org/2000/svg" - > - <path - d="M12.5 2L2 7.5L12.5 13L23 7.5L12.5 2Z" - stroke="currentColor" - strokeWidth="2" - strokeLinecap="round" - strokeLinejoin="round" - /> - <path - d="M2 17.5L12.5 23L23 17.5" - stroke="currentColor" - strokeWidth="2" - strokeLinecap="round" - strokeLinejoin="round" - /> - <path - d="M2 12.5L12.5 18L23 12.5" - stroke="currentColor" - strokeWidth="2" - strokeLinecap="round" - strokeLinejoin="round" - /> - </svg> + <IconAssistant /> )}app/share/[shareId]/page.tsx (2)
20-30: Add error handling for database queryThe current implementation only checks if the conversation exists but doesn't handle other potential database errors. Consider adding a try/catch block to properly handle and report database errors.
- const supabase = createServerComponentClient({ cookies }) - const { data: sharedConversation } = await supabase - .from('shared_conversations') - .select('*') - .eq('share_id', params.shareId) - .eq('is_active', true) - .single() - - if (!sharedConversation) { - notFound() - } + try { + const supabase = createServerComponentClient({ cookies }) + const { data: sharedConversation, error } = await supabase + .from('shared_conversations') + .select('*') + .eq('share_id', params.shareId) + .eq('is_active', true) + .single() + + if (error || !sharedConversation) { + console.error('Error fetching shared conversation:', error) + notFound() + } + } catch (error) { + console.error('Unexpected error:', error) + throw new Error('Failed to load shared conversation') + }
32-58: Consider adding loading and error statesThe page lacks visual feedback for loading states and error conditions. Consider adding loading indicators and error messages to improve user experience.
+ const [isLoading, setIsLoading] = useState(true) + const [error, setError] = useState<string | null>(null) + + // In the try/catch block: + try { + setIsLoading(true) + // Fetch data... + setIsLoading(false) + } catch (err) { + setIsLoading(false) + setError('Failed to load the conversation') + } return ( <div className="flex-1 overflow-hidden"> <div className="flex flex-col h-full overflow-hidden bg-zinc-50 dark:bg-zinc-900"> <div className="flex-1 overflow-hidden"> <div className="max-w-4xl mx-auto p-4 md:p-8 overflow-auto"> + {isLoading && ( + <div className="flex justify-center p-8"> + <div className="animate-spin rounded-full h-12 w-12 border-t-2 border-b-2 border-gray-900 dark:border-gray-100"></div> + </div> + )} + {error && ( + <div className="bg-red-50 dark:bg-red-900/20 p-4 rounded-lg text-red-800 dark:text-red-200"> + {error} + </div> + )} - <div className="rounded-lg border bg-background p-8"> + {!isLoading && !error && <div className="rounded-lg border bg-background p-8"> // Rest of the component... + </div>} </div> </div> </div> </div> )Note: You'll need to add the useState import and make the component a client component to implement this suggestion.
components/ui/markdown.tsx (1)
13-41: Well-implemented markdown renderer with syntax highlighting.The implementation is clean and follows best practices:
- Uses appropriate plugins for extended markdown functionality
- Properly implements syntax highlighting with language detection
- Handles both inline and block code cases
One suggestion to consider:
- <SyntaxHighlighter - {...props} - style={oneDark} - language={match[1]} - PreTag="div" - > - {String(children).replace(/\n$/, '')} - </SyntaxHighlighter> + <SyntaxHighlighter + {...props} + style={oneDark} + language={match[1]} + PreTag="div" + wrapLines={true} + showLineNumbers={match[1] !== 'text'} + > + {String(children).replace(/\n$/, '')} + </SyntaxHighlighter>app/api/share/route.ts (1)
6-52: POST route implementation is solid with proper authentication.The POST route correctly:
- Validates user authentication
- Generates a unique share ID
- Handles errors appropriately
- Returns proper status codes
One suggestion regarding data validation:
Consider adding explicit validation for the request body to ensure required fields are present and properly formatted:
try { const supabase = createRouteHandlerClient({ cookies }) const { title, content } = await req.json() + + // Validate request data + if (!title || !content) { + return NextResponse.json( + { error: 'Title and content are required' }, + { status: 400 } + ) + } // Get the user's sessionapp/Chat.tsx (1)
509-557: Well-implemented share functionality with proper validation and error handling.The
handleSharefunction:
- Validates conversation existence before sharing
- Properly formats the conversation data
- Handles response errors
- Copies the share URL to clipboard
- Provides good user feedback
One potential improvement:
Consider implementing rate limiting to prevent abuse of the sharing feature:
const handleShare = async () => { + // Simple client-side rate limiting + const lastShareTime = localStorage.getItem('lastShareTime'); + const now = Date.now(); + if (lastShareTime && now - parseInt(lastShareTime) < 10000) { + Swal.fire({ + title: 'Please wait', + text: 'You can only share a conversation every 10 seconds', + icon: 'warning', + }); + return; + } + if (!conversationId || !messages.length) { Swal.fire({ title: 'Error', text: 'No conversation to share', icon: 'error', }); return; } try { const response = await fetch('/api/share', { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify({ title: messages[0]?.content || 'Shared Conversation', content: { messages: messages.map(({ id, content, isUser, metadata }) => ({ id, role: isUser ? 'user' : 'assistant', content, metadata, })), }, }), }); const { data, error } = await response.json(); if (error) throw new Error(error); const shareUrl = `${window.location.origin}/share/${data.share_id}`; await navigator.clipboard.writeText(shareUrl); + + // Store the current time for rate limiting + localStorage.setItem('lastShareTime', now.toString()); Swal.fire({ title: 'Success', text: 'Share link copied to clipboard!', icon: 'success', });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
app/Chat.tsx(3 hunks)app/api/share/route.ts(1 hunks)app/share/[shareId]/page.tsx(1 hunks)app/types.ts(1 hunks)components/chat/ChatMessage.tsx(1 hunks)components/icons.tsx(1 hunks)components/ui/markdown.tsx(1 hunks)package.json(4 hunks)schema.sql(1 hunks)types/index.ts(1 hunks)utils/cn.ts(1 hunks)utils/supabase/actions.ts(1 hunks)
🔇 Additional comments (14)
utils/cn.ts (1)
1-6: Well-implemented utility function for Tailwind CSS class mergingThis is a standard and highly effective utility function commonly used in Next.js projects with Tailwind CSS. The
cnfunction correctly combines the power ofclsxfor conditional class rendering withtailwind-mergefor resolving class conflicts according to Tailwind's specificity rules.This implementation follows best practices and will be useful throughout the application for components that need to conditionally apply classes while avoiding conflicts.
components/icons.tsx (1)
1-36: Well-implemented icon componentsBoth
IconUserandIconShareare implemented following React and SVG best practices:
- React functional components with proper capitalization
- SVG attributes correctly camelCased (strokeWidth, strokeLinecap, etc.)
- Using
currentColorfor stroke allows the icons to inherit color from parent elements- Consistent dimensions and styling attributes
These icons will integrate well with the sharing functionality and user message indicators in the chat interface.
types/index.ts (2)
1-6: Well-defined Message interfaceThe Message interface is clearly structured with appropriate types. The optional fields (id and createdAt) are correctly marked with '?', and the role constraint to 'user' or 'assistant' provides good type safety.
8-19: Well-structured SharedConversation interfaceThe SharedConversation interface properly defines all the necessary fields for a shareable conversation with appropriate types. The nested content structure with messages array provides a clean organization of the data.
package.json (1)
23-23: Well-chosen dependencies for the conversation sharing featureThe added dependencies are appropriate:
@supabase/auth-helpers-nextjs- For Supabase authentication in server componentsnanoid- For generating unique share IDsremark-gfm- For GitHub Flavored Markdown support in shared conversations@types/katex- For TypeScript type definitions for math renderingThese dependencies align well with the requirements of the conversation sharing feature.
Also applies to: 36-36, 49-49, 66-66
components/ui/markdown.tsx (2)
1-8: Good choice of dependencies for markdown rendering.The imports include everything needed for a robust markdown renderer with support for GitHub Flavored Markdown, mathematical expressions, and syntax highlighting.
9-11: LGTM on the interface declaration.Clean and simple interface definition for the Markdown component props.
app/api/share/route.ts (2)
1-5: LGTM on the imports.All necessary dependencies are properly imported.
54-86: GET route properly retrieves shared conversations by ID.The implementation correctly:
- Retrieves the share ID from params
- Queries only active shared conversations
- Handles not found cases
- Includes proper error handling
app/Chat.tsx (2)
24-30: Updated imports to use new types location and added icon.The imports are correctly updated to reference the types from their new location.
596-614: Good UI implementation for the share button.The share button is well-positioned in the UI alongside other controls, with proper layout and styling.
schema.sql (3)
149-170: Well-designed shared_conversations table schema.The table structure is well thought out with:
- Appropriate fields for storing conversation data
- A unique share_id field for URL usage
- Created_at timestamp for tracking
- Optional expiration date
- Is_active flag for soft deletion/disabling
171-200: Comprehensive row-level security policies.The RLS policies properly secure the data by:
- Allowing users to operate only on their own conversations
- Allowing public access only to active shared conversations
- Properly restricting all CRUD operations
201-203: Good indexing strategy for performance.The indexes on share_id and user_id will help with query performance for the most common access patterns.
| export function ChatMessage({ message }: ChatMessageProps) { | ||
| const isUser = message.role === 'user' || message.isUser | ||
|
|
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.
Type inconsistency with isUser property
The isUser check at line 11 references a property that isn't defined in the Message interface from types/index.ts. This could lead to type errors.
- const isUser = message.role === 'user' || message.isUser
+ const isUser = message.role === 'user'Alternatively, update the Message interface in types/index.ts to include this property:
export interface Message {
id?: string
role: 'user' | 'assistant'
content: string
createdAt?: Date
isUser?: boolean
}| {sharedConversation.content.messages.map((message: SharedMessage) => ( | ||
| <ChatMessage | ||
| key={message.id} | ||
| message={{ | ||
| id: message.id, | ||
| content: message.content, | ||
| isUser: message.role === 'user', | ||
| metadata: message.metadata, | ||
| }} | ||
| /> | ||
| ))} |
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.
Type inconsistency in message mapping
The component transforms each SharedMessage into a different structure with an isUser property instead of using the role property directly. This creates inconsistency with the Message interface and explains the conditional in the ChatMessage component. Consider maintaining the original message structure.
- {sharedConversation.content.messages.map((message: SharedMessage) => (
- <ChatMessage
- key={message.id}
- message={{
- id: message.id,
- content: message.content,
- isUser: message.role === 'user',
- metadata: message.metadata,
- }}
- />
+ {sharedConversation.content.messages.map((message: SharedMessage) => (
+ <ChatMessage
+ key={message.id}
+ message={{
+ id: message.id,
+ role: message.role,
+ content: message.content
+ }}
+ />
))}Alternatively, update the Message interface in types/index.ts to include metadata:
export interface Message {
id?: string
role: 'user' | 'assistant'
content: string
createdAt?: Date
metadata?: Record<string, unknown>
}| interface SharedMessage { | ||
| id: string | ||
| role: 'user' | 'assistant' | ||
| content: string | ||
| metadata: Record<string, unknown> | ||
| } |
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
Consolidate message interfaces
The SharedMessage interface defined here duplicates functionality from the Message interface in types/index.ts but adds a metadata field. Consider extending the base interface or using it directly to maintain type consistency across the application.
-interface SharedMessage {
- id: string
- role: 'user' | 'assistant'
- content: string
- metadata: Record<string, unknown>
-}
+import { Message } from '@/types'
+
+interface SharedMessage extends Message {
+ metadata: Record<string, unknown>
+}📝 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 SharedMessage { | |
| id: string | |
| role: 'user' | 'assistant' | |
| content: string | |
| metadata: Record<string, unknown> | |
| } | |
| import { Message } from '@/types' | |
| interface SharedMessage extends Message { | |
| metadata: Record<string, unknown> | |
| } |
Built conversation sharing feature.
Summary by CodeRabbit
New Features
Chores