Skip to content

Conversation

@speaker-ender
Copy link
Contributor

Ticket []

Description Of Changes

Fixing bug where incorrect action was taken because of incorrect variable usage.

Code Changes

  • Uses the listSelelectMode to determine if bulk actions should be used instead of the isBulkSelect variable which was intended for the checkbox props

Steps to Confirm

  1. Visit a monitor screen in the new Helios V2 UI
  2. Use the Select All checkbox to enter the exclusive selection mode
  3. Go to a second page and de-select one of the list items
  4. Performa an action from the dropdown
  5. Confirm that a bulk action event was sent with the excluded id correctly set

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 Nov 12, 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 Nov 14, 2025 9:05pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Nov 14, 2025 9:05pm

@lucanovera
Copy link
Contributor

@speaker-ender tested in the preview link, it now stays in select all mode but the deselected items are not taken into account. https://www.loom.com/share/c97ac01e7d8d46628d30ee264a755a3a

@speaker-ender
Copy link
Contributor Author

@speaker-ender tested in the preview link, it now stays in select all mode but the deselected items are not taken into account. https://www.loom.com/share/c97ac01e7d8d46628d30ee264a755a3a

@lucanovera can you confirm if the values are sent in the request?
I am seeing the POST request correctly including the values in the body as specified by the swagger docs.
This might be a BE bug.

@lucanovera
Copy link
Contributor

@speaker-ender tested in the preview link, it now stays in select all mode but the deselected items are not taken into account. https://www.loom.com/share/c97ac01e7d8d46628d30ee264a755a3a

@lucanovera can you confirm if the values are sent in the request? I am seeing the POST request correctly including the values in the body as specified by the swagger docs. This might be a BE bug.

You're right, it looks like the parms are correct so this would be a BE issue:
Captura de pantalla 2025-11-14 a la(s) 4 56 32 p  m

@speaker-ender speaker-ender marked this pull request as ready for review November 14, 2025 20:08
@speaker-ender speaker-ender requested a review from a team as a code owner November 14, 2025 20:08
@speaker-ender speaker-ender requested review from gilluminate and removed request for a team November 14, 2025 20:08
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Greptile Summary

Fixed a bug where bulk actions were incorrectly triggered based on a derived boolean state (isBulkSelect) instead of the actual selection mode (listSelectMode). This caused bulk actions to fail when using exclusive selection mode with exclusions.

Key changes:

  • Replaced all isBulkSelect checks with listSelectMode === "exclusive" to correctly determine when to use bulk actions
  • Refactored checkbox props into a consolidated checkboxProps object in the useBulkListSelect hook for better encapsulation
  • Removed the confusing isBulkSelect derived state that represented "all items selected" rather than the selection mode

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix correctly addresses a logical bug by replacing a derived state check with the actual source of truth. The changes are focused, well-scoped, and improve code clarity by consolidating checkbox props. No breaking changes or side effects identified.
  • 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/useBulkListSelect.ts 5/5 Refactored to consolidate checkbox props and removed the confusing isBulkSelect derived state in favor of direct mode checking
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/page.tsx 5/5 Fixed bug where isBulkSelect was incorrectly used instead of checking listSelectMode === "exclusive" for determining bulk action behavior

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.

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@gilluminate gilluminate left a comment

Choose a reason for hiding this comment

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

I'm seeing the same BE bug Lucano is, but the code looks solid and the Post body does look correct.

@speaker-ender speaker-ender added this pull request to the merge queue Nov 14, 2025
Merged via the queue into main with commit 77ea71e Nov 14, 2025
47 checks passed
@speaker-ender speaker-ender deleted the fix/bulk-select-mode branch November 14, 2025 21:25
jjdaurora pushed a commit that referenced this pull request Dec 5, 2025
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.

4 participants