Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added features to filter chats, edit messages, add users to group chat, share images as messages, and create dedicated group chats for events #2360

Open
wants to merge 72 commits into
base: develop-postgres
Choose a base branch
from

Conversation

disha1202
Copy link

@disha1202 disha1202 commented Oct 27, 2024

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

  • New Features

    • Enhanced chat functionalities across multiple languages, including options to create chats and manage group chats.
    • Added a checkbox for creating a chat when creating events.
    • Introduced filtering options for chat views (all, unread, group).
    • New state management for chat interactions and event creation.
  • Bug Fixes

    • Improved error handling in event creation.
  • Documentation

    • Updated tests to cover new chat functionalities and event creation scenarios.
  • Style

    • New CSS classes added to improve layout and presentation of chat and group chat components.
  • Tests

    • Expanded test coverage for chat functionalities and event creation processes.

Copy link

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 github-actions bot added the no-pr-activity No pull request activity label Nov 23, 2024
@palisadoes
Copy link
Contributor

@disha1202 Are you still working on this? Please fix the failing tests and conflicting files

@github-actions github-actions bot removed the no-pr-activity No pull request activity label Nov 24, 2024
@palisadoes
Copy link
Contributor

palisadoes commented Nov 29, 2024

@disha1202 We have made significant changes to the develop-postgres branch that there will be major conflicts with this PR when we merge.

Can you close this and merge against that branch?

@palisadoes palisadoes changed the base branch from develop to develop-postgres November 30, 2024 05:27
@palisadoes
Copy link
Contributor

@disha1202 I rebased your PR against the develop-postgres branch. We did some changes to the UI/UX color scheme. Please make sure that those are maintained when you refactor the conflicting files.

@Cioppolo14
Copy link

@disha1202 I am closing this as it seems inactive.

@Cioppolo14 Cioppolo14 closed this Dec 13, 2024
@palisadoes
Copy link
Contributor

Reopening. GSoC

@gurramkarthiknetha
Copy link

If no one is currently working on this issue, I would like to take it on. Please assign it to me.

@palisadoes palisadoes added the wip Work in Progress label Dec 21, 2024
@palisadoes
Copy link
Contributor

Please fix:

  1. Any failing tests (Click on the details link for more information)
  2. Any conflicting files

Make sure CodeRabbit.ai approves your changes

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (6)
public/locales/zh/translation.json (1)

583-584: 🛠️ Refactor suggestion

Consolidate duplicate chat-related translations.

The translations for "done" and "createChat" in the "eventListCard" section are duplicates of those in "organizationEvents". These should be consolidated in the "userChat" section.

Apply this diff to remove the duplicates:

  "eventListCard": {
-   "done": "完成",
-   "createChat": "创建聊天"
  }
public/locales/hi/translation.json (3)

442-443: 🛠️ Refactor suggestion

Move chat-related translations to the appropriate section.

The translations for "done" and "createChat" appear to be misplaced in the "organizationEvents" section. These strings are related to chat functionality and should be moved to the "userChat" section.

Apply this diff to move the translations to the appropriate section:

  "organizationEvents": {
-   "done": "पूर्ण",
-   "createChat": "चैट बनाएं"
  },
  "userChat": {
+   "done": "पूर्ण",
+   "createChat": "चैट बनाएं"
  }

583-584: 🛠️ Refactor suggestion

Consolidate duplicate chat-related translations.

The translations for "done" and "createChat" in the "eventListCard" section are duplicates. These should be consolidated in the "userChat" section.

Apply this diff to remove the duplicates:

  "eventListCard": {
-   "done": "समाप्त",
-   "createChat": "चैट बनाएं"
  }

1264-1264: ⚠️ Potential issue

Translate "Add Members" to Hindi.

The text "Add Members" should be translated to Hindi to maintain consistency with the rest of the translations.

Apply this diff to fix the translation:

-    "addMembers": "Add Members"
+    "addMembers": "सदस्य जोड़ें"
public/locales/fr/translation.json (2)

442-443: 🛠️ Refactor suggestion

Move chat-related translations to the appropriate section.

The translations for "done" and "createChat" appear to be misplaced in the "organizationEvents" section. These strings are related to chat functionality and should be moved to the "userChat" section.

Apply this diff to move the translations to the appropriate section:

  "organizationEvents": {
-   "done": "Fait",
-   "createChat": "Créer une discussion"
  },
  "userChat": {
+   "done": "Fait",
+   "createChat": "Créer une discussion"
  }

1264-1264: ⚠️ Potential issue

Translate "Add Members" to French.

The text "Add Members" should be translated to French to maintain consistency with the rest of the translations.

Apply this diff to fix the translation:

-    "addMembers": "Add Members"
+    "addMembers": "Ajouter des membres"
🧹 Nitpick comments (4)
src/screens/UserPortal/Events/Events.spec.tsx (1)

570-572: Double toggle of createChatCheck
This toggles the checkbox on and off. If intended to test multiple toggles, that’s fine. Otherwise, consider if one toggle suffices for coverage.

- userEvent.click(screen.getByTestId('createChatCheck'));
- userEvent.click(screen.getByTestId('createChatCheck'));
+ // Possibly remove one toggle if double toggling isn't strictly necessary.
src/GraphQl/Queries/PlugInQueries.ts (1)

304-314: Consider optimizing the search conditions

The current search implementation in the where clause combines multiple conditions. Consider:

  1. Using OR instead of AND for more flexible matching
  2. Adding a search across message content
  3. Implementing fuzzy matching for better user experience
- where: {
-   name_contains: $searchString
-   user: {
-     firstName_contains: $searchString
-     lastName_contains: $searchString
-   }
- }
+ where: {
+   OR: [
+     { name_contains: $searchString },
+     { "users.firstName_contains": $searchString },
+     { "users.lastName_contains": $searchString },
+     { "messages.messageContent_contains": $searchString }
+   ]
+ }
src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)

3744-3841: Enhance test coverage for message marking

The MARK_CHAT_MESSAGES_AS_READ mock should include additional test cases:

  1. Invalid chat IDs
  2. Error scenarios
  3. Batch marking of multiple messages

Add these test cases:

test('should handle error when marking messages as read', async () => {
  // Test implementation for error handling
});

test('should handle invalid chat ID', async () => {
  // Test implementation for invalid chat ID
});

test('should handle batch marking of messages', async () => {
  // Test implementation for batch operations
});
src/components/GroupChatDetails/GroupChatDetails.test.tsx (1)

71-73: Simplify JSON.parse usage for mock data

Using JSON.parse for static objects is unnecessary and reduces readability.

-unseenMessagesByUsers: JSON.parse(
-  '{"hbjguyt7y9890i9otyugttiyuh": 0, "gjhnbbjku68979089ujhvserty": 0}',
-),
+unseenMessagesByUsers: {
+  "hbjguyt7y9890i9otyugttiyuh": 0,
+  "gjhnbbjku68979089ujhvserty": 0
+},

Also applies to: 426-428

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 566e8c2 and 8a6c1d0.

📒 Files selected for processing (15)
  • public/locales/en/translation.json (4 hunks)
  • public/locales/fr/translation.json (4 hunks)
  • public/locales/hi/translation.json (4 hunks)
  • public/locales/sp/translation.json (4 hunks)
  • public/locales/zh/translation.json (4 hunks)
  • schema.graphql (1 hunks)
  • src/App.tsx (1 hunks)
  • src/GraphQl/Mutations/OrganizationMutations.ts (1 hunks)
  • src/GraphQl/Queries/PlugInQueries.ts (4 hunks)
  • src/components/GroupChatDetails/GroupChatDetails.test.tsx (1 hunks)
  • src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (21 hunks)
  • src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx (4 hunks)
  • src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (10 hunks)
  • src/screens/OrganizationEvents/OrganizationEvents.tsx (2 hunks)
  • src/screens/UserPortal/Events/Events.spec.tsx (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • schema.graphql
  • src/components/UserPortal/CreateDirectChat/CreateDirectChat.tsx
  • public/locales/sp/translation.json
  • src/App.tsx
  • src/screens/OrganizationEvents/OrganizationEvents.tsx
  • public/locales/en/translation.json
🧰 Additional context used
📓 Learnings (1)
src/GraphQl/Queries/PlugInQueries.ts (1)
Learnt from: disha1202
PR: PalisadoesFoundation/talawa-admin#2394
File: src/GraphQl/Queries/PlugInQueries.ts:103-106
Timestamp: 2024-11-12T10:41:04.351Z
Learning: The `isGroup` field in the `Chat` type is essential for distinguishing between group chats and direct chats and should not be removed.
🔇 Additional comments (28)
src/screens/UserPortal/Events/Events.spec.tsx (13)

101-101: Consistent introduction of the createChat field for mocked data
The addition of "createChat: false" for this event aligns with the new chat creation feature. Ensure that all tests correctly handle events with and without chat creation.


133-133: Validation of createChat set to true
Including an event that has "createChat: true" is a good test scenario to validate chat creation. Confirm that the UI logic correctly differentiates between "createChat: true" and "createChat: false."


184-184: Coverage for createChat in additional event
This maintains consistency in mocked data. Make sure there is a test path verifying behavior when createChat is false for an event that is otherwise public and non-registerable.


213-233: Inclusion of createChat in CREATE_EVENT_MUTATION variables
Adding "createChat" to the mutation payload is correct. Be sure your test covers both toggled and untoggled states.
[approve]


234-260: Repeated mutation block with createChat
No issues spotted. Ensure you keep the data consistent with the feature requirements and that you have tests for this scenario.


261-287: Consistent usage of createChat throughout
Continuing to use "createChat: false" in the test data adheres to consistent coverage. Confirm that the UI handles repeated event creation calls with varying createChat states.


Line range hint 288-314: Multiple test entries for event creation
It’s good to see different test mocks. Consider verifying that each event variant (with or without chat) is tested to prevent regressions.


321-332: Testing createChat edge cases
Testing "createChat: false" again ensures coverage of non-chat events. No immediate concerns.


370-386: Mutation data block consistency
Ensure the date/time generation logic and the createChat field are tested for correctness, especially with different time zones.


425-440: Standardized usage of mutation variables
No anomalies found. Keep in mind potential interactions between new UI elements (the createChat checkbox) and these mocked variable sets.


451-463: Positive test path for createChat = true
This scenario ensures an all-day event with a chat. Confirm the final coverage includes verifying the creation of that chat via UI or GraphQL mocks.


397-413: Maintaining alignment between FE and BE
The “createChat: false” property is consistently applied. Verify that the server-side schema can handle the new property gracefully.


343-359: Redundant or repeated test structures
The repeated blocks might be combining coverage scenarios. Double-check any changes in other files (like resolvers) to ensure the logic matches exactly what’s mocked here.

src/GraphQl/Queries/PlugInQueries.ts (3)

170-170: LGTM: Media field added to support image sharing

The addition of the media field to message objects enables image sharing functionality in chats.


Line range hint 257-303: LGTM: Well-structured unread chats query

The UNREAD_CHAT_LIST query is properly implemented with all necessary fields for handling unread messages and chat details.


210-255: Verify pagination for group chat list

The GROUP_CHAT_LIST query looks well-structured but consider implementing pagination to handle large numbers of group chats efficiently.

Run this script to check if the number of group chats could be significant:

src/components/UserPortal/CreateDirectChat/CreateDirectChat.test.tsx (1)

467-479: Refactor mock data to improve maintainability

The mock data contains significant duplication. Consider extracting common objects and using factory functions as suggested in previous reviews.

Also applies to: 540-552, 613-625

src/GraphQl/Mutations/OrganizationMutations.ts (2)

66-66: LGTM: Image parameter added to CREATE_CHAT mutation

The addition of the image parameter aligns with the PR objectives for supporting image sharing in chats.

Also applies to: 74-74


90-96: ⚠️ Potential issue

Security concern: userId parameter in MARK_CHAT_MESSAGES_AS_READ

The mutation accepts userId as a parameter, which could allow users to mark messages as read for other users. Consider retrieving the userId from the authenticated session instead.

Apply this diff to fix the security issue:

-export const MARK_CHAT_MESSAGES_AS_READ = gql`
-  mutation markChatMessagesAsRead($chatId: ID!, $userId: ID!) {
-    markChatMessagesAsRead(chatId: $chatId, userId: $userId) {
+export const MARK_CHAT_MESSAGES_AS_READ = gql`
+  mutation markChatMessagesAsRead($chatId: ID!) {
+    markChatMessagesAsRead(chatId: $chatId) {
       _id
     }
   }
`;

Likely invalid or redundant comment.

src/components/UserPortal/CreateGroupChat/CreateGroupChat.tsx (2)

217-217: ⚠️ Potential issue

Fix typo in data-testid attribute

The attribute data-tsetid should be data-testid.

-data-tsetid="groupDescriptionInput"
+data-testid="groupDescriptionInput"

Likely invalid or redundant comment.


151-159: 🛠️ Refactor suggestion

Add file validation to handleImageChange

The image upload handler lacks important validations:

  1. File size limits
  2. File type restrictions
  3. Error handling for base64 conversion

Apply this diff to add validation:

 const handleImageChange = async (
   e: React.ChangeEvent<HTMLInputElement>,
 ): Promise<void> => {
   const file = e.target.files?.[0];
   if (file) {
+    if (file.size > 5 * 1024 * 1024) {
+      alert(t('fileSizeError'));
+      return;
+    }
+    if (!file.type.match(/^image\/(jpeg|png|gif)$/)) {
+      alert(t('fileTypeError'));
+      return;
+    }
+    try {
       const base64 = await convertToBase64(file);
       setSelectedImage(base64);
+    } catch (error) {
+      console.error('Error converting image:', error);
+      alert(t('imageProcessingError'));
+    }
   }
 };

Likely invalid or redundant comment.

src/components/GroupChatDetails/GroupChatDetails.test.tsx (1)

24-30: 🛠️ Refactor suggestion

Replace custom wait function with React Testing Library utilities

The custom wait function should be replaced with React Testing Library's built-in waitFor utility for better reliability and maintainability.

Remove the custom wait function and update its usages:

-async function wait(ms = 100): Promise<void> {
-  await act(() => {
-    return new Promise((resolve) => {
-      setTimeout(resolve, ms);
-    });
-  });
-}

// Replace usages with waitFor:
-await wait();
+await waitFor(() => {
+  // Add an assertion here
+}, { timeout: 1000 });

Likely invalid or redundant comment.

public/locales/zh/translation.json (3)

1268-1269: LGTM! Chat room translations look good.

The translations in the userChatRoom section are properly placed and correctly translated to Chinese.


1254-1264: ⚠️ Potential issue

Translate "Add Members" to Chinese.

The text "Add Members" should be translated to Chinese to maintain consistency with the rest of the translations.

Apply this diff to fix the translation:

-    "addMembers": "Add Members"
+    "addMembers": "添加成员"

Likely invalid or redundant comment.


442-443: 🛠️ Refactor suggestion

Move chat-related translations to the appropriate section.

The translations for "done" and "createChat" appear to be misplaced in the "organizationEvents" section. These strings are related to chat functionality and should be moved to the "userChat" section for better organization and maintainability.

Apply this diff to move the translations to the appropriate section:

  "organizationEvents": {
-   "done": "完成",
-   "createChat": "创建聊天"
  },
  "userChat": {
+   "done": "完成",
+   "createChat": "创建聊天"
  }

Likely invalid or redundant comment.

public/locales/fr/translation.json (3)

1268-1269: 🛠️ Refactor suggestion

Fix inconsistent capitalization in chat room translations.

The reply action should start with a capital letter to maintain consistency with other translations.

Apply this diff to fix the capitalization:

-    "reply": "répondre"
+    "reply": "Répondre"

Likely invalid or redundant comment.


583-584: ⚠️ Potential issue

Fix inconsistent translation for "done".

The key "done" is translated differently across sections:

  • "Fait" in organizationEvents
  • "Terminé" in eventListCard

This inconsistency should be resolved by using the same translation throughout.

Apply this diff to make the translation consistent:

  "eventListCard": {
-   "done": "Terminé",
+   "done": "Fait",
    "createChat": "Créer une discussion"
  }

Likely invalid or redundant comment.


1254-1264: 🛠️ Refactor suggestion

Fix inconsistent capitalization in chat-related translations.

French translations should maintain consistent capitalization. Currently, some translations start with lowercase letters while most translations in the file start with uppercase.

Apply this diff to fix the capitalization:

-    "chat": "Chat",
-    "search": "rechercher",
-    "messages": "messages",
-    "create": "créer",
-    "newChat": "nouvelle discussion",
+    "chat": "Chat",
+    "search": "Rechercher",
+    "messages": "Messages",
+    "create": "Créer",
+    "newChat": "Nouvelle discussion",

Likely invalid or redundant comment.

Comment on lines +3867 to +3877
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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add test cases for error scenarios and edge cases

Current test coverage focuses on happy path scenarios. Consider adding tests for:

  1. Network errors
  2. Invalid user selections
  3. Maximum user limits
  4. Duplicate user selections

Add these 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
});

test('should handle empty user selection', async () => {
  // Test implementation
});

test('should handle maximum users limit', async () => {
  // Test implementation
});

Comment on lines +82 to +88
export const ADD_USER_TO_GROUP_CHAT = gql`
mutation addUserToGroupChat($userId: ID!, $chatId: ID!) {
addUserToGroupChat(userId: $userId, chatId: $chatId) {
_id
}
}
`;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider enhancing error handling in ADD_USER_TO_GROUP_CHAT mutation

While the mutation is well-structured, consider adding:

  1. Error handling for when the user is already in the group
  2. Validation to ensure the chat exists and is a group chat
  3. Permission checks to verify if the requester has rights to add users

Comment on lines +130 to +131
$media: String
$messageContent: String
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation to SEND_MESSAGE_TO_CHAT mutation

The mutation allows both messageContent and media to be optional. Consider adding validation to ensure at least one is provided to prevent empty messages.

Also applies to: 137-137

fireEvent.click(await screen.findByTestId('addUserBtn'));
});

await wait(10000);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reduce excessive wait times

The wait time of 10000ms is excessive and could slow down the test suite.

-await wait(10000);
+await waitFor(() => {
+  // Add specific assertions here
+}, { timeout: 2000 });

Also applies to: 601-601

Comment on lines +515 to +563
it('add user to group chat', async () => {
setItem('userId', 'hbjguyt7y9890i9otyugttiyuh');
render(
<I18nextProvider i18n={i18n}>
<MockedProvider mocks={mocks} addTypename={false}>
<GroupChatDetails
toggleGroupChatDetailsModal={jest.fn()}
groupChatDetailsModalisOpen={true}
chat={mockChat}
chatRefetch={jest.fn()}
/>
</MockedProvider>
</I18nextProvider>,
);

await wait();
await act(async () => {
fireEvent.click(await screen.findByTestId('addMembers'));
});

await waitFor(async () => {
expect(await screen.findByTestId('searchUser')).toBeInTheDocument();
});

await act(async () => {
fireEvent.change(await screen.findByTestId('searchUser'), {
target: { value: 'Disha' },
});
});

await act(async () => {
fireEvent.click(await screen.findByTestId('searchBtn'));
});

await wait();

await waitFor(
async () => {
expect(await screen.findByTestId('user')).toBeInTheDocument();
},
{ timeout: 5000 },
);

await act(async () => {
fireEvent.click(await screen.findByTestId('addUserBtn'));
});

await wait(10000);
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling tests

The test suite lacks coverage for error scenarios when adding users to group chat.

Add test cases for:

  1. Adding a user who is already in the group
  2. Network errors during user addition
  3. Permission denied scenarios

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
schema.graphql (2)

511-511: Consider using a more type-safe approach for tracking unseen messages.

While using JSON type provides flexibility, it may lead to runtime errors due to lack of type safety. Consider creating a dedicated type for tracking unseen messages.

type UnseenMessages {
  userId: ID!
  lastSeenMessageId: ID!
  unseenCount: Int!
}

type Chat {
  # ... other fields
  unseenMessagesByUsers: [UnseenMessages!]
}

697-698: LGTM! Consider adding documentation.

The mutation changes look good. The removal of the type parameter simplifies the API, and the new markChatMessagesAsRead mutation is well-structured.

Consider adding documentation comments to describe:

  • Expected behavior when marking messages as read
  • Whether it marks all messages or only up to the latest
  • Any side effects on the unseenMessagesByUsers field
+"""
+Marks messages in a chat as read for a specific user.
+Updates the unseenMessagesByUsers field in the Chat type.
+"""
markChatMessagesAsRead(chatId: ID!, userId: ID!): Chat
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a6c1d0 and 44942ab.

📒 Files selected for processing (1)
  • schema.graphql (2 hunks)

@Cioppolo14
Copy link

@disha1202 Please fix the failed tests & coderabbit comments.

@khushipatil1523
Copy link

@palisadoes If no one is working on this issue, can I work on it?

@palisadoes
Copy link
Contributor

@disha1202 is working on it

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/state/reducers/userRoutersReducer.spec.ts (1)

19-19: Ensure consistent handling for "undefined" organization IDs.
Currently, the URL uses /user/chat/undefined if the organization ID is not provided. To improve clarity, consider conditionally omitting the organization part of the path if not needed, or provide a placeholder.

public/locales/sp/translation.json (1)

1268-1278: Fix and improve chat-related translations

  1. The text "Add Members" at line 1278 needs to be translated to Spanish.
  2. Some translations could be improved for better Spanish localization:
-    "addMembers": "Add Members"
+    "addMembers": "Agregar miembros"
-    "newChat": "nueva charla"
+    "newChat": "Nueva conversación"
-    "create": "crear"
+    "create": "Crear"

Also applies to: 1282-1283

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b3f412 and 33b5f24.

📒 Files selected for processing (9)
  • public/locales/en/translation.json (4 hunks)
  • public/locales/fr/translation.json (4 hunks)
  • public/locales/hi/translation.json (4 hunks)
  • public/locales/sp/translation.json (4 hunks)
  • public/locales/zh/translation.json (4 hunks)
  • src/App.tsx (1 hunks)
  • src/components/UserPortal/UserSidebar/UserSidebar.test.tsx (2 hunks)
  • src/state/reducers/userRoutersReducer.spec.ts (4 hunks)
  • src/state/reducers/userRoutesReducer.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/App.tsx
  • public/locales/zh/translation.json
  • src/state/reducers/userRoutesReducer.ts
🔇 Additional comments (10)
src/state/reducers/userRoutersReducer.spec.ts (3)

43-43: Check UI or routing coverage for the "Chat" component.
The addition of this component in the reducer’s initial state looks clean. Verify that corresponding routes and UI references properly use this component.


73-73: Great job updating the URL with the orgId.
This ensures that multiple organizations can have unique chat endpoints and handle context properly.


97-97: Verify alignment of component naming with the rest of the codebase.
The Chat component is consistent with the naming convention of other components (e.g., Events, Posts).

src/components/UserPortal/UserSidebar/UserSidebar.test.tsx (2)

410-410: Confirm intentional removal of 'Chat' link from the navigation.

The expectedLinks array no longer includes 'Chat'. Ensure you have removed or relocated all chat references if chat functionality is indeed being eliminated or moved.


467-468: Test logic aligned with updated sidebar links.

Switching the click event from 'Chat' to 'Settings' reflects the updated navigation flow. The test accurately verifies drawer closure.

public/locales/fr/translation.json (2)

1279-1280: 🛠️ Refactor suggestion

Fix capitalization in reply translation.

The translation for "reply" should start with an uppercase letter to maintain consistency with other translations.

Apply this diff:

  "userChatRoom": {
    "selectContact": "Sélectionnez un contact pour démarrer la conversation",
    "sendMessage": "Envoyer le message",
-   "reply": "répondre"
+   "reply": "Répondre"
  }

Likely invalid or redundant comment.


1265-1275: 🛠️ Refactor suggestion

Fix inconsistent capitalization in chat-related translations.

Some translations start with lowercase letters while most translations in the file start with uppercase. This should be consistent throughout the file.

Apply this diff to fix capitalization:

  "userChat": {
    "add": "Ajouter",
-   "chat": "Chat",
-   "search": "rechercher",
-   "messages": "messages",
-   "create": "créer",
-   "newChat": "nouvelle discussion",
+   "chat": "Chat",
+   "search": "Rechercher",
+   "messages": "Messages",
+   "create": "Créer",
+   "newChat": "Nouvelle discussion",
    "newGroupChat": "Nouvelle discussion de groupe",
    "groupInfo": "Informations sur le groupe",
    "members": "Membres",
-   "addMembers": "Add Members"
+   "addMembers": "Ajouter des membres"
  }

Likely invalid or redundant comment.

public/locales/en/translation.json (1)

417-417: LGTM! English translations are consistent and complete.

The chat-related translations are properly structured and use consistent capitalization.

Also applies to: 557-557, 1209-1209, 1265-1275, 1279-1280

public/locales/hi/translation.json (1)

442-443: LGTM! Hindi translations are consistent and complete.

The chat-related translations are properly localized and use consistent formatting.

Also applies to: 583-584, 1220-1221, 1265-1275, 1279-1280

public/locales/sp/translation.json (1)

442-443: LGTM! Event-related translations are well done

The translations for event-related features are accurate and follow proper Spanish language conventions.

Also applies to: 1223-1224

Comment on lines +442 to +443
"done": "Fait",
"createChat": "Créer une discussion"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix inconsistent translation for "done".

The key "done" is translated differently across sections:

  • "Fait" in organizationEvents
  • "Terminé" in eventListCard

This inconsistency should be resolved by using the same translation throughout.

Apply this diff to use consistent translation:

  "organizationEvents": {
-   "done": "Fait",
+   "done": "Terminé",
    "createChat": "Créer une discussion"
  },
  "eventListCard": {
    "done": "Terminé",
    "createChat": "Créer une discussion"
  }

Also applies to: 583-584

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC Priority wip Work in Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants