-
Notifications
You must be signed in to change notification settings - Fork 84
feat: support removals [ENG-1595] #6941
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
828fcf0 to
0c9595d
Compare
0c9595d to
0c40e68
Compare
b1a7663 to
247079e
Compare
Greptile OverviewGreptile SummaryThis PR adds support for removing fields in the action center with proper confirmation workflows. The implementation makes field actions dynamic based on resource status using readonly mapping objects, following best practices. Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files ChangedFile Analysis
|
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.
5 files reviewed, 2 comments
| const [promoteResourcesMutation] = usePromoteResourcesMutation(); | ||
| const [unMuteMonitorResultAssetsMutation] = useUnmuteResourcesMutation(); | ||
| const [updateResourcesCategoryMutation] = useUpdateResourceCategoryMutation(); | ||
| const [prmoteRemovalMutation] = usePromoteRemovalStagedResourcesMutation(); |
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.
syntax: typo: prmoteRemovalMutation missing 'o' - should be promoteRemovalMutation
| const [prmoteRemovalMutation] = usePromoteRemovalStagedResourcesMutation(); | |
| const [promoteRemovalMutation] = usePromoteRemovalStagedResourcesMutation(); |
| }; | ||
|
|
||
| const handlePromoteRemoval = async (urns: string[]) => { | ||
| return prmoteRemovalMutation({ |
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.
syntax: update variable reference after fixing typo on line 60
| return prmoteRemovalMutation({ | |
| return promoteRemovalMutation({ |
247079e to
51ccb98
Compare
gilluminate
left a comment
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.
Overall looks good. One grammar fix is needed for when there's only 1 item selected. I did notice during testing that restoring an item will put it in the "confirmed" status. Is that expected?
| "promote-removals": (targetItemCount: number) => | ||
| `Are you sure you want to remove these ${targetItemCount.toLocaleString()} resources?`, |
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.
This will end up being weird if only 1 item is selected
| "promote-removals": (targetItemCount: number) => | |
| `Are you sure you want to remove these ${targetItemCount.toLocaleString()} resources?`, | |
| "promote-removals": (targetItemCount: number) => | |
| `Are you sure you want to remove ${targetItemCount.toLocaleString()} ${pluralize(targetItemCount, "resource", "resources")}?`, |
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.
We don't use that message for single items but I can add that since it isn't obvious/if someone decides to change that behavior.
Yes, that is currently expected but Adam is working to change that.
51ccb98 to
1713c8d
Compare
1713c8d to
fe9f1a8
Compare
chore: add remove action to dropdown fix: linting and formatting fix: adding disabled message wip: conditional action types feat: conditional actions chore: adding mute action to removals chore: update changelog fix: typo
43ee6c1 to
a494e87
Compare
Ticket ENG-1595
Description Of Changes
removefields using the dropdown.Code Changes
Steps to Confirm
Helios V2beta flag is enabled in the settingsRestorebuttonRestorebuttonRemovedstatusMuteandRemovebutton on the list item as well as in the details trayRemovebutton handles the confirm-removal actionPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works