Skip to content

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

Open
dombesz wants to merge 7 commits intorelease/17.3from
bug/73446-sum-of-work-packages-and-their-story-points-in-sprint-header-does-not-respect-project-boundaries
Open

[#73446] Sum of work packages and their story points in sprint header are incorrect#22633
dombesz wants to merge 7 commits intorelease/17.3from
bug/73446-sum-of-work-packages-and-their-story-points-in-sprint-header-does-not-respect-project-boundaries

Conversation

@dombesz
Copy link
Copy Markdown
Contributor

@dombesz dombesz commented Apr 1, 2026

Ticket

https://un5kw2gk1a5ewenr6bvyaezm1xctjhkthr.julianrbryant.com/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 via a sprint scope for_project_with_work_packages.
  • Introduce the work_packages_for method in the Agile::Sprint model to ensure there is a single way of retrieving work packages for a project in a sprint.
  • Ensure the for_project_with_work_packages benefits from the eager loaded work package association.

Merge checklist

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 10 out of 13 changed files in this pull request and generated 2 comments.

@myabc myabc self-requested a review April 2, 2026 11:17
Copy link
Copy Markdown
Contributor

@myabc myabc left a comment

Choose a reason for hiding this comment

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

@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
Copy link
Copy Markdown
Contributor Author

dombesz commented Apr 2, 2026

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.

@dombesz dombesz requested a review from myabc April 2, 2026 14:43
@dombesz dombesz force-pushed the bug/73446-sum-of-work-packages-and-their-story-points-in-sprint-header-does-not-respect-project-boundaries branch from 44de7f4 to ecb2507 Compare April 2, 2026 15:29
@dombesz dombesz force-pushed the bug/73446-sum-of-work-packages-and-their-story-points-in-sprint-header-does-not-respect-project-boundaries branch from ecb2507 to 3aeb706 Compare April 3, 2026 08:29
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.

4 participants