[DREAM-789] Move enumerations via sortable-lists

app/components/admin/enumerations/index_component.html.erb

@@ -1,6 +1,6 @@
 <%=
-component_wrapper do
-  flex_layout(data: wrapper_data_attributes) do |flex|
+component_wrapper(data: wrapper_data_attributes) do
+  flex_layout do |flex|
     flex.with_row do
       render(Primer::OpenProject::SubHeader.new) do |subheader|
         subheader.with_action_button(
@@ -21,12 +21,12 @@ component_wrapper do
         OpenProject::Common::BorderBoxListComponent.new(
           container: "#{wrapper_key}-box",
           position: :relative,
-          data: drop_target_config
+          data: list_data
         )
       ) do |list|
         list.with_header(
           title_tag: :h3,
-          title: enumeration_class.model_name.human(count: :other)
+          title: enumeration_title
         )
 
         list.with_empty_state(
@@ -36,8 +36,8 @@ component_wrapper do
         )
 
         enumerations.each do |enumeration|
-          list.with_item(test_selector: "enumeration-row-#{enumeration.id}", data: draggable_item_config(enumeration)) do
-            render(item_component_class.new(enumeration: enumeration, max_position: max_position))
+          list.with_item(test_selector: "enumeration-row-#{enumeration.id}", data: item_data(enumeration)) do
+            render(item_component_class.new(enumeration:))
           end
         end
       end

app/components/admin/enumerations/index_component.rb

@@ -35,38 +35,55 @@ class IndexComponent < ApplicationComponent
       include OpPrimer::ComponentHelpers
       include OpTurbo::Streamable
 
-      options :enumerations
+      def initialize(enumerations:)
+        super()
+        @enumerations = enumerations
+      end
 
       private
 
-      def max_position
-        enumerations.map(&:position).max
-      end
+      attr_reader :enumerations
 
       def wrapper_data_attributes
         {
-          controller: "generic-drag-and-drop"
+          controller: "sortable-lists",
+          sortable_lists_move_url_template_value: move_url_template,
+          sortable_lists_sortable_lists__list_outlet: "##{wrapper_key} [data-controller~='sortable-lists--list']",
+          sortable_lists_sortable_lists__item_outlet: "##{wrapper_key} [data-controller~='sortable-lists--item']"
         }
       end
 
-      def drop_target_config
+      # Built from the route helper with a sentinel so relative-URL-root
+      # installations keep working; {id} is expanded client-side.
+      def move_url_template
+        id_placeholder = "__id__"
+        helpers.url_for(action: :move, id: id_placeholder).sub(id_placeholder, "{id}")
+      end
+
+      def list_data
         {
-          generic_drag_and_drop_target: "container",
-          "target-container-accessor": ":scope > ul",
-          "target-allowed-drag-type": "enumeration"
+          controller: "sortable-lists--list",
+          sortable_lists__list_type_value: sortable_list_type,
+          sortable_lists__list_accepted_type_value: sortable_list_type,
+          sortable_lists__list_name_value: enumeration_title
         }
       end
 
-      def draggable_item_config(enumeration)
+      def item_data(enumeration)
         {
-          "draggable-id": enumeration.id,
-          "draggable-type": "enumeration",
-          "drop-url": helpers.url_for(action: :move, id: enumeration.id)
+          controller: "sortable-lists--item",
+          sortable_lists__item_id_value: enumeration.id,
+          sortable_lists__item_type_value: sortable_list_type,
+          sortable_lists__item_label_value: enumeration.name
         }
       end
 
-      def enumeration_class
-        enumerations.klass
+      def sortable_list_type
+        enumerations.model_name.param_key
+      end
+
+      def enumeration_title
+        enumerations.model_name.human(count: :other)
       end
 
       def item_component_class

app/components/admin/enumerations/item_component.html.erb

@@ -3,7 +3,7 @@
     flex_layout(align_items: :center, justify_content: :space_between) do |enumeration_container|
       enumeration_container.with_column(flex_layout: true) do |enumeration_info|
         enumeration_info.with_column(mr: 2) do
-          render(Primer::OpenProject::DragHandle.new)
+          render(Primer::OpenProject::DragHandle.new(data: { sortable_lists__item_target: "handle" }))
         end
 
         if colored?

app/components/admin/enumerations/item_component.rb

@@ -34,37 +34,27 @@ class ItemComponent < ApplicationComponent
       include ApplicationHelper
       include OpPrimer::ComponentHelpers
       include OpTurbo::Streamable
-
-      options :enumeration
-      options :max_position
+      include SortableLists::MoveMenu
 
       delegate :colored?, to: :enumeration
 
+      def initialize(enumeration:)
+        super()
+        @enumeration = enumeration
+      end
+
       private
 
+      attr_reader :enumeration
+
       def wrapper_uniq_by
         enumeration.id
       end
 
-      def first_item?
-        enumeration.position == 1
-      end
-
-      def last_item?
-        enumeration.position == max_position
-      end
-
       def build_enumeration_menu(menu)
-        with_item_group(menu) { edit_enumeration(menu) }
         with_item_group(menu) do
-          unless first_item?
-            move_to_top_enumeration(menu)
-            move_up_enumeration(menu)
-          end
-          unless last_item?
-            move_down_enumeration(menu)
-            move_to_bottom_enumeration(menu)
-          end
+          edit_enumeration(menu)
+          move_enumeration(menu)
         end
         with_item_group(menu) { deletion_enumeration(menu) }
       end
@@ -77,51 +67,17 @@ def edit_enumeration(menu)
         end
       end
 
-      def move_to_top_enumeration(menu)
-        form_inputs = [{ name: "move_to", value: "highest" }]
-
-        menu.with_item(label: I18n.t(:label_sort_highest),
-                       tag: :button,
-                       href: helpers.url_for(action: :move, id: enumeration),
-                       # content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
-                       form_arguments: { method: :put, inputs: form_inputs }) do |item|
-          item.with_leading_visual_icon(icon: "move-to-top")
-        end
-      end
-
-      def move_up_enumeration(menu)
-        form_inputs = [{ name: "move_to", value: "higher" }]
-
-        menu.with_item(label: I18n.t(:label_sort_higher),
-                       tag: :button,
-                       href: helpers.url_for(action: :move, id: enumeration),
-                       # content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
-                       form_arguments: { method: :put, inputs: form_inputs }) do |item|
-          item.with_leading_visual_icon(icon: "chevron-up")
-        end
-      end
-
-      def move_down_enumeration(menu)
-        form_inputs = [{ name: "move_to", value: "lower" }]
-
-        menu.with_item(label: I18n.t(:label_sort_lower),
-                       tag: :button,
-                       href: helpers.url_for(action: :move, id: enumeration),
-                       #  content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
-                       form_arguments: { method: :put, inputs: form_inputs }) do |item|
-          item.with_leading_visual_icon(icon: "chevron-down")
-        end
-      end
-
-      def move_to_bottom_enumeration(menu)
-        form_inputs = [{ name: "move_to", value: "lowest" }]
-
-        menu.with_item(label: I18n.t(:label_sort_lowest),
-                       tag: :button,
-                       href: helpers.url_for(action: :move, id: enumeration),
-                       #    content_arguments: { data: { turbo_frame: ItemsComponent.wrapper_key } },
-                       form_arguments: { method: :put, inputs: form_inputs }) do |item|
-          item.with_leading_visual_icon(icon: "move-to-bottom")
+      def move_enumeration(menu)
+        menu.with_item(
+          component_klass: Primer::Alpha::ActionMenu::SubMenuItem,
+          label: I18n.t(:button_move),
+          select_variant: :none,
+          form_arguments: {},
+          data: { sortable_lists__item_target: "moveMenu" }
+        ) do |submenu|
+          submenu.with_leading_visual_icon(icon: :"op-arrow-in")
+
+          with_move_items(submenu)
         end
       end
 

app/controllers/admin/settings/enumerations_controller_base.rb

@@ -82,21 +82,17 @@ def destroy
       end
 
       def move
-        if @enumeration.update(move_params)
-          render_success_flash_message_via_turbo_stream(
-            message: I18n.t(:enumeration_caption_order_changed)
-          )
+        moved = move_after_anchor
+
+        if moved
+          render_move_success
         else
           render_error_flash_message_via_turbo_stream(
-            message: I18n.t(:enumeration_could_not_be_moved)
+            message: I18n.t(:error_invalid_list_move_anchor)
           )
         end
 
-        replace_via_turbo_stream(
-          component: index_component_class.new(enumerations: enumeration_class.all)
-        )
-
-        respond_with_turbo_streams
+        respond_with_turbo_streams(status: moved ? :ok : :unprocessable_entity)
       end
 
       def reassign
@@ -105,17 +101,43 @@ def reassign
 
       private
 
-      def move_params
-        move_to = params[:move_to]
-        position = Integer(params[:position], exception: false)
+      # Morph first: Turbo applies streams in order, so the flash only shows
+      # once the list has been reconciled.
+      def render_move_success
+        update_via_turbo_stream(component: index_component, method: :morph)
+        render_success_flash_message_via_turbo_stream(
+          message: I18n.t(:enumeration_caption_order_changed)
+        )
+      end
+
+      def index_component
+        index_component_class.new(enumerations: enumeration_class.all)
+      end
 
-        if move_to.in? %w(highest higher lower lowest)
-          { move_to: move_to }
-        elsif position
-          { position: position }
-        else
-          {}
-        end
+      def move_after_anchor
+        return false unless valid_drop_request?
+
+        @enumeration.move_after_anchor(drop_params[:prev_id], scope: enumeration_class.all)
+      end
+
+      def valid_drop_request?
+        drop_params[:list_type] == sortable_list_type &&
+          unscoped_list_id? &&
+          drop_params.key?(:prev_id)
+      end
+
+      # Enumeration lists carry no list id. The raw param is checked because
+      # permit cannot tell an absent value from a filtered-out array or hash.
+      def unscoped_list_id?
+        params[:list_id].nil? || params[:list_id] == ""
+      end
+
+      def drop_params
+        @drop_params ||= params.permit(:list_type, :list_id, :prev_id)
+      end
+
+      def sortable_list_type
+        enumeration_class.model_name.param_key
       end
 
       def handle_reassignment_on_deletion

app/models/concerns/lists/move_after_anchor.rb

@@ -33,18 +33,22 @@ module Lists
   # directly below another record of the same list, addressed by id.
   module MoveAfterAnchor
     # Moves the record below the record identified by `prev_id` within
-    # `scope` (a relation over the same acts_as_list list). A blank
-    # `prev_id` moves the record to the top.
+    # `scope` (a relation over the same acts_as_list list). `nil` or an
+    # empty string moves the record to the top; otherwise `prev_id` must
+    # be a positive Integer or its canonical decimal String.
     #
-    # Returns false without mutating when the anchor is unknown, outside
-    # the scope, or the record itself.
+    # Returns false without mutating for any other `prev_id`, or when the
+    # anchor is unknown, outside the scope, or the record itself.
     def move_after_anchor(prev_id, scope:) # rubocop:disable Naming/PredicateMethod -- verb command, not a query
-      if prev_id.blank?
+      if prev_id.nil? || prev_id == ""
         move_to_top
         return true
       end
 
-      anchor = scope.find_by(id: prev_id)
+      anchor_id = anchor_id_from(prev_id)
+      return false if anchor_id.nil?
+
+      anchor = scope.find_by(id: anchor_id)
       return false if anchor.nil? || anchor.id == id
 
       # Removing the record first shifts the anchor up by one when the
@@ -53,5 +57,14 @@ def move_after_anchor(prev_id, scope:) # rubocop:disable Naming/PredicateMethod
       insert_at(position > anchor.position ? anchor.position + 1 : anchor.position)
       true
     end
+
+    private
+
+    def anchor_id_from(prev_id)
+      case prev_id
+      when Integer then prev_id if prev_id.positive?
+      when /\A[1-9]\d*\z/ then prev_id.to_i
+      end
+    end
   end
 end

app/models/enumeration.rb

@@ -29,6 +29,8 @@
 #++
 
 class Enumeration < ApplicationRecord
+  include Lists::MoveAfterAnchor
+
   default_scope { order("#{Enumeration.table_name}.position ASC") }
 
   belongs_to :project, optional: true

modules/costs/app/controllers/admin/settings/time_entry_activities_controller.rb

@@ -38,10 +38,6 @@ class TimeEntryActivitiesController < EnumerationsControllerBase
       def enumeration_class
         TimeEntryActivity
       end
-
-      def enumeration_param_key
-        enumeration_class.model_name.param_key
-      end
     end
   end
 end