BloodHound icon indicating copy to clipboard operation
BloodHound copied to clipboard

fix: remove duplicate datatestid manage-users_table BED-6909

Open catsiller opened this issue 4 weeks ago • 1 comments

Description

  1. <UsersTable /> component and where component is implemented (Inside <Users />) are both wrapped in <Paper data-testid='manage-users_table'>, removed from UsersTable component to remove duplication

Motivation and Context

Resolves https://specterops.atlassian.net/browse/BED-6909

Why is this change required? What problem does it solve?

How Has This Been Tested?

  1. tested locally, looked for data-testid='manage-users_table' in DOM and found 1 (instead of 2/duplicate)

Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc.

Screenshots (optional):

Types of changes

  • Chore (a change that does not modify the application functionality)
  • [x] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Database Migrations

Checklist:

  • [ ] I have met the contributing prerequisites
    • Assigned myself to this PR
    • Added the appropriate labels
    • Associated an issue: https://github.com/SpecterOps/BloodHound/issues/672
    • Read the Contributing guide: https://github.com/SpecterOps/BloodHound/wiki/Contributing
  • [ ] I have ensured that related documentation is up-to-date
    • Open API docs
    • Code comments (GoDocs / JSDocs)
  • [ ] I have followed proper test practices
    • Added/updated tests to cover my changes
    • All new and existing tests passed

catsiller avatar Dec 01 '25 20:12 catsiller

Walkthrough

The Paper wrapper component is removed from the UsersTable DataTable rendering. The Material-UI Paper import is deleted, and the DataTable is rendered directly without the Paper container, while maintaining all existing DataTable props unchanged.

Changes

Cohort / File(s) Change Summary
Paper wrapper removal
packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx
Removed Paper import from Material-UI dependencies; removed <Paper> container element wrapping DataTable; DataTable rendering remains functionally identical with no prop modifications

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A wrapper unwrapped, now clean and free,
The DataTable stands where it's meant to be,
No Paper walls, just pure display—
Simpler styling wins the day! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the primary change: removing a duplicate data-testid from the ManageUsersTable component, with a reference to the associated Jira ticket (BED-6909).
Description check ✅ Passed The PR description covers all required sections with meaningful content: clear description of the change, motivation/context with Jira ticket link, testing approach with verification steps, and proper checklist completion.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch BED-6909--remove-duplicate-datatestid

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16ddadbe4e437faa40da5c3479632024b526cb43 and c71af8de892f64981c9cdf9f9438a8bc78d33101.

📒 Files selected for processing (1)
  • packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx (2 hunks)
🔇 Additional comments (2)
packages/javascript/bh-shared-ui/src/views/Users/UsersTable.tsx (2)

19-19: Tooltip import usage looks correct

Tooltip is still used for the duplicate‑email warning, so keeping this import is appropriate and consistent.


155-161: Paper removal aligns with single data-testid goal; confirm layout for other call sites

Returning DataTable directly here cleanly removes the inner manage-users_table wrapper and resolves the duplicate test id, while keeping all table props unchanged. Please just confirm that any other usages of UsersTable (if they exist) don’t rely on the old Paper styling/background.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Dec 01 '25 20:12 coderabbitai[bot]