Skip to content

Conversation

@speaker-ender
Copy link
Contributor

Description Of Changes

  • Inline actions are now a static list of Classify and Confirm that are disabled if not available
  • Details drawer actions are now a static list of Approve and Confirm that are disabled if not available
  • Un-mute label was renamed to Restore

Code Changes

Steps to Confirm

  1. Go to the new action center list view
  2. Confirm that all list items have a Classify and Confirm action that are appropriately enabled and disabled based on status
  3. Confirm that when you open a details drawer for an item, it has Approve and Confirm buttons that are appropriately enabled and disabled based on status.
  4. Select any list item, hover over the actions dropdown, and confirm that Restore is visible and Un-mute is not

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link

vercel bot commented Oct 24, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ready Ready Preview Comment Oct 24, 2025 0:25am
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Oct 24, 2025 0:25am

@speaker-ender speaker-ender marked this pull request as ready for review October 24, 2025 00:18
@speaker-ender speaker-ender requested a review from a team as a code owner October 24, 2025 00:18
@speaker-ender speaker-ender requested review from eastandwestwind, gilluminate and jack-gale-ethyca and removed request for a team and eastandwestwind October 24, 2025 00:18
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR refactors the action center field actions to use static lists instead of dynamically filtering available actions. Actions are now always displayed but conditionally disabled based on the resource's diff status.

Key changes:

  • Inline actions now display Classify and Confirm buttons for all list items, disabled when not applicable
  • Details drawer actions show Approve and Confirm buttons, disabled when not applicable
  • Renamed "Un-mute" label to "Restore" for better UX
  • Added disabled property to DetailsAction type and implemented in footer component

Issue found:

  • Syntax error in page.tsx:334 where .map() wraps elements in unnecessary brackets, creating nested arrays instead of a flat array

Confidence Score: 2/5

  • This PR has a critical syntax error that will cause runtime issues with list item rendering
  • The nested array syntax error in page.tsx line 334 will cause the list item actions to render incorrectly, breaking the UI. This must be fixed before merge.
  • page.tsx requires immediate attention to fix the nested array syntax error on line 334

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/page.tsx 2/5 Refactored to use static action lists with disabled states; found syntax error creating nested arrays in list item actions

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

]
: [],
)
? LIST_ITEM_ACTIONS.map((action) => [
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: Wrapping the Tooltip in brackets creates nested arrays since .map() already returns an array. Change [<Tooltip>...] to (<Tooltip>...) on this line and line 354.

@speaker-ender speaker-ender force-pushed the refactor/static-actions-and-renaming branch from 9300382 to 4a30dc5 Compare October 24, 2025 00:39
@speaker-ender
Copy link
Contributor Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This refactor improves the action center UI by standardizing how actions are displayed and enabled/disabled based on resource status. Instead of dynamically filtering actions, the code now shows a static list of actions that are conditionally disabled.

Key changes:

  • Static action arrays (LIST_ITEM_ACTIONS, DRAWER_ACTIONS, DROPDOWN_ACTIONS) define which actions appear in each context
  • Actions are now conditionally disabled based on AVAILABLE_ACTIONS[status] lookup rather than filtered from the DOM
  • "Un-mute" label renamed to "Restore" for better UX clarity

Minor issue identified:

  • Unnecessary spread operators used in two places when checking if actions are included in AVAILABLE_ACTIONS arrays

Confidence Score: 4/5

  • Safe to merge with minor style improvements recommended
  • The refactoring follows good patterns by moving from dynamic action filtering to static actions with disabled states. The logic is sound and aligns with the PR's stated goals. Two minor style issues with unnecessary spread operators don't affect functionality.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/FieldActions.const.tsx 5/5 Added static action arrays (DRAWER_ACTIONS, DROPDOWN_ACTIONS, LIST_ITEM_ACTIONS) and renamed "Un-mute" to "Restore"
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/page.tsx 4/5 Refactored to use static action lists with dynamic disabled states; unnecessary spread operators in includes() checks

4 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@speaker-ender speaker-ender added this pull request to the merge queue Oct 24, 2025
Merged via the queue into main with commit b93a5e7 Oct 24, 2025
46 checks passed
@speaker-ender speaker-ender deleted the refactor/static-actions-and-renaming branch October 24, 2025 16:15
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.

3 participants