Merge branch 'bw-board-sorting' into 'master'

Project board names now sorted correctly in CE

See merge request gitlab-org/gitlab!22807

app/models/board.rb

@@ -11,6 +11,8 @@ class Board < ApplicationRecord
   validates :group, presence: true, unless: :project
 
   scope :with_associations, -> { preload(:destroyable_lists) }
+  scope :order_by_name_asc, -> { order(arel_table[:name].lower.asc) }
+  scope :first_board, -> { where(id: self.order_by_name_asc.limit(1).select(:id)) }
 
   def project_needed?
     !group

app/services/boards/list_service.rb

@@ -4,13 +4,24 @@ module Boards
   class ListService < Boards::BaseService
     def execute
       create_board! if parent.boards.empty?
-      boards
+
+      if parent.multiple_issue_boards_available?
+        boards
+      else
+        # When multiple issue boards are not available
+        # a user is only allowed to view the default shown board
+        first_board
+      end
     end
 
     private
 
     def boards
-      parent.boards
+      parent.boards.order_by_name_asc
+    end
+
+    def first_board
+      parent.boards.first_board
     end
 
     def create_board!
@@ -18,5 +29,3 @@ module Boards
     end
   end
 end
-
-Boards::ListService.prepend_if_ee('EE::Boards::ListService')

changelogs/unreleased/bw-board-sorting.yml

+---
+title: Project issue board names now sorted correctly in FOSS
+merge_request: 22807
+author:
+type: fixed

ee/app/services/ee/boards/list_service.rb

-# frozen_string_literal: true
-
-module EE
-  module Boards
-    module ListService
-      extend ::Gitlab::Utils::Override
-
-      override :execute
-      # rubocop: disable CodeReuse/ActiveRecord
-      def execute
-        if parent.multiple_issue_boards_available?
-          super
-        else
-          # When multiple issue boards is not available
-          # user is only allowed to view the default shown board
-
-          # We could use just one query but MYSQL does not support nested queries using LIMIT.
-          boards.where(id: super.first).reorder(nil)
-        end
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-
-      private
-
-      override :boards
-      # rubocop: disable CodeReuse/ActiveRecord
-      def boards
-        super.order(Arel.sql('LOWER(name) ASC'))
-      end
-      # rubocop: enable CodeReuse/ActiveRecord
-    end
-  end
-end

ee/spec/services/boards/list_service_spec.rb

@@ -3,33 +3,20 @@
 require 'spec_helper'
 
 describe Boards::ListService do
-  shared_examples 'boards list service' do
-    let(:service) { described_class.new(resource_parent, double) }
-    let!(:boards) { create_list(:board, 3, resource_parent: resource_parent) }
+  it_behaves_like 'multiple boards list service' do
+    let(:parent) { create(:project, :empty_repo) }
 
-    describe '#execute' do
-      it 'returns all issue boards when multiple issue boards is enabled' do
-        stub_licensed_features(multiple_group_issue_boards: true)
-
-        expect(service.execute.size).to eq(3)
-      end
-
-      it 'returns boards ordered by name' do
-        board_names = %w[a-board B-board c-board].shuffle
-        boards.each_with_index { |board, i| board.update_column(:name, board_names[i]) }
-        stub_licensed_features(multiple_group_issue_boards: true)
-
-        expect(service.execute.pluck(:name)).to eq(%w[a-board B-board c-board])
-      end
+    before do
+      stub_licensed_features(multiple_group_issue_boards: true)
     end
   end
 
-  it_behaves_like 'boards list service' do
-    let(:resource_parent) { create(:project, :empty_repo) }
-  end
+  it_behaves_like 'multiple boards list service' do
+    let(:parent) { create(:group) }
 
-  it_behaves_like 'boards list service' do
-    let(:resource_parent) { create(:group) }
+    before do
+      stub_licensed_features(multiple_group_issue_boards: true)
+    end
 
     it 'returns the first issue board when multiple issue boards is disabled' do
       stub_licensed_features(multiple_group_issue_boards: false)

spec/models/board_spec.rb

@@ -3,6 +3,9 @@
 require 'spec_helper'
 
 describe Board do
+  let(:project) { create(:project) }
+  let(:other_project) { create(:project) }
+
   describe 'relationships' do
     it { is_expected.to belong_to(:project) }
     it { is_expected.to have_many(:lists).order(list_type: :asc, position: :asc).dependent(:delete_all) }
@@ -11,4 +14,28 @@ describe Board do
   describe 'validations' do
     it { is_expected.to validate_presence_of(:project) }
   end
+
+  describe '#order_by_name_asc' do
+    let!(:second_board) { create(:board, name: 'Secondary board', project: project) }
+    let!(:first_board)  { create(:board, name: 'First board', project: project) }
+
+    it 'returns in alphabetical order' do
+      expect(project.boards.order_by_name_asc).to eq [first_board, second_board]
+    end
+  end
+
+  describe '#first_board' do
+    let!(:other_board)  { create(:board, name: 'Other board', project: other_project) }
+    let!(:second_board) { create(:board, name: 'Secondary board', project: project) }
+    let!(:first_board)  { create(:board, name: 'First board', project: project) }
+
+    it 'return the first alphabetical board as a relation' do
+      expect(project.boards.first_board).to eq [first_board]
+    end
+
+    # BoardsActions#board expects this behavior
+    it 'raises an error when find is done on a non-existent record' do
+      expect { project.boards.first_board.find(second_board.id) }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+  end
 end

spec/services/boards/list_service_spec.rb

@@ -10,6 +10,7 @@ describe Boards::ListService do
       subject(:service) { described_class.new(parent, double) }
 
       it_behaves_like 'boards list service'
+      it_behaves_like 'multiple boards list service'
     end
 
     context 'when board parent is a group' do

spec/support/shared_examples/services/boards/boards_list_service.rb

@@ -29,3 +29,20 @@ shared_examples 'boards list service' do
     expect(service.execute).to eq [board]
   end
 end
+
+shared_examples 'multiple boards list service' do
+  let(:service)  { described_class.new(parent, double) }
+  let!(:board_B) { create(:board, resource_parent: parent, name: 'B-board') }
+  let!(:board_c) { create(:board, resource_parent: parent, name: 'c-board') }
+  let!(:board_a) { create(:board, resource_parent: parent, name: 'a-board') }
+
+  describe '#execute' do
+    it 'returns all issue boards' do
+      expect(service.execute.size).to eq(3)
+    end
+
+    it 'returns boards ordered by name' do
+      expect(service.execute).to eq [board_a, board_B, board_c]
+    end
+  end
+end