Added features to filter chats, edit messages, add users to group chat, share images as messages, and create dedicated group chats for events #2360
Conversation
|
Please fix:
Make sure CodeRabbit.ai approves your changes |
…ation/talawa-admin into group-chat-features
…lawa-admin into group-chat-features
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
public/locales/zh/translation.json (1)
583-584: 🛠️ Refactor suggestionConsolidate duplicate chat-related translations.
The translations for "done" and "createChat" in the "eventListCard" section are duplicates of those in "organizationEvents". These should be consolidated in the "userChat" section.
Apply this diff to remove the duplicates:
"eventListCard": { - "done": "完成", - "createChat": "创建聊天" }public/locales/hi/translation.json (3)
442-443: 🛠️ Refactor suggestionMove chat-related translations to the appropriate section.
The translations for "done" and "createChat" appear to be misplaced in the "organizationEvents" section. These strings are related to chat functionality and should be moved to the "userChat" section.
Apply this diff to move the translations to the appropriate section:
"organizationEvents": { - "done": "पूर्ण", - "createChat": "चैट बनाएं" }, "userChat": { + "done": "पूर्ण", + "createChat": "चैट बनाएं" }
583-584: 🛠️ Refactor suggestionConsolidate duplicate chat-related translations.
The translations for "done" and "createChat" in the "eventListCard" section are duplicates. These should be consolidated in the "userChat" section.
Apply this diff to remove the duplicates:
"eventListCard": { - "done": "समाप्त", - "createChat": "चैट बनाएं" }
1264-1264:⚠️ Potential issueTranslate "Add Members" to Hindi.
The text "Add Members" should be translated to Hindi to maintain consistency with the rest of the translations.
Apply this diff to fix the translation:
- "addMembers": "Add Members" + "addMembers": "सदस्य जोड़ें"public/locales/fr/translation.json (2)
442-443: 🛠️ Refactor suggestionMove chat-related translations to the appropriate section.
The translations for "done" and "createChat" appear to be misplaced in the "organizationEvents" section. These strings are related to chat functionality and should be moved to the "userChat" section.
Apply this diff to move the translations to the appropriate section:
"organizationEvents": { - "done": "Fait", - "createChat": "Créer une discussion" }, "userChat": { + "done": "Fait", + "createChat": "Créer une discussion" }
1264-1264:⚠️ Potential issueTranslate "Add Members" to French.
The text "Add Members" should be translated to French to maintain consistency with the rest of the translations.
Apply this diff to fix the translation:
- "addMembers": "Add Members" + "addMembers": "Ajouter des membres"
🧹 Nitpick comments (4)
src/screens/UserPortal/Events/Events.spec.tsx (1)
570-572: Double toggle of createChatCheck
This toggles the checkbox on and off. If intended to test multiple toggles, that’s fine. Otherwise, consider if one toggle suffices for coverage.- userEvent.click(screen.getByTestId('createChatCheck')); - userEvent.click(screen.getByTestId('createChatCheck')); + // Possibly remove one toggle if double toggling isn't strictly necessary.src/GraphQl/Queries/PlugInQueries.ts (1)
304-314: Consider optimizing the search conditionsThe current search implementation in the where clause combines multiple conditions. Consider:
- Using OR instead of AND for more flexible matching
- Adding a search across message content
- Implementing fuzzy matching for better user experience
- where: { - name_contains: $searchString - user: { - firstName_contains: $searchString - lastName_contains: $searchString - } - } + where: { + OR: [ + { name_contains: $searchString }, + { "users.firstName_contains": $searchString }, + { "users.lastName_contains": $searchString }, + { "messages.messageContent_contains": $searchString } + ] + }src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)
3744-3841: Enhance test coverage for message markingThe MARK_CHAT_MESSAGES_AS_READ mock should include additional test cases:
- Invalid chat IDs
- Error scenarios
- Batch marking of multiple messages
Add these test cases:
test('should handle error when marking messages as read', async () => { // Test implementation for error handling }); test('should handle invalid chat ID', async () => { // Test implementation for invalid chat ID }); test('should handle batch marking of messages', async () => { // Test implementation for batch operations });src/components/GroupChatDetails/GroupChatDetails.test.tsx (1)
71-73: Simplify JSON.parse usage for mock dataUsing
JSON.parsefor static objects is unnecessary and reduces readability.-unseenMessagesByUsers: JSON.parse( - '{"hbjguyt7y9890i9otyugttiyuh": 0, "gjhnbbjku68979089ujhvserty": 0}', -), +unseenMessagesByUsers: { + "hbjguyt7y9890i9otyugttiyuh": 0, + "gjhnbbjku68979089ujhvserty": 0 +},Also applies to: 426-428
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
public/locales/en/translation.json(4 hunks)public/locales/fr/translation.json(4 hunks)public/locales/hi/translation.json(4 hunks)public/locales/sp/translation.json(4 hunks)public/locales/zh/translation.json(4 hunks)schema.graphql(1 hunks)src/App.tsx(1 hunks)src/GraphQl/Mutations/OrganizationMutations.ts(1 hunks)src/GraphQl/Queries/PlugInQueries.ts(4 hunks)src/components/GroupChatDetails/GroupChatDetails.test.tsx(1 hunks)src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx(21 hunks)src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx(4 hunks)src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx(10 hunks)src/screens/OrganizationEvents/OrganizationEvents.tsx(2 hunks)src/screens/UserPortal/Events/Events.spec.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- schema.graphql
- src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx
- public/locales/sp/translation.json
- src/App.tsx
- src/screens/OrganizationEvents/OrganizationEvents.tsx
- public/locales/en/translation.json
🧰 Additional context used
📓 Learnings (1)
src/GraphQl/Queries/PlugInQueries.ts (1)
Learnt from: disha1202
PR: PalisadoesFoundation/talawa-admin#2394
File: src/GraphQl/Queries/PlugInQueries.ts:103-106
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The `isGroup` field in the `Chat` type is essential for distinguishing between group chats and direct chats and should not be removed.
🔇 Additional comments (28)
src/screens/UserPortal/Events/Events.spec.tsx (13)
101-101: Consistent introduction of the createChat field for mocked data
The addition of "createChat: false" for this event aligns with the new chat creation feature. Ensure that all tests correctly handle events with and without chat creation.
133-133: Validation of createChat set to true
Including an event that has "createChat: true" is a good test scenario to validate chat creation. Confirm that the UI logic correctly differentiates between "createChat: true" and "createChat: false."
184-184: Coverage for createChat in additional event
This maintains consistency in mocked data. Make sure there is a test path verifying behavior when createChat is false for an event that is otherwise public and non-registerable.
213-233: Inclusion of createChat in CREATE_EVENT_MUTATION variables
Adding "createChat" to the mutation payload is correct. Be sure your test covers both toggled and untoggled states.
[approve]
234-260: Repeated mutation block with createChat
No issues spotted. Ensure you keep the data consistent with the feature requirements and that you have tests for this scenario.
261-287: Consistent usage of createChat throughout
Continuing to use "createChat: false" in the test data adheres to consistent coverage. Confirm that the UI handles repeated event creation calls with varying createChat states.
Line range hint 288-314: Multiple test entries for event creation
It’s good to see different test mocks. Consider verifying that each event variant (with or without chat) is tested to prevent regressions.
321-332: Testing createChat edge cases
Testing "createChat: false" again ensures coverage of non-chat events. No immediate concerns.
370-386: Mutation data block consistency
Ensure the date/time generation logic and the createChat field are tested for correctness, especially with different time zones.
425-440: Standardized usage of mutation variables
No anomalies found. Keep in mind potential interactions between new UI elements (the createChat checkbox) and these mocked variable sets.
451-463: Positive test path for createChat = true
This scenario ensures an all-day event with a chat. Confirm the final coverage includes verifying the creation of that chat via UI or GraphQL mocks.
397-413: Maintaining alignment between FE and BE
The “createChat: false” property is consistently applied. Verify that the server-side schema can handle the new property gracefully.
343-359: Redundant or repeated test structures
The repeated blocks might be combining coverage scenarios. Double-check any changes in other files (like resolvers) to ensure the logic matches exactly what’s mocked here.
src/GraphQl/Queries/PlugInQueries.ts (3)
170-170: LGTM: Media field added to support image sharing
The addition of the media field to message objects enables image sharing functionality in chats.
Line range hint 257-303: LGTM: Well-structured unread chats query
The UNREAD_CHAT_LIST query is properly implemented with all necessary fields for handling unread messages and chat details.
210-255: Verify pagination for group chat list
The GROUP_CHAT_LIST query looks well-structured but consider implementing pagination to handle large numbers of group chats efficiently.
Run this script to check if the number of group chats could be significant:
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)
467-479: Refactor mock data to improve maintainability
The mock data contains significant duplication. Consider extracting common objects and using factory functions as suggested in previous reviews.
Also applies to: 540-552, 613-625
src/GraphQl/Mutations/OrganizationMutations.ts (2)
66-66: LGTM: Image parameter added to CREATE_CHAT mutation
The addition of the image parameter aligns with the PR objectives for supporting image sharing in chats.
Also applies to: 74-74
90-96:
Security concern: userId parameter in MARK_CHAT_MESSAGES_AS_READ
The mutation accepts userId as a parameter, which could allow users to mark messages as read for other users. Consider retrieving the userId from the authenticated session instead.
Apply this diff to fix the security issue:
-export const MARK_CHAT_MESSAGES_AS_READ = gql`
- mutation markChatMessagesAsRead($chatId: ID!, $userId: ID!) {
- markChatMessagesAsRead(chatId: $chatId, userId: $userId) {
+export const MARK_CHAT_MESSAGES_AS_READ = gql`
+ mutation markChatMessagesAsRead($chatId: ID!) {
+ markChatMessagesAsRead(chatId: $chatId) {
_id
}
}
`;Likely invalid or redundant comment.
src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (2)
217-217:
Fix typo in data-testid attribute
The attribute data-tsetid should be data-testid.
-data-tsetid="groupDescriptionInput"
+data-testid="groupDescriptionInput"Likely invalid or redundant comment.
151-159: 🛠️ Refactor suggestion
Add file validation to handleImageChange
The image upload handler lacks important validations:
- File size limits
- File type restrictions
- Error handling for base64 conversion
Apply this diff to add validation:
const handleImageChange = async (
e: React.ChangeEvent<HTMLInputElement>,
): Promise<void> => {
const file = e.target.files?.[0];
if (file) {
+ if (file.size > 5 * 1024 * 1024) {
+ alert(t('fileSizeError'));
+ return;
+ }
+ if (!file.type.match(/^image\/(jpeg|png|gif)$/)) {
+ alert(t('fileTypeError'));
+ return;
+ }
+ try {
const base64 = await convertToBase64(file);
setSelectedImage(base64);
+ } catch (error) {
+ console.error('Error converting image:', error);
+ alert(t('imageProcessingError'));
+ }
}
};Likely invalid or redundant comment.
src/components/GroupChatDetails/GroupChatDetails.test.tsx (1)
24-30: 🛠️ Refactor suggestion
Replace custom wait function with React Testing Library utilities
The custom wait function should be replaced with React Testing Library's built-in waitFor utility for better reliability and maintainability.
Remove the custom wait function and update its usages:
-async function wait(ms = 100): Promise<void> {
- await act(() => {
- return new Promise((resolve) => {
- setTimeout(resolve, ms);
- });
- });
-}
// Replace usages with waitFor:
-await wait();
+await waitFor(() => {
+ // Add an assertion here
+}, { timeout: 1000 });Likely invalid or redundant comment.
public/locales/zh/translation.json (3)
1268-1269: LGTM! Chat room translations look good.
The translations in the userChatRoom section are properly placed and correctly translated to Chinese.
1254-1264:
Translate "Add Members" to Chinese.
The text "Add Members" should be translated to Chinese to maintain consistency with the rest of the translations.
Apply this diff to fix the translation:
- "addMembers": "Add Members"
+ "addMembers": "添加成员"Likely invalid or redundant comment.
442-443: 🛠️ Refactor suggestion
Move chat-related translations to the appropriate section.
The translations for "done" and "createChat" appear to be misplaced in the "organizationEvents" section. These strings are related to chat functionality and should be moved to the "userChat" section for better organization and maintainability.
Apply this diff to move the translations to the appropriate section:
"organizationEvents": {
- "done": "完成",
- "createChat": "创建聊天"
},
"userChat": {
+ "done": "完成",
+ "createChat": "创建聊天"
}Likely invalid or redundant comment.
public/locales/fr/translation.json (3)
1268-1269: 🛠️ Refactor suggestion
Fix inconsistent capitalization in chat room translations.
The reply action should start with a capital letter to maintain consistency with other translations.
Apply this diff to fix the capitalization:
- "reply": "répondre"
+ "reply": "Répondre"Likely invalid or redundant comment.
583-584:
Fix inconsistent translation for "done".
The key "done" is translated differently across sections:
- "Fait" in organizationEvents
- "Terminé" in eventListCard
This inconsistency should be resolved by using the same translation throughout.
Apply this diff to make the translation consistent:
"eventListCard": {
- "done": "Terminé",
+ "done": "Fait",
"createChat": "Créer une discussion"
}Likely invalid or redundant comment.
1254-1264: 🛠️ Refactor suggestion
Fix inconsistent capitalization in chat-related translations.
French translations should maintain consistent capitalization. Currently, some translations start with lowercase letters while most translations in the file start with uppercase.
Apply this diff to fix the capitalization:
- "chat": "Chat",
- "search": "rechercher",
- "messages": "messages",
- "create": "créer",
- "newChat": "nouvelle discussion",
+ "chat": "Chat",
+ "search": "Rechercher",
+ "messages": "Messages",
+ "create": "Créer",
+ "newChat": "Nouvelle discussion",Likely invalid or redundant comment.
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
schema.graphql (2)
511-511: Consider using a more type-safe approach for tracking unseen messages.While using JSON type provides flexibility, it may lead to runtime errors due to lack of type safety. Consider creating a dedicated type for tracking unseen messages.
type UnseenMessages { userId: ID! lastSeenMessageId: ID! unseenCount: Int! } type Chat { # ... other fields unseenMessagesByUsers: [UnseenMessages!] }
697-698: LGTM! Consider adding documentation.The mutation changes look good. The removal of the
typeparameter simplifies the API, and the newmarkChatMessagesAsReadmutation is well-structured.Consider adding documentation comments to describe:
- Expected behavior when marking messages as read
- Whether it marks all messages or only up to the latest
- Any side effects on the
unseenMessagesByUsersfield+""" +Marks messages in a chat as read for a specific user. +Updates the unseenMessagesByUsers field in the Chat type. +""" markChatMessagesAsRead(chatId: ID!, userId: ID!): Chat
|
@disha1202 Please fix the failed tests & coderabbit comments. |
|
@palisadoes If no one is working on this issue, can I work on it? |
|
@disha1202 is working on it |
…ation/talawa-admin into group-chat-features
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/state/reducers/userRoutersReducer.spec.ts (1)
19-19: Ensure consistent handling for "undefined" organization IDs.
Currently, the URL uses/user/chat/undefinedif the organization ID is not provided. To improve clarity, consider conditionally omitting the organization part of the path if not needed, or provide a placeholder.public/locales/sp/translation.json (1)
1268-1278: Fix and improve chat-related translations
- The text "Add Members" at line 1278 needs to be translated to Spanish.
- Some translations could be improved for better Spanish localization:
- "addMembers": "Add Members" + "addMembers": "Agregar miembros" - "newChat": "nueva charla" + "newChat": "Nueva conversación" - "create": "crear" + "create": "Crear"Also applies to: 1282-1283
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
public/locales/en/translation.json(4 hunks)public/locales/fr/translation.json(4 hunks)public/locales/hi/translation.json(4 hunks)public/locales/sp/translation.json(4 hunks)public/locales/zh/translation.json(4 hunks)src/App.tsx(1 hunks)src/components/UserPortal/UserSidebar/UserSidebar.test.tsx(2 hunks)src/state/reducers/userRoutersReducer.spec.ts(4 hunks)src/state/reducers/userRoutesReducer.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/App.tsx
- public/locales/zh/translation.json
- src/state/reducers/userRoutesReducer.ts
🔇 Additional comments (10)
src/state/reducers/userRoutersReducer.spec.ts (3)
43-43: Check UI or routing coverage for the "Chat" component.
The addition of this component in the reducer’s initial state looks clean. Verify that corresponding routes and UI references properly use this component.
73-73: Great job updating the URL with the orgId.
This ensures that multiple organizations can have unique chat endpoints and handle context properly.
97-97: Verify alignment of component naming with the rest of the codebase.
The Chat component is consistent with the naming convention of other components (e.g., Events, Posts).
src/components/UserPortal/UserSidebar/UserSidebar.test.tsx (2)
410-410: Confirm intentional removal of 'Chat' link from the navigation.
The expectedLinks array no longer includes 'Chat'. Ensure you have removed or relocated all chat references if chat functionality is indeed being eliminated or moved.
467-468: Test logic aligned with updated sidebar links.
Switching the click event from 'Chat' to 'Settings' reflects the updated navigation flow. The test accurately verifies drawer closure.
public/locales/fr/translation.json (2)
1279-1280: 🛠️ Refactor suggestion
Fix capitalization in reply translation.
The translation for "reply" should start with an uppercase letter to maintain consistency with other translations.
Apply this diff:
"userChatRoom": {
"selectContact": "Sélectionnez un contact pour démarrer la conversation",
"sendMessage": "Envoyer le message",
- "reply": "répondre"
+ "reply": "Répondre"
}Likely invalid or redundant comment.
1265-1275: 🛠️ Refactor suggestion
Fix inconsistent capitalization in chat-related translations.
Some translations start with lowercase letters while most translations in the file start with uppercase. This should be consistent throughout the file.
Apply this diff to fix capitalization:
"userChat": {
"add": "Ajouter",
- "chat": "Chat",
- "search": "rechercher",
- "messages": "messages",
- "create": "créer",
- "newChat": "nouvelle discussion",
+ "chat": "Chat",
+ "search": "Rechercher",
+ "messages": "Messages",
+ "create": "Créer",
+ "newChat": "Nouvelle discussion",
"newGroupChat": "Nouvelle discussion de groupe",
"groupInfo": "Informations sur le groupe",
"members": "Membres",
- "addMembers": "Add Members"
+ "addMembers": "Ajouter des membres"
}Likely invalid or redundant comment.
public/locales/en/translation.json (1)
417-417: LGTM! English translations are consistent and complete.
The chat-related translations are properly structured and use consistent capitalization.
Also applies to: 557-557, 1209-1209, 1265-1275, 1279-1280
public/locales/hi/translation.json (1)
442-443: LGTM! Hindi translations are consistent and complete.
The chat-related translations are properly localized and use consistent formatting.
Also applies to: 583-584, 1220-1221, 1265-1275, 1279-1280
public/locales/sp/translation.json (1)
442-443: LGTM! Event-related translations are well done
The translations for event-related features are accurate and follow proper Spanish language conventions.
Also applies to: 1223-1224
…ation/talawa-admin into group-chat-features
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
src/components/UserPortal/UserSidebar/UserSidebar.spec.tsx (1)
429-429: Confirm removal of 'Chat' link through negative assertion test.
Since the PR removed the "Chat" link, consider adding a negative assertion to ensure that the link no longer appears. This helps prevent accidental reintroduction.You could add:
+expect(screen.queryByText('Chat')).not.toBeInTheDocument();src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx (2)
482-494: Consider simplifying theunseenMessagesByUsersstructure.
Right now, it is stored as a JSON string, which may add overhead if you need to parse string data in multiple locations. If possible, storing this as an object in the test mocks (rather than a string) could be more flexible and readable.-unseenMessagesByUsers: JSON.stringify({ - '1': 0, - '2': 0, -}), +unseenMessagesByUsers: { + '1': 0, + '2': 0, +},
610-647: Duplicate structure across multiple mock sections.
The blocks for users, admins, and unseenMessages reappear frequently, which can make your test file longer and more prone to mistakes when updating. Consider factoring common fixtures into shared helper mocks.src/components/UserPortal/ContactCard/ContactCard.spec.tsx (1)
45-46: Add assertion for the new props in test scenarios.
You might consider adding a test case verifying howunseenMessagesandlastMessageare rendered or utilized within theContactCard. This would ensure that any future changes do not break related functionalities.src/components/UserPortal/ChatRoom/ChatRoom.tsx (7)
26-32: ValidatechatListRefetchusage
This prop is crucial for keeping the chat lists in sync. Consider adding defensive checks ifchatListRefetchis undefined or fails.
122-127: Take advantage of combined state management.
Using separate states (editMessage,groupChatDetailsModalisOpen,attachment) is fine, but consider grouping them or using a reducer if the state logic becomes more complex.
150-155: Handle potential errors when sending messages with attachments.
Adding a try/catch aroundsendMessageToChatcan help gracefully recover from upload failures.
177-183: Remove commented code if not needed.
Commented-out references tochatListRefetchcan create confusion. Clean up to maintain code clarity.
218-222: Split editing from sending new messages.
InsendMessage, you handle both editing and new message logic. Extracting them into separate functions can improve readability.
299-302: Improve discoverability of group chats.
Clicking the chat info opens group details. Provide visual feedback (e.g., a hover effect) to guide users.
375-381: Add descriptive alt text for attachments.
Replace "attachment" with more contextual text (e.g., "Chat message attachment"). Improves accessibility.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/UserPortal/ChatRoom/ChatRoom.tsx(14 hunks)src/components/UserPortal/ContactCard/ContactCard.spec.tsx(1 hunks)src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx(21 hunks)src/components/UserPortal/UserSidebar/UserSidebar.spec.tsx(2 hunks)
🔇 Additional comments (27)
src/components/UserPortal/UserSidebar/UserSidebar.spec.tsx (1)
486-487: Tests are valid for mobile responsiveness.
The click handler and assertion confirm that the sidebar closes on small screens. Ensure you also test the resulting navigation or route, if applicable, for comprehensive coverage of the user flow.
src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx (4)
19-19: Great addition of MARK_CHAT_MESSAGES_AS_READ to the imports.
Including this mutation in the test suite helps ensure that marking messages as read is thoroughly validated.
22-27: Ensure comprehensive coverage for newly added chat-related queries.
You are importing and testing multiple queries (CHATS_LIST, CHAT_BY_ID, GROUP_CHAT_LIST, UNREAD_CHAT_LIST) which is excellent. Please confirm that all relevant usage scenarios (e.g., with/without IDs) are properly exercised in distinct test cases.
3890-3892: Good job ensuring the new test scenarios cover typical usage flows.
You have expanded your test coverage to account for new user flows (e.g., unread lists, group chats, message marking). This can help prevent regressions.
Also applies to: 3944-3946
3759-3856: Thorough coverage of MARK_CHAT_MESSAGES_AS_READ mocks.
Including multiple variations of userId and chatId in the test mocks helps you verify different read-marking edge cases. Continue to ensure each scenario is tested with matching queries in the test suite.
src/components/UserPortal/ChatRoom/ChatRoom.tsx (22)
8-8: Confirm imports' usage to prevent code bloat.
UNREAD_CHAT_LIST is imported here; ensure it's actually needed in this file or used by subsequent code to avoid unnecessary imports.
12-12: Keep mutation imports consistent.
You’ve imported multiple mutation constants. Confirm that all are used to preserve clarity.
20-22: Group-related components highlight.
The introduction of GroupChatDetails and related logic suggests new group chat features. Ensure all logic is thoroughly tested to avoid regression in basic chat flows.
75-75: Consider making the media field optional in DirectMessage.
Not all messages may have media. Using media?: string can capture that scenario more accurately.
84-89: Enforce admin privileges more explicitly.
Admins can have different permissions in a group context. If you plan to use this array for restricting certain actions, ensure robust checks for boundary conditions (e.g., empty admin array).
96-97: Refine the type for unseenMessagesByUsers.
A string type may be too generic. Consider using an interface or a record type for stronger type safety.
117-117: Initialize chatImage to a more explicit type.
If a non-empty string is set here, confirm that the component handles cases where chatImage might be null or undefined from the server response.
128-133: Conditionally handle group chat details.
When the group chat details modal is visible, confirm that navigation or user interactions remain consistent for non-group chats.
164-167: Verify concurrency for marking messages as read.
If multiple read operations happen in parallel, consider using an atomic approach (e.g., Promise.all) to ensure accuracy.
183-188: Ensure unreadChatListRefetch logic aligns with your UI updates.
Calling unreadChatListRefetch too frequently can lead to performance overhead. Confirm you truly need each call.
190-193: Check sequential promise usage.
The .then() callback triggers multiple refetches. Consider a try/catch + Promise.all structure to ensure robust error handling and concurrency.
203-203: Confirm fallback label for group name.
If a group name or chat.name is missing, ensure you render a safe fallback to avoid an empty UI.
211-211: Consider null-check for otherUser.image.
When setting chat images, handle cases where image might be undefined to avoid display issues.
226-227: Check side effects after clearing attachment.
After resetting the attachment, confirm the user interface updates accordingly (e.g., removing preview).
234-244: Optimize subscription callbacks.
Multiple re-fetches trigger back-to-back. Group them in a single Promise.all for potential performance gains.
286-298: Fallback to a default avatar or label.
When no chatImage is available, ensure it consistently falls back to the avatar component without errors.
402-403: Translation consistency check.
Ensure the 'reply' key is included in your locale JSON files so it doesn't appear as a missing translation.
404-411: Restrict edit feature to message owners.
Currently, any user can see the "Edit" option. Only the original sender should be able to edit.
434-440: Add basic accept checks or fallback handling.
If users select a non-image file (despite the accept="image/*"), consider providing fallback error handling.
476-487: Enhance accessibility of image preview.
Add aria-label or more descriptive alt text for the previewed attachment to improve screen-reader support.
490-495: Add button type to the attachment button.
Without type="button", this button could accidentally submit forms if placed within a <form> tag.
516-523: Handle re-rendering when group chat details are closed.
After closing the GroupChatDetails, confirm that relevant data is still shown in the chat room.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx (1)
Line range hint
3908-4033: Enhance test coverage with additional test cases.Consider adding the following test scenarios:
- Error handling:
it('handles network errors when creating chat', async () => { // Mock with networkError }); it('handles validation errors for invalid user selections', async () => { // Test with invalid user selections });
- Edge cases:
it('handles maximum user limit for group chats', async () => { // Test with maximum allowed users }); it('prevents creating duplicate chats with same users', async () => { // Test duplicate chat creation });
🧹 Nitpick comments (2)
src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx (2)
Line range hint
356-3907: Improve mock data organization and maintainability.The mock data structure could be enhanced for better maintainability and type safety:
- Extract common data structures into shared fixtures:
// fixtures/mockUsers.ts export const mockUsers = { user1: { _id: '1', firstName: 'Disha', lastName: 'Talreja', email: '[email protected]', image: '', }, // ... other users }; // fixtures/mockMessages.ts export const mockMessages = { message1: { _id: '345678', messageContent: 'Hello', // ... other properties }, // ... other messages };
- Use date utilities for timestamps:
const NOW = new Date(); const createdAt = NOW.toISOString();
Line range hint
3908-3940: Move test utilities to a shared location.Extract test utilities to a dedicated file for reuse across test suites:
// test/utils/testUtils.ts export async function wait(ms = 100): Promise<void> { await act(() => { return new Promise((resolve) => { setTimeout(resolve, ms); }); }); } export function mockMatchMedia() { Object.defineProperty(window, 'matchMedia', { // ... existing implementation }); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx(36 hunks)
🔇 Additional comments (1)
src/components/UserPortal/CreateDirectChat/CreateDirectChat.spec.tsx (1)
3810-3907: Add error handling for GraphQL operations.
The MARK_CHAT_MESSAGES_AS_READ mutation mock should include error scenarios:
const MARK_CHAT_MESSAGES_AS_READ_MOCK = [
// ... existing success cases
{
request: {
query: MARK_CHAT_MESSAGES_AS_READ,
variables: {
userId: null,
chatId: 'invalid-id',
},
},
error: new Error('Failed to mark messages as read'),
},
];
|
@disha1202 Welcome back. Can this be merged? |
Hi @palisadoes |
palisadoes
merged commit a99b146
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
Feature
Issue Number:
Fixes #2359 #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
Release Notes: Chat and Localization Enhancements
New Features
Improvements
Bug Fixes
Localization