Skip to content
Snippets Groups Projects
Commit 00f5fcc4 authored by Vasilii Iakliushin's avatar Vasilii Iakliushin :two:
Browse files

Fix "empty" approval rules when user_ids or group_ids provided

Contrbutes to #415431

**Problem**

Grape v1.7.0 introduced a change to parameter renaming functionality.

Grape update issue: #395809

With out previous configuration it was possible to send `user_ids` and
`group_ids` as parameters. But after the gem upgrade this configuration
became invalid and prevents us from accessing `user_ids` and `group_ids`.

**Solution**

Explicitly add a parameter definition for `user_ids` and `group_ids`. It
will permit us to set `user_ids` and `group_ids` in the query.

Changelog: fixed
EE: true
parent e4f2632a
No related branches found
No related tags found
No related merge requests found
......@@ -9,8 +9,10 @@ module ProjectApprovalRulesHelpers
requires :name, type: String, desc: 'The name of the approval rule'
requires :approvals_required, type: Integer, desc: 'The number of required approvals for this rule'
optional :rule_type, type: String, desc: 'The type of approval rule', documentation: { example: 'regular' }
optional :users, as: :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The user ids for this rule'
optional :groups, as: :group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The group ids for this rule'
optional :users, as: :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Deprecated: The user ids for this rule'
optional :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The user ids for this rule'
optional :groups, as: :group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Deprecated: The group ids for this rule'
optional :group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The group ids for this rule'
optional :usernames, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The usernames for this rule'
optional :protected_branch_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The protected branch ids for this rule'
optional :applies_to_all_protected_branches, default: false, type: Boolean, desc: 'Apply this rule to all protected branches within the project'
......@@ -34,8 +36,10 @@ module ProjectApprovalRulesHelpers
requires :approval_rule_id, type: Integer, desc: 'The ID of an approval_rule'
optional :name, type: String, desc: 'The name of the approval rule'
optional :approvals_required, type: Integer, desc: 'The number of required approvals for this rule'
optional :users, as: :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The user ids for this rule'
optional :groups, as: :group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The group ids for this rule'
optional :users, as: :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Deprecated: The user ids for this rule'
optional :user_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The user ids for this rule'
optional :groups, as: :group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'Deprecated: The group ids for this rule'
optional :group_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The group ids for this rule'
optional :usernames, type: Array[String], coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, desc: 'The usernames for this rule'
optional :protected_branch_ids, type: Array[Integer], coerce_with: ::API::Validations::Types::CommaSeparatedToIntegerArray.coerce, desc: 'The protected branch ids for this rule'
optional :applies_to_all_protected_branches, default: false, type: Boolean, desc: 'Apply this rule to all protected branches within the project'
......
......@@ -4,6 +4,7 @@
RSpec.describe API::ProjectApprovalRules, :aggregate_failures, feature_category: :source_code_management do
let_it_be(:group) { create(:group_with_members) }
let_it_be(:group2) { create(:group_with_members) }
let_it_be(:user) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:admin) { create(:user, :admin) }
......
......@@ -4,6 +4,7 @@
RSpec.describe API::ProjectApprovalSettings, :aggregate_failures, feature_category: :source_code_management do
let_it_be(:group) { create(:group_with_members) }
let_it_be(:group2) { create(:group_with_members) }
let_it_be(:user) { create(:user) }
let_it_be(:user2) { create(:user) }
let_it_be(:admin) { create(:user, :admin) }
......
......@@ -182,6 +182,72 @@
end
end
context 'with users or groups params' do
before do
post api(url, current_user), params: params.merge!(extra_params)
end
context 'with a user' do
let(:extra_params) { { users: [user.id] } }
it 'returns a user' do
expect(response).to have_gitlab_http_status(:created)
expect(json_response['users'].size).to be 1
expect(json_response.dig('users', 0, 'id')).to eq(user.id)
end
end
context 'with users_ids' do
let(:extra_params) { { user_ids: [user.id] } }
it 'returns a user' do
expect(response).to have_gitlab_http_status(:created)
expect(json_response['users'].size).to be 1
expect(json_response.dig('users', 0, 'id')).to eq(user.id)
end
end
context 'when both users and user_ids are set' do
let(:extra_params) { { users: [user2.id], user_ids: [user.id] } }
it 'returns a user from user_ids' do
expect(response).to have_gitlab_http_status(:created)
expect(json_response['users'].size).to be 1
expect(json_response.dig('users', 0, 'id')).to eq(user.id)
end
end
context 'with a group' do
let(:extra_params) { { groups: [group.id] } }
it 'returns a group' do
expect(response).to have_gitlab_http_status(:created)
expect(json_response['groups'].size).to be 1
expect(json_response.dig('groups', 0, 'id')).to eq(group.id)
end
end
context 'with group_ids' do
let(:extra_params) { { group_ids: [group.id] } }
it 'returns a group' do
expect(response).to have_gitlab_http_status(:created)
expect(json_response['groups'].size).to be 1
expect(json_response.dig('groups', 0, 'id')).to eq(group.id)
end
end
context 'when both groups and group_ids are set' do
let(:extra_params) { { groups: [group2.id], group_ids: [group.id] } }
it 'returns a group from group_ids' do
expect(response).to have_gitlab_http_status(:created)
expect(json_response['groups'].size).to be 1
expect(json_response.dig('groups', 0, 'id')).to eq(group.id)
end
end
end
context 'with valid scanners' do
let(:scanners) { ['sast'] }
......@@ -278,6 +344,66 @@
end
end
context 'with users or groups params' do
before do
put api(url, current_user, admin_mode: current_user.admin?), params: params.to_json, headers: { CONTENT_TYPE: 'application/json' }
end
context 'with a user' do
let(:params) { { users: [user.id] } }
it 'sets a user' do
expect(response).to have_gitlab_http_status(:ok)
expect(approval_rule.users).to contain_exactly(user)
end
end
context 'with users_ids' do
let(:params) { { user_ids: [user.id] } }
it 'sets a user' do
expect(response).to have_gitlab_http_status(:ok)
expect(approval_rule.users).to contain_exactly(user)
end
end
context 'when both users and user_ids are set' do
let(:params) { { users: [user2.id], user_ids: [user.id] } }
it 'sets a user from user_ids' do
expect(response).to have_gitlab_http_status(:ok)
expect(approval_rule.users).to contain_exactly(user)
end
end
context 'with a group' do
let(:params) { { groups: [group.id] } }
it 'sets a group' do
expect(response).to have_gitlab_http_status(:ok)
expect(approval_rule.groups).to contain_exactly(group)
end
end
context 'with group_ids' do
let(:params) { { group_ids: [group.id] } }
it 'sets a group' do
expect(response).to have_gitlab_http_status(:ok)
expect(approval_rule.groups).to contain_exactly(group)
end
end
context 'when both groups and group_ids are set' do
let(:params) { { groups: [group2.id], group_ids: [group.id] } }
it 'sets a group from group_ids' do
expect(response).to have_gitlab_http_status(:ok)
expect(approval_rule.groups).to contain_exactly(group)
end
end
end
context 'with valid scanners' do
let(:scanners) { ['sast'] }
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment