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 57d5e7714809..d28d9eacd57d 100644 --- a/app/contracts/custom_fields/hierarchy/insert_item_contract.rb +++ b/app/contracts/custom_fields/hierarchy/insert_item_contract.rb @@ -32,24 +32,18 @@ module CustomFields module Hierarchy class InsertItemContract < Dry::Validation::Contract 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 if CustomField::Hierarchy::Item.exists?(parent_id: values[: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/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 70af1da8caae..16726baac258 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,7 +67,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/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