From 4cacf470ce949e356ae002a6a68a7ae6b0f2da81 Mon Sep 17 00:00:00 2001
From: Andreas Pfohl <a.pfohl@openproject.com>
Date: Fri, 11 Oct 2024 14:51:08 +0200
Subject: [PATCH 1/4] [58105] Added update_item and delete_branch methods to
 HierarchicalItemService

---
 .../hierarchy/update_item_contract.rb         |  61 ++++++++++
 app/models/custom_field.rb                    |   2 +-
 app/models/custom_field/hierarchy/item.rb     |   2 +-
 .../hierarchy/hierarchical_item_service.rb    |  28 ++++-
 .../hierarchy/update_item_contract_spec.rb    | 115 ++++++++++++++++++
 .../hierarchical_item_service_spec.rb         |  86 +++++++++++--
 6 files changed, 276 insertions(+), 18 deletions(-)
 create mode 100644 app/contracts/custom_fields/hierarchy/update_item_contract.rb
 create mode 100644 spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb

diff --git a/app/contracts/custom_fields/hierarchy/update_item_contract.rb b/app/contracts/custom_fields/hierarchy/update_item_contract.rb
new file mode 100644
index 000000000000..feff8ebd54f7
--- /dev/null
+++ b/app/contracts/custom_fields/hierarchy/update_item_contract.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.
+#++
+
+module CustomFields
+  module Hierarchy
+    class UpdateItemContract < Dry::Validation::Contract
+      params do
+        required(:item).filled
+        optional(:label).filled(:string)
+        optional(:short).filled(:string)
+      end
+
+      rule(:item) do
+        if value.is_a?(CustomField::Hierarchy::Item)
+          if !value.persisted?
+            key.failure("Item must exist")
+          elsif value.parent.nil?
+            key.failure("Item must not be a root item")
+          end
+        else
+          key.failure("Item must be of type 'Item'")
+        end
+      end
+
+      rule(:label) do
+        next unless key?
+
+        if CustomField::Hierarchy::Item.exists?(parent_id: values[:item].parent, label: value)
+          key.failure("Label must be unique within the same hierarchy level")
+        end
+      end
+    end
+  end
+end
diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb
index e8596a9e53e1..a3902325a1d7 100644
--- a/app/models/custom_field.rb
+++ b/app/models/custom_field.rb
@@ -43,7 +43,7 @@ class CustomField < ApplicationRecord
 
   has_one :hierarchy_root,
           class_name: "CustomField::Hierarchy::Item",
-          dependent: :delete, # todo: cascade into children with service
+          dependent: :destroy,
           inverse_of: "custom_field"
 
   acts_as_list scope: [:type]
diff --git a/app/models/custom_field/hierarchy/item.rb b/app/models/custom_field/hierarchy/item.rb
index 3ae4d4ff52a7..9c56113c2877 100644
--- a/app/models/custom_field/hierarchy/item.rb
+++ b/app/models/custom_field/hierarchy/item.rb
@@ -32,5 +32,5 @@ class CustomField::Hierarchy::Item < ApplicationRecord
   self.table_name = "hierarchical_items"
 
   belongs_to :custom_field
-  has_closure_tree order: "sort_order", numeric_order: true
+  has_closure_tree order: "sort_order", numeric_order: true, dependent: :delete_all
 end
diff --git a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
index d64ae9e79279..f7576103de51 100644
--- a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
+++ b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
@@ -55,7 +55,22 @@ def insert_item(parent:, label:, short: nil)
           .new
           .call({ parent:, label:, short: }.compact)
           .to_monad
-          .bind { |validation| create_child_item(validation) }
+          .bind { |validation| create_child_item(validation:) }
+      end
+
+      def update_item(item:, label: nil, short: nil)
+        CustomFields::Hierarchy::UpdateItemContract
+          .new
+          .call({ item:, label:, short: }.compact)
+          .to_monad
+          .bind { |attributes| update_item_attributes(item:, attributes:) }
+      end
+
+      def delete_branch(item:)
+        return Failure(:item_is_root) if item.root?
+
+        # CustomField::Hierarchy::Item sets "dependent: :destroy"
+        item.destroy ? Success() : Failure(item.errors)
       end
 
       private
@@ -67,13 +82,18 @@ def create_root_item
         Success(item)
       end
 
-      def create_child_item(validation)
-        item = CustomField::Hierarchy::Item
-                 .create(parent: validation[:parent], label: validation[:label], short: validation[:short])
+      def create_child_item(validation:)
+        item = validation[:parent].children.create(label: validation[:label], short: validation[:short])
         return Failure(item.errors) unless item.persisted?
 
         Success(item)
       end
+
+      def update_item_attributes(item:, attributes:)
+        item.update(label: attributes[:label], short: attributes[:short])
+
+        item.errors.empty? ? Success(item) : Failure(item.errors)
+      end
     end
   end
 end
diff --git a/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb
new file mode 100644
index 000000000000..e1d86fcc8fc3
--- /dev/null
+++ b/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb
@@ -0,0 +1,115 @@
+# 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 "rails_helper"
+
+RSpec.describe CustomFields::Hierarchy::UpdateItemContract do
+  subject { described_class.new }
+
+  # rubocop:disable Rails/DeprecatedActiveModelErrorsMethods
+  describe "#call" do
+    let(:vader) { create(:hierarchy_item) }
+    let(:luke) { create(:hierarchy_item, label: "luke", short: "ls", parent: vader) }
+    let(:leia) { create(:hierarchy_item, label: "leia", short: "lo", parent: vader) }
+
+    before do
+      luke
+      leia
+    end
+
+    context "when all required fields are valid" do
+      it "is valid" do
+        [
+          { item: luke, label: "Luke Skywalker", short: "LS" },
+          { item: luke, label: "Luke Skywalker" },
+          { item: luke, short: "LS" },
+          { item: luke, short: "lo" },
+          { item: luke }
+        ].each { |params| expect(subject.call(params)).to be_success }
+      end
+    end
+
+    context "when item is a root item" do
+      let(:params) { { item: vader } }
+
+      it("is invalid") do
+        result = subject.call(params)
+        expect(result).to be_failure
+        expect(result.errors.to_h).to include(item: ["Item must not be a root item"])
+      end
+    end
+
+    context "when item is not of type 'Item'" do
+      let(:invalid_item) { create(:custom_field) }
+      let(:params) { { item: invalid_item } }
+
+      it("is invalid") do
+        result = subject.call(params)
+        expect(result).to be_failure
+        expect(result.errors.to_h).to include(item: ["Item must be of type 'Item'"])
+      end
+    end
+
+    context "when item is not persisted" do
+      let(:item) { build(:hierarchy_item) }
+      let(:params) { { item: } }
+
+      it "is invalid" do
+        result = subject.call(params)
+        expect(result).to be_failure
+        expect(result.errors.to_h).to include(item: ["Item must exist"])
+      end
+    end
+
+    context "when the label already exist in the same hierarchy level" do
+      let(:params) { { item: luke, label: "leia" } }
+
+      it "is invalid" do
+        result = subject.call(params)
+        expect(result).to be_failure
+        expect(result.errors.to_h).to include(label: ["Label must be unique within the same hierarchy level"])
+      end
+    end
+
+    context "when fields are invalid" do
+      it "is invalid" do
+        [
+          {},
+          { item: nil },
+          { item: luke, label: nil },
+          { item: luke, label: 42 },
+          { item: luke, short: nil },
+          { item: luke, short: 42 }
+        ].each { |params| expect(subject.call(params)).to be_failure }
+      end
+    end
+  end
+  # rubocop:enable Rails/DeprecatedActiveModelErrorsMethods
+end
diff --git a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
index b5199522c2c5..919d940b1e91 100644
--- a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
+++ b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
@@ -28,16 +28,20 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
+require "dry/monads/all"
+
 require "rails_helper"
 
 RSpec.describe CustomFields::Hierarchy::HierarchicalItemService do
   let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: nil) }
   let(:invalid_custom_field) { create(:custom_field, field_format: "text", hierarchy_root: nil) }
 
+  subject { described_class.new(custom_field) }
+
   describe "#initialize" do
     context "with valid custom field" do
       it "initializes successfully" do
-        expect { described_class.new(custom_field) }.not_to raise_error
+        expect { subject }.not_to raise_error
       end
     end
 
@@ -49,11 +53,9 @@
   end
 
   describe "#generate_root" do
-    let(:service) { described_class.new(custom_field) }
-
     context "with valid hierarchy root" do
       it "creates a root item successfully" do
-        expect(service.generate_root).to be_success
+        expect(subject.generate_root).to be_success
       end
     end
 
@@ -63,7 +65,7 @@
           .to receive(:create)
                 .and_return(instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors"))
 
-        result = service.generate_root
+        result = subject.generate_root
         expect(result).to be_failure
       end
     end
@@ -71,7 +73,6 @@
 
   describe "#insert_item" do
     let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: parent) }
-    let(:service) { described_class.new(custom_field) }
 
     let(:parent) { create(:hierarchy_item) }
     let(:label) { "Child Item" }
@@ -79,23 +80,84 @@
 
     context "with valid parameters" do
       it "inserts an item successfully without short" do
-        result = service.insert_item(parent:, label:)
+        result = subject.insert_item(parent:, label:)
         expect(result).to be_success
       end
 
       it "inserts an item successfully with short" do
-        result = service.insert_item(parent:, label:, short:)
+        result = subject.insert_item(parent:, label:, short:)
         expect(result).to be_success
       end
     end
 
     context "with invalid item" do
       it "fails to insert an item" do
-        allow(CustomField::Hierarchy::Item)
-          .to receive(:create).and_return(instance_double(CustomField::Hierarchy::Item,
-                                                          persisted?: false, errors: "some errors"))
+        # rubocop:disable RSpec/VerifiedDoubles
+        children = double(create: instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors"))
+        # rubocop:enable RSpec/VerifiedDoubles
+
+        allow(parent).to receive(:children).and_return(children)
+
+        result = subject.insert_item(parent:, label:, short:)
+        expect(result).to be_failure
+      end
+    end
+  end
+
+  describe "#update_item" do
+    let(:items) do
+      Dry::Monads::Do.() do
+        root = Dry::Monads::Do.bind subject.generate_root
+        luke = Dry::Monads::Do.bind subject.insert_item(parent: root, label: "luke")
+        leia = Dry::Monads::Do.bind subject.insert_item(parent: root, label: "leia")
+
+        Dry::Monads::Success({ root:, luke:, leia: })
+      end
+    end
+
+    context "with valid parameters" do
+      it "updates the item with new attributes" do
+        result = subject.update_item(item: items.value![:luke], label: "Luke Skywalker", short: "LS")
+        expect(result).to be_success
+      end
+    end
+
+    context "with invalid parameters" do
+      it "refuses to update the item with new attributes" do
+        result = subject.update_item(item: items.value![:luke], label: "leia", short: "LS")
+        expect(result).to be_failure
+      end
+    end
+  end
+
+  describe "#delete_branch" do
+    let(:items) do
+      Dry::Monads::Do.() do
+        root = Dry::Monads::Do.bind subject.generate_root
+        luke = Dry::Monads::Do.bind subject.insert_item(parent: root, label: "luke")
+        leia = Dry::Monads::Do.bind subject.insert_item(parent: luke, label: "leia")
+
+        Dry::Monads::Success({ root:, luke:, leia: })
+      end
+    end
+
+    before do
+      items
+    end
+
+    context "with valid item to destroy" do
+      it "deletes the entire branch" do
+        result = subject.delete_branch(item: items.value![:luke])
+        expect(result).to be_success
+        expect(items.value![:luke]).to be_frozen
+        expect(CustomField::Hierarchy::Item.all).to be_one
+        expect(items.value![:root].reload.children).to be_empty
+      end
+    end
 
-        result = service.insert_item(parent:, label:, short:)
+    context "with root item" do
+      it "refuses to delete the item" do
+        result = subject.delete_branch(item: items.value![:root])
         expect(result).to be_failure
       end
     end

From 160b3e60321829bc12151bdd96fecff8355488da Mon Sep 17 00:00:00 2001
From: Marcello Rocha <marcello.rocha@gmail.com>
Date: Tue, 15 Oct 2024 16:40:37 +0200
Subject: [PATCH 2/4] Clean up some contracts adds get_branch

# Conflicts:
#	app/contracts/custom_fields/hierarchy/insert_item_contract.rb
#	spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
---
 .../hierarchy/generate_root_contract.rb       |  2 +-
 .../hierarchy/insert_item_contract.rb         | 10 +-
 .../hierarchy/update_item_contract.rb         | 15 +--
 app/models/custom_field/hierarchy/item.rb     |  2 +-
 .../hierarchy/hierarchical_item_service.rb    | 26 ++++-
 .../hierarchy/insert_item_contract_spec.rb    |  3 +-
 .../hierarchy/update_item_contract_spec.rb    | 10 +-
 .../hierarchical_item_service_spec.rb         | 97 +++++++++----------
 8 files changed, 84 insertions(+), 81 deletions(-)

diff --git a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
index 4e993e5f5b40..03ebff45ff62 100644
--- a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
+++ b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
@@ -32,7 +32,7 @@ module CustomFields
   module Hierarchy
     class GenerateRootContract < Dry::Validation::Contract
       params do
-        required(:hierarchy_root)
+        required(:hierarchy_root).value(:nil?)
       end
 
       rule(:hierarchy_root) do
diff --git a/app/contracts/custom_fields/hierarchy/insert_item_contract.rb b/app/contracts/custom_fields/hierarchy/insert_item_contract.rb
index fe656cd6efb3..d633761738c8 100644
--- a/app/contracts/custom_fields/hierarchy/insert_item_contract.rb
+++ b/app/contracts/custom_fields/hierarchy/insert_item_contract.rb
@@ -34,19 +34,13 @@ class InsertItemContract < Dry::Validation::Contract
       config.messages.backend = :i18n
 
       params do
-        required(:parent).filled
+        required(:parent).filled(type?: CustomField::Hierarchy::Item)
         required(:label).filled(:string)
         optional(:short).filled(:string)
       end
 
       rule(:parent) do
-        if value.is_a?(CustomField::Hierarchy::Item)
-          unless value.persisted?
-            key.failure("Parent must exist")
-          end
-        else
-          key.failure("Parent must be of type 'Item'")
-        end
+        key.failure("must exist") unless value.persisted?
       end
 
       rule(:label) do
diff --git a/app/contracts/custom_fields/hierarchy/update_item_contract.rb b/app/contracts/custom_fields/hierarchy/update_item_contract.rb
index feff8ebd54f7..e61289df2643 100644
--- a/app/contracts/custom_fields/hierarchy/update_item_contract.rb
+++ b/app/contracts/custom_fields/hierarchy/update_item_contract.rb
@@ -32,28 +32,21 @@ module CustomFields
   module Hierarchy
     class UpdateItemContract < Dry::Validation::Contract
       params do
-        required(:item).filled
+        required(:item).filled(type?: CustomField::Hierarchy::Item)
         optional(:label).filled(:string)
         optional(:short).filled(:string)
       end
 
       rule(:item) do
-        if value.is_a?(CustomField::Hierarchy::Item)
-          if !value.persisted?
-            key.failure("Item must exist")
-          elsif value.parent.nil?
-            key.failure("Item must not be a root item")
-          end
-        else
-          key.failure("Item must be of type 'Item'")
-        end
+        key.failure("must exist") unless value.persisted?
+        key.failure("must not be a root item") if value.root?
       end
 
       rule(:label) do
         next unless key?
 
         if CustomField::Hierarchy::Item.exists?(parent_id: values[:item].parent, label: value)
-          key.failure("Label must be unique within the same hierarchy level")
+          key.failure("must be unique at the same hierarchical level")
         end
       end
     end
diff --git a/app/models/custom_field/hierarchy/item.rb b/app/models/custom_field/hierarchy/item.rb
index 9c56113c2877..5be42dd0febd 100644
--- a/app/models/custom_field/hierarchy/item.rb
+++ b/app/models/custom_field/hierarchy/item.rb
@@ -32,5 +32,5 @@ class CustomField::Hierarchy::Item < ApplicationRecord
   self.table_name = "hierarchical_items"
 
   belongs_to :custom_field
-  has_closure_tree order: "sort_order", numeric_order: true, dependent: :delete_all
+  has_closure_tree order: "sort_order", numeric_order: true, dependent: :destroy
 end
diff --git a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
index f7576103de51..a0602c285dd1 100644
--- a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
+++ b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
@@ -35,9 +35,7 @@ class HierarchicalItemService
 
       def initialize(custom_field)
         validation = ServiceInitializationContract.new.call(field_format: custom_field.field_format)
-        # rubocop:disable Rails/DeprecatedActiveModelErrorsMethods
-        raise ArgumentError, "Invalid custom field: #{validation.errors.to_h}" if validation.failure?
-        # rubocop:enable Rails/DeprecatedActiveModelErrorsMethods
+        raise ArgumentError, "Invalid custom field: #{validation.errors(full: true).to_h}" if validation.failure?
 
         @custom_field = custom_field
       end
@@ -63,16 +61,34 @@ def update_item(item:, label: nil, short: nil)
           .new
           .call({ item:, label:, short: }.compact)
           .to_monad
-          .bind { |attributes| update_item_attributes(item:, attributes:) }
+          .fmap { |attributes| update_item_attributes(item:, attributes:) }
       end
 
       def delete_branch(item:)
         return Failure(:item_is_root) if item.root?
 
-        # CustomField::Hierarchy::Item sets "dependent: :destroy"
         item.destroy ? Success() : Failure(item.errors)
       end
 
+      def get_branch(item:)
+        Success(item.ancestors.reverse)
+      end
+
+      def move_item(item:, new_parent:, sort_order:)
+        # Move with all the children
+        raise NotImplementedError
+      end
+
+      def reorder_item(item:, new_sort_order:)
+        # move it around. Check closure_tree
+        raise NotImplementedError
+      end
+
+      def soft_delete_item(item)
+        # Soft delete the item and children?
+        raise NotImplementedError
+      end
+
       private
 
       def create_root_item
diff --git a/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
index 6fd825ce2591..58e5d9c0cfc6 100644
--- a/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
+++ b/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
@@ -53,7 +53,7 @@
       it "is invalid" do
         result = subject.call(params)
         expect(result).to be_failure
-        expect(result.errors.to_h).to include(parent: ["Parent must be of type 'Item'"])
+        expect(result.errors.to_h).to include(parent: ["must be CustomField::Hierarchy::Item"])
       end
     end
 
@@ -67,6 +67,7 @@
       it "is invalid" do
         result = subject.call(params)
         expect(result).to be_failure
+        expect(result.errors.to_h).to include(label: ["must be unique at the same hierarchical level"])
         expect(result.errors.to_h).to include(label: ["must be unique within the same hierarchy level"])
       end
 
diff --git a/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb
index e1d86fcc8fc3..eef698f84478 100644
--- a/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb
+++ b/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb
@@ -62,7 +62,7 @@
       it("is invalid") do
         result = subject.call(params)
         expect(result).to be_failure
-        expect(result.errors.to_h).to include(item: ["Item must not be a root item"])
+        expect(result.errors.to_h).to include(item: ["must not be a root item"])
       end
     end
 
@@ -73,18 +73,18 @@
       it("is invalid") do
         result = subject.call(params)
         expect(result).to be_failure
-        expect(result.errors.to_h).to include(item: ["Item must be of type 'Item'"])
+        expect(result.errors.to_h).to include(item: ["must be CustomField::Hierarchy::Item"])
       end
     end
 
     context "when item is not persisted" do
-      let(:item) { build(:hierarchy_item) }
+      let(:item) { build(:hierarchy_item, parent: vader) }
       let(:params) { { item: } }
 
       it "is invalid" do
         result = subject.call(params)
         expect(result).to be_failure
-        expect(result.errors.to_h).to include(item: ["Item must exist"])
+        expect(result.errors.to_h).to include(item: ["must exist"])
       end
     end
 
@@ -94,7 +94,7 @@
       it "is invalid" do
         result = subject.call(params)
         expect(result).to be_failure
-        expect(result.errors.to_h).to include(label: ["Label must be unique within the same hierarchy level"])
+        expect(result.errors.to_h).to include(label: ["must be unique at the same hierarchical level"])
       end
     end
 
diff --git a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
index 919d940b1e91..4b070f19cde5 100644
--- a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
+++ b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
@@ -28,20 +28,22 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-require "dry/monads/all"
-
-require "rails_helper"
+require "spec_helper"
 
 RSpec.describe CustomFields::Hierarchy::HierarchicalItemService do
   let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: nil) }
   let(:invalid_custom_field) { create(:custom_field, field_format: "text", hierarchy_root: nil) }
 
-  subject { described_class.new(custom_field) }
+  let!(:root) { service.generate_root.value! }
+  let!(:luke) { service.insert_item(parent: root, label: "luke").value! }
+  let!(:leia) { service.insert_item(parent: luke, label: "leia").value! }
+
+  subject(:service) { described_class.new(custom_field) }
 
   describe "#initialize" do
     context "with valid custom field" do
       it "initializes successfully" do
-        expect { subject }.not_to raise_error
+        expect { service }.not_to raise_error
       end
     end
 
@@ -55,7 +57,7 @@
   describe "#generate_root" do
     context "with valid hierarchy root" do
       it "creates a root item successfully" do
-        expect(subject.generate_root).to be_success
+        expect(service.generate_root).to be_success
       end
     end
 
@@ -65,101 +67,98 @@
           .to receive(:create)
                 .and_return(instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors"))
 
-        result = subject.generate_root
+        result = service.generate_root
         expect(result).to be_failure
       end
     end
   end
 
   describe "#insert_item" do
-    let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: parent) }
-
-    let(:parent) { create(:hierarchy_item) }
     let(:label) { "Child Item" }
     let(:short) { "Short Description" }
 
     context "with valid parameters" do
       it "inserts an item successfully without short" do
-        result = subject.insert_item(parent:, label:)
+        result = service.insert_item(parent: root, label:)
         expect(result).to be_success
       end
 
       it "inserts an item successfully with short" do
-        result = subject.insert_item(parent:, label:, short:)
+        result = service.insert_item(parent: root, label:, short:)
         expect(result).to be_success
       end
     end
 
     context "with invalid item" do
       it "fails to insert an item" do
-        # rubocop:disable RSpec/VerifiedDoubles
-        children = double(create: instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors"))
-        # rubocop:enable RSpec/VerifiedDoubles
-
-        allow(parent).to receive(:children).and_return(children)
+        child = instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors")
+        allow(root.children).to receive(:create).and_return(child)
 
-        result = subject.insert_item(parent:, label:, short:)
+        result = service.insert_item(parent: root, label:, short:)
         expect(result).to be_failure
       end
     end
   end
 
   describe "#update_item" do
-    let(:items) do
-      Dry::Monads::Do.() do
-        root = Dry::Monads::Do.bind subject.generate_root
-        luke = Dry::Monads::Do.bind subject.insert_item(parent: root, label: "luke")
-        leia = Dry::Monads::Do.bind subject.insert_item(parent: root, label: "leia")
-
-        Dry::Monads::Success({ root:, luke:, leia: })
-      end
-    end
-
     context "with valid parameters" do
       it "updates the item with new attributes" do
-        result = subject.update_item(item: items.value![:luke], label: "Luke Skywalker", short: "LS")
+        result = service.update_item(item: luke, label: "Luke Skywalker", short: "LS")
         expect(result).to be_success
       end
     end
 
     context "with invalid parameters" do
+      let!(:leia) { service.insert_item(parent: root, label: "leia").value! }
+
       it "refuses to update the item with new attributes" do
-        result = subject.update_item(item: items.value![:luke], label: "leia", short: "LS")
+        result = service.update_item(item: luke, label: "leia", short: "LS")
         expect(result).to be_failure
+
+        errors = result.failure.errors
+        expect(errors.to_h).to eq({ label: ["must be unique at the same hierarchical level"] })
       end
     end
   end
 
   describe "#delete_branch" do
-    let(:items) do
-      Dry::Monads::Do.() do
-        root = Dry::Monads::Do.bind subject.generate_root
-        luke = Dry::Monads::Do.bind subject.insert_item(parent: root, label: "luke")
-        leia = Dry::Monads::Do.bind subject.insert_item(parent: luke, label: "leia")
-
-        Dry::Monads::Success({ root:, luke:, leia: })
-      end
-    end
-
-    before do
-      items
-    end
-
     context "with valid item to destroy" do
       it "deletes the entire branch" do
-        result = subject.delete_branch(item: items.value![:luke])
+        result = service.delete_branch(item: luke)
         expect(result).to be_success
-        expect(items.value![:luke]).to be_frozen
-        expect(CustomField::Hierarchy::Item.all).to be_one
-        expect(items.value![:root].reload.children).to be_empty
+        expect(luke).to be_frozen
+        expect(CustomField::Hierarchy::Item.count).to eq(1)
+        expect(root.reload.children).to be_empty
       end
     end
 
     context "with root item" do
       it "refuses to delete the item" do
-        result = subject.delete_branch(item: items.value![:root])
+        result = service.delete_branch(item: root)
         expect(result).to be_failure
       end
     end
   end
+
+  describe "#get_branch" do
+    context "with a non-root node" do
+      it "returns all the ancestors to that item" do
+        result = service.get_branch(item: leia)
+        expect(result).to be_success
+
+        ancestors = result.value!
+        expect(ancestors.size).to eq(2)
+        expect(ancestors).to contain_exactly(root, luke)
+        expect(ancestors.last).to eq(luke)
+      end
+    end
+
+    context "with a root node" do
+      it "returns a empty list" do
+        result = service.get_branch(item: root)
+        expect(result).to be_success
+        expect(result.value!).to be_empty
+      end
+    end
+  end
 end

From 80edc9e04b9212af2b6bf0edabe97063ac60f472 Mon Sep 17 00:00:00 2001
From: Marcello Rocha <marcello.rocha@gmail.com>
Date: Tue, 15 Oct 2024 18:28:20 +0200
Subject: [PATCH 3/4] Update specs, contracts, draft inplementation of the
 missing actions

---
 .../hierarchy/generate_root_contract.rb       |  4 ---
 .../hierarchy/update_item_contract.rb         |  4 +--
 .../hierarchy/hierarchical_item_service.rb    | 26 ++++++++-------
 .../hierarchy/generate_root_contract_spec.rb  |  2 +-
 .../hierarchy/insert_item_contract_spec.rb    |  1 -
 .../hierarchical_item_service_spec.rb         | 32 +++++++++++++++++--
 6 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
index 03ebff45ff62..6dafd9cdf3e8 100644
--- a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
+++ b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
@@ -34,10 +34,6 @@ class GenerateRootContract < Dry::Validation::Contract
       params do
         required(:hierarchy_root).value(:nil?)
       end
-
-      rule(:hierarchy_root) do
-        key.failure("Hierarchical root already set") unless value.nil?
-      end
     end
   end
 end
diff --git a/app/contracts/custom_fields/hierarchy/update_item_contract.rb b/app/contracts/custom_fields/hierarchy/update_item_contract.rb
index e61289df2643..b78f45369eb6 100644
--- a/app/contracts/custom_fields/hierarchy/update_item_contract.rb
+++ b/app/contracts/custom_fields/hierarchy/update_item_contract.rb
@@ -38,14 +38,14 @@ class UpdateItemContract < Dry::Validation::Contract
       end
 
       rule(:item) do
-        key.failure("must exist") unless value.persisted?
+        key.failure("must exist") if value.new_record?
         key.failure("must not be a root item") if value.root?
       end
 
       rule(:label) do
         next unless key?
 
-        if CustomField::Hierarchy::Item.exists?(parent_id: values[:item].parent, label: value)
+        if CustomField::Hierarchy::Item.exists?(parent_id: values[:item].parent_id, label: value)
           key.failure("must be unique at the same hierarchical level")
         end
       end
diff --git a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
index a0602c285dd1..8625107355ca 100644
--- a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
+++ b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
@@ -34,6 +34,7 @@ class HierarchicalItemService
       include Dry::Monads[:result]
 
       def initialize(custom_field)
+        # Is this overkill? We are only checking `custom_field.field_format`
         validation = ServiceInitializationContract.new.call(field_format: custom_field.field_format)
         raise ArgumentError, "Invalid custom field: #{validation.errors(full: true).to_h}" if validation.failure?
 
@@ -61,7 +62,7 @@ def update_item(item:, label: nil, short: nil)
           .new
           .call({ item:, label:, short: }.compact)
           .to_monad
-          .fmap { |attributes| update_item_attributes(item:, attributes:) }
+          .bind { |attributes| update_item_attributes(item:, attributes:) }
       end
 
       def delete_branch(item:)
@@ -74,18 +75,17 @@ def get_branch(item:)
         Success(item.ancestors.reverse)
       end
 
-      def move_item(item:, new_parent:, sort_order:)
-        # Move with all the children
-        raise NotImplementedError
+      def move_item(item:, new_parent:)
+        Success(new_parent.append_child(item))
       end
 
       def reorder_item(item:, new_sort_order:)
-        # move it around. Check closure_tree
-        raise NotImplementedError
+        old_item = item.siblings.where(sort_order: new_sort_order).first
+        old_item.prepend_sibling(item)
       end
 
       def soft_delete_item(item)
-        # Soft delete the item and children?
+        # Soft delete the item and children
         raise NotImplementedError
       end
 
@@ -93,22 +93,24 @@ def soft_delete_item(item)
 
       def create_root_item
         item = CustomField::Hierarchy::Item.create(custom_field: @custom_field)
-        return Failure(item.errors) unless item.persisted?
+        return Failure(item.errors) if item.new_record?
 
         Success(item)
       end
 
       def create_child_item(validation:)
         item = validation[:parent].children.create(label: validation[:label], short: validation[:short])
-        return Failure(item.errors) unless item.persisted?
+        return Failure(item.errors) if item.new_record?
 
         Success(item)
       end
 
       def update_item_attributes(item:, attributes:)
-        item.update(label: attributes[:label], short: attributes[:short])
-
-        item.errors.empty? ? Success(item) : Failure(item.errors)
+        if item.update(label: attributes[:label], short: attributes[:short])
+          Success(item)
+        else
+          Failure(item.errors)
+        end
       end
     end
   end
diff --git a/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb
index 78429458a2d9..6b484c101001 100644
--- a/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb
+++ b/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb
@@ -51,7 +51,7 @@
         result = subject.call(hierarchy_root: custom_field.hierarchy_root)
         expect(result).to be_failure
         # rubocop:disable Rails/DeprecatedActiveModelErrorsMethods
-        expect(result.errors.to_h).to include(hierarchy_root: ["Hierarchical root already set"])
+        expect(result.errors.to_h).to include(hierarchy_root: ["cannot be defined"])
         # rubocop:enable Rails/DeprecatedActiveModelErrorsMethods
       end
     end
diff --git a/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
index 58e5d9c0cfc6..900e519481c6 100644
--- a/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
+++ b/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb
@@ -67,7 +67,6 @@
       it "is invalid" do
         result = subject.call(params)
         expect(result).to be_failure
-        expect(result.errors.to_h).to include(label: ["must be unique at the same hierarchical level"])
         expect(result.errors.to_h).to include(label: ["must be unique within the same hierarchy level"])
       end
 
diff --git a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
index 4b070f19cde5..7bac82f32470 100644
--- a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
+++ b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
@@ -65,7 +65,7 @@
       it "fails to create a root item" do
         allow(CustomField::Hierarchy::Item)
           .to receive(:create)
-                .and_return(instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors"))
+                .and_return(instance_double(CustomField::Hierarchy::Item, new_record?: true, errors: "some errors"))
 
         result = service.generate_root
         expect(result).to be_failure
@@ -91,7 +91,7 @@
 
     context "with invalid item" do
       it "fails to insert an item" do
-        child = instance_double(CustomField::Hierarchy::Item, persisted?: false, errors: "some errors")
+        child = instance_double(CustomField::Hierarchy::Item, new_record?: true, errors: "some errors")
         allow(root.children).to receive(:create).and_return(child)
 
         result = service.insert_item(parent: root, label:, short:)
@@ -161,4 +161,32 @@
       end
     end
   end
+
+  describe "#move_item" do
+    let(:lando) { service.insert_item(parent: root, label: "lando").value! }
+
+    it "move the node to the new parent" do
+      expect { service.move_item(item: leia, new_parent: lando) }.to change { leia.reload.ancestors.first }.to(lando)
+    end
+
+    it "all child nodes follow" do
+      service.move_item(item: luke, new_parent: lando)
+
+      expect(luke.reload.ancestors).to contain_exactly(root, lando)
+      expect(leia.reload.ancestors).to contain_exactly(root, lando, luke)
+    end
+  end
+
+  describe "reorder_item" do
+    let!(:lando) { service.insert_item(parent: root, label: "lando").value! }
+    let!(:chewbacca) { service.insert_item(parent: root, label: "AWOOO").value! }
+
+    it "moves the note to the requested position" do
+      service.reorder_item(item: chewbacca, new_sort_order: 1)
+
+      expect(luke.reload.sort_order).to eq(0)
+      expect(chewbacca.reload.sort_order).to eq(1)
+      expect(lando.reload.sort_order).to eq(2)
+    end
+  end
 end

From 9670a166e7effb8f6cab6014ae53387e2c3267e7 Mon Sep 17 00:00:00 2001
From: Marcello Rocha <marcello.rocha@gmail.com>
Date: Wed, 16 Oct 2024 18:04:19 +0200
Subject: [PATCH 4/4] Remove instance variable, adds some docs, wraps some
 return values.

---
 .../hierarchy/generate_root_contract.rb       |  7 +-
 .../service_initialization_contract.rb        | 43 --------
 .../hierarchy/items_controller.rb             |  2 +-
 app/services/custom_fields/create_service.rb  |  2 +-
 .../hierarchy/hierarchical_item_service.rb    | 48 ++++++---
 .../hierarchy/generate_root_contract_spec.rb  | 14 ++-
 .../service_initialization_contract_spec.rb   | 97 -------------------
 .../hierarchical_item_service_spec.rb         | 31 +++---
 8 files changed, 60 insertions(+), 184 deletions(-)
 delete mode 100644 app/contracts/custom_fields/hierarchy/service_initialization_contract.rb
 delete mode 100644 spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb

diff --git a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
index 6dafd9cdf3e8..137d13ab0c23 100644
--- a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
+++ b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb
@@ -32,7 +32,12 @@ module CustomFields
   module Hierarchy
     class GenerateRootContract < Dry::Validation::Contract
       params do
-        required(:hierarchy_root).value(:nil?)
+        required(:custom_field).filled(type?: CustomField)
+      end
+
+      rule(:custom_field) do
+        key.failure("must have field format 'hierarchy'") if value.field_format != "hierarchy"
+        key.failure("cannot be defined") if value.hierarchy_root.present?
       end
     end
   end
diff --git a/app/contracts/custom_fields/hierarchy/service_initialization_contract.rb b/app/contracts/custom_fields/hierarchy/service_initialization_contract.rb
deleted file mode 100644
index 8b72a534ba20..000000000000
--- a/app/contracts/custom_fields/hierarchy/service_initialization_contract.rb
+++ /dev/null
@@ -1,43 +0,0 @@
-# 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 CustomFields
-  module Hierarchy
-    class ServiceInitializationContract < Dry::Validation::Contract
-      params do
-        required(:field_format).filled(:string)
-      end
-
-      rule(:field_format) do
-        key.failure("Custom field must have field format 'hierarchy'") if value != "hierarchy"
-      end
-    end
-  end
-end
diff --git a/app/controllers/admin/custom_fields/hierarchy/items_controller.rb b/app/controllers/admin/custom_fields/hierarchy/items_controller.rb
index 416e71f6e9f3..c8fa3440e330 100644
--- a/app/controllers/admin/custom_fields/hierarchy/items_controller.rb
+++ b/app/controllers/admin/custom_fields/hierarchy/items_controller.rb
@@ -53,7 +53,7 @@ def new
 
         def create
           ::CustomFields::Hierarchy::HierarchicalItemService
-            .new(@custom_field)
+            .new
             .insert_item(**item_input)
             .either(
               ->(_) { update_via_turbo_stream(component: ItemsComponent.new(custom_field: @custom_field)) },
diff --git a/app/services/custom_fields/create_service.rb b/app/services/custom_fields/create_service.rb
index 6919f05b247e..314ea55e46e6 100644
--- a/app/services/custom_fields/create_service.rb
+++ b/app/services/custom_fields/create_service.rb
@@ -57,7 +57,7 @@ def after_perform(call)
       if cf.is_a?(ProjectCustomField)
         add_cf_to_visible_columns(cf)
       elsif cf.field_format_hierarchy?
-        CustomFields::Hierarchy::HierarchicalItemService.new(cf).generate_root
+        CustomFields::Hierarchy::HierarchicalItemService.new.generate_root(cf)
       end
 
       call
diff --git a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
index 8625107355ca..0e595505abbc 100644
--- a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
+++ b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb
@@ -33,22 +33,22 @@ module Hierarchy
     class HierarchicalItemService
       include Dry::Monads[:result]
 
-      def initialize(custom_field)
-        # Is this overkill? We are only checking `custom_field.field_format`
-        validation = ServiceInitializationContract.new.call(field_format: custom_field.field_format)
-        raise ArgumentError, "Invalid custom field: #{validation.errors(full: true).to_h}" if validation.failure?
-
-        @custom_field = custom_field
-      end
-
-      def generate_root
+      # Generate the root item for the CustomField of type hierarchy
+      # @param custom_field [CustomField] custom field of type hierarchy
+      # @return [Success(CustomField::Hierarchy::Item), Failure(Dry::Validation::Result), Failure(ActiveModel::Errors)]
+      def generate_root(custom_field)
         CustomFields::Hierarchy::GenerateRootContract
           .new
-          .call(hierarchy_root: @custom_field.hierarchy_root)
+          .call(custom_field:)
           .to_monad
-          .bind { create_root_item }
+          .bind { |validation| create_root_item(validation[:custom_field]) }
       end
 
+      # Insert a new node on the hierarchy tree.
+      # @param parent [CustomField::Hierarchy::Item] the parent of the node
+      # @param label [String] the node label/name that must be unique at the same tree level
+      # @param short [String] an alias for the node
+      # @return [Success(CustomField::Hierarchy::Item), Failure(Dry::Validation::Result), Failure(ActiveModel::Errors)]
       def insert_item(parent:, label:, short: nil)
         CustomFields::Hierarchy::InsertItemContract
           .new
@@ -57,6 +57,11 @@ def insert_item(parent:, label:, short: nil)
           .bind { |validation| create_child_item(validation:) }
       end
 
+      # Updates an item/node
+      # @param item [CustomField::Hierarchy::Item] the item to be updated
+      # @param label [String] the node label/name that must be unique at the same tree level
+      # @param short [String] an alias for the node
+      # @return [Success(CustomField::Hierarchy::Item), Failure(Dry::Validation::Result), Failure(ActiveModel::Errors)]
       def update_item(item:, label: nil, short: nil)
         CustomFields::Hierarchy::UpdateItemContract
           .new
@@ -65,23 +70,38 @@ def update_item(item:, label: nil, short: nil)
           .bind { |attributes| update_item_attributes(item:, attributes:) }
       end
 
+      # Delete an entire branch of the hierarchy/tree
+      # @param item [CustomField::Hierarchy::Item] the parent of the node
+      # @return [Success(CustomField::Hierarchy::Item), Failure(Symbol), Failure(ActiveModel::Errors)]
       def delete_branch(item:)
         return Failure(:item_is_root) if item.root?
 
         item.destroy ? Success() : Failure(item.errors)
       end
 
+      # Gets all nodes in a tree from the item/node back to the root.
+      # Ordered from root to leaf
+      # @param item [CustomField::Hierarchy::Item] the parent of the node
+      # @return [Success(Array<CustomField::Hierarchy::Item>)]
       def get_branch(item:)
         Success(item.ancestors.reverse)
       end
 
+      # Move an item/node to a new parent item/node
+      # @param item [CustomField::Hierarchy::Item] the parent of the node
+      # @param new_parent [CustomField::Hierarchy::Item] the new parent of the node
+      # @return [Success(CustomField::Hierarchy::Item)]
       def move_item(item:, new_parent:)
         Success(new_parent.append_child(item))
       end
 
+      # Reorder the item along its siblings.
+      # @param item [CustomField::Hierarchy::Item] the parent of the node
+      # @param new_sort_order [Integer] the new parent of the node
+      # @return [Success(CustomField::Hierarchy::Item)]
       def reorder_item(item:, new_sort_order:)
         old_item = item.siblings.where(sort_order: new_sort_order).first
-        old_item.prepend_sibling(item)
+        Success(old_item.prepend_sibling(item))
       end
 
       def soft_delete_item(item)
@@ -91,8 +111,8 @@ def soft_delete_item(item)
 
       private
 
-      def create_root_item
-        item = CustomField::Hierarchy::Item.create(custom_field: @custom_field)
+      def create_root_item(custom_field)
+        item = CustomField::Hierarchy::Item.create(custom_field.hierarchy_root)
         return Failure(item.errors) if item.new_record?
 
         Success(item)
diff --git a/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb
index 6b484c101001..6089fe297420 100644
--- a/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb
+++ b/spec/contracts/custom_fields/hierarchy/generate_root_contract_spec.rb
@@ -38,7 +38,7 @@
       let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: nil) }
 
       it "is valid" do
-        result = subject.call(hierarchy_root: custom_field.hierarchy_root)
+        result = subject.call(custom_field:)
         expect(result).to be_success
       end
     end
@@ -48,19 +48,17 @@
       let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root:) }
 
       it "is invalid" do
-        result = subject.call(hierarchy_root: custom_field.hierarchy_root)
+        result = subject.call(custom_field:)
         expect(result).to be_failure
-        # rubocop:disable Rails/DeprecatedActiveModelErrorsMethods
-        expect(result.errors.to_h).to include(hierarchy_root: ["cannot be defined"])
-        # rubocop:enable Rails/DeprecatedActiveModelErrorsMethods
+        expect(result.errors[:custom_field]).to match_array("cannot be defined")
       end
     end
 
     context "when inputs are valid" do
+      let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: nil) }
+
       it "creates a success result" do
-        [
-          { hierarchy_root: nil }
-        ].each { |params| expect(subject.call(params)).to be_success }
+        expect(subject.call(custom_field:)).to be_success
       end
     end
 
diff --git a/spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb
deleted file mode 100644
index 2cf1979cc78d..000000000000
--- a/spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb
+++ /dev/null
@@ -1,97 +0,0 @@
-# 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 "rails_helper"
-
-RSpec.describe CustomFields::Hierarchy::ServiceInitializationContract do
-  subject { described_class.new }
-
-  # rubocop:disable Rails/DeprecatedActiveModelErrorsMethods
-  describe "#call" do
-    context "when field_format is 'hierarchy'" do
-      let(:params) { { field_format: "hierarchy" } }
-
-      it "is valid" do
-        result = subject.call(params)
-        expect(result).to be_success
-      end
-    end
-
-    context "when field_format is not 'hierarchy'" do
-      let(:params) { { field_format: "text" } }
-
-      it "is invalid" do
-        result = subject.call(params)
-        expect(result).to be_failure
-        expect(result.errors.to_h).to include(field_format: ["Custom field must have field format 'hierarchy'"])
-      end
-    end
-
-    context "when field_format is missing" do
-      let(:params) { {} }
-
-      it "is invalid" do
-        result = subject.call(params)
-        expect(result).to be_failure
-        expect(result.errors.to_h).to include(field_format: ["is missing"])
-      end
-    end
-
-    context "when field_format is nil" do
-      let(:params) { { field_format: nil } }
-
-      it "is invalid" do
-        result = subject.call(params)
-        expect(result).to be_failure
-        expect(result.errors.to_h).to include(field_format: ["must be filled"])
-      end
-    end
-
-    context "when inputs are valid" do
-      it "creates a success result" do
-        [
-          { field_format: "hierarchy" }
-        ].each { |params| expect(subject.call(params)).to be_success }
-      end
-    end
-
-    context "when inputs are invalid" do
-      it "creates a failure result" do
-        [
-          {},
-          { field_format: "text" },
-          { field_format: nil },
-          { field_format: 42 }
-        ].each { |params| expect(subject.call(params)).to be_failure }
-      end
-    end
-  end
-  # rubocop:enable Rails/DeprecatedActiveModelErrorsMethods
-end
diff --git a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
index 7bac82f32470..73e6d019d00c 100644
--- a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
+++ b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb
@@ -34,40 +34,32 @@
   let(:custom_field) { create(:custom_field, field_format: "hierarchy", hierarchy_root: nil) }
   let(:invalid_custom_field) { create(:custom_field, field_format: "text", hierarchy_root: nil) }
 
-  let!(:root) { service.generate_root.value! }
+  let!(:root) { service.generate_root(custom_field).value! }
   let!(:luke) { service.insert_item(parent: root, label: "luke").value! }
   let!(:leia) { service.insert_item(parent: luke, label: "leia").value! }
 
-  subject(:service) { described_class.new(custom_field) }
-
-  describe "#initialize" do
-    context "with valid custom field" do
-      it "initializes successfully" do
-        expect { service }.not_to raise_error
-      end
-    end
-
-    context "with invalid custom field" do
-      it "raises an ArgumentError" do
-        expect { described_class.new(invalid_custom_field) }.to raise_error(ArgumentError, /Invalid custom field/)
-      end
-    end
-  end
+  subject(:service) { described_class.new }
 
   describe "#generate_root" do
     context "with valid hierarchy root" do
       it "creates a root item successfully" do
-        expect(service.generate_root).to be_success
+        expect(service.generate_root(custom_field)).to be_success
       end
     end
 
+    it "requires a custom field of type hierarchy" do
+      result = service.generate_root(invalid_custom_field).failure
+
+      expect(result.errors[:custom_field]).to eq(["must have field format 'hierarchy'"])
+    end
+
     context "with persistence of hierarchy root fails" do
       it "fails to create a root item" do
         allow(CustomField::Hierarchy::Item)
           .to receive(:create)
                 .and_return(instance_double(CustomField::Hierarchy::Item, new_record?: true, errors: "some errors"))
 
-        result = service.generate_root
+        result = service.generate_root(custom_field)
         expect(result).to be_failure
       end
     end
@@ -79,8 +71,9 @@
 
     context "with valid parameters" do
       it "inserts an item successfully without short" do
-        result = service.insert_item(parent: root, label:)
+        result = service.insert_item(parent: luke, label:)
         expect(result).to be_success
+        expect(luke.reload.children.count).to eq(2)
       end
 
       it "inserts an item successfully with short" do