fix: wire up column sort in Process Instances table#2838
Conversation
The TableSortLabel arrow was rendered on every header but had no click handler or active state — hovering showed a sort affordance that did nothing. Backend already supports order_by on stock columns, so wire the click through to the existing report_metadata.order_by. - Click a sortable header to sort desc, click again for asc, click again to clear back to default - Active column shows up/down arrow; inactive columns no arrow - Sortable: Id, Process, Start, End, Last milestone, Status - "Started by" is not sortable in the backend (it's a join field, not a stock column), so the misleading arrow is removed from that header
📝 WalkthroughWalkthrough
ChangesColumn sort state and rendering
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec6af099ce
ℹ️ 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".
| setSortOrderBy([`-${accessor}`]); | ||
| } else if (current === 'desc') { | ||
| setSortOrderBy([accessor]); |
There was a problem hiding this comment.
When a user sorts by a non-unique column such as status or process name, the request now sends only that single order_by field, and the backend paginates directly over that SQL ordering. For process lists with many identical values across pages, the database can return ties in arbitrary order between page requests, which can make rows appear on multiple pages or be skipped. Please include a deterministic secondary key such as id when generating the sort order.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx (2)
468-475: 💤 Low valueMove
SORTABLE_ACCESSORSoutside the component to avoid recreation on every render.This
Setis constant and doesn't depend on props or state. Defining it inside the component causes unnecessary allocation on each render.Suggested refactor
+const SORTABLE_ACCESSORS = new Set([ + 'id', + 'process_model_display_name', + 'start_in_seconds', + 'end_in_seconds', + 'last_milestone_bpmn_name', + 'status', +]); + export default function ProcessInstanceListTable({ // ... }: OwnProps) { // ... - const SORTABLE_ACCESSORS = new Set([ - 'id', - 'process_model_display_name', - 'start_in_seconds', - 'end_in_seconds', - 'last_milestone_bpmn_name', - 'status', - ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx` around lines 468 - 475, Move the constant Set named SORTABLE_ACCESSORS out of the ProcessInstanceListTable component so it isn't re-created on every render; locate the declaration of SORTABLE_ACCESSORS (currently defined inside the component) and hoist it to module scope (above the component) so the component can reference the single shared Set instance without allocating a new one each render.
598-627: 💤 Low valuePrefer
accessoroverlabelfor the React key to avoid potential duplicates.Using
col.labelas the key could cause issues if two columns share the same header text. Usingaccessor(with a fallback for the actions column) is more robust.Suggested change
return ( <TableCell - key={col.label} + key={col.accessor ?? col.label} title={🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx` around lines 598 - 627, The TableCell key currently uses col.label which can duplicate; change it to use col.accessor as the primary key with a fallback (e.g., 'actions' or another unique identifier) for columns without an accessor so keys are stable and unique—update the mapping over headerColumns where TableCell is rendered (reference: headerColumns, col.accessor, col.label, TableCell key).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx`:
- Around line 468-475: Move the constant Set named SORTABLE_ACCESSORS out of the
ProcessInstanceListTable component so it isn't re-created on every render;
locate the declaration of SORTABLE_ACCESSORS (currently defined inside the
component) and hoist it to module scope (above the component) so the component
can reference the single shared Set instance without allocating a new one each
render.
- Around line 598-627: The TableCell key currently uses col.label which can
duplicate; change it to use col.accessor as the primary key with a fallback
(e.g., 'actions' or another unique identifier) for columns without an accessor
so keys are stable and unique—update the mapping over headerColumns where
TableCell is rendered (reference: headerColumns, col.accessor, col.label,
TableCell key).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4f5a467a-9717-43f2-801e-08342d298190
📒 Files selected for processing (1)
spiffworkflow-frontend/src/components/ProcessInstanceListTable.tsx
1 participant
Summary
TableSortLabelarrow on every header, but had no click handler or active state — hovering showed a sort affordance that did nothing.report_metadata.order_by, which the backend already supports for stock columns.Behavior
-start_in_seconds, -id)Test plan