Add move to list option to Methodology cards

CHANGELOG

@@ -1,6 +1,7 @@
 [v#.#.#] ([month] [YYYY])
   - [entity]:
     - [future tense verb] [feature]
+  - Cards: add option to move a task to a different list
   - Upgraded gems:
     - rails, rails-html-sanitizer, sqlite3, websocket-driver
   - Bugs fixes:

app/assets/stylesheets/hera/modules.scss

@@ -199,10 +199,6 @@
     margin-left: auto;
 
     .action {
-      &:not(:first-child)::before {
-        content: '-';
-      }
-
       &:first-child {
         margin-left: auto;
       }

app/assets/stylesheets/hera/modules/_dropdown.scss

@@ -36,6 +36,12 @@
       font-weight: 600;
     }
 
+    &.disabled {
+      color: var(--text-muted);
+      cursor: not-allowed;
+      pointer-events: none;
+    }
+
     i {
       margin-right: 0.25rem;
     }

app/controllers/cards/position_controller.rb

@@ -0,0 +1,107 @@
+class Cards::PositionController < AuthenticatedController
+  include EventPublisher
+  include LinkedListMoveValidator
+  include ProjectScoped
+
+  before_action :set_current_board_and_list
+  before_action :set_card
+  before_action :require_list_change, if: :append_to_list?
+  before_action :set_prev_item_and_next_item
+  before_action :validate_move_params
+
+  def create
+    Card.transaction do
+      List.move(@card, prev_item: @prev_item, next_item: @next_item)
+
+      if new_list
+        @card.list = new_list
+        @card.save!
+      end
+    end
+
+    publish_event('card.updated', @card.to_event_payload)
+    @card.reload
+
+    respond_to do |format|
+      format.html do
+        redirect_to project_board_list_card_path(current_project, @board, @card.list, @card),
+          notice: 'Task moved.'
+      end
+
+      format.json do
+        render json: {
+          is_card: true,
+          id: @card.id,
+          link: polymorphic_path([current_project, @board, @card.list, @card]),
+          moveLink: project_board_list_card_position_path(current_project, @board, @card.list, @card)
+        }
+      end
+    end
+  end
+
+  private
+
+  # A request with a new_list_id but no explicit position appends the card to
+  # the end of the target list (e.g. the 'move to list' dropdown).
+  def append_to_list?
+    move_params[:new_list_id].present? &&
+      move_params[:prev_id].blank? &&
+      move_params[:next_id].blank?
+  end
+
+  # Override EventPublisher#event_action_payload: the RESTful action is
+  # 'create', but moving a card is semantically an update.
+  #
+  # FIXME: Replace with ActivityService action registration once
+  # convention-over-configuration approach is implemented.
+  def event_action_payload
+    super.merge(action: 'update')
+  end
+
+  def move_params
+    params.permit(:card_id, :project_id, :board_id, :list_id, :next_id, :prev_id, :new_list_id)
+  end
+
+  def moveable_item_name
+    'card'
+  end
+
+  def moveable_items
+    @board.cards
+  end
+
+  def moveable_parent
+    new_list || @list
+  end
+
+  def new_list
+    return if move_params[:new_list_id].blank?
+
+    @new_list ||= @board.lists.find(move_params[:new_list_id])
+  end
+
+  def require_list_change
+    return if new_list.id != @card.list_id
+
+    redirect_to project_board_list_card_path(current_project, @board, @list, @card),
+      alert: 'Task is already in that list.'
+  end
+
+  def set_card
+    @card = @board.cards.find(params[:card_id])
+  end
+
+  def set_current_board_and_list
+    @board = current_project.boards.includes(:lists).find(params[:board_id])
+    @list = @board.lists.includes(:cards).find(params[:list_id])
+  end
+
+  def set_prev_item_and_next_item
+    if append_to_list?
+      @prev_item = new_list.last_card
+      @next_item = nil
+    else
+      super
+    end
+  end
+end

app/controllers/cards_controller.rb

@@ -11,12 +11,10 @@ class CardsController < AuthenticatedController
   before_action :initialize_sidebar, only: [:show, :new, :edit]
   before_action :set_auto_save_key, only: [:new, :create, :edit, :update]
 
-  # Not at top because we need board and list set first
-  include ValidateMove
-
   layout 'cards'
 
   def show
+    @lists = @board.ordered_lists
     render layout: !request.xhr?
   end
 
@@ -52,24 +50,6 @@ def update
     end
   end
 
-  def move
-    List.move(@card, prev_item: @prev_item, next_item: @next_item)
-
-    if new_list
-      @card.list = new_list
-      @card.save
-    end
-
-    track_updated(@card)
-
-    render json: {
-      is_card:  true,
-      id:       @card.id,
-      link:     polymorphic_path([current_project, @board, @card.reload.list, @card]),
-      moveLink: move_project_board_list_card_path(current_project, @board, @card.reload.list, @card)
-    }
-  end
-
   def destroy
     if @card.destroy
       track_destroyed(@card)
@@ -85,13 +65,6 @@ def card_params
     params.require(:card).permit(:name, :description, :due_date, assignee_ids: [])
   end
 
-  def move_params
-    params.
-      permit(:id, :project_id, :board_id, :list_id,
-        :next_id, :prev_id, :new_list_id
-      )
-  end
-
   def initialize_sidebar
     @sorted_cards = @list.ordered_cards.select(&:persisted?)
   end
@@ -107,7 +80,7 @@ def set_or_initialize_card
 
   def set_current_board_and_list
     @board = current_project.boards.includes(:lists).find(params[:board_id])
-    @list  = @board.lists.includes(:cards).find(params[:list_id])
+    @list = @board.lists.includes(:cards).find(params[:list_id])
   end
 
   def set_auto_save_key
@@ -119,8 +92,4 @@ def set_auto_save_key
       "#{@list.id}-card"
     end
   end
-
-  def new_list
-    @board.lists.find(move_params[:new_list_id]) if move_params[:new_list_id]
-  end
 end

app/controllers/concerns/linked_list_move_validator.rb

@@ -0,0 +1,35 @@
+module LinkedListMoveValidator
+  extend ActiveSupport::Concern
+
+  protected
+
+  def set_prev_item_and_next_item
+    @prev_item = moveable_items.find_by(id: move_params[:prev_id])
+    @next_item = moveable_items.find_by(id: move_params[:next_id])
+  end
+
+  def validate_move_params
+    unless valid_move_params?
+      redirect_to project_board_path(current_project, @board), alert: 'Something fishy is going on...'
+    end
+  end
+
+  private
+
+  def valid_move_params?
+    if @prev_item.present?
+      next_item_of_prev_item = @prev_item.send("next_#{moveable_item_name}")
+      if next_item_of_prev_item
+        @next_item == next_item_of_prev_item
+      else
+        @next_item.nil?
+      end
+    else
+      if moveable_parent.items.empty?
+        @next_item.nil?
+      else
+        @next_item == moveable_parent.first_item
+      end
+    end
+  end
+end

app/controllers/lists/position_controller.rb

@@ -0,0 +1,44 @@
+class Lists::PositionController < AuthenticatedController
+  include ActivityTracking
+  include LinkedListMoveValidator
+  include ProjectScoped
+
+  before_action :set_current_board
+  before_action :set_list
+  before_action :set_prev_item_and_next_item
+  before_action :validate_move_params
+
+  def create
+    Board.move(@list, prev_item: @prev_item, next_item: @next_item)
+
+    track_updated(@list)
+
+    render json: @list
+  end
+
+  private
+
+  def move_params
+    params.permit(:list_id, :project_id, :board_id, :next_id, :prev_id)
+  end
+
+  def moveable_item_name
+    'list'
+  end
+
+  def moveable_items
+    @board.lists
+  end
+
+  def moveable_parent
+    @board
+  end
+
+  def set_current_board
+    @board = current_project.boards.find(params[:board_id])
+  end
+
+  def set_list
+    @list = @board.lists.find(params[:list_id])
+  end
+end

app/controllers/lists_controller.rb

@@ -3,10 +3,7 @@ class ListsController < AuthenticatedController
   include ProjectScoped
 
   before_action :set_current_board
-  before_action :set_list, only: [:edit, :update, :destroy, :move]
-
-  # Not at top because we need board set first
-  include ValidateMove
+  before_action :set_list, only: [:edit, :update, :destroy]
 
   def new
     @list = @board.lists.new
@@ -35,14 +32,6 @@ def update
     end
   end
 
-  def move
-    Board.move(@list, prev_item: @prev_item, next_item: @next_item)
-
-    track_updated(@list)
-
-    render json: @list
-  end
-
   def destroy
     if @list.destroy
       track_destroyed(@list)
@@ -58,11 +47,6 @@ def list_params
     params.require(:list).permit(:name)
   end
 
-  def move_params
-    params.
-      permit(:id, :project_id, :board_id, :next_id, :prev_id)
-  end
-
   def set_current_board
     @board = current_project.boards.find(params[:board_id])
   end