poc: Multiple Categories

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

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)

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

app/controllers/work_packages/target_version_normalization.rb → 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

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

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)

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,15 +53,52 @@ 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)
     name <=> other.name
   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

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
 

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

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,

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