Repository navigation
feat(desktop): group, drag, and bulk-edit automations - #9210
Conversation
|
Caution Review failedThe pull request is closed.
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. |
[Medium impact] The changes affect how owners organize and manage automations in the desktop list. The PR appears safe to merge with non-blocking fixes for group-name input, stale detail status, and returning selections. FindingsFix with agent promptSummaryAdds grouped automation lists, searchable move menus, group breadcrumbs, row selection, bulk actions, and drag-to-group support.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart LR
List[Automation list] --> Sections[Sorted group sections]
Sections --> Selection[Select visible rows]
Sections --> Drag[Drag rows to a section]
Selection --> Bulk[Move, pause, resume, or delete]
Drag --> Move[setGroup]
Bulk --> API[Per-row API requests]
Move --> API
API --> Refresh[Refresh query data]
Reviews (1) · Last reviewed commit: "feat(desktop): group, drag, and bulk-edi..." · Reviewed by Greptile |
| const onInputKeyDown = (event: KeyboardEvent<HTMLInputElement>) => { | ||
| if (event.key === "Enter") { | ||
| event.preventDefault(); | ||
| items()[0]?.click(); | ||
| } else if (event.key === "ArrowDown" || event.key === "ArrowUp") { | ||
| event.preventDefault(); | ||
| const list = items(); | ||
| (event.key === "ArrowDown" ? list[0] : list.at(-1))?.focus(); |
There was a problem hiding this comment.
Confirming text moves an automation
The group filter treats Enter used to confirm Chinese, Japanese, or Korean text as a move command. onInputKeyDown clicks the first menu item, which can move the automation and close the menu before the user finishes typing. Ignore Enter and arrow keys while event.nativeEvent.isComposing is true.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/components/AutomationGroupPicker/AutomationGroupPicker.tsx
Line: 63-70
Comment:
**Confirming text moves an automation**
The group filter treats Enter used to confirm Chinese, Japanese, or Korean text as a move command. `onInputKeyDown` clicks the first menu item, which can move the automation and close the menu before the user finishes typing. Ignore Enter and arrow keys while `event.nativeEvent.isComposing` is true.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| toast.error( | ||
| errorMessage(error, t({ message: "Failed to update automation" })), | ||
| ), | ||
| onSettled: () => void utils.automation.list.invalidate(), |
There was a problem hiding this comment.
Bulk Pause and Resume invalidate only automation.list, unlike the single-row action, which also invalidates automation.get. Open an automation, return to the list, bulk-pause it, then reopen it within the detail query's 30-second staleTime: the detail page still shows it as enabled. Invalidate automation.get for every affected ID when the batch settles, including partial failures.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/page.tsx
Line: 281
Comment:
**Details show the old status**
Bulk Pause and Resume invalidate only `automation.list`, unlike the single-row action, which also invalidates `automation.get`. Open an automation, return to the list, bulk-pause it, then reopen it within the detail query's 30-second `staleTime`: the detail page still shows it as enabled. Invalidate `automation.get` for every affected ID when the batch settles, including partial failures.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| const selected = useMemo(() => { | ||
| const shown = new Set(order); | ||
| return new Set([...picked].filter((id) => shown.has(id))); | ||
| }, [picked, order]); |
There was a problem hiding this comment.
selected hides IDs that leave order, but those IDs remain in picked. Select a row, filter it out, select another row, then clear the search: the first row becomes selected again and joins later bulk actions. This contradicts the hook's promise that filtered or collapsed rows drop out of the selection. Prune picked when rows leave order, and cover filtering and collapsing in hook tests.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/hooks/useRowSelection/useRowSelection.ts
Line: 16-19
Comment:
**Old selections return**
`selected` hides IDs that leave `order`, but those IDs remain in `picked`. Select a row, filter it out, select another row, then clear the search: the first row becomes selected again and joins later bulk actions. This contradicts the hook's promise that filtered or collapsed rows drop out of the selection. Prune `picked` when rows leave `order`, and cover filtering and collapsing in hook tests.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
45b5129 to
c2d3a26
Compare
|
Verdict: minor issues. The UI works end to end in the dev app (grouping, Move to group with create, drag between sections, ⌘/⇧ selection, and bulk pause/resume/delete). However, CI
Full review with screenshots: https://app.superset.sh/page/pr-9210-review-group-drag-bulk-edit-automations-5c8n8p |
19f7d21 to
2197d1c
Compare
- collapsible group sections (A-Z, No group last) with a hover + to create in a group - "Move to group" submenu with a filter field and inline "Create group" - multiselect started by cmd-click (shift range, X, cmd-A, Esc) with a bulk bar: move to group, pause, resume, delete - drag rows or a selection between group sections - group crumb on the detail page breadcrumb
…t returns - the group filter ignores Enter and arrows while an IME composes text - bulk pause and resume invalidate each automation's detail query too - rows that leave the screen (filtered, collapsed) leave the selection, so they don't come back selected; regression test in useRowSelection.test.ts
- catalog-integrity: stripIcu counts braces, so a branch that ends in a
nested placeholder ("Moved # automation to {group}") no longer cuts the
plural block short and flags correct translations as missing {group}
- deleting one row no longer clears an unrelated selection; deleted rows
leave the selection on their own
- X toggles the row only without cmd/ctrl/alt (cmd-X is cut)
- cancel an in-flight list refetch before the optimistic group move
- UNGROUPED_KEY moved out from under the "Create with AI" comment
Creating a group needed you to know that typing an unknown name offers "Create group". The menu now always ends with "New group…": it keeps the menu open, turns the field into "New group name…", and Enter creates the group (or joins one with that exact name).
"Delete group…" asks first, then moves the group's automations to No group; the automations themselves are kept. Mine tab only (owners regroup), and not on "No group". Component test covers the menu and its absence on No group.
21132c7 to
ef86f91
Compare
Stack (merge in order)
Film and walkthrough (78s, real input in the dev app): https://app.superset.sh/page/automation-groups-mlki3d
Summary
This PR bundles four related changes:
Added during review:
Keyboard and mouse
How it works
utils.automation.list.setData, then invalidate. Bulk actions callsetGroup,setEnabled, anddeleteper row throughPromise.allSettled, so no batch endpoint is needed.@dnd-kit/core, like the Linear board. The mouse sensor starts after 8px so clicks still open rows. A one-shot capture listener swallows the click that follows a drop. Auto-scroll starts only in the outer 10% of the list.Design decisions
automations.tagsfile run workspaces into sidebar folders and default to["automation"]. Grouping by them would put every row in one section and tie list organization to the sidebar.Manual QA (dev app over CDP, real mouse and keyboard input)
TableCellnudges checkboxes 2px; overridden); names do not shift when checkboxes appearTesting
bun run typecheck,./scripts/lint.sh,bun run check:i18n(strict compile; new strings translated in all 17 locales with plural forms)bun test apps/desktop/src/renderer/routes/_authenticated/_dashboard: 1,797 tests, 0 failures, including newgroupAutomationsandselectRangetestsKnown limitations
Deployment
Merge after #9209 is live in production; this PR calls
automation.setGroupand readsgroup.Summary by cubic
Groups automations into collapsible A–Z sections on the Mine tab, with "No group" last, and adds "Move to group" (filter with inline create), drag-and-drop between sections, and multiselect (⌘‑click, ⇧‑range, X, ⌘A, Esc) with a bulk bar for moving, pausing, resuming, and deleting. The detail breadcrumb shows the group, and a hover + on a section header creates an automation in that group. The "Move to group" menu always ends with "New group…": picking it turns the field into a name field, and Enter creates the group (or joins an exact match). Right-clicking a section header offers "Delete group…", which confirms first and then moves that group's automations to "No group".
Groups are now also settable through the API (
automation.setGroup, create/update), the CLI (--group,--clear-group, list--group), and the MCP create/update tools. Names are trimmed and capped at 64 characters and matched case-insensitively to existing groups, so CLI/MCP spelling joins the current spelling.Fixes in this update
automation.updateno longer drops extra schedules when only non-schedule fields (name, group) change.Moves are optimistic and applied per row through existing endpoints, so no batch API is needed. Docs and all 17 locales are included.
Written for commit ef86f91. Summary will update on new commits.