Merge pull request #17099 from opf/impl/insert-high-and-low

Implements the ability to add items above or below another

app/components/admin/custom_fields/hierarchy/item_component.html.erb

@@ -29,15 +29,14 @@ See COPYRIGHT and LICENSE files for more details.
 
 <%=
   component_wrapper(tag: "turbo-frame", refresh: :morph) do
-    if show_edit_form?
-      render Admin::CustomFields::Hierarchy::ItemFormComponent.new(
-        target_item: model,
-        url: custom_field_item_path(@root.custom_field_id, model),
-        method: :put
-      )
+    if show_form?
+      render Admin::CustomFields::Hierarchy::ItemFormComponent.new(model)
     else
       flex_layout(align_items: :center, justify_content: :space_between, test_selector: "op-custom-fields--hierarchy-item") do |item_container|
         item_container.with_column(flex_layout: true) do |item_information|
+          item_information.with_column(mr: 2) do
+            render(Primer::OpenProject::DragHandle.new)
+          end
           item_information.with_column(mr: 2) do
             render(Primer::Beta::Link.new(href: custom_field_item_path(@root.custom_field_id, model), underline: false)) do
               render(Primer::Beta::Text.new(font_weight: :bold)) { model.label }
@@ -62,8 +61,7 @@ See COPYRIGHT and LICENSE files for more details.
             menu.with_show_button(icon: "kebab-horizontal",
                                   scheme: :invisible,
                                   "aria-label": I18n.t("custom_fields.admin.items.actions"))
-            edit_action_item(menu)
-            deletion_action_item(menu)
+            menu_items(menu)
           end
         end
       end

app/components/admin/custom_fields/hierarchy/item_component.rb

@@ -38,7 +38,7 @@ class ItemComponent < ApplicationComponent
         def initialize(item:, show_edit_form: false)
           super(item)
           @show_edit_form = show_edit_form
-          @root = item.root
+          @root = item.root || item.parent.root
         end
 
         def wrapper_uniq_by
@@ -49,12 +49,24 @@ def short_text
           "(#{model.short})"
         end
 
-        def show_edit_form? = @show_edit_form
+        def show_form? = @show_edit_form || model.new_record?
 
         def children_count
           I18n.t("custom_fields.admin.hierarchy.subitems", count: model.children.count)
         end
 
+        def menu_items(menu)
+          edit_action_item(menu)
+          menu.with_divider
+          add_above_action_item(menu)
+          add_below_action_item(menu)
+          add_sub_item_action_item(menu)
+          menu.with_divider
+          deletion_action_item(menu)
+        end
+
+        private
+
         def deletion_action_item(menu)
           menu.with_item(label: I18n.t(:button_delete),
                          scheme: :danger,
@@ -72,6 +84,33 @@ def edit_action_item(menu)
             item.with_leading_visual_icon(icon: :pencil)
           end
         end
+
+        def add_above_action_item(menu)
+          menu.with_item(
+            label: I18n.t(:button_add_item_above),
+            tag: :a,
+            content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
+            href: new_child_custom_field_item_path(@root.custom_field_id, model.parent, position: model.sort_order)
+          ) { _1.with_leading_visual_icon(icon: "fold-up") }
+        end
+
+        def add_below_action_item(menu)
+          menu.with_item(
+            label: I18n.t(:button_add_item_below),
+            tag: :a,
+            content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
+            href: new_child_custom_field_item_path(@root.custom_field_id, model.parent, position: model.sort_order + 1)
+          ) { _1.with_leading_visual_icon(icon: "fold-down") }
+        end
+
+        def add_sub_item_action_item(menu)
+          menu.with_item(
+            label: I18n.t(:button_add_sub_item),
+            tag: :a,
+            content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
+            href: new_child_custom_field_item_path(@root.custom_field_id, model)
+          ) { _1.with_leading_visual_icon(icon: "op-arrow-in") }
+        end
       end
     end
   end

app/components/admin/custom_fields/hierarchy/item_form_component.html.erb

@@ -28,7 +28,7 @@ See COPYRIGHT and LICENSE files for more details.
 ++#%>
 
 <%=
-  primer_form_with(url: @url, method: @method, test_selector: "op-custom-fields--new-item-form") do |f|
+  primer_form_with(**item_options) do |f|
     render(CustomFields::Hierarchy::ItemForm.new(f, target_item: model))
   end
 %>

app/components/admin/custom_fields/hierarchy/item_form_component.rb

@@ -34,10 +34,29 @@ module Hierarchy
       class ItemFormComponent < ApplicationComponent
         include OpTurbo::Streamable
 
-        def initialize(target_item:, url:, method:)
-          super(target_item)
-          @url = url
-          @method = method
+        def item_options
+          options = { url:, method: http_verb, data: { test_selector: "op-custom-fields--new-item-form" } }
+          options[:data][:turbo_frame] = ItemsComponent.wrapper_key if model.new_record?
+
+          options
+        end
+
+        def http_verb
+          model.new_record? ? :post : :put
+        end
+
+        def url
+          if model.new_record?
+            new_child_custom_field_item_path(root.custom_field_id, model.parent)
+          else
+            custom_field_item_path(root.custom_field_id, model)
+          end
+        end
+
+        private
+
+        def root
+          @root ||= model.new_record? ? model.parent.root : model.root
         end
       end
     end

app/components/admin/custom_fields/hierarchy/items_component.html.erb

@@ -32,7 +32,7 @@ See COPYRIGHT and LICENSE files for more details.
     flex_layout do |container|
       container.with_row do
         render(Primer::OpenProject::SubHeader.new) do |subheader|
-          subheader.with_action_button(tag: :a, scheme: :primary, href: new_item_path)  do |button|
+          subheader.with_action_button(tag: :a, scheme: :primary, href: new_item_path) do |button|
             button.with_leading_visual_icon(icon: :plus)
             I18n.t(:label_item)
           end
@@ -42,12 +42,17 @@ See COPYRIGHT and LICENSE files for more details.
         render(Primer::Beta::BorderBox.new) do |box|
           box.with_header { item_header }
 
-          if children.empty? && !show_new_item_form?
+          if children.empty?
             box.with_row do
               render(Primer::Beta::Blankslate.new(test_selector: "op-custom-fields--hierarchy-items-blankslate")) do |component|
-                component.with_visual_icon(icon: "list-ordered")
-                component.with_heading(tag: :h3).with_content(I18n.t("custom_fields.admin.items.blankslate.title"))
-                component.with_description { I18n.t("custom_fields.admin.items.blankslate.description") }
+                component.with_visual_icon(icon: blank_icon)
+
+                component.with_heading(tag: :h3).with_content(I18n.t(blank_header_text))
+                component.with_description { I18n.t(blank_description_text) }
+                component.with_primary_action(tag: :a, href: new_item_path) do |button|
+                  button.with_leading_visual_icon(icon: :plus)
+                  I18n.t(:label_item)
+                end
               end
             end
           else
@@ -56,16 +61,6 @@ See COPYRIGHT and LICENSE files for more details.
                 render Admin::CustomFields::Hierarchy::ItemComponent.new(item: item)
               end
             end
-
-            if show_new_item_form?
-              box.with_footer(test_selector: "op-custom-fields--new-item-form") do
-                render Admin::CustomFields::Hierarchy::ItemFormComponent.new(
-                  target_item: @new_item,
-                  url: new_child_custom_field_item_path(root.custom_field_id, model),
-                  method: :post
-                )
-              end
-            end
           end
         end
       end

app/components/admin/custom_fields/hierarchy/items_component.rb

@@ -35,21 +35,32 @@ class ItemsComponent < ApplicationComponent
         include OpTurbo::Streamable
         include OpPrimer::ComponentHelpers
 
-        property :children
-
         def initialize(item:, new_item: nil)
           super(item)
           @new_item = new_item
         end
 
-        def show_new_item_form? = @new_item
-
         def root
           @root ||= model.root? ? model : model.root
         end
 
         def new_item_path
-          new_child_custom_field_item_path(root.custom_field_id, model)
+          position = model.children.any? ? model.children.last.sort_order + 1 : 0
+
+          new_child_custom_field_item_path(root.custom_field_id, model, position:)
+        end
+
+        def children
+          list = model.children
+          return list unless @new_item
+
+          position = @new_item.sort_order&.to_i
+
+          if position
+            list[0...position] + [@new_item] + list[position..]
+          else
+            list + [@new_item]
+          end
         end
 
         def item_header
@@ -60,6 +71,26 @@ def item_header
           end
         end
 
+        def blank_icon
+          model.root? ? "list-ordered" : "op-arrow-in"
+        end
+
+        def blank_header_text
+          if model.root?
+            "custom_fields.admin.items.blankslate.root.title"
+          else
+            "custom_fields.admin.items.blankslate.item.title"
+          end
+        end
+
+        def blank_description_text
+          if model.root?
+            "custom_fields.admin.items.blankslate.root.description"
+          else
+            "custom_fields.admin.items.blankslate.item.description"
+          end
+        end
+
         private
 
         def slices

app/controllers/admin/custom_fields/hierarchy/items_controller.rb

@@ -56,7 +56,7 @@ def show
         end
 
         def new
-          @new_item = ::CustomField::Hierarchy::Item.new(parent: @active_item)
+          @new_item = ::CustomField::Hierarchy::Item.new(parent: @active_item, sort_order: params[:position])
         end
 
         def edit; end
@@ -65,8 +65,13 @@ def create
           item_service
             .insert_item(**item_input)
             .either(
-              ->(_) { redirect_to(new_child_custom_field_item_path(@custom_field, @active_item), status: :see_other) },
-              ->(validation_result) do
+              lambda do |item|
+                redirect_to(
+                  new_child_custom_field_item_path(@custom_field, @active_item, position: item.sort_order + 1),
+                  status: :see_other
+                )
+              end,
+              lambda do |validation_result|
                 add_errors_to_form(validation_result)
                 render action: :new
               end
@@ -77,10 +82,10 @@ def update
           item_service
             .update_item(item: @active_item, label: item_input[:label], short: item_input[:short])
             .either(
-              ->(_) do
+              lambda do |_|
                 redirect_to(custom_field_item_path(@custom_field, @active_item.parent), status: :see_other)
               end,
-              ->(validation_result) do
+              lambda do |validation_result|
                 add_errors_to_edit_form(validation_result)
                 render action: :edit
               end
@@ -111,6 +116,7 @@ def item_service
         def item_input
           input = { parent: @active_item, label: params[:label] }
           input[:short] = params[:short] if params[:short].present?
+          input[:sort_order] = params[:sort_order].to_i if params[:sort_order].present?
 
           input
         end

app/forms/custom_fields/hierarchy/item_form.rb

@@ -30,6 +30,8 @@ module CustomFields
   module Hierarchy
     class ItemForm < ApplicationForm
       form do |item_form|
+        item_form.hidden name: :sort_order, value: @target_item.sort_order
+
         item_form.group(layout: :horizontal) do |input_group|
           input_group.text_field(
             name: :label,

app/models/custom_field/hierarchy/item.rb

@@ -32,7 +32,7 @@ class CustomField::Hierarchy::Item < ApplicationRecord
   self.table_name = "hierarchical_items"
 
   belongs_to :custom_field
-  has_closure_tree order: "sort_order", numeric_order: true, dependent: :destroy
+  has_closure_tree order: "sort_order", numeric_order: true, dont_order_roots: true, dependent: :destroy
 
   scope :including_children, -> { includes(children: :children) }
 end

app/services/custom_fields/hierarchy/hierarchical_item_service.rb

@@ -44,17 +44,18 @@ def generate_root(custom_field)
           .bind { |validation| create_root_item(validation[:custom_field]) }
       end
 
-      # Insert a new node on the hierarchy tree.
+      # Insert a new node on the hierarchy tree at a desired position or at the end if no sort_order is passed.
       # @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
+      # @param sort_order [Integer] the position into which insert the item.
       # @return [Success(CustomField::Hierarchy::Item), Failure(Dry::Validation::Result), Failure(ActiveModel::Errors)]
-      def insert_item(parent:, label:, short: nil)
+      def insert_item(parent:, label:, short: nil, sort_order: nil)
         CustomFields::Hierarchy::InsertItemContract
           .new
           .call({ parent:, label:, short: }.compact)
           .to_monad
-          .bind { |validation| create_child_item(validation:) }
+          .bind { |validation| create_child_item(validation:, sort_order:) }
       end
 
       # Updates an item/node
@@ -118,8 +119,11 @@ def create_root_item(custom_field)
         Success(item)
       end
 
-      def create_child_item(validation:)
-        item = validation[:parent].children.create(label: validation[:label], short: validation[:short])
+      def create_child_item(validation:, sort_order: nil)
+        attributes = validation.to_h
+        attributes[:sort_order] = sort_order - 1 if sort_order
+
+        item = validation[:parent].children.create(**attributes)
         return Failure(item.errors) if item.new_record?
 
         Success(item)

config/locales/en.yml

@@ -247,8 +247,12 @@ en:
       items:
         actions: "Item actions"
         blankslate:
-          title: "Your list of items is empty"
-          description: "Start by adding items to the custom field of type hierarchy. Each item can be used to create a hierarchy bellow it. To navigate and create sub-items inside a hierarchy click on the created item."
+          root:
+            title: "Your list of items is empty"
+            description: "Start by adding items to the custom field of type hierarchy. Each item can be used to create a hierarchy bellow it. To navigate and create sub-items inside a hierarchy click on the created item."
+          item:
+            title: This item doesn't have any hierarchy level below
+            description: Add items to this list to create sub-items inside another one
         placeholder:
           label: "Item label"
           short: "Short name"
@@ -1470,6 +1474,9 @@ en:
   button_actions: "Actions"
   button_add: "Add"
   button_add_comment: "Add comment"
+  button_add_item_above: "Add item above"
+  button_add_item_below: "Add item below"
+  button_add_sub_item: "Add sub-item"
   button_add_member: Add member
   button_add_watcher: "Add watcher"
   button_annotate: "Annotate"

spec/features/custom_fields/hierarchy_custom_field_spec.rb

@@ -85,7 +85,7 @@
     hierarchy_page.expect_current_path
     expect(page).to have_test_selector("op-custom-fields--hierarchy-items-blankslate")
 
-    click_on "Item"
+    within("sub-header") { click_on "Item" }
     expect(page).not_to have_test_selector("op-custom-fields--hierarchy-items-blankslate")
     fill_in "Label", with: "Stormtroopers"
     fill_in "Short", with: "ST"