diff --git a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb index 6dafd9cdf3e8..02a6f350c941 100644 --- a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb +++ b/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 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)] 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/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