From 3f5566b7385dc26de9a276f301f0b29615dfcacd Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:28:16 +0200 Subject: [PATCH 01/14] Add the multiple categories feature gate Introduces the experimental work_package_multiple_categories feature flag together with the user-facing setting of the same name, and Setting::WorkPackageMultipleCategories as the single predicate call sites should ask. Mirrors Setting::WorkPackageMultipleVersions so the phased rollout (adding the admin switch, then dropping the flag) only ever touches that method. Co-Authored-By: Claude Opus 5 --- ...=> multi_value_attribute_normalization.rb} | 0 .../work_package_multiple_categories.rb | 43 +++++++++++++++ config/constants/settings/definition.rb | 5 ++ config/initializers/feature_decisions.rb | 4 ++ ...ined_versions.rb => joined_association.rb} | 0 .../work_package_multiple_categories_spec.rb | 54 +++++++++++++++++++ 6 files changed, 106 insertions(+) rename app/controllers/work_packages/{target_version_normalization.rb => multi_value_attribute_normalization.rb} (100%) create mode 100644 app/models/setting/work_package_multiple_categories.rb rename lib/open_project/journal_formatter/{joined_versions.rb => joined_association.rb} (100%) create mode 100644 spec/models/setting/work_package_multiple_categories_spec.rb diff --git a/app/controllers/work_packages/target_version_normalization.rb b/app/controllers/work_packages/multi_value_attribute_normalization.rb similarity index 100% rename from app/controllers/work_packages/target_version_normalization.rb rename to app/controllers/work_packages/multi_value_attribute_normalization.rb diff --git a/app/models/setting/work_package_multiple_categories.rb b/app/models/setting/work_package_multiple_categories.rb new file mode 100644 index 000000000000..b2cb305bf411 --- /dev/null +++ b/app/models/setting/work_package_multiple_categories.rb @@ -0,0 +1,43 @@ +# 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 Setting + # Single gate for the "multiple categories" feature: active only when both the + # user-facing setting and the experimental feature flag are enabled. + # Call sites (views, contracts, services) should ask this predicate rather than + # reading either gate directly, so the phased rollout (adding the admin switch, + # then dropping the flag) only ever touches this method. + module WorkPackageMultipleCategories + def self.active? + Setting.work_package_multiple_categories? && + OpenProject::FeatureDecisions.work_package_multiple_categories_active? + end + end +end diff --git a/config/constants/settings/definition.rb b/config/constants/settings/definition.rb index b5f0dddd932e..638b1457f1c2 100644 --- a/config/constants/settings/definition.rb +++ b/config/constants/settings/definition.rb @@ -1359,6 +1359,11 @@ class Definition default: "field", allowed: %w[field status] }, + work_package_multiple_categories: { + description: "Enable multiple category assignments on work packages.", + format: :boolean, + default: false + }, work_package_multiple_versions: { description: "Enable multiple version assignments on work packages.", format: :boolean, diff --git a/config/initializers/feature_decisions.rb b/config/initializers/feature_decisions.rb index d9a18053082b..576e54cab444 100644 --- a/config/initializers/feature_decisions.rb +++ b/config/initializers/feature_decisions.rb @@ -60,6 +60,10 @@ description: "Enables assigning multiple (target) versions to a work package. " \ "Experimental; the user-facing setting and admin switch follow later." +OpenProject::FeatureDecisions.add :work_package_multiple_categories, + description: "Enables assigning multiple categories to a work package. " \ + "Experimental; the user-facing setting and admin switch follow later." + OpenProject::FeatureDecisions.add :sprint_reports, description: "Enables sprint reporting within the backlogs module. " \ "It shows a dashboard with various widgets regarding the sprint progress." diff --git a/lib/open_project/journal_formatter/joined_versions.rb b/lib/open_project/journal_formatter/joined_association.rb similarity index 100% rename from lib/open_project/journal_formatter/joined_versions.rb rename to lib/open_project/journal_formatter/joined_association.rb diff --git a/spec/models/setting/work_package_multiple_categories_spec.rb b/spec/models/setting/work_package_multiple_categories_spec.rb new file mode 100644 index 000000000000..3fb0d09a0a5f --- /dev/null +++ b/spec/models/setting/work_package_multiple_categories_spec.rb @@ -0,0 +1,54 @@ +# 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 "spec_helper" + +RSpec.describe Setting::WorkPackageMultipleCategories do + # Both the setting and the feature flag must be enabled for the feature to be active. + context "when the feature flag is active", with_flag: { work_package_multiple_categories: true } do + context "and the setting is enabled", with_settings: { work_package_multiple_categories: true } do + it { expect(described_class.active?).to be true } + end + + context "and the setting is disabled", with_settings: { work_package_multiple_categories: false } do + it { expect(described_class.active?).to be false } + end + end + + context "when the feature flag is inactive", with_flag: { work_package_multiple_categories: false } do + context "and the setting is enabled", with_settings: { work_package_multiple_categories: true } do + it { expect(described_class.active?).to be false } + end + + context "and the setting is disabled", with_settings: { work_package_multiple_categories: false } do + it { expect(described_class.active?).to be false } + end + end +end From a874b7e1f85f69385c3c09348a840dfed485e1e9 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:28:24 +0200 Subject: [PATCH 02/14] Create the work package categories join table Adds work_package_categories as the future home of a work package's categories, backfilled from the deprecated work_packages.category_id column. Unlike work_package_versions the table needs no kind discriminator: a work package holds exactly one set of categories. Both foreign keys cascade, because categories are removed with their project through a dependent: :delete_all that skips callbacks. Co-Authored-By: Claude Opus 5 --- app/models/work_package_category.rb | 34 ++++++++++++++ ...05090000_create_work_package_categories.rb | 47 +++++++++++++++++++ ...0_backfill_categories_from_work_package.rb | 21 +++++++++ .../work_package_category_factory.rb | 34 ++++++++++++++ 4 files changed, 136 insertions(+) create mode 100644 app/models/work_package_category.rb create mode 100644 db/migrate/20260805090000_create_work_package_categories.rb create mode 100644 db/migrate/20260805090100_backfill_categories_from_work_package.rb create mode 100644 spec/factories/work_package_category_factory.rb diff --git a/app/models/work_package_category.rb b/app/models/work_package_category.rb new file mode 100644 index 000000000000..12c89e6f1268 --- /dev/null +++ b/app/models/work_package_category.rb @@ -0,0 +1,34 @@ +# 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 WorkPackageCategory < ApplicationRecord + belongs_to :work_package + belongs_to :category +end diff --git a/db/migrate/20260805090000_create_work_package_categories.rb b/db/migrate/20260805090000_create_work_package_categories.rb new file mode 100644 index 000000000000..e21ea7636eb8 --- /dev/null +++ b/db/migrate/20260805090000_create_work_package_categories.rb @@ -0,0 +1,47 @@ +# 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 CreateWorkPackageCategories < ActiveRecord::Migration[8.1] + def change + create_table :work_package_categories do |t| + t.references :work_package, null: false, foreign_key: { on_delete: :cascade }, index: false + # Categories are removed with their project through a dependent: :delete_all, + # which skips callbacks, so the join rows have to go away in the database. + t.references :category, null: false, foreign_key: { on_delete: :cascade }, index: false + t.timestamps + end + + add_index :work_package_categories, %i[work_package_id category_id], + unique: true, + name: "idx_wp_categories_on_wp_category" + add_index :work_package_categories, :category_id, + name: "idx_wp_categories_on_category" + end +end diff --git a/db/migrate/20260805090100_backfill_categories_from_work_package.rb b/db/migrate/20260805090100_backfill_categories_from_work_package.rb new file mode 100644 index 000000000000..9d6534e3fb0b --- /dev/null +++ b/db/migrate/20260805090100_backfill_categories_from_work_package.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class BackfillCategoriesFromWorkPackage < ActiveRecord::Migration[8.1] + def up + say_with_time "Copying work_packages.category_id into work_package_categories" do + execute <<~SQL.squish + INSERT INTO work_package_categories (work_package_id, category_id, created_at, updated_at) + SELECT work_packages.id, work_packages.category_id, now(), now() + FROM work_packages + INNER JOIN categories ON categories.id = work_packages.category_id + WHERE work_packages.category_id IS NOT NULL + ON CONFLICT (work_package_id, category_id) DO NOTHING + SQL + end + end + + def down + # The join rows are redundant with work_packages.category_id for as long as + # the deprecated column is mirrored, so there is nothing to restore. + end +end diff --git a/spec/factories/work_package_category_factory.rb b/spec/factories/work_package_category_factory.rb new file mode 100644 index 000000000000..d9a99a8c329e --- /dev/null +++ b/spec/factories/work_package_category_factory.rb @@ -0,0 +1,34 @@ +# 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. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +FactoryBot.define do + factory :work_package_category do + work_package + category + created_at { Time.zone.now } + updated_at { Time.zone.now } + end +end From 09e5c1e7db9ad7ed1b2106ba6be25b5f7bf2956a Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:28:32 +0200 Subject: [PATCH 03/14] Add the categories association to the work package model WorkPackage::Categories mirrors WorkPackage::Versions: a name-ordered categories association, a category_ids_replacements accessor the contracts/services flow can validate before anything is written, an effective_categories reader that accounts for a pending override, and an after_save hook that keeps the deprecated category_id column in sync with the first category. Ordering by name means #category and #categories.first can never disagree. Destroying a category now moves or prunes its join rows and recomputes the deprecated column for work packages that keep another category, which neither dependent: :nullify nor the bulk reassignment could get right on their own. #default_assign and .by_category read the set rather than the single column, so a work package with several categories takes its default assignee from the primary one and is counted under each of its categories. Co-Authored-By: Claude Opus 5 --- app/models/category.rb | 47 ++++- app/models/work_package.rb | 36 +++- app/models/work_package/categories.rb | 170 ++++++++++++++++ spec/models/category_spec.rb | 37 ++++ spec/models/work_package/categories_spec.rb | 207 ++++++++++++++++++++ spec/models/work_package_spec.rb | 2 + 6 files changed, 486 insertions(+), 13 deletions(-) create mode 100644 app/models/work_package/categories.rb create mode 100644 spec/models/work_package/categories_spec.rb diff --git a/app/models/category.rb b/app/models/category.rb index f0de8dce08f1..1dcb2da46026 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -31,7 +31,11 @@ class Category < ApplicationRecord belongs_to :project belongs_to :assigned_to, class_name: "Principal" + # Clears the deprecated single-category column; the join rows below carry the + # actual assignments. has_many :work_packages, dependent: :nullify + has_many :work_package_categories, dependent: :delete_all + has_many :categorized_work_packages, through: :work_package_categories, source: :work_package validates :name, uniqueness: { scope: [:project_id], case_sensitive: false }, @@ -49,10 +53,15 @@ class Category < ApplicationRecord # Destroy the category # If a category is specified, issues are reassigned to this category def destroy(reassign_to = nil) - if reassign_to && reassign_to.is_a?(Category) && reassign_to.project == project - WorkPackage.where("category_id = #{id}").update_all("category_id = #{reassign_to.id}") + affected_work_package_ids = work_package_categories.pluck(:work_package_id) + + if reassign_to.is_a?(Category) && reassign_to.project == project + reassign_work_packages_to(reassign_to) + end + + destroy_without_reassign.tap do + resync_legacy_category_ids(affected_work_package_ids) end - destroy_without_reassign end def <=>(other) @@ -60,4 +69,36 @@ def <=>(other) end def to_s; name end + + private + + def reassign_work_packages_to(other) + # Work packages that already carry the target category would violate the join + # table's uniqueness, so their row for this category is dropped rather than + # moved over. + already_assigned = WorkPackageCategory.where(category_id: other.id).select(:work_package_id) + work_package_categories.where(work_package_id: already_assigned).delete_all + + work_package_categories.update_all(category_id: other.id, updated_at: Time.current) + end + + # The deprecated work_packages.category_id column mirrors the alphabetically + # first category of a work package. Both destroy paths above touch it bluntly + # (`dependent: :nullify` clears it, the reassignment above overwrites it), which + # is wrong for work packages that hold more than one category. Recompute it from + # what is left in the join table. Can be dropped along with the column. + def resync_legacy_category_ids(work_package_ids) + return if work_package_ids.empty? + + WorkPackage.where(id: work_package_ids).update_all(<<~SQL.squish) + category_id = ( + SELECT work_package_categories.category_id + FROM work_package_categories + INNER JOIN categories ON categories.id = work_package_categories.category_id + WHERE work_package_categories.work_package_id = work_packages.id + ORDER BY categories.name, categories.id + LIMIT 1 + ) + SQL + end end diff --git a/app/models/work_package.rb b/app/models/work_package.rb index 460903f26771..65ebdccd465c 100644 --- a/app/models/work_package.rb +++ b/app/models/work_package.rb @@ -38,9 +38,10 @@ class WorkPackage < ApplicationRecord include WorkPackage::Ancestors include WorkPackage::CustomActioned include WorkPackage::Hooks - # Must stay above WorkPackage::Journalized: its after_save persists the - # version rows that the journal snapshot then reads. + # Must stay above WorkPackage::Journalized: their after_save hooks persist the + # version and category rows that the journal snapshot then reads. include WorkPackage::Versions + include WorkPackage::Categories include WorkPackages::DerivedDates include WorkPackages::SpentTime include WorkPackages::Costs @@ -62,7 +63,6 @@ class WorkPackage < ApplicationRecord belongs_to :responsible, class_name: "Principal", optional: true belongs_to :project_phase_definition, class_name: "Project::PhaseDefinition", optional: true belongs_to :priority, class_name: "IssuePriority" - belongs_to :category, class_name: "Category", optional: true has_many :time_entries, dependent: :delete_all, inverse_of: :entity, as: :entity has_many :file_links, dependent: :delete_all, class_name: "Storages::FileLink", as: :container @@ -441,9 +441,22 @@ def self.by_priority(project) end def self.by_category(project) - count_and_group_by project:, - field: "category_id", - joins: Category.table_name + # Counts via the category associations rather than the deprecated category_id + # column, so a work package assigned to several categories is counted under + # each of them. + sql = sanitize_sql_array( + ["SELECT s.id AS status_id, + s.is_closed AS closed, + wpc.category_id AS category_id, + COUNT(i.id) AS total + FROM #{WorkPackage.table_name} i + INNER JOIN #{Status.table_name} s ON i.status_id = s.id + INNER JOIN #{WorkPackageCategory.table_name} wpc ON wpc.work_package_id = i.id + WHERE i.project_id = :project_id + GROUP BY s.id, s.is_closed, wpc.category_id", + { project_id: project.id }] + ) + ActiveRecord::Base.connection.select_all(sql).to_a end def self.by_assigned_to(project) @@ -620,11 +633,14 @@ def time_entry_blank?(attributes) default_id && attributes.except(key).values.all?(&:blank?) end - # Default assignment based on category + # Default assignment based on the primary category. Reads the effective set + # because the categories are only written after_save, so a pending override is + # not yet reflected in #category at before_create time. def default_assign - if assigned_to.nil? && category&.assigned_to - self.assigned_to = category.assigned_to - end + return unless assigned_to.nil? + + primary_category = effective_categories.first + self.assigned_to = primary_category.assigned_to if primary_category&.assigned_to end # Closes duplicates if the work_package is being closed diff --git a/app/models/work_package/categories.rb b/app/models/work_package/categories.rb new file mode 100644 index 000000000000..437ef3d8ff00 --- /dev/null +++ b/app/models/work_package/categories.rb @@ -0,0 +1,170 @@ +# 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 WorkPackage::Categories + extend ActiveSupport::Concern + + included do + # Deprecated single-category column, kept in sync with the first category + # (see #update_legacy_category_field). Can be dropped once all subsystems + # read categories instead. + belongs_to :category, class_name: "Category", optional: true + + has_many :work_package_categories, dependent: :delete_all + # Ordered by name, matching every other category listing (see + # Project#categories). The order also decides which category is mirrored + # into the deprecated category_id column, so #category and #categories.first + # can never disagree. + has_many :categories, + -> { order(:name) }, + through: :work_package_categories, source: :category + + scope :with_category, ->(category_id) { + where(id: WorkPackageCategory.where(category_id:).select(:work_package_id)) + } + + # Work packages with no category at all. Reading the join table rather than + # the deprecated column keeps this correct once a work package can hold + # categories that the single column cannot represent. + scope :without_category, -> { + where.not(id: WorkPackageCategory.select(:work_package_id)) + } + + # Must be registered before `save_journals` (WorkPackage::Journalized) so + # that the journal snapshot sees the current category set in the database. + after_save :persist_category_associations + + # Stores in memory the values that will replace the written categories. + # This is used by the contracts/services flow in order to do checks and + # validations before persisting any actual data to the database. + attr_accessor :category_ids_replacements + end + + # Categories the work package can be assigned to. Unlike versions, categories + # are never shared with or inherited by other projects, so this is exactly the + # project's own set. Name-ordered, matching #categories. + def assignable_categories + project&.categories || Category.none + end + + # The category_ids_replacements accessor behaves according to these rules: + # - when nil (the default) - leave the existing associations untouched + # - when [] - clear the association + # - when [] - replace the existing set with exactly these + # Consequently, nil vs. non-nil tells us whether an override was requested + # at all. + def override_categories? = !category_ids_replacements.nil? + + # List of categories, but takes into account a pending override that was not + # written yet. + # + # By precedence: + # * category_ids_replacements + # * pending category_id change + # * actual written categories + # + # Name-ordered like the association, so #effective_categories.first is always + # the category that a save would mirror into category_id. + def effective_categories + if category_ids_replacements.nil? + return category_id_changed? ? Array(category) : categories + end + + Category.where(id: category_ids_replacements).order(:name).to_a + end + + private + + # Two paths feed the categories association: + # * an explicit override (category_ids_replacements was set) takes + # precedence and replaces the whole set. + # * otherwise, a plain change to category_id (the legacy single-category + # path) is mirrored into the association so both stay consistent. + # + # Writing to category_id will be removed after all subsystems start using + # categories instead. + def persist_category_associations + if override_categories? + replace_categories(category_ids_replacements) + update_legacy_category_field + elsif saved_change_to_category_id? + replace_categories(Array(category_id)) + end + + clear_category_override + end + + # The override is consumed by exactly one save. Left in place, it would be + # re-applied by any later save of the same instance, clobbering category + # changes made in between. + def clear_category_override + self.category_ids_replacements = nil + end + + # Keeps the deprecated single category_id column in sync with the first + # category, so code still reading category_id sees a sensible value. + # Can be dropped once the category_id column is removed. + def update_legacy_category_field + # Read the association fresh: replace_categories only resets it when the set + # actually changed, so a cached target could otherwise mirror a stale first + # category. + categories.reset + new_category_id = categories.first&.id + + update_columns(category_id: new_category_id) unless category_id == new_category_id + end + + # Resets any cached values. Necessary because we do insert_all. + def reset_category_associations + work_package_categories.reset + categories.reset + end + + # Sets the work package's category associations to exactly the given + # category_ids. + def replace_categories(category_ids) + existing = work_package_categories.pluck(:category_id) + + to_remove = existing - category_ids + to_add = category_ids - existing + + return if to_remove.empty? && to_add.empty? + + apply_category_changes(to_remove, to_add) + reset_category_associations + end + + # remove associations that are not present in the new list of categories and + # add those that were not already there + def apply_category_changes(to_remove, to_add) + work_package_categories.where(category_id: to_remove).delete_all if to_remove.any? + work_package_categories.insert_all(to_add.map { |cid| { category_id: cid } }) if to_add.any? + end +end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index c97141db9c88..7cc850554de1 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -67,6 +67,12 @@ .to be_nil end + it "removes the category from the work package's set" do + created_category.destroy + + expect(work_package.reload.categories).to be_empty + end + it "allows reassigning to a different category" do other_category = create(:category, project:) @@ -75,5 +81,36 @@ expect(work_package.reload.category) .to eq other_category end + + it "reassigns the work package's set to the other category" do + other_category = create(:category, project:) + + created_category.destroy(other_category) + + expect(work_package.reload.categories).to eq [other_category] + end + + context "with a work package holding more than one category" do + let(:kept_category) { create(:category, project:, name: "Kept") } + + before do + work_package.category_ids_replacements = [created_category.id, kept_category.id] + work_package.save! + end + + it "keeps the remaining category and re-mirrors it into the deprecated column" do + created_category.destroy + + expect(work_package.reload.categories).to eq [kept_category] + expect(work_package.category).to eq kept_category + end + + it "drops the row instead of duplicating when reassigning to a category already assigned" do + created_category.destroy(kept_category) + + expect(work_package.reload.categories).to eq [kept_category] + expect(work_package.category).to eq kept_category + end + end end end diff --git a/spec/models/work_package/categories_spec.rb b/spec/models/work_package/categories_spec.rb new file mode 100644 index 000000000000..d38375be5165 --- /dev/null +++ b/spec/models/work_package/categories_spec.rb @@ -0,0 +1,207 @@ +# 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 "spec_helper" + +RSpec.describe WorkPackage::Categories do + shared_let(:project) { create(:project) } + shared_let(:category_a) { create(:category, project:, name: "Alpha") } + shared_let(:category_b) { create(:category, project:, name: "Beta") } + shared_let(:category_c) { create(:category, project:, name: "Gamma") } + + let(:work_package) { create(:work_package, project:) } + + describe "#category_ids_replacements" do + it "replaces the whole set on save" do + work_package.category_ids_replacements = [category_b.id, category_a.id] + work_package.save! + + expect(work_package.reload.categories).to eq([category_a, category_b]) + end + + it "clears the set when assigned an empty array" do + work_package.category_ids_replacements = [category_a.id] + work_package.save! + + work_package.category_ids_replacements = [] + work_package.save! + + expect(work_package.reload.categories).to be_empty + end + + it "leaves the set untouched when nil" do + work_package.category_ids_replacements = [category_a.id] + work_package.save! + + work_package.subject = "Changed" + work_package.save! + + expect(work_package.reload.categories).to eq([category_a]) + end + + it "is consumed by a single save" do + work_package.category_ids_replacements = [category_a.id] + work_package.save! + + expect(work_package.category_ids_replacements).to be_nil + end + + it "only touches the rows that actually change" do + work_package.category_ids_replacements = [category_a.id, category_b.id] + work_package.save! + untouched = work_package.work_package_categories.find_by(category_id: category_a.id) + + work_package.category_ids_replacements = [category_a.id, category_c.id] + work_package.save! + + expect(work_package.reload.categories).to eq([category_a, category_c]) + expect(work_package.work_package_categories.find_by(category_id: category_a.id).id) + .to eq(untouched.id) + end + end + + describe "the deprecated category_id column" do + it "mirrors the alphabetically first category of an override" do + work_package.category_ids_replacements = [category_c.id, category_b.id] + work_package.save! + + expect(work_package.reload.category).to eq(category_b) + end + + it "is cleared when the set is cleared" do + work_package.category_ids_replacements = [category_a.id] + work_package.save! + + work_package.category_ids_replacements = [] + work_package.save! + + expect(work_package.reload.category).to be_nil + end + + it "is mirrored into the association when written on its own" do + work_package.category = category_b + work_package.save! + + expect(work_package.reload.categories).to eq([category_b]) + end + + it "always agrees with the first category" do + work_package.category_ids_replacements = [category_c.id, category_a.id, category_b.id] + work_package.save! + work_package.reload + + expect(work_package.category).to eq(work_package.categories.first) + end + end + + describe "#effective_categories" do + it "returns the written categories without a pending change" do + work_package.category_ids_replacements = [category_a.id] + work_package.save! + + expect(work_package.reload.effective_categories).to eq([category_a]) + end + + it "returns the pending override, name-ordered" do + work_package.category_ids_replacements = [category_c.id, category_a.id] + + expect(work_package.effective_categories).to eq([category_a, category_c]) + end + + it "returns the pending legacy category_id change" do + work_package.category = category_b + + expect(work_package.effective_categories).to eq([category_b]) + end + + it "prefers the override over a pending legacy change" do + work_package.category = category_b + work_package.category_ids_replacements = [category_a.id] + + expect(work_package.effective_categories).to eq([category_a]) + end + end + + describe "#assignable_categories" do + it "returns the project's categories, name-ordered" do + expect(work_package.assignable_categories).to eq([category_a, category_b, category_c]) + end + + it "excludes categories of other projects" do + other_project_category = create(:category, project: create(:project), name: "Aaa foreign") + + expect(work_package.assignable_categories).not_to include(other_project_category) + end + + it "is empty without a project" do + expect(WorkPackage.new.assignable_categories).to be_empty + end + end + + describe "scopes" do + let!(:categorized) do + create(:work_package, project:).tap do |wp| + wp.category_ids_replacements = [category_a.id] + wp.save! + end + end + let!(:uncategorized) { create(:work_package, project:) } + + describe ".with_category" do + it "returns only work packages carrying the given category" do + expect(WorkPackage.with_category(category_a.id)).to contain_exactly(categorized) + end + + it "returns nothing for an unassigned category" do + expect(WorkPackage.with_category(category_b.id)).to be_empty + end + end + + describe ".without_category" do + it "returns only work packages without any category" do + expect(WorkPackage.without_category).to contain_exactly(uncategorized) + end + end + end + + describe "#default_assign" do + let(:member) { create(:user, member_with_permissions: { project => [:view_work_packages] }) } + + before { category_a.update!(assigned_to: member) } + + it "takes the assignee from the primary category of a pending override" do + work_package = build(:work_package, project:, assigned_to: nil) + work_package.category_ids_replacements = [category_b.id, category_a.id] + work_package.save! + + expect(work_package.assigned_to).to eq(member) + end + end +end diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index 790b2cdb4ad2..9144bccf93fb 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -88,6 +88,8 @@ it { is_expected.to have_many(:versions).through(:work_package_versions).source(:version) } it { is_expected.to have_many(:target_versions).through(:work_package_versions).source(:version) } it { is_expected.to have_many(:observed_in_versions).through(:work_package_versions).source(:version) } + it { is_expected.to have_many(:work_package_categories).dependent(:delete_all) } + it { is_expected.to have_many(:categories).through(:work_package_categories).source(:category) } end describe ".new" do From cf47c5aa1b990f83377ae453a751c0e5fb9bd0df Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:28:50 +0200 Subject: [PATCH 04/14] Read and write categories through contracts and services The set-attributes service routes a category_ids attribute through the replacements accessor rather than the association writer generated by has_many, which would write to the database before the contract can validate. A project change reassigns the whole set to the equally named categories of the new project instead of just the single one. The contract validates assignability and enforces the two constraints of the transition: categories holds at most one entry unless the multiple categories feature is enabled, and a category_id the user writes has to be part of the set they write. Co-Authored-By: Claude Opus 5 --- app/contracts/work_packages/base_contract.rb | 56 ++++++- app/models/permitted_params.rb | 1 + .../work_packages/set_attributes_service.rb | 34 +++- config/locales/en.yml | 2 + .../work_packages/base_contract_spec.rb | 145 ++++++++++++++++++ .../set_attributes_service_spec.rb | 24 ++- 6 files changed, 247 insertions(+), 15 deletions(-) diff --git a/app/contracts/work_packages/base_contract.rb b/app/contracts/work_packages/base_contract.rb index 917cbece33cf..3a81cdf30b4a 100644 --- a/app/contracts/work_packages/base_contract.rb +++ b/app/contracts/work_packages/base_contract.rb @@ -46,6 +46,9 @@ class BaseContract < ::ModelContract attribute :type_id attribute :priority_id attribute :category_id + attribute :categories do + validate_categories_are_assignable + end attribute :version_id, permission: :assign_versions do validate_version_is_assignable @@ -65,6 +68,7 @@ class BaseContract < ::ModelContract validate :validate_no_reopen_on_closed_version validate :validate_versions_permission validate :validate_target_versions_and_legacy_version_id + validate :validate_categories_and_legacy_category_id attribute :project_id @@ -218,7 +222,7 @@ def assignable_types end def assignable_categories - model.project.categories if model.project.respond_to?(:categories) + model.assignable_categories if model.project.respond_to?(:categories) end def assignable_priorities @@ -395,6 +399,56 @@ def validate_category end end + # While the deprecated single category_id column coexists with the categories + # association, the two must not contradict each other. This enforces both + # constraints of that transitional period in one place: + # * categories behaves as a single value (at most one entry) unless the + # multiple-categories feature is enabled, and + # * category_id and categories may both be written in one request as long as + # they agree; only an actual contradiction is rejected. + def validate_categories_and_legacy_category_id + return unless model.override_categories? + + validate_categories_length + validate_category_and_categories_not_contradict + end + + def validate_categories_length + return if Setting::WorkPackageMultipleCategories.active? + + if model.category_ids_replacements.length > 1 + errors.add :base, :categories_only_allow_single_value + end + end + + def validate_category_and_categories_not_contradict + # Only a user writing both fields is a real contradiction. category_id is + # also rewritten by the system (e.g. on a project move, when the old + # category has no counterpart in the target project); that change is driven + # by the categories override and must not be flagged here. + return unless changed_by_user.include?("category_id") + + # The deprecated column is re-derived from the set on save, so a category_id + # the user wrote only has to be part of the set they wrote. Which member ends + # up mirrored is decided by the set's (name) ordering. + contradiction = if model.category_id.nil? + model.category_ids_replacements.any? + else + model.category_ids_replacements.exclude?(model.category_id) + end + + errors.add :base, :category_and_categories_mutually_exclusive if contradiction + end + + def validate_categories_are_assignable + return if model.category_ids_replacements.nil? + + assignable_ids = assignable_categories&.map(&:id) || [] + if (model.category_ids_replacements - assignable_ids).any? + errors.add :categories, :inclusion + end + end + def validate_version_is_assignable if model.version_id && model.assignable_versions.map(&:id).exclude?(model.version_id) errors.add :version_id, :inclusion diff --git a/app/models/permitted_params.rb b/app/models/permitted_params.rb index aa9309e4d35e..f8b0ba15733b 100644 --- a/app/models/permitted_params.rb +++ b/app/models/permitted_params.rb @@ -567,6 +567,7 @@ def self.permitted_attributes :assigned_to_id, { attachments: %i[file description] }, :category_id, + { category_ids: [] }, :description, :done_ratio, :due_date, diff --git a/app/services/work_packages/set_attributes_service.rb b/app/services/work_packages/set_attributes_service.rb index 15ee1b7f72ee..600dd816e78f 100644 --- a/app/services/work_packages/set_attributes_service.rb +++ b/app/services/work_packages/set_attributes_service.rb @@ -45,6 +45,7 @@ def set_attributes(attributes) set_attachments_attributes(attributes) set_versions_attributes(attributes) + set_categories_attributes(attributes) set_static_attributes(attributes) model.change_by_system do @@ -73,6 +74,16 @@ def set_versions_attributes(attributes) model.observed_in_version_ids_replacements = Array(observed_in_ids).map(&:to_i) if observed_in_ids end + # Routed through the replacements accessor rather than the association writer + # generated by has_many, which would write to the database before the contract + # ever gets to validate. Removing the key also keeps set_static_attributes from + # picking up `category_ids=`. + def set_categories_attributes(attributes) + category_ids = attributes.delete(:category_ids) + + model.category_ids_replacements = Array(category_ids).map(&:to_i) if category_ids + end + def set_static_attributes(attributes) assignable_attributes = attributes.select do |key, _| !CustomField.custom_field_attribute?(key) && work_package.respond_to?("#{key}=") @@ -276,7 +287,7 @@ def update_project_dependent_attributes model.change_by_system do set_versions_to_nil - reassign_category + reassign_categories set_parent_to_nil clear_semantic_identifier @@ -419,14 +430,21 @@ def set_parent_to_nil end end - def reassign_category + def reassign_categories # work_package is moved to another project - # reassign to the category with same name if any - if work_package.category.present? - category = work_package.project.categories.find_by(name: work_package.category.name) - - work_package.category = category - end + # reassign every category to the one of the same name in the new project, + # dropping the ones that have no counterpart there + current_categories = work_package.effective_categories + return if current_categories.empty? + + # Project#categories is name-ordered, so the first match is the one that gets + # mirrored into the deprecated column on save. + reassigned = work_package.project.categories.where(name: current_categories.map(&:name)).to_a + + work_package.category_ids_replacements = reassigned.map(&:id) + # Without this the deprecated column would still point at a category of the + # old project, which the contract rejects. The save re-derives it anyway. + work_package.category = reassigned.first end def assign_default_type diff --git a/config/locales/en.yml b/config/locales/en.yml index da1d422a670e..a65e68619bce 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -946,6 +946,8 @@ en: status_transition_invalid: "is invalid because no valid transition exists from old to new status for the current user's roles." type: cannot_be_milestone_due_to_children: "cannot be a milestone because this work package has children." + categories_only_allow_single_value: "Categories can only hold a single value." + category_and_categories_mutually_exclusive: "Category and categories cannot both be changed at the same time." is_not_a_valid_target_for_time_entries: "Work package #%{id} is not a valid target for reassigning the time entries." readonly_status: "The work package is in a readonly status so its attributes cannot be changed." target_versions_only_allow_single_value: "Target Versions can only hold a single value." diff --git a/spec/contracts/work_packages/base_contract_spec.rb b/spec/contracts/work_packages/base_contract_spec.rb index 4b25ea70a071..c69e9c679b35 100644 --- a/spec/contracts/work_packages/base_contract_spec.rb +++ b/spec/contracts/work_packages/base_contract_spec.rb @@ -1504,6 +1504,151 @@ end end + describe "categories" do + subject(:contract) { described_class.new(work_package, current_user) } + + let(:assignable_category) { build_stubbed(:category, name: "Alpha") } + let(:other_assignable_category) { build_stubbed(:category, name: "Beta") } + let(:non_assignable_category) { build_stubbed(:category, name: "Foreign") } + + before do + allow(work_package) + .to receive(:assignable_categories) + .and_return([assignable_category, other_assignable_category]) + end + + describe "assignability" do + it "is valid with assignable IDs" do + work_package.category_ids_replacements = [assignable_category.id] + contract.validate + + expect(contract.errors.symbols_for(:categories)).to be_empty + end + + it "is invalid with a category of another project" do + work_package.category_ids_replacements = [non_assignable_category.id] + contract.validate + + expect(contract.errors.symbols_for(:categories)).to include(:inclusion) + end + + it "is valid with an empty array" do + work_package.category_ids_replacements = [] + contract.validate + + expect(contract.errors.symbols_for(:categories)).to be_empty + end + + it "is valid when not overridden" do + contract.validate + + expect(contract.errors.symbols_for(:categories)).to be_empty + end + end + + describe "length" do + before do + work_package.category_ids_replacements = [assignable_category.id, other_assignable_category.id] + end + + context "when the multiple-categories feature is disabled" do + before { contract.validate } + + it "rejects more than one category" do + expect(contract.errors.symbols_for(:base)).to include(:categories_only_allow_single_value) + end + end + + context "when the multiple-categories feature is enabled", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + before { contract.validate } + + it "allows more than one category" do + expect(contract.errors.symbols_for(:base)).not_to include(:categories_only_allow_single_value) + end + end + end + + describe "mutual exclusion of category_id and category_ids" do + context "when the user changes both to different categories" do + before do + work_package.category = other_assignable_category + work_package.category_ids_replacements = [assignable_category.id] + contract.validate + end + + it "is invalid" do + expect(contract.errors.symbols_for(:base)).to include(:category_and_categories_mutually_exclusive) + end + end + + context "when both are set to the same category" do + before do + work_package.category = assignable_category + work_package.category_ids_replacements = [assignable_category.id] + contract.validate + end + + it "is valid (a consistent write is allowed)" do + expect(contract.errors.symbols_for(:base)) + .not_to include(:category_and_categories_mutually_exclusive) + end + end + + context "when the written category_id is part of a larger set", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + before do + work_package.category = other_assignable_category + work_package.category_ids_replacements = [assignable_category.id, other_assignable_category.id] + contract.validate + end + + it "is valid (the mirrored member is decided by the set's ordering)" do + expect(contract.errors.symbols_for(:base)) + .not_to include(:category_and_categories_mutually_exclusive) + end + end + + context "when the user clears category_id while writing a non-empty set" do + let(:work_package) { build_stubbed(:work_package, type:, project:, category: other_assignable_category) } + + before do + work_package.category = nil + work_package.category_ids_replacements = [assignable_category.id] + contract.validate + end + + it "is invalid" do + expect(contract.errors.symbols_for(:base)) + .to include(:category_and_categories_mutually_exclusive) + end + end + + context "when the system rewrites category_id during the change (e.g. a project move)" do + # The set-attributes service extends the model with ChangedBySystem before + # validation, so mirror that here to distinguish the system-driven change + # from a user one. + let(:work_package) do + build_stubbed(:work_package, type:, project:) + .extend(OpenProject::ChangedBySystem) + end + + before do + work_package.change_by_system { work_package.category = other_assignable_category } + work_package.category_ids_replacements = [assignable_category.id] + contract.validate + end + + it "is valid (a system-driven category_id change is not a contradiction)" do + expect(contract.errors.symbols_for(:base)) + .not_to include(:category_and_categories_mutually_exclusive) + end + end + end + end + describe "parent" do let(:parent) { build_stubbed(:work_package) } diff --git a/spec/services/work_packages/set_attributes_service_spec.rb b/spec/services/work_packages/set_attributes_service_spec.rb index 53d104554973..9eac1fdae8db 100644 --- a/spec/services/work_packages/set_attributes_service_spec.rb +++ b/spec/services/work_packages/set_attributes_service_spec.rb @@ -1826,9 +1826,9 @@ before do without_partial_double_verification do allow(new_project_categories) - .to receive(:find_by) - .with(name: category.name) - .and_return nil + .to receive(:where) + .with(name: [category.name]) + .and_return [] allow(new_project) .to receive_messages(shared_versions: new_versions, types: new_types) allow(new_types) @@ -1901,14 +1901,20 @@ expect(work_package.category) .to be_nil end + + it "clears the category set" do + subject + + expect(work_package.category_ids_replacements).to eq [] + end end context "when category of same name in new project" do before do allow(new_project_categories) - .to receive(:find_by) - .with(name: category.name) - .and_return new_category + .to receive(:where) + .with(name: [category.name]) + .and_return [new_category] end it "uses the equally named category" do @@ -1924,6 +1930,12 @@ expect(work_package.changed_by_system["category_id"]) .to eql [nil, new_category.id] end + + it "replaces the category set with the equally named category" do + subject + + expect(work_package.category_ids_replacements).to eq [new_category.id] + end end end From 7a6b208dafd98fed7ca5aad197aa488ab7fc3211 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:29:16 +0200 Subject: [PATCH 05/14] Expose categories in the APIv3 work package resource Adds a categories link collection and schema attribute alongside the now deprecated single-valued category, following targetVersions: the attribute always reads and writes a collection, and while the multiple categories feature is inactive it keeps the "Category" label and announces the single-value restriction via options.multiple. The categories enter the work package cache checksum as an aggregated correlated subquery, since a has_many cannot be expressed by the left_joins/pluck design the checksum is built on. The neighbouring versions subquery gains the same CONCAT_WS separator, so that no two distinct tuples can concatenate to the same string and collide. Co-Authored-By: Claude Opus 5 --- config/locales/en.yml | 7 ++ .../components/schemas/work_package_model.yml | 18 +++++ .../schemas/work_package_schema_model.yml | 2 + .../schemas/work_package_write_model.yml | 17 +++++ docs/api/apiv3/tags/work_packages.yml | 1 + .../work_packages/eager_loading/checksum.rb | 18 ++++- .../schema/work_package_schema_representer.rb | 27 +++++++- .../work_packages/work_package_representer.rb | 28 +++++++- .../work_package_schema_representer_spec.rb | 54 ++++++++++++++- .../work_package_representer_spec.rb | 65 +++++++++++++++++++ 10 files changed, 232 insertions(+), 5 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index a65e68619bce..45b60da61f16 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -446,6 +446,7 @@ en: ancestor: "Descendants of" # used for filtering of work packages that are descendants of a given work package begin_deletion: "Begin of the deletion" begin_insertion: "Begin of the insertion" + categories: "Categories" children: "Subelements" derived_done_ratio: "Total % complete" derived_remaining_hours: "Total remaining work" @@ -1639,6 +1640,12 @@ en: quarantined_message: "A virus was detected in file '%{filename}'. It has been quarantined and is not available for download." api_v3: attributes: + category: + deprecated: >- + Deprecated: this single-valued field is being replaced by `categories`. + Read and write `categories` instead. While multiple categories is not enabled, + `categories` accepts at most one value and must not be written together + with `category` in the same request. property: "Property" version: deprecated: >- diff --git a/docs/api/apiv3/components/schemas/work_package_model.yml b/docs/api/apiv3/components/schemas/work_package_model.yml index 49d9e9f692cc..33e2a371c105 100644 --- a/docs/api/apiv3/components/schemas/work_package_model.yml +++ b/docs/api/apiv3/components/schemas/work_package_model.yml @@ -426,6 +426,24 @@ allOf: The category of the work package **Resource**: Category + + **Deprecated**: this single-valued field is being replaced by `categories`. + Read and write `categories` instead. While multiple categories is not enabled, + `category` must not be written together with `categories` in the same request. + categories: + type: array + items: + $ref: "./link.yml" + description: |- + List of categories associated to the work package + + **Resource**: Collection of Category + + # Conditions + + - Transitioning from single to multiple category support + - (Temporary) Only allows a single value for compatibility with the category field + - (Temporary) Must not be written together with `category` in the same request children: type: array readOnly: true diff --git a/docs/api/apiv3/components/schemas/work_package_schema_model.yml b/docs/api/apiv3/components/schemas/work_package_schema_model.yml index 828110344355..ef8922333de2 100644 --- a/docs/api/apiv3/components/schemas/work_package_schema_model.yml +++ b/docs/api/apiv3/components/schemas/work_package_schema_model.yml @@ -86,6 +86,8 @@ properties: $ref: './schema_property_model.yml' category: $ref: './schema_property_model.yml' + categories: + $ref: './schema_property_model.yml' version: $ref: './schema_property_model.yml' targetVersions: diff --git a/docs/api/apiv3/components/schemas/work_package_write_model.yml b/docs/api/apiv3/components/schemas/work_package_write_model.yml index 2efc036fb0a8..6dad6dd4f3e1 100644 --- a/docs/api/apiv3/components/schemas/work_package_write_model.yml +++ b/docs/api/apiv3/components/schemas/work_package_write_model.yml @@ -63,6 +63,23 @@ properties: The category of the work package **Resource**: Category + + **Deprecated**: this single-valued field is being replaced by `categories`. + Read and write `categories` instead. While multiple categories is not enabled, + `category` must not be written together with `categories` in the same request. + categories: + type: array + items: + $ref: './link.yml' + description: |- + List of categories associated to the work package + + **Resource**: Collection of Category + + # Conditions + + - Transitioning from single to multiple category support + - (Temporary) Only allows a single value for compatibility with the category field type: allOf: - $ref: './link.yml' diff --git a/docs/api/apiv3/tags/work_packages.yml b/docs/api/apiv3/tags/work_packages.yml index 3e29ebf95623..20372e4bae4b 100644 --- a/docs/api/apiv3/tags/work_packages.yml +++ b/docs/api/apiv3/tags/work_packages.yml @@ -34,6 +34,7 @@ description: |- | availableWatchers | All users that can be added to the work package as watchers. | User | | READ | **Permission** add work package watchers | | budget | The budget this work package is associated to | Budget | | READ / WRITE | **Permission** view cost objects | | category | The category of the work package | Category | | READ / WRITE | | + | categories | List of categories associated to the work package | []Category | | READ / WRITE | | | children | Array of all visible children of the work package | Collection | not null | READ | **Permission** view work packages | | parent | Parent work package | WorkPackage | Needs to be visible (to the current user) | READ / WRITE | | | priority | The priority of the work package | Priority | not null | READ / WRITE | | diff --git a/lib/api/v3/work_packages/eager_loading/checksum.rb b/lib/api/v3/work_packages/eager_loading/checksum.rb index f94bed8601a2..3ddb716c5e64 100644 --- a/lib/api/v3/work_packages/eager_loading/checksum.rb +++ b/lib/api/v3/work_packages/eager_loading/checksum.rb @@ -63,13 +63,28 @@ def fetch_checksums_for(work_packages) # target (as `version` and `targetVersions`) and the observed_in # rows (as `observedInVersions`). The kind is part of the aggregated # value so that moving a version between kinds busts the cache too. + # The parts are separated so that no two distinct tuples can + # concatenate to the same string and collide. VERSIONS_CHECKSUM_SQL = <<~SQL.squish - (SELECT COALESCE(STRING_AGG(CONCAT(wpv.kind, v.id, v.updated_at), ',' ORDER BY wpv.kind, v.id), '') + (SELECT COALESCE(STRING_AGG(CONCAT_WS('-', wpv.kind, v.id, v.updated_at), ',' ORDER BY wpv.kind, v.id), '') FROM work_package_versions wpv INNER JOIN versions v ON v.id = wpv.version_id WHERE wpv.work_package_id = work_packages.id) SQL + # Same reasoning as VERSIONS_CHECKSUM_SQL: the categories are a + # has_many that the left_joins/pluck design cannot express, and the + # representer renders every one of them (as `categories`), not just + # the one the deprecated category association can see. + # The parts are separated so that no two distinct tuples can + # concatenate to the same string and collide. + CATEGORIES_CHECKSUM_SQL = <<~SQL.squish + (SELECT COALESCE(STRING_AGG(CONCAT_WS('-', c.id, c.updated_at), ',' ORDER BY c.id), '') + FROM work_package_categories wpc + INNER JOIN categories c ON c.id = wpc.category_id + WHERE wpc.work_package_id = work_packages.id) + SQL + def md5_concat md5_parts = checksum_associations.flat_map do |association_name| table_name = md5_checksum_table_name(association_name) @@ -77,6 +92,7 @@ def md5_concat %W[#{table_name}.id #{table_name}.updated_at] end md5_parts << VERSIONS_CHECKSUM_SQL + md5_parts << CATEGORIES_CHECKSUM_SQL <<-SQL MD5(CONCAT(#{md5_parts.join(', ')})) diff --git a/lib/api/v3/work_packages/schema/work_package_schema_representer.rb b/lib/api/v3/work_packages/schema/work_package_schema_representer.rb index 504dddec0cf9..6308822e7456 100644 --- a/lib/api/v3/work_packages/schema/work_package_schema_representer.rb +++ b/lib/api/v3/work_packages/schema/work_package_schema_representer.rb @@ -39,7 +39,8 @@ class WorkPackageSchemaRepresenter < ::API::Decorators::SchemaRepresenter dependencies: -> { all_permissions_granted_to_user_under_project + [Setting.work_package_done_ratio, - Setting::WorkPackageMultipleVersions.active?] + Setting::WorkPackageMultipleVersions.active?, + Setting::WorkPackageMultipleCategories.active?] } custom_field_injector type: :schema_representer @@ -300,7 +301,29 @@ def initialize(schema, self_link:, **context) title: category.name } }, - required: false + required: false, + deprecated: true, + description: -> { I18n.t("api_v3.attributes.category.deprecated") } + + # While multiple categories is not enabled, the field keeps the label of the + # single-valued category field it replaces and announces via options.multiple + # that the UI must restrict it to a single value. + schema_with_allowed_collection :categories, + type: "[]Category", + name_source: -> { + attribute = Setting::WorkPackageMultipleCategories.active? ? :categories : :category + WorkPackage.human_attribute_name(attribute) + }, + value_representer: Categories::CategoryRepresenter, + link_factory: ->(category) { + { + href: api_v3_paths.category(category.id), + title: category.name + } + }, + writable: ->(*) { represented.writable?(:categories) }, + required: false, + options: -> { { multiple: Setting::WorkPackageMultipleCategories.active? } } schema_with_allowed_collection :version, value_representer: Versions::VersionRepresenter, diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index fcd237dfe1a0..f9c6e0244c41 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -501,6 +501,31 @@ def self_v3_path(*) associated_resource :category + associated_resources :categories, + v3_path: :category, + representer: ::API::V3::Categories::CategoryRepresenter, + getter: ->(*) { + next unless embed_link?(:categories) + + represented.effective_categories.map do |category| + ::API::V3::Categories::CategoryRepresenter.create(category, current_user:) + end + }, + link: ->(*) { + represented.effective_categories.map do |category| + ::API::Decorators::LinkObject + .new(category, + property_name: :itself, + path: :category, + getter: :id, + title_attribute: :name) + .to_hash + end + }, + setter: ->(fragment:, **) do + represented.category_ids = parse_link_ids_from_fragment(fragment, :category).compact + end + associated_resource :type associated_resource :priority @@ -856,7 +881,8 @@ def ordered_custom_actions attachments budget target_versions - observed_in_versions] + observed_in_versions + categories] # The dynamic class generation introduced because of the custom fields interferes with # the class naming as well as prevents calls to super diff --git a/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb b/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb index f3e3269b150b..447a720f3226 100644 --- a/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/schema/work_package_schema_representer_spec.rb @@ -969,7 +969,7 @@ end end - describe "categories" do + describe "category" do it_behaves_like "has basic schema properties" do let(:path) { "category" } let(:type) { "Category" } @@ -977,6 +977,7 @@ let(:required) { false } let(:writable) { true } let(:location) { "_links" } + let(:description) { I18n.t("api_v3.attributes.category.deprecated") } end it_behaves_like "has a collection of allowed values" do @@ -986,6 +987,57 @@ end end + describe "categories" do + before do + allow(schema).to receive(:writable?).with(:categories).and_return true + end + + it_behaves_like "has basic schema properties" do + let(:path) { "categories" } + let(:type) { "[]Category" } + let(:name) { I18n.t("attributes.category") } + let(:required) { false } + let(:writable) { true } + let(:location) { "_links" } + end + + it "announces the single value restriction while multiple categories is inactive" do + expect(subject).to be_json_eql(false.to_json).at_path("categories/options/multiple") + end + + context "when not writable" do + before do + allow(schema).to receive(:writable?).with(:categories).and_return false + end + + it_behaves_like "has basic schema properties" do + let(:path) { "categories" } + let(:type) { "[]Category" } + let(:name) { I18n.t("attributes.category") } + let(:required) { false } + let(:writable) { false } + let(:location) { "_links" } + end + end + + context "when multiple categories is active", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + it_behaves_like "has basic schema properties" do + let(:path) { "categories" } + let(:type) { "[]Category" } + let(:name) { I18n.t("activerecord.attributes.work_package.categories") } + let(:required) { false } + let(:writable) { true } + let(:location) { "_links" } + end + + it "announces that multiple values are allowed" do + expect(subject).to be_json_eql(true.to_json).at_path("categories/options/multiple") + end + end + end + describe "versions" do context "if having the assign_versions permission" do let(:permissions) { [:assign_versions] } diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index ac1a95fe64bd..8570020f634a 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -879,6 +879,71 @@ end end + describe "categories" do + context "when no category is set" do + it "renders an empty links collection and an empty embedded collection" do + expect(subject).to have_json_size(0).at_path("_links/categories") + expect(subject).to have_json_size(0).at_path("_embedded/categories") + end + end + + context "when a category is set" do + let!(:category) { create(:category, project: workspace) } + + before do + allow(work_package).to receive(:categories).and_return([category]) + end + + it "wraps the category in the links collection" do + expect(subject).to have_json_size(1).at_path("_links/categories") + expect(subject) + .to be_json_eql(api_v3_paths.category(category.id).to_json) + .at_path("_links/categories/0/href") + expect(subject) + .to be_json_eql(category.name.to_json) + .at_path("_links/categories/0/title") + end + + it "wraps the category in the embedded collection" do + expect(subject).to have_json_size(1).at_path("_embedded/categories") + expect(subject) + .to be_json_eql("Category".to_json) + .at_path("_embedded/categories/0/_type") + expect(subject) + .to be_json_eql(category.name.to_json) + .at_path("_embedded/categories/0/name") + end + end + + context "when categories are assigned but not yet persisted" do + let!(:category) { create(:category, project: workspace, name: "Beta") } + let!(:other_category) { create(:category, project: workspace, name: "Alpha") } + + before do + # The pending set is what #effective_categories exposes; the model spec + # covers how it is derived from category_ids_replacements. + allow(work_package).to receive(:effective_categories).and_return([other_category, category]) + end + + it "renders the pending categories" do + expect(subject).to have_json_size(2).at_path("_links/categories") + expect(subject) + .to be_json_eql(api_v3_paths.category(other_category.id).to_json) + .at_path("_links/categories/0/href") + expect(subject) + .to be_json_eql(api_v3_paths.category(category.id).to_json) + .at_path("_links/categories/1/href") + end + + it "embeds the pending categories" do + expect(subject).to have_json_size(2).at_path("_embedded/categories") + expect(subject) + .to be_json_eql(other_category.name.to_json) + .at_path("_embedded/categories/0/name") + end + end + end + describe "priority" do it_behaves_like "has a titled link" do let(:link) { "priority" } From a73ddaceefef05be2f2a9c468eb2f98599845a2c Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:29:23 +0200 Subject: [PATCH 06/14] Read and write categories in the type form configuration Only one of the two category attributes is offered at a time, matching the query columns and the versions handling: categories with the multiple categories feature enabled, the deprecated single category without it. A saved configuration holding either name renders as whichever one the current feature state exposes, and the new gate joins the form attributes cache key. The stored-to-offered substitution now covers both attribute pairs through one map instead of a version-specific pair of locals. Co-Authored-By: Claude Opus 5 --- app/models/type/attribute_groups.rb | 30 +++++++++++----- app/models/type/attributes.rb | 13 ++++++- ...update_form_configuration_contract_spec.rb | 36 +++++++++++++++++++ 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/app/models/type/attribute_groups.rb b/app/models/type/attribute_groups.rb index 6adbc0683d46..40ee6f48a80e 100644 --- a/app/models/type/attribute_groups.rb +++ b/app/models/type/attribute_groups.rb @@ -156,19 +156,33 @@ def custom_attribute_groups groups = self[:attribute_groups].presence return if groups.nil? - # Only one version attribute is offered at a time, so render whichever one a - # saved configuration holds as the one the current feature state exposes. - stored, offered = if Setting::WorkPackageMultipleVersions.active? - %w[version target_versions] - else - %w[target_versions version] - end + # Only one version and one category attribute is offered at a time, so render + # whichever one a saved configuration holds as the one the current feature + # state exposes. + substitutions = feature_gated_attribute_substitutions groups.map do |key, attributes, *rest| - [key, attributes.map { |attribute| attribute == stored ? offered : attribute }.uniq, *rest] + [key, attributes.map { |attribute| substitutions.fetch(attribute, attribute) }.uniq, *rest] end end + # Maps the attribute name a stored configuration may hold onto the one the + # current feature state exposes. + def feature_gated_attribute_substitutions + version_pair = if Setting::WorkPackageMultipleVersions.active? + %w[version target_versions] + else + %w[target_versions version] + end + category_pair = if Setting::WorkPackageMultipleCategories.active? + %w[category categories] + else + %w[categories category] + end + + [version_pair, category_pair].to_h + end + def default_group_key(key) if CustomField.custom_field_attribute?(key) :other diff --git a/app/models/type/attributes.rb b/app/models/type/attributes.rb index 14a41c0f9eab..f63f8c44fe20 100644 --- a/app/models/type/attributes.rb +++ b/app/models/type/attributes.rb @@ -86,6 +86,7 @@ def all_work_package_form_attributes(merge_date: false) *wp_cf_cache_parts, EXCLUDED.length, Setting::WorkPackageMultipleVersions.active?, + Setting::WorkPackageMultipleCategories.active?, merge_date) do calculate_all_work_package_form_attributes(merge_date) end @@ -139,7 +140,10 @@ def skipped_attribute?(key, definition) # We always want to include the priority even if its required return false if key == "priority" - excluded_version_attribute?(key) || EXCLUDED.include?(key) || definition[:required] + excluded_version_attribute?(key) || + excluded_category_attribute?(key) || + EXCLUDED.include?(key) || + definition[:required] end # Only one of the two version attributes is offered at a time, matching the @@ -149,6 +153,13 @@ def excluded_version_attribute?(key) key == (Setting::WorkPackageMultipleVersions.active? ? "version" : "target_versions") end + # As for versions, only one of the two category attributes is offered at a + # time: categories with the multiple categories feature enabled, the + # deprecated single category without it. + def excluded_category_attribute?(key) + key == (Setting::WorkPackageMultipleCategories.active? ? "category" : "categories") + end + def merge_date_for_form_attributes(attributes) attributes["date"] = { required: false, has_default: false } attributes.delete "due_date" diff --git a/spec/contracts/work_package_types/update_form_configuration_contract_spec.rb b/spec/contracts/work_package_types/update_form_configuration_contract_spec.rb index 9d2fdf525f47..5b856e422997 100644 --- a/spec/contracts/work_package_types/update_form_configuration_contract_spec.rb +++ b/spec/contracts/work_package_types/update_form_configuration_contract_spec.rb @@ -273,6 +273,42 @@ module WorkPackageTypes end end + context "when the multiple categories feature is inactive" do + it "accepts the deprecated category" do + model.attribute_groups = [["foo", ["category"]]] + + expect(contract).to be_valid + end + + it "rejects categories as an unknown attribute" do + model.attribute_groups = [["foo", ["categories"]]] + + expect(contract).not_to be_valid + expect(contract.errors.details[:attribute_groups]).to include( + error: "Invalid work package attribute used: categories" + ) + end + end + + context "when the multiple categories feature is active", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + it "accepts categories" do + model.attribute_groups = [["foo", ["categories"]]] + + expect(contract).to be_valid + end + + it "rejects the deprecated category as an unknown attribute" do + model.attribute_groups = [["foo", ["category"]]] + + expect(contract).not_to be_valid + expect(contract.errors.details[:attribute_groups]).to include( + error: "Invalid work package attribute used: category" + ) + end + end + context "with invalid query group" do let(:query) { Query.new(name: "Invalid Query", user:) } let(:invalid_query_group) { ["query_group", [query]] } From 39909bc0cde0956943ffdcdee1453a20900d30d7 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:29:45 +0200 Subject: [PATCH 07/14] Journal work package category changes Snapshots a work package's categories into work_package_category_journals, diffs the whole set as a single value and renders it as one line in the activity. Existing journals are backfilled from work_package_journals' category_id, so no work package journals a spurious "categories set" change on its next save. While the deprecated category_id column mirrors the set, a category change diffs under both keys; only the set representation is exposed, and the journal entry keeps the "Category" label until the multiple categories feature is enabled. JoinedVersions is renamed to JoinedAssociation: nothing in it was version-specific, and the categories formatter needs exactly the same rendering. Co-Authored-By: Claude Opus 5 --- app/models/activities/fetcher.rb | 2 +- app/models/journal.rb | 2 + .../journal/work_package_category_journal.rb | 35 ++++++ app/models/work_package/journalized.rb | 1 + .../journals/create_service/association.rb | 2 +- .../create_service/work_package_category.rb | 88 ++++++++++++++ .../work_packages/activities_tab/paginator.rb | 3 +- ...0_create_work_package_category_journals.rb | 61 ++++++++++ .../activity_eager_loading_wrapper.rb | 2 +- .../journal_formatter/categories.rb | 45 +++++++ .../journal_formatter/joined_association.rb | 10 +- .../journal_formatter/observed_in_versions.rb | 2 +- .../journal_formatter/target_versions.rb | 2 +- .../lib/journal_changes.rb | 31 ++++- .../work_package_category_journal_factory.rb | 33 +++++ spec/factories/work_package_factory.rb | 7 +- .../journal_formatter/categories_spec.rb | 108 ++++++++++++++++ ...ate_work_package_category_journals_spec.rb | 74 +++++++++++ spec/models/journal_spec.rb | 23 ++++ .../work_package_acts_as_journalized_spec.rb | 115 +++++++++++++++++- .../activities_tab/paginator_spec.rb | 6 +- 21 files changed, 627 insertions(+), 25 deletions(-) create mode 100644 app/models/journal/work_package_category_journal.rb create mode 100644 app/services/journals/create_service/work_package_category.rb create mode 100644 db/migrate/20260805090200_create_work_package_category_journals.rb create mode 100644 lib/open_project/journal_formatter/categories.rb create mode 100644 spec/factories/journal/work_package_category_journal_factory.rb create mode 100644 spec/lib/open_project/journal_formatter/categories_spec.rb create mode 100644 spec/migrations/create_work_package_category_journals_spec.rb diff --git a/app/models/activities/fetcher.rb b/app/models/activities/fetcher.rb index a50dfde3dc5f..41ce75d0e8c9 100644 --- a/app/models/activities/fetcher.rb +++ b/app/models/activities/fetcher.rb @@ -125,7 +125,7 @@ def journals_of_event_set(events) Journal .includes(:data, :customizable_journals, :attachable_journals, :work_package_version_journals, - :bcf_comment) + :work_package_category_journals, :bcf_comment) .find(journal_ids) .then { |journals| ::API::V3::Activities::ActivityEagerLoadingWrapper.wrap(journals) } .index_by(&:id) diff --git a/app/models/journal.rb b/app/models/journal.rb index 84b9bae66e67..f6d0bd01cb2e 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -51,6 +51,7 @@ class Journal < ApplicationRecord register_journal_formatter OpenProject::JournalFormatter::AgendaItemTitle register_journal_formatter OpenProject::JournalFormatter::AllocatedTime register_journal_formatter OpenProject::JournalFormatter::Attachment + register_journal_formatter OpenProject::JournalFormatter::Categories register_journal_formatter OpenProject::JournalFormatter::Cause register_journal_formatter OpenProject::JournalFormatter::CustomComment register_journal_formatter OpenProject::JournalFormatter::CustomField @@ -123,6 +124,7 @@ class Journal < ApplicationRecord has_many :project_phase_journals, class_name: "Journal::ProjectPhaseJournal", dependent: :delete_all has_many :storable_journals, class_name: "Journal::StorableJournal", dependent: :delete_all has_many :work_package_version_journals, class_name: "Journal::WorkPackageVersionJournal", dependent: :delete_all + has_many :work_package_category_journals, class_name: "Journal::WorkPackageCategoryJournal", dependent: :delete_all has_many :notifications, dependent: :destroy diff --git a/app/models/journal/work_package_category_journal.rb b/app/models/journal/work_package_category_journal.rb new file mode 100644 index 000000000000..63ec8e0ce14e --- /dev/null +++ b/app/models/journal/work_package_category_journal.rb @@ -0,0 +1,35 @@ +# 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 Journal::WorkPackageCategoryJournal < Journal::AssociatedJournal + self.table_name = "work_package_category_journals" + + belongs_to :category +end diff --git a/app/models/work_package/journalized.rb b/app/models/work_package/journalized.rb index d50d9d1f17fa..56e19920ff9c 100644 --- a/app/models/work_package/journalized.rb +++ b/app/models/work_package/journalized.rb @@ -103,6 +103,7 @@ def self.event_url register_journal_formatted_fields "project_phase_definition_id", formatter_key: :project_phase_definition register_journal_formatted_fields "target_versions", formatter_key: :target_versions register_journal_formatted_fields "observed_in_versions", formatter_key: :observed_in_versions + register_journal_formatted_fields "categories", formatter_key: :categories # Joined register_journal_formatted_fields :parent_id, :project_id, diff --git a/app/services/journals/create_service/association.rb b/app/services/journals/create_service/association.rb index 60c431eabafd..8d605cd67504 100644 --- a/app/services/journals/create_service/association.rb +++ b/app/services/journals/create_service/association.rb @@ -34,7 +34,7 @@ class Association # Core associations are defined here. Module-specific associations can be defined in engines # using `Journals::CreateService::Association.register`. - @registry = Set.new(%i[Attachable CustomComment Customizable ProjectPhase WorkPackageVersion]) + @registry = Set.new(%i[Attachable CustomComment Customizable ProjectPhase WorkPackageVersion WorkPackageCategory]) class << self def register(*names) diff --git a/app/services/journals/create_service/work_package_category.rb b/app/services/journals/create_service/work_package_category.rb new file mode 100644 index 000000000000..7f766e7f2302 --- /dev/null +++ b/app/services/journals/create_service/work_package_category.rb @@ -0,0 +1,88 @@ +# 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 Journals::CreateService + # Journals the category associations of a work package. + class WorkPackageCategory < Association + def associated? + journable.respond_to?(:categories) + end + + def cleanup_predecessor(predecessor, notes, cause) + cleanup_predecessor_for(predecessor, + notes, + cause, + "work_package_category_journals", + :journal_id, + :id) + end + + def insert_sql + sanitize(<<~SQL.squish, journable_id:) + INSERT INTO + work_package_category_journals ( + journal_id, + category_id + ) + SELECT + #{id_from_inserted_journal_sql}, + work_package_categories.category_id + FROM work_package_categories + WHERE + #{only_if_created_sql} + AND work_package_categories.work_package_id = :journable_id + SQL + end + + # Compares the current category id set with the one stored for the most recent + # journal. Each side aggregates into a single row (NULL for an empty set), so a + # plain IS DISTINCT FROM detects additions, removals and replacements alike. + def changes_sql + sanitize(<<~SQL.squish, journable_id:) + SELECT + :journable_id AS journable_id + FROM + ( + SELECT ARRAY_AGG(category_id ORDER BY category_id) AS categories + FROM work_package_categories + WHERE work_package_categories.work_package_id = :journable_id + ) current_categories + CROSS JOIN + ( + SELECT ARRAY_AGG(category_id ORDER BY category_id) AS categories + FROM work_package_category_journals + WHERE journal_id IN (SELECT id FROM max_journals) + ) journal_categories + WHERE + current_categories.categories IS DISTINCT FROM journal_categories.categories + SQL + end + end +end diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb index f003ad2d3bee..b2eb87f097f8 100644 --- a/app/services/work_packages/activities_tab/paginator.rb +++ b/app/services/work_packages/activities_tab/paginator.rb @@ -179,7 +179,8 @@ def with_changesets(scope) def page_journals(page_relation) page_relation .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, - :work_package_version_journals, :notifications, :attachments) + :work_package_version_journals, :work_package_category_journals, + :notifications, :attachments) .to_a end diff --git a/db/migrate/20260805090200_create_work_package_category_journals.rb b/db/migrate/20260805090200_create_work_package_category_journals.rb new file mode 100644 index 000000000000..f93cff76527e --- /dev/null +++ b/db/migrate/20260805090200_create_work_package_category_journals.rb @@ -0,0 +1,61 @@ +# 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 CreateWorkPackageCategoryJournals < ActiveRecord::Migration[8.1] + def change + create_table :work_package_category_journals do |t| # rubocop:disable Rails/CreateTableWithTimestamps + t.belongs_to :journal, null: false, foreign_key: true + # No foreign key: journal snapshots must outlive deleted categories, + # like the other *_journals side tables. + t.belongs_to :category, null: false + end + + # Existing journals predate the snapshot table. Without snapshot rows for the + # most recent journal, the next save of any work package with a category would + # journal a spurious "categories set" change. During the transition the + # categories mirror the single category_id, so every historical journal's + # category set can be reconstructed exactly from its data row. On rollback the + # rows go away with the table. + reversible do |dir| + dir.up do + say_with_time "Copying work_package_journals.category_id into work_package_category_journals" do + execute(<<~SQL.squish) + INSERT INTO work_package_category_journals (journal_id, category_id) + SELECT journals.id, work_package_journals.category_id + FROM journals + INNER JOIN work_package_journals ON work_package_journals.id = journals.data_id + WHERE journals.data_type = 'Journal::WorkPackageJournal' + AND work_package_journals.category_id IS NOT NULL + SQL + end + end + end + end +end diff --git a/lib/api/v3/activities/activity_eager_loading_wrapper.rb b/lib/api/v3/activities/activity_eager_loading_wrapper.rb index f3f96777d312..daa1dc13f89d 100644 --- a/lib/api/v3/activities/activity_eager_loading_wrapper.rb +++ b/lib/api/v3/activities/activity_eager_loading_wrapper.rb @@ -151,7 +151,7 @@ def predecessor_journals(journals) SQL ) .includes(:attachable_journals, :customizable_journals, :storable_journals, - :work_package_version_journals) + :work_package_version_journals, :work_package_category_journals) end end end diff --git a/lib/open_project/journal_formatter/categories.rb b/lib/open_project/journal_formatter/categories.rb new file mode 100644 index 000000000000..0e54059fcfc1 --- /dev/null +++ b/lib/open_project/journal_formatter/categories.rb @@ -0,0 +1,45 @@ +# 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. +#++ + +# Renders the change to the set of categories +# (see JournalChanges#get_categories_changes). +class OpenProject::JournalFormatter::Categories < OpenProject::JournalFormatter::JoinedAssociation + private + + # While the multiple categories feature is inactive, the rest of the UI still + # labels the attribute "Category"; the journal entry follows suit. + def label(key) + if Setting::WorkPackageMultipleCategories.active? + super + else + super("category") + end + end +end diff --git a/lib/open_project/journal_formatter/joined_association.rb b/lib/open_project/journal_formatter/joined_association.rb index e9a8e681019f..9e1160ec3f52 100644 --- a/lib/open_project/journal_formatter/joined_association.rb +++ b/lib/open_project/journal_formatter/joined_association.rb @@ -28,11 +28,11 @@ # See COPYRIGHT and LICENSE files for more details. #++ -# Renders the change to a set of versions referenced by a work package. Each -# value is the sorted, comma-joined version ids (see JournalChanges); every id -# is resolved to the version's name, dropping versions that have been deleted in -# the meantime. -class OpenProject::JournalFormatter::JoinedVersions < JournalFormatter::NamedAssociation +# Renders the change to a whole set of records referenced by a journable through +# a has_many. Each value is the sorted, comma-joined record ids (see +# JournalChanges); every id is resolved to the record's name, dropping records +# that have been deleted in the meantime. +class OpenProject::JournalFormatter::JoinedAssociation < JournalFormatter::NamedAssociation private def format_values(values, key, cache:) diff --git a/lib/open_project/journal_formatter/observed_in_versions.rb b/lib/open_project/journal_formatter/observed_in_versions.rb index f99881cf4ae8..ebfd3d71065c 100644 --- a/lib/open_project/journal_formatter/observed_in_versions.rb +++ b/lib/open_project/journal_formatter/observed_in_versions.rb @@ -30,5 +30,5 @@ # Renders the change to the set of versions a work package was observed in # (see JournalChanges#get_observed_in_versions_changes). -class OpenProject::JournalFormatter::ObservedInVersions < OpenProject::JournalFormatter::JoinedVersions +class OpenProject::JournalFormatter::ObservedInVersions < OpenProject::JournalFormatter::JoinedAssociation end diff --git a/lib/open_project/journal_formatter/target_versions.rb b/lib/open_project/journal_formatter/target_versions.rb index 38a552689e6a..59b01d6a35ed 100644 --- a/lib/open_project/journal_formatter/target_versions.rb +++ b/lib/open_project/journal_formatter/target_versions.rb @@ -30,7 +30,7 @@ # Renders the change to the set of target versions # (see JournalChanges#get_target_versions_changes). -class OpenProject::JournalFormatter::TargetVersions < OpenProject::JournalFormatter::JoinedVersions +class OpenProject::JournalFormatter::TargetVersions < OpenProject::JournalFormatter::JoinedAssociation private # While the multiple versions feature is inactive, the rest of the UI still diff --git a/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb b/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb index 4e4bf75925f2..bb828eccad59 100644 --- a/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb +++ b/lib_static/plugins/acts_as_journalized/lib/journal_changes.rb @@ -35,7 +35,7 @@ def get_changes merged = all_changes.reduce({}.with_indifferent_access, :merge!) - @changes = suppress_mirrored_version_change(merged) + @changes = suppress_mirrored_association_changes(merged) end def get_cause_changes @@ -142,6 +142,19 @@ def get_observed_in_versions_changes { observed_in_versions: [old_value, new_value] } end + # The whole set of categories is diffed as a single value (the sorted, + # comma-joined category ids), matching how the change is rendered: one + # "Categories" line with the old and the new list. + def get_categories_changes + return unless journable.respond_to?(:categories) + + old_value = predecessor && joined_category_ids(predecessor) + new_value = joined_category_ids(self) + return if old_value == new_value + + { categories: [old_value, new_value] } + end + def get_file_links_changes return unless has_file_links? @@ -196,18 +209,20 @@ def all_changes get_project_phases_changes, get_target_versions_changes, get_observed_in_versions_changes, + get_categories_changes, get_file_links_changes, get_participants_changes, get_agenda_items_changes ].compact end - # While the deprecated version_id column mirrors the target versions, a - # version change diffs under both keys; only the target_versions - # representation is rendered. Historical journals render the same way, - # since every versioned journal has a backfilled target snapshot. - def suppress_mirrored_version_change(changes) + # While the deprecated version_id and category_id columns mirror the target + # versions and the categories, such a change diffs under both keys; only the + # set representation is rendered. Historical journals render the same way, + # since every affected journal has a backfilled snapshot. + def suppress_mirrored_association_changes(changes) changes.delete("version_id") if changes.key?("target_versions") + changes.delete("category_id") if changes.key?("categories") changes end @@ -220,6 +235,10 @@ def joined_observed_in_version_ids(journal) journal.observed_in_version_journals.map(&:version_id).sort.join(",").presence end + def joined_category_ids(journal) + journal.work_package_category_journals.map(&:category_id).sort.join(",").presence + end + def participant_baseline_journal journals = journable.journals.to_a current_index = journals.index { |entry| entry.id == id } diff --git a/spec/factories/journal/work_package_category_journal_factory.rb b/spec/factories/journal/work_package_category_journal_factory.rb new file mode 100644 index 000000000000..b199d9e46538 --- /dev/null +++ b/spec/factories/journal/work_package_category_journal_factory.rb @@ -0,0 +1,33 @@ +# 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. +#++ + +FactoryBot.define do + factory :journal_work_package_category_journal, class: "Journal::WorkPackageCategoryJournal" +end diff --git a/spec/factories/work_package_factory.rb b/spec/factories/work_package_factory.rb index a26b520c87cb..49e975aeb1ba 100644 --- a/spec/factories/work_package_factory.rb +++ b/spec/factories/work_package_factory.rb @@ -138,12 +138,17 @@ work_package_cv_attributes = work_package.custom_values.map { it.attributes.slice("custom_field_id", "value") } version_attributes = work_package.work_package_versions.where(kind: "target") .map { it.attributes.slice("version_id", "kind") } + category_attributes = work_package.work_package_categories + .map { it.attributes.slice("category_id") } create(:work_package_journal, **journal_attributes, data: build(:journal_work_package_journal, data_attributes), customizable_journals: work_package_cv_attributes.map { build(:journal_customizable_journal, it) }, - work_package_version_journals: version_attributes.map { build(:journal_work_package_version_journal, it) }) + work_package_version_journals: version_attributes.map { build(:journal_work_package_version_journal, it) }, + work_package_category_journals: category_attributes.map do |attributes| + build(:journal_work_package_category_journal, attributes) + end) end work_package.journals.reload diff --git a/spec/lib/open_project/journal_formatter/categories_spec.rb b/spec/lib/open_project/journal_formatter/categories_spec.rb new file mode 100644 index 000000000000..78e61111d21f --- /dev/null +++ b/spec/lib/open_project/journal_formatter/categories_spec.rb @@ -0,0 +1,108 @@ +# 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 "spec_helper" + +RSpec.describe OpenProject::JournalFormatter::Categories do + describe "#render" do + let(:category) { build_stubbed(:category, name: "Alpha") } + let(:other_category) { build_stubbed(:category, name: "Beta") } + let(:work_package) { build_stubbed(:work_package) } + let(:journal) { build_stubbed(:work_package_journal, journable: work_package) } + let(:instance) { described_class.new(journal) } + # While the multiple categories feature is inactive, the single-category + # "Category" label used across the UI is kept. + let(:label) { "Category" } + + before do + allow(Category).to receive(:find_by).and_return(nil) + + [category, other_category].each do |c| + allow(Category).to receive(:find_by).with(id: c.id).and_return(c) + end + end + + context "when setting categories" do + it "renders the category names as the new value" do + expect(instance.render(:categories, [nil, "#{category.id},#{other_category.id}"])) + .to eq(I18n.t(:text_journal_set_to, label:, value: "Alpha, Beta")) + end + end + + context "when changing categories" do + it "renders the old and new category names" do + expect(instance.render(:categories, [category.id.to_s, other_category.id.to_s])) + .to eq(I18n.t(:text_journal_changed_plain, + label:, + linebreak: nil, + old: "Alpha", + new: "Beta")) + end + end + + context "when removing all categories" do + it "renders the old category names as deleted" do + expect(instance.render(:categories, ["#{category.id},#{other_category.id}", nil])) + .to eq(I18n.t(:text_journal_deleted, label:, old: "Alpha, Beta")) + end + end + + context "with a category that no longer exists" do + it "renders only the existing category names" do + expect(instance.render(:categories, [nil, "#{category.id},99999"])) + .to eq(I18n.t(:text_journal_set_to, label:, value: "Alpha")) + end + end + + context "when the multiple categories feature is active", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + it "labels the change with 'Categories'" do + expect(instance.render(:categories, [category.id.to_s, other_category.id.to_s])) + .to eq(I18n.t(:text_journal_changed_plain, + label: "Categories", + linebreak: nil, + old: "Alpha", + new: "Beta")) + end + end + + context "with html: false" do + it "renders plain text" do + expect(instance.render(:categories, [category.id.to_s, other_category.id.to_s], html: false)) + .to eq(I18n.t(:text_journal_changed_plain, + label: "Category", + linebreak: nil, + old: "Alpha", + new: "Beta")) + end + end + end +end diff --git a/spec/migrations/create_work_package_category_journals_spec.rb b/spec/migrations/create_work_package_category_journals_spec.rb new file mode 100644 index 000000000000..48d8818e3f99 --- /dev/null +++ b/spec/migrations/create_work_package_category_journals_spec.rb @@ -0,0 +1,74 @@ +# 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 "spec_helper" +require Rails.root.join("db/migrate/20260805090200_create_work_package_category_journals.rb") + +RSpec.describe CreateWorkPackageCategoryJournals, type: :model, + with_settings: { journal_aggregation_time_minutes: 0 } do + subject(:migrate) { ActiveRecord::Migration.suppress_messages { described_class.new.migrate(:up) } } + + let(:project) { create(:project) } + let(:category) { create(:category, project:) } + let(:other_category) { create(:category, project:) } + + let!(:work_package_with_category) { create(:work_package, project:, category:) } + let!(:work_package_without_category) { create(:work_package, project:, category: nil) } + + before do + # A second journal with a different category, so the backfill has to + # reconstruct a distinct set per journal, not per work package. + work_package_with_category.update!(category: other_category) + + # Journals exist but the snapshot table does not yet. + ActiveRecord::Base.connection.drop_table(:work_package_category_journals) + end + + it "succeeds" do + expect { migrate }.not_to raise_error + end + + it "creates a snapshot row per journal, matching that journal's category_id" do + migrate + + first_journal, second_journal = work_package_with_category.journals.order(:version) + + expect(Journal::WorkPackageCategoryJournal.pluck(:journal_id, :category_id)) + .to contain_exactly([first_journal.id, category.id], + [second_journal.id, other_category.id]) + end + + it "creates no rows for journals without a category" do + migrate + + expect(Journal::WorkPackageCategoryJournal.where(journal: work_package_without_category.journals)) + .to be_empty + end +end diff --git a/spec/models/journal_spec.rb b/spec/models/journal_spec.rb index cd633abdd241..4694e8be7005 100644 --- a/spec/models/journal_spec.rb +++ b/spec/models/journal_spec.rb @@ -182,4 +182,27 @@ end end end + + describe "formatter registration" do + # register_journal_formatted_fields only maps a field to a formatter key; the + # class behind that key has to be registered here as well. Miss it and + # rendering the activity raises a NoMethodError on nil deep inside + # JournalFormatter#formatter_instances, which no per-formatter unit spec can + # catch because those instantiate their class directly. + it "has a formatter class for every registered formatted field" do + Rails.application.eager_load! + + registrations = JournalFormatter.registered_fields.flat_map do |journal_data_type, fields| + fields.map { |field, formatter_key| [journal_data_type, field, formatter_key] } + end + + expect(registrations).not_to be_empty + + unregistered = registrations.reject { |_, _, formatter_key| JournalFormatter.formatters[formatter_key] } + + expect(unregistered).to be_empty, + "no formatter class is registered for: " \ + "#{unregistered.map { |type, field, key| "#{type}##{field.source} (#{key})" }.join(', ')}" + end + end end diff --git a/spec/models/work_package/work_package_acts_as_journalized_spec.rb b/spec/models/work_package/work_package_acts_as_journalized_spec.rb index 8abe4c28af0d..79f070c61b7b 100644 --- a/spec/models/work_package/work_package_acts_as_journalized_spec.rb +++ b/spec/models/work_package/work_package_acts_as_journalized_spec.rb @@ -44,6 +44,7 @@ shared_let(:other_user) { create(:user) } shared_let(:other_project) { create(:project) } shared_let(:category) { create(:category) } + shared_let(:other_category) { create(:category) } shared_let(:version) { create(:version) } shared_let(:other_version) { create(:version) } shared_let(:project_phase_definition) { create(:project_phase_definition) } @@ -89,7 +90,7 @@ "status_id" => [nil, :status], "priority_id" => [nil, :priority], "project_id" => [nil, :project], - "category_id" => [nil, :category], + "categories" => [nil, -> { category.id.to_s }], "target_versions" => [nil, -> { version.id.to_s }], "start_date" => [nil, Date.new(2013, 1, 24)], "due_date" => [nil, Date.new(2013, 1, 31)], @@ -202,7 +203,7 @@ "status_id" => %i[status other_status], "priority_id" => %i[priority other_priority], "project_id" => %i[project other_project], - "category_id" => [nil, :category], + "categories" => [nil, -> { category.id.to_s }], "target_versions" => [-> { version.id.to_s }, -> { other_version.id.to_s }], "start_date" => [Date.new(2026, 1, 9), Date.new(2013, 1, 24)], "due_date" => [nil, Date.new(2013, 1, 31)], @@ -278,7 +279,7 @@ "status_id" => [nil, :other_status], "priority_id" => [nil, :other_priority], "project_id" => [nil, :other_project], - "category_id" => [nil, :category], + "categories" => [nil, -> { category.id.to_s }], "target_versions" => [nil, -> { other_version.id.to_s }], "start_date" => [nil, Date.new(2013, 1, 24)], "due_date" => [nil, Date.new(2013, 1, 31)], @@ -357,7 +358,7 @@ "status_id" => %i[status other_status], "priority_id" => %i[priority other_priority], "project_id" => %i[project other_project], - "category_id" => [nil, :category], + "categories" => [nil, -> { category.id.to_s }], "target_versions" => [-> { version.id.to_s }, -> { other_version.id.to_s }], "start_date" => [Date.new(2026, 1, 9), Date.new(2013, 1, 24)], "due_date" => [nil, Date.new(2013, 1, 31)], @@ -616,6 +617,112 @@ def set_target_versions(versions) end end + # These examples also guard the callback order between the + # WorkPackage::Categories and WorkPackage::Journalized concerns: the journal + # snapshots table state during the save, so it only captures the category + # associations if they are persisted by the earlier after_save callback. + context "on category changes", with_settings: { journal_aggregation_time_minutes: 0 } do + shared_let(:journable) do + create(:work_package) + end + + def set_categories(categories) + journable.category_ids_replacements = categories.map(&:id) + journable.save! + end + + context "when setting categories" do + it "creates a new journal listing the categories in the details" do + expect { set_categories([category, other_category]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["categories"]) + .to eq([nil, [category.id, other_category.id].sort.join(",")]) + end + + it "touches the journable to match the journal's timestamp" do + expect { set_categories([category]) } + .to change { journable.reload.updated_at } + + expect(journable.updated_at).to eq(journable.last_journal.updated_at) + end + end + + context "when replacing a category" do + before do + set_categories([category]) + end + + it "creates a new journal with the old and new categories in the details" do + expect { set_categories([other_category]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["categories"]) + .to eq([category.id.to_s, other_category.id.to_s]) + end + end + + context "when removing all categories" do + before do + set_categories([category]) + end + + it "creates a new journal with an empty new value in the details" do + expect { set_categories([]) } + .to change { journable.journals.count }.by(1) + + expect(journable.last_journal.details["categories"]) + .to eq([category.id.to_s, nil]) + end + end + + context "when saving with unchanged categories" do + before do + set_categories([category]) + end + + it "creates no journal and does not touch the journable" do + expect { set_categories([category]) } + .to not_change { journable.journals.count } + .and(not_change { journable.reload.updated_at }) + end + end + + context "on work package creation" do + it "includes the categories in the initial journal's details" do + journable = build(:work_package) + journable.category_ids_replacements = [category.id] + journable.save! + + expect(journable.last_journal.details["categories"]) + .to eq([nil, category.id.to_s]) + end + end + + # While the deprecated category_id column mirrors the categories, every + # category change produces both a category_id and a categories diff. Only + # the categories representation is exposed. + context "when changing the category via the legacy category field" do + it "journals the change as categories only" do + journable.update!(category:) + + expect(journable.last_journal.details["categories"]) + .to eq([nil, category.id.to_s]) + expect(journable.last_journal.details) + .not_to have_key("category_id") + end + end + + context "when setting categories via the replacements" do + it "does not additionally journal the mirrored category_id" do + set_categories([category]) + + expect(journable.last_journal.details) + .not_to have_key("category_id") + end + end + end + context "on custom value changes" do # The explicit id is needed so that the accessors ('custom_field_1') can be used shared_let(:custom_field) do diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb index 771a59349067..ae50e443da5b 100644 --- a/spec/services/work_packages/activities_tab/paginator_spec.rb +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -270,9 +270,9 @@ end # The wrapper runs against the page slice, so query count does not scale - # with history size. Ceiling leaves headroom for an extra eager-load - # without becoming brittle; actual count today is ~10. - expect(recorder.count).to be < 20, + # with history size. Ceiling leaves headroom for a few more eager-loads + # without becoming brittle; actual count today is ~21. + expect(recorder.count).to be < 30, "expected query count bounded regardless of history; got #{recorder.count}:\n" \ "#{recorder.log.join("\n")}" end From ce02cddfe4bd2683ca906381c14bcc2b5e7886ce Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:29:52 +0200 Subject: [PATCH 08/14] Add a categories work package query column and filter Introduces the categories column, offered instead of the deprecated single category once the multiple categories feature is enabled. Both sort and group through the join table, so the category set is the group key and a work package holding several categories forms a group of its own. The category filter matches against the join table as well, so it finds work packages by any of their categories. Unlike versions there is no second filter to add: categories form a single set, and the API name derived from the existing key is the one the new attribute would want anyway. Only the label follows the feature. Co-Authored-By: Claude Opus 5 --- .../work_packages/filter/category_filter.rb | 39 ++++ .../work_packages/selects/property_select.rb | 45 +++- .../table/group_by/group_headers_spec.rb | 29 +++ .../selects/property_select_spec.rb | 59 +++++ .../results_categories_integration_spec.rb | 214 ++++++++++++++++++ spec/models/query/sort_criteria_spec.rb | 6 +- 6 files changed, 387 insertions(+), 5 deletions(-) create mode 100644 spec/models/query/results_categories_integration_spec.rb diff --git a/app/models/queries/work_packages/filter/category_filter.rb b/app/models/queries/work_packages/filter/category_filter.rb index 1cad9ccbd606..8f0c6e113418 100644 --- a/app/models/queries/work_packages/filter/category_filter.rb +++ b/app/models/queries/work_packages/filter/category_filter.rb @@ -28,6 +28,13 @@ # See COPYRIGHT and LICENSE files for more details. #++ +# Filters on the categories referenced through work_package_categories, which are +# replacing the legacy `work_packages.category_id` column. +# +# Unlike versions, categories form a single set, so there is no second filter to +# introduce alongside this one: the API name derived from the key below +# ("category") is the one the new attribute would want anyway. Only the label +# follows the multiple-categories feature. class Queries::WorkPackages::Filter::CategoryFilter < Queries::WorkPackages::Filter::WorkPackageFilter def allowed_values @@ -46,6 +53,12 @@ def self.key :category_id end + def human_name + attribute = Setting::WorkPackageMultipleCategories.active? ? "categories" : "category" + + WorkPackage.human_attribute_name(attribute) + end + def value_objects available_categories = all_project_categories.index_by(&:id) @@ -57,8 +70,34 @@ def ar_object_filter? true end + def where + case operator + when "!" # is not + "NOT (#{categories_matching_values})" + when "!*" # empty + "NOT (#{any_category_associated})" + when "*" # not empty + any_category_associated + else # "=" is (or) + categories_matching_values + end + end + private + def any_category_associated + "EXISTS (#{category_associations.select(1).to_sql})" + end + + def categories_matching_values + "EXISTS (#{category_associations.where(category_id: values).select(1).to_sql})" + end + + def category_associations + WorkPackageCategory + .where("#{WorkPackageCategory.table_name}.work_package_id = #{WorkPackage.table_name}.id") + end + def all_project_categories @all_project_categories ||= project.categories end diff --git a/app/models/queries/work_packages/selects/property_select.rb b/app/models/queries/work_packages/selects/property_select.rb index bd7f3ab1e625..00157b81793e 100644 --- a/app/models/queries/work_packages/selects/property_select.rb +++ b/app/models/queries/work_packages/selects/property_select.rb @@ -96,9 +96,48 @@ def caption default_order: "desc" }, category: { - association: "category", - sortable: "name", - groupable: "#{WorkPackage.table_name}.category_id" + if: -> { !Setting::WorkPackageMultipleCategories.active? }, + group_by_class_name: "Category", + sortable: <<~SQL.squish, + (SELECT LOWER(c.name) + FROM work_package_categories wpc + INNER JOIN categories c ON c.id = wpc.category_id + WHERE wpc.work_package_id = work_packages.id + ORDER BY LOWER(c.name), wpc.category_id + LIMIT 1) + SQL + groupable: <<~SQL.squish + (SELECT wpc.category_id + FROM work_package_categories wpc + INNER JOIN categories c ON c.id = wpc.category_id + WHERE wpc.work_package_id = work_packages.id + ORDER BY LOWER(c.name), wpc.category_id + LIMIT 1) + SQL + }, + categories: { + if: -> { Setting::WorkPackageMultipleCategories.active? }, + sortable: [ + <<~SQL.squish, + (SELECT STRING_AGG(LOWER(c.name), ' ' ORDER BY LOWER(c.name), wpc.category_id) + FROM work_package_categories wpc + INNER JOIN categories c ON c.id = wpc.category_id + WHERE wpc.work_package_id = work_packages.id) + SQL + <<~SQL.squish + (SELECT STRING_AGG(wpc.category_id::text, '.' ORDER BY LOWER(c.name), wpc.category_id) + FROM work_package_categories wpc + INNER JOIN categories c ON c.id = wpc.category_id + WHERE wpc.work_package_id = work_packages.id) + SQL + ], + groupable: + <<~SQL.squish + (SELECT STRING_AGG(wpc.category_id::text, '.' ORDER BY LOWER(c.name), wpc.category_id) + FROM work_package_categories wpc + INNER JOIN categories c ON c.id = wpc.category_id + WHERE wpc.work_package_id = work_packages.id) + SQL }, version: { if: -> { !Setting::WorkPackageMultipleVersions.active? }, diff --git a/spec/features/work_packages/table/group_by/group_headers_spec.rb b/spec/features/work_packages/table/group_by/group_headers_spec.rb index 9bab15b9cb1a..d5836be5005a 100644 --- a/spec/features/work_packages/table/group_by/group_headers_spec.rb +++ b/spec/features/work_packages/table/group_by/group_headers_spec.rb @@ -82,4 +82,33 @@ group_by.expect_grouped_by_value "-", 1 end end + + context "with multiple categories active", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + # The single category column no longer exists once the feature is on, so the + # query would not even validate with the shared column names. + let!(:query) do + query = build(:query, user:, project:) + query.column_names = ["subject", "categories"] + query.show_hierarchies = false + + query.save! + query + end + + it "shows one group header per category set" do + wp_cat2.category_ids_replacements = [category.id, category2.id] + wp_cat2.save! + + group_by.enable_via_menu "Categories" + + # The category set is the group key: {Foo} and {Bar, Foo} are separate + # groups, work packages without categories are grouped under "-" + group_by.expect_number_of_groups 3 + group_by.expect_grouped_by_value "Foo", 1 + group_by.expect_grouped_by_value "Bar, Foo", 1 + group_by.expect_grouped_by_value "-", 1 + end + end end diff --git a/spec/models/queries/work_packages/selects/property_select_spec.rb b/spec/models/queries/work_packages/selects/property_select_spec.rb index 78a2bf222e62..ec10253db19f 100644 --- a/spec/models/queries/work_packages/selects/property_select_spec.rb +++ b/spec/models/queries/work_packages/selects/property_select_spec.rb @@ -116,5 +116,64 @@ end end end + + describe "category and categories columns" do + context "with the feature flag and the setting enabled", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + it "replaces the category column with the categories column" do + names = described_class.instances.map(&:name) + + expect(names).to include :categories + expect(names).not_to include :category + end + + it "is displayable, sortable and groupable" do + column = described_class.instances.find { it.name == :categories } + + expect(column).to be_displayable + expect(column).to be_sortable + expect(column).to be_groupable + expect(column.caption).to eq WorkPackage.human_attribute_name(:categories) + end + + it "sorts and groups via the work_package_categories join rows" do + column = described_class.instances.find { it.name == :categories } + + expect(Array(column.sortable)).to all include("work_package_categories") + expect(column.groupable).to include("work_package_categories") + end + end + + context "with the feature flag disabled", + with_flag: { work_package_multiple_categories: false }, + with_settings: { work_package_multiple_categories: true } do + it "keeps the category column" do + names = described_class.instances.map(&:name) + + expect(names).to include :category + expect(names).not_to include :categories + end + + it "sorts and groups the category column via the work_package_categories join rows" do + column = described_class.instances.find { it.name == :category } + + expect(column.sortable).to include("work_package_categories") + expect(column.groupable).to include("work_package_categories") + expect(column.groupable).not_to include("#{WorkPackage.table_name}.category_id") + end + end + + context "with the setting disabled", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: false } do + it "keeps the category column" do + names = described_class.instances.map(&:name) + + expect(names).to include :category + expect(names).not_to include :categories + end + end + end end end diff --git a/spec/models/query/results_categories_integration_spec.rb b/spec/models/query/results_categories_integration_spec.rb new file mode 100644 index 000000000000..bf5f0fd6c0de --- /dev/null +++ b/spec/models/query/results_categories_integration_spec.rb @@ -0,0 +1,214 @@ +# 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 "spec_helper" + +RSpec.describe Query::Results, "Grouping and sorting for categories", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + let(:query_results) do + described_class.new query + end + let(:project) { create(:project) } + let(:user) do + create(:user, + firstname: "user", + lastname: "1", + member_with_permissions: { project => [:view_work_packages] }) + end + + let(:alpha_category) do + create(:category, name: "1. Alpha", project:) + end + + let(:beta_category) do + create(:category, name: "2. Beta", project:) + end + + let!(:alpha_wp) do + create_wp_with_categories("Alpha wp", [alpha_category]) + end + let!(:both_categories_wp) do + create_wp_with_categories("Both categories wp", [beta_category, alpha_category]) + end + let!(:beta_wp) do + create_wp_with_categories("Beta wp", [beta_category]) + end + let!(:no_category_wp) do + create(:work_package, subject: "No category wp", project:) + end + + let(:group_by) { nil } + let(:sort_criteria) { [["categories", "asc"]] } + + let(:query) do + build(:query, + user:, + group_by:, + show_hierarchies: false, + project:).tap do |q| + q.filters.clear + q.sort_criteria = sort_criteria + end + end + + # Sorted by the aggregated category names ("1. alpha" < "1. alpha 2. beta" < "2. beta" < NULL) + let(:work_packages_asc) { [alpha_wp, both_categories_wp, beta_wp, no_category_wp] } + let(:work_packages_desc) { work_packages_asc.reverse } + + def create_wp_with_categories(subject, categories, **attributes) + create(:work_package, subject:, project:, **attributes).tap do |wp| + wp.category_ids_replacements = categories.map(&:id) + wp.save! + end + end + + before do + login_as(user) + end + + describe "sorting ASC by categories" do + let(:sort_criteria) { [["categories", "asc"]] } + + it "sorts by the aggregated category names with absent categories last" do + expect(query_results.work_packages.pluck(:id)) + .to eq work_packages_asc.map(&:id) + end + end + + describe "sorting DESC by categories" do + let(:sort_criteria) { [["categories", "desc"]] } + + it "sorts by the aggregated category names with absent categories first" do + expect(query_results.work_packages.pluck(:id)) + .to eq work_packages_desc.map(&:id) + end + end + + describe "grouping by categories" do + let(:group_by) { "categories" } + + it "groups by the set of assigned categories" do + # The set of categories is the group key, so {alpha, beta} is a group of + # its own, distinct from {alpha} and {beta}. Work packages without any + # category are grouped under the empty set. + expect(query_results.work_package_count_by_group) + .to eql([alpha_category] => 1, + [alpha_category, beta_category] => 1, + [beta_category] => 1, + [] => 1) + + # Group keys are sorted like the work packages themselves + expect(query_results.work_package_count_by_group.keys) + .to eql [[alpha_category], [alpha_category, beta_category], [beta_category], []] + + # Groups are contiguous in the row order + expect(query_results.work_packages.pluck(:id)) + .to eq work_packages_asc.map(&:id) + end + + context "with sums displayed" do + let(:query) do + build(:query, + user:, + group_by:, + show_hierarchies: false, + project:).tap do |q| + q.filters.clear + q.sort_criteria = sort_criteria + q.display_sums = true + end + end + + let!(:alpha_wp) do + create_wp_with_categories("Alpha wp", [alpha_category], estimated_hours: 2) + end + let!(:both_categories_wp) do + create_wp_with_categories("Both categories wp", [beta_category, alpha_category], estimated_hours: 3) + end + + it "sums per category set" do + sums = query_results.all_group_sums.transform_values do |by_column| + by_column.transform_keys(&:name)[:estimated_hours] + end + + expect(sums) + .to eq([alpha_category] => 2.0, + [alpha_category, beta_category] => 3.0, + [beta_category] => nil, + [] => nil) + end + end + end + + context "with equally named categories in different projects" do + let(:other_project) { create(:project) } + let(:user) do + create(:user, + firstname: "user", + lastname: "1", + member_with_permissions: { + project => [:view_work_packages], + other_project => [:view_work_packages] + }) + end + + let(:same_name_category) do + create(:category, name: alpha_category.name, project: other_project) + end + + let!(:same_name_wp) do + create(:work_package, subject: "Same name other project wp", project: other_project).tap do |wp| + wp.category_ids_replacements = [same_name_category.id] + wp.save! + end + end + + let(:query) do + build(:query, + user:, + group_by: "categories", + show_hierarchies: false, + project: nil).tap do |q| + q.filters.clear + q.sort_criteria = sort_criteria + end + end + + it "keeps the equally named category sets in separate groups" do + expect(query_results.work_package_count_by_group) + .to eql([alpha_category] => 1, + [same_name_category] => 1, + [alpha_category, beta_category] => 1, + [beta_category] => 1, + [] => 1) + end + end +end diff --git a/spec/models/query/sort_criteria_spec.rb b/spec/models/query/sort_criteria_spec.rb index 48fb12d84b12..970ef4ac81f7 100644 --- a/spec/models/query/sort_criteria_spec.rb +++ b/spec/models/query/sort_criteria_spec.rb @@ -112,11 +112,13 @@ end context "with multiple sort_criteria with order handling and misc order statement" do - let(:sort_criteria) { [%w[category desc], %w[start_date asc]] } + # `type` is sorted through its association, so its sortable is a bare column + # name rather than a work_packages one. + let(:sort_criteria) { [%w[type desc], %w[start_date asc]] } it "adds the order handling (and the default order by id)" do expect(subject) - .to eq [["name DESC"], + .to eq [["position DESC"], ["work_packages.start_date"], ["work_packages.id DESC"]] end From 766261022604007e7fb6b4ef7634a965b93e85fe Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:29:58 +0200 Subject: [PATCH 09/14] Export categories Formats the categories column as the joined category names in CSV, XLS and PDF exports, and the deprecated category column from the same data. The workPackageValue macro maps the deprecated attribute to the set it is replaced by, alongside the equivalent version mapping. Co-Authored-By: Claude Opus 5 --- .../exports/formatters/categories.rb | 45 +++++++++++ .../work_package/exports/macros/attributes.rb | 21 ++--- config/initializers/export_formats.rb | 1 + .../xls_export/work_package/exporter/xls.rb | 3 +- .../exporter/csv_integration_spec.rb | 48 +++++++++++ .../exports/formatters/categories_spec.rb | 80 +++++++++++++++++++ 6 files changed, 187 insertions(+), 11 deletions(-) create mode 100644 app/models/work_package/exports/formatters/categories.rb create mode 100644 spec/models/work_package/exports/formatters/categories_spec.rb diff --git a/app/models/work_package/exports/formatters/categories.rb b/app/models/work_package/exports/formatters/categories.rb new file mode 100644 index 000000000000..554af6014616 --- /dev/null +++ b/app/models/work_package/exports/formatters/categories.rb @@ -0,0 +1,45 @@ +# 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 WorkPackage::Exports + module Formatters + class Categories < ::Exports::Formatters::Default + # Also covers the deprecated category column, which exports the same + # categories data while the single-category UI is active. + def self.apply?(attribute, _export_format) + %i[categories category].include?(attribute.to_sym) + end + + def retrieve_value(object) + object.categories.map(&:name) + end + end + end +end diff --git a/app/models/work_package/exports/macros/attributes.rb b/app/models/work_package/exports/macros/attributes.rb index 2b4524646794..4a750341b2b0 100644 --- a/app/models/work_package/exports/macros/attributes.rb +++ b/app/models/work_package/exports/macros/attributes.rb @@ -43,6 +43,8 @@ module Macros # workPackageValue:1234:targetVersions:singleline # Outputs the values of #1234 comma-separated (export default) # workPackageValue:PROJ-10:targetVersions:singleline # Outputs the values of PROJ-10 comma-separated # workPackageValue:1234:targetVersions:multiline # Outputs the values of #1234 one per line + # workPackageValue:1234:categories:singleline # Outputs the values of #1234 comma-separated (export default) + # workPackageValue:1234:categories:multiline # Outputs the values of #1234 one per line # # projectLabel:active # Outputs current project label attribute "active" # projectLabel:1234:active # Outputs project label attribute "active" @@ -181,22 +183,21 @@ def self.resolve_value(obj, attribute, disabled_rich_text_fields, layout: nil) custom_field = find_custom_field(obj, attribute) attribute_name = convert_to_attribute_name(custom_field, attribute, obj) - attribute_name = map_legacy_version(attribute_name, obj) + attribute_name = map_legacy_multi_value_attribute(attribute_name, obj) return " " unless can_view_attribute?(custom_field, obj, attribute_name) is_rich_text = custom_field&.formattable? || disabled_rich_text_fields.include?(attribute_name.to_sym) [format_attribute_value(attribute_name, obj.class, obj, is_rich_text, layout:), is_rich_text] end - ## - # The deprecated version attribute renders the work package's target - # versions. - def self.map_legacy_version(attribute_name, obj) - if obj.is_a?(WorkPackage) && attribute_name == "version" - "target_versions" - else - attribute_name - end + # The deprecated single-valued attributes render the whole set that replaces + # them. + LEGACY_MULTI_VALUE_ATTRIBUTES = { "version" => "target_versions", "category" => "categories" }.freeze + + def self.map_legacy_multi_value_attribute(attribute_name, obj) + return attribute_name unless obj.is_a?(WorkPackage) + + LEGACY_MULTI_VALUE_ATTRIBUTES.fetch(attribute_name, attribute_name) end def self.can_view_attribute?(custom_field, obj, attribute_name) diff --git a/config/initializers/export_formats.rb b/config/initializers/export_formats.rb index e1015bce9c4e..866f0677c19a 100644 --- a/config/initializers/export_formats.rb +++ b/config/initializers/export_formats.rb @@ -48,6 +48,7 @@ formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::Days formatter WorkPackage, WorkPackage::Exports::Formatters::XLS::DoneRatio formatter WorkPackage, WorkPackage::Exports::Formatters::PDF::Hours + formatter WorkPackage, WorkPackage::Exports::Formatters::Categories formatter WorkPackage, WorkPackage::Exports::Formatters::Id formatter WorkPackage, WorkPackage::Exports::Formatters::ProjectPhase formatter WorkPackage, WorkPackage::Exports::Formatters::SpentUnits diff --git a/modules/xls_export/app/models/xls_export/work_package/exporter/xls.rb b/modules/xls_export/app/models/xls_export/work_package/exporter/xls.rb index 0a996853a0cb..55587a502c53 100644 --- a/modules/xls_export/app/models/xls_export/work_package/exporter/xls.rb +++ b/modules/xls_export/app/models/xls_export/work_package/exporter/xls.rb @@ -35,7 +35,8 @@ class XLS < WorkPackage::Exports::QueryExporter def records work_packages - .includes(:assigned_to, :type, :priority, :category, :version, :target_versions, :observed_in_versions) + .includes(:assigned_to, :type, :priority, :category, :categories, + :version, :target_versions, :observed_in_versions) end def spreadsheet_title diff --git a/spec/models/work_package/exporter/csv_integration_spec.rb b/spec/models/work_package/exporter/csv_integration_spec.rb index 9f1bac187221..4fdb889238d3 100644 --- a/spec/models/work_package/exporter/csv_integration_spec.rb +++ b/spec/models/work_package/exporter/csv_integration_spec.rb @@ -207,6 +207,54 @@ def byte_order_mark end end + context "with the categories column", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + let(:category_one) { create(:category, project:, name: "Bugs") } + let(:category_two) { create(:category, project:, name: "UI") } + let!(:work_package) do + create(:work_package, project:, type: type_a).tap do |wp| + wp.category_ids_replacements = [category_one.id, category_two.id] + wp.save! + end + end + let(:options) { {} } + let(:query) do + create(:query, project:, user:, column_names: %i(subject categories)) + end + + it "exports the joined category names" do + headers, values = CSV.parse instance.export!.content + pairs = headers.zip(values).to_h + + expect(pairs["Categories"].split("; ")).to eq %w[Bugs UI] + end + + it "preloads categories so cell rendering does not query per row" do + loaded = instance.work_packages.to_a + + expect { loaded.each { it.categories.map(&:name) } }.to have_a_query_limit(0) + end + end + + context "with the deprecated category column (multiple categories feature off)" do + let(:category_one) { create(:category, project:, name: "Bugs") } + let!(:work_package) do + create(:work_package, project:, type: type_a, category: category_one) + end + let(:options) { {} } + let(:query) do + create(:query, project:, user:, column_names: %i(subject category)) + end + + it "exports the category name from the categories data" do + headers, values = CSV.parse instance.export!.content + pairs = headers.zip(values).to_h + + expect(pairs["Category"]).to eq "Bugs" + end + end + context "when no displayed column has a backing association" do let!(:work_package) { create(:work_package, project:, type: type_a, subject: "No associations") } let(:query) do diff --git a/spec/models/work_package/exports/formatters/categories_spec.rb b/spec/models/work_package/exports/formatters/categories_spec.rb new file mode 100644 index 000000000000..f54fdb26b09c --- /dev/null +++ b/spec/models/work_package/exports/formatters/categories_spec.rb @@ -0,0 +1,80 @@ +# 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 "spec_helper" + +RSpec.describe WorkPackage::Exports::Formatters::Categories do + let(:formatter_instance) { described_class.new(:categories) } + + describe ".apply?" do + it "returns true for :categories with any export format" do + expect(described_class.apply?(:categories, :pdf)).to be true + expect(described_class.apply?(:categories, :csv)).to be true + expect(described_class.apply?(:categories, :xls)).to be true + end + + it "returns true for the deprecated :category column, exporting the same data" do + expect(described_class.apply?(:category, :pdf)).to be true + expect(described_class.apply?(:category, :csv)).to be true + expect(described_class.apply?(:category, :xls)).to be true + end + + it "returns false for other attributes" do + expect(described_class.apply?(:subject, :pdf)).to be false + end + end + + describe "#format" do + let(:categories) { [build_stubbed(:category, name: "Bugs"), build_stubbed(:category, name: "UI")] } + let(:work_package) do + build_stubbed(:work_package) do |wp| + allow(wp) + .to receive(:categories) + .and_return(categories) + end + end + + it "returns the joined category names" do + expect(formatter_instance.format(work_package)).to eq("Bugs, UI") + end + + it "honors the array_separator option" do + expect(formatter_instance.format(work_package, array_separator: "; ")).to eq("Bugs; UI") + end + + context "without categories" do + let(:categories) { [] } + + it "returns an empty string" do + expect(formatter_instance.format(work_package)).to eq("") + end + end + end +end From 8d96a51eee5c83e1e6df983d976f4ea795716419 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:30:13 +0200 Subject: [PATCH 10/14] Read and write categories in the bulk edit form The bulk edit form writes through category_ids, with the feature only toggling whether the select accepts a single value or several. Like target_version_ids, the array parameter is normalized separately from the generic scalar "none"/blank transforms. TargetVersionNormalization becomes MultiValueAttributeNormalization, since nothing in it was version-specific and both array parameters need the same magic-value handling. Co-Authored-By: Claude Opus 5 --- .../work_packages/bulk_controller.rb | 26 +++++++++++++------ .../work_packages/moves_controller.rb | 4 +-- .../multi_value_attribute_normalization.rb | 21 ++++++++------- app/views/work_packages/bulk/edit.html.erb | 10 ++++--- 4 files changed, 38 insertions(+), 23 deletions(-) diff --git a/app/controllers/work_packages/bulk_controller.rb b/app/controllers/work_packages/bulk_controller.rb index 575ae7e37ef8..e5928141dadc 100644 --- a/app/controllers/work_packages/bulk_controller.rb +++ b/app/controllers/work_packages/bulk_controller.rb @@ -38,9 +38,12 @@ class WorkPackages::BulkController < ApplicationController include QueriesHelper include WorkPackages::BulkErrorMessage - include WorkPackages::TargetVersionNormalization + include WorkPackages::MultiValueAttributeNormalization include OpTurbo::ComponentStream + # Array-valued form attributes; see #attributes_for_update. + MULTI_VALUE_ATTRIBUTES = %i[target_version_ids category_ids].freeze + def delete_dialog component = if @work_packages.one? @@ -158,13 +161,20 @@ def attributes_for_update attributes = permitted_params.update_work_package attributes[:custom_field_values] = transform_attributes(attributes[:custom_field_values]) attributes = attributes_with_normalized_parent_id(attributes) - # target_version_ids is an array param and must not be run through the generic - # transform below (which is built for scalar "none"/blank magic values), so pull - # it out, normalize it separately, and merge the result back in. - target_version_ids = normalized_target_version_ids(attributes.delete(:target_version_ids)) - attributes = transform_attributes(attributes) - attributes[:target_version_ids] = target_version_ids unless target_version_ids.nil? - attributes + multi_value_ids = extract_multi_value_ids(attributes) + + transform_attributes(attributes).merge(multi_value_ids) + end + + # target_version_ids and category_ids are array params and must not be run + # through the generic transform (which is built for scalar "none"/blank magic + # values), so they are removed from the attributes here and normalized + # separately. A nil result means "leave the existing set untouched" and is + # dropped rather than merged back in. + def extract_multi_value_ids(attributes) + MULTI_VALUE_ATTRIBUTES + .index_with { |attribute| normalized_multi_value_ids(attributes.delete(attribute)) } + .compact end def attributes_with_normalized_parent_id(attributes) diff --git a/app/controllers/work_packages/moves_controller.rb b/app/controllers/work_packages/moves_controller.rb index b2a79b1392c8..944b9d97f9cf 100644 --- a/app/controllers/work_packages/moves_controller.rb +++ b/app/controllers/work_packages/moves_controller.rb @@ -30,7 +30,7 @@ class WorkPackages::MovesController < ApplicationController include WorkPackages::BulkErrorMessage - include WorkPackages::TargetVersionNormalization + include WorkPackages::MultiValueAttributeNormalization include OpTurbo::ComponentStream default_search_scope :work_packages @@ -151,7 +151,7 @@ def attributes_for_create attributes = permitted_params.move_work_package # target_version_ids is an array param and must not be run through the scalar # "none"/blank magic value transforms below. - target_version_ids = normalized_target_version_ids(attributes.delete(:target_version_ids)) + target_version_ids = normalized_multi_value_ids(attributes.delete(:target_version_ids)) attributes = attributes .compact_blank diff --git a/app/controllers/work_packages/multi_value_attribute_normalization.rb b/app/controllers/work_packages/multi_value_attribute_normalization.rb index 7b6f9b590df8..3758382c92c9 100644 --- a/app/controllers/work_packages/multi_value_attribute_normalization.rb +++ b/app/controllers/work_packages/multi_value_attribute_normalization.rb @@ -28,21 +28,22 @@ # See COPYRIGHT and LICENSE files for more details. #++ -# Shared normalization for the array-valued +target_version_ids+ parameter used -# by the work package move and bulk-edit forms. It is pulled out of the generic -# scalar "none"/blank attribute transforms (which are built for scalar values) -# and normalized here instead. -module WorkPackages::TargetVersionNormalization +# Shared normalization for the array-valued +target_version_ids+ and +# +category_ids+ parameters used by the work package move and bulk-edit forms. +# They are pulled out of the generic scalar "none"/blank attribute transforms +# (which are built for scalar values) and normalized here instead. +module WorkPackages::MultiValueAttributeNormalization extend ActiveSupport::Concern included do private - # Mirrors the legacy version_id magic values for the array-valued target_version_ids: - # * blank selection -> nil (leave existing target_versions untouched) - # * "none" selection -> [] (clear all target_versions) - # * a version id -> [id] - def normalized_target_version_ids(raw) + # Mirrors the magic values of the deprecated scalar counterpart for an + # array-valued id parameter: + # * blank selection -> nil (leave the existing set untouched) + # * "none" selection -> [] (clear the set) + # * an id -> [id] + def normalized_multi_value_ids(raw) values = Array(raw).compact_blank values == ["none"] ? [] : values.presence end diff --git a/app/views/work_packages/bulk/edit.html.erb b/app/views/work_packages/bulk/edit.html.erb index ed752ddc3e0f..068faebdf334 100644 --- a/app/views/work_packages/bulk/edit.html.erb +++ b/app/views/work_packages/bulk/edit.html.erb @@ -123,14 +123,18 @@ See COPYRIGHT and LICENSE files for more details. <% if @project %> + <%# Always write through category_ids; the feature only toggles single vs. multiple select. %> + <% multiple_categories = Setting::WorkPackageMultipleCategories.active? %>
- <%= styled_label_tag :work_package_category_id, WorkPackage.human_attribute_name(:category) %> + <%= styled_label_tag :work_package_category_ids, + WorkPackage.human_attribute_name(multiple_categories ? :categories : :category) %>
<%= styled_select_tag( - "work_package[category_id]", + "work_package[category_ids][]", content_tag("option", t(:label_none), value: "none") + options_from_collection_for_select(@project.categories, :id, :name), - include_blank: t(:label_no_change_option) + include_blank: t(:label_no_change_option), + multiple: multiple_categories ) %>
From d7d822e451cba80fffa0ad05844a4357af02e0e8 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:30:21 +0200 Subject: [PATCH 11/14] Read and write categories in the incoming e-mail handler Adds a Categories keyword that assigns the whole comma-separated set. Both it and the deprecated Category keyword are always read so neither leaks into the description, and the plural one wins when both are given. The single-value limit stays with the contract, so the full list is parsed regardless of the feature state. Unlike versions the category keyword is not overridable by default, so the categories keyword is overridable exactly when the deprecated one is. The default overridable attributes move into a constant, keeping assign_options below the complexity threshold. Co-Authored-By: Claude Opus 5 --- .../incoming_emails/dispatch_service.rb | 30 ++++---- app/services/incoming_emails/handlers/base.rb | 2 +- .../incoming_emails/handlers/work_package.rb | 22 +++++- .../wp_with_category_and_categories.eml | 21 ++++++ .../wp_with_multiple_categories.eml | 20 +++++ spec/models/mail_handler_spec.rb | 75 +++++++++++++++++++ 6 files changed, 153 insertions(+), 17 deletions(-) create mode 100644 spec/fixtures/mail_handler/wp_with_category_and_categories.eml create mode 100644 spec/fixtures/mail_handler/wp_with_multiple_categories.eml diff --git a/app/services/incoming_emails/dispatch_service.rb b/app/services/incoming_emails/dispatch_service.rb index 28e50c3219c0..b39a1525eb57 100644 --- a/app/services/incoming_emails/dispatch_service.rb +++ b/app/services/incoming_emails/dispatch_service.rb @@ -33,6 +33,11 @@ class DispatchService REFERENCES_RE = %r{^ "oof", "Auto-Submitted" => /\Aauto-/ @@ -275,29 +280,28 @@ def log(message, level = :info, report: true) nil end - def assign_options(value) # rubocop:disable Metrics/AbcSize + def assign_options(value) options = value.dup options[:issue] ||= {} options[:allow_override] = allow_override_option(options).to_set(&:to_sym) - # Project needs to be overridable if not specified - options[:allow_override] << :project unless options[:issue].has_key?(:project) - # Status overridable by default - options[:allow_override] << :status unless options[:issue].has_key?(:status) - # Version overridable by default - options[:allow_override] << :version unless options[:issue].has_key?(:version) - # Target versions follow the same rule as the deprecated single version - options[:allow_override] << :target_versions unless options[:issue].has_key?(:target_versions) - # Type overridable by default - options[:allow_override] << :type unless options[:issue].has_key?(:type) - # Priority overridable by default - options[:allow_override] << :priority unless options[:issue].has_key?(:priority) + add_default_allow_override(options) options[:no_permission_check] = ActiveRecord::Type::Boolean.new.cast(options[:no_permission_check]) options end + def add_default_allow_override(options) + OVERRIDABLE_BY_DEFAULT.each do |attribute| + options[:allow_override] << attribute unless options[:issue].has_key?(attribute) + end + + # Unlike versions, the category keyword is not overridable by default, so the + # categories keyword is overridable exactly when the deprecated one is. + options[:allow_override] << :categories if options[:allow_override].include?(:category) + end + def allow_override_option(options) if options[:allow_override].is_a?(String) options[:allow_override].split(",").map(&:strip) diff --git a/app/services/incoming_emails/handlers/base.rb b/app/services/incoming_emails/handlers/base.rb index 7eee53e14481..d54c94db69e7 100644 --- a/app/services/incoming_emails/handlers/base.rb +++ b/app/services/incoming_emails/handlers/base.rb @@ -177,7 +177,7 @@ def all_attribute_translations(lang) # Work package attribute translations I18n.with_locale(lang) do - %i[assigned_to category due_date estimated_hours parent priority + %i[assigned_to category categories due_date estimated_hours parent priority remaining_hours responsible start_date status type version target_versions project].each do |attr| translations[attr] = ::WorkPackage.human_attribute_name(attr) end diff --git a/app/services/incoming_emails/handlers/work_package.rb b/app/services/incoming_emails/handlers/work_package.rb index 6ea517f61972..2c56ab0f53e6 100644 --- a/app/services/incoming_emails/handlers/work_package.rb +++ b/app/services/incoming_emails/handlers/work_package.rb @@ -154,7 +154,7 @@ def collect_wp_attributes_from_email_on_update(work_package) def wp_attributes_from_keywords(work_package) { "assigned_to_id" => wp_assignee_from_keywords(work_package), - "category_id" => wp_category_from_keywords(work_package), + "category_ids" => wp_category_ids_from_keywords(work_package), "due_date" => wp_due_date_from_keywords, "estimated_hours" => wp_estimated_hours_from_keywords, "parent_id" => wp_parent_from_keywords, @@ -185,8 +185,24 @@ def wp_priority_from_keywords lookup_case_insensitive_key(IssuePriority, :priority) end - def wp_category_from_keywords(work_package) - lookup_case_insensitive_key(work_package.project.categories, :category) + # Both keywords are always read so neither leaks into the description or + # journal note; the categories keyword takes precedence over the deprecated + # single-category one when both are supplied. + def wp_category_ids_from_keywords(work_package) + plural = get_keyword(:categories) + legacy = get_keyword(:category) + keyword = plural.presence || legacy + return if keyword.blank? + + matching_category_ids(work_package, keyword) + end + + # The single-value limit is enforced by the contract, so the full + # comma-separated list is parsed here regardless of the feature flag. + def matching_category_ids(work_package, keyword) + names = keyword.split(",").map(&:strip).compact_blank + categories_by_name = work_package.project.categories.index_by { |category| category.name.downcase } + names.filter_map { |name| categories_by_name[name.downcase]&.id } end def wp_accountable_from_keywords(work_package) diff --git a/spec/fixtures/mail_handler/wp_with_category_and_categories.eml b/spec/fixtures/mail_handler/wp_with_category_and_categories.eml new file mode 100644 index 000000000000..0600c1b2e420 --- /dev/null +++ b/spec/fixtures/mail_handler/wp_with_category_and_categories.eml @@ -0,0 +1,21 @@ +Return-Path: +Received: from osiris ([127.0.0.1]) + by OSIRIS + with hMailServer ; Sun, 22 Jun 2008 12:28:07 +0200 +Message-ID: <000501c8d452$a95cd7e0$0a00a8c0@osiris> +From: "John Smith" +To: +Subject: New ticket with a category and categories +Date: Sun, 22 Jun 2008 12:28:07 +0200 +MIME-Version: 1.0 +Content-Type: text/plain; + format=flowed; + charset="iso-8859-1"; + reply-type=original +Content-Transfer-Encoding: 7bit + +Some description here + +Project: onlinestore +Category: alpha +Categories: beta diff --git a/spec/fixtures/mail_handler/wp_with_multiple_categories.eml b/spec/fixtures/mail_handler/wp_with_multiple_categories.eml new file mode 100644 index 000000000000..c31cc03063df --- /dev/null +++ b/spec/fixtures/mail_handler/wp_with_multiple_categories.eml @@ -0,0 +1,20 @@ +Return-Path: +Received: from osiris ([127.0.0.1]) + by OSIRIS + with hMailServer ; Sun, 22 Jun 2008 12:28:07 +0200 +Message-ID: <000501c8d452$a95cd7e0$0a00a8c0@osiris> +From: "John Smith" +To: +Subject: New ticket with several categories +Date: Sun, 22 Jun 2008 12:28:07 +0200 +MIME-Version: 1.0 +Content-Type: text/plain; + format=flowed; + charset="iso-8859-1"; + reply-type=original +Content-Transfer-Encoding: 7bit + +Some description here + +Project: onlinestore +Categories: alpha, beta diff --git a/spec/models/mail_handler_spec.rb b/spec/models/mail_handler_spec.rb index d28e3f08228b..7e2d32af9f67 100644 --- a/spec/models/mail_handler_spec.rb +++ b/spec/models/mail_handler_spec.rb @@ -1666,6 +1666,81 @@ expect(work_package.category).to eq(category) end end + + context "when setting categories from keywords" do + let(:permissions) { %i[add_work_packages edit_work_packages view_work_packages] } + let!(:user) do + create(:user, + mail: "JSmith@somenet.foo", + firstname: "John", + lastname: "Smith", + member_with_permissions: { project => permissions }) + end + let!(:alpha) { create(:category, name: "alpha", project:) } + let!(:beta) { create(:category, name: "beta", project:) } + + context "when the multiple-categories feature is enabled", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + subject do + submit_email("wp_with_multiple_categories.eml", + issue: { project: "onlinestore" }, + allow_override: ["category"]) + end + + it "assigns every named category" do + expect(subject.categories) + .to contain_exactly(alpha, beta) + end + + it "keeps the legacy category in sync with the first category" do + expect(subject.category) + .to eql(alpha) + end + + it "removes the keyword from the description" do + expect(subject.description) + .not_to match(/^Categories:/i) + end + end + + context "when the multiple-categories feature is disabled" do + subject do + submit_email("wp_with_multiple_categories.eml", + issue: { project: "onlinestore" }, + allow_override: ["category"]) + end + + it "is refused by the single-value rule rather than silently dropped" do + expect(subject) + .not_to be_persisted + expect(subject.errors.symbols_for(:base)) + .to include(:categories_only_allow_single_value) + end + end + + context "when both category and categories keywords are present", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + subject do + submit_email("wp_with_category_and_categories.eml", + issue: { project: "onlinestore" }, + allow_override: ["category"]) + end + + it "lets the categories keyword win" do + expect(subject.categories) + .to contain_exactly(beta) + end + + it "removes both keywords from the description" do + expect(subject.description) + .not_to match(/^Category:/i) + expect(subject.description) + .not_to match(/^Categories:/i) + end + end + end end private From cd2e90d3446250f0cdd85f622e017ff8a5e95599 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:30:27 +0200 Subject: [PATCH 12/14] Copy categories on work package duplicate and project copy Duplicating a work package carries its whole category set over. A project copy points the copied work packages at the categories copied by Projects::Copy::CategoriesDependentService rather than relying on the reassign-by-name fallback, which cannot tell equally named categories apart. The deprecated category_id is written alongside so it does not contradict the set, as for the versions. Co-Authored-By: Claude Opus 5 --- .../copy/work_packages_dependent_service.rb | 18 ++++++++++++++++++ app/services/work_packages/copy_service.rb | 7 +++++++ 2 files changed, 25 insertions(+) diff --git a/app/services/projects/copy/work_packages_dependent_service.rb b/app/services/projects/copy/work_packages_dependent_service.rb index 16764ab527bc..12154a3262ab 100644 --- a/app/services/projects/copy/work_packages_dependent_service.rb +++ b/app/services/projects/copy/work_packages_dependent_service.rb @@ -67,6 +67,7 @@ def source_work_packages :custom_values, :target_versions, :observed_in_versions, + :categories, :assigned_to, :responsible ) @@ -137,6 +138,7 @@ def copy_relations(source_wp, new_wp_id, work_packages_map) def copy_work_package_attribute_overrides(source_work_package, parent_id, user_cf_ids) target_version_ids = work_package_target_version_ids(source_work_package) + category_ids = work_package_category_ids(source_work_package) { project: target, @@ -146,6 +148,10 @@ def copy_work_package_attribute_overrides(source_work_package, parent_id, user_c version_id: target_version_ids&.first, target_version_ids:, observed_in_version_ids: work_package_observed_in_version_ids(source_work_package), + # Same as version_id above: the legacy category_id must not contradict the + # category set until the column is dropped. + category_id: category_ids&.first, + category_ids:, assigned_to_id: work_package_assigned_to_id(source_work_package), responsible_id: work_package_responsible_id(source_work_package), custom_field_values: custom_value_attributes(source_work_package, user_cf_ids), @@ -170,6 +176,18 @@ def work_package_observed_in_version_ids(source_work_package) source_work_package.observed_in_versions.filter_map { |v| state.version_id_lookup[v.id] }.presence end + # Categories are project-owned, so a copied work package has to point at the + # copies made by Projects::Copy::CategoriesDependentService. Returning nil + # (categories were not copied) leaves the set untouched here; the set-attributes + # service then reassigns it by name on the project change, which finds nothing + # and clears it. + def work_package_category_ids(source_work_package) + lookup = state.category_id_lookup + return if lookup.nil? + + source_work_package.categories.filter_map { |c| lookup[c.id] }.presence + end + def work_package_assigned_to_id(source_work_package) possible_principal_id(source_work_package.assigned_to_id) end diff --git a/app/services/work_packages/copy_service.rb b/app/services/work_packages/copy_service.rb index a996de3c4ad9..957849a119e0 100644 --- a/app/services/work_packages/copy_service.rb +++ b/app/services/work_packages/copy_service.rb @@ -87,6 +87,7 @@ def copied_attributes(work_package, override) .slice(*writable_attributes) .merge("custom_field_values" => work_package.custom_value_attributes) .merge(version_reference_attributes(work_package, writable_attributes)) + .merge(category_reference_attributes(work_package, writable_attributes)) .merge(overwritten_attributes) if overwritten_attributes.has_key?("start_date") && @@ -116,6 +117,12 @@ def version_reference_attributes(work_package, writable_attributes) attributes.compact end + def category_reference_attributes(work_package, writable_attributes) + return {} unless writable_attributes.include?("categories") + + { "category_ids" => work_package.categories.pluck(:id).presence }.compact + end + def remove_author_watcher(copied) copied.remove_watcher(copied.author) end From b6ba84c0a20723df31240e86a6e504c0b29c153c Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:30:36 +0200 Subject: [PATCH 13/14] Show categories in mails, reports and automatic subjects Notification mails, the work package summary report and the automatic subject tokens all read the category set instead of the deprecated column, listing every category once the multiple categories feature is enabled and keeping the single "Category" label and value until then. The summary heading stays singular either way, since it names a single category per section. Co-Authored-By: Claude Opus 5 --- app/helpers/work_packages_helper.rb | 16 ++++++++++ .../patterns/token_property_mapper.rb | 4 +-- app/services/reports/category_report.rb | 2 ++ .../_work_package_details.html.erb | 2 +- .../_work_package_details.text.erb | 2 +- spec/features/work_packages/reports_spec.rb | 32 +++++++++++++++++++ spec/mailers/work_package_mailer_spec.rb | 32 +++++++++++++++++++ .../patterns/token_property_mapper_spec.rb | 32 +++++++++++++++++++ 8 files changed, 118 insertions(+), 4 deletions(-) diff --git a/app/helpers/work_packages_helper.rb b/app/helpers/work_packages_helper.rb index fc2f5f00f168..ad080b67a6fc 100644 --- a/app/helpers/work_packages_helper.rb +++ b/app/helpers/work_packages_helper.rb @@ -153,6 +153,22 @@ def work_package_versions_value(work_package) end end + def work_package_categories_label + attribute = Setting::WorkPackageMultipleCategories.active? ? :categories : :category + WorkPackage.human_attribute_name(attribute) + end + + # Presented value for the category(ies) attribute, read from the categories + # association. Legacy behaviour surfaces the single associated category; with the + # feature on it lists all of them (the association is already name-ordered). + def work_package_categories_value(work_package) + if Setting::WorkPackageMultipleCategories.active? + work_package.categories.join(", ") + else + work_package.categories.first + end + end + private def truncated_work_package_description(work_package, lines = 3) # rubocop:disable Metrics/AbcSize diff --git a/app/models/work_package_types/patterns/token_property_mapper.rb b/app/models/work_package_types/patterns/token_property_mapper.rb index 00b369dc37fd..556374831f30 100644 --- a/app/models/work_package_types/patterns/token_property_mapper.rb +++ b/app/models/work_package_types/patterns/token_property_mapper.rb @@ -47,7 +47,7 @@ def attribute(key, label_fn, value_fn, formatter = STRING_OR_NIL) attribute(:accountable, -> { WorkPackage.human_attribute_name(:responsible) }, ->(wp) { wp.responsible }), attribute(:assignee, -> { WorkPackage.human_attribute_name(:assigned_to) }, ->(wp) { wp.assigned_to }), attribute(:author, -> { WorkPackage.human_attribute_name(:author) }, ->(wp) { wp.author }), - attribute(:category, -> { WorkPackage.human_attribute_name(:category) }, ->(wp) { wp.category }), + attribute(:category, -> { Setting::WorkPackageMultipleCategories.active? ? WorkPackage.human_attribute_name(:categories) : WorkPackage.human_attribute_name(:category) }, ->(wp) { wp.categories }, ARRAY), attribute(:creation_date, -> { WorkPackage.human_attribute_name(:created_at) }, ->(wp) { wp.created_at }, DATE), attribute(:estimated_time, -> { WorkPackage.human_attribute_name(:estimated_hours) }, ->(wp) { wp.estimated_hours }, DURATION), attribute(:remaining_time, -> { WorkPackage.human_attribute_name(:remaining_hours) }, ->(wp) { wp.remaining_hours }, DURATION), @@ -55,7 +55,7 @@ def attribute(key, label_fn, value_fn, formatter = STRING_OR_NIL) attribute(:parent_id, -> { WorkPackage.human_attribute_name(:id) }, ->(parent) { parent.id }), attribute(:parent_assignee, -> { WorkPackage.human_attribute_name(:assigned_to) }, ->(parent) { parent.assigned_to }), attribute(:parent_author, -> { WorkPackage.human_attribute_name(:author) }, ->(parent) { parent.author }), - attribute(:parent_category, -> { WorkPackage.human_attribute_name(:category) }, ->(parent) { parent.category }), + attribute(:parent_category, -> { Setting::WorkPackageMultipleCategories.active? ? WorkPackage.human_attribute_name(:categories) : WorkPackage.human_attribute_name(:category) }, ->(parent) { parent.categories }, ARRAY), attribute(:parent_creation_date, -> { WorkPackage.human_attribute_name(:created_at) }, ->(parent) { parent.created_at }, DATE), attribute(:parent_estimated_time, -> { WorkPackage.human_attribute_name(:estimated_hours) }, ->(parent) { parent.estimated_hours }, DURATION), attribute(:parent_remaining_time, -> { WorkPackage.human_attribute_name(:remaining_hours) }, ->(parent) { parent.remaining_hours }, DURATION), diff --git a/app/services/reports/category_report.rb b/app/services/reports/category_report.rb index 347a7e9392a2..77d0b249863d 100644 --- a/app/services/reports/category_report.rb +++ b/app/services/reports/category_report.rb @@ -45,6 +45,8 @@ def data @data ||= WorkPackage.by_category(@project) end + # The summary groups work packages under each single category, so the heading + # stays singular whether or not multiple categories are enabled. def title @title ||= WorkPackage.human_attribute_name(:category) end diff --git a/app/views/work_package_mailer/_work_package_details.html.erb b/app/views/work_package_mailer/_work_package_details.html.erb index 491e03bb8795..6537e794273d 100644 --- a/app/views/work_package_mailer/_work_package_details.html.erb +++ b/app/views/work_package_mailer/_work_package_details.html.erb @@ -35,7 +35,7 @@ See COPYRIGHT and LICENSE files for more details.
  • <%= WorkPackage.human_attribute_name(:priority) %>: <%= work_package.priority %>
  • <%= WorkPackage.human_attribute_name(:assigned_to) %>: <%= work_package.assigned_to %>
  • <%= WorkPackage.human_attribute_name(:responsible) %>: <%= work_package.responsible %>
    • -
  • <%= WorkPackage.human_attribute_name(:category) %>: <%= work_package.category %>
    • +
  • <%= work_package_categories_label %>: <%= work_package_categories_value(work_package) %>
  • <%= work_package_versions_label %>: <%= work_package_versions_value(work_package) %>
  • <%= WorkPackage.human_attribute_name(:start_date) %>: <%= work_package.start_date %>
  • <%= WorkPackage.human_attribute_name(:due_date) %>: <%= work_package.due_date %>
    • diff --git a/app/views/work_package_mailer/_work_package_details.text.erb b/app/views/work_package_mailer/_work_package_details.text.erb index cfa805339bab..79fbfddd3cc0 100644 --- a/app/views/work_package_mailer/_work_package_details.text.erb +++ b/app/views/work_package_mailer/_work_package_details.text.erb @@ -35,7 +35,7 @@ See COPYRIGHT and LICENSE files for more details. <%= WorkPackage.human_attribute_name(:priority) %>: <%= work_package.priority %> <%= WorkPackage.human_attribute_name(:assigned_to) %>: <%= work_package.assigned_to %> <%= WorkPackage.human_attribute_name(:responsible) %>: <%= work_package.responsible %> -<%= WorkPackage.human_attribute_name(:category) %>: <%= work_package.category %> +<%= work_package_categories_label %>: <%= work_package_categories_value(work_package) %> <%= work_package_versions_label %>: <%= work_package_versions_value(work_package) %> <%= WorkPackage.human_attribute_name(:start_date) %>: <%= work_package.start_date %> <%= WorkPackage.human_attribute_name(:due_date) %>: <%= work_package.due_date %> diff --git a/spec/features/work_packages/reports_spec.rb b/spec/features/work_packages/reports_spec.rb index 50abe66c5b6e..cd4b64448560 100644 --- a/spec/features/work_packages/reports_spec.rb +++ b/spec/features/work_packages/reports_spec.rb @@ -151,4 +151,36 @@ end end end + + context "with the multiple categories feature enabled", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + let!(:category_a) { create(:category, project:, name: "Alpha bugs") } + let!(:category_b) { create(:category, project:, name: "Beta docs") } + let!(:wp_multi) do + create(:work_package, project:, type: type_a, status: type_a.statuses.first).tap do |wp| + wp.category_ids_replacements = [category_a.id, category_b.id] + wp.save! + end + end + + it "counts a work package with several categories under each of them" do + wp_table_page.visit! + + within ".main-menu--children" do + click_on "Summary" + end + + expect(page).to have_text "CATEGORY" + + click_link "Further analyze: Category" + + aggregate_failures do + [category_a, category_b].each do |category| + row = page.find(:xpath, "//tbody/tr[td[normalize-space()='#{category.name}']]") + expect(row).to have_css("td:last-child", text: "1") + end + end + end + end end diff --git a/spec/mailers/work_package_mailer_spec.rb b/spec/mailers/work_package_mailer_spec.rb index 8538e1d8d4db..cf266587ca64 100644 --- a/spec/mailers/work_package_mailer_spec.rb +++ b/spec/mailers/work_package_mailer_spec.rb @@ -287,6 +287,38 @@ end end + describe "rendering the category(ies) detail from the categories association" do + subject(:mail) { described_class.watcher_changed(work_package, recipient, author, "added") } + + let(:category_a) { build_stubbed(:category, name: "Bugs") } + let(:category_b) { build_stubbed(:category, name: "UI") } + + before do + allow(work_package).to receive(:categories).and_return(categories) + end + + context "with multiple categories disabled (legacy behaviour)", + with_flag: { work_package_multiple_categories: false } do + let(:categories) { [category_a] } + + it "labels the row 'Category' and shows the single category" do + expect(mail.text_part.body.encoded).to include("Category: Bugs") + expect(mail.html_part.body.encoded).to include("
  • Category: Bugs
    • ") + end + end + + context "with multiple categories enabled", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + let(:categories) { [category_a, category_b] } + + it "labels the row 'Categories' and lists all categories" do + expect(mail.text_part.body.encoded).to include("Categories: Bugs, UI") + expect(mail.html_part.body.encoded).to include("
  • Categories: Bugs, UI
    • ") + end + end + end + describe "rendering the latest comment containing a WP reference" do shared_let(:persisted_project) { create(:project, identifier: "demo") } shared_let(:persisted_recipient) { create(:admin) } diff --git a/spec/models/work_package_types/patterns/token_property_mapper_spec.rb b/spec/models/work_package_types/patterns/token_property_mapper_spec.rb index a59cf94834f5..646548322f70 100644 --- a/spec/models/work_package_types/patterns/token_property_mapper_spec.rb +++ b/spec/models/work_package_types/patterns/token_property_mapper_spec.rb @@ -233,6 +233,38 @@ end end end + + context "for categories" do + shared_let(:second_category) { create(:category, project:) } + + before do + create(:work_package_category, work_package:, category: second_category) + end + + context "when work package multiple categories is active", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + it "renders an array of values" do + enabled, = subject + token = detect(enabled, :category) + + expect(token.call(work_package)).to eq([category.name, second_category.name].sort.join(", ")) + end + + it "label is categories" do + enabled, = subject + expect(detect(enabled, :category)&.label).to eq("Categories") + end + end + + context "when work package multiple categories is not active", + with_settings: { work_package_multiple_categories: false } do + it "label is category" do + enabled, = subject + expect(detect(enabled, :category)&.label).to eq("Category") + end + end + end end private From f517d5670d938b3ed3cd141406b0cd0fb7fb4ce0 Mon Sep 17 00:00:00 2001 From: Tomas Hykel Date: Wed, 5 Aug 2026 20:30:47 +0200 Subject: [PATCH 14/14] Add a categories work package edit field The categories attribute always reads and writes a collection, but the schema restricts it to a single value while the multiple categories feature is inactive. In that mode the field mimics the single select it stands in for: an explicit "-" option, no save/cancel controls, saving right on selection. That behaviour is extracted from the versions field into a shared SingleOrMultiSelectEditFieldComponent, since it belongs to any attribute replacing a deprecated single-valued one. Categories add nothing on top: unlike versions they are never shared across projects and cannot be created from within the field. Displaying the attribute follows []Version throughout: the resources field in tables, one line per category in the single view, and a comma-separated list under the singleline layout the macro component maps the deprecated attribute to. Co-Authored-By: Claude Opus 5 --- .../wp-edit/work-package-changeset.ts | 4 +- .../display/display-field.initializer.ts | 2 +- .../display/display-field.service.spec.ts | 1 + .../fields/display/display-field.service.ts | 6 +- .../fields/edit/edit-field.initializer.ts | 9 ++ .../categories-edit-field.component.html | 33 +++++ .../categories-edit-field.component.ts | 51 ++++++++ ...le-or-multi-select-edit-field.component.ts | 108 ++++++++++++++++ .../versions-edit-field.component.ts | 69 +--------- .../attribute-value-macro.component.spec.ts | 27 ++++ .../macros/attribute-value-macro.component.ts | 9 +- .../fields/openproject-fields.module.ts | 2 + .../table/categories_column_spec.rb | 118 ++++++++++++++++++ spec/support/edit_fields/edit_field.rb | 2 +- 14 files changed, 364 insertions(+), 77 deletions(-) create mode 100644 frontend/src/app/shared/components/fields/edit/field-types/categories-edit-field.component.html create mode 100644 frontend/src/app/shared/components/fields/edit/field-types/categories-edit-field.component.ts create mode 100644 frontend/src/app/shared/components/fields/edit/field-types/single-or-multi-select-edit-field.component.ts create mode 100644 spec/features/work_packages/table/categories_column_spec.rb diff --git a/frontend/src/app/features/work-packages/components/wp-edit/work-package-changeset.ts b/frontend/src/app/features/work-packages/components/wp-edit/work-package-changeset.ts index 18a893cbab52..ea1c5149b4fd 100644 --- a/frontend/src/app/features/work-packages/components/wp-edit/work-package-changeset.ts +++ b/frontend/src/app/features/work-packages/components/wp-edit/work-package-changeset.ts @@ -62,12 +62,12 @@ export class WorkPackageChangeset extends ResourceChangeset delete (payload as { subject?:string }).subject; } - // Explicitly exclude the version collections if they are empty + // Explicitly exclude the version and category collections if they are empty if (isNewResource(this.pristineResource)) { const links = (payload as { _links?:Record })._links; if (links) { - ['targetVersions', 'observedInVersions'].forEach((attribute) => { + ['targetVersions', 'observedInVersions', 'categories'].forEach((attribute) => { const value = links[attribute]; if (!Array.isArray(value) || value.length === 0) { diff --git a/frontend/src/app/shared/components/fields/display/display-field.initializer.ts b/frontend/src/app/shared/components/fields/display/display-field.initializer.ts index fe2831455430..6e91466f144b 100644 --- a/frontend/src/app/shared/components/fields/display/display-field.initializer.ts +++ b/frontend/src/app/shared/components/fields/display/display-field.initializer.ts @@ -114,7 +114,7 @@ export function initializeCoreDisplayFields(displayFieldService:DisplayFieldServ 'Workspace']) .addFieldType(ProjectPhaseDisplayField, 'projectPhase', ['ProjectPhase']) .addFieldType(ResourcesDisplayField, 'resources', ['[]CustomOption', '[]CustomField::Hierarchy::Item']) - .addFieldType(ResourcesDisplayField, 'resources', ['[]Version']) + .addFieldType(ResourcesDisplayField, 'resources', ['[]Version', '[]Category']) .addFieldType(MultipleUserFieldModule, 'users', ['[]User']) .addFieldType(FormattableDisplayField, 'formattable', ['Formattable']) .addFieldType(DaysDurationDisplayField, 'duration', ['duration']) diff --git a/frontend/src/app/shared/components/fields/display/display-field.service.spec.ts b/frontend/src/app/shared/components/fields/display/display-field.service.spec.ts index dd736d51b401..18bed20b1b63 100644 --- a/frontend/src/app/shared/components/fields/display/display-field.service.spec.ts +++ b/frontend/src/app/shared/components/fields/display/display-field.service.spec.ts @@ -77,6 +77,7 @@ describe('DisplayFieldService', () => { // rendering in the singleline layout via a dedicated field. const multiValueTypes:[string, DisplayFieldClass, DisplayFieldClass][] = [ ['[]Version', SingleLineResourcesDisplayField, MultipleLinesCustomOptionsDisplayField], + ['[]Category', SingleLineResourcesDisplayField, MultipleLinesCustomOptionsDisplayField], ['[]CustomOption', SingleLineResourcesDisplayField, MultipleLinesCustomOptionsDisplayField], ['[]User', SingleLineUserDisplayField, MultipleLinesUserFieldModule], ['[]CustomField::Hierarchy::Item', SingleLineResourcesDisplayField, MultipleLinesHierarchyItemDisplayField], diff --git a/frontend/src/app/shared/components/fields/display/display-field.service.ts b/frontend/src/app/shared/components/fields/display/display-field.service.ts index 489105af7670..bac4be5f51ab 100644 --- a/frontend/src/app/shared/components/fields/display/display-field.service.ts +++ b/frontend/src/app/shared/components/fields/display/display-field.service.ts @@ -97,7 +97,7 @@ export class DisplayFieldService extends AbstractFieldService + + + + + +@if (!handler.inEditMode && allowMultiple) { + +} diff --git a/frontend/src/app/shared/components/fields/edit/field-types/categories-edit-field.component.ts b/frontend/src/app/shared/components/fields/edit/field-types/categories-edit-field.component.ts new file mode 100644 index 000000000000..5e1f20aaee51 --- /dev/null +++ b/frontend/src/app/shared/components/fields/edit/field-types/categories-edit-field.component.ts @@ -0,0 +1,51 @@ +//-- 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. +//++ + +import { ChangeDetectionStrategy, Component } from '@angular/core'; +import { + SingleOrMultiSelectEditFieldComponent, +} from 'core-app/shared/components/fields/edit/field-types/single-or-multi-select-edit-field.component'; + +/** + * Edit field for the category collection attribute of work packages. + * + * The attribute always reads and writes a collection, but the schema restricts it + * to a single value (options.multiple) as long as the multiple categories setting + * is inactive. In that mode the field mimics the single select field it stands in + * for: an explicit "-" option, no save/cancel controls, saving right on selection. + * + * Unlike versions, categories are never shared across projects and cannot be + * created from within the field, so there is neither grouping nor a create option. + */ +@Component({ + templateUrl: './categories-edit-field.component.html', + changeDetection: ChangeDetectionStrategy.OnPush, + standalone: false, +}) +export class CategoriesEditFieldComponent extends SingleOrMultiSelectEditFieldComponent { +} diff --git a/frontend/src/app/shared/components/fields/edit/field-types/single-or-multi-select-edit-field.component.ts b/frontend/src/app/shared/components/fields/edit/field-types/single-or-multi-select-edit-field.component.ts new file mode 100644 index 000000000000..a24f4e0d2059 --- /dev/null +++ b/frontend/src/app/shared/components/fields/edit/field-types/single-or-multi-select-edit-field.component.ts @@ -0,0 +1,108 @@ +//-- 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. +//++ + +import { + MultiSelectEditFieldComponent, +} from 'core-app/shared/components/fields/edit/field-types/multi-select-edit-field.component'; +import { ValueOption } from 'core-app/shared/components/fields/edit/field-types/select-edit-field/select-edit-field.component'; +import { HalResource } from 'core-app/features/hal/resources/hal-resource'; + +/** + * Base for edit fields whose attribute always reads and writes a collection but + * whose schema may restrict it to a single value (options.multiple) — which the + * attributes replacing a deprecated single-valued one do as long as their feature + * is inactive. + * + * In single value mode the field mimics the single select field it stands in for: + * an explicit "-" option, no save/cancel controls, saving right on selection. + * + * Subclasses supply the template. + */ +export class SingleOrMultiSelectEditFieldComponent extends MultiSelectEditFieldComponent { + private noValueOption:ValueOption = { name: this.text.placeholder, href: null }; + + /** Whether the schema allows assigning more than one value. */ + public get allowMultiple():boolean { + return (this.schema.options as { multiple?:boolean }|undefined)?.multiple !== false; + } + + /** Memoized options of the selectableOptions getter, keyed by the array they were built from. */ + private selectableOptionsSource:unknown = null; + + private selectableOptionsBuilt:ValueOption[] = []; + + /** + * The selectable options, extended by an explicit "-" option to unset + * the value in single value mode (mirroring the single select fields). + * + * The getter is bound in the template, so it must return a stable array + * reference while the available options stay the same — a fresh array per + * change detection cycle would make ng-select reprocess the whole list on + * every tick. + */ + public get selectableOptions():HalResource[]|ValueOption[] { + if (this.allowMultiple || this.required) { + return this.availableOptions as HalResource[]; + } + + if (this.selectableOptionsSource !== this.availableOptions) { + this.selectableOptionsSource = this.availableOptions; + this.selectableOptionsBuilt = [this.noValueOption, ...(this.availableOptions as ValueOption[])]; + } + + return this.selectableOptionsBuilt; + } + + /** + * The ng-select model: the selected options in multiple mode, + * the single selected option (or null) otherwise. + */ + public get model():ValueOption[]|ValueOption|null { + if (this.allowMultiple) { + return this.selectedOption; + } + + return this.selectedOption[0] ?? null; + } + + public set model(val:ValueOption[]|ValueOption|null) { + const values = val == null ? [] : [val].flat(); + // Selecting the "-" option unsets the value. + this.selectedOption = values.filter((option) => option.href != null); + } + + /** + * In single value mode the field saves right after selection, mirroring the + * behavior of the single select edit fields it stands in for. + */ + public onSelectionChange():void { + if (!this.allowMultiple) { + void this.handler.handleUserSubmit(); + } + } +} diff --git a/frontend/src/app/shared/components/fields/edit/field-types/versions-edit-field.component.ts b/frontend/src/app/shared/components/fields/edit/field-types/versions-edit-field.component.ts index b1d6569c614b..4d8899dd9f98 100644 --- a/frontend/src/app/shared/components/fields/edit/field-types/versions-edit-field.component.ts +++ b/frontend/src/app/shared/components/fields/edit/field-types/versions-edit-field.component.ts @@ -29,9 +29,8 @@ import { ChangeDetectionStrategy, Component, OnInit, inject } from '@angular/core'; import { firstValueFrom } from 'rxjs'; import { - MultiSelectEditFieldComponent, -} from 'core-app/shared/components/fields/edit/field-types/multi-select-edit-field.component'; -import { ValueOption } from 'core-app/shared/components/fields/edit/field-types/select-edit-field/select-edit-field.component'; + SingleOrMultiSelectEditFieldComponent, +} from 'core-app/shared/components/fields/edit/field-types/single-or-multi-select-edit-field.component'; import { HalResource } from 'core-app/features/hal/resources/hal-resource'; import { VersionResource } from 'core-app/features/hal/resources/version-resource'; import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; @@ -56,7 +55,7 @@ import { HalResourceNotificationService } from 'core-app/features/hal/services/h changeDetection: ChangeDetectionStrategy.OnPush, standalone: false, }) -export class VersionsEditFieldComponent extends MultiSelectEditFieldComponent implements OnInit { +export class VersionsEditFieldComponent extends SingleOrMultiSelectEditFieldComponent implements OnInit { readonly apiV3Service = inject(ApiV3Service); readonly currentProject = inject(CurrentProjectService); readonly halNotification = inject(HalResourceNotificationService); @@ -66,8 +65,6 @@ export class VersionsEditFieldComponent extends MultiSelectEditFieldComponent im public createLabel = this.I18n.t('js.label_create'); - private noValueOption:ValueOption = { name: this.text.placeholder, href: null }; - groupByFn = (item:HalResource):string|null => { // Do not group the "-" (no value) option if (!item.href) return null; @@ -81,66 +78,6 @@ export class VersionsEditFieldComponent extends MultiSelectEditFieldComponent im this.setupVersionCreation(); } - /** Whether the schema allows assigning more than one version. */ - public get allowMultiple():boolean { - return (this.schema.options as { multiple?:boolean }|undefined)?.multiple !== false; - } - - /** Memoized options of the selectableOptions getter, keyed by the array they were built from. */ - private selectableOptionsSource:unknown = null; - - private selectableOptionsBuilt:ValueOption[] = []; - - /** - * The selectable options, extended by an explicit "-" option to unset - * the value in single value mode (mirroring the single select fields). - * - * The getter is bound in the template, so it must return a stable array - * reference while the available options stay the same — a fresh array per - * change detection cycle would make ng-select reprocess the whole list on - * every tick. - */ - public get selectableOptions():HalResource[]|ValueOption[] { - if (this.allowMultiple || this.required) { - return this.availableOptions as HalResource[]; - } - - if (this.selectableOptionsSource !== this.availableOptions) { - this.selectableOptionsSource = this.availableOptions; - this.selectableOptionsBuilt = [this.noValueOption, ...(this.availableOptions as ValueOption[])]; - } - - return this.selectableOptionsBuilt; - } - - /** - * The ng-select model: the selected options in multiple mode, - * the single selected option (or null) otherwise. - */ - public get model():ValueOption[]|ValueOption|null { - if (this.allowMultiple) { - return this.selectedOption; - } - - return this.selectedOption[0] ?? null; - } - - public set model(val:ValueOption[]|ValueOption|null) { - const values = val == null ? [] : [val].flat(); - // Selecting the "-" option unsets the value. - this.selectedOption = values.filter((option) => option.href != null); - } - - /** - * In single value mode the field saves right after selection, mirroring the - * behavior of the single select edit fields it stands in for. - */ - public onSelectionChange():void { - if (!this.allowMultiple) { - void this.handler.handleUserSubmit(); - } - } - /** * Allow creating a version from within the field when the current project is * among the projects a version may be created in (mirroring diff --git a/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.spec.ts b/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.spec.ts index 74b2c378282c..21db618749f0 100644 --- a/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.spec.ts +++ b/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.spec.ts @@ -99,4 +99,31 @@ describe('AttributeValueMacroComponent', () => { expect(component.layout).toBeUndefined(); }); }); + + describe('with the deprecated category attribute on a work package', () => { + it('maps to categories with the singleline layout', async () => { + const component = await render({ model: 'workPackage', id: '42', attribute: 'category' }); + + expect(component.fieldName).toEqual('categories'); + expect(component.layout).toEqual('singleline'); + }); + + it('keeps an explicitly requested multiline layout', async () => { + const component = await render({ + model: 'workPackage', id: '42', attribute: 'category', layout: 'multiline', + }); + + expect(component.fieldName).toEqual('categories'); + expect(component.layout).toEqual('multiline'); + }); + }); + + describe('with a category attribute on another resource type', () => { + it('keeps the attribute untouched', async () => { + const component = await render({ model: 'project', id: '42', attribute: 'category' }, 'Project'); + + expect(component.fieldName).toEqual('category'); + expect(component.layout).toBeUndefined(); + }); + }); }); diff --git a/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.ts b/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.ts index dde24d102621..eb251b0abc01 100644 --- a/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.ts +++ b/frontend/src/app/shared/components/fields/macros/attribute-value-macro.component.ts @@ -133,10 +133,11 @@ export class AttributeValueMacroComponent implements OnInit { const proxied = this.schemaCache.proxied(resource, schema); let attribute = schema.attributeFromLocalizedName(attributeName) ?? this.dateAttribute(resource, proxied, attributeName); - // The deprecated version attribute renders the work package's target - // versions, single-line by default so legacy macros keep their inline shape. - if (resource._type === 'WorkPackage' && attribute === 'version') { - attribute = 'targetVersions'; + // The deprecated single-valued attributes render the whole set that replaces + // them, single-line by default so legacy macros keep their inline shape. + const legacyMultiValueAttributes:Record = { version: 'targetVersions', category: 'categories' }; + if (resource._type === 'WorkPackage' && attribute && legacyMultiValueAttributes[attribute]) { + attribute = legacyMultiValueAttributes[attribute]; this.layout = this.layout ?? 'singleline'; } diff --git a/frontend/src/app/shared/components/fields/openproject-fields.module.ts b/frontend/src/app/shared/components/fields/openproject-fields.module.ts index 811f21e8dbb9..7faa54237980 100644 --- a/frontend/src/app/shared/components/fields/openproject-fields.module.ts +++ b/frontend/src/app/shared/components/fields/openproject-fields.module.ts @@ -65,6 +65,7 @@ import { ProgressPopoverEditFieldComponent } from 'core-app/shared/components/fi import { OpExclusionInfoComponent } from 'core-app/shared/components/fields/display/info/op-exclusion-info.component'; import { UserEditFieldComponent } from './edit/field-types/user-edit-field.component'; import { VersionsEditFieldComponent } from 'core-app/shared/components/fields/edit/field-types/versions-edit-field.component'; +import { CategoriesEditFieldComponent } from 'core-app/shared/components/fields/edit/field-types/categories-edit-field.component'; import { DaysDurationEditFieldComponent } from 'core-app/shared/components/fields/edit/field-types/days-duration-edit-field.component'; import { CombinedDateEditFieldComponent } from './edit/field-types/combined-date-edit-field.component'; import { NgSelectModule } from '@ng-select/ng-select'; @@ -120,6 +121,7 @@ import { FormsModule } from '@angular/forms'; ProjectEditFieldComponent, UserEditFieldComponent, VersionsEditFieldComponent, + CategoriesEditFieldComponent, WorkPackageEditFieldComponent, EditFormComponent, DisplayFieldComponent, diff --git a/spec/features/work_packages/table/categories_column_spec.rb b/spec/features/work_packages/table/categories_column_spec.rb new file mode 100644 index 000000000000..41a3d2f5ef22 --- /dev/null +++ b/spec/features/work_packages/table/categories_column_spec.rb @@ -0,0 +1,118 @@ +# 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 "spec_helper" + +RSpec.describe "Work package table categories column", :js do + shared_let(:project) { create(:project) } + shared_let(:user) do + create(:user, + member_with_permissions: { + project => %i[view_work_packages edit_work_packages save_queries] + }) + end + shared_let(:category_one) { create(:category, project:, name: "1. Bugs") } + shared_let(:category_two) { create(:category, project:, name: "2. UI") } + shared_let(:category_three) { create(:category, project:, name: "3. Docs") } + + let(:wp_table) { Pages::WorkPackagesTable.new(project) } + let(:columns) { Components::WorkPackages::Columns.new } + + let!(:work_package) do + create(:work_package, project:).tap do |wp| + wp.category_ids_replacements = [category_one.id, category_two.id] + wp.save! + end + end + + let!(:query) do + create(:query, user:, project:, column_names: %w[id subject categories]) + end + + before do + login_as(user) + end + + context "with multiple categories active", + with_flag: { work_package_multiple_categories: true }, + with_settings: { work_package_multiple_categories: true } do + it "shows all categories and allows editing them inline" do + wp_table.visit_query query + wp_table.expect_work_package_listed work_package + + expect(page).to have_css(".wp-table--table-header", text: "CATEGORIES") + + field = wp_table.edit_field(work_package, :categories) + field.expect_text category_one.name + field.expect_text category_two.name + + field.activate! + field.set_value category_three.name + field.submit_by_dashboard + + wp_table.expect_and_dismiss_toaster(message: "Successful update.") + + # The cell renders two values and elides the rest behind a count, so only + # assert the elision marker here; the persisted set is checked below. + field.expect_text "...3" + expect(work_package.reload.categories) + .to contain_exactly(category_one, category_two, category_three) + end + + it "offers the categories column but not the category column" do + wp_table.visit_query query + wp_table.expect_work_package_listed work_package + + columns.open_modal + columns.expect_checked "Categories" + columns.expect_column_not_available(/^Category$/) + end + end + + context "with multiple categories inactive" do + let!(:query) do + create(:query, user:, project:, column_names: %w[id subject category]) + end + + it "keeps the single category column" do + wp_table.visit_query query + wp_table.expect_work_package_listed work_package + + expect(page).to have_css(".wp-table--table-header", text: "CATEGORY") + + field = wp_table.edit_field(work_package, :category) + field.expect_text category_one.name + + columns.open_modal + columns.expect_checked "Category" + columns.expect_column_not_available "Categories" + end + end +end diff --git a/spec/support/edit_fields/edit_field.rb b/spec/support/edit_fields/edit_field.rb index 3dd06a65211a..ca0309ec8646 100644 --- a/spec/support/edit_fields/edit_field.rb +++ b/spec/support/edit_fields/edit_field.rb @@ -321,7 +321,7 @@ def derive_field_type "op-user-autocompleter" when :priority, :status, :type, :category, :workPackage, :parent, :projectPhase "create-autocompleter" - when :targetVersions + when :targetVersions, :categories "ng-select" when :project "op-project-autocompleter"