Skip to content

[AGILE-363] Add positional actions for contiguous selections - #24780

Open
myabc wants to merge 8 commits into
implementation/AGILE-362-batch-destinationsfrom
implementation/AGILE-363-batch-position-actions
Open

[AGILE-363] Add positional actions for contiguous selections#24780
myabc wants to merge 8 commits into
implementation/AGILE-362-batch-destinationsfrom
implementation/AGILE-363-batch-position-actions

Conversation

@myabc

@myabc myabc commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Note

This PR is part of a stack. Please review and merge in this order: #24525#24778#24779 → this PR.

Ticket

https://community.openproject.org/wp/AGILE-363

What are you trying to accomplish?

Make the existing Backlogs Move to position submenu move either one card or one contiguous selected block in the same list.

The change:

  • resolves Top, Up, Down, and Bottom from one pure block-placement contract
  • omits positional actions for sparse, cross-list, truncated, unaddressable, and effective no-op scopes
  • preserves selected document order and excludes selected rows from predecessor resolution
  • reuses the existing optimistic atomic collection move, rollback, announcement, busy, and selection lifecycle

What approach did you choose and why?

Menu availability and activation both call the same resolver. Activation settles the live action scope, resolves again against the current innermost owned list, and passes the ordered rows and IDs to the existing collection performMove path. Roots without the selection/collection capability retain their singular member-move behavior.

This supersedes the menu-collapse behaviour added in #24778. There, a menu move relocated exactly the card it named, so collapseForAction collapsed any wider selection onto that card rather than leaving a batch the move never touched. A positional move here carries the whole block, so collapsing would be wrong: the orchestrator method is removed on this branch, and the selection is committed with selectForAction only once the move is known to be executable.

The resolver validates row ownership so nested sortable lists cannot substitute an outer host row for an inner work package. Fixed rows remain addressable neighbours but cannot enter the selected block, and one-step moves never cross a truncation marker.

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...) — chromium only

@myabc myabc changed the title implementation/AGILE 363 batch position actions [AGILE-363] Add positional actions for contiguous selections Aug 16, 2026
@myabc
myabc requested a lite review from Copilot August 16, 2026 09:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for Move to position actions to operate on a contiguous selected block of backlog cards (while preserving existing single-card behavior), using a shared block-move resolver for both menu gating and execution.

Changes:

  • Introduces resolveBlockMove / resolveBlockMoveAvailability to resolve predecessor placement for contiguous selections (and to reject sparse/cross-list/truncation-boundary/no-op scopes).
  • Updates sortable-lists controller/menu plumbing to use action-scope-based availability and to execute batch menu moves through the existing optimistic collection-move path.
  • Expands backend + frontend test coverage for contiguous selection moves, gating behavior, and rollback/selection lifecycle.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
modules/backlogs/spec/support/pages/backlog.rb Adds page helpers for selecting contiguous cards and asserting Move-to-position availability.
modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb New Selenium spec covering contiguous-block positional moves and rollback behavior.
modules/backlogs/spec/features/work_packages/batch_destination_menu_spec.rb Updates destination menu spec to expect Move-to-position availability for contiguous multi-card scopes.
modules/backlogs/spec/components/backlogs/work_package_card_menu_component_spec.rb Tightens expectations around Move-to-position menu rendering (client-driven availability, no legacy form fields).
frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts Updates documentation/comments to distinguish singular-collapse vs batch menu moves.
frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts Implements block-move resolution + availability helpers for contiguous selections.
frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts Adds unit tests for block-move resolution across many boundary cases.
frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts Switches Move-to-position gating to rely on root-provided availability (no longer hides submenu purely for multi-card scope).
frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts Updates/extends menu tests for showing/hiding batch position directions and the submenu.
frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts Clarifies move availability semantics in the interface comment.
frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts Uses block-move availability/execution for selection-backed menu moves.
frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts Adds coverage for batch menu moves, rollback behavior, and batch availability reporting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread modules/backlogs/spec/support/pages/backlog.rb
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch 2 times, most recently from cbddd59 to d5e8674 Compare August 17, 2026 00:05
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from d5e8674 to 5513fa3 Compare August 17, 2026 12:20
@myabc myabc added feature javascript Pull requests that update Javascript code DO NOT MERGE labels Aug 17, 2026
@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7:1]
  • rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]
  • rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]
🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24780, linked for reference only):

- `rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24780. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24780 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @myabc to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @myabc, and request a review from @myabc.
On every commit, set @myabc as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from 5513fa3 to f29fbb1 Compare August 18, 2026 13:40
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from f29fbb1 to 05ef05a Compare August 18, 2026 17:44
@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]
  • rspec ./spec/features/workflows/edit_spec.rb[1:4:4]
🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24780, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`
- `rspec ./spec/features/workflows/edit_spec.rb[1:4:4]`

Treat this as a standalone task, unrelated to PR #24780. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24780 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @myabc to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @myabc, and request a review from @myabc.
On every commit, set @myabc as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

@myabc
myabc marked this pull request as ready for review August 18, 2026 22:23
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from 05ef05a to c3e8e19 Compare August 19, 2026 12:55
@myabc
myabc requested a balanced review from Copilot August 19, 2026 13:07
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from c3e8e19 to bbe0734 Compare August 19, 2026 13:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts:541

  • selectForAction mutates the current selection before the owner-list, block-resolution, and identity checks below. If a menu opened on an unselected card becomes stale (for example, that direction becomes a no-op before activation), one of those checks returns without moving anything, but the previous batch has already been replaced by the invoker. Resolve and validate the prospective scope with actionScopeFor first, and only commit the selection change once the move is known to be executable.
      const scope = this.selectForAction(itemElement);

@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from 930320d to 52368f7 Compare August 19, 2026 18:13
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch 2 times, most recently from cff495a to b2713ea Compare August 19, 2026 21:16
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from b2713ea to f1aca08 Compare August 19, 2026 21:18
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch 2 times, most recently from 1ac2c21 to aba0d1a Compare August 20, 2026 07:17
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from aba0d1a to 2a5efb0 Compare August 20, 2026 12:26
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from 2a5efb0 to 2df9b71 Compare August 20, 2026 14:40
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from 2df9b71 to 6b4c6d7 Compare September 1, 2026 20:53
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from 6b4c6d7 to f09226a Compare September 4, 2026 10:06
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from f09226a to af5baf2 Compare September 5, 2026 16:24
Establishes one DOM resolution for availability and execution. Validates
contiguous blocks and explicit unavailable reasons while preserving singular
placement and sparse-list boundaries.

https://community.openproject.org/wp/AGILE-363
Verifies that every resolved row owns its supplied item and prevents
nested lists from substituting an outer host row.

https://community.openproject.org/wp/AGILE-363
Resolves availability over the invoker's prospective scope and exposes
only moves the live block resolver can execute. Preserves destinations for
contiguous same-list selections.

https://community.openproject.org/wp/AGILE-363
Routes selection roots through the shared block resolver and moves the
ordered block through the optimistic collection path. Keeps roots without
selection on their singular member endpoint.

https://community.openproject.org/wp/AGILE-363
Exercises every direction and gating boundary through Selenium. Covers
optimistic order, feedback, cleanup, and 422 rollback while protecting
one-card, sparse, cross-list, and confined scopes.

https://community.openproject.org/wp/AGILE-363
Resolves collection capability before availability or mutation. Makes
incomplete selection roots fail closed while keeping roots without selection
on their singular member behavior.

https://community.openproject.org/wp/AGILE-363
Restores per-direction availability for plain sortable roots while
keeping selection roots on block and capability gating. Protects blank
predecessors and removes the obsolete collapse shim.

https://community.openproject.org/wp/AGILE-363
Replaces the persisted-order database checks with a reload and a
rendered-order assertion, so the spec proves persistence by what the
user sees surviving the reload. Payload and ordering semantics remain
covered by the controller unit specs and the request specs.
@myabc
myabc force-pushed the implementation/AGILE-363-batch-position-actions branch from af5baf2 to 9b06e65 Compare September 5, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature javascript Pull requests that update Javascript code needs review

Development

Successfully merging this pull request may close these issues.

2 participants