-
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-1883] chore: moving issue activity store to respective folder #5169
Conversation
WalkthroughThe recent changes enhance the modularity and clarity of the issue management system through refined state management and updated type definitions. Key modifications include restructuring the Changes
Sequence Diagram(s)sequenceDiagram
participant A as IssueDetail
participant B as IssueActivityStore
participant C as RootStore
A->>C: Request rootStore
C-->>A: Provide rootStore
A->>B: Instantiate IssueActivityStore(rootStore)
B-->>A: Activity management initialized
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 as PR comments)
Additionally, you can add 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
Outside diff range, codebase verification and nitpick comments (2)
web/ce/store/issue/issue-details/activity.store.ts (1)
Issue Found: Invalid Constructor Argument
The instantiation of
IssueActivityStore
inweb/core/store/issue/issue-details/root.store.ts
usesrootStore.rootStore
, but theIIssueRootStore
interface does not define arootStore
property. This discrepancy indicates a potential issue with the constructor call.
web/core/store/issue/issue-details/root.store.ts
:this.activity = new IssueActivityStore(rootStore.rootStore);Please verify and correct the constructor argument to ensure it matches the expected
RootStore
type.Analysis chain
Line range hint
49-58
:
LGTM! But verify the constructor usage in the codebase.The changes to the constructor improve the modularity and maintainability of the code.
However, ensure that all instantiations of
IssueActivityStore
in the codebase match the new constructor signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueActivityStore` match the new constructor signature. # Test: Search for the constructor usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'new IssueActivityStore'Length of output: 79
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueActivityStore` match the new constructor signature. # Test: Search for the constructor usage. Expect: Only occurrences of the new signature. rg --type typescript -A 5 'new IssueActivityStore'Length of output: 87
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueActivityStore` match the new constructor signature. # Test: Search for the constructor usage. Expect: Only occurrences of the new signature. rg -A 5 'new IssueActivityStore'Length of output: 657
Script:
#!/bin/bash # Description: Verify the type of `rootStore` in `web/core/store/issue/issue-details/root.store.ts`. # Test: Search for the definition of `rootStore` in the specified file. rg 'rootStore' web/core/store/issue/issue-details/root.store.tsLength of output: 268
Script:
#!/bin/bash # Description: Verify the type of `rootStore.rootStore` in `IIssueRootStore`. # Test: Search for the definition of `IIssueRootStore` in the codebase. rg 'interface IIssueRootStore' -A 10Length of output: 858
web/core/store/issue/issue-details/root.store.ts (1)
191-191
: Update required: Ensure all instantiations ofIssueDetail
match the new constructor signature.The following files contain instances of
new IssueDetail(this)
that need to be updated tonew IssueDetail(rootStore.rootStore)
:
space/core/store/root.store.ts
web/core/store/issue/root.store.ts
Analysis chain
LGTM! But verify the class usage in the codebase.
The changes to the class improve the dependency injection strategy and enhance the store's capabilities.
However, ensure that all instantiations of
IssueDetail
in the codebase match the new constructor signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueDetail` match the new constructor signature. # Test: Search for the class usage. Expect: Only occurances of the new constructor signature. rg --type python -A 5 $'new IssueDetail'Length of output: 72
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueDetail` match the new constructor signature. # Test: Search for the class usage. Expect: Only occurrences of the new constructor signature. rg --type typescript -A 5 'new IssueDetail'Length of output: 80
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueDetail` match the new constructor signature. # Test: Search for the class usage. Expect: Only occurrences of the new constructor signature. rg --type ts -A 5 'new IssueDetail'Length of output: 1443
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- web/ce/store/issue/issue-details/activity.store.ts (4 hunks)
- web/core/store/issue/issue-details/root.store.ts (2 hunks)
- web/core/store/issue/root.store.ts (4 hunks)
- web/ee/store/issue/issue-details/activity.store.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- web/ee/store/issue/issue-details/activity.store.ts
Additional comments not posted (4)
web/ce/store/issue/issue-details/activity.store.ts (1)
79-98
: LGTM! But verify the method usage in the codebase.The changes to the method enhance the structure and maintainability of the code.
However, ensure that all calls to
getActivityCommentByIssueId
in the codebase are correctly accessing the updated state structure.Verification successful
Verified: The method usage is correct.
The
getActivityCommentByIssueId
method is correctly accessed and used inactivity-comment-root.tsx
throughuseIssueDetail()
. No issues were found with the updated state structure.
web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `getActivityCommentByIssueId` are correctly accessing the updated state structure. # Test: Search for the method usage. Expect: Only occurances of the new structure. rg --type python -A 5 $'getActivityCommentByIssueId'Length of output: 82
Script:
#!/bin/bash # Description: Verify all calls to `getActivityCommentByIssueId` are correctly accessing the updated state structure. # Test: Search for the method usage. Expect: Only occurrences of the new structure. rg --type typescript -A 5 'getActivityCommentByIssueId'Length of output: 90
Script:
#!/bin/bash # Description: Verify all calls to `getActivityCommentByIssueId` are correctly accessing the updated state structure. # Test: Search for the method usage. Expect: Only occurrences of the new structure. rg 'getActivityCommentByIssueId'Length of output: 552
Script:
#!/bin/bash # Description: Verify the correct usage of `getActivityCommentByIssueId` in `activity-comment-root.tsx`. # Extract relevant lines around the usage in `activity-comment-root.tsx`. rg 'getActivityCommentByIssueId' -A 10 web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsxLength of output: 641
web/core/store/issue/root.store.ts (2)
47-47
: LGTM! But verify the interface usage in the codebase.The changes to the interface improve the modularity and maintainability of the code.
However, ensure that all implementations of
IIssueRootStore
in the codebase match the new type definitions.Verification successful
LGTM! But verify the interface usage in the codebase.
The changes to the interface improve the modularity and maintainability of the code.
However, ensure that all implementations of
IIssueRootStore
in the codebase match the new type definitions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all implementations of `IIssueRootStore` match the new type definitions. # Test: Search for the interface usage. Expect: Only occurances of the new type definitions. rg --type python -A 5 $'IIssueRootStore'Length of output: 70
Script:
#!/bin/bash # Description: Verify all implementations of `IIssueRootStore` match the new type definitions. # Test: Search for the interface usage. Expect: Only occurrences of the new type definitions. rg --type typescript -A 5 'IIssueRootStore'Length of output: 78
Script:
#!/bin/bash # Description: Verify all implementations of `IIssueRootStore` match the new type definitions. # Test: Search for the interface usage. Expect: Only occurrences of the new type definitions. rg --type ts -A 5 'IIssueRootStore'Length of output: 22481
102-102
: LGTM! But verify the class usage in the codebase.The changes to the class improve the modularity and maintainability of the code.
However, ensure that all instantiations of
IssueRootStore
in the codebase match the new constructor signature.Also applies to: 137-137
Verification successful
Verified: The
IssueRootStore
class usage matches the new constructor signature.The instantiations of
IssueRootStore
inweb/core/store/root.store.ts
are consistent with the updated constructor that takes a single parameter of typeRootStore
.
web/core/store/root.store.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueRootStore` match the new constructor signature. # Test: Search for the class usage. Expect: Only occurances of the new constructor signature. rg --type python -A 5 $'new IssueRootStore'Length of output: 75
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueRootStore` match the new constructor signature. # Test: Search for the class usage. Expect: Only occurrences of the new constructor signature. rg --type typescript -A 5 'new IssueRootStore'Length of output: 83
Script:
#!/bin/bash # Description: Verify all instantiations of `IssueRootStore` match the new constructor signature. # Test: Search for the class usage. Expect: Only occurrences of the new constructor signature. rg -A 5 'new IssueRootStore'Length of output: 929
web/core/store/issue/issue-details/root.store.ts (1)
13-19
: LGTM!The changes to the import statements enhance the modularity and clarity of the code.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/ce/constants/workspace.ts (1 hunks)
- web/ce/store/issue/issue-details/activity.store.ts (5 hunks)
Additional comments not posted (9)
web/ce/constants/workspace.ts (4)
7-56
: LGTM! The restructuring enhances readability and maintainability.The
WORKSPACE_SETTINGS
constant has been restructured to organize settings into named categories, which improves the organization and readability of the code.
58-71
: LGTM! The update enhances maintainability.The
WORKSPACE_SETTINGS_LINKS
constant now references the individual settings fromWORKSPACE_SETTINGS
, centralizing the settings in one place.
Line range hint
1-6
:
LGTM! The import statements are necessary and correctly included.The import statements bring in necessary dependencies for the constants.
Line range hint
1-71
:
LGTM! The overall structure is consistent and well-organized.The restructuring enhances readability and maintainability.
web/ce/store/issue/issue-details/activity.store.ts (5)
49-49
: LGTM! The updated constructor improves modularity and maintainability.The constructor now accepts a
RootStore
parameter instead ofIIssueDetail
, consolidating state management.
49-49
: LGTM! The removal ofrootIssueDetailStore
streamlines state management.The functionality previously associated with
rootIssueDetailStore
has been refactored to use the newstore
parameter, improving clarity.
73-104
: LGTM! The update enhances type safety and maintainability.The
activity_type
values now utilize constants fromEActivityFilterType
, reducing the risk of errors from typos in string literals.
Line range hint
107-143
:
LGTM! The update improves consistency and cohesion.The
fetchActivities
method has been updated to use the newstore
parameter, enhancing state management.
Line range hint
1-143
:
LGTM! The overall structure is consistent and well-organized.The modifications enhance modularity, maintainability, and type safety.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/ce/components/issues/worklog/activity/root.tsx (1 hunks)
- web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (1 hunks)
Additional comments not posted (2)
web/ce/components/issues/worklog/activity/root.tsx (1)
11-11
: LGTM! The addition of theends
property enhances flexibility.The new optional property
ends
allows specifying the positioning of the activity log in the user interface.web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (1)
65-65
: LGTM! The conditional assignment of theends
prop is logical.The
ends
prop enhances the component's ability to indicate the position of comments in a list for styling or layout purposes.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- packages/ui/src/popovers/popover-menu.tsx (2 hunks)
- packages/ui/src/popovers/popover.tsx (3 hunks)
- packages/ui/src/popovers/types.ts (2 hunks)
- web/core/store/issue/issue-details/root.store.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- web/core/store/issue/issue-details/root.store.ts
Additional comments not posted (8)
packages/ui/src/popovers/types.ts (2)
1-1
: Import statement update is correct.The import statement now includes
MutableRefObject
, which is necessary for the newpopoverButtonRef
property.
18-18
: Addition ofpopoverButtonRef
property is appropriate.The new property
popoverButtonRef
enhances the flexibility of the popover component by allowing it to manage a reference to the button element that triggers the popover.packages/ui/src/popovers/popover-menu.tsx (2)
21-21
: Addition ofpopoverButtonRef
property is appropriate.The new property
popoverButtonRef
is correctly added to the component's props, enhancing its functionality by enabling better control over the button's behavior.
36-36
: PassingpopoverButtonRef
toPopover
component is correct.The
popoverButtonRef
is correctly passed to thePopover
component, allowing it to manage the reference to the button element.packages/ui/src/popovers/popover.tsx (4)
1-1
: Import statement update is correct.The import statement now includes
Ref
, which is necessary for the newpopoverButtonRef
property.
20-20
: Addition ofpopoverButtonRef
property is appropriate.The new property
popoverButtonRef
is correctly added to the component's props, enhancing its functionality by enabling external components to directly interact with the button element.
23-23
: Change inreferenceElement
state type is appropriate.The type for
referenceElement
state has been changed fromHTMLButtonElement | null
toHTMLDivElement | null
, allowing for a broader range of HTML elements to be used as the reference element.
41-55
: Usage ofpopoverButtonRef
inHeadlessReactPopover.Button
is correct.The
popoverButtonRef
is correctly passed to theHeadlessReactPopover.Button
, enabling external components to manage the button element.
Summary
This PR moves the issue activity store from the ce to ee to facilitate code upgrades in the activity store.
Changes
References
[WEB-1883]
Summary by CodeRabbit
New Features
IssueActivityStore
.IssueDetail
class.ends
property to theTIssueActivityWorklog
type for flexible activity log positioning.ends
in theActivityComment
component to indicate comment positions.popoverButtonRef
to thePopoverMenu
component for improved interaction with triggering buttons.Improvements
Refactor
referenceElement
in thePopover
component to support a broader range of HTML elements.Chores