Commit 124e4170 authored by Patrick Derichs's avatar Patrick Derichs

Add add max_issue_weight to lists

Extract a module MaxLimits for lists to make
limits behavior reusable.

Refactor existing logic a bit

Remove explicit namespaces and rubocop
disable comment in includes
parent 5b30e143
---
title: Add possibility to save max issue weight on lists
merge_request: 19220
author:
type: added
# frozen_string_literal: true
class AddMaxIssueWeightToList < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
add_column_with_default :lists, :max_issue_weight, :integer, default: 0
end
def down
remove_column :lists, :max_issue_weight
end
end
...@@ -2250,6 +2250,7 @@ ActiveRecord::Schema.define(version: 2019_11_19_023952) do ...@@ -2250,6 +2250,7 @@ ActiveRecord::Schema.define(version: 2019_11_19_023952) do
t.integer "user_id" t.integer "user_id"
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.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"
......
...@@ -49,7 +49,8 @@ Example response: ...@@ -49,7 +49,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 2, "id" : 2,
...@@ -59,7 +60,8 @@ Example response: ...@@ -59,7 +60,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 2, "position" : 2,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 3, "id" : 3,
...@@ -69,7 +71,8 @@ Example response: ...@@ -69,7 +71,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 3, "position" : 3,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
] ]
} }
...@@ -121,7 +124,8 @@ Example response: ...@@ -121,7 +124,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 2, "id" : 2,
...@@ -131,7 +135,8 @@ Example response: ...@@ -131,7 +135,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 2, "position" : 2,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 3, "id" : 3,
...@@ -141,7 +146,8 @@ Example response: ...@@ -141,7 +146,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 3, "position" : 3,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
] ]
} }
...@@ -192,7 +198,8 @@ Example response: ...@@ -192,7 +198,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 2, "id" : 2,
...@@ -202,7 +209,8 @@ Example response: ...@@ -202,7 +209,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 2, "position" : 2,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 3, "id" : 3,
...@@ -212,7 +220,8 @@ Example response: ...@@ -212,7 +220,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 3, "position" : 3,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
] ]
} }
...@@ -346,7 +355,8 @@ Example response: ...@@ -346,7 +355,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 2, "id" : 2,
...@@ -356,7 +366,8 @@ Example response: ...@@ -356,7 +366,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 2, "position" : 2,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
}, },
{ {
"id" : 3, "id" : 3,
...@@ -366,7 +377,8 @@ Example response: ...@@ -366,7 +377,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 3, "position" : 3,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
] ]
``` ```
...@@ -400,7 +412,8 @@ Example response: ...@@ -400,7 +412,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
``` ```
...@@ -441,7 +454,8 @@ Example response: ...@@ -441,7 +454,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
``` ```
...@@ -475,7 +489,8 @@ Example response: ...@@ -475,7 +489,8 @@ Example response:
"description" : null "description" : null
}, },
"position" : 1, "position" : 1,
"max_issue_count": 0 "max_issue_count": 0,
"max_issue_weight": 0
} }
``` ```
......
...@@ -9,10 +9,12 @@ module EE ...@@ -9,10 +9,12 @@ 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
override :list_creation_attrs override :list_creation_attrs
def list_creation_attrs def list_creation_attrs
additional_attrs = %i[assignee_id milestone_id] additional_attrs = %i[assignee_id milestone_id]
additional_attrs << :max_issue_count if wip_limits_available? additional_attrs += EE_MAX_LIMITS_PARAMS if wip_limits_available?
super + additional_attrs super + additional_attrs
end end
...@@ -21,13 +23,13 @@ module EE ...@@ -21,13 +23,13 @@ module EE
def list_update_attrs def list_update_attrs
return super unless wip_limits_available? return super unless wip_limits_available?
super + %i[max_issue_count] super + EE_MAX_LIMITS_PARAMS
end end
override :serialization_attrs override :serialization_attrs
def serialization_attrs def serialization_attrs
super.merge(user: true, milestone: true).tap do |attrs| super.merge(user: true, milestone: true).tap do |attrs|
attrs[:only] << :max_issue_count if wip_limits_available? attrs[:only] += EE_MAX_LIMITS_PARAMS if wip_limits_available?
end end
end end
......
...@@ -4,6 +4,8 @@ module EE ...@@ -4,6 +4,8 @@ module EE
module List module List
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
include ::Gitlab::Utils::StrongMemoize
# 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)
...@@ -21,6 +23,7 @@ module EE ...@@ -21,6 +23,7 @@ module EE
base.validates :user_id, uniqueness: { scope: :board_id }, if: :assignee? base.validates :user_id, uniqueness: { scope: :board_id }, if: :assignee?
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 :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) }
...@@ -33,6 +36,12 @@ module EE ...@@ -33,6 +36,12 @@ module EE
self.user = user self.user = user
end end
def wip_limits_available?
strong_memoize(:wip_limits_available) do
board.resource_parent.feature_available?(:wip_limits)
end
end
override :title override :title
def title def title
case list_type case list_type
......
...@@ -6,6 +6,8 @@ module EE ...@@ -6,6 +6,8 @@ module EE
module CreateService module CreateService
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
include MaxLimits
override :type override :type
def type def type
# We don't ever expect to have more than one list # We don't ever expect to have more than one list
...@@ -35,13 +37,13 @@ module EE ...@@ -35,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 = if wip_limits_available? max_issue_count, max_issue_weight = if wip_limits_available?
params[:max_issue_count] || 0 [max_issue_count_by_params, max_issue_weight_by_params]
else else
0 [0, 0]
end end
super.merge(max_issue_count: max_issue_count) super.merge(max_issue_count: max_issue_count, max_issue_weight: max_issue_weight)
end end
private private
......
# frozen_string_literal: true
module EE
module Boards
module Lists
module MaxLimits
def list_max_limit_attributes_by_params
{}.tap do |list_attrs|
list_attrs[:max_issue_count] = max_issue_count_by_params if max_issue_count?
list_attrs[:max_issue_weight] = max_issue_weight_by_params if max_issue_weight?
end
end
def max_issue_count?
params[:max_issue_count].present?
end
def max_issue_weight?
params[:max_issue_weight].present?
end
def max_limits_provided?
max_issue_count? || max_issue_weight?
end
def max_issue_count_by_params
params.fetch(:max_issue_count, 0).to_i
end
def max_issue_weight_by_params
params.fetch(:max_issue_weight, 0).to_i
end
end
end
end
end
...@@ -6,25 +6,25 @@ module EE ...@@ -6,25 +6,25 @@ module EE
module UpdateService module UpdateService
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
include MaxLimits
private private
override :execute_by_params override :execute_by_params
def execute_by_params(list) def execute_by_params(list)
updated_max_issue_count = update_max_issue_count(list) if can_admin?(list) updated_max_limits = update_max_limits(list)
super || updated_max_issue_count
end
def max_issue_count?(list) super || updated_max_limits
params.has_key?(:max_issue_count) && list.board.resource_parent.feature_available?(:wip_limits)
end end
def update_max_issue_count(list) def update_max_limits(list)
return unless max_issue_count?(list) return unless max_limits_update_possible?(list)
max_issue_count = params[:max_issue_count] || 0 list.update(list_max_limit_attributes_by_params)
end
list.update(max_issue_count: max_issue_count) def max_limits_update_possible?(list)
max_limits_provided? && list.wip_limits_available? && can_admin?(list)
end end
end end
end end
......
...@@ -181,7 +181,8 @@ module EE ...@@ -181,7 +181,8 @@ module EE
prepended do prepended do
expose :milestone, using: ::API::Entities::Milestone, if: -> (entity, _) { entity.milestone? } expose :milestone, using: ::API::Entities::Milestone, if: -> (entity, _) { entity.milestone? }
expose :user, as: :assignee, using: ::API::Entities::UserSafe, if: -> (entity, _) { entity.assignee? } expose :user, as: :assignee, using: ::API::Entities::UserSafe, if: -> (entity, _) { entity.assignee? }
expose :max_issue_count, if: -> (list, _) { list.board.resource_parent.feature_available?(:wip_limits) } expose :max_issue_count, if: -> (list, _) { list.wip_limits_available? }
expose :max_issue_weight, if: -> (list, _) { list.wip_limits_available? }
end end
end end
......
...@@ -75,7 +75,7 @@ describe Boards::ListsController do ...@@ -75,7 +75,7 @@ describe Boards::ListsController do
end end
it 'returns the created list' do it 'returns the created list' do
create_board_list user: user, board: board, label_id: label.id, max_issue_count: 2 create_board_list user: user, board: board, label_id: label.id, params: { max_issue_count: 2 }
expect(response).to match_response_schema('list', dir: 'ee') expect(response).to match_response_schema('list', dir: 'ee')
expect(json_response).to include('max_issue_count' => 2) expect(json_response).to include('max_issue_count' => 2)
...@@ -88,7 +88,7 @@ describe Boards::ListsController do ...@@ -88,7 +88,7 @@ describe Boards::ListsController do
end end
it 'ignores max issue count' do it 'ignores max issue count' do
create_board_list user: user, board: board, label_id: label.id, max_issue_count: 2 create_board_list user: user, board: board, label_id: label.id, params: { max_issue_count: 2 }
expect(response).to match_response_schema('list', dir: 'ee') expect(response).to match_response_schema('list', dir: 'ee')
expect(json_response).not_to include('max_issue_count') expect(json_response).not_to include('max_issue_count')
...@@ -96,6 +96,36 @@ describe Boards::ListsController do ...@@ -96,6 +96,36 @@ describe Boards::ListsController do
end end
end end
context 'with max issue weight' do
let(:label) { create(:group_label, group: group, name: 'Development') }
context 'with licensed wip limits' do
before do
stub_licensed_features(wip_limits: true)
end
it 'returns the created list' do
create_board_list user: user, board: board, label_id: label.id, params: { max_issue_weight: 3 }
expect(response).to match_response_schema('list', dir: 'ee')
expect(json_response).to include('max_issue_weight' => 3)
end
end
context 'without licensed wip limits' do
before do
stub_licensed_features(wip_limits: false)
end
it 'ignores max issue count' do
create_board_list user: user, board: board, label_id: label.id, params: { max_issue_weight: 3 }
expect(response).to match_response_schema('list', dir: 'ee')
expect(json_response).not_to include('max_issue_weight')
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
...@@ -126,12 +156,12 @@ describe Boards::ListsController do ...@@ -126,12 +156,12 @@ describe Boards::ListsController do
end end
end end
def create_board_list(user:, board:, label_id:, max_issue_count: 0) def create_board_list(user:, board:, label_id:, params: {})
sign_in(user) sign_in(user)
post :create, params: { post :create, params: {
board_id: board.to_param, board_id: board.to_param,
list: { label_id: label_id, max_issue_count: max_issue_count } list: { label_id: label_id }.merge(params)
}, },
format: :json format: :json
end end
...@@ -141,19 +171,43 @@ describe Boards::ListsController do ...@@ -141,19 +171,43 @@ describe Boards::ListsController do
let!(:planning) { create(:list, board: board, position: 0) } let!(:planning) { create(:list, board: board, position: 0) }
let!(:development) { create(:list, board: board, position: 1) } let!(:development) { create(:list, board: board, position: 1) }
context 'when updating max issue count' do context 'when updating max limits' do
before do before do
sign_in(user) sign_in(user)
stub_licensed_features(wip_limits: true) stub_licensed_features(wip_limits: true)
end end
it 'returns a successful 200 response' do it 'returns a successful 200 response when max issue count should be updated' do
params = update_params_with_max_issue_count_of(42)
patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(200)
expect(development.reload.max_issue_count).to eq(42)
end
it 'does not overwrite existing weight when max issue count is provided' do
development.update!(max_issue_weight: 22)
params = update_params_with_max_issue_count_of(42) params = update_params_with_max_issue_count_of(42)
patch :update, params: params, as: :json patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(200) expect(response).to have_gitlab_http_status(200)
expect(development.reload.max_issue_count).to eq(42) expect(development.reload.max_issue_count).to eq(42)
expect(development.reload.max_issue_weight).to eq(22)
end
it 'does not overwrite existing count when max issue weight is provided' do
development.update!(max_issue_count: 22)
params = update_params_with_max_issue_weight_of(42)
patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(200)
expect(development.reload.max_issue_weight).to eq(42)
expect(development.reload.max_issue_count).to eq(22)
end end
context 'multiple fields update behavior' do context 'multiple fields update behavior' do
...@@ -169,6 +223,7 @@ describe Boards::ListsController do ...@@ -169,6 +223,7 @@ describe Boards::ListsController do
expect(reloaded_list.position).to eq(expected_position) expect(reloaded_list.position).to eq(expected_position)
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)
end end
end end
...@@ -178,6 +233,7 @@ describe Boards::ListsController do ...@@ -178,6 +233,7 @@ describe Boards::ListsController do
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 }
end end
it_behaves_like 'field updates' do it_behaves_like 'field updates' do
...@@ -186,6 +242,7 @@ describe Boards::ListsController do ...@@ -186,6 +242,7 @@ describe Boards::ListsController do
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 }
end end
it_behaves_like 'field updates' do it_behaves_like 'field updates' do
...@@ -194,6 +251,7 @@ describe Boards::ListsController do ...@@ -194,6 +251,7 @@ describe Boards::ListsController do
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 }
end end
it_behaves_like 'field updates' do it_behaves_like 'field updates' do
...@@ -202,6 +260,7 @@ describe Boards::ListsController do ...@@ -202,6 +260,7 @@ describe Boards::ListsController do
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 }
end end
it_behaves_like 'field updates' do it_behaves_like 'field updates' do
...@@ -210,6 +269,7 @@ describe Boards::ListsController do ...@@ -210,6 +269,7 @@ describe Boards::ListsController do
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 }
end end
it_behaves_like 'field updates' do it_behaves_like 'field updates' do
...@@ -218,6 +278,7 @@ describe Boards::ListsController do ...@@ -218,6 +278,7 @@ describe Boards::ListsController do
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 }
end end
it_behaves_like 'field updates' do it_behaves_like 'field updates' do
...@@ -226,6 +287,25 @@ describe Boards::ListsController do ...@@ -226,6 +287,25 @@ describe Boards::ListsController do
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 }
end
it_behaves_like 'field updates' do
let(:update_params) { { max_issue_weight: 42, position: 0 } }
let(:expected_position) { 0 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 0 }
let(:expected_max_issue_weight) { 42 }
end
it_behaves_like 'field updates' do
let(:update_params) { { max_issue_count: 99, max_issue_weight: 42, position: 0 } }
let(:expected_position) { 0 }
let(:expected_collapsed) { nil }
let(:expected_max_issue_count) { 99 }
let(:expected_max_issue_weight) { 42 }
end end
end end
...@@ -235,7 +315,16 @@ describe Boards::ListsController do ...@@ -235,7 +315,16 @@ describe Boards::ListsController do
patch :update, params: params, as: :json patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(422) expect(response).to have_gitlab_http_status(422)
expect(development.reload.max_issue_count).not_to eq(-1) expect(development.reload.max_issue_count).to eq(0)
end
it 'fails if negative max_issue_weight is provided' do
params = update_params_with_max_issue_weight_of(-1)
patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(422)
expect(development.reload.max_issue_weight).to eq(0)
end end
context 'when wip limits are not licensed' do context 'when wip limits are not licensed' do
...@@ -249,7 +338,16 @@ describe Boards::ListsController do ...@@ -249,7 +338,16 @@ describe Boards::ListsController do
patch :update, params: params, as: :json patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(422) expect(response).to have_gitlab_http_status(422)
expect(development.reload.max_issue_count).not_to eq(2) expect(development.reload.max_issue_count).to eq(0)
end
it 'fails to update max issue weight with expected status' do
params = update_params_with_max_issue_weight_of(2)
patch :update, params: params, as: :json
expect(response).to have_gitlab_http_status(422)
expect(development.reload.max_issue_weight).to eq(0)
end end
end end
...@@ -257,6 +355,10 @@ describe Boards::ListsController do ...@@ -257,6 +355,10 @@ describe Boards::ListsController do
update_params_with_list_params(max_issue_count: count) update_params_with_list_params(max_issue_count: count)
end end
def update_params_with_max_issue_weight_of(count)
update_params_with_list_params(max_issue_weight: count)
end
def update_params_with_list_params(list_update_params) def update_params_with_list_params(list_update_params)
{ namespace_id: group.to_param, { namespace_id: group.to_param,
project_id: board.project, project_id: board.project,
......
...@@ -12,6 +12,7 @@ describe List do ...@@ -12,6 +12,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) }
end end
context 'when it is an assignee type' do context 'when it is an assignee type' do
...@@ -58,4 +59,37 @@ describe List do ...@@ -58,4 +59,37 @@ describe List do
end end
end end
end end
describe '#wip_limits_available?' do
let!(:project) { create(:project) }
let!(:group) { create(:group) }
let!(:board1) { create(:board, resource_parent: project, name: 'b') }
let!(:board2) { create(:board, resource_parent: group, name: 'a') }
let!(:list1) { create(:list, board: board1) }
let!(:list2) { create(:list, board: board2) }
context 'with enabled wip_limits' do
before do
stub_licensed_features(wip_limits: true)
end
it 'returns the expected values' do
expect(list1.wip_limits_available?).to be_truthy
expect(list2.wip_limits_available?).to be_truthy
end
end
context 'with disabled wip_limits' do
before do
stub_licensed_features(wip_limits: false)
end
it 'returns the expected values' do
expect(list1.wip_limits_available?).to be_falsy
expect(list2.wip_limits_available?).to be_falsy
end
end
end
end end
...@@ -43,24 +43,44 @@ describe API::Boards do ...@@ -43,24 +43,44 @@ describe API::Boards do
let!(:list) { create(:list, board: board) } let!(:list) { create(:list, board: board) }
context 'with WIP limits license' do context 'with WIP limits license' do
it 'includes max_issue_count' do before do
stub_licensed_features(wip_limits: true) stub_licensed_features(wip_limits: true)
get api(url, user) get api(url, user)
end
expect(json_response).not_to be_empty it 'includes max_issue_count' do
expect(json_response.all? { |list_response| list_response.include?('max_issue_count') }).to be_truthy all_lists_in_response(include: 'max_issue_count')
end
it 'includes max_issue_weight' do
all_lists_in_response(include: 'max_issue_weight')
end end
end end
context 'without WIP limits license' do context 'without WIP limits license' do
it 'does not include max_issue_count' do before do
stub_licensed_features(wip_limits: false) stub_licensed_features(wip_limits: false)
get api(url, user) get api(url, user)
end
it 'does not include max_issue_weight' do
all_lists_in_response(do_not_include: 'max_issue_weight')
end
it 'does not include max_issue_count' do
all_lists_in_response(do_not_include: 'max_issue_count')
end
end
def all_lists_in_response(params)
expect(json_response).not_to be_empty
expect(json_response).not_to be_empty if params.key?(:include)
expect(json_response.none? { |list_response| list_response.include?('max_issue_count') }).to be_truthy expect(json_response).to all(include(params[:include]))
elsif params.key?(:do_not_include)
expect(json_response.none? { |list_response| list_response.include?(params[:do_not_include]) }).to be_truthy
end end
end end
end end
......
...@@ -14,27 +14,87 @@ describe 'EE::Boards::Lists::UpdateService' do ...@@ -14,27 +14,87 @@ describe 'EE::Boards::Lists::UpdateService' do
end end
it 'updates the list if max_issue_count is given' do it 'updates the list if max_issue_count is given' do
service = Boards::Lists::UpdateService.new(board, user, max_issue_count: 42) update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_count: 42 })
expect(service.execute(list)).to be_truthy end
reloaded_list = list.reload it 'updates the list if max_issue_weight is given' do
expect(reloaded_list.max_issue_count).to eq(42) update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_weight: 42 })
end end
it 'updates the list with max_issue_count of 0 if max_issue_count is nil' do it 'updates the list if max_issue_count is nil' do
service = Boards::Lists::UpdateService.new(board, user, max_issue_count: nil) update_list_and_test_result(list, { max_issue_count: nil }, { max_issue_count: 0 })
expect(service.execute(list)).to be_truthy end
reloaded_list = list.reload it 'updates the list if max_issue_weight is nil' do
expect(reloaded_list.max_issue_count).to eq(0) update_list_and_test_result(list, { max_issue_weight: nil }, { max_issue_weight: 0 })
end end
it 'does not update the list if can_admin returns false' do it 'updates the max issue count of the list if both count and weight limits are provided' do
service = Boards::Lists::UpdateService.new(board, unpriviledged_user, max_issue_count: 42) update_list_and_test_result(list, { max_issue_count: 10, max_issue_weight: 42 }, { max_issue_count: 10, max_issue_weight: 42 })
expect(service.execute(list)).to be_truthy end
it 'does not change count if weight is updated' do
list.update!(max_issue_count: 55)
update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_count: 55, max_issue_weight: 42 })
end
it 'does not change weight if count is updated' do
list.update!(max_issue_weight: 55)
update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_weight: 55, max_issue_count: 42 })
end
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 })
end
it 'sets max_issue_count to 0 if requested' do
list.update!(max_issue_count: 3)
update_list_and_test_result(list, { max_issue_count: 0 }, { max_issue_count: 0 })
end
it 'sets max_issue_weight to 0 if requested' do
list.update!(max_issue_weight: 3)
update_list_and_test_result(list, { max_issue_weight: 0 }, { max_issue_weight: 0 })
end
it 'sets max_issue_count to 0 if requested' do
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 })
end
it 'sets max_issue_weight to 0 if requested' do
list.update!(max_issue_weight: 10)
reloaded_list = list.reload update_list_and_test_result(list, { max_issue_count: 0, max_issue_weight: 0 }, { max_issue_count: 0, max_issue_weight: 0 })
expect(reloaded_list.max_issue_count).to eq(0) end
it 'does not update count and weight when negative values for both are given' do
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 })
end
it 'sets count and weight to 0 when non numerical values are given' do
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 })
end
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,
{ max_issue_count: 42 },
{ max_issue_count: 0 })
end
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,
{ max_issue_weight: 42 },
{ max_issue_weight: 0 })
end end
end end
...@@ -44,20 +104,34 @@ describe 'EE::Boards::Lists::UpdateService' do ...@@ -44,20 +104,34 @@ 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
service = Boards::Lists::UpdateService.new(board, user, max_issue_count: 42) update_list_and_test_result(list, { max_issue_count: 42 }, { max_issue_count: 0 })
expect(service.execute(list)).to be_truthy
reloaded_list = list.reload
expect(reloaded_list.max_issue_count).to eq(0)
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
service = Boards::Lists::UpdateService.new(board, unpriviledged_user, max_issue_count: 42) update_list_and_test_result_by_user(unpriviledged_user, list, { max_issue_count: 42 }, { max_issue_count: 0 })
expect(service.execute(list)).to be_truthy end
reloaded_list = list.reload it 'does not update the list even if max_issue_weight is given' do
expect(reloaded_list.max_issue_count).to eq(0) update_list_and_test_result(list, { max_issue_weight: 42 }, { max_issue_weight: 0 })
end end
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 })
end
end
def update_list_and_test_result(list, initialization_params, expected_list_attributes)
update_list_and_test_result_by_user(user, list, initialization_params, expected_list_attributes)
end
def update_list_and_test_result_by_user(user, list, initialization_params, expected_list_attributes)
service = Boards::Lists::UpdateService.new(board, user, initialization_params)
expect(service.execute(list)).to be_truthy
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_weight).to eq(expected_list_attributes.fetch(:max_issue_weight, 0))
end end
end end
......
...@@ -46,29 +46,56 @@ describe Boards::Lists::CreateService do ...@@ -46,29 +46,56 @@ describe Boards::Lists::CreateService do
end end
end end
context 'wip limits' do context 'max limits' do
describe '#create_list_attributes' do describe '#create_list_attributes' do
subject(:service) { described_class.new(project, user, max_issue_count: 42) } shared_examples 'attribute provider for list creation' do
context 'license unavailable' do
before do before do
stub_licensed_features(wip_limits: false) stub_licensed_features(wip_limits: wip_limits_enabled)
end end
it 'contains a max_issue_count of 0' do where(:params, :expected_max_issue_count, :expected_max_issue_weight) do
expect(service.create_list_attributes(nil, nil, nil)).to include(max_issue_count: 0) [
end [{ max_issue_count: 0 }, 0, 0],
end [{ max_issue_count: nil }, 0, 0],
[{ max_issue_count: 1 }, 1, 0],
context 'license available' do [{ max_issue_weight: 0 }, 0, 0],
before do [{ max_issue_weight: nil }, 0, 0],
stub_licensed_features(wip_limits: true) [{ max_issue_weight: 1 }, 0, 1],
[{ max_issue_count: 1, max_issue_weight: 0 }, 1, 0],
[{ max_issue_count: 0, max_issue_weight: 1 }, 0, 1],
[{ max_issue_count: 1, max_issue_weight: 1 }, 1, 1],
[{ max_issue_count: nil, max_issue_weight: 1 }, 0, 1],
[{ max_issue_count: 1, max_issue_weight: nil }, 1, 0],
[{ max_issue_count: nil, max_issue_weight: nil }, 0, 0]
]
end end
it 'contains the params provided max issue count' do with_them do
expect(service.create_list_attributes(nil, nil, nil)).to include(max_issue_count: 42) it 'contains the expected max limits' do
service = described_class.new(project, user, params)
attrs = service.create_list_attributes(nil, nil, nil)
if wip_limits_enabled
expect(attrs).to include(max_issue_count: expected_max_issue_count, max_issue_weight: expected_max_issue_weight)
else
expect(attrs).to include(max_issue_count: 0, max_issue_weight: 0)
end
end
end end
end end
it_behaves_like 'attribute provider for list creation' do
let(:wip_limits_enabled) { true }
end
it_behaves_like 'attribute provider for list creation' do
let(:wip_limits_enabled) { false }
end
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe 'Max Limits Module' do
let(:clazz) do
Class.new do
include EE::Boards::Lists::MaxLimits
attr_reader :params
def initialize(params)
@params = params
end
end
end
describe 'max limits query methods' do
where(:params, :expected_max_issue_count?,
:expected_max_issue_weight?,
:expected_max_issue_count_by_params,
:expected_max_issue_weight_by_params,
:expected_list_attributes) do
[
[{ max_issue_count: 0 }, true, false, 0, 0, { max_issue_count: 0 }],
[{ max_issue_count: nil }, false, false, 0, 0, {}],
[{ max_issue_count: -1 }, true, false, -1, 0, { max_issue_count: -1 }],
[{ max_issue_count: 1 }, true, false, 1, 0, { max_issue_count: 1 }],
[{ max_issue_count: '1' }, true, false, 1, 0, { max_issue_count: 1 }],
[{ max_issue_weight: 0 }, false, true, 0, 0, { max_issue_weight: 0 }],
[{ max_issue_weight: nil }, false, false, 0, 0, {}],
[{ max_issue_weight: -1 }, false, true, 0, -1, { max_issue_weight: -1 }],
[{ max_issue_weight: 1 }, false, true, 0, 1, { max_issue_weight: 1 }],
[{ max_issue_weight: '1' }, false, true, 0, 1, { max_issue_weight: 1 }],
[{ max_issue_count: 1, max_issue_weight: 1 }, true, true, 1, 1, { max_issue_count: 1, max_issue_weight: 1 }],
[{ max_issue_count: '1', max_issue_weight: 1 }, true, true, 1, 1, { max_issue_count: 1, max_issue_weight: 1 }],
[{ max_issue_count: 1, max_issue_weight: '1' }, true, true, 1, 1, { max_issue_count: 1, max_issue_weight: 1 }],
[{ max_issue_count: nil, max_issue_weight: '1' }, false, true, 0, 1, { max_issue_weight: 1 }],
[{ max_issue_count: 1, max_issue_weight: 2 }, true, true, 1, 2, { max_issue_count: 1, max_issue_weight: 2 }],
[{ max_issue_count: 0, max_issue_weight: 3 }, true, true, 0, 3, { max_issue_count: 0, max_issue_weight: 3 }],
[{ max_issue_count: nil, max_issue_weight: 3 }, false, true, 0, 3, { max_issue_weight: 3 }],
[{ max_issue_count: -1, max_issue_weight: 3 }, true, true, -1, 3, { max_issue_count: -1, max_issue_weight: 3 }],
[{ max_issue_count: nil, max_issue_weight: nil }, false, false, 0, 0, {}],
[{ max_issue_count: -1, max_issue_weight: -1 }, true, true, -1, -1, { max_issue_count: -1, max_issue_weight: -1 }],
[{ max_issue_count: 1, max_issue_weight: 'hello' }, true, true, 1, 0, { max_issue_count: 1, max_issue_weight: 0 }],
[{ max_issue_count: '2', max_issue_weight: '9' }, true, true, 2, 9, { max_issue_count: 2, max_issue_weight: 9 }],
[{ max_issue_weight: '9' }, false, true, 0, 9, { max_issue_weight: 9 }],
[{ max_issue_count: 'hello1', max_issue_weight: 'hello2' }, true, true, 0, 0, { max_issue_count: 0, max_issue_weight: 0 }]
]
end
with_them do
it 'returns the expected values' do
instance = clazz.new(params)
expect(instance.max_issue_count?).to eq(expected_max_issue_count?)
expect(instance.max_issue_weight?).to eq(expected_max_issue_weight?)
expect(instance.max_issue_count_by_params).to eq(expected_max_issue_count_by_params)
expect(instance.max_issue_weight_by_params).to eq(expected_max_issue_weight_by_params)
expect(instance.list_max_limit_attributes_by_params).to eq(expected_list_attributes)
end
end
end
end
...@@ -6,6 +6,7 @@ FactoryBot.define do ...@@ -6,6 +6,7 @@ FactoryBot.define do
label label
list_type { :label } list_type { :label }
max_issue_count { 0 } max_issue_count { 0 }
max_issue_weight { 0 }
sequence(:position) sequence(:position)
end end
......
...@@ -36,7 +36,8 @@ ...@@ -36,7 +36,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" }
}, },
"additionalProperties": true "additionalProperties": true
} }
...@@ -77,7 +77,8 @@ ...@@ -77,7 +77,8 @@
} }
}, },
"position": { "type": ["integer", "null"] }, "position": { "type": ["integer", "null"] },
"max_issue_count": { "type": "integer" } "max_issue_count": { "type": "integer" },
"max_issue_weight": { "type": "integer" }
}, },
"additionalProperties": false "additionalProperties": false
} }
......
...@@ -728,6 +728,7 @@ List: ...@@ -728,6 +728,7 @@ List:
- milestone_id - milestone_id
- user_id - user_id
- max_issue_count - max_issue_count
- max_issue_weight
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