Merge branch 'epic_hidden_lists' into 'master'

Ignore hidden lists when listing epic board lists [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56338

app/models/board.rb

@@ -34,14 +34,6 @@ class Board < ApplicationRecord
     project_id.present?
   end
 
-  def backlog_list
-    lists.merge(List.backlog).take
-  end
-
-  def closed_list
-    lists.merge(List.closed).take
-  end
-
   def scoped?
     false
   end

app/models/concerns/boards/listable.rb

@@ -13,6 +13,7 @@ module Boards
       scope :ordered, -> { order(:list_type, :position) }
       scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
       scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
+      scope :without_types, ->(list_types) { where.not(list_type: list_types) }
 
       class << self
         def preload_preferences_for_user(lists, user)

app/models/list.rb

@@ -14,7 +14,6 @@ class List < ApplicationRecord
   validates :label_id, uniqueness: { scope: :board_id }, if: :label?
 
   scope :preload_associated_models, -> { preload(:board, label: :priorities) }
-  scope :without_types, ->(list_types) { where.not(list_type: list_types) }
 
   alias_method :preferences, :list_user_preferences
 

app/services/boards/lists/list_service.rb

@@ -23,12 +23,10 @@ module Boards
       end
 
       def hidden_lists_for(board)
-        hidden = []
-
-        hidden << ::List.list_types[:backlog] if board.hide_backlog_list
-        hidden << ::List.list_types[:closed] if board.hide_closed_list
-
-        hidden
+        [].tap do |hidden|
+          hidden << ::List.list_types[:backlog] if board.hide_backlog_list?
+          hidden << ::List.list_types[:closed] if board.hide_closed_list?
+        end
       end
     end
   end

ee/app/graphql/resolvers/boards/epic_lists_resolver.rb

@@ -17,23 +17,27 @@ module Resolvers
       def resolve_with_lookahead(id: nil)
         authorize!
 
-        # eventually we may want to (re)use Boards::Lists::ListService
-        # but we don't support yet creation of default lists so at this
-        # point there is not reason to introduce a ListService
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/294043
-        lists = epic_board.epic_lists
+        lists = board_lists(id)
 
         if load_preferences?(lookahead)
           ::Boards::EpicList.preload_preferences_for_user(lists, current_user)
         end
 
-        lists = lists.where(id: id.model_id) if id # rubocop: disable CodeReuse/ActiveRecord
-
         offset_pagination(apply_lookahead(lists))
       end
 
       private
 
+      def board_lists(id)
+        service = ::Boards::EpicLists::ListService.new(
+          epic_board.resource_parent,
+          current_user,
+          list_id: id&.model_id
+        )
+
+        service.execute(epic_board, create_default_lists: false)
+      end
+
       def load_preferences?(lookahead)
         lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed) ||
             lookahead&.selection(:nodes)&.selects?(:collapsed)

ee/app/models/boards/epic_list.rb

@@ -12,6 +12,8 @@ module Boards
 
     enum list_type: { backlog: 0, label: 1, closed: 2 }
 
+    scope :preload_associated_models, -> { preload(:epic_board, label: :priorities) }
+
     alias_method :preferences, :epic_list_user_preferences
 
     def preferences_for(user)

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

+# frozen_string_literal: true
+
+module Boards
+  module EpicLists
+    class ListService < ::Boards::Lists::ListService
+      private
+
+      def unavailable_list_types_for(board)
+        [].tap do |hidden|
+          hidden << ::Boards::EpicList.list_types[:backlog] if board.hide_backlog_list?
+          hidden << ::Boards::EpicList.list_types[:closed] if board.hide_closed_list?
+        end
+      end
+    end
+  end
+end

ee/spec/graphql/resolvers/boards/epic_lists_resolvers_spec.rb

@@ -7,8 +7,8 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do
 
   let_it_be(:user) { create(:user) }
   let_it_be_with_refind(:group) { create(:group, :private) }
-  let_it_be(:epic_board) { create(:epic_board, group: group) }
-  let_it_be(:epic_list1) { create(:epic_list, epic_board: epic_board) }
+  let_it_be_with_reload(:epic_board) { create(:epic_board, group: group) }
+  let_it_be(:epic_list1) { create(:epic_list, epic_board: epic_board, list_type: :backlog) }
   let_it_be(:epic_list2) { create(:epic_list, epic_board: epic_board) }
 
   specify do
@@ -43,7 +43,17 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do
         let(:resolver) { described_class.single }
 
         it 'returns an array with single epic list' do
-          expect(result).to eq epic_list1
+          expect(result).to eq(epic_list1)
+        end
+      end
+
+      context 'when the board has hidden lists' do
+        before do
+          epic_board.update_column(:hide_backlog_list, true)
+        end
+
+        it 'returns an array with single epic list' do
+          expect(result).to match_array(epic_list2)
         end
       end
     end

ee/spec/services/boards/epic_lists/list_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Boards::EpicLists::ListService do
+  let(:user) { create(:user) }
+
+  describe '#execute' do
+    let_it_be(:parent) { create(:group) }
+    let_it_be(:label) { create(:group_label, group: parent) }
+    let_it_be(:unrelated_list) { create(:epic_list) }
+    let_it_be_with_reload(:board) { create(:epic_board, group: parent) }
+    let_it_be(:list) { create(:epic_list, epic_board: board, label: label) }
+    let_it_be(:closed_list) { create(:epic_list, epic_board: board, list_type: :closed) }
+
+    let(:service) { described_class.new(parent, user) }
+
+    it_behaves_like 'lists list service'
+
+    def create_backlog_list(board)
+      create(:epic_list, epic_board: board, list_type: :backlog)
+    end
+  end
+end

ee/spec/services/ee/boards/lists/list_service_spec.rb

@@ -23,13 +23,13 @@ RSpec.describe Boards::Lists::ListService do
         end
 
         it 'returns all lists' do
-          expect(execute_service).to match_array [backlog_list, list, assignee_list, board.closed_list]
+          expect(execute_service).to match_array [backlog_list, list, assignee_list, board.lists.closed.first]
         end
       end
 
       context 'when the feature is disabled' do
         it 'filters out assignee lists that might have been created while subscribed' do
-          expect(execute_service).to match_array [backlog_list, list, board.closed_list]
+          expect(execute_service).to match_array [backlog_list, list, board.lists.closed.first]
         end
       end
     end
@@ -46,13 +46,13 @@ RSpec.describe Boards::Lists::ListService do
 
         it 'returns all lists' do
           expect(execute_service)
-            .to match_array([backlog_list, list, milestone_list, board.closed_list])
+            .to match_array([backlog_list, list, milestone_list, board.lists.closed.first])
         end
       end
 
       context 'when the feature is disabled' do
         it 'filters out assignee lists that might have been created while subscribed' do
-          expect(execute_service).to match_array [backlog_list, list, board.closed_list]
+          expect(execute_service).to match_array [backlog_list, list, board.lists.closed.first]
         end
       end
     end
@@ -69,7 +69,7 @@ RSpec.describe Boards::Lists::ListService do
 
         it 'returns all lists' do
           expect(execute_service)
-            .to match_array([backlog_list, list, iteration_list, board.closed_list])
+            .to match_array([backlog_list, list, iteration_list, board.lists.closed.first])
         end
 
         context 'when the feature flag is disabled' do
@@ -78,14 +78,14 @@ RSpec.describe Boards::Lists::ListService do
           end
 
           it 'filters out iteration lists that might have been created while subscribed' do
-            expect(execute_service).to match_array [backlog_list, list, board.closed_list]
+            expect(execute_service).to match_array [backlog_list, list, board.lists.closed.first]
           end
         end
       end
 
       context 'when feature is disabled' do
         it 'filters out iteration lists that might have been created while subscribed' do
-          expect(execute_service).to match_array [backlog_list, list, board.closed_list]
+          expect(execute_service).to match_array [backlog_list, list, board.lists.closed.first]
         end
       end
     end

spec/models/list_spec.rb

@@ -17,17 +17,4 @@ RSpec.describe List do
     it { is_expected.to validate_presence_of(:label) }
     it { is_expected.to validate_presence_of(:list_type) }
   end
-
-  describe '.without_types' do
-    it 'exclude lists of given types' do
-      board = create(:list, list_type: :label).board
-      # closed list is created by default
-      backlog_list = create(:list, list_type: :backlog, board: board)
-
-      exclude_type = [described_class.list_types[:label], described_class.list_types[:closed]]
-
-      lists = described_class.without_types(exclude_type)
-      expect(lists.where(board: board)).to match_array([backlog_list])
-    end
-  end
 end

spec/services/boards/lists/list_service_spec.rb

@@ -8,46 +8,32 @@ RSpec.describe Boards::Lists::ListService do
   describe '#execute' do
     let(:service) { described_class.new(parent, user) }
 
-    shared_examples 'hidden lists' do
-      let!(:list) { create(:list, board: board, label: label) }
-
-      context 'when hide_backlog_list is true' do
-        it 'hides backlog list' do
-          board.update!(hide_backlog_list: true)
-
-          expect(service.execute(board)).to match_array([board.closed_list, list])
-        end
-      end
-
-      context 'when hide_closed_list is true' do
-        it 'hides closed list' do
-          board.update!(hide_closed_list: true)
-
-          expect(service.execute(board)).to match_array([board.backlog_list, list])
-        end
-      end
-    end
-
     context 'when board parent is a project' do
-      let(:project) { create(:project) }
-      let(:board) { create(:board, project: project) }
-      let(:label) { create(:label, project: project) }
-      let!(:list) { create(:list, board: board, label: label) }
+      let_it_be(:project) { create(:project) }
+      let_it_be_with_reload(:board) { create(:board, project: project) }
+      let_it_be(:label) { create(:label, project: project) }
+      let_it_be(:list) { create(:list, board: board, label: label) }
+      let_it_be(:unrelated_list) { create(:list) }
+
       let(:parent) { project }
 
       it_behaves_like 'lists list service'
-      it_behaves_like 'hidden lists'
     end
 
     context 'when board parent is a group' do
-      let(:group) { create(:group) }
-      let(:board) { create(:board, group: group) }
-      let(:label) { create(:group_label, group: group) }
-      let!(:list) { create(:list, board: board, label: label) }
+      let_it_be(:group) { create(:group) }
+      let_it_be_with_reload(:board) { create(:board, group: group) }
+      let_it_be(:label) { create(:group_label, group: group) }
+      let_it_be(:list) { create(:list, board: board, label: label) }
+      let_it_be(:unrelated_list) { create(:list) }
+
       let(:parent) { group }
 
       it_behaves_like 'lists list service'
-      it_behaves_like 'hidden lists'
+    end
+
+    def create_backlog_list(board)
+      create(:backlog_list, board: board)
     end
   end
 end

spec/support/shared_examples/models/boards/listable_shared_examples.rb

@@ -16,18 +16,23 @@ RSpec.shared_examples 'boards listable model' do |list_factory|
   end
 
   describe 'scopes' do
+    let_it_be(:list1) { create(list_factory, list_type: :backlog) }
+    let_it_be(:list2) { create(list_factory, list_type: :closed) }
+    let_it_be(:list3) { create(list_factory, position: 1) }
+    let_it_be(:list4) { create(list_factory, position: 2) }
+
     describe '.ordered' do
       it 'returns lists ordered by type and position' do
-        # rubocop:disable Rails/SaveBang
-        lists = [
-          create(list_factory, list_type: :backlog),
-          create(list_factory, list_type: :closed),
-          create(list_factory, position: 1),
-          create(list_factory, position: 2)
-        ]
-        # rubocop:enable Rails/SaveBang
-
-        expect(described_class.where(id: lists).ordered).to eq([lists[0], lists[2], lists[3], lists[1]])
+        expect(described_class.where(id: [list1, list2, list3, list4]).ordered)
+          .to eq([list1, list3, list4, list2])
+      end
+    end
+
+    describe '.without_types' do
+      it 'excludes lists of given types' do
+        lists = described_class.without_types([:label, :closed])
+
+        expect(lists).to match_array([list1])
       end
     end
   end

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

@@ -13,7 +13,7 @@ RSpec.shared_examples 'lists destroy service' do
       development = create(:list, board: board, position: 0)
       review      = create(:list, board: board, position: 1)
       staging     = create(:list, board: board, position: 2)
-      closed      = board.closed_list
+      closed      = board.lists.closed.first
 
       described_class.new(parent, user).execute(development)
 
@@ -24,7 +24,7 @@ RSpec.shared_examples 'lists destroy service' do
   end
 
   it 'does not remove list from board when list type is closed' do
-    list = board.closed_list
+    list = board.lists.closed.first
     service = described_class.new(parent, user)
 
     expect { service.execute(list) }.not_to change(board.lists, :count)

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

@@ -2,14 +2,34 @@
 
 RSpec.shared_examples 'lists list service' do
   context 'when the board has a backlog list' do
-    let!(:backlog_list) { create(:backlog_list, board: board) }
+    let!(:backlog_list) { create_backlog_list(board) }
 
     it 'does not create a backlog list' do
       expect { service.execute(board) }.not_to change(board.lists, :count)
     end
 
     it "returns board's lists" do
-      expect(service.execute(board)).to eq [backlog_list, list, board.closed_list]
+      expect(service.execute(board)).to eq [backlog_list, list, board.lists.closed.first]
+    end
+
+    context 'when hide_backlog_list is true' do
+      before do
+        board.update_column(:hide_backlog_list, true)
+      end
+
+      it 'hides backlog list' do
+        expect(service.execute(board)).to match_array([board.lists.closed.first, list])
+      end
+    end
+
+    context 'when hide_closed_list is true' do
+      before do
+        board.update_column(:hide_closed_list, true)
+      end
+
+      it 'hides closed list' do
+        expect(service.execute(board)).to match_array([backlog_list, list])
+      end
     end
   end
 
@@ -23,25 +43,21 @@ RSpec.shared_examples 'lists list service' do
     end
 
     it "returns board's lists" do
-      expect(service.execute(board)).to eq [board.backlog_list, list, board.closed_list]
+      expect(service.execute(board)).to eq [board.lists.backlog.first, list, board.lists.closed.first]
     end
   end
 
   context 'when wanting a specific list' do
-    let!(:list1) { create(:list, board: board) }
-
     it 'returns list specified by id' do
-      service = described_class.new(parent, user, list_id: list1.id)
+      service = described_class.new(parent, user, list_id: list.id)
 
-      expect(service.execute(board, create_default_lists: false)).to eq [list1]
+      expect(service.execute(board, create_default_lists: false)).to eq [list]
     end
 
     it 'returns empty result when list is not found' do
-      external_board = create(:board, resource_parent: create(:project))
-      external_list = create(:list, board: external_board)
-      service = described_class.new(parent, user, list_id: external_list.id)
+      service = described_class.new(parent, user, list_id: unrelated_list.id)
 
-      expect(service.execute(board, create_default_lists: false)).to eq(List.none)
+      expect(service.execute(board, create_default_lists: false)).to be_empty
     end
   end
 end