Merge pull request #17387 from opf/bugfix/hierarchy-field-journals

Adds handling of Hierarchy fields to the JournalFormatters

app/models/custom_field/hierarchy/item.rb

@@ -37,4 +37,8 @@ class CustomField::Hierarchy::Item < ApplicationRecord
   scope :including_children, -> { includes(children: :children) }
 
   def to_s = short.nil? ? label : "#{label} (#{short})"
+
+  def ancestry_path
+    self_and_ancestors.filter_map(&:to_s).reverse.join(" / ")
+  end
 end

lib/open_project/journal_formatter/custom_field/plain.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) the OpenProject GmbH
@@ -53,6 +55,8 @@ def get_formatted_values(custom_field, values)
 
   def get_modifier_function(custom_field)
     case custom_field.field_format
+    when "hierarchy"
+      :find_item_value
     when "list"
       :find_list_value
     when "user"
@@ -79,13 +83,23 @@ def find_user_value(value, _custom_field)
 
     # Lookup any visible user we can find
     user_lookup = Principal
-      .in_visible_project_or_me(User.current)
-      .where(id: ids)
-      .index_by(&:id)
+                  .in_visible_project_or_me(User.current)
+                  .where(id: ids)
+                  .index_by(&:id)
 
     ids_to_names(ids, user_lookup)
   end
 
+  def find_item_value(value, _custom_field)
+    ids = value.split(",").map(&:to_i)
+
+    CustomField::Hierarchy::Item.where(id: ids).map do |item|
+      next I18n.t(:label_deleted_custom_option) unless ids.include?(item.id)
+
+      item.ancestry_path
+    end.join(", ")
+  end
+
   def find_list_value(value, custom_field)
     ids = value.split(",").map(&:to_i)
 
@@ -105,9 +119,9 @@ def find_version_value(value, _custom_field)
 
     # Lookup visible versions we can find
     version_lookup = Version
-      .visible(User.current)
-      .where(id: ids)
-      .index_by(&:id)
+                     .visible(User.current)
+                     .where(id: ids)
+                     .index_by(&:id)
 
     ids_to_names(ids, version_lookup)
   end

spec/lib/journal_formatter/custom_field_spec.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 #-- copyright
 # OpenProject is an open source project management software.
 # Copyright (C) the OpenProject GmbH
@@ -50,8 +52,8 @@ def url_helper = Rails.application.routes.url_helpers
     allow(CustomField).to receive(:find_by).and_return(nil)
     allow(CustomField)
       .to receive(:find_by)
-            .with(id: custom_field.id)
-            .and_return(custom_field)
+      .with(id: custom_field.id)
+      .and_return(custom_field)
   end
 
   context "with html requested by default" do
@@ -196,8 +198,8 @@ def url_helper = Rails.application.routes.url_helpers
 
       allow(wherestub)
         .to receive(:where)
-              .with(id: [user1.id, user2.id])
-              .and_return(visible_users)
+        .with(id: [user1.id, user2.id])
+        .and_return(visible_users)
     end
 
     describe "with two visible users" do
@@ -248,37 +250,37 @@ def url_helper = Rails.application.routes.url_helpers
 
       allow(custom_field)
         .to receive(:custom_options)
-              .and_return(cf_options)
+        .and_return(cf_options)
 
       allow(cf_options)
         .to receive(:where)
-              .with(id: [1, 2])
-              .and_return(old_options)
+        .with(id: [1, 2])
+        .and_return(old_options)
 
       allow(cf_options)
         .to receive(:where)
-              .with(id: [3, 4])
-              .and_return(new_options)
+        .with(id: [3, 4])
+        .and_return(new_options)
 
       allow(old_options)
         .to receive(:order)
-              .with(:position)
-              .and_return(old_options)
+        .with(:position)
+        .and_return(old_options)
 
       allow(new_options)
         .to receive(:order)
-              .with(:position)
-              .and_return(new_options)
+        .with(:position)
+        .and_return(new_options)
 
       allow(old_options)
         .to receive(:pluck)
-              .with(:id, :value)
-              .and_return(old_custom_option_names)
+        .with(:id, :value)
+        .and_return(old_custom_option_names)
 
       allow(new_options)
         .to receive(:pluck)
-              .with(:id, :value)
-              .and_return(new_custom_option_names)
+        .with(:id, :value)
+        .and_return(new_custom_option_names)
     end
 
     describe "with both values being a comma separated list of ids" do
@@ -395,4 +397,78 @@ def url_helper = Rails.application.routes.url_helpers
       end
     end
   end
+
+  context "for hierarchy custom field" do
+    shared_let(:custom_field) { create(:hierarchy_wp_custom_field) }
+    shared_let(:service) { CustomFields::Hierarchy::HierarchicalItemService.new }
+    shared_let(:root) { service.generate_root(custom_field).value! }
+    shared_let(:luke) { service.insert_item(parent: root, label: "luke", short: "LS").value! }
+    shared_let(:mara) { service.insert_item(parent: luke, label: "mara").value! }
+
+    describe "first value being nil and second value a string" do
+      let(:values) { [nil, mara.id.to_s] }
+      let(:formatted_value) { mara.ancestry_path }
+      let(:expected) do
+        I18n.t(:text_journal_set_to,
+               label: "<strong>#{custom_field.name}</strong>",
+               value: "<i>#{formatted_value}</i>")
+      end
+
+      it { expect(rendered).to be_html_eql(expected) }
+    end
+
+    describe "first value being a string and second value being nil" do
+      let(:values) { [mara.id.to_s, nil] }
+      let(:formatted_value) { mara.ancestry_path }
+      let(:expected) do
+        I18n.t(:text_journal_deleted,
+               label: "<strong>#{custom_field.name}</strong>",
+               old: "<strike><i>#{formatted_value}</i></strike>")
+      end
+
+      it { expect(rendered).to be_html_eql(expected) }
+    end
+
+    context "with multiple values" do
+      describe "first value being nil and second value a string" do
+        let(:values) { [nil, [luke.id, mara.id].join(",")] }
+        let(:formatted_value) { [luke.ancestry_path, mara.ancestry_path].join(", ") }
+        let(:expected) do
+          I18n.t(:text_journal_set_to,
+                 label: "<strong>#{custom_field.name}</strong>",
+                 value: "<i>#{formatted_value}</i>")
+        end
+
+        it { expect(rendered).to be_html_eql(expected) }
+      end
+
+      describe "first value being a string and second value being nil" do
+        let(:values) { [[luke.id, mara.id].join(","), nil] }
+        let(:formatted_value) { [luke.ancestry_path, mara.ancestry_path].join(", ") }
+        let(:expected) do
+          I18n.t(:text_journal_deleted,
+                 label: "<strong>#{custom_field.name}</strong>",
+                 old: "<strike><i>#{formatted_value}</i></strike>")
+        end
+
+        it { expect(rendered).to be_html_eql(expected) }
+      end
+
+      describe "both values being strings" do
+        let(:values) { [[luke.id, mara.id].join(","), luke.id.to_s] }
+        let(:original_value) { [luke.ancestry_path, mara.ancestry_path].join(", ") }
+        let(:formatted_value) { luke.ancestry_path }
+
+        let(:expected) do
+          I18n.t(:text_journal_changed_plain,
+                 label: "<strong>#{custom_field.name}</strong>",
+                 linebreak: "",
+                 old: "<i>#{original_value}</i>",
+                 new: "<i>#{formatted_value}</i>")
+        end
+
+        it { expect(rendered).to be_html_eql(expected) }
+      end
+    end
+  end
 end