diff --git a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb index 4e993e5f5b40..137d13ab0c23 100644 --- a/app/contracts/custom_fields/hierarchy/generate_root_contract.rb +++ b/app/contracts/custom_fields/hierarchy/generate_root_contract.rb @@ -32,11 +32,12 @@ module CustomFields module Hierarchy class GenerateRootContract < Dry::Validation::Contract params do - required(:hierarchy_root) + required(:custom_field).filled(type?: CustomField) end - rule(:hierarchy_root) do - key.failure("Hierarchical root already set") unless value.nil? + 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/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/service_initialization_contract.rb b/app/contracts/custom_fields/hierarchy/update_item_contract.rb similarity index 69% rename from app/contracts/custom_fields/hierarchy/service_initialization_contract.rb rename to app/contracts/custom_fields/hierarchy/update_item_contract.rb index 8b72a534ba20..b78f45369eb6 100644 --- a/app/contracts/custom_fields/hierarchy/service_initialization_contract.rb +++ b/app/contracts/custom_fields/hierarchy/update_item_contract.rb @@ -30,13 +30,24 @@ module CustomFields module Hierarchy - class ServiceInitializationContract < Dry::Validation::Contract + class UpdateItemContract < Dry::Validation::Contract params do - required(:field_format).filled(:string) + required(:item).filled(type?: CustomField::Hierarchy::Item) + optional(:label).filled(:string) + optional(:short).filled(:string) end - rule(:field_format) do - key.failure("Custom field must have field format 'hierarchy'") if value != "hierarchy" + rule(:item) do + 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_id, label: value) + key.failure("must be unique at the same hierarchical level") + 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/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..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 + has_closure_tree order: "sort_order", numeric_order: true, dependent: :destroy end 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 d64ae9e79279..0e595505abbc 100644 --- a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb +++ b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb @@ -33,47 +33,105 @@ module Hierarchy class HierarchicalItemService include Dry::Monads[:result] - 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 - - @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 .call({ parent:, label:, short: }.compact) .to_monad - .bind { |validation| create_child_item(validation) } + .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 + .call({ item:, label:, short: }.compact) + .to_monad + .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 + Success(old_item.prepend_sibling(item)) + end + + def soft_delete_item(item) + # Soft delete the item and children + raise NotImplementedError end private - def create_root_item - item = CustomField::Hierarchy::Item.create(custom_field: @custom_field) - return Failure(item.errors) unless item.persisted? + 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) end - def create_child_item(validation) - item = CustomField::Hierarchy::Item - .create(parent: validation[:parent], label: validation[:label], short: validation[:short]) - return Failure(item.errors) unless item.persisted? + def create_child_item(validation:) + item = validation[:parent].children.create(label: validation[:label], short: validation[:short]) + return Failure(item.errors) if item.new_record? Success(item) end + + def update_item_attributes(item:, attributes:) + if item.update(label: attributes[:label], short: attributes[:short]) + Success(item) + else + Failure(item.errors) + end + 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..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: ["Hierarchical root already set"]) - # 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/insert_item_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/insert_item_contract_spec.rb index 6fd825ce2591..900e519481c6 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 diff --git a/spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb b/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb similarity index 54% rename from spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb rename to spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb index 2cf1979cc78d..eef698f84478 100644 --- a/spec/contracts/custom_fields/hierarchy/service_initialization_contract_spec.rb +++ b/spec/contracts/custom_fields/hierarchy/update_item_contract_spec.rb @@ -30,65 +30,83 @@ require "rails_helper" -RSpec.describe CustomFields::Hierarchy::ServiceInitializationContract do +RSpec.describe CustomFields::Hierarchy::UpdateItemContract do subject { described_class.new } # rubocop:disable Rails/DeprecatedActiveModelErrorsMethods describe "#call" do - context "when field_format is 'hierarchy'" do - let(:params) { { field_format: "hierarchy" } } + 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 - result = subject.call(params) - expect(result).to be_success + [ + { 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 field_format is not 'hierarchy'" do - let(:params) { { field_format: "text" } } + context "when item is a root item" do + let(:params) { { item: vader } } - it "is invalid" do + 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'"]) + expect(result.errors.to_h).to include(item: ["must not be a root item"]) end end - context "when field_format is missing" do - let(:params) { {} } + context "when item is not of type 'Item'" do + let(:invalid_item) { create(:custom_field) } + let(:params) { { item: invalid_item } } - it "is invalid" do + it("is invalid") do result = subject.call(params) expect(result).to be_failure - expect(result.errors.to_h).to include(field_format: ["is missing"]) + expect(result.errors.to_h).to include(item: ["must be CustomField::Hierarchy::Item"]) end end - context "when field_format is nil" do - let(:params) { { field_format: nil } } + context "when item is not persisted" do + 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(field_format: ["must be filled"]) + expect(result.errors.to_h).to include(item: ["must exist"]) 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 } + 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: ["must be unique at the same hierarchical level"]) end end - context "when inputs are invalid" do - it "creates a failure result" do + context "when fields are invalid" do + it "is invalid" do [ {}, - { field_format: "text" }, - { field_format: nil }, - { field_format: 42 } + { 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 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..73e6d019d00c 100644 --- a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb +++ b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb @@ -28,76 +28,158 @@ # See COPYRIGHT and LICENSE files for more details. #++ -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) } - describe "#initialize" do - context "with valid custom field" do - it "initializes successfully" do - expect { described_class.new(custom_field) }.not_to raise_error - end - end + 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! } - 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 - 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(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, persisted?: false, errors: "some errors")) + .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 end 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" } let(:short) { "Short Description" } context "with valid parameters" do it "inserts an item successfully without short" do - result = service.insert_item(parent:, 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 - result = service.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 - allow(CustomField::Hierarchy::Item) - .to receive(:create).and_return(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:) + expect(result).to be_failure + end + end + end + + describe "#update_item" do + context "with valid parameters" do + it "updates the item with new attributes" do + 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 = 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 - result = service.insert_item(parent:, label:, short:) + describe "#delete_branch" do + context "with valid item to destroy" do + it "deletes the entire branch" do + result = service.delete_branch(item: luke) + expect(result).to be_success + 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 = 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 + + 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