Skip to content

Bug/agile 306 backlogs non admin user can t sort or group by backlog bucket#24090

Draft
EinLama wants to merge 6 commits into
release/17.6from
bug/agile-306-backlogs-non-admin-user-can-t-sort-or-group-by-backlog-bucket
Draft

Bug/agile 306 backlogs non admin user can t sort or group by backlog bucket#24090
EinLama wants to merge 6 commits into
release/17.6from
bug/agile-306-backlogs-non-admin-user-can-t-sort-or-group-by-backlog-bucket

Conversation

@EinLama

@EinLama EinLama commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Ticket

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

Grouping by buckets would fail for non-admin users if (and only if) semantic work package identifiers were enabled.

This happens because: Query::Results always eagerly loads the project association via WorkPackage.includes(:project). Rails internally runs AliasTracker to decide whether to alias the resulting join. It does this by text-scanning every raw SQL string already in the query for JOIN ... "table" ON patterns. If the table was seen at least once, Rails aliases its generated join — so projects becomes projects_work_packages. So far, no problem.

The backlog bucket grouping adds a raw SQL join that embeds a permission subquery (projects_with_view_sprints.to_sql). For non-admin users, that subquery internally contains INNER JOIN "projects" ON (member permission check). This references the projects table -> table was seen once. Admin users take a different code path that only joins enabled_modules, so their subquery never mentions projects and aliasing never triggers -> projects table was not seen.

For non-admin users, Rails aliases the outmost projects join to projects_work_packages. As a result, the ORDER BY of the query breaks. The reason is that with semantic identifiers enabled, it will sort via a hardcoded project table name that is only defined deep down in the permission check subquery and thus unknown.

        if Setting::WorkPackageIdentifier.semantic?
          ["#{Project.table_name}.identifier", "#{WorkPackage.table_name}.sequence_number"]
        else
          "#{WorkPackage.table_name}.id"
        end

Queries::Results#include_aliases tries to predict which alias will be used by Rails for each table name, in order to build the correct ORDER BY expressions. But, it misses table references inside raw SQL join strings, so our aliases and the aliases defined by Rails can diverge in some cases. Here, this diversion caused the bug.

What approach did you choose and why?

Query::Results#include_aliases now pre-scans all raw SQL join strings (sort criteria joins, group-by join, filter joins) using the same regex pattern as AliasTracker, before computing aliases. This ensures its alias prediction matches what Rails will generate.

The regex was copied from Rails (to Query::Results::RAW_JOIN_TABLE_SCAN_REGEX). To avoid that a change to Rails silently breaks our application, there is a spec that directly compares our regex against the live AliasTracker.initial_count_for implementation, so any future change to the Rails implementation will cause it to go red.

Query::Results#aliased_sorting_by_column_name now also applies the computed aliases to non-association sort expressions — replacing hardcoded table names like projects.identifier with their actual aliases like projects_work_packages.identifier.

As a drive-by cleanup, the project phase definitions join previously worked around the same aliasing issue by adding an explicit LEFT OUTER JOIN "projects" to keep the table accessible regardless of the alias. This workaround could now be removed 👏

Merge checklist

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

@EinLama EinLama force-pushed the bug/agile-306-backlogs-non-admin-user-can-t-sort-or-group-by-backlog-bucket branch 2 times, most recently from b06df44 to f2f2dae Compare July 3, 2026 11:36
@EinLama EinLama force-pushed the bug/agile-306-backlogs-non-admin-user-can-t-sort-or-group-by-backlog-bucket branch from f2f2dae to 9830dc2 Compare July 3, 2026 13:50
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/projects/create_spec.rb[1:7]
🤖 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 #24090, linked for reference only):

- `rspec ./spec/features/projects/create_spec.rb[1:7]`

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

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 pull request fixes a regression where non-admin users could not group/sort work packages by backlog bucket when semantic work package identifiers are enabled, caused by a mismatch between Rails’ internal join aliasing and OpenProject’s alias prediction in Query::Results.

Changes:

  • Pre-scan raw SQL join strings using a Rails-parity regex to align Query::Results#include_aliases with Rails’ AliasTracker aliasing decisions.
  • Apply computed association aliases to non-association sort expressions (e.g., replacing hardcoded projects.identifier with the actual alias).
  • Remove an obsolete “explicit projects join” workaround from the project phase definitions query and add regression coverage.

Reviewed changes

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

Show a summary per file
File Description
spec/models/query/results_project_phase_filter_integration_spec.rb Regression coverage ensuring the project phase filter join no longer needs the explicit projects join under semantic/classic identifiers.
spec/models/query/results_alias_tracker_parity_spec.rb Spec that guards RAW_JOIN_TABLE_SCAN_REGEX parity against Rails’ AliasTracker.initial_count_for.
modules/backlogs/spec/models/query_results_group_by_bucket_spec.rb Regression spec for AGILE-306 (non-admin grouping by backlog bucket under semantic identifiers).
app/models/query/results.rb Core fix: raw-join pre-scan for alias prediction + alias substitution for non-association sortable expressions.
app/models/queries/project_phase_definitions/database_queries.rb Removes the prior explicit LEFT OUTER JOIN "projects" workaround now made unnecessary by corrected alias handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants