Skip to content

Conversation

@pt-tsl
Copy link
Contributor

@pt-tsl pt-tsl commented Dec 10, 2024

Acceptance Criteria
Business rules - Archiving and Archive Chat List
As a user I want to be able to archive selected chats and access them in a dedicated "Archived Chat List" so that I can keep my active chats organized while preserving old conversations for future reference.

When I click on the "Archive" button on a chat card overlay, the selected chat room is removed from my active chat list and moved to the "Archived Chat List."

When I navigate to the "Archived Chat List" section, I can view all archived chat rooms.

The Archived Chat rooms should be sorted by the timestamp in which they were archived.

Summary by CodeRabbit

  • New Features
    • Introduced functionality for archiving and unarchiving chat rooms.
    • Added new UnarchiveIcon to the design system.
  • Enhancements
    • Improved state management for unread and archived chat rooms in the ChatRoomsList component.
    • Enhanced ChatRoomItem component to handle archived and unread states.
    • Updated RoomsListFragment to support filtering of archived chat rooms.
    • Upgraded dependencies, including the design system to version 0.0.24.
  • Bug Fixes
    • Updated mutation handling for better management of chat room states.
  • Documentation
    • Updated changelog to reflect version 0.0.32 and the new features included.

@changeset-bot
Copy link

changeset-bot bot commented Dec 10, 2024

⚠️ No Changeset found

Latest commit: 9d90daa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2024

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: Cannot read config file: /packages/components/.eslintrc.js
Error: Cannot find module '@baseapp-frontend/config/.eslintrc.js'
Require stack:

  • /packages/components/.eslintrc.js
  • /node_modules/.pnpm/@eslint+eslintrc@2.1.4/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/cli-engine/cli-engine.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/eslint/eslint.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/eslint/index.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/lib/cli.js
  • /node_modules/.pnpm/eslint@8.57.1/node_modules/eslint/bin/eslint.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1248:15)
    at Module._load (node:internal/modules/cjs/loader:1074:27)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Module.require (node:internal/modules/cjs/loader:1339:12)
    at require (node:internal/modules/helpers:135:16)
    at Object. (/packages/components/.eslintrc.js:1:18)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)

Walkthrough

The changes in this pull request introduce version 0.0.32 of the @baseapp-frontend/components package, which adds functionality for archiving and unarchiving chat rooms. Key modifications include updates to the ChatRoomItem and ChatRoomsList components to manage chat room states based on their archived and unread status, the addition of a new GraphQL mutation for archiving chat rooms, and updates to relevant GraphQL queries and fragments. The package's changelog has also been updated to reflect these changes.

Changes

File Change Summary
packages/components/CHANGELOG.md Updated changelog to include version 0.0.32, detailing new archiving functionality and previous version changes.
packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx Added new props isInArchivedTab and isInUnreadTab to manage chat room states; integrated archiving mutation logic; updated profile destructuring.
packages/components/modules/messages/ChatRoomsList/ChatRoomItem/types.ts Added properties isInArchivedTab and isInUnreadTab to ChatRoomItemProps interface.
packages/components/modules/messages/ChatRoomsList/index.tsx Introduced state management for unread and archived tabs; updated handleChange function to refetch based on tab selection; passed new props to ChatRoomItem.
packages/components/modules/messages/CreateChatRoomList/ChatRoomListItem/index.tsx Updated mutation handling logic to support different chat room states based on message status.
packages/components/modules/messages/graphql/mutations/ArchiveChatRoom.ts Added a new GraphQL mutation for archiving chat rooms, including a custom hook useArchiveChatRoomMutation with error handling and notifications.
packages/components/modules/messages/graphql/queries/RoomsList.ts Introduced archived argument in RoomsListFragment to filter chat rooms based on archived status.
packages/components/modules/messages/graphql/subscriptions/useRoomListSubscription.tsx Added archived property to connection ID configuration for subscriptions.
packages/components/package.json Updated version from 0.0.31 to 0.0.32.

Possibly related PRs

  • BA-1831: Action overlay #141: The changes in this PR involve the ChatRoomItem component, which has been updated to include an ActionsOverlay that provides options for archiving and marking chat rooms as unread. This is directly related to the main PR's updates to the ChatRoomItem component, which also introduced new props for handling archived and unread states.
  • BA-1882: update unread mechanism #146: This PR modifies the ChatRoomsList component to include a subscription for real-time message count updates, which complements the main PR's enhancements to the ChatRoomItem and ChatRoomsList components, particularly in managing chat room states.
  • BA-1912: render newly created chats in chatrooms list #147: This PR focuses on rendering newly created chats in the chat rooms list, which aligns with the main PR's updates to the chat room functionalities, including the management of archived and unread states in the chat list.

Suggested reviewers

  • anicioalexandre

🐇 In a world where chats can hide,
Archiving rooms, we take in stride.
With buttons to click, and states to see,
Our chat rooms dance, so joyfully!
Version 0.0.32, a leap we take,
For every chat, a new path we make! 🌟

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. We would love to hear your feedback on Discord.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@pt-tsl pt-tsl force-pushed the BA-1912-display-new-chats branch from 7674772 to 9a1f958 Compare December 11, 2024 17:24
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from 954e83f to 1ea175c Compare December 11, 2024 17:25
@pt-tsl pt-tsl force-pushed the BA-1912-display-new-chats branch from 9a1f958 to e5d5d76 Compare December 11, 2024 17:33
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from 1ea175c to 00ec0e1 Compare December 11, 2024 17:33
@pt-tsl pt-tsl force-pushed the BA-1912-display-new-chats branch from e5d5d76 to e10730f Compare December 12, 2024 12:21
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch 2 times, most recently from 2b8c875 to 8c21d01 Compare December 12, 2024 12:27
@pt-tsl pt-tsl force-pushed the BA-1912-display-new-chats branch from e10730f to 32c764f Compare December 12, 2024 12:57
Base automatically changed from BA-1912-display-new-chats to master December 12, 2024 13:25
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from 1610fd6 to c4888c0 Compare December 12, 2024 13:44
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: 2

🧹 Outside diff range and nitpick comments (5)
packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx (1)

68-71: Update the label and icon to reflect archiving state

Currently, the "Archive Chat" action uses the same label and icon regardless of the chat's archived state. Consider updating the label and icon to "Unarchive Chat" when isInArchivedTab is true, to enhance user clarity.

Apply this diff to adjust the label and icon dynamically:

{
  disabled: isMutationInFlight,
- icon: <ArchiveIcon />,
- label: 'Archive Chat',
+ icon: isInArchivedTab ? <UnarchiveIcon /> : <ArchiveIcon />,
+ label: isInArchivedTab ? 'Unarchive Chat' : 'Archive Chat',
  onClick: () => {
    //...
  },
  hasPermission: true,
},

Ensure that you have an UnarchiveIcon component available, or use an appropriate existing icon.

packages/components/modules/messages/ChatRoomsList/ChatRoomItem/types.ts (1)

13-14: Make isInArchivedTab and isInUnreadTab optional in the interface

Since default values are set in the component, making these props optional enhances flexibility and avoids unnecessary prop requirements.

Apply this diff:

export interface ChatRoomItemProps {
  roomRef: RoomFragment$key
  isCardSelected?: boolean
  handleClick?: () => void
  Badge?: FC<BadgeProps>
  BadgeProps?: Partial<BadgeProps>
- isInArchivedTab: boolean
- isInUnreadTab: boolean
+ isInArchivedTab?: boolean
+ isInUnreadTab?: boolean
}
packages/components/modules/messages/graphql/mutations/ArchiveChatRoom.ts (1)

33-43: Consider enhancing error handling for field-specific errors.

The current implementation handles general errors well, but could be improved to handle field-specific errors from the GraphQL response.

Consider updating the error handling to utilize the field property:

 onCompleted: (response, errors) => {
   errors?.forEach((error) => {
-    sendToast(error.message, { type: 'error' })
+    const fieldErrors = response?.chatRoomArchive?.errors
+    if (fieldErrors?.length) {
+      fieldErrors.forEach(({ field, messages }) => {
+        messages.forEach(message => {
+          sendToast(`${field}: ${message}`, { type: 'error' })
+        })
+      })
+    }
   })
   config?.onCompleted?.(response, errors)
 },
packages/components/modules/messages/ChatRoomsList/index.tsx (1)

41-42: Consider memoizing tab state flags.

The boolean flags are recalculated on every render. Since they depend only on the tab state, they could be memoized for better performance.

- const isInUnreadTab = tab === CHAT_TAB_VALUES.unread
- const isInArchivedTab = tab === CHAT_TAB_VALUES.archived
+ const { isInUnreadTab, isInArchivedTab } = useMemo(
+   () => ({
+     isInUnreadTab: tab === CHAT_TAB_VALUES.unread,
+     isInArchivedTab: tab === CHAT_TAB_VALUES.archived
+   }),
+   [tab]
+ )
packages/components/modules/messages/graphql/queries/RoomsList.ts (1)

16-22: Consider adding database index for archived field

Since the archived field is now used as a filter parameter in the paginated query, consider adding an index on this field in the backend database to optimize query performance, especially as the number of chat rooms grows.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52f397f and c4888c090c9cf02dac9bcf9348537dfa8dc963c4.

⛔ Files ignored due to path filters (4)
  • packages/components/__generated__/ArchiveChatRoomMutation.graphql.ts is excluded by !**/__generated__/**
  • packages/components/__generated__/ChatRoomsQuery.graphql.ts is excluded by !**/__generated__/**
  • packages/components/__generated__/RoomsListFragment.graphql.ts is excluded by !**/__generated__/**
  • packages/components/__generated__/chatRoomsPaginationQuery.graphql.ts is excluded by !**/__generated__/**
📒 Files selected for processing (9)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx (5 hunks)
  • packages/components/modules/messages/ChatRoomsList/ChatRoomItem/types.ts (1 hunks)
  • packages/components/modules/messages/ChatRoomsList/index.tsx (3 hunks)
  • packages/components/modules/messages/CreateChatRoomList/ChatRoomListItem/index.tsx (1 hunks)
  • packages/components/modules/messages/graphql/mutations/ArchiveChatRoom.ts (1 hunks)
  • packages/components/modules/messages/graphql/queries/RoomsList.ts (1 hunks)
  • packages/components/modules/messages/graphql/subscriptions/useRoomListSubscription.tsx (1 hunks)
  • packages/components/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/components/package.json
🔇 Additional comments (8)
packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx (1)

72-98: ⚠️ Potential issue

Add error handling for the archive mutation

The mutation does not currently handle errors returned from the server. To improve user experience, consider adding onError and onCompleted callbacks to handle errors and provide user feedback if the archiving fails.

Apply this diff to include error handling:

commit({
  variables: {
    input: {
      roomId: room.id,
      profileId: currentProfile.id,
      archive: !isInArchivedTab,
    },
  },
  updater: (store, data) => {
    // existing updater logic
  },
+ onCompleted: (response, errors) => {
+   if (errors) {
+     // Handle errors, e.g., show an error notification
+     console.error('Archiving failed:', errors)
+   } else {
+     // Success logic if needed
+   }
+ },
+ onError: (error) => {
+   // Handle network or unexpected errors
+   console.error('Archiving encountered an error:', error)
+ },
})
packages/components/modules/messages/graphql/mutations/ArchiveChatRoom.ts (1)

7-19: LGTM! Well-structured GraphQL mutation.

The mutation is well-defined with proper error handling fields and follows GraphQL best practices.

packages/components/modules/messages/CreateChatRoomList/ChatRoomListItem/index.tsx (1)

52-56: LGTM! Proper connection handling for both unread and regular chat rooms.

The implementation correctly maintains separate connections for unread and regular chat rooms, ensuring both lists are updated appropriately when creating new chat rooms. Setting archived: false for both connections is correct as new chat rooms should never be archived.

packages/components/modules/messages/ChatRoomsList/index.tsx (1)

62-65: LGTM! Proper refetch implementation for tab changes.

The refetch implementation correctly updates the query variables based on the selected tab, with appropriate fetch policy.

packages/components/modules/messages/graphql/subscriptions/useRoomListSubscription.tsx (2)

22-22: LGTM: Correctly filters out archived rooms from active list

The addition of archived: false ensures that archived rooms are excluded from the active chat rooms list, which aligns with the PR objective.


26-26: LGTM: Correctly filters out archived rooms from unread list

The addition of archived: false ensures that archived rooms are excluded from the unread chat rooms list, maintaining consistency with the active list behavior.

Let's verify if we need a separate subscription for archived rooms:

✅ Verification successful

Based on the search results, I can now generate the final response:

Verified: Archived rooms are properly handled in the subscription system

The codebase shows that:

  • The archived: false filter is consistently used across both queries and subscriptions
  • The main chat rooms query (ChatRoomsQuery) and pagination query both include archived: false as a default parameter
  • There's no separate subscription needed for archived rooms as they are meant to be excluded from the active chat interface
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any GraphQL subscription handling archived rooms
rg -l "subscription.*archived.*chatRoom" --type ts --type tsx

# Check if there are any other files handling archived room updates
rg -l "archived.*chatRoom.*subscription" --type ts --type tsx

Length of output: 183


Script:

#!/bin/bash
# Search for any GraphQL subscription handling archived rooms with correct file extensions
rg -l "subscription.*archived.*chatRoom" -g "*.ts" -g "*.tsx"

# Check if there are any other files handling archived room updates
rg -l "archived.*chatRoom.*subscription" -g "*.ts" -g "*.tsx"

# Look for any GraphQL subscription definitions related to archived rooms
ast-grep --pattern 'subscription $_($$$) {
  $$$
  archived
  $$$
}'

# Search for any usage of archived rooms in subscriptions
rg "archived.*room" -A 5 -B 5 -g "*.{ts,tsx,graphql}"

Length of output: 254


Script:

#!/bin/bash
# Look for any GraphQL files or type definitions related to room subscriptions
fd -e graphql -e ts -e tsx | xargs rg "subscription.*Room" -A 5 -B 5

# Search for any files handling room queries or mutations with archived flag
fd -e graphql -e ts -e tsx | xargs rg "archived.*Room|Room.*archived" -A 5 -B 5

# Look specifically in the messages module for room-related GraphQL operations
fd . packages/components/modules/messages -e graphql -e ts -e tsx | xargs rg "Room.*{" -A 10

Length of output: 17421

packages/components/modules/messages/graphql/queries/RoomsList.ts (2)

13-13: LGTM: Well-defined archived argument

The archived argument is correctly defined with a Boolean type and default value of false, maintaining backward compatibility.


16-22: Verify connection key consistency

The query structure looks good with proper formatting and the archived parameter is correctly added. However, let's verify the connection key consistency.

Let's check if the connection key is used consistently across the codebase:

✅ Verification successful

Connection key is used consistently across the codebase

The connection key "roomsList_chatRooms" is used consistently across related components and subscriptions:

  • Query definition in RoomsList.ts
  • Subscription handling in useRoomListSubscription.tsx
  • Chat room components in ChatRoomItem and ChatRoomListItem
  • Generated GraphQL types

The implementation follows a consistent pattern with other @connection directives in the codebase, which also use descriptive, feature-specific keys.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any other usage of the roomsList_chatRooms connection key
rg "roomsList_chatRooms" --type ts --type tsx

# Check for any other @connection directives that might need the archived parameter
ast-grep --pattern '@connection($$$)'

Length of output: 113


Script:

#!/bin/bash
# Search for any other usage of the roomsList_chatRooms connection key with proper file type
rg "roomsList_chatRooms" -t ts

# Check for any other @connection directives that might need the archived parameter
# Using a more specific pattern to match GraphQL connection directive
rg "@connection\(.*\)" -t ts

Length of output: 2351

@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from c4888c0 to b07f039 Compare December 13, 2024 16:11
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

🧹 Outside diff range and nitpick comments (1)
packages/components/modules/messages/ChatRoomsList/index.tsx (1)

62-65: Consider adding TypeScript type for refetch parameters.

While the implementation is correct, consider defining an interface for the refetch parameters to improve type safety and documentation.

+ interface RoomsListRefetchParams {
+   unreadMessages?: boolean;
+   archived?: boolean;
+   q?: string;
+ }

  refetch(
-   {
-     unreadMessages: newTab === CHAT_TAB_VALUES.unread,
-     archived: newTab === CHAT_TAB_VALUES.archived,
-   },
+   {
+     unreadMessages: newTab === CHAT_TAB_VALUES.unread,
+     archived: newTab === CHAT_TAB_VALUES.archived,
+   } as RoomsListRefetchParams,
    { fetchPolicy: 'store-and-network' },
  )
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4888c090c9cf02dac9bcf9348537dfa8dc963c4 and b07f0392a1fb39cb99b7d6baa73829c889d9c156.

📒 Files selected for processing (3)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx (5 hunks)
  • packages/components/modules/messages/ChatRoomsList/index.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx
🧰 Additional context used
🪛 LanguageTool
packages/components/CHANGELOG.md

[uncategorized] ~9-~9: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...pdated ChatRoomsList to support archived and unread tabs - Added new GraphQL mutatio...

(COMMA_COMPOUND_SENTENCE_2)

🔇 Additional comments (4)
packages/components/CHANGELOG.md (1)

7-12: Enhance the changelog entries with consistent punctuation and technical details

The changelog entries should maintain consistent punctuation and provide clear technical context for developers.

Apply this diff to improve the changelog entries:

-Added archiving/unarchiving chat rooms functionality
-Enhanced ChatRoomItem with isInArchivedTab and isInUnreadTab props
-Updated ChatRoomsList to support archived and unread tabs
-Added new GraphQL mutation for archiving chat rooms
-Updated RoomsListFragment to support archived status
-Modified useRoomListSubscription to handle archived rooms
+- Added archiving/unarchiving chat rooms functionality:
+  - Enhanced ChatRoomItem with isInArchivedTab and isInUnreadTab props for conditional rendering
+  - Updated ChatRoomsList to support archived and unread tabs with filtered views
+  - Added new GraphQL mutation for archiving chat rooms with timestamp tracking
+  - Updated RoomsListFragment to support archived status filtering
+  - Modified useRoomListSubscription to handle real-time updates for archived rooms
🧰 Tools
🪛 LanguageTool

[uncategorized] ~9-~9: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...pdated ChatRoomsList to support archived and unread tabs - Added new GraphQL mutatio...

(COMMA_COMPOUND_SENTENCE_2)

packages/components/modules/messages/ChatRoomsList/index.tsx (3)

41-42: LGTM! Clear and semantic tab state management.

The new boolean constants provide clear semantic meaning and follow React's best practices for derived state.


96-96: Dependencies array correctly updated.

The renderItem callback dependencies now properly include all used values, addressing the previous review comment.


90-91: Verify ChatRoomItem implementation for new props.

The new props enable tab-specific rendering, but we should verify their implementation in the ChatRoomItem component.

✅ Verification successful

Props are correctly implemented in ChatRoomItem component

The verification shows that:

  • Props are properly typed in ChatRoomItem/types.ts
  • Props have default values (false) in ChatRoomItem/index.tsx
  • Props are used meaningfully in the component:
    • For archive mutation logic (archive: !isInArchivedTab)
    • For connection handling with correct filters ({ unreadMessages: isInUnreadTab, archived: isInArchivedTab })
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the ChatRoomItem component implementation to verify prop handling
ast-grep --pattern 'interface ChatRoomItemProps {
  $$$
  isInUnreadTab?: boolean;
  isInArchivedTab?: boolean;
  $$$
}'

# Search for usage of these props in the component
rg -A 5 'isInUnreadTab|isInArchivedTab' packages/components/modules/messages/ChatRoomItem

Length of output: 304


Script:

#!/bin/bash
# First, let's find the correct path to ChatRoomItem
fd ChatRoomItem

# Then search for the props interface and usage
rg -A 5 'isInUnreadTab|isInArchivedTab'

Length of output: 5283

tsl-ps2 and others added 4 commits December 16, 2024 08:18
* BA-1809 squash all previous commits

* BA-1809 Replacing catalogs temporarily, revert later

* BA-1809 Fix rebase mistakes

* Revert "BA-1809 Replacing catalogs temporarily, revert later"

This reverts commit 1071e6e2098c27d40eb28bce840c285805db1b80.

* BA-1809 Do not pass currentProfile as props

* BA-1809 Fix component tests

* BA-1809 Remove duplicated try ... catch

* BA-1809 Move useCurrentProfile into authentication package

* BA-1809 Remove profileId from Comment props and use useCurrentProfile instead

* BA-1809 set current profile in useLogin hook

* BA-1809 Fix tests

* BA-1809 changesets

* BA-1809 pnpm-lock.yaml

* BA-1809 Replace catalogs, revert later!

* BA-1809 Remove profile id from comment props

* BA-1809 Make image paths absolute

* BA-1809 Implement remaining gitlab suggestions

* BA-1809 Test whether env variable is undefined before using it

* tweaks

---------

Co-authored-by: Alexandre Anicio <aa@tsl.io>
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from b07f039 to f89873c Compare December 16, 2024 13:19
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from f89873c to bf84f3f Compare December 17, 2024 14:26
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from bf84f3f to 0369019 Compare December 17, 2024 14:42
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch 2 times, most recently from 2ef0570 to 1b917c7 Compare December 17, 2024 19:17
@pt-tsl pt-tsl force-pushed the BA-1917-archive-chats branch from 1b917c7 to 9d90daa Compare December 17, 2024 21:47
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
13.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

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: 2

🧹 Nitpick comments (5)
packages/design-system/components/icons/UnarchiveIcon/index.tsx (1)

5-5: Consider adding prop documentation

Adding JSDoc comments would improve maintainability by documenting the component's props and usage.

+/**
+ * UnarchiveIcon component - Displays an unarchive action icon
+ * @param {SvgIconProps} props - Material-UI SvgIcon props
+ * @returns {JSX.Element} Unarchive icon component
+ */
 const UnarchiveIcon: FC<SvgIconProps> = ({ sx, ...props }) => (
packages/components/schema.graphql (4)

1-1: Typographical Consistency: Use American English Spelling

In line 1, the word "behaviour" is used. Consider standardizing the spelling to American English ("behavior") for consistency across the codebase.

Apply this diff to correct the spelling:

-"""Exposes a URL that specifies the behaviour of this scalar."""
+"""Exposes a URL that specifies the behavior of this scalar."""

110-110: Parameter Naming Clarity in isArchived Field

In the ChatRoom type, the isArchived(profileId: ID): Boolean field uses profileId as a parameter. For better clarity, consider renaming it to currentProfileId or userProfileId to explicitly indicate whose profile ID is expected.

Apply this diff to rename the parameter:

-isArchived(profileId: ID): Boolean
+isArchived(userProfileId: ID): Boolean

Line range hint 169-171: Consistency in Field Naming for ChatRoomParticipant

The field hasArchivedRoom in the ChatRoomParticipant type could be renamed to isArchived to maintain consistency with the ChatRoom type.

Apply this diff for consistency:

-hasArchivedRoom: Boolean!
+isArchived: Boolean!

261-261: Typographical Error in Comment Description

There's a typo in the comment on line 261. It currently reads "languaged used in the comment." It should be corrected to "language used in the comment."

Apply this diff to fix the typo:

-"""languaged used in the comment"""
+"""Language used in the comment"""
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0369019 and 9d90daa.

📒 Files selected for processing (9)
  • packages/components/CHANGELOG.md (1 hunks)
  • packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx (5 hunks)
  • packages/components/schema.graphql (61 hunks)
  • packages/design-system/CHANGELOG.md (1 hunks)
  • packages/design-system/components/icons/UnarchiveIcon/index.tsx (1 hunks)
  • packages/design-system/components/icons/index.ts (1 hunks)
  • packages/design-system/package.json (1 hunks)
  • packages/wagtail/CHANGELOG.md (1 hunks)
  • packages/wagtail/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (4)
  • packages/wagtail/package.json
  • packages/design-system/package.json
  • packages/wagtail/CHANGELOG.md
  • packages/design-system/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/components/modules/messages/ChatRoomsList/ChatRoomItem/index.tsx
🔇 Additional comments (5)
packages/design-system/components/icons/index.ts (1)

21-21: LGTM! Export statement is well-placed

The UnarchiveIcon export follows the established pattern and maintains alphabetical ordering.

packages/design-system/components/icons/UnarchiveIcon/index.tsx (1)

5-18: LGTM! Well-structured icon component

The implementation follows Material-UI best practices with proper prop handling and theme integration.

packages/components/CHANGELOG.md (1)

3-15: LGTM! Comprehensive changelog entries

The changelog entries effectively document all significant changes related to the archiving functionality, including UI updates, component enhancements, and GraphQL changes.

packages/components/schema.graphql (2)

7-21: Well-Defined ActivityLog Type

The ActivityLog type is properly structured with essential fields for tracking user activities. This will enhance the auditing capabilities of the application.


657-658: Redundancy in NodeActivityLogInterface

The NodeActivityLogInterface interface defines the nodeActivityLogs field, which seems to replicate functionality in implementing types. Ensure that this interface is necessary and does not introduce redundancy. If it's needed, verify that all implementing types utilize it correctly.

Please confirm whether this interface is essential or if its functionality can be integrated directly into the relevant types.

Comment on lines +1656 to +1665
enum VisibilityTypes {
"""public"""
PUBLIC

"""private"""
PRIVATE

"""internal"""
INTERNAL
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validation of VisibilityTypes Enum Usage

The VisibilityTypes enum includes PUBLIC, PRIVATE, and INTERNAL. Ensure that these visibility levels are correctly implemented throughout the application to enforce proper access control and prevent unauthorized data exposure.

If assistance is needed to audit the usage of these visibility types, please let me know.

"""
hasPerm(perm: String!): Boolean
nodeActivityLogs(visibility: VisibilityTypes, first: Int = 10, offset: Int, before: String, after: String, last: Int, createdFrom: Date, createdTo: Date, userPk: Decimal, profilePk: Decimal): ActivityLogConnection
pk: Int!
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Inconsistent Argument Types in nodeActivityLogs Field

The nodeActivityLogs field uses userPk: Decimal and profilePk: Decimal as arguments. Elsewhere in the schema, IDs are typically of type ID or ID!. For consistency and type safety, consider changing these arguments to use the ID type.

Apply this diff to update the argument types:

-nodeActivityLogs(visibility: VisibilityTypes, first: Int = 10, offset: Int, before: String, after: String, last: Int, createdFrom: Date, createdTo: Date, userPk: Decimal, profilePk: Decimal): ActivityLogConnection
+nodeActivityLogs(visibility: VisibilityTypes, first: Int = 10, offset: Int, before: String, after: String, last: Int, createdFrom: Date, createdTo: Date, userPk: ID, profilePk: ID): ActivityLogConnection
📝 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.

Suggested change
nodeActivityLogs(visibility: VisibilityTypes, first: Int = 10, offset: Int, before: String, after: String, last: Int, createdFrom: Date, createdTo: Date, userPk: Decimal, profilePk: Decimal): ActivityLogConnection
nodeActivityLogs(visibility: VisibilityTypes, first: Int = 10, offset: Int, before: String, after: String, last: Int, createdFrom: Date, createdTo: Date, userPk: ID, profilePk: ID): ActivityLogConnection

@anicioalexandre anicioalexandre merged commit 05c6ec7 into master Dec 17, 2024
8 of 9 checks passed
@anicioalexandre anicioalexandre deleted the BA-1917-archive-chats branch December 17, 2024 22:04
@coderabbitai coderabbitai bot mentioned this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants