-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(backend): refactor chat service to support creating chats with i… #169
Conversation
WalkthroughThis pull request modifies several components of the chat functionality. In the chat module, the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant CS as ChatService
participant U as User Repository
participant DB as Database
C->>CS: createChatWithMessage(userId, { title, message })
CS->>U: Retrieve user by userId
U-->>CS: Return user data (or error)
alt User exists
CS->>DB: Save new chat with title
DB-->>CS: Return chat with ID
CS->>DB: Save initial message (using chat ID & message)
DB-->>CS: Return updated chat
CS-->>C: Return chat with new message
else User not found
CS-->>C: Return error
end
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
backend/src/chat/chat.module.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-prettier". (The package "eslint-plugin-prettier" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-prettier" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. backend/src/chat/message.model.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-prettier". (The package "eslint-plugin-prettier" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-prettier" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. backend/src/chat/chat.service.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-prettier". (The package "eslint-plugin-prettier" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-prettier" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ 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: 0
🧹 Nitpick comments (6)
backend/src/project/project.service.ts (1)
554-554
: Resolve the FIXME by implementing proper chat copying logic.You have a
FIXME
indicating the need to copy the original chat. I can help propose a solution to clone or replicate the messages from the source chat if desired. Let me know if you'd like me to open a new issue or provide a snippet.backend/src/chat/message.model.ts (1)
32-52
: Manually handle lifecycle fields after removing TypeORM decorators.With TypeORM removed,
createdAt
,updatedAt
,isActive
, andisDeleted
will no longer be auto-managed. Ensure you have the necessary logic in your codebase to correctly initialize and update these fields (e.g., settingupdatedAt
on edits) so data integrity remains intact.backend/src/chat/chat.service.ts (4)
49-49
: Consider adding explicit return type for better code documentationThe return type annotation has been removed from this method. While TypeScript can infer the return type, explicit return types improve code readability and serve as documentation.
- async getChatHistory(chatId: string) { + async getChatHistory(chatId: string): Promise<any[]> {
111-146
: Great implementation of the new createChatWithMessage methodThis new method successfully implements the core feature of this PR: creating a chat with an initial message in one operation. The code properly handles:
- User verification
- Chat creation
- Message generation with appropriate ID format
- Linking the message to the chat
One suggestion to consider: add error handling for the chat save operation.
// Save the chat first to get an ID - const savedChat = await this.chatRepository.save(newChat); + let savedChat; + try { + savedChat = await this.chatRepository.save(newChat); + } catch (error) { + throw new Error(`Failed to create chat: ${error.message}`); + }
190-194
: Enhanced error handling in saveMessageGood addition of a null check for chat existence. Consider adding explicit return type for better code documentation.
- async saveMessage(chatId: string, messageContent: string, role: MessageRole) { + async saveMessage(chatId: string, messageContent: string, role: MessageRole): Promise<any> {
111-146
: Consider adding transaction support for chat and message creationThe current implementation creates a chat and then adds a message in two separate database operations. Consider using TypeORM transactions to ensure atomicity.
async createChatWithMessage( userId: string, newChatInput: { title: string; message: string }, ): Promise<Chat> { const user = await this.userRepository.findOne({ where: { id: userId } }); if (!user) { throw new Error('User not found'); } + // Use a transaction to ensure atomicity + return await this.chatRepository.manager.transaction(async transactionalEntityManager => { // Create a new chat with the initial message const newChat = this.chatRepository.create({ title: newChatInput.title, messages: [], createdAt: new Date(), updatedAt: new Date(), user: user, }); // Save the chat first to get an ID - const savedChat = await this.chatRepository.save(newChat); + const savedChat = await transactionalEntityManager.save(newChat); // Create the message with the chat's ID as a prefix const message = { id: `${savedChat.id}/0`, content: newChatInput.message, role: MessageRole.User, createdAt: new Date(), updatedAt: new Date(), isActive: true, isDeleted: false, }; // Update the chat with the message savedChat.messages = [message]; - return await this.chatRepository.save(savedChat); + return await transactionalEntityManager.save(savedChat); + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/src/chat/chat.module.ts
(1 hunks)backend/src/chat/chat.service.ts
(6 hunks)backend/src/chat/message.model.ts
(1 hunks)backend/src/project/project.service.ts
(2 hunks)
🔇 Additional comments (8)
backend/src/chat/chat.module.ts (1)
14-14
: Ensure all references to the removedMessage
entity are fully updated.Removing
Message
fromforFeature
indicates a shift away from TypeORM for messages. Please verify that no legacy references to TypeORM-basedMessage
remain in your codebase, as they could lead to runtime errors.backend/src/project/project.service.ts (1)
193-195
: Validate and sanitize theinput.description
prior to creating the initial chat message.By passing
input.description
tocreateChatWithMessage
, you risk potential XSS or injection if the input is not sanitized. Please verify user input integrity and confirm that empty or undefined values are handled gracefully.backend/src/chat/chat.service.ts (6)
3-3
: Nice addition of the MessageRole importThis import helps maintain type safety when working with message roles in the chat service.
105-105
: Minor code cleanupThis is a small syntax improvement, using the shorthand object property notation.
167-167
: Improved clarity in clearChatHistoryThis change makes it more explicit that the messages array is being reset, which improves code readability.
196-198
: Defensive programming to prevent null reference errorsThis is a good practice - checking if the messages array exists before attempting to use it prevents potential runtime errors.
200-201
: Clear message ID generation patternThe comment clarifies how message IDs are constructed, which is helpful for maintenance.
219-219
: Updated relations to include userThis ensures the user data is eagerly loaded when fetching a chat, which aligns with the new functionality.
…nitial messages
Summary by CodeRabbit
New Features
Improvements