diff --git a/app/components/admin/custom_fields/hierarchy/item_component.rb b/app/components/admin/custom_fields/hierarchy/item_component.rb index 579a16e8a653..48440dfc9aad 100644 --- a/app/components/admin/custom_fields/hierarchy/item_component.rb +++ b/app/components/admin/custom_fields/hierarchy/item_component.rb @@ -55,6 +55,14 @@ def children_count I18n.t("custom_fields.admin.hierarchy.subitems", count: model.children.count) end + def first_item? + model.sort_order == 0 + end + + def last_item? + model.sort_order == model.parent.children.length - 1 + end + def menu_items(menu) edit_action_item(menu) menu.with_divider @@ -62,21 +70,14 @@ def menu_items(menu) add_below_action_item(menu) add_sub_item_action_item(menu) menu.with_divider + move_up_action_item(menu) unless first_item? + move_down_action_item(menu) unless last_item? + menu.with_divider deletion_action_item(menu) end private - def deletion_action_item(menu) - menu.with_item(label: I18n.t(:button_delete), - scheme: :danger, - tag: :a, - href: deletion_dialog_custom_field_item_path(custom_field_id: @root.custom_field_id, id: model.id), - content_arguments: { data: { controller: "async-dialog" } }) do |item| - item.with_leading_visual_icon(icon: :trash) - end - end - def edit_action_item(menu) menu.with_item(label: I18n.t(:button_edit), tag: :a, @@ -111,6 +112,40 @@ def add_sub_item_action_item(menu) href: new_child_custom_field_item_path(@root.custom_field_id, model) ) { _1.with_leading_visual_icon(icon: "op-arrow-in") } end + + def move_up_action_item(menu) + form_inputs = [{ name: "new_sort_order", value: model.sort_order - 1 }] + + menu.with_item(label: I18n.t(:label_sort_higher), + tag: :button, + href: move_custom_field_item_path(@root.custom_field_id, model), + content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } }, + form_arguments: { method: :post, inputs: form_inputs }) do |item| + item.with_leading_visual_icon(icon: "chevron-up") + end + end + + def move_down_action_item(menu) + form_inputs = [{ name: "new_sort_order", value: model.sort_order + 2 }] + + menu.with_item(label: I18n.t(:label_sort_lower), + tag: :button, + href: move_custom_field_item_path(@root.custom_field_id, model), + content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } }, + form_arguments: { method: :post, inputs: form_inputs }) do |item| + item.with_leading_visual_icon(icon: "chevron-down") + end + end + + def deletion_action_item(menu) + menu.with_item(label: I18n.t(:button_delete), + scheme: :danger, + tag: :a, + href: deletion_dialog_custom_field_item_path(custom_field_id: @root.custom_field_id, id: model.id), + content_arguments: { data: { controller: "async-dialog" } }) do |item| + item.with_leading_visual_icon(icon: :trash) + end + 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 78e9b51ed4b7..1d978ac08d55 100644 --- a/app/controllers/admin/custom_fields/hierarchy/items_controller.rb +++ b/app/controllers/admin/custom_fields/hierarchy/items_controller.rb @@ -92,6 +92,13 @@ def update ) end + def move + item_service + .reorder_item(item: @active_item, new_sort_order: params.require(:new_sort_order)) + + redirect_to(custom_field_items_path(@custom_field), status: :see_other) + end + def destroy item_service .delete_branch(item: @active_item) diff --git a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb index 09fccae93c85..e0274491d148 100644 --- a/app/services/custom_fields/hierarchy/hierarchical_item_service.rb +++ b/app/services/custom_fields/hierarchy/hierarchical_item_service.rb @@ -98,11 +98,18 @@ def move_item(item:, new_parent:) # 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)] + # @param new_sort_order [Integer] the new position of the node + # @return [Success] def reorder_item(item:, new_sort_order:) - old_item = item.siblings.where(sort_order: new_sort_order).first - Success(old_item.prepend_sibling(item)) + return Success() if item.siblings.empty? + + new_sort_order = [0, new_sort_order.to_i].max + + return Success() if item.sort_order == new_sort_order + + update_item_order(item:, new_sort_order:) + + Success() end def soft_delete_item(item) @@ -136,6 +143,16 @@ def update_item_attributes(item:, attributes:) Failure(item.errors) end end + + def update_item_order(item:, new_sort_order:) + target_item = item.siblings.find_by(sort_order: new_sort_order) + if target_item.present? + target_item.prepend_sibling(item) + else + target_item = item.siblings.last + target_item.append_sibling(item) + end + end end end end diff --git a/config/routes.rb b/config/routes.rb index e7a185ba0771..81282e04ecf9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -175,9 +175,12 @@ resources :projects, controller: "/admin/custom_fields/custom_field_projects", only: %i[index new create] resource :project, controller: "/admin/custom_fields/custom_field_projects", only: :destroy resources :items, controller: "/admin/custom_fields/hierarchy/items" do - get :deletion_dialog, on: :member - get :new_child, on: :member, action: :new - post :new_child, on: :member, action: :create + member do + get :deletion_dialog + get :new_child, action: :new + post :new_child, action: :create + post :move + end end end end diff --git a/spec/controllers/admin/custom_fields/hierarchy/items_controller_spec.rb b/spec/controllers/admin/custom_fields/hierarchy/items_controller_spec.rb index ef6a8632ac3a..a8a94ee69a8d 100644 --- a/spec/controllers/admin/custom_fields/hierarchy/items_controller_spec.rb +++ b/spec/controllers/admin/custom_fields/hierarchy/items_controller_spec.rb @@ -124,7 +124,7 @@ end.to change { luke.reload.label }.from("luke").to(updated_name) end - it "renders the update page" do + it "redirects" do post :update, params: { custom_field_id: custom_field.id, id: luke.id, label: "luke s." } expect(response).to be_redirect end @@ -139,6 +139,34 @@ end end + describe "PUT #move" do + before do + service.insert_item(parent: root, label: "not relevant") + service.insert_item(parent: root, label: "not important") + service.insert_item(parent: root, label: "unused") + end + + context "when it is successful" do + it "redirects to the index" do + post :move, params: { custom_field_id: custom_field.id, id: luke.id, new_sort_order: 3 } + expect(response).to be_redirect + end + + it "moves the item to the new position" do + expect do + post :move, params: { custom_field_id: custom_field.id, id: luke.id, new_sort_order: 2 } + end.to change { luke.reload.sort_order }.from(0).to(1) + end + end + + context "when missing parameters" do + it "fails with bad request" do + post :move, params: { custom_field_id: custom_field.id, id: luke.id } # missing new_sort_order + expect(response).to be_bad_request + end + end + end + describe "DELETE #destroy" do context "when validation successful" do it "deletes an item" do 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 797f66f36c7f..21f6d5d2b191 100644 --- a/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb +++ b/spec/services/custom_fields/hierarchy/hierarchical_item_service_spec.rb @@ -182,12 +182,60 @@ 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 + it "reorders the item to the target 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 + + it "reorders the item even if sort order is a string" 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 + + it "reorders the item to the last position" do + service.reorder_item(item: lando, new_sort_order: root.children.length) + + 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 + + it "reorders the item to the first position" do + service.reorder_item(item: chewbacca, new_sort_order: 0) + + expect(chewbacca.reload.sort_order).to eq(0) + expect(luke.reload.sort_order).to eq(1) + expect(lando.reload.sort_order).to eq(2) + end + + it "does not reorder before first" do + service.reorder_item(item: lando, new_sort_order: -10) + + expect(lando.reload.sort_order).to eq(0) + expect(luke.reload.sort_order).to eq(1) + expect(chewbacca.reload.sort_order).to eq(2) + end + + it "does not reorder after last" do + service.reorder_item(item: chewbacca, new_sort_order: 99) + + expect(luke.reload.sort_order).to eq(0) + expect(lando.reload.sort_order).to eq(1) + expect(chewbacca.reload.sort_order).to eq(2) + end + + it "does not reorder when changing self" do + service.reorder_item(item: lando, new_sort_order: lando.sort_order) + + expect(luke.reload.sort_order).to eq(0) + expect(lando.reload.sort_order).to eq(1) + expect(chewbacca.reload.sort_order).to eq(2) + end end end