[WEB-2326] fix: issue activity mutation on attachments upload.#5886
[WEB-2326] fix: issue activity mutation on attachments upload.#5886
Conversation
WalkthroughThe changes introduce a new required property, Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
web/core/components/issues/issue-detail-widgets/attachments/content.tsx (1)
Line range hint
21-26: Consider using prop spreading for cleaner code.While the current implementation is correct, you could consider using the spread operator to pass all props to the
IssueAttachmentItemListcomponent. This would make the code more concise and less prone to errors when adding or removing props in the future.Here's a suggested refactor:
<IssueAttachmentItemList - workspaceSlug={workspaceSlug} - projectId={projectId} - issueId={issueId} - disabled={disabled} + {...props} handleAttachmentOperations={handleAttachmentOperations} />This change would automatically pass all props received by
IssueAttachmentsCollapsibleContenttoIssueAttachmentItemList, including any future additions.web/core/components/issues/issue-detail-widgets/attachments/quick-action-button.tsx (1)
35-37: LGTM: Introduction ofhandleFetchPropertyActivitiesThe new function
handleFetchPropertyActivitiesis well-implemented and correctly memoized. It enhances the component's ability to fetch updated activities after attachment operations.Consider adding a comment explaining the purpose of this function for better code readability:
+// Fetch updated activities after attachment operations const handleFetchPropertyActivities = useCallback(() => { fetchActivities(workspaceSlug, projectId, issueId); }, [fetchActivities, workspaceSlug, projectId, issueId]);web/core/components/issues/attachment/attachment-item-list.tsx (1)
43-45: LGTM: Addition ofhandleFetchPropertyActivitiesfunctionThe new
handleFetchPropertyActivitiesfunction is well-implemented and correctly memoized. It enhances the component's ability to fetch project-specific activities.Consider renaming the function to
handleFetchIssueActivitiesfor improved clarity, as it specifically fetches activities related to an issue within a project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- web/core/components/issues/attachment/attachment-item-list.tsx (3 hunks)
- web/core/components/issues/issue-detail-widgets/attachments/content.tsx (1 hunks)
- web/core/components/issues/issue-detail-widgets/attachments/quick-action-button.tsx (3 hunks)
🧰 Additional context used
🔇 Additional comments (9)
web/core/components/issues/issue-detail-widgets/attachments/content.tsx (2)
22-22: LGTM: Correct implementation ofprojectIdprop.The addition of
projectId={projectId}to theIssueAttachmentItemListcomponent is correct and aligns with the changes described in the AI-generated summary. This change ensures that theprojectIdis properly passed down to the child component, which is necessary for the new functionality related to fetching project-specific activities.
22-22: Verify alignment with PR objective.The addition of
projectIdtoIssueAttachmentItemListseems to be part of the fix for issue activity mutation on attachments upload, as mentioned in the PR title. However, it would be helpful to clarify how this change specifically contributes to resolving the issue.Could you please explain how passing
projectIdtoIssueAttachmentItemListhelps in fixing the issue activity mutation on attachments upload? This will help ensure that the change aligns with the PR objectives.web/core/components/issues/issue-detail-widgets/attachments/quick-action-button.tsx (3)
29-29: LGTM: Addition offetchActivitiesfromuseIssueDetailhookThe inclusion of
fetchActivitiesaligns with the PR objective and enhances the component's ability to update issue activities after attachment operations.
58-58: LGTM: Integration ofhandleFetchPropertyActivitiesThe integration of
handleFetchPropertyActivitiesis well-implemented:
- It's correctly placed in the
finallyblock, ensuring execution regardless of the attachment operation's outcome.- It's properly added to the
onDropcallback's dependency array for correct memoization.These changes effectively address the PR objective of fixing issue activity mutation on attachments upload.
Also applies to: 75-75
Line range hint
1-100: Overall: Excellent implementation addressing the PR objectiveThe changes in this file effectively address the PR objective of fixing issue activity mutation on attachments upload. The implementation is clean, well-structured, and follows React best practices. The new
handleFetchPropertyActivitiesfunction is properly integrated into the existing code, ensuring that activities are fetched after attachment operations.Great job on this implementation!
web/core/components/issues/attachment/attachment-item-list.tsx (4)
20-20: LGTM: Addition ofprojectIdtoTIssueAttachmentItemListtypeThe addition of
projectIdto theTIssueAttachmentItemListtype is appropriate and consistent with the changes described in the summary. This enhancement allows for better project-specific functionality within the component.
27-27: LGTM: Updated props destructuringThe addition of
projectIdto the destructured props is consistent with the type changes and ensures that the new prop is available for use within the component.
65-68: LGTM: UpdatedonDropfunction with activity fetchingThe modifications to the
onDropfunction are well-implemented:
- The addition of
handleFetchPropertyActivitiesin thefinallyblock ensures that activities are fetched after each successful file upload.- The dependency array for
useCallbackhas been correctly updated to includehandleFetchPropertyActivities.These changes effectively integrate activity fetching into the file attachment process.
Also applies to: 82-82
20-20: Summary: Successful integration of project-specific activity fetchingThe changes in this file effectively implement project-specific activity fetching for issue attachments. Key improvements include:
- Addition of
projectIdto the component's props and type definition.- Implementation of
handleFetchPropertyActivitiesfor fetching issue activities.- Integration of activity fetching into the file upload process.
These modifications enhance the component's functionality and align well with the PR objectives. The code quality is maintained, and the changes are consistent across the file.
Also applies to: 27-27, 43-45, 65-68, 82-82
3 participants
Issue link: WEB-2326
Summary by CodeRabbit
New Features
projectIdproperty to enhance the functionality of file attachments by enabling project-specific activity fetching.projectIdis passed through the data flow, improving integration and functionality.Bug Fixes
Documentation
projectIdin relevant components.