fix: removed unused references of group chat and direct chat #2394
Conversation
WalkthroughThe changes in this pull request involve significant modifications to the GraphQL schema, primarily removing types and inputs associated with chat functionalities. This includes the elimination of Changes
Possibly related PRs
Suggested reviewers
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/GraphQl/Queries/PlugInQueries.ts (1)
Line range hint
110-187: Consider optimizing query field selection.Both chat queries fetch detailed user information (firstName, lastName, email, image) for multiple user references (sender, users). Consider:
- Reviewing if all these fields are necessary for each context
- Using fragments to manage repeated user field selections
- Implementing field selection based on actual UI requirements
Example optimization using fragments:
export const CHAT_BY_ID = gql` + fragment UserFields on User { + _id + firstName + lastName + # Include only necessary fields + } + query chatById($id: ID!) { chatById(id: $id) { _id isGroup name organization { _id } createdAt messages { _id createdAt messageContent replyTo { _id createdAt messageContent sender { - _id - firstName - lastName - email - image + ...UserFields } } sender { - _id - firstName - lastName - email - image + ...UserFields } } users { - _id - firstName - lastName - email + ...UserFields } } } `;src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)
Line range hint
1-1000: Consider updating test descriptions and variable names.The test descriptions and variable names should be updated to reflect the new unified chat approach rather than specifically mentioning direct chat:
- Test suite description: "Testing Create Direct Chat Modal [User Portal]"
- Variable names like
newDirectChatBtn- Test descriptions like "create new direct chat"
Consider renaming these to use more generic terms like "chat" instead of "direct chat".
src/components/UserPortal/CreateGroupChat/CreateGroupChat.test.tsx (3)
Line range hint
1-1237: Consider refactoring mock data to reduce duplication.The mock data contains repeated organization structures with only slight variations in IDs. Consider extracting these into shared fixtures or helper functions to improve maintainability.
Example approach:
const createBaseMockOrg = (id: string, name: string) => ({ __typename: 'Organization', _id: id, name, image: '', description: 'New Desc', address: { // ... base address }, // ... other common fields }); const USER_JOINED_ORG_MOCK = [ { request: { query: USER_JOINED_ORGANIZATIONS, variables: { id: '1' } }, result: { data: { users: [{ user: { joinedOrganizations: [ createBaseMockOrg('6401ff65ce8e8406b8f07af2', 'Test Org 1'), // ... other orgs ] } }] } } } ];
Line range hint
2394-2428: Add assertions for modal state and error scenarios.The test for opening/closing the modal could be enhanced with additional assertions to verify the modal's state and content.
Consider adding:
- Assertions for modal title and content
- Error scenario testing (e.g., invalid group name)
- Verification of form validation messages
Line range hint
2429-2517: Improve async operation handling in group chat creation test.The test for creating a new group chat has some potential race conditions and missing assertions.
Consider these improvements:
test('create new group chat', async () => { // ... existing setup ... await act(async () => { fireEvent.click(await screen.findByTestId('createBtn')); }); + // Add assertions to verify successful creation + await waitFor(() => { + expect(screen.queryByTestId('createGroupChatModal')).not.toBeInTheDocument(); + }); + + // Verify the new chat appears in the list + await waitFor(() => { + expect(screen.getByText('Test Group')).toBeInTheDocument(); + }); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
schema.graphql(0 hunks)src/GraphQl/Mutations/OrganizationMutations.ts(1 hunks)src/GraphQl/Queries/PlugInQueries.ts(1 hunks)src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx(2 hunks)src/components/UserPortal/CreateGroupChat/CreateGroupChat.test.tsx(2 hunks)src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx(0 hunks)src/screens/UserPortal/Chat/Chat.test.tsx(2 hunks)
💤 Files with no reviewable changes (2)
- schema.graphql
- src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx
✅ Files skipped from review due to trivial changes (1)
- src/GraphQl/Mutations/OrganizationMutations.ts
🔇 Additional comments (4)
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (2)
Line range hint 1-1000: Remove remaining group chat references from mock data.
The PR's objective is to remove unused references of group chat, but the test file still contains multiple instances of group chat related data:
GROUP_CHAT_BY_ID_QUERY_MOCKconstant nameisGroup: truein mock responses- Test descriptions mentioning "group chat"
This suggests that the cleanup of group chat references is incomplete.
#!/bin/bash
# Description: Check for remaining group chat references in test files
# Test: Search for group chat related terms. Expect: No occurrences
rg -i "group.?chat" "src/**/*.test.{ts,tsx}"
# Test: Search for isGroup field. Expect: No occurrences
rg '"isGroup":\s*true' "src/**/*.test.{ts,tsx}"Line range hint 359-385: Verify the consistency of message sent field name.
The change from messageSentToGroupChat to messageSentToChat aligns with the PR objective of removing group chat references. However, let's verify if this change is consistent across the codebase.
✅ Verification successful
The change from messageSentToGroupChat to messageSentToChat is consistent with the codebase
Based on the verification results:
- The
MESSAGE_SENT_TO_CHATsubscription is consistently used across all chat-related test files - The schema defines
messageSentToChatas the subscription field in the GraphQL schema - The field name change aligns with the PR objective while maintaining functionality, as the
isGroupflag in the Chat type properly distinguishes between group and direct chats
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of messageSentToGroupChat
# Test: Search for old field name. Expect: No occurrences of messageSentToGroupChat
rg "messageSentToGroupChat"
# Test: Verify the new field name usage
rg "messageSentToChat"
Length of output: 222
Script:
#!/bin/bash
# Let's try searching without directory restrictions
# Check for messageSentToGroupChat occurrences
rg "messageSentToGroupChat" --type-add 'test:*.{test.tsx,test.ts}' -t test
# Check for messageSentToChat occurrences
rg "messageSentToChat" --type-add 'test:*.{test.tsx,test.ts}' -t test
# Check for any GraphQL related files that might define these fields
fd "\.(graphql|gql)$" -x cat {}
# Look for subscription or mutation definitions
ast-grep --pattern 'const MESSAGE_SENT_TO_$_ = gql`
$$$
`'
Length of output: 29994
Script:
#!/bin/bash
# Let's search for the subscription definition and its usage
# Search for MESSAGE_SENT_TO_CHAT subscription definition
rg "MESSAGE_SENT_TO_CHAT" -A 10 -B 2
# Search for any remaining group chat related fields in test files
rg "isGroup|groupChat" --type-add 'test:*.{test.tsx,test.ts}' -t test
# Look for chat message related GraphQL fragments or types
ast-grep --pattern 'type ChatMessage {
$$$
}'
Length of output: 45844
src/screens/UserPortal/Chat/Chat.test.tsx (1)
Line range hint 701-715: LGTM! The mock data structure has been updated correctly.
The changes align with the PR objective of removing specific group chat references by updating the mock response structure from messageSentToGroupChat to messageSentToChat.
Let's verify if all chat-related references have been consistently updated across the codebase:
Also applies to: 727-741
✅ Verification successful
All chat-related references have been consistently updated across test files
The verification shows that all test files (Chat.test.tsx, CreateGroupChat.test.tsx, CreateDirectChat.test.tsx, and ChatRoom.test.tsx) are consistently using messageSentToChat in their mock data structures. There are no remaining references to messageSentToGroupChat, which aligns with the PR's objective of removing specific group chat references.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to group chat in the codebase
# Expect: No occurrences of messageSentToGroupChat in mock data or tests
# Search for any remaining references to messageSentToGroupChat
rg "messageSentToGroupChat"
# Search for any test files that might need similar updates
fd -e test.tsx -e test.ts --exec rg -l "messageSent.*Chat"
Length of output: 327
Script:
#!/bin/bash
# Let's check the content of these test files for any remaining references
# and ensure consistency in the chat-related naming
# Check CreateGroupChat.test.tsx
echo "=== CreateGroupChat.test.tsx ==="
rg "messageSent" src/components/UserPortal/CreateGroupChat/CreateGroupChat.test.tsx -A 2 -B 2
echo "\n=== CreateDirectChat.test.tsx ==="
rg "messageSent" src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx -A 2 -B 2
echo "\n=== ChatRoom.test.tsx ==="
rg "messageSent" src/components/UserPortal/ChatRoom/ChatRoom.test.tsx -A 2 -B 2
# Double check Chat.test.tsx for any inconsistencies
echo "\n=== Chat.test.tsx ==="
rg "messageSent" src/screens/UserPortal/Chat/Chat.test.tsx -A 2 -B 2
Length of output: 2412
src/components/UserPortal/CreateGroupChat/CreateGroupChat.test.tsx (1)
Line range hint 1237-1263: GraphQL schema changes are properly reflected in test mocks.
The mock data structure correctly implements the consolidated chat types, removing specific group chat references as intended.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2394 +/- ##
========================================
Coverage 98.06% 98.06%
========================================
Files 255 255
Lines 7169 7169
Branches 2085 2085
========================================
Hits 7030 7030
Misses 129 129
Partials 10 10 ☔ View full report in Codecov by Sentry. |
|
Please make adjustments to the code so that code rabbit approves the PR |
rishav-jha-mech
merged commit 2ca214d
into
PalisadoesFoundation:develop
What kind of change does this PR introduce?
Issue Number:
Fixes #
Did you add tests for your changes?
Snapshots/Videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?
Other information
Have you read the contributing guide?
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation