fix(ui): ensure unauthenticated miners can be selected and unpaired#290
fix(ui): ensure unauthenticated miners can be selected and unpaired#290b-rowan wants to merge 2 commits into
Conversation
Fixes: block#286 Signed-off-by: Brett Rowan <121075405+b-rowan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #286 by changing ProtoFleet’s miner list selection behavior so miners in an AUTHENTICATION_NEEDED state remain selectable, enabling users to unpair them even when authentication fails.
Changes:
- Decouples “row disabled for selection” from “row limited for actions” so auth-needed miners can still be selected.
- Restricts bulk actions to Unpair when the current selection includes auth-needed miners (including “select all” mode).
- Updates/extends UI tests to validate selectable auth-needed miners and the restricted bulk-action menu behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/protoFleet/features/fleetManagement/components/MinerList/MinerListActionBar.tsx | Threads through a new flag indicating selection includes auth-needed miners. |
| client/src/protoFleet/features/fleetManagement/components/MinerList/MinerList.tsx | Makes auth-needed rows selectable and computes a selection-level “includes auth-needed” signal. |
| client/src/protoFleet/features/fleetManagement/components/MinerList/MinerList.test.tsx | Updates test to ensure auth-needed miners remain selectable and included via “Select all”. |
| client/src/protoFleet/features/fleetManagement/components/MinerActionsMenu/MinerActionsMenu.tsx | Limits available bulk actions to Unpair when selection includes auth-needed miners. |
| client/src/protoFleet/features/fleetManagement/components/MinerActionsMenu/MinerActionsMenu.test.tsx | Adds coverage for restricting actions to Unpair when auth-needed miners are selected. |
|
This may have caused issues with unpairing devices, but I can't tell if that's because of these changes or because they're offline, I'll do more digging. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Brett Rowan <121075405+b-rowan@users.noreply.github.com>
I think this is unrelated, will open a separate issue: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82d59b9dd3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| selectionMode === "all" | ||
| ? totalDisabledMiners > 0 |
There was a problem hiding this comment.
Include selected rows in all-mode auth-needed detection
This all-mode branch trusts only totalDisabledMiners > 0, but that count is fed by a separate async query and is initialized/stale as 0 until it refreshes. If a user hits “Select all” before that count is ready (or during a stale window), selectionIncludesUnauthenticatedMiner becomes false even when the current selected rows already include AUTHENTICATION_NEEDED miners, so non-unpair bulk actions are exposed again. Please keep a fallback check against selectedMinerIds/deviceItems in all-mode so the restriction cannot be bypassed by timing.
Useful? React with 👍 / 👎.
This is a bit more verbose than I would like, and there may be better ways to do this, but my experience with typescript is pretty limited. Tested and working, this may affect cases outside the issue specific case, but I think that is generally expected behavior, EG I should always be able to remove a device no matter what state its in, as removing and re-adding may solve some issues.
Fixes: #286