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
WalkthroughThe pull request introduces extensive modifications across various files, focusing on enhancing chat functionalities within the application. Key changes include the addition of new translation keys for multiple languages, updates to the GraphQL schema for chat-related mutations and queries, and adjustments to the routing structure for chat components. Additionally, new UI elements and styles are implemented to improve the user experience in managing group chats, while the testing files are updated to reflect these changes. 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 #2360 +/- ##
===========================================
- Coverage 98.04% 97.71% -0.33%
===========================================
Files 249 250 +1
Lines 7197 7351 +154
Branches 2066 2090 +24
===========================================
+ Hits 7056 7183 +127
- Misses 131 158 +27
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚨 Try these New 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: 42
🧹 Outside diff range and nitpick comments (32)
src/components/UserPortal/ContactCard/ContactCard.module.css (2)
40-48: Consider adding hover states for better interactivity.While the styling for messages and notification badges is good, adding hover states would improve user experience.
Consider adding these styles:
.lastMessage { margin-bottom: 0; font-size: 14px; color: rgba(163, 163, 163, 0.839); + transition: color 0.2s ease; } +.contact:hover .lastMessage { + color: rgba(163, 163, 163, 1); +} .unseenMessagesCount { border-radius: 50%; + transition: transform 0.2s ease; } +.unseenMessagesCount:hover { + transform: scale(1.1); +}
Let me analyze the color contrast in the ContactCard.module.css file. The key colors used are:
- Grey text:
color: grey- Background colors:
background-color: rgba(196, 255, 211, 0.3)(light green)background-color: white- Last message color:
color: rgba(163, 163, 163, 0.839)(light grey)Color contrast issues identified:
- The grey text (
color: grey) on white background has a contrast ratio of approximately 3.5:1, which fails WCAG AA standards for normal text (minimum 4.5:1)- The light grey last message color (
rgba(163, 163, 163, 0.839)) on white background has a contrast ratio of approximately 3:1, which fails WCAG AA standardsRecommended fixes:
- Use a darker grey color like
#6B6B6Bor#595959for the grey text to achieve at least 4.5:1 contrast ratio- For the last message text, use a darker grey like
#737373to meet accessibility standards🔗 Analysis chain
Line range hint
1-48: Verify color contrast ratios for accessibility.The grey colors used in the CSS should be tested to ensure they meet WCAG accessibility guidelines for color contrast.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for color definitions to verify contrast ratios rg -n "color:|background-color:" --type cssLength of output: 130852
src/state/reducers/userRoutesReducer.ts (1)
59-59: Consider extending route structure for advanced chat features.The current route structure
user/chat/{orgId}might need to be enhanced to support the new chat features:
- Event-specific group chats might need a different route pattern (e.g.,
user/chat/{orgId}/event/{eventId})- Direct messages vs group chats might need distinct routes
- Chat filtering functionality might require query parameter support
Consider defining additional component entries or subTargets to handle these specialized chat routes. For example:
{ name: 'Chat', comp_id: 'chat', component: 'Chat', subTargets: [ { name: 'Event Chats', comp_id: 'event-chat', component: 'EventChat', icon: 'event' // Optional: for navigation clarity }, { name: 'Direct Messages', comp_id: 'direct-messages', component: 'DirectChat', icon: 'message' } ] }src/state/reducers/userRoutersReducer.test.ts (1)
60-60: LGTM! Consider adding more test cases for chat-specific routing scenarios.The UPDATE_TARGETS test correctly verifies the basic route update behavior for the Chat feature. However, given that this is a new feature with potentially complex routing requirements (e.g., direct messages vs. group chats, event-specific chats), consider adding more test cases.
Consider adding these test cases:
- Invalid orgId handling
- Navigation between different chat types (direct/group/event)
- Chat-specific route parameters validation
Also applies to: 78-78
src/components/UserPortal/ContactCard/ContactCard.test.tsx (1)
35-36: Consider making unseenMessages optional with a default value.The
unseenMessagesprop is hardcoded to 2 in the default props. Since not all contacts may have unseen messages, consider making it optional or defaulting to 0.- unseenMessages: 2, + unseenMessages: 0,src/screens/UserPortal/Chat/Chat.module.css (2)
2-3: Remove commented code and consolidate height declarations.The height adjustments look good, but there's unnecessary commented code that should be removed for better maintainability.
Apply this diff to clean up the code:
.containerHeight { - /* padding: 1rem 1.5rem 0 calc(300px); */ - /* height: 100vh; */ height: 88vh; } @media (max-width: 820px) { .containerHeight { - /* height: 100vh; */ - /* padding: 2rem; */ }Also applies to: 35-35, 191-192
220-240: Consider using CSS variables for colors.The filter button styles look good and align well with the PR objective to implement chat filtering. However, consider using CSS variables for colors to maintain consistency and support theming.
Apply this diff to improve maintainability:
.filterButton { border-radius: 14px; padding: 5px 10px; background-color: white; - color: #a5a5a5; + color: var(--text-secondary); border: none; - border: 1px solid #a5a5a5; + border: 1px solid var(--border-color); } .selectedBtn, .filterButton:hover { - border: 1px solid #98e0b6; - background-color: #98e0b6; + border: 1px solid var(--primary-light); + background-color: var(--primary-light); - color: rgb(255, 255, 255); + color: var(--text-light); }src/components/UserPortal/ChatRoom/ChatRoom.module.css (1)
7-9: Consider enhancing input field styling for better user experience.The input styling could be improved with:
- Border definition for visual separation
- Focus/hover states for better interactivity
- Input padding for text spacing
.sendMessageInput { background-color: white; border-radius: 12px 0px 0px 12px; + border: 1px solid #e0e0e0; + padding: 8px 12px; + transition: border-color 0.2s ease; } +.sendMessageInput:focus { + outline: none; + border-color: #c4ffd3; +}src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (2)
195-195: Good use of translation, but other text needs internationalization too.While the "add" button text is correctly translated, there are still hardcoded strings in the component that should use the translation system.
Apply these changes for consistency:
- <Modal.Title>{'Chat'}</Modal.Title> + <Modal.Title>{t('chat')}</Modal.Title> - placeholder="searchFullName" + placeholder={t('searchFullName')} - <StyledTableCell align="center">{'user'}</StyledTableCell> + <StyledTableCell align="center">{t('user')}</StyledTableCell>
Line range hint
77-93: Consider enhancing error handling and user feedback.The chat creation logic could benefit from some improvements:
- Add loading state during chat creation
- Implement error handling for failed operations
- Consider debouncing the search input to prevent excessive API calls
Here's a suggested implementation:
const [isCreating, setIsCreating] = useState(false); const handleCreateDirectChat = async (id: string): Promise<void> => { setIsCreating(true); try { await createChat({ variables: { organizationId, userIds: [userId, id], isGroup: false, }, }); await chatsListRefetch(); toggleCreateDirectChatModal(); } catch (error) { // Show error notification to user console.error('Failed to create chat:', error); } finally { setIsCreating(false); } };And update the button:
<Button onClick={() => handleCreateDirectChat(userDetails.user._id)} data-testid="addBtn" + disabled={isCreating} > - {t('add')} + {isCreating ? t('creating') : t('add')} </Button>src/App.tsx (2)
189-189: Remove redundant commented code.There are two instances of commented-out chat routes. Since we're moving to the new organization-specific chat route structure, these commented lines should be removed to maintain code cleanliness.
- {/* <Route path="/user/chat" element={<Chat />} /> */} <Route element={<UserScreen />}> // ... other routes ... </Route> - {/* <SecuredRouteForUser path="/user/chat" component={Chat} /> */}Also applies to: 206-206
Line range hint
191-203: LGTM: Improved route organization under UserScreen.The new chat route is properly organized under the UserScreen component along with other organization-specific routes. This structure:
- Maintains consistent routing patterns
- Supports organization-specific chat contexts
- Aligns with the application's multi-tenant architecture
Consider documenting the routing structure in the project's architecture documentation to help maintain consistency as new features are added.
src/screens/UserPortal/Events/Events.tsx (2)
396-410: Consider enhancing the chat creation toggle UXWhile the implementation is correct, consider these improvements:
- Add a tooltip explaining what creating a chat means for the event
- Group this toggle with related switches in the
checkboxdivsection aboveHere's how you could enhance the implementation:
- <div> - <div className={styles.dispflex}> - <label htmlFor="createChat">{t('createChat')}?</label> - <Form.Switch - className="me-4" - id="chat" - type="checkbox" - data-testid="createChatCheck" - checked={createChatCheck} - onChange={(): void => - setCreateChatCheck(!createChatCheck) - } - /> - </div> - </div> + <div className={styles.checkboxdiv}> + <div className={styles.dispflex}> + <label htmlFor="createChat" title={t('createChatTooltip')}>{t('createChat')}?</label> + <Form.Switch + className="ms-2 mt-3" + id="chat" + type="checkbox" + data-testid="createChatCheck" + checked={createChatCheck} + onChange={(): void => setCreateChatCheck(!createChatCheck)} + /> + </div> + </div>
139-139: Consider enhancing error loggingWhile adding console.error is helpful for development, consider using a proper logging service for production environments.
Consider implementing a logging utility:
// utils/logger.ts export const logger = { error: (message: string, error: unknown): void => { console.error(message, error); // Add integration with your preferred logging service } };src/screens/OrganizationEvents/OrganizationEvents.tsx (1)
74-74: Add tests for chat creation functionalityThe new chat creation feature should be covered by tests:
- Test the checkbox state changes
- Test the integration with event creation
- Verify chat creation in the event creation flow
Would you like me to help generate the test cases for this new functionality?
Also applies to: 479-491
src/screens/UserPortal/Events/Events.test.tsx (2)
Line range hint
192-434: Reduce code duplication in mock requestsThere are multiple identical mock requests for CREATE_EVENT_MUTATION that could be consolidated using a helper function or shared configuration object.
Consider refactoring like this:
const createEventMockBase = { query: CREATE_EVENT_MUTATION, result: { data: { createEvent: { _id: '2', }, }, }, }; const createEventMock = (variables) => ({ ...createEventMockBase, variables: { title: 'testEventTitle', description: 'testEventDescription', location: 'testEventLocation', isPublic: true, recurring: false, isRegisterable: true, createChat: false, organizationId: '', ...variables, }, }); const MOCKS = [ // ... other mocks ... createEventMock({ startDate: dayjs(new Date()).format('YYYY-MM-DD'), endDate: dayjs(new Date()).format('YYYY-MM-DD'), allDay: true, startTime: null, endTime: null, }), // ... other variations ... ];
555-557: Enhance createChat checkbox test coverageWhile the basic toggle functionality is tested, consider adding more test cases:
- Verify the createChat value is correctly passed to the mutation
- Test different combinations of createChat with other event settings
- Add negative test cases
schema.graphql (1)
766-767: LGTM! Consider organizing chat-related types and mutations together.The unified Chat type is a good design decision. However, consider grouping all chat-related types and mutations together in the schema for better organization and maintainability.
Consider adding descriptions to the Chat type and its fields to improve schema documentation:
""" Represents a chat conversation that can be either a direct message or a group chat """ type Chat { _id: ID! """ Indicates whether this is a group chat (true) or direct message (false) """ isGroup: Boolean! # Add similar descriptions for other fields }src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (2)
Line range hint
3544-3569: Improve test setup and cleanup practicesThe current test setup could be enhanced for better reliability and maintainability:
- Move global mocks to a separate setup file
- Add proper cleanup after tests
- Use more reliable wait strategies
Example improvements:
// test-setup.ts export const setupTests = () => { beforeAll(() => { window.HTMLElement.prototype.scrollIntoView = jest.fn(); window.matchMedia = jest.fn().mockImplementation(/* ... */); }); afterEach(() => { jest.clearAllMocks(); }); afterAll(() => { jest.restoreAllMocks(); }); }; // Improved wait utility const waitForElement = async (testId: string) => { return await waitFor(() => { expect(screen.getByTestId(testId)).toBeInTheDocument(); }); };
Line range hint
3632-3673: Enhance code quality with TypeScript and constantsConsider improving code quality and maintainability:
- Define constants for test IDs
- Add TypeScript interfaces for mock data
- Use consistent async/await patterns
Example improvements:
// constants.ts export const TEST_IDS = { DROPDOWN: 'dropdown', NEW_DIRECT_CHAT: 'newDirectChat', SUBMIT_BUTTON: 'submitBtn', SEARCH_USER: 'searchUser', ADD_BUTTON: 'addBtn' } as const; // types.ts interface MockUser { _id: string; firstName: string; lastName: string; email: string; image: string; } interface MockChat { _id: string; isGroup: boolean; creator: MockUser; // ... other properties } // Improved test test('create new direct chat', async () => { // Use constants const dropdown = await screen.findByTestId(TEST_IDS.DROPDOWN); // Consistent async/await await fireEvent.click(dropdown); const newDirectChatBtn = await screen.findByTestId(TEST_IDS.NEW_DIRECT_CHAT); await fireEvent.click(newDirectChatBtn); // ... rest of the test });src/screens/UserPortal/Chat/Chat.test.tsx (3)
Line range hint
3989-4003: Enhance test description and assertions.The test case "Screen should be rendered properly" lacks specific assertions about what constitutes proper rendering. Consider adding explicit assertions for key UI elements.
test('Should render chat screen with all required components', async () => { // ... existing setup code ... await wait(); // Add specific assertions expect(screen.getByText('Messages')).toBeInTheDocument(); expect(screen.getByTestId('contactCardContainer')).toBeInTheDocument(); // Add more assertions for other key UI elements });
Line range hint
3973-3977: Consider making the wait timeout configurable.The
waitfunction uses a hardcoded timeout of 100ms. Consider making this configurable or using a more reliable way to wait for component updates.const DEFAULT_WAIT_MS = 100; async function wait(ms: number = DEFAULT_WAIT_MS): Promise<void> { await act(() => { return new Promise((resolve) => { setTimeout(resolve, ms); }); }); }
Line range hint
4056-4098: Add error scenario tests.The test case for creating a new direct chat only tests the happy path. Consider adding tests for error scenarios.
Add test cases for error scenarios:
test('Should handle errors when creating new direct chat', async () => { const errorMock = { request: { query: CREATE_DIRECT_CHAT, variables: { /* ... */ }, }, error: new Error('Failed to create chat'), }; render(/* ... with errorMock ... */); // Test error handling const newDirectChatBtn = await screen.findByTestId('newDirectChat'); fireEvent.click(newDirectChatBtn); // Verify error message is displayed expect(await screen.findByText('Failed to create chat')).toBeInTheDocument(); });src/screens/UserPortal/Chat/Chat.tsx (3)
132-132: Simplify conditional checks using optional chainingYou can simplify your conditional statements by utilizing optional chaining (
?.). This makes the code more concise and readable.Apply this diff to refactor the conditionals:
if (filterType === 'all') { chatsListRefetch(); - if (chatsListData && chatsListData.chatsByUserId) { + if (chatsListData?.chatsByUserId) { setChats(chatsListData.chatsByUserId); } } else if (filterType === 'unread') { unreadChatListRefetch(); - if (unreadChatListData && unreadChatListData.getUnreadChatsByUserId) { + if (unreadChatListData?.getUnreadChatsByUserId) { setChats(unreadChatListData.getUnreadChatsByUserId); } } else if (filterType === 'group') { groupChatListRefetch(); - if (groupChatListData && groupChatListData.getGroupChatsByUserId) { + if (groupChatListData?.getGroupChatsByUserId) { setChats(groupChatListData.getGroupChatsByUserId); } } --- - if (chatsListData && chatsListData?.chatsByUserId.length) { + if (chatsListData?.chatsByUserId.length) { setChats(chatsListData.chatsByUserId); }Also applies to: 137-137, 144-144, 201-201
🧰 Tools
🪛 Biome
[error] 132-132: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Line range hint
227-288: Internationalize hard-coded strings for localizationStrings like
'All','Unread','Groups', and'Starred Messages'are hard-coded. For better localization support, use thetfunction fromreact-i18next.Apply this diff to internationalize strings:
<Dropdown.Item onClick={openCreateDirectChatModal} data-testid="newDirectChat" > - {t('newChat')} + {t('newChat')} </Dropdown.Item> <Dropdown.Item onClick={openCreateGroupChatModal} data-testid="newGroupChat" > - {t('newGroupChat')} + {t('newGroupChat')} </Dropdown.Item> <Dropdown.Item href="#/action-3"> - Starred Messages + {t('starredMessages')} </Dropdown.Item> --- <Button onClick={() => { setFilterType('all'); }} className={[ styles.filterButton, filterType === 'all' && styles.selectedBtn, ] .filter(Boolean) .join(' ')} > - All + {t('all')} </Button> <Button onClick={() => { setFilterType('unread'); }} className={[ styles.filterButton, filterType === 'unread' && styles.selectedBtn, ] .filter(Boolean) .join(' ')} > - Unread + {t('unread')} </Button> <Button onClick={() => { setFilterType('group'); }} className={[ styles.filterButton, filterType === 'group' && styles.selectedBtn, ] .filter(Boolean) .join(' ')} > - Groups + {t('groups')} </Button>Remember to add these keys to your translation files.
140-141: Remove unnecessaryconsole.logstatementsThere are
console.logstatements that may not be intended for production code. Consider removing them to clean up the console output.console.log(unreadChatListData);console.log(groupChatListData);Also applies to: 244-245
src/components/GroupChatDetails/GroupChatDetails.tsx (3)
332-333: Localize Placeholder Text in Search InputThe placeholder text
'searchFullName'is hardcoded and not localized. To maintain consistency and support internationalization, use thetfunction from theuseTranslationhook.Apply this diff:
<Form.Control type="name" id="searchUser" data-testid="searchUser" - placeholder="searchFullName" + placeholder={t('searchFullName')} autoComplete="off" className={styles.inputFieldModal} value={userName} onChange={(e): void => { const { value } = e.target; setUserName(value); }} />Ensure that
'searchFullName'is defined in your translation files under the appropriate key.
316-318: Localize Modal TitleThe modal title
'Chat'is hardcoded and not localized. To support internationalization, use thetfunction for the title.Apply this diff:
<Modal.Header closeButton data-testid="pluginNotificationHeader"> - <Modal.Title>{'Chat'}</Modal.Title> + <Modal.Title>{t('chat')}</Modal.Title> </Modal.Header>Ensure that
'chat'is defined in your translation files for proper localization.
355-357: Localize Table Header LabelsThe table header labels
'user'and'Chat'are hardcoded strings. To maintain consistency and support internationalization, they should be localized.Apply this diff:
<StyledTableCell>#</StyledTableCell> -<StyledTableCell align="center">{'user'}</StyledTableCell> -<StyledTableCell align="center">{'Chat'}</StyledTableCell> +<StyledTableCell align="center">{t('user')}</StyledTableCell> +<StyledTableCell align="center">{t('chat')}</StyledTableCell>Make sure
'user'and'chat'are available in your translation resources.src/components/UserPortal/ChatRoom/ChatRoom.tsx (3)
171-176: Remove obsolete commented-out codeThe commented-out
chatListRefetchquery appears to be obsolete sincechatListRefetchis now being passed as a prop. Removing this code can improve readability.Apply this diff to remove the unnecessary code:
- // const { refetch: chatListRefetch } = useQuery(CHATS_LIST, { - // variables: { - // id: userId, - // }, - // });
231-235: Use strict equality operator===for comparisonIn the subscription callback, use the strict equality operator
===to avoid unexpected type coercion.Apply this diff to update the comparison:
if ( messageSubscriptionData?.data.data.messageSentToChat && - messageSubscriptionData?.data.data.messageSentToChat.chatMessageBelongsTo['_id'] == props.selectedContact + messageSubscriptionData?.data.data.messageSentToChat.chatMessageBelongsTo['_id'] === props.selectedContact ) { await markChatMessagesAsRead(); chatRefetch(); } else {
374-380: Sanitize and validate media content before renderingWhen displaying user-uploaded media, ensure that the content is sanitized and validated to prevent security risks such as XSS attacks.
Consider implementing server-side validation and sanitization of media content. Additionally, handle cases where the media might fail to load by adding error handling to the image element:
{message.media && ( <img className={styles.messageAttachment} src={message.media} alt="attachment" + onError={(e) => { + e.currentTarget.src = 'path/to/placeholder-image.png'; + }} /> )}Replace
'path/to/placeholder-image.png'with an appropriate fallback image path.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/assets/svgs/add.svgis excluded by!**/*.svg
📒 Files selected for processing (30)
- 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 (2 hunks)
- src/GraphQl/Mutations/mutations.ts (2 hunks)
- src/GraphQl/Queries/PlugInQueries.ts (4 hunks)
- src/components/GroupChatDetails/GroupChatDetails.module.css (1 hunks)
- src/components/GroupChatDetails/GroupChatDetails.tsx (1 hunks)
- src/components/UserPortal/ChatRoom/ChatRoom.module.css (3 hunks)
- src/components/UserPortal/ChatRoom/ChatRoom.tsx (14 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 (20 hunks)
- src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (3 hunks)
- src/components/UserPortal/CreateGroupChat/CreateGroupChat.module.css (1 hunks)
- src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (10 hunks)
- src/components/UserPortal/UserSidebar/UserSidebar.tsx (0 hunks)
- src/screens/OrganizationEvents/OrganizationEvents.tsx (2 hunks)
- src/screens/UserPortal/Chat/Chat.module.css (4 hunks)
- src/screens/UserPortal/Chat/Chat.test.tsx (21 hunks)
- src/screens/UserPortal/Chat/Chat.tsx (7 hunks)
- src/screens/UserPortal/Events/Events.test.tsx (9 hunks)
- src/screens/UserPortal/Events/Events.tsx (4 hunks)
- src/state/reducers/userRoutersReducer.test.ts (4 hunks)
- src/state/reducers/userRoutesReducer.ts (1 hunks)
💤 Files with no reviewable changes (1)
- src/components/UserPortal/UserSidebar/UserSidebar.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/GroupChatDetails/GroupChatDetails.module.css
🧰 Additional context used
🪛 Biome
src/screens/UserPortal/Chat/Chat.tsx
[error] 132-132: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 144-144: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 201-201: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (46)
src/components/UserPortal/CreateGroupChat/CreateGroupChat.module.css (2)
11-16: LGTM! Clean flexbox implementation.The
.groupInfoclass follows best practices for centering content using flexbox.
18-20: LGTM! Clear spacing implementation.The
.groupImageclass provides appropriate spacing.src/components/UserPortal/ContactCard/ContactCard.module.css (2)
13-14: LGTM! Contact image styling follows best practices.The circular image styling with equal width and height ensures consistent presentation of contact avatars.
23-25: LGTM! Layout improvements enhance content organization.The flex container properties create a well-structured layout with proper spacing and alignment.
src/state/reducers/userRoutesReducer.ts (1)
59-59: LGTM! Verify route pattern compatibility.The Chat component entry follows the established pattern and will generate routes correctly. However, let's verify that the route pattern
user/chat/{orgId}is sufficient for the chat features being implemented.✅ Verification successful
Route pattern is compatible and correctly implemented
After examining the routing system, the chat route pattern
user/chat/{orgId}follows the established pattern used by other components in the user portal. The test files confirm that:
- The route generation works correctly with both undefined and specific orgIds
- The chat component is properly integrated into the navigation structure
- The route follows the same pattern as other user-facing features like Posts, Events, and People
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any other chat-related route patterns in the codebase # that might conflict with or complement this implementation # Search for chat-related route definitions or path constants rg -i 'chat.*route|route.*chat|path.*chat|chat.*path' --type ts --type tsx # Search for route-related files that might contain chat paths fd -e ts -e tsx routeLength of output: 481
Script:
#!/bin/bash # Let's examine the route-related files to understand the routing patterns # and check for any chat-related configurations # Check routesReducer.ts for any chat routes cat src/state/reducers/routesReducer.ts # Check SecuredRoute components for any chat-specific logic cat src/components/SecuredRoute/SecuredRoute.tsx cat src/components/UserPortal/SecuredRouteForUser/SecuredRouteForUser.tsx # Check the test files for any chat route test cases cat src/state/reducers/routesReducer.test.ts cat src/state/reducers/userRoutersReducer.test.tsLength of output: 20088
src/components/UserPortal/ContactCard/ContactCard.tsx (1)
4-4: LGTM! Appropriate use of Badge componentThe Badge component from react-bootstrap is a good choice for displaying notification counts.
src/state/reducers/userRoutersReducer.test.ts (1)
18-18: LGTM! New Chat route and component follow established patterns.The additions maintain consistency with existing route and component structures, using the same patterns for URLs and component definitions.
Let's verify the Chat component exists and matches the expected structure:
Also applies to: 36-36
✅ Verification successful
The Chat component exists and is properly implemented
The verification confirms that:
- The Chat component exists at
src/screens/UserPortal/Chat/Chat.tsx- It's a fully implemented React component with proper exports
- The component follows the expected structure and integrates with the routing system
The test's usage of
{ name: 'Chat', url: 'user/chat/undefined' }is valid and consistent with the codebase structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Chat component implementation exists # Test: Search for Chat component definition ast-grep --pattern 'const Chat = $$$' # Test: Search for Chat component export ast-grep --pattern 'export $$ Chat'Length of output: 77
Script:
#!/bin/bash # Let's try a broader search for Chat-related components and files # Search for any files containing "Chat" in their name fd -t f "chat|Chat" # Search for Chat-related imports or component definitions using ripgrep rg -i "chat" --type ts --type tsx -A 2 # Search for any exports containing Chat rg "export.*Chat" --type ts --type tsxLength of output: 1268
Script:
#!/bin/bash # Search for Chat component in the screens directory since we found Chat.tsx there cat "src/screens/UserPortal/Chat/Chat.tsx" # Also check the route configuration in the router file rg "Chat.*component" -A 2 --type typescriptLength of output: 12409
src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (2)
28-31: LGTM! Improved type safety.Good improvements to type safety:
- Changed
idfromanytostringfor better type specificity- Updated return type from
anytounknownwhich enforces proper type checking
8-8: Verify CREATE_CHAT mutation implementation.The consolidation of chat mutations looks good. The code correctly uses the new
CREATE_CHATmutation with theisGroupparameter set tofalsefor direct chats.Let's verify the mutation implementation:
✅ Verification successful
The CREATE_CHAT mutation is correctly implemented and used
The verification confirms that:
- The
CREATE_CHATmutation is properly defined insrc/GraphQl/Mutations/OrganizationMutations.tswith the correct parameters includingisGroup- The old
CREATE_DIRECT_CHATmutation has been completely removed from the codebase- The component correctly imports and uses the new consolidated mutation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify CREATE_CHAT mutation implementation and usage # Test: Check if the mutation is defined correctly and there are no leftover references to CREATE_DIRECT_CHAT # Check mutation definition rg -l "CREATE_CHAT.*mutation" # Verify no lingering references to old mutation rg "CREATE_DIRECT_CHAT"Length of output: 55
Script:
#!/bin/bash # Let's try a broader search to locate the mutation definitions and usages # Search for CREATE_CHAT in GraphQL mutations directory with context rg -p "CREATE_CHAT" src/GraphQl/Mutations/ # Search for any chat creation related mutations rg -p "mutation.*createChat" -g "*.{ts,tsx,graphql}" # Search for direct chat related code rg -p "DirectChat" -g "*.{ts,tsx,graphql}" # Check the specific import path mentioned cat src/GraphQl/Mutations/OrganizationMutations.ts || echo "File not found"Length of output: 9888
src/App.tsx (1)
196-196: Breaking Change: Chat route now requires orgId parameter.The chat route has been updated to require an organization ID parameter (
/user/chat/:orgId). This is a breaking change that affects existing chat functionality. Please ensure that:
- All existing chat navigation links in the application are updated to include the orgId parameter
- The Chat component handles the orgId parameter appropriately
- This change is documented in the PR description
Let's verify the chat route usage across the codebase:
✅ Verification successful
Let me try a different approach to search for chat route references and navigation patterns.
Let me check the content of these files to understand how the chat route is being used.
Based on the git history and code analysis, I can now provide the final response:
Breaking Change Confirmed: Chat Route Migration to Organization-Specific Routes
The changes are part of an intentional feature migration where chat functionality was moved from a global
/user/chatroute to organization-specific routes/user/chat/:orgId. The git history shows this was implemented in commit e125d96 with the commit message "feat: moved chat to organization & added functionality to share media in chat".
- The old route
/user/chatwas commented out and replaced with the new organization-specific route- The navigation code in
OrganizationNavbar.tsxalready uses the new format withorganizationId- This change is part of a larger feature enhancement to support organization-specific chat functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential broken chat route references echo "Searching for old chat route references..." rg -l "/user/chat[\"']" --type ts --type tsx echo "Searching for navigation to chat routes..." ast-grep --pattern 'navigate($_, "/user/chat")'Length of output: 316
Script:
#!/bin/bash # Search for chat route references with proper file extensions echo "Searching for chat route references..." rg -l "/user/chat" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts echo "Searching for navigation patterns..." rg "navigate.*user/chat" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts -A 2 echo "Searching for Link components with chat routes..." rg "Link.*to.*user/chat" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type tsx --type ts -A 2Length of output: 636
Script:
#!/bin/bash # Check the content of OrganizationNavbar for chat route usage echo "Content of OrganizationNavbar.tsx:" rg -A 5 -B 5 "/user/chat" src/components/UserPortal/OrganizationNavbar/OrganizationNavbar.tsx # Check if there are any recent changes to the chat route echo -e "\nGit history for chat route changes:" git log -p --grep="chat" src/App.tsxLength of output: 8037
src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (1)
1-2: LGTM! Type safety improvement and necessary imports added.The change from
anytounknownin the return type improves type safety. The new imports support the added image handling functionality.Also applies to: 20-22, 33-34
src/screens/UserPortal/Events/Events.tsx (2)
66-66: LGTM: State management for chat creationThe new state variable follows React conventions and has an appropriate default value.
119-121: Verify timezone handling for event timesThe addition of 'Z' suffix to time values forces UTC timezone. Ensure that:
- The backend properly handles UTC times
- The UI displays times in the user's local timezone
- All-day events are not affected by timezone conversions
✅ Verification successful
Let me analyze the timezone handling in the codebase based on the search results:
- The codebase uses
dayjslibrary for date/time handling- Found a dedicated timezone utility at
src/utils/timezoneUtils/dateTimeMiddleware.tsthat handles UTC conversion- The Events component properly formats dates using dayjs
- All-day events have null time values as shown in the original code
Let me check the timezone middleware implementation to verify proper handling:
Let me check how these timezone utilities are integrated with the Events component:
Based on the findings, I can now provide a final response:
Timezone handling is properly implemented
The addition of 'Z' suffix to time values is correct and safe because:
- The application has a robust timezone middleware (
dateTimeMiddleware.ts) that handles UTC conversions- The middleware is properly integrated in the Apollo client setup (confirmed in
src/index.tsx)- The middleware automatically:
- Converts local times to UTC before sending to the backend
- Converts UTC times back to local time when receiving responses
- All-day events correctly bypass timezone handling by setting times to null
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for timezone handling in related files rg -g '!*.test.*' -g '*.{ts,tsx}' "new Date\(|dayjs\(|moment\(|getTime\(|toISOString\(" ./srcLength of output: 37336
Script:
#!/bin/bash # Check timezone middleware implementation cat src/utils/timezoneUtils/dateTimeMiddleware.tsLength of output: 2983
Script:
#!/bin/bash # Check Apollo client setup and middleware integration rg -g '*.{ts,tsx}' "requestMiddleware|responseMiddleware" ./src # Check the Events component's GraphQL mutation ast-grep --pattern 'const [CREATE_EVENT_MUTATION, UPDATE_EVENT_MUTATION]' ./src/screens/UserPortal/Events/Events.tsxLength of output: 1355
src/GraphQl/Mutations/mutations.ts (2)
286-286: LGTM: Event chat creation parameter added correctly.The addition of the
createChatparameter toCREATE_EVENT_MUTATIONaligns with the PR objective to support dedicated group chats for events. The implementation correctly:
- Adds a required boolean parameter
- Includes it in the mutation input
Also applies to: 302-302
Line range hint
1-624: Verify the impact of removing CREATE_DIRECT_CHAT.The removal of
CREATE_DIRECT_CHATfrom exports suggests a change in the chat creation workflow. Let's verify there are no remaining references to this mutation.src/screens/OrganizationEvents/OrganizationEvents.tsx (2)
74-74: LGTM: State declaration follows conventionsThe state variable declaration follows the component's naming conventions and is appropriately initialized.
Line range hint
191-270: Add createChat to event mutation variablesThe event creation mutation should include the
createChatflag to fulfill the requirement of creating dedicated group chats for events.Add the createChat variable to the mutation:
const { data: createEventData } = await create({ variables: { title: formState.title, description: formState.eventdescrip, isPublic: publicchecked, recurring: recurringchecked, isRegisterable: registrablechecked, + createChat: createChatCheck, organizationId: currentUrl, startDate: dayjs(startDate).format('YYYY-MM-DD'), endDate: dayjs(endDate).format('YYYY-MM-DD'),Also, verify that the GraphQL mutation type supports this new field:
src/screens/UserPortal/Events/Events.test.tsx (2)
86-86: LGTM: Mock data correctly updated with createChat fieldThe addition of the
createChatboolean field to event mock data is consistent and properly implements the new group chat feature for events.Also applies to: 118-118, 169-169
564-566: LGTM: Test assertions follow best practicesThe updated assertions using
toHaveBeenCalledWithfollow Jest best practices and properly verify the success message.Also applies to: 609-611
schema.graphql (1)
766-767: Verify the impact of removing the message type field.The removal of the
typefield from ChatMessage might affect features that rely on message type differentiation (e.g., system messages, media messages, etc.).Run the following script to check for any code depending on message types:
public/locales/zh/translation.json (2)
399-400: LGTM: Consistent translations across event-related sections.The translations for "done" (完成) and "createChat" (创建聊天) are consistently implemented across event-related sections, maintaining a uniform user experience.
Also applies to: 535-536, 1120-1121
1178-1179: LGTM: Basic message interaction translations.The translations for sending messages (发信息) and replying (回复) are appropriate and consistent with Chinese language conventions.
public/locales/en/translation.json (2)
375-375: LGTM: Consistent "createChat" key across event sections.The "createChat" key has been consistently added to all event-related sections (
organizationEvents,eventListCard, anduserEvents), which maintains translation consistency for the event chat creation feature.Also applies to: 509-509, 1109-1109
1165-1174: LGTM: Comprehensive chat functionality translation keys.The new translation keys provide complete coverage for the chat features mentioned in the PR objectives:
- Group chat management: "newGroupChat", "groupInfo", "members"
- User interactions: "add", "create", "newChat"
- Message handling: "sendMessage", "reply"
Also applies to: 1178-1179
public/locales/hi/translation.json (4)
399-400: LGTM: Consistent translation keys across event-related sectionsThe addition of "done" and "createChat" translation keys is consistent across all event-related sections (organizationEvents, eventListCard, and userEvents), maintaining uniformity in the UI.
Also applies to: 535-536, 1120-1121
1165-1174: LGTM: Comprehensive chat-related translationsAdded essential translations for chat functionality including:
- Basic actions (add, create)
- Chat types (new chat, group chat)
- UI elements (contacts, search, messages)
- Group-specific features (group info, members)
These translations align well with the PR objectives for chat feature enhancements.
1178-1179: LGTM: Message interaction translationsAdded translations for core message interaction features:
- Send message functionality
- Reply capability
These additions support the PR objective of enhancing chat functionality.
Line range hint
1-1500: Verify translation consistency across localesThe translations look good, but we should verify that these new keys are consistently added across all language files to maintain i18n coverage.
✅ Verification successful
Based on the output, I can see that all the checked chat-related translation keys ("createChat", "newChat", "newGroupChat", "groupInfo", "sendMessage", "reply") are present across all locale files (en, es, fr, hi, zh). Each key search returned matches in all 5 language files, indicating consistent coverage.
Translation keys are properly synchronized across all locale files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the new chat-related keys exist in all locale files # Expected: The same keys should be present in all language files (en, es, fr, zh) echo "Checking translation keys across locale files..." # Check for chat-related keys in other locale files for key in "createChat" "newChat" "newGroupChat" "groupInfo" "sendMessage" "reply"; do echo "Checking key: $key" rg -l "\"$key\"" public/locales/ doneLength of output: 2236
public/locales/fr/translation.json (4)
399-400: LGTM! Translations are consistent.The French translations for "done" and "createChat" in the organizationEvents section are appropriate and align with the chat functionality requirements.
535-536: LGTM! Translations maintain consistency.The translations in eventListCard section mirror those in organizationEvents, maintaining consistency across the application.
1178-1179: LGTM! Message interaction translations are appropriate.The French translations for "sendMessage" and "reply" in the userChatRoom section are accurate and consistent with chat functionality.
1165-1174: Consider adding missing chat-related translations.While the core chat translations are present, some common chat-related terms might be missing compared to other language files:
- No translation for "typing..."
- No translation for "online/offline status"
- No translation for "group chat settings"
Let's verify if these translations exist in other language files:
public/locales/sp/translation.json (3)
399-400: LGTM! Consistent chat creation translations across sections.The Spanish translations for "done" and "createChat" are consistently implemented across organizationEvents, eventListCard, and userEvents sections.
Also applies to: 535-536, 1121-1122
1166-1175: LGTM! Comprehensive chat functionality translations.The Spanish translations for core chat functionality are properly implemented, including:
- Adding contacts
- Creating new chats and group chats
- Managing group information and members
1179-1180: LGTM! Chat room interaction translations.The Spanish translations for message sending and reply functionality are correctly implemented.
src/GraphQl/Mutations/OrganizationMutations.ts (3)
82-85: LGTM!The
ADD_USER_TO_GROUP_CHATmutation is correctly defined and aligns with the intended functionality.
98-104: LGTM!The
UPDATE_CHATmutation is well-defined and utilizes an input object, enhancing maintainability and scalability.
60-75: Ensure all usages ofCREATE_CHATare updated with new parametersThe
CREATE_CHATmutation now includes additional parameters:$isGroup,$name, and$image. Please verify that all client-side code invoking this mutation is updated to include these new parameters to prevent runtime errors.You can run the following script to find all usages of
CREATE_CHATand check if they have been updated:✅ Verification successful
Let me analyze the usages of
CREATE_CHATto check if they are properly updated with the new parameters.
All usages of
CREATE_CHATare properly updated with the new parametersBased on the codebase analysis:
CreateDirectChatcorrectly usesCREATE_CHATwithisGroup: falseand doesn't neednameorimageparameters as they are only required for group chatsCreateGroupChatcorrectly usesCREATE_CHATwith all new parameters:isGroup: true,name: title, andimage: selectedImage- Test files are also updated to include the new parameters in their mock data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of the `CREATE_CHAT` mutation and display surrounding code. # Test: Search for `CREATE_CHAT` in TypeScript and JavaScript files. rg -A 5 'CREATE_CHAT' --glob '*.{ts,tsx,js,jsx}'Length of output: 13308
Script:
#!/bin/bash # Let's check the actual implementation of CreateDirectChat and CreateGroupChat components # to see how they use the CREATE_CHAT mutation # Check CreateDirectChat implementation rg -A 10 "handleCreateDirectChat.*async.*" src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx # Check CreateGroupChat implementation rg -A 10 "createChat.*variables" src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsxLength of output: 485
Script:
#!/bin/bash # Let's check the implementation of CreateGroupChat component where the group chat is created # Search for any method that creates a group chat rg -B 5 -A 10 "createChat\(" src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx # Also check if there's a form or handler that collects group chat data rg -B 5 -A 10 "handleSubmit|onSubmit|handleCreate" src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsxLength of output: 1882
src/GraphQl/Queries/PlugInQueries.ts (6)
125-125: Addition ofmediafield tomessagesThe inclusion of the
mediafield in themessagesof theCHAT_BY_IDquery enhances the ability to retrieve media content associated with messages.
151-151: Addition ofimagefield tousersAdding the
imagefield to theusersin theCHAT_BY_IDquery allows for fetching user profile images, enriching the user data retrieved.
153-153: Inclusion ofunseenMessagesByUsersfieldThe
unseenMessagesByUsersfield added to theCHAT_BY_IDquery aids in tracking unread messages for users within a chat.
244-291:⚠️ Potential issuePossible issue with the
whereclause inCHATS_LISTqueryIn the
CHATS_LISTquery, thewhereclause uses theuserfield to filter byfirstName_containsandlastName_contains. Ensure that the fielduseris correct as per the schema. If the schema usesusers(plural) or requires different syntax for filtering arrays, the query should be adjusted accordingly.Apply the following diff to correct the field name and adjust the filter for array fields:
where: { name_contains: $searchString - user: { + users_some: { firstName_contains: $searchString lastName_contains: $searchString } }To confirm the correct field name in the schema, run this script:
#!/bin/bash # Description: Verify whether the `Chat` type has a `users` field and its correct usage # Test: Search for the `Chat` type definition and check for the `users` field # Expect: The `Chat` type should have a `users` field suitable for filtering rg "type Chat.*{" -A 20 schema.graphql | rg "users"
Line range hint
204-242: EnsureUNREAD_CHAT_LISTquery functions without parametersThe
UNREAD_CHAT_LISTquery is defined without parameters and fetches unread chats viagetUnreadChatsByUserId. Verify that the backend supports this operation without explicit parameters, relying on the user's session or authentication context.You can run the following script to verify the query's definition:
#!/bin/bash # Description: Check if `getUnreadChatsByUserId` is defined without required parameters # Test: Search for the `getUnreadChatsByUserId` query definition in the schema # Expect: The query should be defined without mandatory parameters rg "type Query.*getUnreadChatsByUserId\(" -A 5 schema.graphql
158-202: Verify parameter handling inGROUP_CHAT_LISTqueryThe
GROUP_CHAT_LISTquery does not accept any parameters and retrieves group chats usinggetGroupChatsByUserId. Please verify that the server-side implementation correctly handles this query without explicit user ID parameters, possibly by utilizing the authenticated user's context.Consider running the following script to confirm the server's support for this query without parameters:
✅ Verification successful
Based on the schema.graphql content, I can now generate the final response:
GROUP_CHAT_LISTquery is correctly implemented without parametersThe query is correctly designed as the schema shows that
getGroupChatsByUserIdquery field doesn't require any parameters. The server-side implementation uses the authenticated user's context, which is verified by:
- The presence of
@authdirective in the schema, indicating authentication support- The existence of
checkAuthquery that returns the authenticatedUser- The schema definition showing
getGroupChatsByUserIdwithout any required parameters🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if `getGroupChatsByUserId` is defined without required parameters # Test: Search for the `getGroupChatsByUserId` query definition in the schema # Expect: The query should be defined without mandatory parameters rg "type Query.*getGroupChatsByUserId\(" -A 5 schema.graphqlLength of output: 63
Script:
#!/bin/bash # Let's try different approaches to locate the GraphQL schema and resolver # Search for schema files fd "schema.*" --type f # Search for GraphQL type definitions containing getGroupChatsByUserId rg "getGroupChatsByUserId" --type ts -A 5 # Search for resolver implementation ast-grep --pattern 'getGroupChatsByUserId: async ($$$) => { $$$ }'Length of output: 15367
Script:
#!/bin/bash # Let's check the schema.graphql file content to see the query definition cat schema.graphql # Also search for any authentication context usage in resolvers rg "getGroupChatsByUserId.*context" --type ts -A 5Length of output: 30718
src/screens/UserPortal/Chat/Chat.tsx (1)
333-336: EnsureChatRoomcomponent receives necessary propsVerify that the
ChatRoomcomponent functions correctly with the updated props, especially with the inclusion ofchatListRefetch.Please confirm that
ChatRoomhandleschatListRefetchappropriately and that any required props are being passed.src/components/UserPortal/ChatRoom/ChatRoom.tsx (1)
212-221: Ensure editing messages handles attachments appropriatelyWhen editing a message, the
attachmentstate is cleared, but it's unclear if attachments can be edited alongside the message content. Verify if theEDIT_CHAT_MESSAGEmutation supports updating the media attachment.Please confirm whether the
editChatMessagemutation accepts amediaparameter. If it does, includemedia: attachmentin the variables:const [editChatMessage] = useMutation(EDIT_CHAT_MESSAGE, { variables: { messageId: editMessage?._id, messageContent: newMessage, + media: attachment, chatId: props.selectedContact, }, });Also, ensure that
setAttachment('');is called appropriately after editing.
| .chatImage { | ||
| height: 120px; | ||
| border-radius: 100%; | ||
| width: 120px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding responsive image dimensions.
The .chatImage class uses fixed dimensions which might not scale well across different screen sizes.
Consider using relative units or media queries:
.chatImage {
- height: 120px;
+ height: clamp(80px, 120px, 15vw);
border-radius: 100%;
- width: 120px;
+ width: clamp(80px, 120px, 15vw);
+ object-fit: cover;
}📝 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.
| .chatImage { | |
| height: 120px; | |
| border-radius: 100%; | |
| width: 120px; | |
| } | |
| .chatImage { | |
| height: clamp(80px, 120px, 15vw); | |
| border-radius: 100%; | |
| width: clamp(80px, 120px, 15vw); | |
| object-fit: cover; | |
| } |
| .editImgBtn { | ||
| padding: 2px 6px 6px 8px; | ||
| border-radius: 100%; | ||
| background-color: white; | ||
| border: 1px solid #959595; | ||
| color: #959595; | ||
| outline: none; | ||
| position: relative; | ||
| top: -40px; | ||
| left: 40px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving button accessibility and maintainability.
The .editImgBtn class has some potential issues:
- Removing outline can harm keyboard navigation accessibility
- Hardcoded colors should use CSS variables for better maintainability
Consider these improvements:
.editImgBtn {
padding: 2px 6px 6px 8px;
border-radius: 100%;
background-color: white;
- border: 1px solid #959595;
- color: #959595;
- outline: none;
+ border: 1px solid var(--grey-2, #959595);
+ color: var(--grey-2, #959595);
+ outline: 0;
+ &:focus-visible {
+ outline: 2px solid var(--primary-color);
+ outline-offset: 2px;
+ }
position: relative;
top: -40px;
left: 40px;
}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
Enhance accessibility and code readability
The Badge component needs accessibility improvements, and the boolean conversion could be more readable.
- {!!props.unseenMessages && (
+ {Boolean(props.unseenMessages) && (
<Badge
+ aria-label={`${props.unseenMessages} unread messages`}
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> | |
| )} | |
| {Boolean(props.unseenMessages) && ( | |
| <Badge | |
| aria-label={`${props.unseenMessages} unread messages`} | |
| className={styles.unseenMessagesCount} | |
| > | |
| {props.unseenMessages} | |
| </Badge> | |
| )} |
| unseenMessages: number; | ||
| lastMessage: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making new properties optional
Since these properties represent chat-related data that might not always be available (e.g., for new contacts or group chats), consider making them optional to improve component reusability.
- unseenMessages: number;
- lastMessage: string;
+ unseenMessages?: number;
+ lastMessage?: string;📝 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.
| unseenMessages: number; | |
| lastMessage: string; | |
| unseenMessages?: number; | |
| lastMessage?: string; |
💡 Codebase verification
Additional test coverage needed for message-related features
The current test suite lacks specific assertions for the new unseenMessages and lastMessage properties, despite them being used in the UI:
- No tests verify the Badge component renders with correct count when
unseenMessages > 0 - No tests verify the Badge is not rendered when
unseenMessages = 0 - No tests verify the
lastMessagetext is displayed correctly - No tests verify different combinations of these properties
Suggested test cases to add:
- Test rendering with non-zero unseenMessages
- Test rendering with zero unseenMessages
- Test rendering with different lastMessage values
- Test rendering with both properties having different values
🔗 Analysis chain
Verify test coverage for new properties
The new properties and UI elements need proper test coverage to ensure they work as expected.
Also applies to: 72-80
Let me gather more context about the test coverage.
Let me check how these properties are actually used in the component to ensure proper test coverage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for relevant test cases in the corresponding test file
rg -A 5 "unseenMessages|lastMessage" "src/components/UserPortal/ContactCard/ContactCard.test.tsx"
Length of output: 309
Script:
#!/bin/bash
# Get full content of the test file to analyze test coverage
cat "src/components/UserPortal/ContactCard/ContactCard.test.tsx"
Length of output: 3223
Script:
#!/bin/bash
# Get the component implementation to see how unseenMessages and lastMessage are used
cat "src/components/UserPortal/ContactCard/ContactCard.tsx"
Length of output: 2798
| <div> | ||
| <b>{props.title}</b>{' '} | ||
| <p className={styles.lastMessage}>{props.lastMessage}</p> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper text contrast and semantic structure
The last message should be semantically distinct from the title. Consider using semantic HTML and ensuring proper text contrast for accessibility.
<div>
- <b>{props.title}</b>{' '}
- <p className={styles.lastMessage}>{props.lastMessage}</p>
+ <h3 className={styles.contactTitle}>{props.title}</h3>
+ <p className={styles.lastMessage} aria-label="Last message">
+ {props.lastMessage}
+ </p>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div> | |
| <b>{props.title}</b>{' '} | |
| <p className={styles.lastMessage}>{props.lastMessage}</p> | |
| </div> | |
| <div> | |
| <h3 className={styles.contactTitle}>{props.title}</h3> | |
| <p className={styles.lastMessage} aria-label="Last message"> | |
| {props.lastMessage} | |
| </p> | |
| </div> |
| chatListRefetch: ( | ||
| variables?: | ||
| | Partial<{ | ||
| id: string; | ||
| }> | ||
| | undefined, | ||
| ) => Promise<ApolloQueryResult<{ chatList: Chat[] }>>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the chatListRefetch function type definition
The type definition for chatListRefetch includes an unnecessary union with undefined. Since variables is already optional, the union with undefined can be omitted for clarity.
Apply this diff to simplify the type definition:
export interface InterfaceChatRoomProps {
selectedContact: string;
chatListRefetch: (
variables?: Partial<{
id: string;
}>
- | undefined,
) => Promise<ApolloQueryResult<{ chatList: Chat[] }>>;
}📝 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.
| chatListRefetch: ( | |
| variables?: | |
| | Partial<{ | |
| id: string; | |
| }> | |
| | undefined, | |
| ) => Promise<ApolloQueryResult<{ chatList: Chat[] }>>; | |
| chatListRefetch: ( | |
| variables?: Partial<{ | |
| id: string; | |
| }>, | |
| ) => Promise<ApolloQueryResult<{ chatList: Chat[] }>>; |
| @@ -59,7 +72,7 @@ | |||
| } | |||
| | undefined; | |||
| messageContent: string; | |||
| type: string; | |||
| media: string; | |||
There was a problem hiding this comment.
Make the media property optional in the DirectMessage type
To accurately represent messages that may not have media attached, consider making the media property optional.
Apply this diff to update the type definition:
type DirectMessage = {
_id: string;
createdAt: Date;
sender: {
_id: string;
firstName: string;
lastName: string;
image: string;
};
replyTo:
| {
_id: string;
createdAt: Date;
sender: {
_id: string;
firstName: string;
lastName: string;
image: string;
};
messageContent: string;
receiver: {
_id: string;
firstName: string;
lastName: string;
};
}
| undefined;
messageContent: string;
- media: string;
+ media?: string;
};📝 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.
| media: string; | |
| type DirectMessage = { | |
| _id: string; | |
| createdAt: Date; | |
| sender: { | |
| _id: string; | |
| firstName: string; | |
| lastName: string; | |
| image: string; | |
| }; | |
| replyTo: | |
| | { | |
| _id: string; | |
| createdAt: Date; | |
| sender: { | |
| _id: string; | |
| firstName: string; | |
| lastName: string; | |
| image: string; | |
| }; | |
| messageContent: string; | |
| receiver: { | |
| _id: string; | |
| firstName: string; | |
| lastName: string; | |
| }; | |
| } | |
| | undefined; | |
| messageContent: string; | |
| media?: string; | |
| }; |
| <Dropdown.Item | ||
| onClick={() => { | ||
| setEditMessage(message); | ||
| setNewMessage(message.messageContent); | ||
| }} | ||
| data-testid="replyToMessage" | ||
| > | ||
| Edit |
There was a problem hiding this comment.
Restrict editing messages to the sender only
The "Edit" option is currently available for all messages. To maintain message integrity, ensure that only the sender can edit their messages.
Apply this diff to conditionally render the "Edit" option:
<Dropdown.Menu>
<Dropdown.Item
onClick={() => {
setReplyToDirectMessage(message);
}}
data-testid="replyBtn"
>
{t('reply')}
</Dropdown.Item>
+ {message.sender._id === userId && (
<Dropdown.Item
onClick={() => {
setEditMessage(message);
setNewMessage(message.messageContent);
}}
data-testid="replyToMessage"
>
Edit
</Dropdown.Item>
+ )}
</Dropdown.Menu>Committable suggestion was skipped due to low confidence.
| const handleImageChange = async ( | ||
| e: React.ChangeEvent<HTMLInputElement>, | ||
| ): Promise<void> => { | ||
| const file = e.target.files?.[0]; | ||
| if (file) { | ||
| const base64 = await convertToBase64(file); | ||
| setAttachment(base64); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add validation for image attachments
Currently, there's no validation for the selected file's type or size. Adding checks can prevent issues with unsupported file formats or excessively large files, enhancing security and user experience.
Update the handleImageChange function:
const handleImageChange = async (
e: React.ChangeEvent<HTMLInputElement>,
): Promise<void> => {
const file = e.target.files?.[0];
if (file) {
+ if (!file.type.startsWith('image/')) {
+ alert('Please select a valid image file.');
+ return;
+ }
+ const maxSizeInBytes = 5 * 1024 * 1024; // 5 MB limit
+ if (file.size > maxSizeInBytes) {
+ alert('Image size should not exceed 5 MB.');
+ return;
+ }
const base64 = await convertToBase64(file);
setAttachment(base64);
}
};📝 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.
| const handleImageChange = async ( | |
| e: React.ChangeEvent<HTMLInputElement>, | |
| ): Promise<void> => { | |
| const file = e.target.files?.[0]; | |
| if (file) { | |
| const base64 = await convertToBase64(file); | |
| setAttachment(base64); | |
| } | |
| }; | |
| const handleImageChange = async ( | |
| e: React.ChangeEvent<HTMLInputElement>, | |
| ): Promise<void> => { | |
| const file = e.target.files?.[0]; | |
| if (file) { | |
| if (!file.type.startsWith('image/')) { | |
| alert('Please select a valid image file.'); | |
| return; | |
| } | |
| const maxSizeInBytes = 5 * 1024 * 1024; // 5 MB limit | |
| if (file.size > maxSizeInBytes) { | |
| alert('Image size should not exceed 5 MB.'); | |
| return; | |
| } | |
| const base64 = await convertToBase64(file); | |
| setAttachment(base64); | |
| } | |
| }; |
| @@ -74,6 +87,8 @@ | |||
| lastName: string; | |||
| email: string; | |||
| }[]; | |||
| unseenMessagesByUsers: JSON; | |||
There was a problem hiding this comment.
Specify a proper type for unseenMessagesByUsers
Using JSON as a type is not recommended because it's not a valid TypeScript type. Consider defining a specific interface or using a more appropriate type like Record<string, any>.
Update the type definition as follows:
type Chat = {
_id: string;
isGroup: boolean;
name?: string;
image?: string;
messages: DirectMessage[];
users: {
_id: string;
firstName: string;
lastName: string;
email: string;
}[];
- unseenMessagesByUsers: JSON;
+ unseenMessagesByUsers: Record<string, any>;
description: string;
};Or define a specific interface for unseenMessagesByUsers to enhance type safety.
📝 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.
| unseenMessagesByUsers: JSON; | |
| type Chat = { | |
| _id: string; | |
| isGroup: boolean; | |
| name?: string; | |
| image?: string; | |
| messages: DirectMessage[]; | |
| users: { | |
| _id: string; | |
| firstName: string; | |
| lastName: string; | |
| email: string; | |
| }[]; | |
| unseenMessagesByUsers: Record<string, any>; | |
| description: string; | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
public/locales/fr/translation.json (2)
1172-1181: Review chat-related translations for clarity and consistency.The translations for chat features are concise and clear. However, some improvements could be made:
- "chat" is kept in English - consider using "discussion" consistently
- "rechercher" should be capitalized for consistency with other action words
Consider these refinements:
"userChat": { "add": "Ajouter", - "chat": "Chat", + "chat": "Discussion", "contacts": "Contacts", - "search": "rechercher", + "search": "Rechercher", "messages": "messages", "create": "créer", "newChat": "nouvelle discussion", "newGroupChat": "Nouvelle discussion de groupe", "groupInfo": "Informations sur le groupe", "members": "Membres" }
1185-1186: Capitalize action words for consistency.The translations "répondre" should be capitalized to maintain consistency with other action words in the translation file.
"userChatRoom": { "selectContact": "Sélectionnez un contact pour démarrer la conversation", "sendMessage": "Envoyer le message", - "reply": "répondre" + "reply": "Répondre" }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- 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)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locales/sp/translation.json
🔇 Additional comments (8)
public/locales/zh/translation.json (1)
1172-1181: Add missing translations for core chat features.This is a duplicate of a past review comment. The translations for several key features mentioned in the PR objectives are still missing:
- Filtering chats
- Editing messages
- Image sharing in messages
public/locales/en/translation.json (4)
382-382: LGTM: Consistent addition of createChat key.The "createChat" key has been consistently added across organizationEvents, eventListCard, and userEvents sections, providing a uniform translation for chat creation functionality.
Also applies to: 516-516, 1116-1116
1172-1181: LGTM: Comprehensive chat functionality translations.The userChat section includes all necessary translation keys for the new chat features:
- Basic actions: add, create, search
- Chat types: newChat, newGroupChat
- Group management: groupInfo, members
- Message organization: messages, contacts
The translations are clear and follow the established naming patterns.
1185-1186: LGTM: Message interaction translations.The userChatRoom section includes essential message interaction translations:
- sendMessage for sending new messages
- reply for message responses
The translations are concise and user-friendly.
Line range hint
1-1500: Verify translations in other language files.Please ensure that these new translation keys are also added to other language files (fr, hi, sp, zh) to maintain consistent localization support across the application.
✅ Verification successful
Let me verify if all the new translation keys from the English file are present in other language files by checking a broader set of keys.
All translations are properly synchronized across language files
Based on the verification results, all the new chat-related translation keys are present and properly translated in all language files (fr, hi, sp, zh):
- The
userChatsection with all its keys (add, chat, search, messages, contacts, create, newChat, newGroupChat, groupInfo, members) exists in all language files- The
userChatRoomsection with its keys (selectContact, sendMessage, reply) is present in all language files- Group chat related keys (newGroupChat, groupInfo, members) are properly translated in all languages
The translations are consistent and no action is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the new chat-related keys exist in other language files # Test: Search for createChat, newChat, and sendMessage keys in other language files echo "Checking French translations..." rg "createChat|newChat|sendMessage" public/locales/fr/translation.json echo "Checking Hindi translations..." rg "createChat|newChat|sendMessage" public/locales/hi/translation.json echo "Checking Spanish translations..." rg "createChat|newChat|sendMessage" public/locales/sp/translation.json echo "Checking Chinese translations..." rg "createChat|newChat|sendMessage" public/locales/zh/translation.jsonLength of output: 1220
Script:
#!/bin/bash # Get all keys from the English file and check their presence in other language files echo "Listing all language files..." fd "translation.json" public/locales echo -e "\nChecking for new keys in userChat section..." rg '"userChat": \{' -A 10 public/locales/*/translation.json echo -e "\nChecking for new keys in userChatRoom section..." rg '"userChatRoom": \{' -A 5 public/locales/*/translation.json echo -e "\nChecking for group chat related keys..." rg '"newGroupChat"|"groupInfo"|"members"' public/locales/*/translation.jsonLength of output: 8981
public/locales/hi/translation.json (2)
1172-1181: LGTM! Chat UI translations are well-structured.The Hindi translations for chat UI elements are accurate, complete, and properly organized in the userChat section. The translations align well with the new chat features mentioned in the PR objectives.
1185-1186: LGTM! Message-related translations are accurate.The Hindi translations for message sending and replying functionality are appropriate and well-placed in the userChatRoom section.
public/locales/fr/translation.json (1)
406-407: Verify translation consistency for chat creation.The translation "Créer une discussion" for "createChat" is consistent with chat-related terminology, but appears in multiple sections. Ensure this translation is used consistently across all chat-related features.
✅ Verification successful
Translation "Créer une discussion" is consistently used across the French locale file
The verification shows that the translation "Créer une discussion" for "createChat" is used consistently in all three occurrences within the French locale file. This translation choice is appropriate and maintains consistency throughout the application. Other language files (Hindi, Spanish, Chinese, English) also maintain their respective consistent translations for this key.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check consistency of "createChat" translation across all locale files rg -A 1 '"createChat"' public/locales/Length of output: 1787
| "done": "पूर्ण", | ||
| "createChat": "चैट बनाएं" |
There was a problem hiding this comment.
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" or "userChatRoom" sections for better organization and maintainability.
Apply this diff to move the translations to the appropriate section:
"organizationEvents": {
- "done": "पूर्ण",
- "createChat": "चैट बनाएं"
},
"userChat": {
+ "done": "पूर्ण",
+ "createChat": "चैट बनाएं"
}Committable suggestion was skipped due to low confidence.
…lawa-admin into group-chat-features
…awa-admin into group-chat-features
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Outside diff range and nitpick comments (5)
src/components/GroupChatDetails/GroupChatDetails.test.tsx (2)
37-80: Enhance test coverage with additional mocks.
Consider the following improvements:
- Add error scenario mocks to test error handling
- Add mocks for image message functionality
- Use more specific search criteria in USERS_CONNECTION_LIST mock
Example addition for error scenario:
{
request: {
query: ADD_USER_TO_GROUP_CHAT,
variables: { userId: '3', chatId: '1' },
},
error: new Error('Failed to add user'),
},82-98: Improve test descriptions and add cleanup.
The test descriptions could be more specific about what they're verifying. Also, consider adding cleanup between tests.
describe('GroupChatDetails', () => {
+ afterEach(() => {
+ jest.clearAllMocks();
+ });
- it('renders correctly', () => {
+ it('should render group name and description when component mounts', () => {src/screens/UserPortal/Events/Events.test.tsx (1)
Line range hint 475-607: Enhance test coverage for chat-related edge cases.
Consider adding the following test scenarios:
- Error handling when chat creation fails
- Interaction between event visibility (public/private) and chat creation
- Validation of chat creation constraints (if any)
This will ensure the feature is thoroughly tested across different scenarios.
Example test structure:
test('Should handle chat creation failure gracefully', async () => {
// Mock with error response
const errorMock = {
request: {
query: CREATE_EVENT_MUTATION,
variables: {
// ... event details
createChat: true
}
},
error: new Error('Failed to create chat')
};
// Test implementation
// Verify error handling
expect(toast.error).toHaveBeenCalledWith('Failed to create event chat.');
});
test('Should enforce chat creation rules based on event visibility', async () => {
// Test chat creation with private events
// Verify appropriate behavior
});public/locales/fr/translation.json (2)
1172-1182: Maintain consistent capitalization in translations.
Some translations lack proper capitalization:
- "chat" should be "Chat"
- "messages" should be "Messages"
- "créer" should be "Créer"
- "nouvelle discussion" should be "Nouvelle discussion"
Apply this diff to fix capitalization:
- "chat": "chat",
- "messages": "messages",
- "create": "créer",
- "newChat": "nouvelle discussion",
+ "chat": "Chat",
+ "messages": "Messages",
+ "create": "Créer",
+ "newChat": "Nouvelle discussion",1186-1187: Maintain consistent translation style.
The translation for "reply" uses a verb form while other similar actions use nouns. Consider using "Réponse" instead of "répondre" for consistency. Also, maintain proper capitalization.
Apply this diff:
- "reply": "répondre"
+ "reply": "Réponse"📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
- 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/GraphQl/Queries/PlugInQueries.ts (4 hunks)
- src/components/GroupChatDetails/GroupChatDetails.module.css (1 hunks)
- src/components/GroupChatDetails/GroupChatDetails.test.tsx (1 hunks)
- src/components/GroupChatDetails/GroupChatDetails.tsx (1 hunks)
- src/components/UserPortal/ChatRoom/ChatRoom.tsx (13 hunks)
- src/screens/UserPortal/Events/Events.test.tsx (9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- public/locales/sp/translation.json
🔇 Additional comments (15)
src/components/GroupChatDetails/GroupChatDetails.module.css (2)
17-32: LGTM! Well-structured member list styling.
The implementation uses modern CSS features effectively:
- Flexbox for alignment
- Gap property for spacing
- Clear visual hierarchy with color
90-94: LGTM! Clean implementation of user details layout.
The implementation follows CSS best practices with appropriate use of flexbox and consistent spacing.
src/screens/UserPortal/Events/Events.test.tsx (2)
85-85: LGTM: Mock data structure is consistent with new chat functionality.
The createChat field has been consistently added across all mock objects, properly representing both true and false scenarios for event chat creation.
Also applies to: 117-117, 168-168, 207-207, 234-234, 261-261, 282-282, 315-315, 342-342, 369-369, 390-390, 417-417, 444-444
313-314: Time format inconsistencies in mock data.
Also applies to: 340-341, 367-368, 395-396, 422-423
public/locales/en/translation.json (1)
382-382: LGTM: Event chat creation translations are consistent.
The "createChat" translation key is consistently added across all event-related sections, supporting the feature for creating dedicated group chats for events.
Also applies to: 516-516, 1116-1116
public/locales/hi/translation.json (2)
406-407: Move chat-related translations to the appropriate section.
Also applies to: 542-543
1172-1181: LGTM! Chat-related translations are properly localized.
The translations for chat functionality are accurate and maintain consistency with the Hindi language.
Also applies to: 1183-1187
public/locales/fr/translation.json (1)
406-407: Consider consolidating duplicate translations.
The translation keys "done" and "createChat" appear in multiple sections with similar translations. This was previously flagged in a past review. Consider moving these common translations to a shared section.
src/GraphQl/Queries/PlugInQueries.ts (7)
125-125: Ensure the 'media' field in messages is handled correctly
The addition of the media field in the messages object enhances the capability to include media content in chats. Please verify that the backend schema and resolvers support this new field and that the frontend gracefully handles cases where media might be null or undefined.
151-152: Include 'image' field for user profiles
Adding the image field to the users object allows for displaying user profile pictures within chats. Ensure that this field is consistently populated and rendered in the user interface.
153-160: Verify the addition of the 'admins' field in the 'chatById' query
Including the admins field provides access to chat administrator details. Please ensure that the backend properly supports this field and that appropriate access controls are in place to prevent unauthorized access to admin information.
Line range hint 212-254: Check the implementation of the UNREAD_CHAT_LIST query
The UNREAD_CHAT_LIST query is a valuable addition for fetching chats with unread messages. Ensure that this query accurately retrieves unread chats and that it performs efficiently, especially for users with many chats.
259-313: Validate the updated CHATS_LIST query with search functionality
The CHATS_LIST query now accepts an optional searchString parameter to filter chats. Confirm that the backend correctly implements the filtering based on chat names and user first and last names, and that it handles edge cases like special characters and different casing.
275-280: Ensure proper handling of the creator field
Including detailed information in the creator field is helpful. Verify that the data exposed adheres to privacy policies and that sensitive information is protected.
297-302: Consistency in including the image field for users
Adding the image field to the users in the users array maintains consistency across user representations. Ensure that this field is correctly handled in all parts of the application where user data is displayed.
| .membersImage { | ||
| width: 40px !important; | ||
| } |
There was a problem hiding this comment.
Avoid !important and consider responsive image dimensions.
- The use of
!importantsuggests specificity issues that should be resolved properly - Fixed dimensions might cause image distortion or layout issues on different screen sizes
Consider these improvements:
.membersImage {
- width: 40px !important;
+ width: 40px;
+ height: 40px;
+ object-fit: cover;
}
.chatImage {
- height: 120px;
+ aspect-ratio: 1;
border-radius: 100%;
- width: 120px;
+ width: clamp(80px, 120px, 15vw);
+ object-fit: cover;
}Also applies to: 54-58
| .editImgBtn { | ||
| padding: 2px 6px 6px 8px; | ||
| border-radius: 100%; | ||
| background-color: white; | ||
| border: 1px solid #959595; | ||
| color: #959595; | ||
| outline: none; | ||
| position: relative; | ||
| top: -40px; | ||
| left: 40px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance interactive elements for better UX.
- Fixed positioning offsets might not work well across different screen sizes
- Input element lacks visual feedback for focus state
Consider these improvements:
.editImgBtn {
padding: 2px 6px 6px 8px;
border-radius: 100%;
background-color: white;
border: 1px solid #959595;
color: #959595;
outline: none;
- position: relative;
- top: -40px;
- left: 40px;
+ position: absolute;
+ bottom: 0;
+ right: 0;
+ transform: translate(25%, 25%);
}
.editChatNameContainer input {
border: none;
border-bottom: 1px solid rgb(171, 171, 171);
outline: none;
padding: 0px 5px;
+ transition: border-color 0.2s ease;
+}
+
+.editChatNameContainer input:focus {
+ border-bottom: 2px solid #31bb6b;
}Also applies to: 68-73
| .cancelIcon { | ||
| color: rgb(197, 42, 42); | ||
| cursor: pointer; | ||
| font-size: 16px; | ||
| } | ||
|
|
||
| .checkIcon { | ||
| color: rgb(42, 197, 42); | ||
| cursor: pointer; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance icon styles with semantic colors and interaction states.
Consider using semantic color variables and adding hover/focus states for better interaction feedback.
Consider these improvements:
.cancelIcon {
- color: rgb(197, 42, 42);
+ color: var(--error-color, rgb(197, 42, 42));
cursor: pointer;
font-size: 16px;
+ transition: opacity 0.2s ease;
+}
+
+.cancelIcon:hover,
+.cancelIcon:focus {
+ opacity: 0.8;
}
.checkIcon {
- color: rgb(42, 197, 42);
+ color: var(--success-color, rgb(42, 197, 42));
cursor: pointer;
+ transition: opacity 0.2s ease;
+}
+
+.checkIcon:hover,
+.checkIcon:focus {
+ opacity: 0.8;
}📝 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.
| .cancelIcon { | |
| color: rgb(197, 42, 42); | |
| cursor: pointer; | |
| font-size: 16px; | |
| } | |
| .checkIcon { | |
| color: rgb(42, 197, 42); | |
| cursor: pointer; | |
| } | |
| .cancelIcon { | |
| color: var(--error-color, rgb(197, 42, 42)); | |
| cursor: pointer; | |
| font-size: 16px; | |
| transition: opacity 0.2s ease; | |
| } | |
| .cancelIcon:hover, | |
| .cancelIcon:focus { | |
| opacity: 0.8; | |
| } | |
| .checkIcon { | |
| color: var(--success-color, rgb(42, 197, 42)); | |
| cursor: pointer; | |
| transition: opacity 0.2s ease; | |
| } | |
| .checkIcon:hover, | |
| .checkIcon:focus { | |
| opacity: 0.8; | |
| } |
| .memberList { | ||
| max-height: 300px; | ||
| overflow: scroll; | ||
| } | ||
|
|
||
| .memberList::-webkit-scrollbar { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
Improve cross-browser compatibility for scrollable content.
The current implementation has two issues:
overflow: scrollalways shows scrollbars even when unnecessary- Scrollbar hiding only works in WebKit browsers
Apply this diff to improve cross-browser compatibility:
.memberList {
max-height: 300px;
- overflow: scroll;
+ overflow-y: auto;
+ scrollbar-width: none; /* Firefox */
+ -ms-overflow-style: none; /* IE and Edge */
}Committable suggestion was skipped due to low confidence.
| it('edits chat title', async () => { | ||
| render( | ||
| <MockedProvider mocks={mocks} addTypename={false}> | ||
| <GroupChatDetails | ||
| toggleGroupChatDetailsModal={jest.fn()} | ||
| groupChatDetailsModalisOpen={true} | ||
| chat={mockChat} | ||
| chatRefetch={jest.fn()} | ||
| /> | ||
| </MockedProvider>, | ||
| ); | ||
|
|
||
| fireEvent.click(screen.getByText('Test Group')); | ||
| fireEvent.change(screen.getByDisplayValue('Test Group'), { | ||
| target: { value: 'Updated Group' }, | ||
| }); | ||
| fireEvent.click(screen.getByText('✔')); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('Updated Group')).toBeInTheDocument(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add validation for chat title editing.
The chat title editing test should include validation for:
- Empty title handling
- Maximum length restrictions
- Special characters handling
Example additional test:
it('should not allow empty chat title', async () => {
render(
<MockedProvider mocks={mocks} addTypename={false}>
<GroupChatDetails
toggleGroupChatDetailsModal={jest.fn()}
groupChatDetailsModalisOpen={true}
chat={mockChat}
chatRefetch={jest.fn()}
/>
</MockedProvider>
);
fireEvent.click(screen.getByText('Test Group'));
fireEvent.change(screen.getByDisplayValue('Test Group'), {
target: { value: '' },
});
fireEvent.click(screen.getByText('✔'));
await waitFor(() => {
expect(screen.getByText('Title cannot be empty')).toBeInTheDocument();
});
});| "add": "Add", | ||
| "chat": "Chat", | ||
| "search": "Search", | ||
| "messages": "Messages", | ||
| "contacts": "Contacts" | ||
| "contacts": "Contacts", | ||
| "create": "Create", | ||
| "newChat": "New Chat", | ||
| "newGroupChat": "New Group Chat", | ||
| "groupInfo": "Group Info", | ||
| "members": "Members", | ||
| "addMembers": "Add Members" |
There was a problem hiding this comment.
Missing translations for chat filtering and image sharing.
While the translations cover basic chat and group management features, they're missing keys for:
- Chat filtering functionality
- Image sharing in messages
These features were mentioned in the PR objectives but lack corresponding UI strings.
Add the following translation keys:
"userChat": {
"add": "Add",
"chat": "Chat",
"search": "Search",
"messages": "Messages",
"contacts": "Contacts",
"create": "Create",
"newChat": "New Chat",
"newGroupChat": "New Group Chat",
"groupInfo": "Group Info",
"members": "Members",
"addMembers": "Add Members",
+ "filterChats": "Filter Chats",
+ "filterByName": "Filter by name",
+ "filterByDate": "Filter by date",
+ "attachImage": "Attach image",
+ "imagePreview": "Image preview"
}Committable suggestion was skipped due to low confidence.
| "done": "Terminé", | ||
| "createChat": "Créer une discussion" |
There was a problem hiding this comment.
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.
| export const GROUP_CHAT_LIST = gql` | ||
| query groupChatsByUserId { | ||
| getGroupChatsByUserId { | ||
| _id | ||
| isGroup | ||
| name | ||
| creator { | ||
| _id | ||
| firstName | ||
| lastName | ||
| } | ||
| messages { | ||
| _id | ||
| createdAt | ||
| messageContent | ||
| media | ||
| sender { | ||
| _id | ||
| firstName | ||
| lastName | ||
| } | ||
| } | ||
| organization { | ||
| _id | ||
| name | ||
| } | ||
| users { | ||
| _id | ||
| firstName | ||
| lastName | ||
| image | ||
| } | ||
| admins { | ||
| _id | ||
| firstName | ||
| lastName | ||
| image | ||
| } | ||
| unseenMessagesByUsers | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the new GROUP_CHAT_LIST query for completeness
The GROUP_CHAT_LIST query retrieves group chats for the user. Verify that all necessary fields are included and that the query aligns with the backend schema. Also, consider implementing pagination if the number of group chats could be large to improve performance.
| name_contains: $searchString | ||
| user: { | ||
| firstName_contains: $searchString | ||
| lastName_contains: $searchString | ||
| } |
There was a problem hiding this comment.
Clarify the search conditions in the where clause
The where clause combines name_contains, user.firstName_contains, and user.lastName_contains. Ensure that these conditions are logically combined in a way that meets the search requirements (e.g., using OR instead of AND if needed) to provide expected results to the user.
Consider restructuring the conditions if users might expect to find chats when any of the fields match the search string.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Outside diff range and nitpick comments (4)
src/screens/UserPortal/Chat/Chat.tsx (1)
57-101: LGTM! Well-structured type definitions.The
DirectMessageandChattypes are comprehensive and provide good type safety. However, consider adding JSDoc comments to document these types for better maintainability.Add documentation comments to describe the purpose and usage of these types:
/** * Represents a direct message in the chat system * @property _id - Unique identifier for the message * @property createdAt - Timestamp when the message was created * ... */ type DirectMessage = { // ... existing properties } /** * Represents a chat conversation, which can be either a direct chat or a group chat * @property _id - Unique identifier for the chat * @property isGroup - Indicates if this is a group chat * ... */ type Chat = { // ... existing properties }src/components/GroupChatDetails/GroupChatDetails.test.tsx (1)
5-6: Remove commented-out imports.The file contains commented-out imports that should be removed as they are actually being used later in the code.
- // fireEvent, - // waitFor,src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (2)
Line range hint
3841-3866: Improve test setup and cleanup practices.The current approach of using global mocks without proper cleanup could affect other tests and lead to unexpected behavior.
Consider refactoring the test setup using the following approach:
describe('Testing Create Direct Chat Modal [User Portal]', () => { // Setup before each test beforeEach(() => { // Mock window.matchMedia Object.defineProperty(window, 'matchMedia', { writable: true, value: jest.fn().mockImplementation((query) => ({ matches: false, media: query, onchange: null, addEventListener: jest.fn(), removeEventListener: jest.fn(), dispatchEvent: jest.fn(), })), }); // Mock scrollIntoView window.HTMLElement.prototype.scrollIntoView = jest.fn(); }); // Cleanup after each test afterEach(() => { // Reset all mocks jest.resetAllMocks(); // Clean up any mounted components cleanup(); // Reset localStorage localStorage.clear(); }); // Your test cases... });
Line range hint
3841-3866: Improve code quality and test maintainability.The current implementation has several areas that could be improved for better maintainability and readability.
- Extract the wait function to a test utilities file and make the timeout configurable:
// testUtils.ts export const DEFAULT_TIMEOUT = 100; export async function waitForComponent(ms = DEFAULT_TIMEOUT): Promise<void> { await act(() => { return new Promise((resolve) => { setTimeout(resolve, ms); }); }); }
- Make test descriptions more descriptive:
describe('CreateDirectChat Modal', () => { describe('UI Interactions', () => { it('should open modal when new direct chat button is clicked', async () => { // Test implementation }); it('should close modal when close button is clicked', async () => { // Test implementation }); }); describe('Chat Creation', () => { it('should create new direct chat with selected user', async () => { // Test implementation }); }); });
- Move mock data to separate files:
// __mocks__/chatMocks.ts export const GROUP_CHAT_BY_ID_QUERY_MOCK = [...]; export const MESSAGE_SENT_TO_CHAT_MOCK = [...]; // ... other mocks // CreateDirectChat.test.tsx import { GROUP_CHAT_BY_ID_QUERY_MOCK, MESSAGE_SENT_TO_CHAT_MOCK, // ... other mocks } from './__mocks__/chatMocks';Also applies to: 3867-3877, 3929-3931
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
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/components/GroupChatDetails/GroupChatDetails.module.css(1 hunks)src/components/GroupChatDetails/GroupChatDetails.test.tsx(1 hunks)src/components/GroupChatDetails/GroupChatDetails.tsx(1 hunks)src/components/UserPortal/ChatRoom/ChatRoom.tsx(13 hunks)src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx(22 hunks)src/screens/UserPortal/Chat/Chat.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- public/locales/sp/translation.json
- src/components/GroupChatDetails/GroupChatDetails.module.css
🧰 Additional context used
🪛 Biome
src/screens/UserPortal/Chat/Chat.tsx
[error] 132-132: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 142-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 199-199: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (10)
src/screens/UserPortal/Chat/Chat.tsx (1)
334-337: Verify ChatRoom component's usage of chatListRefetch
The ChatRoom component now receives chatListRefetch as a prop. Let's verify its implementation.
✅ Verification successful
Based on examining the code, I can now provide a definitive response about the chatListRefetch prop usage in the ChatRoom component:
Verified proper implementation of chatListRefetch prop
The ChatRoom component correctly uses the chatListRefetch prop in two scenarios:
- After marking chat messages as read via
markChatMessagesAsReadmutation - After sending new messages via
sendMessageToChatmutation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the ChatRoom component implementation to verify proper usage of chatListRefetch prop
ast-grep --pattern 'interface $interface {
$$$
chatListRefetch: $type
$$$
}'
# Also check for actual usage of the prop
ast-grep --pattern 'chatListRefetch($$$)'
Length of output: 128
Script:
#!/bin/bash
# Let's try a different approach to find ChatRoom component and its props
ast-grep --pattern 'interface ChatRoomProps {
$$$
}'
# Search for ChatRoom component definition
ast-grep --pattern 'const ChatRoom = ($props: $type) => {
$$$
}'
# Also search for any usage of chatListRefetch within ChatRoom component
rg -A 5 "chatListRefetch" --type ts --type tsx
Length of output: 200
Script:
#!/bin/bash
# Let's try finding the ChatRoom component file first
fd ChatRoom
# Then let's look for chatListRefetch usage with proper file extension
rg -A 5 "chatListRefetch" --type typescript
# Also try to find the component definition with a more flexible pattern
ast-grep --pattern 'ChatRoom = {
$$$
}'
Length of output: 471
Script:
#!/bin/bash
# Now that we found the ChatRoom component file, let's examine its contents
cat src/components/UserPortal/ChatRoom/ChatRoom.tsx
# Also check the test file for any usage patterns
cat src/components/UserPortal/ChatRoom/ChatRoom.test.tsx
Length of output: 165560
src/components/UserPortal/ChatRoom/ChatRoom.tsx (2)
75-75:
Make the media property optional in DirectMessage type
The media property should be optional since not all messages will contain attachments.
Apply this diff:
- media: string;
+ media?: string;Likely invalid or redundant comment.
409-416:
Restrict message editing to sender only
Currently, the edit option is available for all messages. Only the message sender should be able to edit their own messages.
Add a condition to show the edit option:
+ {message.sender._id === userId && (
<Dropdown.Item
onClick={() => {
setEditMessage(message);
setNewMessage(message.messageContent);
}}
data-testid="replyToMessage"
>
Edit
</Dropdown.Item>
+ )}Likely invalid or redundant comment.
public/locales/zh/translation.json (1)
1202-1202:
Translate "Add Members" to Chinese.
The text is in English while this file contains Chinese translations.
Apply this diff:
- "addMembers": "Add Members"
+ "addMembers": "添加成员"Likely invalid or redundant comment.
public/locales/en/translation.json (3)
402-402: LGTM: Event chat creation translations added consistently.
The "createChat" translation key has been consistently added across event-related sections, enabling the creation of dedicated group chats for events as specified in the PR objectives.
Also applies to: 536-536, 1136-1136
1192-1202: LGTM: Basic chat UI translations added.
The translation keys for basic chat functionality have been added, including:
- Group chat creation and management
- Member management
- Contact list navigation
1206-1207: LGTM: Basic messaging translations added.
The translation keys for basic messaging functionality have been added.
Note: Past reviews have already identified missing translations for message editing, chat filtering, and image sharing features.
public/locales/hi/translation.json (3)
426-427: 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.
1192-1202: LGTM! Complete chat section translations.
The new chat section translations are well-organized and cover essential functionality like creating new chats, group chats, and managing members.
1206-1207: LGTM! Message-related translations are complete.
The translations for sending messages and replying are appropriate and well-placed in the userChatRoom section.
| <div className={styles.filters}> | ||
| {/* three buttons to filter unread, all and group chats. All selected by default. */} | ||
| <Button | ||
| onClick={() => { | ||
| setFilterType('all'); | ||
| }} | ||
| data-testid="allChat" | ||
| className={[ | ||
| styles.filterButton, | ||
| filterType === 'all' && styles.selectedBtn, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(' ')} | ||
| > | ||
| All | ||
| </Button> | ||
| <Button | ||
| data-testid="unreadChat" | ||
| onClick={() => { | ||
| setFilterType('unread'); | ||
| }} | ||
| className={[ | ||
| styles.filterButton, | ||
| filterType === 'unread' && styles.selectedBtn, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(' ')} | ||
| > | ||
| Unread | ||
| </Button> | ||
| <Button | ||
| onClick={() => { | ||
| setFilterType('group'); | ||
| }} | ||
| data-testid="groupChat" | ||
| className={[ | ||
| styles.filterButton, | ||
| filterType === 'group' && styles.selectedBtn, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(' ')} | ||
| > | ||
| Groups | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
Use translations for filter button labels
The filter buttons use hardcoded English strings which should be translated for internationalization support.
Apply this change to use translations:
- All
+ {t('filters.all')}
- Unread
+ {t('filters.unread')}
- Groups
+ {t('filters.groups')}Also, consider simplifying the button styles using a utility function:
const getButtonClassName = (currentType: string) =>
`${styles.filterButton} ${filterType === currentType ? styles.selectedBtn : ''}`.trim();Then use it like:
className={getButtonClassName('all')}| {!!chats.length && | ||
| chats.map((chat: Chat) => { | ||
| const cardProps: InterfaceContactCardProps = { | ||
| id: chat._id, | ||
| title: !chat.isGroup | ||
| ? chat.users[0]?._id === userId | ||
| ? `${chat.users[1]?.firstName} ${chat.users[1]?.lastName}` | ||
| : `${chat.users[0]?.firstName} ${chat.users[0]?.lastName}` | ||
| : chat.name, | ||
| image: chat.isGroup | ||
| ? chat.image | ||
| : userId | ||
| ? chat.users[1]?.image | ||
| : chat.users[0]?.image, | ||
| setSelectedContact, | ||
| selectedContact, | ||
| isGroup: chat.isGroup, | ||
| unseenMessages: JSON.parse( | ||
| chat.unseenMessagesByUsers, | ||
| )[userId], | ||
| lastMessage: | ||
| chat.messages[chat.messages.length - 1] | ||
| ?.messageContent, | ||
| }; | ||
| return ( | ||
| <ContactCard | ||
| data-testid="chatContact" | ||
| {...cardProps} | ||
| key={chat._id} | ||
| /> | ||
| ); | ||
| })} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify contact card rendering logic
The current implementation could be simplified by extracting the logic for determining chat titles and images into helper functions.
Consider refactoring like this:
const getChatTitle = (chat: Chat, userId: string): string => {
if (!chat.isGroup) {
const otherUser = chat.users.find(user => user._id !== userId);
return otherUser ? `${otherUser.firstName} ${otherUser.lastName}` : '';
}
return chat.name;
};
const getChatImage = (chat: Chat, userId: string): string => {
if (!chat.isGroup) {
const otherUser = chat.users.find(user => user._id !== userId);
return otherUser?.image || '';
}
return chat.image;
};
// Then in the render:
const cardProps: InterfaceContactCardProps = {
id: chat._id,
title: getChatTitle(chat, userId),
image: getChatImage(chat, userId),
// ... other props
};| React.useEffect(() => { | ||
| if (filterType === 'all') { | ||
| chatsListRefetch(); | ||
| if (chatsListData && chatsListData.chatsByUserId) { | ||
| setChats(chatsListData.chatsByUserId); | ||
| } | ||
| } else if (filterType === 'unread') { | ||
| unreadChatListRefetch(); | ||
| if (unreadChatListData && unreadChatListData.getUnreadChatsByUserId) { | ||
| setChats(unreadChatListData.getUnreadChatsByUserId); | ||
| } | ||
| } else if (filterType === 'group') { | ||
| groupChatListRefetch(); | ||
| if (groupChatListData && groupChatListData.getGroupChatsByUserId) { | ||
| setChats(groupChatListData.getGroupChatsByUserId); | ||
| } | ||
| } | ||
| }, [filterType]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve filter implementation robustness and maintainability
The current implementation could be enhanced in several ways:
- Use optional chaining as suggested by static analysis
- Add error handling
- Handle loading states
- Extract filter logic into a separate function
Consider refactoring the code like this:
+ const handleFilterChange = (data: any, filterKey: string) => {
+ if (data?.[filterKey]) {
+ setChats(data[filterKey]);
+ }
+ };
React.useEffect(() => {
if (filterType === 'all') {
chatsListRefetch();
- if (chatsListData && chatsListData.chatsByUserId) {
+ if (chatsListData?.chatsByUserId) {
handleFilterChange(chatsListData, 'chatsByUserId');
}
} else if (filterType === 'unread') {
unreadChatListRefetch();
- if (unreadChatListData && unreadChatListData.getUnreadChatsByUserId) {
+ if (unreadChatListData?.getUnreadChatsByUserId) {
handleFilterChange(unreadChatListData, 'getUnreadChatsByUserId');
}
} else if (filterType === 'group') {
groupChatListRefetch();
- if (groupChatListData && groupChatListData.getGroupChatsByUserId) {
+ if (groupChatListData?.getGroupChatsByUserId) {
handleFilterChange(groupChatListData, 'getGroupChatsByUserId');
}
}
}, [filterType]);📝 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.
| React.useEffect(() => { | |
| if (filterType === 'all') { | |
| chatsListRefetch(); | |
| if (chatsListData && chatsListData.chatsByUserId) { | |
| setChats(chatsListData.chatsByUserId); | |
| } | |
| } else if (filterType === 'unread') { | |
| unreadChatListRefetch(); | |
| if (unreadChatListData && unreadChatListData.getUnreadChatsByUserId) { | |
| setChats(unreadChatListData.getUnreadChatsByUserId); | |
| } | |
| } else if (filterType === 'group') { | |
| groupChatListRefetch(); | |
| if (groupChatListData && groupChatListData.getGroupChatsByUserId) { | |
| setChats(groupChatListData.getGroupChatsByUserId); | |
| } | |
| } | |
| }, [filterType]); | |
| const handleFilterChange = (data: any, filterKey: string) => { | |
| if (data?.[filterKey]) { | |
| setChats(data[filterKey]); | |
| } | |
| }; | |
| React.useEffect(() => { | |
| if (filterType === 'all') { | |
| chatsListRefetch(); | |
| if (chatsListData?.chatsByUserId) { | |
| handleFilterChange(chatsListData, 'chatsByUserId'); | |
| } | |
| } else if (filterType === 'unread') { | |
| unreadChatListRefetch(); | |
| if (unreadChatListData?.getUnreadChatsByUserId) { | |
| handleFilterChange(unreadChatListData, 'getUnreadChatsByUserId'); | |
| } | |
| } else if (filterType === 'group') { | |
| groupChatListRefetch(); | |
| if (groupChatListData?.getGroupChatsByUserId) { | |
| handleFilterChange(groupChatListData, 'getGroupChatsByUserId'); | |
| } | |
| } | |
| }, [filterType]); |
🧰 Tools
🪛 Biome
[error] 132-132: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 137-137: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 142-142: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| lastName: string; | ||
| email: string; | ||
| }[]; | ||
| unseenMessagesByUsers: string; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for unseenMessagesByUsers
The unseenMessagesByUsers field is typed as string but appears to represent a mapping of user IDs to unseen message counts.
- unseenMessagesByUsers: string;
+ unseenMessagesByUsers: Record<string, number>;📝 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.
| unseenMessagesByUsers: string; | |
| unseenMessagesByUsers: Record<string, number>; |
| <StyledTableCell align="center">{'user'}</StyledTableCell> | ||
| <StyledTableCell align="center">{'Chat'}</StyledTableCell> |
There was a problem hiding this comment.
Use translation for table headers
The table headers are hardcoded instead of using the translation system.
- <StyledTableCell align="center">{'user'}</StyledTableCell>
- <StyledTableCell align="center">{'Chat'}</StyledTableCell>
+ <StyledTableCell align="center">{t('user')}</StyledTableCell>
+ <StyledTableCell align="center">{t('action')}</StyledTableCell>📝 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.
| <StyledTableCell align="center">{'user'}</StyledTableCell> | |
| <StyledTableCell align="center">{'Chat'}</StyledTableCell> | |
| <StyledTableCell align="center">{t('user')}</StyledTableCell> | |
| <StyledTableCell align="center">{t('action')}</StyledTableCell> |
| "done": "समाप्त", | ||
| "createChat": "चैट बनाएं" |
There was a problem hiding this comment.
Move chat-related translations to the appropriate section.
Similarly, the translations for "done" and "createChat" in the "eventListCard" section should be moved to the "userChat" section to maintain consistency and avoid duplication.
Apply this diff to consolidate chat-related translations:
"eventListCard": {
- "done": "समाप्त",
- "createChat": "चैट बनाएं"
},
"userChat": {
+ "done": "समाप्त",
+ "createChat": "चैट बनाएं"
}📝 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.
| "done": "समाप्त", | |
| "createChat": "चैट बनाएं" | |
| "done": "समाप्त", | |
| "createChat": "चैट बनाएं" |
| "add": "Ajouter", | ||
| "chat": "Chat", | ||
| "contacts": "Contacts", | ||
| "search": "rechercher", | ||
| "messages": "messages" | ||
| "messages": "messages", | ||
| "create": "créer", | ||
| "newChat": "nouvelle discussion", | ||
| "newGroupChat": "Nouvelle discussion de groupe", | ||
| "groupInfo": "Informations sur le groupe", | ||
| "members": "Membres", | ||
| "addMembers": "Add Members" |
There was a problem hiding this comment.
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.
- "chat": "Chat",
- "search": "rechercher",
- "messages": "messages",
- "create": "créer",
- "newChat": "nouvelle discussion",
+ "chat": "Chat",
+ "search": "Rechercher",
+ "messages": "Messages",
+ "create": "Créer",
+ "newChat": "Nouvelle discussion",
- "reply": "répondre"
+ "reply": "Répondre"Also applies to: 1206-1207
| "newGroupChat": "Nouvelle discussion de groupe", | ||
| "groupInfo": "Informations sur le groupe", | ||
| "members": "Membres", | ||
| "addMembers": "Add Members" |
There was a problem hiding this comment.
Translate the English text to French.
The string "Add Members" should be translated to French to maintain consistency.
- "addMembers": "Add Members"
+ "addMembers": "Ajouter des membres"📝 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.
| "addMembers": "Add Members" | |
| "addMembers": "Ajouter des membres" |
| admins: [ | ||
| { | ||
| _id: '1', | ||
| firstName: 'Disha', | ||
| lastName: 'Talreja', | ||
| email: '[email protected]', | ||
| image: '', | ||
| }, | ||
| ], | ||
| unseenMessagesByUsers: JSON.stringify({ | ||
| '1': 0, | ||
| '2': 0, | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor mock data to improve maintainability and reduce duplication.
The mock data contains significant duplication of user objects, messages, and chat structures. This makes the file harder to maintain and increases the risk of inconsistencies.
Consider refactoring the mock data using the following approach:
// mockData/users.ts
export const mockUsers = {
admin: {
_id: '1',
firstName: 'Disha',
lastName: 'Talreja',
email: '[email protected]',
image: '',
},
regularUser: {
_id: '2',
firstName: 'Test',
lastName: 'User',
email: '[email protected]',
image: '',
},
// ... other users
};
// mockData/messages.ts
export const createMockMessage = (overrides = {}) => ({
_id: '345678',
createdAt: '345678908765',
messageContent: 'Hello',
replyTo: null,
type: 'STRING',
sender: mockUsers.regularUser,
...overrides
});
// mockData/chats.ts
export const createMockChat = (overrides = {}) => ({
_id: '1',
isGroup: false,
creator: mockUsers.admin,
organization: null,
name: '',
messages: [createMockMessage()],
users: [mockUsers.admin, mockUsers.regularUser],
admins: [mockUsers.admin],
unseenMessagesByUsers: JSON.stringify({
'1': 0,
'2': 0,
}),
...overrides
});Also applies to: 540-552, 613-625, 687-699, 758-770, 829-841, 900-912, 971-983, 1046-1058, 1119-1131, 1192-1204, 1265-1277, 1339-1351, 1410-1422, 1481-1493, 1583-1598, 1672-1678, 1766-1781, 1855-1861, 1949-1964, 2047-2053, 2141-2156, 2230-2236, 2324-2339, 2413-2419, 2507-2522, 2596-2602, 2690-2705, 2779-2794, 2882-2897, 2971-2986, 3074-3089, 3163-3178, 3266-3281, 3355-3370, 3458-3473, 3547-3562, 3652-3667
| test('open and close create new direct chat modal', async () => { | ||
| const mock = [ | ||
| ...GROUP_CHAT_BY_ID_QUERY_MOCK, | ||
| ...MESSAGE_SENT_TO_CHAT_MOCK, | ||
| ...UserConnectionListMock, | ||
| ...CHATS_LIST_MOCK, | ||
| ...CHAT_BY_ID_QUERY_MOCK, | ||
| ...CREATE_CHAT_MUTATION_MOCK, | ||
| ...MARK_CHAT_MESSAGES_AS_READ_MOCK, | ||
| ...UNREAD_CHAT_LIST_QUERY_MOCK, | ||
| ...GROUP_CHAT_BY_USER_ID_QUERY_MOCK, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance test coverage with additional test cases.
The current test suite only covers basic happy path scenarios. Consider adding more comprehensive test coverage.
Add the following test cases:
test('should handle network error when creating chat', async () => {
const errorMock = {
request: {
query: CREATE_CHAT,
variables: {
organizationId: undefined,
userIds: ['1'],
isGroup: false
}
},
error: new Error('Network error')
};
// Test implementation
// Verify error handling and user feedback
});
test('should handle empty user selection', async () => {
// Test implementation
// Verify validation and error message
});
test('should handle maximum users limit for group chat', async () => {
// Test implementation
// Verify validation and error message
});
test('should handle duplicate user selection', async () => {
// Test implementation
// Verify validation and error message
});
|
We were having difficulties merging PRs because some of your recent changes created schema introspection check errors. This PR reverted some of your changes. https://github.com/PalisadoesFoundation/talawa-admin/pull/2391/files Please open two PRs for both the API and Admin to restore the feature without the to be simultaneously merged so that the intropection PR errors don't occur. |
|
|
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. |
github-actions
bot
added
no-pr-activity
|
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 Are you still working on this? Please fix the failing tests and conflicting files |
|
@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?
Feature
Issue Number:
Fixes #2359
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
New Features
Improvements
Bug Fixes
Tests
GroupChatDetailscomponent, covering rendering, modal toggling, and user addition.CreateDirectChatandEventscomponents to reflect new chat functionalities.