feat(activity): per-miner bulk action detail modal [frontend]#60
feat(activity): per-miner bulk action detail modal [frontend]#60rongxin-liu merged 22 commits intomainfrom
Conversation
🔐 Codex Security Review
Review SummaryOverall Risk: MEDIUM Findings[MEDIUM] Activity detail modal can accumulate and render large batch payloads without any bound
[LOW] Client-side grouping now under-reports the activity count shown to operators
Notes
Generated by Codex Security Review | |
…ils component Data-fetching hook with in-memory cache and inflight dedup for the GetCommandBatchDeviceResults RPC, plus a detail component that renders loading/error/pruned/data states with a per-device result table. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Strip the .completed suffix as a fallback lookup so completed batch events reuse the same icon as their initiation counterpart. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the shared List component with a custom grid layout that groups initiated+completed batch events into a single row, adds chevron toggle for batch rows, and lazy-fetches per-device results via ActivityBatchDetails on expand. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
10b8d3b to
86c5f32
Compare
…ce info Switch from chevron-based inline expand to a click-to-open modal for activity detail. Every activity row is now clickable, showing a summary (event, timestamp, scope, user, result) and — for batch actions — the per-miner device results table with name, IP, and MAC from PR #71. - Add ActivityDetailModal with summary section + StatusCircle indicator - Remove ActivityBatchDetails (superseded by modal) - Collapse initiated + completed batch rows; fix count mismatch - Strip "completed" from descriptions in the list view - Extract isCompletedEvent/baseEventType to shared eventType utility - Add fetchedRef to useCommandBatchDeviceResults to skip redundant RPCs - Stabilize onDismiss callback and drop derived isPending prop Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6d954003c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds per-activity-row drill-down via a modal in ProtoFleet’s Activity page, including per-miner outcomes for bulk command batches using the backend’s new batch_id and GetCommandBatchDeviceResults RPC.
Changes:
- Replaces the Activity log’s inline expand with a click-to-open
ActivityDetailModal, including a per-device results table for batch activities. - Collapses initiated + completed batch activity rows into a single visible entry and updates icon/event-type handling for
.completedevents. - Introduces a frontend hook to fetch/cache batch device results and updates activity timestamps to include seconds.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/shared/utils/formatTimestamp.ts | Updates activity timestamp formatting to include seconds. |
| client/src/protoFleet/features/activity/utils/eventType.ts | Adds helpers for detecting/normalizing .completed event types. |
| client/src/protoFleet/features/activity/utils/activityIcons.tsx | Falls back to base event type when resolving icons for completed events. |
| client/src/protoFleet/features/activity/components/ActivityTable.tsx | Implements row grouping for batch initiated/completed events and switches to clickable-row layout + modal opening. |
| client/src/protoFleet/features/activity/components/ActivityDetailModal.tsx | Adds the activity detail modal UI, including batch per-device results rendering. |
| client/src/protoFleet/api/useCommandBatchDeviceResults.ts | Adds a hook to fetch and cache per-batch per-device results with inflight dedup. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44c1e0b252
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Only mark batch results as "fetched" when status is terminal (finished or detailsPruned), so in-progress batches refetch on reopen - Show grouped.length for activity count instead of mixing server-side total with local hidden count, which was inaccurate during pagination - Add role="button", tabIndex, and onKeyDown to activity rows for keyboard and screen reader accessibility - Harden baseEventType to strip only a trailing .completed suffix using slice instead of replace Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f96a31a269
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Add pollIntervalMs and activeBatchId options to the batch results hook, following useFleetCounts/useComponentErrors convention - Auto-stop polling when batch reaches terminal state (finished or detailsPruned) by deriving isTerminal from cache - Suppress loading spinner on background poll refetches - Show "In progress" with pending StatusCircle for non-terminal batches instead of the false "Success" from the initiated entry - Use POLL_INTERVAL_MS from shared polling constants Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86d3dd3d52
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The custom grid layout dropped the data-testid attributes that the shared List component previously provided. Add list-row, type, scope, and user test IDs to restore activity E2E test selectors. Made-with: Cursor
… entry When a user opens an in-progress batch, the entry snapshot has result="success". After polling detects the batch finished with failures, isFailed stayed false because it read the stale snapshot. Now derives failure from batchData.failureCount when terminal batch data is available. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aadcb7cd9e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…per-miner-detail-frontend Made-with: Cursor # Conflicts: # client/src/protoFleet/features/activity/components/ActivityTable.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da026f0602
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- Fall back to entry.result for failure detection when detailsPruned is true, since retention removes the failureCount aggregates - Show deviceIdentifier as fallback miner label instead of generic "Unknown device" so historical rows remain distinguishable - Convert 5 && expressions to ternaries for react/jsx-no-leaked-render Made-with: Cursor
- Include the initial loading state (batchData is null) in the batchInProgress check so the modal shows "In progress" instead of a brief false "Success" flash while the first fetch is in flight - Preserve existing cached batch data on polling errors instead of wiping it with null, so transient failures don't regress the modal Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 880f9e3100
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Only show the error-only view when there is no cached data. When previous poll data exists, continue rendering the device results so transient failures don't hide already-loaded outcomes. Made-with: Cursor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2b5b5adfe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…is missing Only treat missing batch data as in-progress for initiated (non- completed) entries. Completed activity entries already carry a terminal result, so showing "In progress" during an RPC failure misreports the audit outcome. Made-with: Cursor
090e7b4 to
5566afc
Compare
Problem
Bulk actions appear in the activity log as a single row with no record of which miners succeeded or failed. Operators have no way to drill into per-miner outcomes after a partial bulk failure. The backend (PR #25) added the
batch_idfield onActivityEntryand theGetCommandBatchDeviceResultsRPC; PR #71 added device name, IP, and MAC to each result row. This PR wires both into the frontend.What ships
Frontend only. Requires backend PRs #25 and #71. Closes #22.
StatusCircleindicator) and, for batch actions, the succeeded/failed counts and a scrollable per-device results table.reboot) and a completed row (reboot.completed). The table collapses them into one visible entry (the completed row). The displayed count usesgrouped.lengthso it always matches the number of visible rows. In-progress batches (no completion yet) still show the initiated row. The word "completed" is stripped from descriptions since the outcome summary already conveys completion.details_prunedshows "Per-miner details are no longer available." In-progress batches show "Results will appear as devices complete." Truncated responses show a count notice.finishedordetailsPruned. In-progress batches will refetch on modal reopen so the UI stays current. Inflight dedup prevents concurrent requests for the same batch.role="button",tabIndex={0}, andonKeyDown(Enter/Space) for screen reader and keyboard users, matching the sharedListcomponent pattern.formatActivityTimestampnow usestoLocaleStringand outputsMM/DD/YY, h:mm:ss AM/PMfor audit precision.Where to review
Data fetching
client/src/protoFleet/api/useCommandBatchDeviceResults.ts— custom hook withuseState-based cache (keyed bybatchId),useRef<Set>inflight dedup,fetchedRefthat only records terminal responses (status === "finished"ordetailsPruned), andhandleAuthErrors/getErrorMessageerror handling. Returns{ fetch, getResult }.Detail modal
client/src/protoFleet/features/activity/components/ActivityDetailModal.tsx— two sections:StatusCircle, optional Error) plus batch counts (Succeeded / Failed) when available.BatchDeviceResults— renders loading, error, pruned, pending, and data states. Device table columns: Miner (name + IP/MAC secondary text), Status (colored text), Message, Time. DerivesisPendinginternally fromdata.status.Table + grouping
client/src/protoFleet/features/activity/components/ActivityTable.tsx— CSS grid layout (grid-cols-[1fr_12rem_10rem_13rem]). Key pieces:groupActivities()— collects completed batch IDs in one pass, then filters out initiated rows whose batch already has a completed counterpart.grouped.lengthso it always matches visible rows regardless of pagination state.handleDismissviauseCallback; every row is clickable with keyboard support.Shared utilities
client/src/protoFleet/features/activity/utils/eventType.ts—isCompletedEvent()andbaseEventType()helpers.baseEventTypeusesisCompletedEventguard +sliceto strip only a trailing.completedsuffix.client/src/protoFleet/features/activity/utils/activityIcons.tsx—getActivityIconusesbaseEventTypefallback so.completedevents resolve to the correct icon.Timestamp formatting
client/src/shared/utils/formatTimestamp.ts—formatActivityTimestampsimplified to a singletoLocaleString("en-US", ...)call with seconds included.Test plan
details_prunedstate shows the "no longer available" message