Commit 94af69c5 authored by Jan Provaznik's avatar Jan Provaznik

Ignore hidden lists when listing board lists

Reuses issue board lists service to filter out lists which are hidden.
parent 60444b92
...@@ -34,14 +34,6 @@ class Board < ApplicationRecord ...@@ -34,14 +34,6 @@ class Board < ApplicationRecord
project_id.present? project_id.present?
end end
def backlog_list
lists.merge(List.backlog).take
end
def closed_list
lists.merge(List.closed).take
end
def scoped? def scoped?
false false
end end
......
...@@ -13,6 +13,7 @@ module Boards ...@@ -13,6 +13,7 @@ module Boards
scope :ordered, -> { order(:list_type, :position) } scope :ordered, -> { order(:list_type, :position) }
scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) } scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) }
scope :movable, -> { where(list_type: list_types.slice(*movable_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 class << self
def preload_preferences_for_user(lists, user) def preload_preferences_for_user(lists, user)
......
...@@ -14,7 +14,6 @@ class List < ApplicationRecord ...@@ -14,7 +14,6 @@ class List < ApplicationRecord
validates :label_id, uniqueness: { scope: :board_id }, if: :label? validates :label_id, uniqueness: { scope: :board_id }, if: :label?
scope :preload_associated_models, -> { preload(:board, label: :priorities) } 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 alias_method :preferences, :list_user_preferences
......
...@@ -23,12 +23,10 @@ module Boards ...@@ -23,12 +23,10 @@ module Boards
end end
def hidden_lists_for(board) def hidden_lists_for(board)
hidden = [] [].tap do |hidden|
hidden << ::List.list_types[:backlog] if board.hide_backlog_list?
hidden << ::List.list_types[:backlog] if board.hide_backlog_list hidden << ::List.list_types[:closed] if board.hide_closed_list?
hidden << ::List.list_types[:closed] if board.hide_closed_list end
hidden
end end
end end
end end
......
...@@ -17,23 +17,27 @@ module Resolvers ...@@ -17,23 +17,27 @@ module Resolvers
def resolve_with_lookahead(id: nil) def resolve_with_lookahead(id: nil)
authorize! authorize!
# eventually we may want to (re)use Boards::Lists::ListService lists = board_lists(id)
# 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
if load_preferences?(lookahead) if load_preferences?(lookahead)
::Boards::EpicList.preload_preferences_for_user(lists, current_user) ::Boards::EpicList.preload_preferences_for_user(lists, current_user)
end end
lists = lists.where(id: id.model_id) if id # rubocop: disable CodeReuse/ActiveRecord
offset_pagination(apply_lookahead(lists)) offset_pagination(apply_lookahead(lists))
end end
private 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) def load_preferences?(lookahead)
lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed) || lookahead&.selection(:edges)&.selection(:node)&.selects?(:collapsed) ||
lookahead&.selection(:nodes)&.selects?(:collapsed) lookahead&.selection(:nodes)&.selects?(:collapsed)
......
...@@ -12,6 +12,8 @@ module Boards ...@@ -12,6 +12,8 @@ module Boards
enum list_type: { backlog: 0, label: 1, closed: 2 } 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 alias_method :preferences, :epic_list_user_preferences
def preferences_for(user) def preferences_for(user)
......
# 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
...@@ -7,8 +7,8 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do ...@@ -7,8 +7,8 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be_with_refind(:group) { create(:group, :private) } let_it_be_with_refind(:group) { create(:group, :private) }
let_it_be(:epic_board) { create(:epic_board, group: group) } let_it_be_with_reload(:epic_board) { create(:epic_board, group: group) }
let_it_be(:epic_list1) { create(:epic_list, epic_board: epic_board) } 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) } let_it_be(:epic_list2) { create(:epic_list, epic_board: epic_board) }
specify do specify do
...@@ -43,7 +43,17 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do ...@@ -43,7 +43,17 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do
let(:resolver) { described_class.single } let(:resolver) { described_class.single }
it 'returns an array with single epic list' do 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 end
end end
......
# 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
...@@ -23,13 +23,13 @@ RSpec.describe Boards::Lists::ListService do ...@@ -23,13 +23,13 @@ RSpec.describe Boards::Lists::ListService do
end end
it 'returns all lists' do 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
end end
context 'when the feature is disabled' do context 'when the feature is disabled' do
it 'filters out assignee lists that might have been created while subscribed' 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 end
end end
...@@ -46,13 +46,13 @@ RSpec.describe Boards::Lists::ListService do ...@@ -46,13 +46,13 @@ RSpec.describe Boards::Lists::ListService do
it 'returns all lists' do it 'returns all lists' do
expect(execute_service) 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
end end
context 'when the feature is disabled' do context 'when the feature is disabled' do
it 'filters out assignee lists that might have been created while subscribed' 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 end
end end
...@@ -69,7 +69,7 @@ RSpec.describe Boards::Lists::ListService do ...@@ -69,7 +69,7 @@ RSpec.describe Boards::Lists::ListService do
it 'returns all lists' do it 'returns all lists' do
expect(execute_service) 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 end
context 'when the feature flag is disabled' do context 'when the feature flag is disabled' do
...@@ -78,14 +78,14 @@ RSpec.describe Boards::Lists::ListService do ...@@ -78,14 +78,14 @@ RSpec.describe Boards::Lists::ListService do
end end
it 'filters out iteration lists that might have been created while subscribed' 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 end
end end
context 'when feature is disabled' do context 'when feature is disabled' do
it 'filters out iteration lists that might have been created while subscribed' 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 end
end end
......
...@@ -17,17 +17,4 @@ RSpec.describe List do ...@@ -17,17 +17,4 @@ RSpec.describe List do
it { is_expected.to validate_presence_of(:label) } it { is_expected.to validate_presence_of(:label) }
it { is_expected.to validate_presence_of(:list_type) } it { is_expected.to validate_presence_of(:list_type) }
end 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 end
...@@ -8,46 +8,32 @@ RSpec.describe Boards::Lists::ListService do ...@@ -8,46 +8,32 @@ RSpec.describe Boards::Lists::ListService do
describe '#execute' do describe '#execute' do
let(:service) { described_class.new(parent, user) } 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 context 'when board parent is a project' do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:board) { create(:board, project: project) } let_it_be_with_reload(:board) { create(:board, project: project) }
let(:label) { create(:label, project: project) } let_it_be(:label) { create(:label, project: project) }
let!(:list) { create(:list, board: board, label: label) } let_it_be(:list) { create(:list, board: board, label: label) }
let_it_be(:unrelated_list) { create(:list) }
let(:parent) { project } let(:parent) { project }
it_behaves_like 'lists list service' it_behaves_like 'lists list service'
it_behaves_like 'hidden lists'
end end
context 'when board parent is a group' do context 'when board parent is a group' do
let(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:board) { create(:board, group: group) } let_it_be_with_reload(:board) { create(:board, group: group) }
let(:label) { create(:group_label, group: group) } let_it_be(:label) { create(:group_label, group: group) }
let!(:list) { create(:list, board: board, label: label) } let_it_be(:list) { create(:list, board: board, label: label) }
let_it_be(:unrelated_list) { create(:list) }
let(:parent) { group } let(:parent) { group }
it_behaves_like 'lists list service' 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 end
end end
...@@ -16,18 +16,23 @@ RSpec.shared_examples 'boards listable model' do |list_factory| ...@@ -16,18 +16,23 @@ RSpec.shared_examples 'boards listable model' do |list_factory|
end end
describe 'scopes' do 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 describe '.ordered' do
it 'returns lists ordered by type and position' do it 'returns lists ordered by type and position' do
# rubocop:disable Rails/SaveBang expect(described_class.where(id: [list1, list2, list3, list4]).ordered)
lists = [ .to eq([list1, list3, list4, list2])
create(list_factory, list_type: :backlog), end
create(list_factory, list_type: :closed), end
create(list_factory, position: 1),
create(list_factory, position: 2) describe '.without_types' do
] it 'excludes lists of given types' do
# rubocop:enable Rails/SaveBang lists = described_class.without_types([:label, :closed])
expect(described_class.where(id: lists).ordered).to eq([lists[0], lists[2], lists[3], lists[1]]) expect(lists).to match_array([list1])
end end
end end
end end
......
...@@ -13,7 +13,7 @@ RSpec.shared_examples 'lists destroy service' do ...@@ -13,7 +13,7 @@ RSpec.shared_examples 'lists destroy service' do
development = create(:list, board: board, position: 0) development = create(:list, board: board, position: 0)
review = create(:list, board: board, position: 1) review = create(:list, board: board, position: 1)
staging = create(:list, board: board, position: 2) staging = create(:list, board: board, position: 2)
closed = board.closed_list closed = board.lists.closed.first
described_class.new(parent, user).execute(development) described_class.new(parent, user).execute(development)
...@@ -24,7 +24,7 @@ RSpec.shared_examples 'lists destroy service' do ...@@ -24,7 +24,7 @@ RSpec.shared_examples 'lists destroy service' do
end end
it 'does not remove list from board when list type is closed' do 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) service = described_class.new(parent, user)
expect { service.execute(list) }.not_to change(board.lists, :count) expect { service.execute(list) }.not_to change(board.lists, :count)
......
...@@ -2,14 +2,34 @@ ...@@ -2,14 +2,34 @@
RSpec.shared_examples 'lists list service' do RSpec.shared_examples 'lists list service' do
context 'when the board has a backlog list' 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 it 'does not create a backlog list' do
expect { service.execute(board) }.not_to change(board.lists, :count) expect { service.execute(board) }.not_to change(board.lists, :count)
end end
it "returns board's lists" do 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
end end
...@@ -23,25 +43,21 @@ RSpec.shared_examples 'lists list service' do ...@@ -23,25 +43,21 @@ RSpec.shared_examples 'lists list service' do
end end
it "returns board's lists" do 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
end end
context 'when wanting a specific list' do context 'when wanting a specific list' do
let!(:list1) { create(:list, board: board) }
it 'returns list specified by id' do 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 end
it 'returns empty result when list is not found' do it 'returns empty result when list is not found' do
external_board = create(:board, resource_parent: create(:project)) service = described_class.new(parent, user, list_id: unrelated_list.id)
external_list = create(:list, board: external_board)
service = described_class.new(parent, user, list_id: external_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 end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment