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/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/target_version_normalization.rb b/app/controllers/work_packages/multi_value_attribute_normalization.rb similarity index 71% rename from app/controllers/work_packages/target_version_normalization.rb rename to app/controllers/work_packages/multi_value_attribute_normalization.rb index 7b6f9b590df8..3758382c92c9 100644 --- a/app/controllers/work_packages/target_version_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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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/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 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/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/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 ) %>
    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/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/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/config/locales/en.yml b/config/locales/en.yml index da1d422a670e..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" @@ -946,6 +947,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." @@ -1637,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/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/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/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/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/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/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/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_versions.rb b/lib/open_project/journal_formatter/joined_association.rb similarity index 81% rename from lib/open_project/journal_formatter/joined_versions.rb rename to lib/open_project/journal_formatter/joined_association.rb index e9a8e681019f..9e1160ec3f52 100644 --- a/lib/open_project/journal_formatter/joined_versions.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/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/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]] } 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/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_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 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/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/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/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/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/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" } 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/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/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/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/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/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 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 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 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/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 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/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 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 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 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 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"