allow sorting custom actions

app/cells/custom_actions/row_cell.rb

@@ -1,6 +1,7 @@
 module CustomActions
   class RowCell < ::RowCell
     include ::IconsHelper
+    include ReorderLinksHelper
 
     def action
       model
@@ -10,6 +11,10 @@ module CustomActions
       link_to h(action.name), edit_custom_action_path(action)
     end
 
+    def sort
+      reorder_links('custom_action', { action: 'update', id: action }, method: :put)
+    end
+
     def button_links
       [
         edit_link,

app/cells/custom_actions/table_cell.rb

@@ -1,7 +1,8 @@
 module CustomActions
   class TableCell < ::TableCell
     columns :name,
-            :description
+            :description,
+            :sort
 
     def initial_sort
       %i[id asc]
@@ -27,7 +28,8 @@ module CustomActions
     def headers
       [
         ['name', caption: CustomAction.human_attribute_name(:name)],
-        ['description', caption: CustomAction.human_attribute_name(:description)]
+        ['description', caption: CustomAction.human_attribute_name(:description)],
+        ['sort', caption: I18n.t(:label_sort)]
       ]
     end
   end

app/controllers/custom_actions_controller.rb

@@ -40,7 +40,7 @@ class CustomActionsController < ApplicationController
   helper_method :gon
 
   def index
-    @custom_actions = CustomAction.order_by_name
+    @custom_actions = CustomAction.order_by_position
   end
 
   def new

app/helpers/application_helper.rb

@@ -324,38 +324,6 @@ module ApplicationHelper
     path.to_s
   end
 
-  def reorder_links(name, url, options = {})
-    method = options[:method] || :post
-
-    content_tag(:span,
-                link_to(content_tag(:span, '',
-                                    class: 'icon-context icon-sort-up icon-small',
-                                    title: l(:label_sort_highest)),
-                        url.merge("#{name}[move_to]" => 'highest'),
-                        method: method,
-                        title: l(:label_sort_highest)) +
-                link_to(content_tag(:span, '',
-                                    class: 'icon-context icon-arrow-up2 icon-small',
-                                    title: l(:label_sort_higher)),
-                        url.merge("#{name}[move_to]" => 'higher'),
-                        method: method,
-                        title: l(:label_sort_higher)) +
-                link_to(content_tag(:span, '',
-                                    class: 'icon-context icon-arrow-down2 icon-small',
-                                    title: l(:label_sort_lower)),
-                        url.merge("#{name}[move_to]" => 'lower'),
-                        method: method,
-                        title: l(:label_sort_lower)) +
-                link_to(content_tag(:span, '',
-                                    class: 'icon-context icon-sort-down icon-small',
-                                    title: l(:label_sort_lowest)),
-                        url.merge("#{name}[move_to]" => 'lowest'),
-                        method: method,
-                        title: l(:label_sort_lowest)),
-                class: 'reorder-icons'
-               )
-  end
-
   def other_formats_links(&block)
     formats = capture(Redmine::Views::OtherFormatsBuilder.new(self), &block)
     unless formats.nil? || formats.strip.empty?

app/helpers/reorder_links_helper.rb

@@ -0,0 +1,55 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See docs/COPYRIGHT.rdoc for more details.
+#++
+
+module ReorderLinksHelper
+  def reorder_links(name, url, options = {})
+    method = options[:method] || :post
+
+    content_tag(:span,
+                reorder_link(name, url, 'highest', 'icon-sort-up', t(:label_sort_highest), method) +
+                  reorder_link(name, url, 'higher', 'icon-arrow-up2', t(:label_sort_higher), method) +
+                  reorder_link(name, url, 'lower', 'icon-arrow-down2', t(:label_sort_lower), method) +
+                  reorder_link(name, url, 'lowest', 'icon-sort-down', t(:label_sort_lowest), method),
+                class: 'reorder-icons')
+  end
+
+  def reorder_link(name, url, direction, icon_class, label, method)
+    text = content_tag(:span,
+                       label,
+                       class: 'hidden-for-sighted')
+    icon = content_tag(:span,
+                       '',
+                       class: "icon-context #{icon_class} icon-small")
+    link_to(text + icon,
+            url.merge("#{name}[move_to]" => direction),
+            method: method,
+            title: label)
+  end
+end

app/models/custom_action.rb

@@ -40,6 +40,8 @@ class CustomAction < ActiveRecord::Base
 
   define_attribute_method 'conditions'
 
+  acts_as_list
+
   def initialize(*args)
     ret = super
 
@@ -65,6 +67,10 @@ class CustomAction < ActiveRecord::Base
     order(:name)
   end
 
+  def self.order_by_position
+    order(:position)
+  end
+
   def all_actions
     all_of(available_actions, actions)
   end

app/models/permitted_params.rb

@@ -466,6 +466,7 @@ class PermittedParams
         custom_action: %i(
           name
           description
+          move_to
         ),
         custom_field: [
           :editable,

app/services/custom_actions/base_service.rb

@@ -49,12 +49,12 @@ class CustomActions::BaseService
   private
 
   def set_attributes(action, attributes)
-    actions_attributes = (attributes.delete(:actions) || {}).symbolize_keys
-    conditions_attributes = (attributes.delete(:conditions) || {}).symbolize_keys
+    actions_attributes = attributes.delete(:actions)
+    conditions_attributes = attributes.delete(:conditions)
     action.attributes = attributes
 
-    set_actions(action, actions_attributes)
-    set_conditions(action, conditions_attributes)
+    set_actions(action, actions_attributes.symbolize_keys) if actions_attributes
+    set_conditions(action, conditions_attributes.symbolize_keys) if conditions_attributes
   end
 
   def set_actions(action, actions_attributes)

db/migrate/20180221151038_add_position_to_custom_action.rb

@@ -0,0 +1,13 @@
+class AddPositionToCustomAction < ActiveRecord::Migration[5.0]
+  def change
+    add_column :custom_actions, :position, :integer
+
+    reversible do |dir|
+      dir.up do
+        CustomAction.order_by_name.each_with_index do |a, i|
+          a.update_attribute(:position, i)
+        end
+      end
+    end
+  end
+end

lib/api/v3/work_packages/work_package_representer.rb

@@ -503,7 +503,7 @@ module API
 
         resources :customActions,
                   link: ->(*) {
-                    represented.custom_actions(current_user).map do |action|
+                    ordered_custom_actions.map do |action|
                       {
                         href: api_v3_paths.custom_action(action.id),
                         title: action.name
@@ -511,7 +511,7 @@ module API
                     end
                   },
                   getter: ->(*) {
-                    represented.custom_actions(current_user).map do |action|
+                    ordered_custom_actions.map do |action|
                       ::API::V3::CustomActions::CustomActionRepresenter.new(action, current_user: current_user)
                     end
                   },
@@ -596,6 +596,11 @@ module API
           # noop
         end
 
+        def ordered_custom_actions
+          # As the custom actions are sometimes set as an array
+          represented.custom_actions(current_user).to_a.sort_by(&:position)
+        end
+
         self.to_eager_load = [{ children: { project: :enabled_modules } },
                               { parent: { project: :enabled_modules } },
                               { project: %i(enabled_modules work_package_custom_fields) },

spec/controllers/custom_actions_controller_spec.rb

@@ -97,7 +97,7 @@ describe CustomActionsController, type: :controller do
     let(:call) { get :index }
     before do
       allow(CustomAction)
-        .to receive(:order_by_name)
+        .to receive(:order_by_position)
         .and_return([action])
     end
 

spec/features/work_packages/custom_actions_spec.rb

@@ -224,28 +224,23 @@ describe 'Custom actions', type: :feature, js: true do
 
     wp_page.visit!
 
-    expect(page)
-      .to have_selector('.custom-action', text: 'Unassign', wait: 10)
-    expect(page)
-      .to have_selector('.custom-action', text: 'Close')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Escalate')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Move project')
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Reset')
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Other roles action')
+    wp_page.expect_custom_action('Unassign')
+    wp_page.expect_custom_action('Close')
+    wp_page.expect_custom_action('Escalate')
+    wp_page.expect_custom_action('Move project')
+    wp_page.expect_no_custom_action('Reset')
+    wp_page.expect_no_custom_action('Other roles action')
+    wp_page.expect_custom_action_order('Unassign', 'Close', 'Escalate', 'Move project')
 
     within('.custom-actions') do
       # When hovering over the button, the description is displayed
       button = find('.custom-action--button', text: 'Unassign')
       expect(button['title'])
         .to eql 'Removes the assignee'
-
-      click_button('Unassign')
     end
 
+    wp_page.click_custom_action('Unassign')
+
     wp_page.expect_attributes assignee: '-'
     wp_page.expect_notification message: 'Successful update'
     wp_page.dismiss_notification!
@@ -253,17 +248,13 @@ describe 'Custom actions', type: :feature, js: true do
     ## Bump the lockVersion and by that force a conflict.
     WorkPackage.where(id: work_package.id).update_all(lock_version: 10)
 
-    within('.custom-actions') do
-      click_button('Escalate')
-    end
+    wp_page.click_custom_action('Escalate')
 
     wp_page.expect_notification type: :error, message: I18n.t('api_v3.errors.code_409')
 
     wp_page.visit!
 
-    within('.custom-actions') do
-      click_button('Escalate')
-    end
+    wp_page.click_custom_action('Escalate')
 
     wp_page.expect_attributes priority: immediate_priority.name,
                               status: default_status.name,
@@ -275,23 +266,17 @@ describe 'Custom actions', type: :feature, js: true do
     wp_page.expect_notification message: 'Successful update'
     wp_page.dismiss_notification!
 
-    within('.custom-actions') do
-      click_button('Close')
-    end
+    wp_page.click_custom_action('Close')
 
     wp_page.expect_attributes status: closed_status.name,
                               priority: immediate_priority.name
     wp_page.expect_notification message: 'Successful update'
     wp_page.dismiss_notification!
 
-    expect(page)
-      .to have_selector('.custom-action', text: 'Reset')
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Close')
+    wp_page.expect_custom_action('Reset')
+    wp_page.expect_no_custom_action('Close')
 
-    within('.custom-actions') do
-      click_button('Reset')
-    end
+    wp_page.click_custom_action('Reset')
 
     wp_page.expect_attributes priority: default_priority.name,
                               status: default_status.name,
@@ -316,27 +301,25 @@ describe 'Custom actions', type: :feature, js: true do
     index_ca_page.expect_current_path
     index_ca_page.expect_listed('Unassign', 'Close', 'Escalate', 'Reject')
 
+    index_ca_page.move_top 'Move project'
+    index_ca_page.move_bottom 'Escalate'
+    index_ca_page.move_up 'Close'
+    index_ca_page.move_down 'Unassign'
+
     # Check the altered button
     login_as(user)
 
     wp_page.visit!
 
-    expect(page)
-      .to have_selector('.custom-action', text: 'Unassign')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Close')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Escalate')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Reject')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Move project')
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Reset')
+    wp_page.expect_custom_action('Unassign')
+    wp_page.expect_custom_action('Close')
+    wp_page.expect_custom_action('Escalate')
+    wp_page.expect_custom_action('Move project')
+    wp_page.expect_custom_action('Reject')
+    wp_page.expect_no_custom_action('Reset')
+    wp_page.expect_custom_action_order('Move project', 'Close', 'Reject', 'Unassign', 'Escalate')
 
-    within('.custom-actions') do
-      click_button('Reject')
-    end
+    wp_page.click_custom_action('Reject')
 
     wp_page.expect_attributes assignee: '-',
                               status: rejected_status.name,
@@ -344,10 +327,8 @@ describe 'Custom actions', type: :feature, js: true do
     wp_page.expect_notification message: 'Successful update'
     wp_page.dismiss_notification!
 
-    expect(page)
-      .to have_selector('.custom-action', text: 'Close')
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Reject')
+    wp_page.expect_custom_action('Close')
+    wp_page.expect_no_custom_action('Reject')
 
     # Delete 'Reject' from list of actions
     login_as(admin)
@@ -363,19 +344,13 @@ describe 'Custom actions', type: :feature, js: true do
 
     wp_page.visit!
 
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Unassign')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Close')
-    expect(page)
-      .to have_selector('.custom-action', text: 'Escalate')
-    expect(page)
-      .to have_no_selector('.custom-action', text: 'Reject')
+    wp_page.expect_no_custom_action('Unassign')
+    wp_page.expect_custom_action('Close')
+    wp_page.expect_custom_action('Escalate')
+    wp_page.expect_no_custom_action('Reject')
 
     # Move project
-    within('.custom-actions') do
-      click_button('Move project')
-    end
+    wp_page.click_custom_action('Move project')
 
     # TODO: check project
     wp_page.expect_attributes assignee: '-',

spec/models/permitted_params_spec.rb

@@ -362,7 +362,8 @@ describe PermittedParams, type: :model do
         'name' => 'blubs',
         'description' => 'blubs blubs',
         'actions' => { 'assigned_to' => '1' },
-        'conditions' => { 'status' => '42' }
+        'conditions' => { 'status' => '42' },
+        'move_to' => 'lower'
       }
     end
 

spec/support/pages/abstract_work_package.rb

@@ -135,6 +135,24 @@ module Pages
       expect(page).to have_selector('#top-menu', visible: true)
     end
 
+    def expect_custom_action(name)
+      expect(page)
+        .to have_selector('.custom-action', text: name)
+    end
+
+    def expect_no_custom_action(name)
+      expect(page)
+        .to have_no_selector('.custom-action', text: name)
+    end
+
+    def expect_custom_action_order(*names)
+      within('.custom-actions') do
+        names.each_cons(2) do |earlier, later|
+          body.index(earlier) < body.index(later)
+        end
+      end
+    end
+
     def update_attributes(key_value_map, save: true)
       set_attributes(key_value_map, save: save)
     end
@@ -183,6 +201,12 @@ module Pages
       page
     end
 
+    def click_custom_action(name)
+      within('.custom-actions') do
+        click_button(name)
+      end
+    end
+
     def trigger_edit_mode
       page.click_button(I18n.t('js.button_edit'))
     end

spec/support/pages/admin/custom_actions/index.rb

@@ -67,17 +67,47 @@ module Pages
           end
         end
 
+        def move_top(name)
+          within_row_of(name) do
+            click_link 'Move to top'
+          end
+        end
+
+        def move_bottom(name)
+          within_row_of(name) do
+            click_link 'Move to bottom'
+          end
+        end
+
+        def move_up(name)
+          within_row_of(name) do
+            click_link 'Move up'
+          end
+        end
+
+        def move_down(name)
+          within_row_of(name) do
+            click_link 'Move down'
+          end
+        end
+
         def path
           custom_actions_path
         end
 
         private
 
-        def within_buttons_of(name)
+        def within_row_of(name)
           within 'table' do
-            row = find('tr', text: name)
+            within find('tr', text: name) do
+              yield
+            end
+          end
+        end
 
-            within row.find('.buttons') do
+        def within_buttons_of(name)
+          within_row_of(name) do
+            within find('.buttons') do
               yield
             end
           end

spec/support/pages/work_packages_table.rb

@@ -93,7 +93,6 @@ module Pages
     end
 
     def click_inline_create
-
       ##
       # When using the inline create on initial page load,
       # there is a delay on travis where inline create can be clicked.