Skip to content

[AGILE-392] Cover unrestricted drag destinations - #24858

Merged
myabc merged 1 commit into
devfrom
implementation/AGILE-392-batch-drag-confinement
Sep 15, 2026
Merged

myabc merged 1 commit into
devfrom
implementation/AGILE-392-batch-drag-confinement

Conversation

@myabc

@myabc myabc commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Ticket

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

What are you trying to accomplish?

Preserve coverage of the batch drag destination policy after the core confinement fix landed through AGILE-278 (#24778). This PR now targets dev independently of the destination and action-menu stack.

What approach did you choose and why?

The remaining regression test ensures an item's own confined mobility does not override an unrestricted permitted-destinations answer from the root. The equivalent per-item policy test is already present on dev and is retained there without duplication.

The original runtime confinement changes are already on dev through AGILE-278. This remaining diff introduces no runtime behavior changes.

Documentation added after the approved scope has been extracted unchanged to AGILE-436 / #25359, a separate draft PR based on AGILE-364. That documentation will receive its own review.

Validation

  • 522 sortable-list Chromium tests passed on this independent branch.
  • Full non-feature Backlogs suite: 2,164 examples, zero failures (Ruby 4.0.6, seed 60108).
  • TypeScript spec compilation and ESLint passed.
  • Feature/browser QA and repeated ~1000-card performance testing were not run.

@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch from e994e8c to f8c47f1 Compare August 20, 2026 14:40
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from bf063b8 to 9a400ca Compare August 20, 2026 14:44
@myabc
myabc requested a lite review from Copilot August 20, 2026 15:04
@myabc
myabc marked this pull request as ready for review August 20, 2026 15:04
@myabc myabc added this to the 17.9.x milestone Aug 20, 2026
@myabc myabc added javascript Pull requests that update Javascript code needs review feature pullpreview labels Aug 20, 2026
@github-actions

github-actions Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Deploying openproject with ⚡ PullPreview

Field Value
Latest commit 6b8ede0
Job deploy
Status 🗑️ Preview destroyed
Preview URL Destroyed

View logs

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

This PR aligns Backlogs batch-drag behavior with the same per-item destination policy used by batch destination menus, ensuring drags don’t offer drops the server will reject (and don’t refuse drops that should be allowed) when the selection contains confined work packages across lists.

Changes:

  • Centralizes per-item destination acceptance into itemAcceptsDestination, and rebuilds destination intersections on that policy.
  • Reworks drag payload gating from “confined + source list” to “permitted list elements” resolved across the whole batch at drag start.
  • Updates Backlogs feature specs and DnD helpers to more reliably observe refusal feedback (animation-frame timing / event stream).

Reviewed changes

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

Show a summary per file
File Description
spec/support/shared/drag_and_drop_helper_spec.rb Adds dwell: option to stabilize native drag event streams for assertions.
modules/backlogs/spec/support/pages/backlog.rb Updates Backlogs drag helpers and refusal assertions to track container state across events; threads dwell: through Selenium drag helper.
modules/backlogs/spec/features/work_packages/batch_move_spec.rb Adds feature specs covering batch moves with confined members across lists.
frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts Updates mocks to the new root API (dragPermittedLists).
frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.ts Switches list indicator gating to permittedListsAllowDrop.
frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts Updates test payloads to use permittedListElements.
frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts Introduces itemAcceptsDestination and uses it to compute permittedDestinations.
frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts Adds unit coverage for itemAcceptsDestination.
frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts Emits permittedListElements on the drag payload instead of confinement/source-list fields.
frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts Updates DnD payload and acceptance tests to the new permitted-lists model.
frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts Replaces confinement checks with permittedListsAllowDrop and updates payload shape.
frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.spec.ts Updates helper specs for the new payload shape.
frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts Adds dragPermittedLists to compute permitted lists across batch members using the shared policy.
frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts Extends root controller specs to validate permitted-lists behavior across multi-list batches.

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

Comment thread frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.ts Outdated
Comment thread frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts Outdated
@myabc myabc removed the pullpreview label Aug 20, 2026
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from 9a400ca to 078374e Compare August 20, 2026 17:07
@myabc
myabc requested a balanced review from Copilot August 20, 2026 17:09

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 14 out of 14 changed files in this pull request and generated no new comments.

Suppressed comments (1)

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

  • This confinement-specific early return bypasses itemAcceptsDestination, so the drag is not actually driven solely by the shared per-item policy. In particular, the PR description says AGILE-357 can add a workflow restriction as another policy arm without changing either consumer, but a batch with no confined members will still return null here and accept every drag destination. Please evaluate the registered destinations through itemAcceptsDestination for every batch, and derive null only when that evaluation leaves all destinations unrestricted.
    const members = [itemElement, ...this.prospectiveDragMateElements(itemElement)];
    if (members.every((member) => !isConfinedItem(member))) {
      return null;

@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from 078374e to fe829fb Compare August 20, 2026 17:24
@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]
🤖 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 #24858, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #24858. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24858 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.

@EinLama EinLama 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.

Works like a charm! Tested multiple scenarios and could never break it. Code looks great. I had one particular function that made me pause for a bit. Please have a look 🔍

Approving since even if this was a valid point, it's not a blocker.

Comment thread frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts Outdated
@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • 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 #24858, linked for reference only):

- `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 #24858. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24858 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-364-batch-action-menus branch from 26d44ad to 8d11d33 Compare September 1, 2026 20:53
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from 09d4c77 to c1d4132 Compare September 1, 2026 20:53
@myabc myabc removed the needs review label Sep 3, 2026
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from c1d4132 to 1d788ce Compare September 4, 2026 10:06
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch 2 times, most recently from 0503d7b to 94a4008 Compare September 5, 2026 16:24
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from 1d788ce to 1510c7c Compare September 5, 2026 16:24
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch from 94a4008 to d65431a Compare September 5, 2026 16:25
@myabc myabc added this to the 18.0.x milestone Sep 9, 2026
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch from 3ea8b0a to d76d601 Compare September 9, 2026 13:45
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch 2 times, most recently from c51a846 to e531ddc Compare September 9, 2026 14:05
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch 2 times, most recently from b4f06f9 to 3f387d1 Compare September 9, 2026 15:12
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from e531ddc to 717e0c1 Compare September 9, 2026 15:12
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch from 3f387d1 to b249809 Compare September 9, 2026 16:48
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch 2 times, most recently from 8dac70e to f8ee525 Compare September 10, 2026 17:23
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch from b249809 to 5282243 Compare September 10, 2026 17:23
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from f8ee525 to 1376650 Compare September 11, 2026 14:58
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch 2 times, most recently from 7fa8153 to cbc6435 Compare September 15, 2026 16:57
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from 1376650 to 0b88d01 Compare September 15, 2026 16:57
@myabc
myabc force-pushed the implementation/AGILE-364-batch-action-menus branch from cbc6435 to f1c768e Compare September 15, 2026 17:32
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from 0b88d01 to c05cfa5 Compare September 15, 2026 17:32
@myabc
myabc removed this pull request from stack #24837 September 15, 2026 17:41
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from c05cfa5 to c0760a7 Compare September 15, 2026 17:42
@myabc
myabc changed the base branch from implementation/AGILE-364-batch-action-menus to dev September 15, 2026 17:42
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from c0760a7 to e2f3488 Compare September 15, 2026 18:43
A confined item's own mobility must not narrow a root's unrestricted
permitted-destinations answer, as the batch it carries may reach
every list.

https://community.openproject.org/wp/AGILE-392
@myabc
myabc force-pushed the implementation/AGILE-392-batch-drag-confinement branch from e2f3488 to 6b8ede0 Compare September 15, 2026 18:48
@myabc myabc removed the pullpreview label Sep 15, 2026
@myabc myabc changed the title [AGILE-392] Constrain batch drags containing confined work packages [AGILE-392] Cover unrestricted drag destinations Sep 15, 2026
@myabc
myabc merged commit 0465dd0 into dev Sep 15, 2026
17 checks passed
@myabc
myabc deleted the implementation/AGILE-392-batch-drag-confinement branch September 15, 2026 19:18
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature javascript Pull requests that update Javascript code

Development

Successfully merging this pull request may close these issues.

3 participants