Remove instance variable, adds some docs, wraps some return values.

app/contracts/custom_fields/hierarchy/generate_root_contract.rb

@@ -32,7 +32,11 @@ 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"
       end
     end
   end

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)) },

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

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)

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