From 57b6091e01eec030cddcb02db2bf99aa228413d2 Mon Sep 17 00:00:00 2001 From: Bruno Pagno Date: Tue, 28 Apr 2026 10:21:39 +0200 Subject: [PATCH] adjust Roadmap & Version pages --- .../work_packages_graph_widget_component.rb | 2 +- app/controllers/versions_controller.rb | 40 +++++++---- app/helpers/versions_helper.rb | 2 +- app/helpers/work_packages_filter_helper.rb | 6 +- app/models/version.rb | 14 ++-- app/views/versions/_overview.html.erb | 6 +- app/views/versions/show.html.erb | 2 +- spec/controllers/versions_controller_spec.rb | 24 +++---- spec/features/versions/graph_spec.rb | 66 +++++++++---------- .../work_packages_filter_helper_spec.rb | 4 +- spec/models/project_spec.rb | 9 ++- spec/models/version_spec.rb | 66 +++++++++++-------- 12 files changed, 137 insertions(+), 104 deletions(-) diff --git a/app/components/projects/settings/versions/work_packages_graph_widget_component.rb b/app/components/projects/settings/versions/work_packages_graph_widget_component.rb index 029e4467c49..e2fae1e5096 100644 --- a/app/components/projects/settings/versions/work_packages_graph_widget_component.rb +++ b/app/components/projects/settings/versions/work_packages_graph_widget_component.rb @@ -39,7 +39,7 @@ module Projects end def call - return unless version.work_packages.any? + return unless version.issues_count > 0 widget_wrapper do |widget| widget.with_body do diff --git a/app/controllers/versions_controller.rb b/app/controllers/versions_controller.rb index d083b8d14cc..29f99689659 100644 --- a/app/controllers/versions_controller.rb +++ b/app/controllers/versions_controller.rb @@ -39,23 +39,15 @@ class VersionsController < ApplicationController def index @types = @project.types.order(Arel.sql("position")) retrieve_selected_type_ids(@types, @types.select(&:is_in_roadmap?)) - @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_work_packages? : (params[:with_subprojects].to_i == 1) - project_ids = @with_subprojects ? @project.self_and_descendants.includes(:wiki).map(&:id) : [@project.id] - @versions = find_versions(@with_subprojects, params[:completed]) - - @wps_by_version = {} - unless @selected_type_ids.empty? - @versions.each do |version| - @wps_by_version[version] = work_packages_of_version(version, project_ids, @selected_type_ids) - end - end - @versions.reject! { |version| !project_ids.include?(version.project_id) && @wps_by_version[version].blank? } + @versions = find_versions(with_subprojects, params[:completed]) + @wps_by_version = build_wps_by_version(@versions) + @versions.reject! { |version| project_ids.exclude?(version.project_id) && @wps_by_version[version].blank? } end def show @issues = @version - .work_packages + .work_packages_target_versions .visible .includes(:status, :type, :priority) .order("#{::Type.table_name}.position, #{WorkPackage.table_name}.id") @@ -119,6 +111,22 @@ class VersionsController < ApplicationController @project = @version.project end + def with_subprojects + @with_subprojects ||= if params[:with_subprojects].nil? + Setting.display_subprojects_work_packages? + else + (params[:with_subprojects].to_i == 1) + end + end + + def project_ids + @project_ids ||= if with_subprojects + @project.self_and_descendants.includes(:wiki).map(&:id) + else + [@project.id] + end + end + def archived_project_mesage if current_user.admin? ApplicationController.helpers.sanitize( @@ -180,10 +188,16 @@ class VersionsController < ApplicationController def work_packages_of_version(version, project_ids, selected_type_ids) version - .work_packages + .work_packages_target_versions .visible .includes(:project, :status, :type, :priority) .where(type_id: selected_type_ids, project_id: project_ids) .order("#{Project.table_name}.lft, #{::Type.table_name}.position, #{WorkPackage.table_name}.id") end + + def build_wps_by_version(versions) + return {} if @selected_type_ids.empty? + + versions.index_with { |version| work_packages_of_version(version, project_ids, @selected_type_ids) } + end end diff --git a/app/helpers/versions_helper.rb b/app/helpers/versions_helper.rb index 359a3ab75d9..b72cc2c3652 100644 --- a/app/helpers/versions_helper.rb +++ b/app/helpers/versions_helper.rb @@ -88,7 +88,7 @@ module VersionsHelper when "descendants" filters << { subprojectId: { operator: "*", values: [] } } end - filters << { version: { operator: "=", values: [version.id] } } + filters << { targetVersion: { operator: "=", values: [version.id] } } filters # return as an array, not JSON string end diff --git a/app/helpers/work_packages_filter_helper.rb b/app/helpers/work_packages_filter_helper.rb index a6dcaae7af3..60affa13e25 100644 --- a/app/helpers/work_packages_filter_helper.rb +++ b/app/helpers/work_packages_filter_helper.rb @@ -34,7 +34,7 @@ module WorkPackagesFilterHelper query = { f: [ filter_object("status_id", "c"), - filter_object("version_id", "=", version.id) + filter_object("target_version_id", "=", version.id) ] } project_work_packages_with_query_path(version.project, query, options) @@ -44,7 +44,7 @@ module WorkPackagesFilterHelper query = { f: [ filter_object("status_id", "o"), - filter_object("version_id", "=", version.id) + filter_object("target_version_id", "=", version.id) ] } project_work_packages_with_query_path(version.project, query, options) @@ -52,7 +52,7 @@ module WorkPackagesFilterHelper def project_work_packages_version_path(version, options = {}) filters = [ - filter_object("version_id", "=", version.id) + filter_object("target_version_id", "=", version.id) ] unless version.sharing == "none" diff --git a/app/models/version.rb b/app/models/version.rb index 33c5b708cc9..e0b175f7b0f 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -87,7 +87,7 @@ class Version < ApplicationRecord systemwide? || user.allowed_in_project?(:view_work_packages, project) || user.allowed_in_project?(:manage_versions, project) || - work_packages.visible(user).exists? + work_packages_target_versions.visible(user).exists? end def due_date @@ -97,7 +97,7 @@ class Version < ApplicationRecord # Returns the total estimated time for this version # (sum of leaves estimated_hours) def estimated_hours - @estimated_hours ||= work_packages.leaves.sum(:estimated_hours).to_f + @estimated_hours ||= work_packages_target_versions.leaves.sum(:estimated_hours).to_f end # Returns the total reported time for this version @@ -155,17 +155,17 @@ class Version < ApplicationRecord # Returns assigned issues count def issues_count - @issue_count ||= work_packages.count + @issues_count ||= work_packages_target_versions.count end # Returns the total amount of open issues for this version. def open_issues_count - @open_issues_count ||= work_packages.merge(WorkPackage.with_status_open).size + @open_issues_count ||= work_packages_target_versions.merge(WorkPackage.with_status_open).size end # Returns the total amount of closed issues for this version. def closed_issues_count - @closed_issues_count ||= work_packages.merge(WorkPackage.with_status_closed).size + @closed_issues_count ||= work_packages_target_versions.merge(WorkPackage.with_status_closed).size end def wiki_page @@ -214,7 +214,7 @@ class Version < ApplicationRecord # Used to weight unestimated issues in progress calculation def estimated_average if @estimated_average.nil? - average = work_packages.average(:estimated_hours).to_f + average = work_packages_target_versions.average(:estimated_hours).to_f if average.zero? average = 1 end @@ -240,7 +240,7 @@ class Version < ApplicationRecord "COALESCE(#{WorkPackage.table_name}.estimated_hours, ?) * #{ratio}", estimated_average ) - done = work_packages + done = work_packages_target_versions .where(statuses: { is_closed: !open }) .includes(:status) .sum(sum_sql) diff --git a/app/views/versions/_overview.html.erb b/app/views/versions/_overview.html.erb index bfd960ee9b0..a5b205bc6b8 100644 --- a/app/views/versions/_overview.html.erb +++ b/app/views/versions/_overview.html.erb @@ -53,7 +53,7 @@ See COPYRIGHT and LICENSE files for more details. <% end %> -<% if version.work_packages.count > 0 %> +<% if version.issues_count > 0 %> <% closed = version.closed_percent %> <% done = version.completed_percent - closed %> <%= progress_bar([closed, done], width: "40%", legend: ("%0.0f" % version.completed_percent)) %> @@ -63,14 +63,14 @@ See COPYRIGHT and LICENSE files for more details. t(:label_x_closed_work_packages_abbr, count: version.closed_issues_count), project_work_packages_closed_version_path(version) ) %> - (<%= "%0.0f" % (version.closed_issues_count.to_f / version.work_packages.count * 100) %>%) + (<%= "%0.0f" % (version.closed_issues_count.to_f / version.issues_count * 100) %>%)   <%= link_to_if( version.open_issues_count > 0, t(:label_x_open_work_packages_abbr, count: version.open_issues_count), project_work_packages_open_version_path(version) ) %> - (<%= "%0.0f" % (version.open_issues_count.to_f / version.work_packages.count * 100) %>%) + (<%= "%0.0f" % (version.open_issues_count.to_f / version.issues_count * 100) %>%) <% if version.projects.archived.exists? %> <%= op_icon("icon-warning", title: I18n.t("versions.overview.work_packages_in_archived_projects")) %> <% end %> diff --git a/app/views/versions/show.html.erb b/app/views/versions/show.html.erb index cabbeabfed3..244198cdd42 100644 --- a/app/views/versions/show.html.erb +++ b/app/views/versions/show.html.erb @@ -97,7 +97,7 @@ See COPYRIGHT and LICENSE files for more details. <% if @issues.present? %> <% grid.with_widget(Projects::Settings::Versions::RelatedIssuesWidgetComponent, version: @version, issues: @issues) %> <% end %> - <% if @version.work_packages.any? %> + <% if @version.issues_count > 0 %> <% grid.with_widget(Projects::Settings::Versions::WorkPackagesGraphWidgetComponent, version: @version) %> <% end %> <% end %> diff --git a/spec/controllers/versions_controller_spec.rb b/spec/controllers/versions_controller_spec.rb index fd2f545de75..4f443fbe951 100644 --- a/spec/controllers/versions_controller_spec.rb +++ b/spec/controllers/versions_controller_spec.rb @@ -33,19 +33,13 @@ require "spec_helper" RSpec.describe VersionsController do let(:user) { create(:admin) } let(:project) { create(:public_project) } - let(:version1) { create(:version, project:, effective_date: nil) } - let(:version2) { create(:version, project:) } - let(:version3) { create(:version, project:, effective_date: (Date.today - 14.days)) } + let!(:version1) { create(:version, project:, effective_date: nil) } + let!(:version2) { create(:version, project:) } + let!(:version3) { create(:version, project:, effective_date: (Time.zone.today - 14.days)) } describe "#index" do render_views - before do - version1 - version2 - version3 - end - context "without additional params" do before do login_as(user) @@ -74,8 +68,16 @@ RSpec.describe VersionsController do let(:type_a) { create(:type) } let(:type_b) { create(:type) } - let(:wp_a) { create(:work_package, type: type_a, project:, version: version1) } - let(:wp_b) { create(:work_package, type: type_b, project:, version: version1) } + let(:wp_a) do + create(:work_package, type: type_a, project:).tap do |wp| + wp.work_package_associated_versions.create!(version: version1, kind: "target") + end + end + let(:wp_b) do + create(:work_package, type: type_b, project:).tap do |wp| + wp.work_package_associated_versions.create!(version: version1, kind: "target") + end + end before do project.types = [type_a, type_b] diff --git a/spec/features/versions/graph_spec.rb b/spec/features/versions/graph_spec.rb index 757fb42a082..0107e80f5f0 100644 --- a/spec/features/versions/graph_spec.rb +++ b/spec/features/versions/graph_spec.rb @@ -54,11 +54,19 @@ RSpec.describe "version show graph", :js, :selenium do create(:work_package, project: main_project, status: status_control, - version:) + version:).tap do |wp| + wp.work_package_associated_versions.create!(version:, kind: "target") + end end current_user { user } + def create_target_work_package(**attrs) + create(:work_package, **attrs).tap do |wp| + wp.work_package_associated_versions.create!(version: attrs[:version], kind: "target") + end + end + def expect_work_packages_visible_in_graph expect(page).to have_css(".work-packages-embedded-view--container", wait: 20) expect(page).to have_css(".op-wp-embedded-graph", visible: :all, wait: 20) @@ -72,10 +80,9 @@ RSpec.describe "version show graph", :js, :selenium do end it "can show a work package from the same project" do - create(:work_package, - project: main_project, - status: status_sut, - version:) + create_target_work_package(project: main_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph @@ -88,10 +95,9 @@ RSpec.describe "version show graph", :js, :selenium do end it "can show a work package from a different project" do - create(:work_package, - project: other_project, - status: status_sut, - version:) + create_target_work_package(project: other_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph @@ -104,10 +110,9 @@ RSpec.describe "version show graph", :js, :selenium do end it "can show a work package from a descendant project" do - create(:work_package, - project: child_project, - status: status_sut, - version:) + create_target_work_package(project: child_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph @@ -120,20 +125,18 @@ RSpec.describe "version show graph", :js, :selenium do end it "can show a work package from a descendant project" do - create(:work_package, - project: child_project, - status: status_sut, - version:) + create_target_work_package(project: child_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph end it "can show a work package from an ancestor project" do - create(:work_package, - project: parent_project, - status: status_sut, - version:) + create_target_work_package(project: parent_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph @@ -146,30 +149,27 @@ RSpec.describe "version show graph", :js, :selenium do end it "can show a work package from a descendant project" do - create(:work_package, - project: child_project, - status: status_sut, - version:) + create_target_work_package(project: child_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph end it "can show a work package from an ancestor project" do - create(:work_package, - project: parent_project, - status: status_sut, - version:) + create_target_work_package(project: parent_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph end it "can show a work package from a sibling project" do - create(:work_package, - project: sibling_project, - status: status_sut, - version:) + create_target_work_package(project: sibling_project, + status: status_sut, + version:) visit version_path(version) expect_work_packages_visible_in_graph diff --git a/spec/helpers/work_packages_filter_helper_spec.rb b/spec/helpers/work_packages_filter_helper_spec.rb index 2b505b8bc22..a60bdadb461 100644 --- a/spec/helpers/work_packages_filter_helper_spec.rb +++ b/spec/helpers/work_packages_filter_helper_spec.rb @@ -58,7 +58,7 @@ RSpec.describe WorkPackagesFilterHelper do o: "c" }, { - n: "version", + n: "targetVersion", o: "=", v: version.id.to_s } @@ -80,7 +80,7 @@ RSpec.describe WorkPackagesFilterHelper do o: "o" }, { - n: "version", + n: "targetVersion", o: "=", v: version.id.to_s } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fbebf0d552e..66a335d7d63 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -278,17 +278,20 @@ RSpec.describe Project do describe "#close_completed_versions" do let!(:completed_version) do create(:version, project:, effective_date: Date.parse("2000-01-01")).tap do |v| - create(:work_package, version: v, status: create(:closed_status)) + wp = create(:work_package, version: v, status: create(:closed_status)) + WorkPackageAssociatedVersion.create!(work_package: wp, version: v, kind: "target") end end let!(:ineffective_version) do create(:version, project:, effective_date: Date.current + 1.day).tap do |v| - create(:work_package, version: v, status: create(:closed_status)) + wp = create(:work_package, version: v, status: create(:closed_status)) + WorkPackageAssociatedVersion.create!(work_package: wp, version: v, kind: "target") end end let!(:version_with_open_wps) do create(:version, project:, effective_date: Date.parse("2000-01-01")).tap do |v| - create(:work_package, version: v) + wp = create(:work_package, version: v) + WorkPackageAssociatedVersion.create!(work_package: wp, version: v, kind: "target") end end diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 70384e22c6b..d9551c5e6df 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -307,7 +307,7 @@ RSpec.describe Version do end context "with assigned work packages without estimated hours" do - let!(:work_package) { create(:work_package, version:) } + let!(:work_package) { create_target_versioned_wp(version:) } it "returns 0.0" do expect(version.estimated_hours) @@ -316,8 +316,8 @@ RSpec.describe Version do end context "with two assigned work packages with estimated hours" do - let!(:work_package1) { create(:work_package, version:, estimated_hours: 2.5) } - let!(:work_package2) { create(:work_package, version:, estimated_hours: 5) } + let!(:work_package1) { create_target_versioned_wp(version:, estimated_hours: 2.5) } + let!(:work_package2) { create_target_versioned_wp(version:, estimated_hours: 5) } it "returns the sum of estimated hours" do expect(version.estimated_hours) @@ -326,9 +326,9 @@ RSpec.describe Version do end context "with assigned work packages with estimated hours in the leaves" do - let!(:parent) { create(:work_package, version:) } - let!(:work_package1) { create(:work_package, parent:, version:, estimated_hours: 2.5) } - let!(:work_package2) { create(:work_package, parent:, version:, estimated_hours: 5) } + let!(:parent) { create_target_versioned_wp(version:) } + let!(:work_package1) { create_target_versioned_wp(parent:, version:, estimated_hours: 2.5) } + let!(:work_package2) { create_target_versioned_wp(parent:, version:, estimated_hours: 5) } it "returns the sum of estimated hours" do expect(version.estimated_hours) @@ -366,6 +366,14 @@ RSpec.describe Version do let(:version) { create(:version, project:) } let(:closed_status) { create(:status, is_closed: true) } + # See note in #estimated_hours: Version aggregations read from + # work_packages_target_versions, which is service-populated. + def create_target_versioned_wp(*traits, **attrs) + create(:work_package, *traits, **attrs).tap do |wp| + wp.work_package_associated_versions.create!(version: attrs[:version], kind: "target") if attrs[:version] + end + end + context "without a work package" do it "is 0 for completed_percent" do expect(version.completed_percent) @@ -380,8 +388,8 @@ RSpec.describe Version do context "with assigned work packages that are not begun" do before do - create(:work_package, version:) - create(:work_package, version:, done_ratio: 0) + create_target_versioned_wp(version:) + create_target_versioned_wp(version:, done_ratio: 0) end it "is 0 for completed_percent" do @@ -397,10 +405,10 @@ RSpec.describe Version do context "with assigned work packages that are closed" do before do - create(:work_package, status: closed_status, version:) - create(:work_package, status: closed_status, version:, done_ratio: 20) - create(:work_package, status: closed_status, version:, done_ratio: 70, estimated_hours: 25) - create(:work_package, status: closed_status, version:, estimated_hours: 15) + create_target_versioned_wp(status: closed_status, version:) + create_target_versioned_wp(status: closed_status, version:, done_ratio: 20) + create_target_versioned_wp(status: closed_status, version:, done_ratio: 70, estimated_hours: 25) + create_target_versioned_wp(status: closed_status, version:, estimated_hours: 15) end it "is 100 for completed_percent" do @@ -416,9 +424,9 @@ RSpec.describe Version do context "with assigned work packages that have only done ratio" do before do - create(:work_package, version:) - create(:work_package, version:, done_ratio: 20) - create(:work_package, version:, done_ratio: 70) + create_target_versioned_wp(version:) + create_target_versioned_wp(version:, done_ratio: 20) + create_target_versioned_wp(version:, done_ratio: 70) end it "considers the done ratio of open work packages" do @@ -434,9 +442,9 @@ RSpec.describe Version do context "with assigned work packages that have only done ratio with one being closed" do before do - create(:work_package, version:) - create(:work_package, version:, done_ratio: 20) - create(:work_package, status: closed_status, version:) + create_target_versioned_wp(version:) + create_target_versioned_wp(version:, done_ratio: 20) + create_target_versioned_wp(status: closed_status, version:) end it "considers the done ratio of open work packages" do @@ -452,10 +460,10 @@ RSpec.describe Version do context "with assigned work packages that have weighted done ratio" do before do - create(:work_package, version:, estimated_hours: 10) - create(:work_package, version:, done_ratio: 30, estimated_hours: 20) - create(:work_package, version:, done_ratio: 10, estimated_hours: 40) - create(:work_package, status: closed_status, version:, estimated_hours: 25) + create_target_versioned_wp(version:, estimated_hours: 10) + create_target_versioned_wp(version:, done_ratio: 30, estimated_hours: 20) + create_target_versioned_wp(version:, done_ratio: 10, estimated_hours: 40) + create_target_versioned_wp(status: closed_status, version:, estimated_hours: 25) end it "considers the weighted done ratio of open work packages" do @@ -471,10 +479,10 @@ RSpec.describe Version do context "with assigned work packages that have partly weighted done ratio" do before do - create(:work_package, version:, done_ratio: 20) - create(:work_package, version:, done_ratio: 30, estimated_hours: 10) - create(:work_package, version:, done_ratio: 10, estimated_hours: 40) - create(:work_package, status: closed_status, version:) + create_target_versioned_wp(version:, done_ratio: 20) + create_target_versioned_wp(version:, done_ratio: 30, estimated_hours: 10) + create_target_versioned_wp(version:, done_ratio: 10, estimated_hours: 40) + create_target_versioned_wp(status: closed_status, version:) end it "considers the weighted done ratio of open work packages and uses default weighting if unset" do @@ -624,4 +632,10 @@ RSpec.describe Version do expect(described_class.order(name: :desc).pluck(:name)).to eql ordered_names.reverse end end + + def create_target_versioned_wp(*traits, **attrs) + create(:work_package, *traits, **attrs).tap do |wp| + wp.work_package_associated_versions.create!(version: attrs[:version], kind: "target") if attrs[:version] + end + end end