[#73446] Sum of work packages and their story points in sprint header are incorrect

[dombesz]

Ticket

https://community.openproject.org/work_packages/73446

What are you trying to accomplish?

• Ensure the sum of work packages and their story points in the sprint header are calculated based work packages from the current project only.

What approach did you choose and why?

• Eager load and filter work packages when loading sprints in the controller
• Introduce the stories_for method in the Agile::Sprint model to ensure there is a single way of retrieving work packages for a project in a sprint.

Merge checklist

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

[Copilot]

Pull request overview

This PR fixes incorrect sprint header totals (work package count + story points) by ensuring calculations only consider work packages from the currently viewed project, even when sprints are included via cross-project work package references.

Changes:

• Add Agile::Sprint.for_project_with_work_packages to preload sprint work packages filtered to the current project.
• Introduce Agile::Sprint#work_packages_for(project) and update sprint components to use it for consistent per-project work package retrieval.
• Add/extend model + feature specs covering the new scope, ordering expectations, and sprint planning flows.

Reviewed changes

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

Show a summary per file

File	Description
modules/backlogs/app/controllers/rb_master_backlogs_controller.rb	Switch sprint loading to the new eager-loading scope when scrum feature flag is active.
modules/backlogs/app/models/agile/sprint.rb	Add work_packages_for(project), include new scope, and tweak work_packages association options.
modules/backlogs/app/models/agile/sprints/scopes/for_project_with_work_packages.rb	New scope that preloads work packages for the given project (ordered by position).
modules/backlogs/lib/open_project/backlogs/patches/work_package_patch.rb	Add WorkPackage.order_by_position helper used by the new scope/method.
modules/backlogs/app/components/backlogs/sprint_header_component.rb	Use work_packages_for(project) for story points + story count in header.
modules/backlogs/app/components/backlogs/sprint_component.rb	Use work_packages_for(project) for sprint story listing.
modules/backlogs/spec/models/agile/sprints/scopes/for_project_with_work_packages_spec.rb	New specs validating eager load behavior, filtering, and bounded queries.
modules/backlogs/spec/models/agile/sprint_spec.rb	Add specs around work package ordering and work_packages_for.
modules/backlogs/spec/support/pages/sprint_planning.rb	Update page object helpers to assert sprint header story points + count.
modules/backlogs/spec/features/sprint_planning/sprint_list_spec.rb	New feature spec verifying header totals only count current-project work packages.
modules/backlogs/spec/features/sprint_planning/create_spec.rb	New feature spec coverage for creating sprints in sprint planning.
modules/backlogs/spec/features/sprint_planning/edit_spec.rb	New feature spec coverage for editing sprints and permission-dependent menu entries.
modules/backlogs/spec/features/sprint_planning/start_finish_spec.rb	New feature spec coverage for starting/finishing sprints and board redirection.

[Copilot]

Pull request overview

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

[myabc]

@dombesz thanks for walking me through the code earlier!

Even though I understand the code a bit better now, this solution still feels over-engineered – a premature optimisation.

For this PR, I would favour going with a simple scope fix, i.e. .where(project:) (see inline comments).

We could go with includes ofc:

  # Controller                                                                                                                                               
  @sprints = Agile::Sprint.for_project(@project).not_completed.order_by_date
                           .includes(:work_packages)  

but, as you mentioned this will over-fetch, so we would still have to filter in Ruby-land:

  def stories                                                                                                                                                
    @stories ||= sprint.work_packages
                        .select { |wp| wp.project_id == project.id }
                        .sort_by { |wp| wp.position || Float::INFINITY }                                                                                     
  end

Preloader is a private API, and it's not something most other devs (including me) will be able to understand without some extra work. I'd vote to drop this solution for now and save it for if/when performance issues come to light.

[dombesz]

Thanks @myabc for the review, I addressed your concerns. The approach I chose, is to load the stories directly in the controller, instead of using the preload internal api.

[myabc]

Overall, this is a good solution! 👍🏻 Thanks for taking the time to look into this - and also removing the private preloader API calls.

Most comments are minor/nitpicks. I'll leave it to your discretion about what to address.

In addition to the inline comments, there are two general comments:

1. Nice to have: PR description The PR description still mentions for_project_with_work_packages, but the implementation no longer uses that approach. It may be worth updating the description for future readers.
2. Nice to have: commit hygiene I'd personally prefer to see this work squashed down to one or two commits (using rebase -i and autosquash, or similar) - unless you feel it's really important to have the intermediate solutions in history for posterity.