-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[WEB-2370]chore: activity filters #6324
Conversation
chore: added sort order to local storage
WalkthroughThe pull request introduces changes to the issue activity management system, focusing on sorting and filtering mechanisms. The modifications include updating the Changes
Sequence DiagramsequenceDiagram
participant User
participant Component
participant LocalStorage
participant Store
User->>Component: Interact with sort/filter
Component->>LocalStorage: Save sort order/filters
LocalStorage-->>Component: Retrieve stored values
Component->>Store: Call method with sort order
Store->>Store: Apply sorting
Store-->>Component: Return sorted activities
Component->>User: Display sorted activities
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
CodeRabbit Configuration File (
|
There was a problem hiding this 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 (3)
web/core/components/issues/issue-detail/issue-activity/sort-root.tsx (1)
6-11
: Use the enum values directly in conditionals.Currently, the conditional checks rely on the literal strings
"asc"
or"desc"
. SincesortOrder
is defined asTSORT_ORDER
, consider usingTSORT_ORDER.ASC
orTSORT_ORDER.DESC
to maintain strict typing and avoid potential mismatches.{props.sortOrder === TSORT_ORDER.ASC ? ( <ArrowUpWideNarrow className={cn("size-4", props.iconClassName)} /> ) : ( <ArrowDownWideNarrow className={cn("size-4", props.iconClassName)} /> )}web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (1)
30-39
: Redundant empty object destructuring.The
comment: {}
destructuring returns an empty pattern. If it's not used, remove it to avoid confusion or consider expanding it if there are intended properties.- const { - activity: { getActivityCommentByIssueId }, - comment: {}, - } = useIssueDetail(); + const { + activity: { getActivityCommentByIssueId }, + // comment: {}, // Remove or include relevant properties if needed. + } = useIssueDetail();web/core/components/issues/issue-detail/issue-activity/root.tsx (1)
45-49
: Encourage fallback checks.Storing filters in local storage is useful. However, if the stored data is corrupted or an unexpected type, consider adding a fallback or validation to avoid potential runtime errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/ce/store/issue/issue-details/activity.store.ts
(4 hunks)web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx
(2 hunks)web/core/components/issues/issue-detail/issue-activity/activity-filter.tsx
(1 hunks)web/core/components/issues/issue-detail/issue-activity/root.tsx
(6 hunks)web/core/components/issues/issue-detail/issue-activity/sort-root.tsx
(1 hunks)web/core/constants/common.ts
(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx
[error] 43-43: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint-web
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (17)
web/core/constants/common.ts (1)
31-35
: Well-defined enum for sort order.Declaring a dedicated enum for
ASC
andDESC
fosters strong type checks and simplifies the promotion of consistent usage across the codebase.web/core/components/issues/issue-detail/issue-activity/activity-filter.tsx (1)
29-31
: Nicely implemented indicator for modified filters.Displaying a small dot to indicate partially selected filters is a clear visual cue. Good job.
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (3)
3-4
: Enum import is appropriate.Importing
TSORT_ORDER
from the common constants ensures consistent usage of the sort order enum.
26-26
: Good property addition for maintaining consistent sorting.Adding
sortOrder
to the prop list aligns well with the new sorting approach.
46-46
: Sorting query usage is clear.Calling
getActivityCommentByIssueId(issueId, sortOrder)
is consistent with the new enum-based approach. Validate that the store function behaves correctly and returns results in the specified order.✅ Verification successful
Sort order implementation is correctly handling comment ordering.
The
getActivityCommentByIssueId
function properly implements sorting using thesortOrder
parameter to order comments by their creation timestamp. The implementation is type-safe and correctly handles the sorting direction.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that getActivityCommentByIssueId responds correctly to the 'sortOrder' argument. rg -A 5 $'getActivityCommentByIssueId(issueId, sortOrder)'Length of output: 59
Script:
#!/bin/bash # Find function definition ast-grep --pattern 'function getActivityCommentByIssueId($_) { $$$ }' # Find export of this function ast-grep --pattern 'export const getActivityCommentByIssueId = $_' # Look for imports and usage rg "getActivityCommentByIssueId" # Look for sort order related code rg -i "sortorder" -A 5Length of output: 34568
web/ce/store/issue/issue-details/activity.store.ts (4)
19-19
: Consistent introduction of a typed sort order.Importing
TSORT_ORDER
clarifies the valid sort values and improves type safety throughout.
87-87
: Leverage computedFn for memoization.Using mobx-utils’
computedFn
is great for memoization based on strict arguments. Make sure there’s no global state usage inside that could invalidate or incorrectly cache results.
118-118
: Validate date parsing edge cases.
new Date(e.created_at || 0)
might parse a fallback epoch date whencreated_at
is undefined or null. Consider verifying all activities consistently have valid timestamps or provide a fallback strategy.
46-46
: Ensure all callsites provide the new parameter.Accepting
sortOrder
as a required parameter is a breaking change. Verify that all existing code that callsgetActivityCommentByIssueId
is updated to supplysortOrder
.✅ Verification successful
All callsites properly provide the new
sortOrder
parameterThe only callsite in
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx
correctly provides bothissueId
andsortOrder
parameters.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all callsites of getActivityCommentByIssueId to confirm they pass the new parameter rg -A 4 'getActivityCommentByIssueId\('Length of output: 720
Script:
#!/bin/bash # Search for any destructuring or references of getActivityCommentByIssueId rg 'getActivityCommentByIssueId' # Search for potential destructuring patterns using AST ast-grep --pattern 'const { $$$, getActivityCommentByIssueId, $$$ } = $_'Length of output: 698
web/core/components/issues/issue-detail/issue-activity/root.tsx (8)
3-3
: Minimal import changes are fine.The addition of
FC, useMemo
import is consistent with React conventions.
5-5
: Local storage hook usage is a good approach.Using
useLocalStorage
is an elegant solution for persisting user preferences across sessions.
68-78
: Prevent empty filters edge cases.By returning early when
selectedFilters
is empty or undefined, you avoid unintended states. Confirm that UI components gracefully handle scenarios where no filters remain.
80-81
: Simple and effective sort toggling.This toggle logic is clean and easy to follow. Good job avoiding in-place mutations.
177-177
: Clear usage of sort order props.Passing
sortOrder
and a toggle function toActivitySortRoot
is consistent with the store changes. Great decoupling of UI from store logic.
179-179
: Use typed local storage defaults.Using
selectedFilters || defaultActivityFilters
ensures a robust fallback. Continue verifying that the UI gracefully handles corner cases.
195-195
: Confirm consistent filter handling.Make sure the logic in
IssueActivityCommentRoot
aligns with any updates you make in the toggle logic and filter array structure.
199-199
: Synchronized sort order prop usage.Passing
sortOrder
toIssueActivityCommentRoot
ensures the comment component respects current user preferences. This is a great extension of your local storage approach.
Description
Added activity filters to local storage.
Added indicator to filters button to indicate modified filters.
Moved activity sort order from store to local storage.
Type of Change
References
WEB-2370
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates