Commit 3c1652f3 authored by Patrick Derichs's avatar Patrick Derichs

Add limit_metric to lists table

Update responses so limit_metric is also provided
to frontend

Add validation for limit_metric values

Move label declaration to top and remove redundant declarations

Refactor list create attributes

Refactor lists max limits update
parent 6f09f635
---
title: Add limit metric to lists
merge_request: 25532
author:
type: added
# frozen_string_literal: true
class AddLimitMetricTypeToList < ActiveRecord::Migration[6.0]
DOWNTIME = false
def change
add_column :lists, :limit_metric, :string, limit: 20
end
end
...@@ -2424,6 +2424,7 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do ...@@ -2424,6 +2424,7 @@ ActiveRecord::Schema.define(version: 2020_03_13_123934) do
t.integer "milestone_id" t.integer "milestone_id"
t.integer "max_issue_count", default: 0, null: false t.integer "max_issue_count", default: 0, null: false
t.integer "max_issue_weight", default: 0, null: false t.integer "max_issue_weight", default: 0, null: false
t.string "limit_metric", limit: 20
t.index ["board_id", "label_id"], name: "index_lists_on_board_id_and_label_id", unique: true t.index ["board_id", "label_id"], name: "index_lists_on_board_id_and_label_id", unique: true
t.index ["label_id"], name: "index_lists_on_label_id" t.index ["label_id"], name: "index_lists_on_label_id"
t.index ["list_type"], name: "index_lists_on_list_type" t.index ["list_type"], name: "index_lists_on_list_type"
......
...@@ -9,7 +9,7 @@ module EE ...@@ -9,7 +9,7 @@ module EE
before_action :push_wip_limits before_action :push_wip_limits
end end
EE_MAX_LIMITS_PARAMS = %i[max_issue_count max_issue_weight].freeze EE_MAX_LIMITS_PARAMS = %i[max_issue_count max_issue_weight limit_metric].freeze
override :list_creation_attrs override :list_creation_attrs
def list_creation_attrs def list_creation_attrs
......
...@@ -6,6 +6,8 @@ module EE ...@@ -6,6 +6,8 @@ module EE
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
LIMIT_METRIC_TYPES = %w[all_metrics issue_count issue_weights].freeze
# ActiveSupport::Concern does not prepend the ClassMethods, # ActiveSupport::Concern does not prepend the ClassMethods,
# so we cannot call `super` if we use it. # so we cannot call `super` if we use it.
def self.prepended(base) def self.prepended(base)
...@@ -24,6 +26,11 @@ module EE ...@@ -24,6 +26,11 @@ module EE
base.validates :milestone_id, uniqueness: { scope: :board_id }, if: :milestone? base.validates :milestone_id, uniqueness: { scope: :board_id }, if: :milestone?
base.validates :max_issue_count, numericality: { only_integer: true, greater_than_or_equal_to: 0 } base.validates :max_issue_count, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
base.validates :max_issue_weight, numericality: { only_integer: true, greater_than_or_equal_to: 0 } base.validates :max_issue_weight, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
base.validates :limit_metric, inclusion: {
in: LIMIT_METRIC_TYPES,
allow_blank: true,
allow_nil: true
}
base.validates :list_type, base.validates :list_type,
exclusion: { in: %w[assignee], message: _('Assignee lists not available with your current license') }, exclusion: { in: %w[assignee], message: _('Assignee lists not available with your current license') },
unless: -> { board&.resource_parent&.feature_available?(:board_assignee_lists) } unless: -> { board&.resource_parent&.feature_available?(:board_assignee_lists) }
......
...@@ -37,13 +37,13 @@ module EE ...@@ -37,13 +37,13 @@ module EE
override :create_list_attributes override :create_list_attributes
def create_list_attributes(type, target, position) def create_list_attributes(type, target, position)
max_issue_count, max_issue_weight = if wip_limits_available? return super unless wip_limits_available?
[max_issue_count_by_params, max_issue_weight_by_params]
else
[0, 0]
end
super.merge(max_issue_count: max_issue_count, max_issue_weight: max_issue_weight) super.merge(
max_issue_count: max_issue_count_by_params,
max_issue_weight: max_issue_weight_by_params,
limit_metric: limit_metric_by_params
)
end end
private private
...@@ -71,6 +71,10 @@ module EE ...@@ -71,6 +71,10 @@ module EE
def wip_limits_available? def wip_limits_available?
parent.feature_available?(:wip_limits) parent.feature_available?(:wip_limits)
end end
def limit_metric_by_params
params[:limit_metric]
end
end end
end end
end end
......
...@@ -18,13 +18,25 @@ module EE ...@@ -18,13 +18,25 @@ module EE
end end
def update_max_limits(list) def update_max_limits(list)
return unless max_limits_update_possible?(list) return unless list.wip_limits_available? && can_admin?(list)
list.update(list_max_limit_attributes_by_params) attrs = max_limit_settings_by_params
list.update(attrs) unless attrs.empty?
end end
def max_limits_update_possible?(list) def max_limit_settings_by_params
max_limits_provided? && list.wip_limits_available? && can_admin?(list) {}.tap do |attrs|
attrs.merge!(list_max_limit_attributes_by_params) if max_limits_provided?
attrs.merge!(limit_metric_by_params) if limit_metric_provided?
end
end
def limit_metric_by_params
{ limit_metric: params[:limit_metric] }
end
def limit_metric_provided?
params.key?(:limit_metric)
end end
end end
end end
......
...@@ -3,10 +3,11 @@ ...@@ -3,10 +3,11 @@
require 'spec_helper' require 'spec_helper'
describe Boards::ListsController do describe Boards::ListsController do
let(:group) { create(:group, :private) } let_it_be(:group) { create(:group, :private) }
let(:board) { create(:board, group: group) } let_it_be(:board) { create(:board, group: group) }
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:guest) { create(:user) } let_it_be(:guest) { create(:user) }
let_it_be(:label) { create(:group_label, group: group, name: 'Development') }
before do before do
group.add_maintainer(user) group.add_maintainer(user)
...@@ -51,8 +52,6 @@ describe Boards::ListsController do ...@@ -51,8 +52,6 @@ describe Boards::ListsController do
describe 'POST create' do describe 'POST create' do
context 'with valid params' do context 'with valid params' do
let(:label) { create(:group_label, group: group, name: 'Development') }
it 'returns a successful 200 response' do it 'returns a successful 200 response' do
create_board_list user: user, board: board, label_id: label.id create_board_list user: user, board: board, label_id: label.id
...@@ -67,8 +66,6 @@ describe Boards::ListsController do ...@@ -67,8 +66,6 @@ describe Boards::ListsController do
end end
context 'with max issue count' do context 'with max issue count' do
let(:label) { create(:group_label, group: group, name: 'Development') }
context 'with licensed wip limits' do context 'with licensed wip limits' do
it 'returns the created list' do it 'returns the created list' do
create_board_list user: user, board: board, label_id: label.id, params: { max_issue_count: 2 } create_board_list user: user, board: board, label_id: label.id, params: { max_issue_count: 2 }
...@@ -93,8 +90,6 @@ describe Boards::ListsController do ...@@ -93,8 +90,6 @@ describe Boards::ListsController do
end end
context 'with max issue weight' do context 'with max issue weight' do
let(:label) { create(:group_label, group: group, name: 'Development') }
context 'with licensed wip limits' do context 'with licensed wip limits' do
it 'returns the created list' do it 'returns the created list' do
create_board_list user: user, board: board, label_id: label.id, params: { max_issue_weight: 3 } create_board_list user: user, board: board, label_id: label.id, params: { max_issue_weight: 3 }
...@@ -118,6 +113,58 @@ describe Boards::ListsController do ...@@ -118,6 +113,58 @@ describe Boards::ListsController do
end end
end end
context 'with limit metric' do
shared_examples 'a limit metric response' do
it 'returns the created list with expected limit_metric' do
create_board_list user: user, board: board, label_id: label.id, params: { limit_metric: limit_metric }
expect(response).to match_response_schema('list', dir: 'ee')
expect(json_response).to include('limit_metric' => limit_metric.to_s)
end
end
context 'with licensed wip limits' do
it_behaves_like 'a limit metric response' do
let(:limit_metric) { :issue_weights }
end
it_behaves_like 'a limit metric response' do
let(:limit_metric) { :issue_count }
end
it_behaves_like 'a limit metric response' do
let(:limit_metric) { :all_metrics }
end
it_behaves_like 'a limit metric response' do
let(:limit_metric) { '' }
end
it_behaves_like 'a limit metric response' do
let(:limit_metric) { nil }
end
it 'fails with an unknown limit metric' do
create_board_list user: user, board: board, label_id: label.id, params: { limit_metric: 'foo' }
expect(response).to have_gitlab_http_status(:unprocessable_entity)
end
end
context 'without licensed wip limits' do
before do
stub_feature_flags(wip_limits: false)
end
it 'ignores limit metric setting' do
create_board_list user: user, board: board, label_id: label.id, params: { limit_metric: 'issue_weights' }
expect(response).to match_response_schema('list', dir: 'ee')
expect(json_response).not_to include('limit_metric')
end
end
end
context 'with invalid params' do context 'with invalid params' do
context 'when label is nil' do context 'when label is nil' do
it 'returns a not found 404 response' do it 'returns a not found 404 response' do
...@@ -140,8 +187,6 @@ describe Boards::ListsController do ...@@ -140,8 +187,6 @@ describe Boards::ListsController do
context 'with unauthorized user' do context 'with unauthorized user' do
it 'returns a forbidden 403 response' do it 'returns a forbidden 403 response' do
label = create(:group_label, group: group, name: 'Development')
create_board_list user: guest, board: board, label_id: label.id create_board_list user: guest, board: board, label_id: label.id
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
...@@ -202,7 +247,7 @@ describe Boards::ListsController do ...@@ -202,7 +247,7 @@ describe Boards::ListsController do
end end
context 'multiple fields update behavior' do context 'multiple fields update behavior' do
shared_examples 'field updates' do shared_examples 'a list update request' do
it 'updates fields as expected' do it 'updates fields as expected' do
params = update_params_with_list_params(update_params) params = update_params_with_list_params(update_params)
...@@ -215,88 +260,148 @@ describe Boards::ListsController do ...@@ -215,88 +260,148 @@ describe Boards::ListsController do
expect(reloaded_list.preferences_for(user).collapsed).to eq(expected_collapsed) expect(reloaded_list.preferences_for(user).collapsed).to eq(expected_collapsed)
expect(reloaded_list.max_issue_count).to eq(expected_max_issue_count) expect(reloaded_list.max_issue_count).to eq(expected_max_issue_count)
expect(reloaded_list.max_issue_weight).to eq(expected_max_issue_weight) expect(reloaded_list.max_issue_weight).to eq(expected_max_issue_weight)
expect(reloaded_list.limit_metric).to eq(expected_limit_metric)
end end
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_count: 99, position: 0, collapsed: true } } let(:update_params) { { max_issue_count: 99, position: 0, collapsed: true } }
let(:expected_position) { 0 } let(:expected_position) { 0 }
let(:expected_collapsed) { true } let(:expected_collapsed) { true }
let(:expected_max_issue_count) { 99 } let(:expected_max_issue_count) { 99 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { position: 0, collapsed: true } } let(:update_params) { { position: 0, collapsed: true } }
let(:expected_position) { 0 } let(:expected_position) { 0 }
let(:expected_collapsed) { true } let(:expected_collapsed) { true }
let(:expected_max_issue_count) { 0 } let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { position: 0 } } let(:update_params) { { position: 0 } }
let(:expected_position) { 0 } let(:expected_position) { 0 }
let(:expected_collapsed) { nil } let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 } let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_count: 42 } } let(:update_params) { { max_issue_count: 42 } }
let(:expected_position) { 1 } let(:expected_position) { 1 }
let(:expected_collapsed) { nil } let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 42 } let(:expected_max_issue_count) { 42 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { collapsed: true } } let(:update_params) { { collapsed: true } }
let(:expected_position) { 1 } let(:expected_position) { 1 }
let(:expected_collapsed) { true } let(:expected_collapsed) { true }
let(:expected_max_issue_count) { 0 } let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_count: 42, collapsed: true } } let(:update_params) { { max_issue_count: 42, collapsed: true } }
let(:expected_position) { 1 } let(:expected_position) { 1 }
let(:expected_collapsed) { true } let(:expected_collapsed) { true }
let(:expected_max_issue_count) { 42 } let(:expected_max_issue_count) { 42 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_count: 42, position: 0 } } let(:update_params) { { max_issue_count: 42, position: 0 } }
let(:expected_position) { 0 } let(:expected_position) { 0 }
let(:expected_collapsed) { nil } let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 42 } let(:expected_max_issue_count) { 42 }
let(:expected_max_issue_weight) { 0 } let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_weight: 42, position: 0 } } let(:update_params) { { max_issue_weight: 42, position: 0 } }
let(:expected_position) { 0 } let(:expected_position) { 0 }
let(:expected_collapsed) { nil } let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 } let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 42 } let(:expected_max_issue_weight) { 42 }
let(:expected_limit_metric) { nil }
end end
it_behaves_like 'field updates' do it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_count: 99, max_issue_weight: 42, position: 0 } } let(:update_params) { { max_issue_count: 99, max_issue_weight: 42, position: 0 } }
let(:expected_position) { 0 } let(:expected_position) { 0 }
let(:expected_collapsed) { nil } let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 99 } let(:expected_max_issue_count) { 99 }
let(:expected_max_issue_weight) { 42 } let(:expected_max_issue_weight) { 42 }
let(:expected_limit_metric) { nil }
end
it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_weight: 42 } }
let(:expected_position) { 1 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 42 }
let(:expected_limit_metric) { nil }
end
it_behaves_like 'a list update request' do
let(:update_params) { { max_issue_weight: 42, limit_metric: 'issue_weights' } }
let(:expected_position) { 1 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 42 }
let(:expected_limit_metric) { 'issue_weights' }
end
it_behaves_like 'a list update request' do
let(:update_params) { { limit_metric: 'issue_weights' } }
let(:expected_position) { 1 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { 'issue_weights' }
end
it_behaves_like 'a list update request' do
let(:update_params) { { limit_metric: 'issue_count' } }
let(:expected_position) { 1 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 0 }
let(:expected_limit_metric) { 'issue_count' }
end
it_behaves_like 'a list update request' do
let(:update_params) { { limit_metric: 'issue_count', max_issue_count: 100, max_issue_weight: 10 } }
let(:expected_position) { 1 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 100 }
let(:expected_max_issue_weight) { 10 }
let(:expected_limit_metric) { 'issue_count' }
end end
end end
......
...@@ -13,6 +13,7 @@ describe List do ...@@ -13,6 +13,7 @@ describe List do
describe 'validations' do describe 'validations' do
it { is_expected.to validate_numericality_of(:max_issue_count).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:max_issue_count).only_integer.is_greater_than_or_equal_to(0) }
it { is_expected.to validate_numericality_of(:max_issue_weight).only_integer.is_greater_than_or_equal_to(0) } it { is_expected.to validate_numericality_of(:max_issue_weight).only_integer.is_greater_than_or_equal_to(0) }
it { is_expected.to validate_inclusion_of(:limit_metric).in_array([nil, *EE::List::LIMIT_METRIC_TYPES]) }
end end
context 'when it is an assignee type' do context 'when it is an assignee type' do
......
...@@ -9,6 +9,41 @@ describe 'EE::Boards::Lists::UpdateService' do ...@@ -9,6 +9,41 @@ describe 'EE::Boards::Lists::UpdateService' do
shared_examples 'board list update' do shared_examples 'board list update' do
context 'with licensed wip limits' do context 'with licensed wip limits' do
context 'limit metric' do
it 'updates the list if limit_metric "issue_count" is given' do
update_list_and_test_result(list, { limit_metric: 'issue_count' }, { limit_metric: 'issue_count' })
end
it 'updates the list if limit_metric "issue_weights" is given' do
update_list_and_test_result(list, { limit_metric: 'issue_weights' }, { limit_metric: 'issue_weights' })
end
it 'updates the list if "all_metrics" limit_metric is given' do
update_list_and_test_result(list, { limit_metric: 'all_metrics' }, { limit_metric: 'all_metrics' })
end
it 'updates the list if "all_metrics" limit_metric is given' do
update_list_and_test_result(list, { limit_metric: '' }, { limit_metric: '' })
end
it 'updates the list if no limit_metric is given' do
list.update!(limit_metric: 'issue_count')
update_list_and_test_result(list, { limit_metric: nil }, { limit_metric: nil })
end
it 'fails if an invalid limit_metric is given' do
service = Boards::Lists::UpdateService.new(board, user, { limit_metric: 'foo' })
result = service.execute(list)
expect(result[:http_status]).to eq(422)
expect(result[:status]).to eq(:error)
reloaded_list = list.reload
expect(reloaded_list.limit_metric).to be_nil
end
end
it 'updates the list if max_issue_count is given' do it 'updates the list if max_issue_count is given' do
update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_count: 42 }) update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_count: 42 })
end end
...@@ -17,80 +52,110 @@ describe 'EE::Boards::Lists::UpdateService' do ...@@ -17,80 +52,110 @@ describe 'EE::Boards::Lists::UpdateService' do
update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_weight: 42 }) update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_weight: 42 })
end end
it 'updates the list if max_issue_count is nil' do it 'does not update the list if max_issue_count is nil' do
update_list_and_test_result(list, { max_issue_count: nil }, { max_issue_count: 0 }) update_list_and_test_result(list,
{ max_issue_count: nil },
{ max_issue_count: 0 },
expected_service_result: :error)
end end
it 'updates the list if max_issue_weight is nil' do it 'does not update the list if max_issue_weight is nil' do
update_list_and_test_result(list, { max_issue_weight: nil }, { max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_weight: nil },
{ max_issue_weight: 0 },
expected_service_result: :error)
end end
it 'updates the max issue count of the list if both count and weight limits are provided' do it 'updates the max issue count of the list if both count and weight limits are provided' do
update_list_and_test_result(list, { max_issue_count: 10, max_issue_weight: 42 }, { max_issue_count: 10, max_issue_weight: 42 }) update_list_and_test_result(list,
{ max_issue_count: 10, max_issue_weight: 42 },
{ max_issue_count: 10, max_issue_weight: 42 })
end end
it 'does not change count if weight is updated' do it 'does not change count if weight is updated' do
list.update!(max_issue_count: 55) list.update!(max_issue_count: 55)
update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_count: 55, max_issue_weight: 42 }) update_list_and_test_result(list,
{ max_issue_weight: 42 },
{ max_issue_count: 55, max_issue_weight: 42 })
end end
it 'does not change weight if count is updated' do it 'does not change weight if count is updated' do
list.update!(max_issue_weight: 55) list.update!(max_issue_weight: 55)
update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_weight: 55, max_issue_count: 42 }) update_list_and_test_result(list,
{ max_issue_count: 42 },
{ max_issue_weight: 55, max_issue_count: 42 })
end end
it 'does not update max_issue_count if max_issue_count is nil' do it 'does not update max_issue_count if max_issue_count is nil' do
update_list_and_test_result(list, { max_issue_count: nil }, { max_issue_count: 0 }) update_list_and_test_result(list,
{ max_issue_count: nil },
{ max_issue_count: 0 },
expected_service_result: :error)
end end
it 'sets max_issue_count to 0 if requested' do it 'sets max_issue_count to 0 if requested' do
list.update!(max_issue_count: 3) list.update!(max_issue_count: 3)
update_list_and_test_result(list, { max_issue_count: 0 }, { max_issue_count: 0 }) update_list_and_test_result(list,
{ max_issue_count: 0 },
{ max_issue_count: 0 })
end end
it 'sets max_issue_weight to 0 if requested' do it 'sets max_issue_weight to 0 if requested' do
list.update!(max_issue_weight: 3) list.update!(max_issue_weight: 3)
update_list_and_test_result(list, { max_issue_weight: 0 }, { max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_weight: 0 },
{ max_issue_weight: 0 })
end end
it 'sets max_issue_count to 0 if requested' do it 'sets max_issue_count to 0 if requested' do
list.update!(max_issue_count: 10) list.update!(max_issue_count: 10)
update_list_and_test_result(list, { max_issue_count: 0, max_issue_weight: 0 }, { max_issue_count: 0, max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_count: 0, max_issue_weight: 0 },
{ max_issue_count: 0, max_issue_weight: 0 })
end end
it 'sets max_issue_weight to 0 if requested' do it 'sets max_issue_weight to 0 if requested' do
list.update!(max_issue_weight: 10) list.update!(max_issue_weight: 10)
update_list_and_test_result(list, { max_issue_count: 0, max_issue_weight: 0 }, { max_issue_count: 0, max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_count: 0, max_issue_weight: 0 },
{ max_issue_count: 0, max_issue_weight: 0 })
end end
it 'does not update count and weight when negative values for both are given' do it 'does not update count and weight when negative values for both are given' do
list.update!(max_issue_count: 10) list.update!(max_issue_count: 10)
update_list_and_test_result(list, { max_issue_count: -1, max_issue_weight: -1 }, { max_issue_count: 10, max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_count: -1, max_issue_weight: -1 },
{ max_issue_count: 10, max_issue_weight: 0 },
expected_service_result: :error)
end end
it 'sets count and weight to 0 when non numerical values are given' do it 'sets count and weight to 0 when non numerical values are given' do
list.update!(max_issue_count: 0, max_issue_weight: 3) list.update!(max_issue_count: 0, max_issue_weight: 3)
update_list_and_test_result(list, { max_issue_count: 'test', max_issue_weight: 'test2' }, { max_issue_count: 0, max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_count: 'test', max_issue_weight: 'test2' },
{ max_issue_count: 0, max_issue_weight: 0 })
end end
it 'does not update the list max issue count if can_admin returns false' do it 'does not update the list max issue count if can_admin returns false' do
update_list_and_test_result_by_user(unpriviledged_user, list, update_list_and_test_result_by_user(unpriviledged_user, list,
{ max_issue_count: 42 }, { max_issue_count: 42 },
{ max_issue_count: 0 }) { max_issue_count: 0 },
expected_service_result: :error)
end end
it 'does not update the list max issue weight if can_admin returns false' do it 'does not update the list max issue weight if can_admin returns false' do
update_list_and_test_result_by_user(unpriviledged_user, list, update_list_and_test_result_by_user(unpriviledged_user, list,
{ max_issue_weight: 42 }, { max_issue_weight: 42 },
{ max_issue_weight: 0 }) { max_issue_weight: 0 },
expected_service_result: :error)
end end
end end
...@@ -100,34 +165,55 @@ describe 'EE::Boards::Lists::UpdateService' do ...@@ -100,34 +165,55 @@ describe 'EE::Boards::Lists::UpdateService' do
end end
it 'does not update the list even if max_issue_count is given' do it 'does not update the list even if max_issue_count is given' do
update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_count: 0 }) update_list_and_test_result(list,
{ max_issue_count: 42 },
{ max_issue_count: 0 },
expected_service_result: :error)
end end
it 'does not update the list if can_admin returns false' do it 'does not update the list if can_admin returns false' do
update_list_and_test_result_by_user(unpriviledged_user, list, { max_issue_count: 42 }, { max_issue_count: 0 }) update_list_and_test_result_by_user(unpriviledged_user,
list,
{ max_issue_count: 42 },
{ max_issue_count: 0 },
expected_service_result: :error)
end end
it 'does not update the list even if max_issue_weight is given' do it 'does not update the list even if max_issue_weight is given' do
update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_weight: 0 }) update_list_and_test_result(list,
{ max_issue_weight: 42 },
{ max_issue_weight: 0 },
expected_service_result: :error)
end end
it 'does not update the list if can_admin returns false' do it 'does not update the list if can_admin returns false' do
update_list_and_test_result_by_user(unpriviledged_user, list, { max_issue_weight: 42 }, { max_issue_weight: 0 }) update_list_and_test_result_by_user(unpriviledged_user,
list,
{ max_issue_weight: 42 },
{ max_issue_weight: 0 },
expected_service_result: :error)
end end
end end
def update_list_and_test_result(list, initialization_params, expected_list_attributes) def update_list_and_test_result(list, initialization_params, expected_list_attributes, expected_service_result: :success)
update_list_and_test_result_by_user(user, list, initialization_params, expected_list_attributes) update_list_and_test_result_by_user(user,
list,
initialization_params,
expected_list_attributes,
expected_service_result: expected_service_result)
end end
def update_list_and_test_result_by_user(user, list, initialization_params, expected_list_attributes) def update_list_and_test_result_by_user(user, list, initialization_params, expected_list_attributes, expected_service_result: :success)
service = Boards::Lists::UpdateService.new(board, user, initialization_params) service = Boards::Lists::UpdateService.new(board, user, initialization_params)
expect(service.execute(list)).to be_truthy result = service.execute(list)
expect(result[:status]).to eq(expected_service_result)
reloaded_list = list.reload reloaded_list = list.reload
expect(reloaded_list.max_issue_count).to eq(expected_list_attributes.fetch(:max_issue_count, 0)) expect(reloaded_list.max_issue_count).to eq(expected_list_attributes.fetch(:max_issue_count, 0))
expect(reloaded_list.max_issue_weight).to eq(expected_list_attributes.fetch(:max_issue_weight, 0)) expect(reloaded_list.max_issue_weight).to eq(expected_list_attributes.fetch(:max_issue_weight, 0))
expect(reloaded_list.limit_metric).to eq(expected_list_attributes[:limit_metric])
end end
end end
......
...@@ -55,24 +55,30 @@ describe Boards::Lists::CreateService do ...@@ -55,24 +55,30 @@ describe Boards::Lists::CreateService do
stub_feature_flags(wip_limits: wip_limits_enabled) stub_feature_flags(wip_limits: wip_limits_enabled)
end end
where(:params, :expected_max_issue_count, :expected_max_issue_weight) do where(:params, :expected_max_issue_count, :expected_max_issue_weight, :expected_limit_metric) do
[ [
[{ max_issue_count: 0 }, 0, 0], [{ max_issue_count: 0 }, 0, 0, nil],
[{ max_issue_count: nil }, 0, 0], [{ max_issue_count: nil }, 0, 0, nil],
[{ max_issue_count: 1 }, 1, 0], [{ max_issue_count: 1 }, 1, 0, nil],
[{ max_issue_weight: 0 }, 0, 0], [{ max_issue_weight: 0 }, 0, 0, nil],
[{ max_issue_weight: nil }, 0, 0], [{ max_issue_weight: nil }, 0, 0, nil],
[{ max_issue_weight: 1 }, 0, 1], [{ max_issue_weight: 1 }, 0, 1, nil],
[{ max_issue_count: 1, max_issue_weight: 0 }, 1, 0], [{ max_issue_count: 1, max_issue_weight: 0 }, 1, 0, nil],
[{ max_issue_count: 0, max_issue_weight: 1 }, 0, 1], [{ max_issue_count: 0, max_issue_weight: 1 }, 0, 1, nil],
[{ max_issue_count: 1, max_issue_weight: 1 }, 1, 1], [{ max_issue_count: 1, max_issue_weight: 1 }, 1, 1, nil],
[{ max_issue_count: nil, max_issue_weight: 1 }, 0, 1], [{ max_issue_count: nil, max_issue_weight: 1 }, 0, 1, nil],
[{ max_issue_count: 1, max_issue_weight: nil }, 1, 0], [{ max_issue_count: 1, max_issue_weight: nil }, 1, 0, nil],
[{ max_issue_count: nil, max_issue_weight: nil }, 0, 0] [{ max_issue_count: nil, max_issue_weight: nil }, 0, 0, nil],
[{ limit_metric: 'all_metrics' }, 0, 0, 'all_metrics'],
[{ limit_metric: 'issue_count' }, 0, 0, 'issue_count'],
[{ limit_metric: 'issue_weights' }, 0, 0, 'issue_weights'],
[{ limit_metric: '' }, 0, 0, ''],
[{ limit_metric: nil }, 0, 0, nil]
] ]
end end
...@@ -83,9 +89,11 @@ describe Boards::Lists::CreateService do ...@@ -83,9 +89,11 @@ describe Boards::Lists::CreateService do
attrs = service.create_list_attributes(nil, nil, nil) attrs = service.create_list_attributes(nil, nil, nil)
if wip_limits_enabled if wip_limits_enabled
expect(attrs).to include(max_issue_count: expected_max_issue_count, max_issue_weight: expected_max_issue_weight) expect(attrs).to include(max_issue_count: expected_max_issue_count,
max_issue_weight: expected_max_issue_weight,
limit_metric: expected_limit_metric)
else else
expect(attrs).to include(max_issue_count: 0, max_issue_weight: 0) expect(attrs).not_to include(max_issue_count: 0, max_issue_weight: 0, limit_metric: nil)
end end
end end
end end
......
...@@ -7,6 +7,7 @@ FactoryBot.define do ...@@ -7,6 +7,7 @@ FactoryBot.define do
list_type { :label } list_type { :label }
max_issue_count { 0 } max_issue_count { 0 }
max_issue_weight { 0 } max_issue_weight { 0 }
limit_metric { nil }
sequence(:position) sequence(:position)
end end
......
...@@ -37,7 +37,8 @@ ...@@ -37,7 +37,8 @@
"title": { "type": "string" }, "title": { "type": "string" },
"position": { "type": ["integer", "null"] }, "position": { "type": ["integer", "null"] },
"max_issue_count": { "type": "integer" }, "max_issue_count": { "type": "integer" },
"max_issue_weight": { "type": "integer" } "max_issue_weight": { "type": "integer" },
"limit_metric": { "type": ["string", "null"] }
}, },
"additionalProperties": true "additionalProperties": true
} }
...@@ -756,6 +756,7 @@ List: ...@@ -756,6 +756,7 @@ List:
- user_id - user_id
- max_issue_count - max_issue_count
- max_issue_weight - max_issue_weight
- limit_metric
ExternalPullRequest: ExternalPullRequest:
- id - id
- created_at - created_at
......
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