[COMMS-916] Read cost report version filter and group-by from target_versions by akabiru · Pull Request #24432 · opf/openproject

akabiru · 2026-07-24T09:16:10Z

Ticket: https://community.openproject.org/wp/COMMS-916

Moves the cost report's Version filter and Version group-by off the deprecated work_packages.version_id column onto the work_package_versions target association. A work package with several target versions is now matched by the filter through any of them and listed under each of them when grouping.

Important

Following the team decision, the overlapping totals a multi-value work package produces are accepted for consistency with how multi-value custom fields already behave here; deduplication is a separate piece of work.

See OPEN POINT FND-178 Cost reports over-count totals when grouping or filtering by a multi-value attribute

Screenshots

DP-18 with versions "1.0.0, 2.0.0", cost 8h

Feature flag ON 🌞

Group by Version

Filter by Version

Feature flag OFF 🌚

github-actions · 2026-07-24T09:38:15Z

Warning

Flaky specs

rspec ./modules/calendar/spec/features/calendar_user_interaction_spec.rb[1:1:4: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 #24432, linked for reference only):

- `rspec ./modules/calendar/spec/features/calendar_user_interaction_spec.rb[1:1:4:1]`

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

Pull request overview

This PR updates the reporting cost query’s Version filter and group-by to use the work_package_versions “target” association instead of the deprecated work_packages.version_id column, allowing work packages with multiple target versions to match via any of them and to appear under each when grouped.

Changes:

Introduces a shared SQL join fragment to reach work_package_versions (kind: target) from reporting entries.
Switches Version group-by and filter to use that join and work_package_versions.version_id.
Adds specs covering grouping a work package under multiple target versions and filtering via a non-primary target version.

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
modules/reporting/app/models/cost_query/work_package_target_version_join.rb	Adds a shared LEFT OUTER JOIN fragment to connect `entries` to target version associations.
modules/reporting/app/models/cost_query/group_by/version_id.rb	Updates Version group-by to use the target-version join and group on `work_package_versions.version_id`.
modules/reporting/app/models/cost_query/filter/version_id.rb	Updates Version filter to use the target-version join and filter on `work_package_versions.version_id`.
modules/reporting/spec/models/cost_query/group_by_spec.rb	Adds coverage for grouping a work package under each of its target versions.
modules/reporting/spec/models/cost_query/filter_spec.rb	Adds coverage for matching a work package through a non-primary target version.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

github-actions · 2026-07-24T14:33:00Z

Warning

Flaky specs

rspec ./spec/features/projects/create_spec.rb[1:12:3:3: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 #24432, linked for reference only):

- `rspec ./spec/features/projects/create_spec.rb[1:12:3:3:1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

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

github-actions · 2026-07-27T14:58:09Z

Warning

Flaky specs

rspec ./modules/backlogs/spec/features/inbox_column_spec.rb[1:7: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 #24432, linked for reference only):

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

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

Pull request overview

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

Comments suppressed due to low confidence (2)

modules/reporting/spec/models/cost_query/group_by_spec.rb:129

This spec hard-codes the English label ("Target versions"), which is brittle across translation updates/locales. Consider asserting against the same I18n-backed source as the implementation (WorkPackage.human_attribute_name).

      it "labels the group 'Target versions'" do
        expect(CostQuery::GroupBy::VersionId.label).to eq("Target versions")
      end

modules/reporting/spec/models/cost_query/filter_spec.rb:352

This spec hard-codes the English label ("Target versions"), which is brittle across translation changes/locales. Consider asserting against WorkPackage.human_attribute_name(:target_versions) to match the implementation.

        it "labels the filter 'Target versions'" do
          expect(CostQuery::Filter::VersionId.label).to eq("Target versions")
        end

Reach a work package's versions through the work_package_versions join instead of the deprecated version_id column, so a work package with several target versions is matched by the filter and listed under each when grouping. Overlapping totals from multi-value work packages are accepted for now; deduplication is separate work.

entries.entity_id is polymorphic, so a Meeting time entry whose id collides with a work package's was inheriting that work package's target versions in cost report filtering and grouping. Guarding on entity_type in the ON clause keeps the group-by's LEFT semantics for non-work-package entries instead of dropping them.

The target-version join is one-to-many, so filtering or grouping by version counts a work package once per matching target version. This over-count is accepted for now; the specs assert the inflated totals and reference OPEN POINT FND-178 so the behavior is clearly tracked rather than mistaken for an untested bug.

With the multiple-versions feature on, version filtering and grouping read every target version (one-to-many, the accepted FND-178 over-count). With it off a work package is single-version, so the join is narrowed to its primary target version (target_versions.first) and the label reads 'Version' instead of 'Target versions'. Both paths read the new target_versions model rather than the deprecated WorkPackage#version_id. The join is resolved per report-build instead of via the join_table DSL so the flag takes effect without a restart.

With every target version joined, the default row-by-row negation let a work package slip through on a non-excluded row (targets [1,2] still matched 'is not 1' via the 2 row). Exclude on the work package as a whole with a NOT EXISTS subquery, guarded by entity_type. Only needed while multiple versions is on; the primary-only join is already one-to-one.

target_versions has no declared ordering, so spell out that the primary target version is the lowest version id (what target_versions.first returns), matching the join's MIN(version_id).

The multi-version NOT EXISTS override is skipped while the feature is off, so pin that the default 'is not' operator negates correctly on the primary-only join: 'is not primary' drops the work package, 'is not secondary' keeps it.

With multiple target versions on, the versions filter is a multi-value attribute, so render it as a chip autocompleter (like the project filter) that runs an IN check instead of the single-select-plus-toggle widget. With the flag off a work package is single-version, so keep a strictly singular select with no add-value toggle.

An empty submitted value would map to a spurious version id 0; compact_blank removes it so only real selections reach the autocompleter.

github-actions · 2026-07-28T10:09:50Z

Warning

Flaky specs

rspec ./spec/features/projects/create_spec.rb[1:12:3: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 #24432, linked for reference only):

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

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

akabiru self-assigned this Jul 24, 2026

akabiru requested a review from Copilot July 24, 2026 12:20

Copilot started reviewing on behalf of akabiru July 24, 2026 12:20 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

akabiru requested a review from Copilot July 24, 2026 13:00

Copilot started reviewing on behalf of akabiru July 24, 2026 13:01 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread modules/reporting/app/models/cost_query/filter/version_id.rb

Comment thread modules/reporting/app/models/cost_query/filter/version_id.rb

akabiru requested a review from Copilot July 24, 2026 13:32

Copilot started reviewing on behalf of akabiru July 24, 2026 13:33 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread modules/reporting/app/models/cost_query/work_package_target_version_join.rb

Comment thread modules/reporting/spec/models/cost_query/group_by_spec.rb Outdated

Comment thread modules/reporting/spec/models/cost_query/filter_spec.rb

akabiru added this to the 17.8.x milestone Jul 27, 2026

akabiru requested a review from Copilot July 27, 2026 15:01

Copilot started reviewing on behalf of akabiru July 27, 2026 15:02 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

akabiru added 9 commits July 28, 2026 12:49

Clarify primary-version comments as lowest version id

93757d5

target_versions has no declared ordering, so spell out that the primary target version is the lowest version id (what target_versions.first returns), matching the join's MIN(version_id).

Drop blank version filter values before id coercion

4e5c883

An empty submitted value would map to a spurious version id 0; compact_blank removes it so only real selections reach the autocompleter.

akabiru force-pushed the implementation/comms-916-adjust-cost-reports-to-read-from-target_versions branch from 53d4151 to 4e5c883 Compare July 28, 2026 09:49

Uh oh!

Conversation

akabiru commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 24, 2026

Uh oh!

github-actions Bot commented Jul 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

akabiru commented Jul 24, 2026 •

edited

Loading