feat: functionality to view unread messages and mark messages as read. #2353
Conversation
WalkthroughThe changes in this pull request involve updates to translation files for multiple languages, enhancing chat functionalities by adding new keys for user interactions. Additionally, the GraphQL schema and related components have been modified to support marking messages as read and managing unseen messages. The updates include the removal of certain mutations and the introduction of new ones, along with adjustments to the component logic for improved user experience and internationalization. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
Documentation and Community
|
Our Pull Request Approval ProcessWe have these basic policies to make the approval process smoother for our volunteer team. Testing Your CodePlease make sure your code passes all tests. Our test code coverage system will fail if these conditions occur:
The process helps maintain the overall reliability of the code base and is a prerequisite for getting your PR approved. Assigned reviewers regularly review the PR queue and tend to focus on PRs that are passing. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2353 +/- ##
===========================================
- Coverage 97.98% 97.85% -0.13%
===========================================
Files 244 244
Lines 6949 6960 +11
Branches 2009 2011 +2
===========================================
+ Hits 6809 6811 +2
- Misses 130 139 +9
Partials 10 10 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🧹 Outside diff range and nitpick comments (11)
src/components/UserPortal/ContactCard/ContactCard.tsx (1)
Line range hint
17-33: Update JSDoc documentation to include new props.The component's documentation is outdated and missing the new props. Please update it to include:
unseenMessages: Number of unread messageslastMessage: The most recent message in the conversation* @param selectedContact - The ID of the currently selected contact. * @param setSelectedContact - Function to set the ID of the selected contact. -* @param setSelectedContactName - Function to set the name of the selected contact. +* @param unseenMessages - Number of unread messages in the conversation +* @param lastMessage - The most recent message in the conversation +* @param isGroup - Boolean indicating if the contact represents a groupsrc/GraphQl/Queries/PlugInQueries.ts (1)
151-151: Update JSDoc comments to include the new field.The queries' documentation should be updated to include information about the
unseenMessagesByUsersfield for better maintainability and developer experience.Add the new field to the
@returnssection of both query documentations:* @returns The list of direct chats associated with the user, including details such as ID, creator, messages, organization, and participating users. + * The `unseenMessagesByUsers` field tracks messages that haven't been read by users. */Also applies to: 191-191
src/GraphQl/Mutations/OrganizationMutations.ts (1)
80-86: Add JSDoc documentation for consistency.Consider adding JSDoc documentation to match the style of other mutations in this file, describing the parameters and return value.
+/** + * GraphQL mutation to mark all messages in a chat as read for a specific user. + * + * @param chatId - The ID of the chat containing the messages + * @param userId - The ID of the user marking messages as read + * @returns The updated chat message object + */ export const MARK_CHAT_MESSAGES_AS_READ = gql`src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (1)
195-195: Consider translating other hardcoded strings.While it's good that the "Add" button text is now translated, there are other hardcoded strings in the component that should also use the translation system for consistency:
Apply similar changes to these hardcoded strings:
- <Modal.Title>{'Chat'}</Modal.Title> + <Modal.Title>{t('chat')}</Modal.Title> - placeholder="searchFullName" + placeholder={t('searchFullName')} - <StyledTableCell>#</StyledTableCell> - <StyledTableCell align="center">{'user'}</StyledTableCell> - <StyledTableCell align="center">{'Chat'}</StyledTableCell> + <StyledTableCell>{t('tableHeader.number')}</StyledTableCell> + <StyledTableCell align="center">{t('tableHeader.user')}</StyledTableCell> + <StyledTableCell align="center">{t('tableHeader.chat')}</StyledTableCell>src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (3)
Line range hint
191-391: Consider localizing remaining hardcoded strings.There are several hardcoded strings in the component that should be internationalized for consistency:
- "New Group"
- "Group name"
- "Next"
- "Chat"
- "searchFullName"
- "Remove"
- Table headers: "#", "user", "Chat"
Example implementation:
- <Modal.Title>New Group</Modal.Title> + <Modal.Title>{t('newGroup')}</Modal.Title> - <Form.Label>Group name</Form.Label> + <Form.Label>{t('groupName')}</Form.Label> - placeholder={'Group name'} + placeholder={t('groupName')} - <Button>Next</Button> + <Button>{t('next')}</Button> - <Modal.Title>{'Chat'}</Modal.Title> + <Modal.Title>{t('chat')}</Modal.Title> - placeholder="searchFullName" + placeholder={t('searchFullName')} - <Button variant="danger">Remove</Button> + <Button variant="danger">{t('remove')}</Button> - <StyledTableCell>#</StyledTableCell> - <StyledTableCell align="center">{'user'}</StyledTableCell> - <StyledTableCell align="center">{'Chat'}</StyledTableCell> + <StyledTableCell>{t('tableIndex')}</StyledTableCell> + <StyledTableCell align="center">{t('user')}</StyledTableCell> + <StyledTableCell align="center">{t('chat')}</StyledTableCell>
Line range hint
191-205: Remove commented-out code.There's a commented-out FormControl section that's duplicated below. Clean up the code by removing the commented section as it's redundant.
Line range hint
282-297: Add error handling for the createChat mutation.The
handleCreateGroupChatfunction should handle potential errors from the GraphQL mutation to provide better user feedback.Example implementation:
async function handleCreateGroupChat(): Promise<void> { - await createChat({ - variables: { - organizationId: selectedOrganization, - userIds: [userId, ...userIds], - name: title, - isGroup: true, - }, - }); - chatsListRefetch(); - toggleAddUserModal(); - toggleCreateGroupChatModal(); - reset(); + try { + await createChat({ + variables: { + organizationId: selectedOrganization, + userIds: [userId, ...userIds], + name: title, + isGroup: true, + }, + }); + await chatsListRefetch(); + toggleAddUserModal(); + toggleCreateGroupChatModal(); + reset(); + } catch (error) { + // Handle error appropriately (e.g., show error message to user) + console.error('Failed to create group chat:', error); + } }schema.graphql (1)
766-767: Consider enhancing read status tracking.While the current implementation allows marking messages as read, consider these enhancements for better message tracking:
- Add a
lastReadMessageIdfield to track each user's read position- Add a
readByfield to ChatMessage type for granular read status- Consider adding a subscription for read status updates
This would enable:
- Unread message counts
- Read receipts
- More efficient message fetching
src/screens/UserPortal/Chat/Chat.test.tsx (1)
Line range hint
2724-2764: Add test coverage for mark-as-read functionalityThe test case for "create new direct chat" lacks coverage for the new mark-as-read feature. Consider adding specific test cases to verify:
- Messages are marked as read when selecting a chat
- Error handling for failed mark-as-read requests
- UI updates after marking messages as read
Add new test cases:
test('marks messages as read when selecting a chat', async () => { // Setup mocks const mock = [ ...MARK_CHAT_MESSAGES_AS_READ_MOCK, // Other required mocks ]; // Render component render( <MockedProvider mocks={mock}> <Chat /> </MockedProvider> ); // Select a chat const chatItem = await screen.findByTestId('chat-item-1'); fireEvent.click(chatItem); // Verify messages are marked as read await waitFor(() => { expect(screen.queryByTestId('unread-indicator')).not.toBeInTheDocument(); }); }); test('handles mark-as-read errors gracefully', async () => { // Similar structure with error mock });src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (2)
Line range hint
2775-2791: Standardize async testing patterns.The test uses a mix of
wait()andwaitFor()for handling async operations. Standardize onwaitFor()as it provides better error messages and retry behavior.test('Open and close create new direct chat modal', async () => { render(/* ... */); await waitFor(() => { expect(screen.getByTestId('dropdown')).toBeInTheDocument(); }); fireEvent.click(screen.getByTestId('dropdown')); await waitFor(() => { expect(screen.getByTestId('newDirectChat')).toBeInTheDocument(); }); // Continue with standardized waitFor pattern });
Line range hint
2827-2869: Add test cases for error scenarios and edge cases.The current tests only cover the happy path. Consider adding test cases for:
- Network errors in GraphQL mutations
- Invalid user selections
- Empty user list
- Error handling when creating a chat fails
Would you like me to help generate additional test cases for these scenarios?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (19)
- public/locales/en/translation.json (1 hunks)
- public/locales/fr/translation.json (1 hunks)
- public/locales/hi/translation.json (1 hunks)
- public/locales/sp/translation.json (1 hunks)
- public/locales/zh/translation.json (1 hunks)
- schema.graphql (1 hunks)
- src/GraphQl/Mutations/OrganizationMutations.ts (2 hunks)
- src/GraphQl/Mutations/mutations.ts (0 hunks)
- src/GraphQl/Queries/PlugInQueries.ts (2 hunks)
- src/components/UserPortal/ChatRoom/ChatRoom.tsx (5 hunks)
- src/components/UserPortal/ContactCard/ContactCard.module.css (2 hunks)
- src/components/UserPortal/ContactCard/ContactCard.test.tsx (1 hunks)
- src/components/UserPortal/ContactCard/ContactCard.tsx (3 hunks)
- src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (18 hunks)
- src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (3 hunks)
- src/components/UserPortal/CreateGroupChat/CreateGroupChat.test.tsx (17 hunks)
- src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (3 hunks)
- src/screens/UserPortal/Chat/Chat.test.tsx (20 hunks)
- src/screens/UserPortal/Chat/Chat.tsx (9 hunks)
💤 Files with no reviewable changes (1)
- src/GraphQl/Mutations/mutations.ts
🧰 Additional context used
🪛 Biome
src/screens/UserPortal/Chat/Chat.tsx
[error] 170-170: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (24)
src/components/UserPortal/ContactCard/ContactCard.module.css (1)
23-25: LGTM! Layout changes improve element spacing.The updated layout properties create a better structure for displaying contact name and unseen message count.
src/components/UserPortal/ContactCard/ContactCard.test.tsx (1)
35-36: Verify prop types in test setup.The
unseenMessagesprop is initialized with a number, but it's recommended to verify if this matches the type definition in the component interface.✅ Verification successful
Props match the component's type definition.
From the search results, we can see in
src/components/UserPortal/ContactCard/ContactCard.tsxthatunseenMessagesis indeed defined as a number in the component's interface:unseenMessages: number;This matches the test setup where it's initialized with a numeric value of 2. The prop type is correctly used in the test.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the ContactCard interface/type definition ast-grep --pattern 'interface $interface { $$$ unseenMessages$_ $$$ }'Length of output: 78
Script:
#!/bin/bash # Let's try a different pattern to find type definitions for ContactCard ast-grep --pattern 'type ContactCard = { $$$ unseenMessages$_ $$$ }' # Also search for prop type definitions ast-grep --pattern 'type ContactCardProps = { $$$ unseenMessages$_ $$$ }' # Let's also try to find any interface or type containing unseenMessages rg "unseenMessages.*:" -A 1 -B 1Length of output: 26924
src/GraphQl/Queries/PlugInQueries.ts (1)
151-151: LGTM! The changes align with the feature requirements.The addition of
unseenMessagesByUsersto both queries is consistent and supports the PR objective of enabling users to view unread messages. The field placement is logical within the chat data structure.Also applies to: 191-191
src/GraphQl/Mutations/OrganizationMutations.ts (1)
Line range hint
39-63: Good architectural decision to consolidate chat creation mutations.The replacement of separate
CREATE_GROUP_CHATandCREATE_DIRECT_CHATmutations with a singleCREATE_CHATmutation using anisGroupflag is a clean architectural improvement that reduces code duplication.src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (3)
28-31: LGTM! Improved type safety.Good improvement in type safety:
- Using
stringinstead ofanyfor the id parameter- Using
unknowninstead ofanyfor the return type follows TypeScript best practices
Line range hint
1-207: Verify relationship to unread messages functionality.While this component handles direct chat creation, it doesn't directly implement the PR's main objectives of viewing unread messages or marking messages as read. Could you clarify how this component fits into the larger feature implementation?
#!/bin/bash # Description: Look for related changes implementing unread messages functionality # Test 1: Search for unread message related code echo "Checking for unread message related code:" rg -i "unread|unseen|mark.*read" --type ts --type tsx # Test 2: Search for related GraphQL operations echo "Checking for related GraphQL operations:" rg -i "mutation.*mark.*read|query.*(unread|unseen)" --type graphql
8-8: Verify the CREATE_CHAT mutation structure.The switch from CREATE_DIRECT_CHAT to CREATE_CHAT suggests a consolidated mutation. Let's verify that the mutation structure matches the component's usage.
✅ Verification successful
The CREATE_CHAT mutation is correctly structured for both direct and group chats
The verification shows that the CREATE_CHAT mutation in
OrganizationMutations.tsis properly consolidated to handle both direct and group chats through theisGroupparameter. The mutation structure matches the component's usage as evidenced by:
- The mutation accepts required parameters:
userIds,isGroup- Optional parameters:
organizationId,name(for group chats)- Test files confirm correct usage for both direct chats (
isGroup: false) and group chats (isGroup: true)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the CREATE_CHAT mutation structure and its usage # Test 1: Check the mutation definition echo "Checking CREATE_CHAT mutation definition:" ast-grep --pattern 'const CREATE_CHAT = gql` mutation $_($$$) { $$$ } `' # Test 2: Check for any other usages of CREATE_CHAT echo "Checking other CREATE_CHAT usages:" rg "CREATE_CHAT" -A 5Length of output: 368
Script:
#!/bin/bash # Let's try a different approach to verify the mutation structure and usage # Check for GraphQL mutation files echo "Checking for GraphQL mutation files:" fd -e ts -e tsx -e graphql "Mutations" # Search for CREATE_CHAT or CREATE_DIRECT_CHAT in the codebase echo -e "\nSearching for CREATE_CHAT and CREATE_DIRECT_CHAT:" rg -l "CREATE_CHAT|CREATE_DIRECT_CHAT" # If we find the files, let's look at their content echo -e "\nChecking content of files containing mutations:" rg "const CREATE_CHAT|const CREATE_DIRECT_CHAT" -A 10Length of output: 3907
src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (3)
39-39: LGTM! Good type safety improvement.Changing from
anytounknownis a good practice as it provides better type safety by requiring explicit type checking.
370-370: LGTM! Good internationalization improvements.The button text has been properly internationalized using the translation hook.
Also applies to: 388-388
Line range hint
1-391: Verify alignment with PR objectives.This component appears to be unrelated to the PR objectives of "viewing unread messages and marking messages as read". Please clarify how this component contributes to those objectives or if it should be part of a different PR.
Let's verify the relationship between this component and the PR objectives:
schema.graphql (1)
766-766: LGTM! Improved message sending with reply support.The mutation signature has been simplified by removing the redundant 'type' parameter while adding support for message replies through the optional 'replyTo' parameter.
public/locales/zh/translation.json (2)
1162-1169: LGTM! Chat action translations are appropriate.The Chinese translations for chat actions are contextually accurate and properly localized:
- "添加" (add) - correct translation for adding contacts/chats
- "创造" (create) - appropriate for creating new chats
- "新聊天" (new chat) - clear and natural translation
- "新群聊" (new group chat) - properly distinguishes group chat
1173-1174: LGTM! Message action translations are accurate.The Chinese translations for message actions are clear and natural:
- "发信息" (send message) - commonly used phrase
- "回复" (reply) - correct translation for replying to messages
public/locales/en/translation.json (2)
1162-1169: LGTM: Chat creation translations added.The new translations support chat creation functionality with clear, concise labels for creating both individual and group chats.
1173-1174: LGTM: Reply functionality translation added.The addition of the "reply" translation supports the new reply feature in chat rooms.
public/locales/hi/translation.json (2)
1162-1169: LGTM! The Hindi translations for chat actions are accurate and natural.The translations maintain proper semantic meaning while being culturally appropriate and easily understandable by Hindi speakers.
1173-1174: LGTM! The Hindi translations for chat room actions are well-translated.The translations are concise and accurately convey the meaning of the English terms.
public/locales/fr/translation.json (2)
1162-1169: LGTM! The French translations for chat actions are accurate.The translations maintain consistency with French language conventions and accurately convey the meaning of the chat-related actions.
1173-1174: LGTM! The French translations for message actions are accurate.The translations "Envoyer le message" and "répondre" are grammatically correct and effectively convey the intended actions.
public/locales/sp/translation.json (2)
1163-1170: LGTM! Chat action translations are accurate.The Spanish translations for chat actions and UI elements are linguistically correct and maintain consistency with the English version.
1174-1175: LGTM! Message action translations are accurate.The Spanish translations for message actions are linguistically correct and maintain consistency with the English version.
src/screens/UserPortal/Chat/Chat.test.tsx (1)
Line range hint
2633-2672: LGTM: Well-structured test utilitiesThe test utilities are well-implemented with:
- Proper async wait helper
- Comprehensive window.matchMedia mock
- Mocked scrollIntoView behavior
src/components/UserPortal/CreateGroupChat/CreateGroupChat.test.tsx (2)
1340-1343: LGTM: Consistent mock data structure for unread messages tracking.The unseenMessagesByUsers field is well-structured and consistently implemented across all mock responses, properly representing the unread message counts per user.
Also applies to: 1404-1407, 1468-1471, 1560-1566, 1639-1645, 1732-1738, 1811-1817, 1983-1989, 2076-2082, 2155-2161, 2248-2254, 2327-2333, 2420-2426, 2499-2505, 2594-2600, 2691-2697, 2785-2791
2798-2875: Verify the empty chatId in MARK_CHAT_MESSAGES_AS_READ mock.While the mutation mock is well-structured, using an empty string for chatId in the test cases might not accurately represent real-world scenarios and could mask potential issues.
Consider using valid chat IDs from the mock data:
- chatId: '', + chatId: '65844efc814dd4003db811c4',
| .unseenMessagesCount { | ||
| border-radius: 50%; | ||
| } |
There was a problem hiding this comment.
Add essential styles for the notification badge.
The current styling only makes the badge circular but lacks other necessary properties for a proper notification indicator.
.unseenMessagesCount {
border-radius: 50%;
+ background-color: #ff4444; /* or your theme's notification color */
+ color: white;
+ min-width: 20px;
+ min-height: 20px;
+ padding: 2px 6px;
+ display: flex;
+ align-items: center;
+ justify-content: center;
+ font-size: 12px;
+ font-weight: bold;
}📝 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.
| .unseenMessagesCount { | |
| border-radius: 50%; | |
| } | |
| .unseenMessagesCount { | |
| border-radius: 50%; | |
| background-color: #ff4444; /* or your theme's notification color */ | |
| color: white; | |
| min-width: 20px; | |
| min-height: 20px; | |
| padding: 2px 6px; | |
| display: flex; | |
| align-items: center; | |
| justify-content: center; | |
| font-size: 12px; | |
| font-weight: bold; | |
| } |
| .lastMessage { | ||
| margin-bottom: 0; | ||
| font-size: 14px; | ||
| color: rgba(163, 163, 163, 0.839); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding text overflow handling for long messages.
The last message preview should gracefully handle long text to prevent layout issues.
.lastMessage {
margin-bottom: 0;
font-size: 14px;
color: rgba(163, 163, 163, 0.839);
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ max-width: 70%; /* Adjust based on design needs */
}📝 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.
| .lastMessage { | |
| margin-bottom: 0; | |
| font-size: 14px; | |
| color: rgba(163, 163, 163, 0.839); | |
| } | |
| .lastMessage { | |
| margin-bottom: 0; | |
| font-size: 14px; | |
| color: rgba(163, 163, 163, 0.839); | |
| white-space: nowrap; | |
| overflow: hidden; | |
| text-overflow: ellipsis; | |
| max-width: 70%; /* Adjust based on design needs */ | |
| } |
| <div> | ||
| <b>{props.title}</b>{' '} | ||
| <p className={styles.lastMessage}>{props.lastMessage}</p> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and user experience.
Consider the following improvements:
- Add ARIA labels for better screen reader support
- Add text truncation for long messages to maintain consistent layout
- <div>
+ <div role="text" aria-label={`Contact: ${props.title}`}>
<b>{props.title}</b>{' '}
- <p className={styles.lastMessage}>{props.lastMessage}</p>
+ <p className={`${styles.lastMessage} text-truncate`} title={props.lastMessage}>
+ {props.lastMessage}
+ </p>
</div>Don't forget to add the corresponding CSS:
.lastMessage {
max-width: 200px; /* adjust based on your layout */
margin-bottom: 0;
}| unseenMessages: number; | ||
| lastMessage: any; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding proper typing for lastMessage.
Using any type reduces type safety. Consider creating a proper interface for the message structure:
- lastMessage: any;
+ lastMessage?: {
+ content: string;
+ timestamp?: Date;
+ // Add other relevant fields
+ };Committable suggestion was skipped due to low confidence.
| {!!props.unseenMessages && ( | ||
| <Badge className={styles.unseenMessagesCount}> | ||
| {props.unseenMessages} | ||
| </Badge> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve badge accessibility and code clarity.
The badge should be more accessible to screen readers and the boolean check could be more explicit.
- {!!props.unseenMessages && (
+ {props.unseenMessages > 0 && (
<Badge
+ aria-label={`${props.unseenMessages} unread messages`}
+ role="status"
className={styles.unseenMessagesCount}>
{props.unseenMessages}
</Badge>
)}📝 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.
| {!!props.unseenMessages && ( | |
| <Badge className={styles.unseenMessagesCount}> | |
| {props.unseenMessages} | |
| </Badge> | |
| )} | |
| {props.unseenMessages > 0 && ( | |
| <Badge | |
| aria-label={`${props.unseenMessages} unread messages`} | |
| role="status" | |
| className={styles.unseenMessagesCount}> | |
| {props.unseenMessages} | |
| </Badge> | |
| )} |
| chat.messages[chat.messages.length - 1] | ||
| ?.messageContent, |
There was a problem hiding this comment.
Handle empty messages array to prevent errors
Accessing the last message without checking if the messages array is empty can lead to undefined values.
Apply this diff to safely access the last message:
- lastMessage:
- chat.messages[chat.messages.length - 1]
- ?.messageContent,
+ lastMessage:
+ chat.messages && chat.messages.length > 0
+ ? chat.messages[chat.messages.length - 1]?.messageContent
+ : '',📝 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.
| chat.messages[chat.messages.length - 1] | |
| ?.messageContent, | |
| chat.messages && chat.messages.length > 0 | |
| ? chat.messages[chat.messages.length - 1]?.messageContent | |
| : '', |
| React.useEffect(() => { | ||
| if (chatsListData) { | ||
| if (chatsListData && chatsListData?.chatsByUserId.length) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify condition using optional chaining
You can simplify the condition by relying on optional chaining, making the code cleaner and more concise.
Apply this diff:
- if (chatsListData && chatsListData?.chatsByUserId.length) {
+ if (chatsListData?.chatsByUserId?.length) {📝 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.
| if (chatsListData && chatsListData?.chatsByUserId.length) { | |
| if (chatsListData?.chatsByUserId?.length) { |
🧰 Tools
🪛 Biome
[error] 170-170: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| useEffect(() => { | ||
| markChatMessagesAsRead().then(() => { | ||
| chatsListRefetch({ id: userId }); | ||
| }); | ||
| }, [selectedContact]); |
There was a problem hiding this comment.
Add error handling for markChatMessagesAsRead mutation
In the useEffect hook, the markChatMessagesAsRead() mutation is called without error handling. If the mutation fails, it could result in unhandled promise rejections and the UI may not reflect the failure.
Apply this diff to handle errors:
useEffect(() => {
- markChatMessagesAsRead().then(() => {
- chatsListRefetch({ id: userId });
- });
+ markChatMessagesAsRead()
+ .then(() => {
+ chatsListRefetch({ id: userId });
+ })
+ .catch((error) => {
+ console.error('Error marking messages as read:', error);
+ // Optionally, display an error notification to the user
+ });
}, [selectedContact]);📝 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.
| useEffect(() => { | |
| markChatMessagesAsRead().then(() => { | |
| chatsListRefetch({ id: userId }); | |
| }); | |
| }, [selectedContact]); | |
| useEffect(() => { | |
| markChatMessagesAsRead() | |
| .then(() => { | |
| chatsListRefetch({ id: userId }); | |
| }) | |
| .catch((error) => { | |
| console.error('Error marking messages as read:', error); | |
| // Optionally, display an error notification to the user | |
| }); | |
| }, [selectedContact]); |
| const [markChatMessagesAsRead] = useMutation(MARK_CHAT_MESSAGES_AS_READ, { | ||
| variables: { | ||
| chatId: selectedContact, | ||
| userId: userId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Pass variables during mutation execution to prevent stale data
Passing variables in the useMutation hook options can cause stale data issues if selectedContact or userId changes. It's better to pass variables when calling the mutation to ensure they are up-to-date.
Apply this diff to pass variables during mutation execution:
- const [markChatMessagesAsRead] = useMutation(MARK_CHAT_MESSAGES_AS_READ, {
- variables: {
- chatId: selectedContact,
- userId: userId,
- },
- });
+ const [markChatMessagesAsRead] = useMutation(MARK_CHAT_MESSAGES_AS_READ);Then, update the call in the useEffect hook:
useEffect(() => {
- markChatMessagesAsRead().then(() => {
+ markChatMessagesAsRead({
+ variables: {
+ chatId: selectedContact,
+ userId: userId,
+ },
+ })
+ .then(() => {
chatsListRefetch({ id: userId });
})
+ .catch((error) => {
+ console.error('Error marking messages as read:', error);
+ // Optionally, display an error notification to the user
+ });
}, [selectedContact]);Committable suggestion was skipped due to low confidence.
| unseenMessages: JSON.parse(chat.unseenMessagesByUsers)[ | ||
| userId | ||
| ], |
There was a problem hiding this comment.
Ensure safe parsing of JSON data
Using JSON.parse without error handling can lead to runtime exceptions if the data is invalid. It's safer to add error handling when parsing JSON.
Apply this diff to add error handling:
- unseenMessages: JSON.parse(chat.unseenMessagesByUsers)[
- userId
- ],
+ let unseenMessages = 0;
+ try {
+ const unseenMessagesData = JSON.parse(chat.unseenMessagesByUsers);
+ unseenMessages = unseenMessagesData[userId] || 0;
+ } catch (error) {
+ console.error('Failed to parse unseenMessagesByUsers:', error);
+ }
+ // Pass unseenMessages to cardPropsAlternatively, ensure that chat.unseenMessagesByUsers is parsed before being passed to this component.
Committable suggestion was skipped due to low confidence.
|
This pull request did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work. |
|
Please:
|
|
This pull request did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work. |
|
@disha1202 Any update on this one ? |
|
@disha1202 We have made significant changes to the Can you close this and merge against that branch? |
|
@disha1202 I rebased your PR against the |
What kind of change does this PR introduce?
Issue Number:
Fixes #2352
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Style