diff --git a/modules/reporting/app/models/cost_query/filter/version_id.rb b/modules/reporting/app/models/cost_query/filter/version_id.rb index 6feae8507f43..483eda62440e 100644 --- a/modules/reporting/app/models/cost_query/filter/version_id.rb +++ b/modules/reporting/app/models/cost_query/filter/version_id.rb @@ -28,15 +28,51 @@ class CostQuery::Filter::VersionId < Report::Filter::Base use :null_operators - join_table WorkPackage => [Entry, :entity] + db_field "work_package_versions.version_id" applies_for :label_work_package_attributes + # Resolved per report-build rather than via the join_table DSL (which freezes + # the join at class load) so the multiple-versions feature flag can switch + # between all target versions and the primary one. + def self.table_joins + [[CostQuery::WorkPackageTargetVersionJoin.sql]] + end + def self.label - WorkPackage.human_attribute_name(:version) + WorkPackage.human_attribute_name(Setting::WorkPackageMultipleVersions.active? ? :target_versions : :version) end def self.available_values(*) - versions = Version.where(project_id: Project.visible.map(&:id)) + versions = Version.where(project_id: Project.visible.select(:id)).includes(:project) versions.map { |a| ["#{a.project.name} - #{a.name}", a.id] }.sort_by { |a| a.first.to_s + a.second.to_s } end + + def apply_operator_to(query) + return super unless excluding_target_versions? + + version_ids = sql_query_values(operator.arity).compact + return super if version_ids.empty? + + query.where(sql_excluding_target_versions(version_ids)) + end + + private + + # A work package can have more than one target version. Filtering out one version + # has to hide the work package entirely, even when it also carries others. + def excluding_target_versions? + Setting::WorkPackageMultipleVersions.active? && operator.to_s == "!" + end + + def sql_excluding_target_versions(version_ids) + <<~SQL.squish + NOT EXISTS ( + SELECT 1 FROM work_package_versions excluded + WHERE excluded.work_package_id = entries.entity_id + AND entries.entity_type = 'WorkPackage' + AND excluded.kind = 'target' + AND excluded.version_id IN #{collection(*version_ids)} + ) + SQL + end end diff --git a/modules/reporting/app/models/cost_query/group_by/version_id.rb b/modules/reporting/app/models/cost_query/group_by/version_id.rb index 5144bfd0399c..5c484d0bcb82 100644 --- a/modules/reporting/app/models/cost_query/group_by/version_id.rb +++ b/modules/reporting/app/models/cost_query/group_by/version_id.rb @@ -27,10 +27,20 @@ #++ class CostQuery::GroupBy::VersionId < Report::GroupBy::Base - join_table WorkPackage => [Entry, :entity] + # Keep the derived "version_id" field bare so it renders through the existing + # :version_id branch; the table name qualifies it for the SELECT and GROUP BY. + table_name "work_package_versions" applies_for :label_work_package_attributes + # Resolved per report-build rather than via the join_table DSL (which freezes + # the join at class load) so the multiple-versions feature flag can switch + # between all target versions and the primary one. Must return the same string + # as the filter so the engine collapses them into a single join. + def self.table_joins + [[CostQuery::WorkPackageTargetVersionJoin.sql]] + end + def self.label - WorkPackage.human_attribute_name(:version) + WorkPackage.human_attribute_name(Setting::WorkPackageMultipleVersions.active? ? :target_versions : :version) end end diff --git a/modules/reporting/app/models/cost_query/work_package_target_version_join.rb b/modules/reporting/app/models/cost_query/work_package_target_version_join.rb new file mode 100644 index 000000000000..988654cb2ce3 --- /dev/null +++ b/modules/reporting/app/models/cost_query/work_package_target_version_join.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +module CostQuery::WorkPackageTargetVersionJoin + # Reaches a work package's target versions from the reporting entries. + # + # entries.entity_id is polymorphic (a time entry can point at a Meeting whose + # id collides with a work package's), so the entity_type guard lives in the ON + # clause: it keeps LEFT semantics for the group-by (non-work-package entries + # stay, grouped under "no version") instead of dropping them via WHERE. + # + # With multiple target versions on, the join is one-to-many: a work package + # with several target versions is reported under each of them. With the + # feature off a work package is single-version, so the join is narrowed to the + # primary target version (the lowest version id, matching target_versions.first) + # and stays one-to-one. + JOIN_ALL = <<~SQL.squish + LEFT OUTER JOIN work_package_versions + ON work_package_versions.work_package_id = entries.entity_id + AND entries.entity_type = 'WorkPackage' + AND work_package_versions.kind = 'target' + SQL + + JOIN_PRIMARY = <<~SQL.squish + #{JOIN_ALL} + AND work_package_versions.version_id = ( + SELECT MIN(primary_target.version_id) + FROM work_package_versions primary_target + WHERE primary_target.work_package_id = entries.entity_id + AND primary_target.kind = 'target' + ) + SQL + + # Read at report-build time so a feature-flag change takes effect without a + # restart. The filter and group-by must resolve to the exact same string so + # the engine collapses them into a single join. + def self.sql + Setting::WorkPackageMultipleVersions.active? ? JOIN_ALL : JOIN_PRIMARY + end +end diff --git a/modules/reporting/lib/widget/filters.rb b/modules/reporting/lib/widget/filters.rb index 974b23c0d8ca..1dd1d5093036 100644 --- a/modules/reporting/lib/widget/filters.rb +++ b/modules/reporting/lib/widget/filters.rb @@ -105,6 +105,8 @@ def render_filter(f_cls, f_inst) render_widget User, f, to: html elsif f_cls == CostQuery::Filter::WorkPackageId render_widget WorkPackage, f, to: html + elsif f_cls == CostQuery::Filter::VersionId && Setting::WorkPackageMultipleVersions.active? + render_widget Version, f, to: html # Handling of generic widgets elsif f_cls.heavy? render_widget Heavy, f, to: html diff --git a/modules/reporting/lib/widget/filters/version.rb b/modules/reporting/lib/widget/filters/version.rb new file mode 100644 index 000000000000..e47a35a0a657 --- /dev/null +++ b/modules/reporting/lib/widget/filters/version.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +class Widget::Filters::Version < Widget::Filters::Base + include AngularHelper + + def render + write(content_tag(:div, id: "#{filter_class.underscore_name}_arg_1", class: "advanced-filters--filter-value") do + label = html_label + + box = angular_component_tag "opce-autocompleter", + inputs: { + filters: [], + InputName: "values[#{filter_class.underscore_name}]", + hiddenFieldAction: "change->reporting--page#selectValueChanged", + multiple: true, + items: available_versions, + bindLabel: "name", + bindValue: "id", + model: selected_version_ids, + labelForId: "#{filter_class.underscore_name}_arg_1_val" + }, + id: "#{filter_class.underscore_name}_select_1", + class: "filter-value advanced-filters--ng-select" + + content_tag(:span, class: "inline-label") do + label + box + end + end) + end + + private + + def html_label + label_tag "#{filter_class.underscore_name}_arg_1_val", + "#{h(filter_class.label)} #{I18n.t(:label_filter_value)}", + class: "sr-only" + end + + # The whole set of selectable versions, project-qualified so identically named + # versions in different projects stay distinguishable. The list is small, so we + # hand it to the autocompleter up front and let it filter client-side. + def available_versions + filter_class.available_values.map { |label, id| { id:, name: label } } + end + + def selected_version_ids + expand_comma_separated_values! + + filter.values.compact_blank.map(&:to_i) + end +end diff --git a/modules/reporting/spec/lib/widget/filters/version_spec.rb b/modules/reporting/spec/lib/widget/filters/version_spec.rb new file mode 100644 index 000000000000..d06e0f55901d --- /dev/null +++ b/modules/reporting/spec/lib/widget/filters/version_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require_relative "../../../spec_helper" + +RSpec.describe Widget::Filters::Version do + let(:admin) { create(:admin) } + let(:project) { create(:project, name: "Demo project") } + let(:version) { create(:version, project:, name: "2.0.0") } + + let(:filter) do + CostQuery::Filter::VersionId.new.tap { |f| f.values = [version.id.to_s] } + end + let(:widget) { described_class.new(filter) } + + before { login_as(admin) } + + describe "#available_versions" do + subject(:items) { widget.send(:available_versions) } + + it "offers every version with a project-qualified label the autocompleter can filter" do + expect(items).to include(id: version.id, name: "Demo project - 2.0.0") + end + + it "builds the items without a query per version" do + other_project = create(:project) + create(:version, project:) + create(:version, project: other_project) + widget + + expect { widget.send(:available_versions) }.to have_a_query_limit(2) + end + end + + describe "#selected_version_ids" do + subject(:ids) { widget.send(:selected_version_ids) } + + it "maps the filter values to integer ids the autocompleter matches against its items" do + expect(ids).to eq([version.id]) + end + + context "when a blank value slips in" do + let(:filter) do + CostQuery::Filter::VersionId.new.tap { |f| f.values = [version.id.to_s, ""] } + end + + it "drops it instead of coercing it to a spurious id 0" do + expect(ids).to eq([version.id]) + end + end + end +end diff --git a/modules/reporting/spec/models/cost_query/filter_spec.rb b/modules/reporting/spec/models/cost_query/filter_spec.rb index db67a7e07f93..2b6c329a3dd5 100644 --- a/modules/reporting/spec/models/cost_query/filter_spec.rb +++ b/modules/reporting/spec/models/cost_query/filter_spec.rb @@ -291,6 +291,107 @@ def create_matching_object_with_time_entries(factory, work_package_field, entry_ expect(query.result.count).to eq(3) end + it "labels the filter 'Version' while multiple versions is off" do + expect(CostQuery::Filter::VersionId.label).to eq("Version") + end + + # While the feature is off a work package is single-version, so the filter + # only sees its primary target version (the lowest version id, i.e. what + # target_versions.first returns). + it "matches a work package through its primary target version" do + primary_version = create(:version, project:) + secondary_version = create(:version, project:) + work_package = create_work_package_with_time_entry(version: primary_version) + work_package.work_package_versions.create!(version: secondary_version, kind: "target") + + query.filter :version_id, operator: "=", value: primary_version.id + expect(query.result.count).to eq(1) + end + + it "ignores a non-primary target version while multiple versions is off" do + primary_version = create(:version, project:) + secondary_version = create(:version, project:) + work_package = create_work_package_with_time_entry(version: primary_version) + work_package.work_package_versions.create!(version: secondary_version, kind: "target") + + query.filter :version_id, operator: "=", value: secondary_version.id + expect(query.result.count).to eq(0) + end + + # Off-mode negation runs on the primary-only (one-to-one) join, so the + # default "is not" operator is already correct without the multi-version + # NOT EXISTS override. + it "negates on the primary target version while multiple versions is off" do + primary_version = create(:version, project:) + secondary_version = create(:version, project:) + work_package = create_work_package_with_time_entry(version: primary_version) + work_package.work_package_versions.create!(version: secondary_version, kind: "target") + + query.filter :version_id, operator: "!", value: [primary_version.id] + # Its primary version is primary_version, so "is not primary" drops it. + expect(query.result.count).to eq(0) + end + + it "keeps a work package when negating a non-primary target version while off" do + primary_version = create(:version, project:) + secondary_version = create(:version, project:) + work_package = create_work_package_with_time_entry(version: primary_version) + work_package.work_package_versions.create!(version: secondary_version, kind: "target") + + query.filter :version_id, operator: "!", value: [secondary_version.id] + # Off-mode only sees the primary; "is not secondary" keeps it because its + # primary version is not the secondary one. + expect(query.result.count).to eq(1) + end + + context "with multiple target versions enabled", + with_flag: { work_package_multiple_versions: true }, + with_settings: { work_package_multiple_versions: true } do + it "labels the filter 'Target versions'" do + expect(CostQuery::Filter::VersionId.label).to eq("Target versions") + end + + it "matches a work package through a non-primary target version" do + primary_version = create(:version, project:) + secondary_version = create(:version, project:) + work_package = create_work_package_with_time_entry(version: primary_version) + work_package.work_package_versions.create!(version: secondary_version, kind: "target") + + query.filter :version_id, operator: "=", value: secondary_version.id + expect(query.result.count).to eq(1) + end + + # OPEN POINT FND-178: cost reports over-count totals when grouping or + # filtering by a multi-value attribute. The target-version join is + # one-to-many, so a work package whose target versions both match the + # filter contributes one row per matching version. This double-count is + # accepted for now (team decision); the spec pins it so a later "fix" + # doesn't silently change the total without revisiting FND-178. + it "counts a work package once per matching target version (FND-178 over-count)" do + version1 = create(:version, project:) + version2 = create(:version, project:) + work_package = create_work_package_with_time_entry(version: version1) + work_package.work_package_versions.create!(version: version2, kind: "target") + + query.filter :version_id, operator: "=", value: [version1.id, version2.id] + expect(query.result.count).to eq(2) + end + + it "excludes a work package that targets the version when filtering 'is not'" do + version1 = create(:version, project:) + version2 = create(:version, project:) + other_version = create(:version, project:) + multi = create_work_package_with_time_entry(version: version1) + multi.work_package_versions.create!(version: version2, kind: "target") + create_work_package_with_time_entry(version: other_version) + + query.filter :version_id, operator: "!", value: [version1.id] + # `multi` targets version1, so "is not version1" must drop it even + # though it also targets version2; only the other work package remains. + expect(query.result.count).to eq(1) + end + end + it "filters subject" do matching_work_package = create_work_package_with_time_entry(subject: "matching subject") query.filter :subject, operator: "=", value: "matching subject" diff --git a/modules/reporting/spec/models/cost_query/group_by_spec.rb b/modules/reporting/spec/models/cost_query/group_by_spec.rb index 4c7d6a5d8bc9..af66df97e29f 100644 --- a/modules/reporting/spec/models/cost_query/group_by_spec.rb +++ b/modules/reporting/spec/models/cost_query/group_by_spec.rb @@ -97,6 +97,101 @@ expect(query.result.size).to eq(2) end + it "labels the group 'Version' while multiple versions is off" do + expect(CostQuery::GroupBy::VersionId.label).to eq("Version") + end + + # While the feature is off a work package is single-version, so it is grouped + # under its primary target version only (the lowest version id, i.e. what + # target_versions.first returns) and the grouped total matches the ungrouped + # entry count. + it "computes group_by Version, listing a work package under its primary target version" do + version1 = create(:version, project: project1) + version2 = create(:version, project: project1) + work_package = create(:work_package, project: project1, type:, version: version1) + work_package.work_package_versions.create!(version: version2, kind: "target") + create(:time_entry, entity: work_package, project: project1, spent_on: Date.new(2012, 1, 1)) + + query.group_by :version_id + # work_package1 / work_package2 entries have no target version (one group); + # the new work package adds a single primary-version group. + expect(query.result.size).to eq(2) + + total_count = query.result.each_direct_result.sum(&:count) + expect(total_count).to eq(Entry.count) + end + + # Filter and group-by declare the same target-version join; the engine + # collapses it only while both emit an identical join statement. If they + # drift apart the combined query fails with a duplicate-table error. + it "combines the version filter and group-by on a single join" do + version = create(:version, project: project1) + work_package = create(:work_package, project: project1, type:, version:) + create(:time_entry, entity: work_package, project: project1, spent_on: Date.new(2012, 1, 1)) + + query.filter :version_id, operator: "=", value: version.id + query.group_by :version_id + expect(query.result.size).to eq(1) + end + + context "with multiple target versions enabled", + with_flag: { work_package_multiple_versions: true }, + with_settings: { work_package_multiple_versions: true } do + it "labels the group 'Target versions'" do + expect(CostQuery::GroupBy::VersionId.label).to eq("Target versions") + end + + it "computes group_by Version, listing a work package under each target version" do + version1 = create(:version, project: project1) + version2 = create(:version, project: project1) + work_package = create(:work_package, project: project1, type:, version: version1) + work_package.work_package_versions.create!(version: version2, kind: "target") + create(:time_entry, entity: work_package, project: project1, spent_on: Date.new(2012, 1, 1)) + + query.group_by :version_id + # work_package1 / work_package2 entries have no target version (one group); + # the new work package is reported under both of its target versions. + expect(query.result.size).to eq(3) + + # OPEN POINT FND-178: cost reports over-count totals when grouping or + # filtering by a multi-value attribute. The single time entry is counted + # once under each target-version group, so the grouped total exceeds the + # ungrouped entry count. This is accepted for now (team decision); the + # assertion pins the inflated total, not just the group count, so a later + # "fix" can't quietly change it without revisiting FND-178. + total_count = query.result.each_direct_result.sum(&:count) + expect(total_count).to eq(Entry.count + 1) + end + + # Same duplicate-join guard as the off-mode spec, on the all-versions join. + it "combines the version filter and group-by on a single join" do + version1 = create(:version, project: project1) + version2 = create(:version, project: project1) + work_package = create(:work_package, project: project1, type:, version: version1) + work_package.work_package_versions.create!(version: version2, kind: "target") + create(:time_entry, entity: work_package, project: project1, spent_on: Date.new(2012, 1, 1)) + + query.filter :version_id, operator: "=", value: [version1.id, version2.id] + query.group_by :version_id + # One group per matching target version of the single work package. + expect(query.result.size).to eq(2) + end + end + + it "does not group a Meeting time entry under a same-id work package's target version" do + version = create(:version, project: project1) + work_package = create(:work_package, project: project1, type:, version:) + # entries.entity_id is polymorphic; simulate a meeting time entry whose id + # collides with the versioned work package's id. + meeting_entry = create(:time_entry, entity: work_package, project: project1, spent_on: Date.new(2012, 1, 1)) + meeting_entry.update_columns(entity_type: "Meeting") + + query.group_by :version_id + # The meeting entry must stay in the no-version group with work_package1 / + # work_package2 rather than inheriting the work package's target version. + expect(query.result.size).to eq(1) + end + it "computes group_by CostType" do query.group_by :cost_type_id # type 'Labor' for time entries, 2 different cost types