Commit a5f4b8bd authored by Mark Chao's avatar Mark Chao

Change feature flag to approval_rules

Apply project scope

It is now wished for the feature to be disabled by default.
Since we can't control when the background migration finishes,
which would set the old flag to true,
changing the flag is easier to work around with.
parent 9cedb3f8
...@@ -15,14 +15,14 @@ module ApprovableForRule ...@@ -15,14 +15,14 @@ module ApprovableForRule
FORWARDABLE_METHODS.each do |method| FORWARDABLE_METHODS.each do |method|
define_method(method) do |*args| define_method(method) do |*args|
return super(*args) if ::Feature.disabled?(:approval_rule) return super(*args) if ::Feature.disabled?(:approval_rules, project)
approval_state.public_send(method, *args) # rubocop:disable GitlabSecurity/PublicSend approval_state.public_send(method, *args) # rubocop:disable GitlabSecurity/PublicSend
end end
end end
def approvers_overwritten? def approvers_overwritten?
return super if ::Feature.disabled?(:approval_rule) return super if ::Feature.disabled?(:approval_rules, project)
approval_state.approval_rules_overwritten? approval_state.approval_rules_overwritten?
end end
......
...@@ -4,13 +4,13 @@ ...@@ -4,13 +4,13 @@
# that include the Approvable concern # that include the Approvable concern
module VisibleApprovableForRule module VisibleApprovableForRule
def approvers_left def approvers_left
return super if ::Feature.disabled?(:approval_rule) return super if ::Feature.disabled?(:approval_rules, project)
approval_state.unactioned_approvers approval_state.unactioned_approvers
end end
def overall_approvers(exclude_code_owners: false) def overall_approvers(exclude_code_owners: false)
return super if ::Feature.disabled?(:approval_rule) return super if ::Feature.disabled?(:approval_rules, project)
options = { target: :users } options = { target: :users }
options[:code_owner] = false if exclude_code_owners options[:code_owner] = false if exclude_code_owners
...@@ -21,13 +21,13 @@ module VisibleApprovableForRule ...@@ -21,13 +21,13 @@ module VisibleApprovableForRule
end end
def all_approvers_including_groups def all_approvers_including_groups
return super if ::Feature.disabled?(:approval_rule) return super if ::Feature.disabled?(:approval_rules, project)
approval_state.approvers approval_state.approvers
end end
def approvers_from_groups def approvers_from_groups
return super if ::Feature.disabled?(:approval_rule) return super if ::Feature.disabled?(:approval_rules, project)
groups = approval_state.wrapped_approval_rules.flat_map(&:groups) groups = approval_state.wrapped_approval_rules.flat_map(&:groups)
User.joins(:group_members).where(members: { source_id: groups }) User.joins(:group_members).where(members: { source_id: groups })
......
...@@ -57,7 +57,7 @@ module EE ...@@ -57,7 +57,7 @@ module EE
strong_memoize(:participant_approvers) do strong_memoize(:participant_approvers) do
next [] unless approval_needed? next [] unless approval_needed?
if ::Feature.enabled?(:approval_rule) if ::Feature.enabled?(:approval_rules, project)
approval_state.filtered_approvers(code_owner: false, unactioned: true) approval_state.filtered_approvers(code_owner: false, unactioned: true)
else else
approvers = [ approvers = [
......
...@@ -100,7 +100,7 @@ describe Projects::MergeRequestsController do ...@@ -100,7 +100,7 @@ describe Projects::MergeRequestsController do
let(:viewer) { user } let(:viewer) { user }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
sign_in(viewer) sign_in(viewer)
end end
......
...@@ -61,7 +61,7 @@ describe 'Merge request > User approves', :js do ...@@ -61,7 +61,7 @@ describe 'Merge request > User approves', :js do
context 'when CI is running but no approval given' do context 'when CI is running but no approval given' do
before do before do
stub_feature_flags(approval_rule: false) # TODO check in !9001 when feature enabled stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
create :approver_group, group: group, target: merge_request create :approver_group, group: group, target: merge_request
pipeline = create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) pipeline = create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch)
merge_request.update(head_pipeline: pipeline) merge_request.update(head_pipeline: pipeline)
......
...@@ -53,7 +53,7 @@ describe 'Merge request > User sets approvers', :js do ...@@ -53,7 +53,7 @@ describe 'Merge request > User sets approvers', :js do
let(:other_user) { create(:user) } let(:other_user) { create(:user) }
before do before do
stub_feature_flags(approval_rule: false) # TODO check in !9001 when feature enabled stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
project.add_developer(user) project.add_developer(user)
project.add_developer(other_user) project.add_developer(other_user)
...@@ -109,7 +109,7 @@ describe 'Merge request > User sets approvers', :js do ...@@ -109,7 +109,7 @@ describe 'Merge request > User sets approvers', :js do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
before do before do
stub_feature_flags(approval_rule: false) # TODO check in !9001 when feature enabled stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
project.add_developer(user) project.add_developer(user)
sign_in(user) sign_in(user)
......
...@@ -14,7 +14,7 @@ describe 'User approves a merge request', :js do ...@@ -14,7 +14,7 @@ describe 'User approves a merge request', :js do
context 'when user can approve' do context 'when user can approve' do
before do before do
stub_feature_flags(approval_rule: false) # TODO check in !9001 when feature enabled stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
visit(merge_request_path(merge_request)) visit(merge_request_path(merge_request))
end end
...@@ -31,7 +31,7 @@ describe 'User approves a merge request', :js do ...@@ -31,7 +31,7 @@ describe 'User approves a merge request', :js do
context 'when a merge request is approved additionally' do context 'when a merge request is approved additionally' do
before do before do
stub_feature_flags(approval_rule: false) # TODO check in !9001 when feature enabled stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
project.add_developer(user2) project.add_developer(user2)
project.add_developer(user3) project.add_developer(user3)
end end
...@@ -75,7 +75,7 @@ describe 'User approves a merge request', :js do ...@@ -75,7 +75,7 @@ describe 'User approves a merge request', :js do
context 'when user cannot approve' do context 'when user cannot approve' do
before do before do
stub_feature_flags(approval_rule: false) # TODO check in !9001 when feature enabled stub_feature_flags(approval_rules: false) # TODO check in !9001 when feature enabled
merge_request.approvers.create(user_id: user2.id) merge_request.approvers.create(user_id: user2.id)
visit(merge_request_path(merge_request)) visit(merge_request_path(merge_request))
......
...@@ -6,7 +6,7 @@ describe Approvable do ...@@ -6,7 +6,7 @@ describe Approvable do
let(:author) { merge_request.author } let(:author) { merge_request.author }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
describe '#approvers_overwritten?' do describe '#approvers_overwritten?' do
......
...@@ -52,7 +52,7 @@ describe MergeRequest do ...@@ -52,7 +52,7 @@ describe MergeRequest do
let(:stranger) { create(:user) } let(:stranger) { create(:user) }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
project.add_developer(author) project.add_developer(author)
project.add_developer(approver) project.add_developer(approver)
......
...@@ -6,7 +6,7 @@ describe VisibleApprovable do ...@@ -6,7 +6,7 @@ describe VisibleApprovable do
let!(:user) { project.creator } let!(:user) { project.creator }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
describe '#requires_approve' do describe '#requires_approve' do
......
...@@ -21,7 +21,7 @@ describe MergeRequestPresenter do ...@@ -21,7 +21,7 @@ describe MergeRequestPresenter do
let!(:approver) { create(:approver, target: resource) } let!(:approver) { create(:approver, target: resource) }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
resource.approvals.create!(user: approver.user) resource.approvals.create!(user: approver.user)
end end
...@@ -71,7 +71,7 @@ describe MergeRequestPresenter do ...@@ -71,7 +71,7 @@ describe MergeRequestPresenter do
subject { described_class.new(resource, current_user: user).all_approvers_including_groups } subject { described_class.new(resource, current_user: user).all_approvers_including_groups }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
it { is_expected.to match_array(public_approver_group.users + [approver.user]) } it { is_expected.to match_array(public_approver_group.users + [approver.user]) }
......
...@@ -10,7 +10,7 @@ describe API::MergeRequestApprovals do ...@@ -10,7 +10,7 @@ describe API::MergeRequestApprovals do
before do before do
project.update_attribute(:approvals_before_merge, 2) project.update_attribute(:approvals_before_merge, 2)
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
describe 'GET :id/merge_requests/:merge_request_iid/approvals' do describe 'GET :id/merge_requests/:merge_request_iid/approvals' do
......
...@@ -11,7 +11,7 @@ describe API::ProjectApprovals do ...@@ -11,7 +11,7 @@ describe API::ProjectApprovals do
let(:url) { "/projects/#{project.id}/approvals" } let(:url) { "/projects/#{project.id}/approvals" }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
describe 'GET /projects/:id/approvers' do describe 'GET /projects/:id/approvers' do
......
...@@ -79,7 +79,7 @@ describe MergeRequests::RefreshService do ...@@ -79,7 +79,7 @@ describe MergeRequests::RefreshService do
let(:relevant_merge_requests) { [merge_request, another_merge_request] } let(:relevant_merge_requests) { [merge_request, another_merge_request] }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
relevant_merge_requests.each do |merge_request| relevant_merge_requests.each do |merge_request|
expect(::Gitlab::CodeOwners).to receive(:for_merge_request).with(merge_request).and_return(new_owners) expect(::Gitlab::CodeOwners).to receive(:for_merge_request).with(merge_request).and_return(new_owners)
......
...@@ -525,7 +525,7 @@ describe EE::NotificationService, :mailer do ...@@ -525,7 +525,7 @@ describe EE::NotificationService, :mailer do
end end
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
project.add_maintainer(merge_request.author) project.add_maintainer(merge_request.author)
project.add_maintainer(merge_request.assignee) project.add_maintainer(merge_request.assignee)
......
...@@ -9,7 +9,7 @@ describe MergeRequests::RemoveApprovalService do ...@@ -9,7 +9,7 @@ describe MergeRequests::RemoveApprovalService do
subject(:service) { described_class.new(project, user) } subject(:service) { described_class.new(project, user) }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
def execute! def execute!
......
...@@ -82,7 +82,7 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -82,7 +82,7 @@ describe MergeRequests::UpdateService, :mailer do
context 'merge' do context 'merge' do
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
end end
let(:opts) { { merge: merge_request.diff_head_sha } } let(:opts) { { merge: merge_request.diff_head_sha } }
...@@ -123,7 +123,7 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -123,7 +123,7 @@ describe MergeRequests::UpdateService, :mailer do
let(:new_approver) { create(:user) } let(:new_approver) { create(:user) }
before do before do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
project.add_developer(existing_approver) project.add_developer(existing_approver)
project.add_developer(removed_approver) project.add_developer(removed_approver)
......
...@@ -231,7 +231,7 @@ describe Projects::UpdateService, '#execute' do ...@@ -231,7 +231,7 @@ describe Projects::UpdateService, '#execute' do
context 'with approval_rules' do context 'with approval_rules' do
it "updates approval_rules' approvals_required" do it "updates approval_rules' approvals_required" do
stub_feature_flags(approval_rule: false) stub_feature_flags(approval_rules: false)
rule = create(:approval_project_rule, project: project) rule = create(:approval_project_rule, project: project)
......
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