Extract approval specs under ee/

The logics are copied over but unchanged.

ee/spec/models/merge_request_spec.rb

@@ -16,6 +16,250 @@ describe MergeRequest do
     it { is_expected.to have_many(:approved_by_users) }
   end
 
+  describe 'approvals' do
+    shared_examples_for 'authors self-approval authorization' do
+      context 'when authors are authorized to approve their own MRs' do
+        before do
+          project.update!(merge_requests_author_approval: true)
+        end
+
+        it 'allows the author to approve the MR if within the approvers list' do
+          expect(merge_request.can_approve?(author)).to be_truthy
+        end
+
+        it 'does not allow the author to approve the MR if not within the approvers list' do
+          merge_request.approvers.delete_all
+
+          expect(merge_request.can_approve?(author)).to be_falsey
+        end
+      end
+
+      context 'when authors are not authorized to approve their own MRs' do
+        it 'does not allow the author to approve the MR' do
+          expect(merge_request.can_approve?(author)).to be_falsey
+        end
+      end
+    end
+
+    let(:project) { create(:project, :repository) }
+    let(:merge_request) { create(:merge_request, source_project: project, author: author) }
+    let(:author) { create(:user) }
+    let(:approver) { create(:user) }
+    let(:approver_2) { create(:user) }
+    let(:developer) { create(:user) }
+    let(:other_developer) { create(:user) }
+    let(:reporter) { create(:user) }
+    let(:stranger) { create(:user) }
+
+    before do
+      stub_feature_flags(approval_rule: false)
+
+      project.add_developer(author)
+      project.add_developer(approver)
+      project.add_developer(approver_2)
+      project.add_developer(developer)
+      project.add_developer(other_developer)
+      project.add_reporter(reporter)
+    end
+
+    context 'when there is one approver required' do
+      before do
+        project.update(approvals_before_merge: 1)
+      end
+
+      context 'when that approver is the MR author' do
+        before do
+          create(:approver, user: author, target: merge_request)
+        end
+
+        it_behaves_like 'authors self-approval authorization'
+
+        it 'requires one approval' do
+          expect(merge_request.approvals_left).to eq(1)
+        end
+
+        it 'allows any other project member with write access to approve the MR' do
+          expect(merge_request.can_approve?(developer)).to be_truthy
+
+          expect(merge_request.can_approve?(reporter)).to be_falsey
+          expect(merge_request.can_approve?(stranger)).to be_falsey
+        end
+
+        it 'does not allow a logged-out user to approve the MR' do
+          expect(merge_request.can_approve?(nil)).to be_falsey
+        end
+      end
+
+      context 'when that approver is not the MR author' do
+        before do
+          create(:approver, user: approver, target: merge_request)
+        end
+
+        it 'requires one approval' do
+          expect(merge_request.approvals_left).to eq(1)
+        end
+
+        it 'only allows the approver to approve the MR' do
+          expect(merge_request.can_approve?(approver)).to be_truthy
+
+          expect(merge_request.can_approve?(author)).to be_falsey
+          expect(merge_request.can_approve?(developer)).to be_falsey
+          expect(merge_request.can_approve?(reporter)).to be_falsey
+          expect(merge_request.can_approve?(stranger)).to be_falsey
+          expect(merge_request.can_approve?(nil)).to be_falsey
+        end
+      end
+    end
+
+    context 'when there are multiple approvers required' do
+      before do
+        project.update(approvals_before_merge: 3)
+      end
+
+      context 'when one of those approvers is the MR author' do
+        before do
+          create(:approver, user: author, target: merge_request)
+          create(:approver, user: approver, target: merge_request)
+          create(:approver, user: approver_2, target: merge_request)
+        end
+
+        it_behaves_like 'authors self-approval authorization'
+
+        it 'requires the original number of approvals' do
+          expect(merge_request.approvals_left).to eq(3)
+        end
+
+        it 'allows any other other approver to approve the MR' do
+          expect(merge_request.can_approve?(approver)).to be_truthy
+        end
+
+        it 'does not allow a logged-out user to approve the MR' do
+          expect(merge_request.can_approve?(nil)).to be_falsey
+        end
+
+        context 'when self-approval is disabled and all of the valid approvers have approved the MR' do
+          before do
+            create(:approval, user: approver, merge_request: merge_request)
+            create(:approval, user: approver_2, merge_request: merge_request)
+          end
+
+          it 'requires the original number of approvals' do
+            expect(merge_request.approvals_left).to eq(1)
+          end
+
+          it 'does not allow the author to approve the MR' do
+            expect(merge_request.can_approve?(author)).to be_falsey
+          end
+
+          it 'does not allow the approvers to approve the MR again' do
+            expect(merge_request.can_approve?(approver)).to be_falsey
+            expect(merge_request.can_approve?(approver_2)).to be_falsey
+          end
+
+          it 'allows any other project member with write access to approve the MR' do
+            expect(merge_request.can_approve?(developer)).to be_truthy
+
+            expect(merge_request.can_approve?(reporter)).to be_falsey
+            expect(merge_request.can_approve?(stranger)).to be_falsey
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
+        end
+
+        context 'when self-approval is enabled and all of the valid approvers have approved the MR' do
+          before do
+            project.update!(merge_requests_author_approval: true)
+            create(:approval, user: author, merge_request: merge_request)
+            create(:approval, user: approver_2, merge_request: merge_request)
+          end
+
+          it 'requires the original number of approvals' do
+            expect(merge_request.approvals_left).to eq(1)
+          end
+
+          it 'does not allow the approvers to approve the MR again' do
+            expect(merge_request.can_approve?(author)).to be_falsey
+            expect(merge_request.can_approve?(approver_2)).to be_falsey
+          end
+
+          it 'allows any other project member with write access to approve the MR' do
+            expect(merge_request.can_approve?(reporter)).to be_falsey
+            expect(merge_request.can_approve?(stranger)).to be_falsey
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
+        end
+
+        context 'when more than the number of approvers have approved the MR' do
+          before do
+            create(:approval, user: approver, merge_request: merge_request)
+            create(:approval, user: approver_2, merge_request: merge_request)
+            create(:approval, user: developer, merge_request: merge_request)
+          end
+
+          it 'marks the MR as approved' do
+            expect(merge_request).to be_approved
+          end
+
+          it 'clamps the approvals left at zero' do
+            expect(merge_request.approvals_left).to eq(0)
+          end
+        end
+      end
+
+      context 'when the approvers do not contain the MR author' do
+        before do
+          create(:approver, user: developer, target: merge_request)
+          create(:approver, user: approver, target: merge_request)
+          create(:approver, user: approver_2, target: merge_request)
+        end
+
+        it 'requires the original number of approvals' do
+          expect(merge_request.approvals_left).to eq(3)
+        end
+
+        it 'only allows the approvers to approve the MR' do
+          expect(merge_request.can_approve?(developer)).to be_truthy
+          expect(merge_request.can_approve?(approver)).to be_truthy
+          expect(merge_request.can_approve?(approver_2)).to be_truthy
+
+          expect(merge_request.can_approve?(author)).to be_falsey
+          expect(merge_request.can_approve?(reporter)).to be_falsey
+          expect(merge_request.can_approve?(stranger)).to be_falsey
+          expect(merge_request.can_approve?(nil)).to be_falsey
+        end
+
+        context 'when only 1 approval approved' do
+          it 'only allows the approvers to approve the MR' do
+            create(:approval, user: approver, merge_request: merge_request)
+
+            expect(merge_request.can_approve?(developer)).to be_truthy
+            expect(merge_request.can_approve?(approver)).to be_falsey
+            expect(merge_request.can_approve?(approver_2)).to be_truthy
+
+            expect(merge_request.can_approve?(author)).to be_falsey
+            expect(merge_request.can_approve?(reporter)).to be_falsey
+            expect(merge_request.can_approve?(other_developer)).to be_falsey
+            expect(merge_request.can_approve?(stranger)).to be_falsey
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
+        end
+
+        context 'when all approvals received' do
+          it 'allows anyone with write access except for author to approve the MR' do
+            create(:approval, user: approver, merge_request: merge_request)
+            create(:approval, user: approver_2, merge_request: merge_request)
+            create(:approval, user: developer, merge_request: merge_request)
+
+            expect(merge_request.can_approve?(author)).to be_falsey
+            expect(merge_request.can_approve?(reporter)).to be_falsey
+            expect(merge_request.can_approve?(other_developer)).to be_truthy
+            expect(merge_request.can_approve?(stranger)).to be_falsey
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
+        end
+      end
+    end
+  end
+
   describe '#participant_approvers' do
     let(:approvers) { create_list(:user, 2) }
     let(:code_owners) { create_list(:user, 2) }
@@ -244,4 +488,201 @@ describe MergeRequest do
       end
     end
   end
+
+  describe '#mergeable_with_quick_action?' do
+    def create_pipeline(status)
+      pipeline = create(:ci_pipeline_with_one_job,
+        project: project,
+        ref:     merge_request.source_branch,
+        sha:     merge_request.diff_head_sha,
+        status:  status,
+        head_pipeline_of: merge_request)
+
+      pipeline
+    end
+
+    let(:project)       { create(:project, :public, :repository, only_allow_merge_if_pipeline_succeeds: true) }
+    let(:developer)     { create(:user) }
+    let(:user)          { create(:user) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
+    let(:mr_sha)        { merge_request.diff_head_sha }
+
+    before do
+      project.add_developer(developer)
+    end
+
+    context 'when autocomplete_precheck is set to false' do
+      context 'with approvals' do
+        before do
+          merge_request.target_project.update(approvals_before_merge: 1)
+        end
+
+        it 'is not mergeable when not approved' do
+          expect(merge_request.mergeable_with_quick_action?(developer, last_diff_sha: mr_sha)).to be_falsey
+        end
+
+        it 'is mergeable when approved' do
+          merge_request.approvals.create(user: user)
+
+          expect(merge_request.mergeable_with_quick_action?(developer, last_diff_sha: mr_sha)).to be_truthy
+        end
+      end
+    end
+  end
+
+  describe "#approvers_left" do
+    let(:merge_request) {create :merge_request}
+
+    it "returns correct value" do
+      user = create(:user)
+      user1 = create(:user)
+      merge_request.approvers.create(user_id: user.id)
+      merge_request.approvers.create(user_id: user1.id)
+      merge_request.approvals.create(user_id: user1.id)
+
+      expect(merge_request.approvers_left).to eq [user]
+    end
+
+    it "returns correct value when there is a group approver" do
+      user = create(:user)
+      user1 = create(:user)
+      user2 = create(:user)
+      group = create(:group)
+
+      group.add_developer(user2)
+      merge_request.approver_groups.create(group: group)
+      merge_request.approvers.create(user_id: user.id)
+      merge_request.approvers.create(user_id: user1.id)
+      merge_request.approvals.create(user_id: user1.id)
+
+      expect(merge_request.approvers_left).to match_array [user, user2]
+    end
+
+    it "returns correct value when there is only a group approver" do
+      user = create(:user)
+      group = create(:group)
+      group.add_developer(user)
+
+      merge_request.approver_groups.create(group: group)
+
+      expect(merge_request.approvers_left).to eq [user]
+    end
+  end
+
+  describe "#overall_approver_groups" do
+    it 'returns a merge request group approver' do
+      project = create :project
+      create :approver_group, target: project
+
+      merge_request = create :merge_request, target_project: project, source_project: project
+      approver_group2 = create :approver_group, target: merge_request
+
+      expect(merge_request.overall_approver_groups).to eq([approver_group2])
+    end
+
+    it 'returns a project group approver' do
+      project = create :project
+      approver_group1 = create :approver_group, target: project
+
+      merge_request = create :merge_request, target_project: project, source_project: project
+
+      expect(merge_request.overall_approver_groups).to eq([approver_group1])
+    end
+
+    it 'returns a merge request approver if there is no project group approver' do
+      project = create :project
+
+      merge_request = create :merge_request, target_project: project, source_project: project
+      approver_group1 = create :approver_group, target: merge_request
+
+      expect(merge_request.overall_approver_groups).to eq([approver_group1])
+    end
+  end
+
+  describe '#all_approvers_including_groups' do
+    it 'returns correct set of users' do
+      user = create :user
+      user1 = create :user
+      user2 = create :user
+      create :user
+
+      project = create :project
+      group = create :group
+      group.add_maintainer user
+      create :approver_group, target: project, group: group
+
+      merge_request = create :merge_request, target_project: project, source_project: project
+      group1 = create :group
+      group1.add_maintainer user1
+      create :approver_group, target: merge_request, group: group1
+
+      create(:approver, user: user2, target: merge_request)
+
+      expect(merge_request.all_approvers_including_groups).to match_array([user1, user2])
+    end
+  end
+
+  describe '#approver_group_ids=' do
+    it 'create approver_groups' do
+      group = create :group
+      group1 = create :group
+
+      merge_request = create :merge_request
+
+      merge_request.approver_group_ids = "#{group.id}, #{group1.id}"
+      merge_request.save!
+
+      expect(merge_request.approver_groups.map(&:group)).to match_array([group, group1])
+    end
+  end
+
+  describe "#approvals_required" do
+    let(:merge_request) { build(:merge_request) }
+
+    before do
+      merge_request.target_project.update(approvals_before_merge: 3)
+    end
+
+    context "when the MR has approvals_before_merge set" do
+      before do
+        merge_request.update(approvals_before_merge: 1)
+      end
+
+      it "uses the approvals_before_merge from the MR" do
+        expect(merge_request.approvals_required).to eq(1)
+      end
+    end
+
+    context "when the MR doesn't have approvals_before_merge set" do
+      it "takes approvals_before_merge from the target project" do
+        expect(merge_request.approvals_required).to eq(3)
+      end
+    end
+  end
+
+  describe '#mergeable?' do
+    let(:project) { create(:project) }
+
+    subject { create(:merge_request, source_project: project) }
+
+    context 'when using approvals' do
+      let(:user) { create(:user) }
+      before do
+        allow(subject).to receive(:mergeable_state?).and_return(true)
+
+        subject.target_project.update(approvals_before_merge: 1)
+        project.add_developer(user)
+      end
+
+      it 'return false if not approved' do
+        expect(subject.mergeable?).to be_falsey
+      end
+
+      it 'return true if approved' do
+        subject.approvals.create(user: user)
+
+        expect(subject.mergeable?).to be_truthy
+      end
+    end
+  end
 end

ee/spec/services/ee/notification_service_spec.rb

@@ -509,4 +509,126 @@ describe EE::NotificationService, :mailer do
     # Make the watcher a subscriber to detect dupes
     issuable.subscriptions.create(user: @watcher_and_subscriber, subscribed: true)
   end
+
+  describe 'Merge Requests' do
+    let(:notification) { NotificationService.new }
+    let(:assignee) { create(:user) }
+    let(:group) { create(:group) }
+    let(:project) { create(:project, :public, :repository, namespace: group) }
+    let(:another_project) { create(:project, :public, namespace: group) }
+    let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' }
+
+    around do |example|
+      perform_enqueued_jobs do
+        example.run
+      end
+    end
+
+    before do
+      stub_feature_flags(approval_rule: false)
+
+      project.add_maintainer(merge_request.author)
+      project.add_maintainer(merge_request.assignee)
+      build_team(merge_request.target_project)
+      add_users_with_subscription(merge_request.target_project, merge_request)
+      update_custom_notification(:new_merge_request, @u_guest_custom, resource: project)
+      update_custom_notification(:new_merge_request, @u_custom_global)
+      reset_delivered_emails!
+    end
+
+    describe '#new_merge_request' do
+      context 'when the target project has approvers set' do
+        let(:project_approvers) { create_list(:user, 3) }
+
+        before do
+          merge_request.target_project.update(approvals_before_merge: 1)
+          project_approvers.each { |approver| create(:approver, user: approver, target: merge_request.target_project) }
+        end
+
+        it 'emails the approvers' do
+          notification.new_merge_request(merge_request, @u_disabled)
+
+          project_approvers.each { |approver| should_email(approver) }
+        end
+
+        it 'does not email the approvers when approval is not necessary' do
+          merge_request.target_project.update(approvals_before_merge: 0)
+          notification.new_merge_request(merge_request, @u_disabled)
+
+          project_approvers.each { |approver| should_not_email(approver) }
+        end
+
+        context 'when the merge request has approvers set' do
+          let(:mr_approvers) { create_list(:user, 3) }
+
+          before do
+            mr_approvers.each { |approver| create(:approver, user: approver, target: merge_request) }
+          end
+
+          it 'emails the MR approvers' do
+            notification.new_merge_request(merge_request, @u_disabled)
+
+            mr_approvers.each { |approver| should_email(approver) }
+          end
+
+          it 'does not email approvers set on the project who are not approvers of this MR' do
+            notification.new_merge_request(merge_request, @u_disabled)
+
+            project_approvers.each { |approver| should_not_email(approver) }
+          end
+        end
+      end
+    end
+
+    def build_team(project)
+      @u_watcher               = create_global_setting_for(create(:user), :watch)
+      @u_participating         = create_global_setting_for(create(:user), :participating)
+      @u_participant_mentioned = create_global_setting_for(create(:user, username: 'participant'), :participating)
+      @u_disabled              = create_global_setting_for(create(:user), :disabled)
+      @u_mentioned             = create_global_setting_for(create(:user, username: 'mention'), :mention)
+      @u_committer             = create(:user, username: 'committer')
+      @u_not_mentioned         = create_global_setting_for(create(:user, username: 'regular'), :participating)
+      @u_outsider_mentioned    = create(:user, username: 'outsider')
+      @u_custom_global         = create_global_setting_for(create(:user, username: 'custom_global'), :custom)
+
+      # User to be participant by default
+      # This user does not contain any record in notification settings table
+      # It should be treated with a :participating notification_level
+      @u_lazy_participant      = create(:user, username: 'lazy-participant')
+
+      @u_guest_watcher = create_user_with_notification(:watch, 'guest_watching')
+      @u_guest_custom = create_user_with_notification(:custom, 'guest_custom')
+
+      project.add_maintainer(@u_watcher)
+      project.add_maintainer(@u_participating)
+      project.add_maintainer(@u_participant_mentioned)
+      project.add_maintainer(@u_disabled)
+      project.add_maintainer(@u_mentioned)
+      project.add_maintainer(@u_committer)
+      project.add_maintainer(@u_not_mentioned)
+      project.add_maintainer(@u_lazy_participant)
+      project.add_maintainer(@u_custom_global)
+    end
+
+    def add_users_with_subscription(project, issuable)
+      @subscriber = create :user
+      @unsubscriber = create :user
+      @unsubscribed_mentioned = create :user, username: 'unsubscribed_mentioned'
+      @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
+      @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
+
+      project.add_maintainer(@subscribed_participant)
+      project.add_maintainer(@subscriber)
+      project.add_maintainer(@unsubscriber)
+      project.add_maintainer(@watcher_and_subscriber)
+      project.add_maintainer(@unsubscribed_mentioned)
+
+      issuable.subscriptions.create(user: @unsubscribed_mentioned, project: project, subscribed: false)
+      issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
+      issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)
+      issuable.subscriptions.create(user: @unsubscriber, project: project, subscribed: false)
+      # Make the watcher a subscriber to detect dupes
+      issuable.subscriptions.create(user: @watcher_and_subscriber, project: project, subscribed: true)
+    end
+  end
 end

ee/spec/services/merge_requests/update_service_spec.rb

@@ -79,5 +79,106 @@ describe MergeRequests::UpdateService, :mailer do
         end
       end
     end
+
+    context 'merge' do
+      before do
+        stub_feature_flags(approval_rule: false)
+      end
+
+      let(:opts) { { merge: merge_request.diff_head_sha } }
+
+      context 'when not approved' do
+        before do
+          merge_request.update(approvals_before_merge: 1)
+
+          perform_enqueued_jobs do
+            update_merge_request(opts)
+            @merge_request = MergeRequest.find(merge_request.id)
+          end
+        end
+
+        it { expect(@merge_request).to be_valid }
+        it { expect(@merge_request.state).to eq('opened') }
+      end
+
+      context 'when approved' do
+        before do
+          merge_request.update(approvals_before_merge: 1)
+          merge_request.approvals.create(user: user)
+
+          perform_enqueued_jobs do
+            update_merge_request(opts)
+            @merge_request = MergeRequest.find(merge_request.id)
+          end
+        end
+
+        it { expect(@merge_request).to be_valid }
+        it { expect(@merge_request.state).to eq('merged') }
+      end
+    end
+
+    context 'when the approvers change' do
+      let(:existing_approver) { create(:user) }
+      let(:removed_approver) { create(:user) }
+      let(:new_approver) { create(:user) }
+
+      before do
+        stub_feature_flags(approval_rule: false)
+        perform_enqueued_jobs do
+          update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
+        end
+
+        Todo.where(action: Todo::APPROVAL_REQUIRED).destroy_all # rubocop: disable DestroyAll
+        ActionMailer::Base.deliveries.clear
+      end
+
+      context 'when an approver is added and an approver is removed' do
+        before do
+          perform_enqueued_jobs do
+            update_merge_request(approver_ids: [new_approver, existing_approver].map(&:id).join(','))
+          end
+        end
+
+        it 'adds todos for and sends emails to the new approvers' do
+          expect(Todo.where(user: new_approver, action: Todo::APPROVAL_REQUIRED)).not_to be_empty
+          should_email(new_approver)
+        end
+
+        it 'does not add todos for or send emails to the existing approvers' do
+          expect(Todo.where(user: existing_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
+          should_not_email(existing_approver)
+        end
+
+        it 'does not add todos for or send emails to the removed approvers' do
+          expect(Todo.where(user: removed_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
+          should_not_email(removed_approver)
+        end
+      end
+
+      context 'when the approvers are set to the same values' do
+        it 'does not create any todos' do
+          expect do
+            update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
+          end.not_to change { Todo.count }
+        end
+
+        it 'does not send any emails' do
+          expect do
+            update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
+          end.not_to change { ActionMailer::Base.deliveries.count }
+        end
+      end
+    end
+
+    context 'updating target_branch' do
+      it 'resets approvals when target_branch is changed' do
+        merge_request.target_project.update(reset_approvals_on_push: true)
+        merge_request.approvals.create(user_id: user2.id)
+
+        update_merge_request(target_branch: 'video')
+
+        expect(merge_request.reload.approvals).to be_empty
+      end
+    end
   end
 end

spec/models/merge_request_spec.rb

@@ -879,136 +879,6 @@ describe MergeRequest do
     end
   end
 
-  describe "#approvers_left" do
-    let(:merge_request) {create :merge_request}
-
-    it "returns correct value" do
-      user = create(:user)
-      user1 = create(:user)
-      merge_request.approvers.create(user_id: user.id)
-      merge_request.approvers.create(user_id: user1.id)
-      merge_request.approvals.create(user_id: user1.id)
-
-      expect(merge_request.approvers_left).to eq [user]
-    end
-
-    it "returns correct value when there is a group approver" do
-      user = create(:user)
-      user1 = create(:user)
-      user2 = create(:user)
-      group = create(:group)
-
-      group.add_developer(user2)
-      merge_request.approver_groups.create(group: group)
-      merge_request.approvers.create(user_id: user.id)
-      merge_request.approvers.create(user_id: user1.id)
-      merge_request.approvals.create(user_id: user1.id)
-
-      expect(merge_request.approvers_left).to match_array [user, user2]
-    end
-
-    it "returns correct value when there is only a group approver" do
-      user = create(:user)
-      group = create(:group)
-      group.add_developer(user)
-
-      merge_request.approver_groups.create(group: group)
-
-      expect(merge_request.approvers_left).to eq [user]
-    end
-  end
-
-  describe "#overall_approver_groups" do
-    it 'returns a merge request group approver' do
-      project = create :project
-      create :approver_group, target: project
-
-      merge_request = create :merge_request, target_project: project, source_project: project
-      approver_group2 = create :approver_group, target: merge_request
-
-      expect(merge_request.overall_approver_groups).to eq([approver_group2])
-    end
-
-    it 'returns a project group approver' do
-      project = create :project
-      approver_group1 = create :approver_group, target: project
-
-      merge_request = create :merge_request, target_project: project, source_project: project
-
-      expect(merge_request.overall_approver_groups).to eq([approver_group1])
-    end
-
-    it 'returns a merge request approver if there is no project group approver' do
-      project = create :project
-
-      merge_request = create :merge_request, target_project: project, source_project: project
-      approver_group1 = create :approver_group, target: merge_request
-
-      expect(merge_request.overall_approver_groups).to eq([approver_group1])
-    end
-  end
-
-  describe '#all_approvers_including_groups' do
-    it 'returns correct set of users' do
-      user = create :user
-      user1 = create :user
-      user2 = create :user
-      create :user
-
-      project = create :project
-      group = create :group
-      group.add_maintainer user
-      create :approver_group, target: project, group: group
-
-      merge_request = create :merge_request, target_project: project, source_project: project
-      group1 = create :group
-      group1.add_maintainer user1
-      create :approver_group, target: merge_request, group: group1
-
-      create(:approver, user: user2, target: merge_request)
-
-      expect(merge_request.all_approvers_including_groups).to match_array([user1, user2])
-    end
-  end
-
-  describe '#approver_group_ids=' do
-    it 'create approver_groups' do
-      group = create :group
-      group1 = create :group
-
-      merge_request = create :merge_request
-
-      merge_request.approver_group_ids = "#{group.id}, #{group1.id}"
-      merge_request.save!
-
-      expect(merge_request.approver_groups.map(&:group)).to match_array([group, group1])
-    end
-  end
-
-  describe "#approvals_required" do
-    let(:merge_request) { build(:merge_request) }
-
-    before do
-      merge_request.target_project.update(approvals_before_merge: 3)
-    end
-
-    context "when the MR has approvals_before_merge set" do
-      before do
-        merge_request.update(approvals_before_merge: 1)
-      end
-
-      it "uses the approvals_before_merge from the MR" do
-        expect(merge_request.approvals_required).to eq(1)
-      end
-    end
-
-    context "when the MR doesn't have approvals_before_merge set" do
-      it "takes approvals_before_merge from the target project" do
-        expect(merge_request.approvals_required).to eq(3)
-      end
-    end
-  end
-
   describe '#can_remove_source_branch?' do
     set(:user) { create(:user) }
     set(:merge_request) { create(:merge_request, :simple) }
@@ -2054,26 +1924,6 @@ describe MergeRequest do
 
       expect(subject.mergeable?).to be_truthy
     end
-
-    context 'when using approvals' do
-      let(:user) { create(:user) }
-      before do
-        allow(subject).to receive(:mergeable_state?).and_return(true)
-
-        subject.target_project.update(approvals_before_merge: 1)
-        project.add_developer(user)
-      end
-
-      it 'return false if not approved' do
-        expect(subject.mergeable?).to be_falsey
-      end
-
-      it 'return true if approved' do
-        subject.approvals.create(user: user)
-
-        expect(subject.mergeable?).to be_truthy
-      end
-    end
   end
 
   describe '#mergeable_state?' do
@@ -2406,250 +2256,6 @@ describe MergeRequest do
     end
   end
 
-  describe 'approvals' do
-    shared_examples_for 'authors self-approval authorization' do
-      context 'when authors are authorized to approve their own MRs' do
-        before do
-          project.update!(merge_requests_author_approval: true)
-        end
-
-        it 'allows the author to approve the MR if within the approvers list' do
-          expect(merge_request.can_approve?(author)).to be_truthy
-        end
-
-        it 'does not allow the author to approve the MR if not within the approvers list' do
-          merge_request.approvers.delete_all
-
-          expect(merge_request.can_approve?(author)).to be_falsey
-        end
-      end
-
-      context 'when authors are not authorized to approve their own MRs' do
-        it 'does not allow the author to approve the MR' do
-          expect(merge_request.can_approve?(author)).to be_falsey
-        end
-      end
-    end
-
-    let(:project) { create(:project, :repository) }
-    let(:merge_request) { create(:merge_request, source_project: project, author: author) }
-    let(:author) { create(:user) }
-    let(:approver) { create(:user) }
-    let(:approver_2) { create(:user) }
-    let(:developer) { create(:user) }
-    let(:other_developer) { create(:user) }
-    let(:reporter) { create(:user) }
-    let(:stranger) { create(:user) }
-
-    before do
-      stub_feature_flags(approval_rule: false)
-
-      project.add_developer(author)
-      project.add_developer(approver)
-      project.add_developer(approver_2)
-      project.add_developer(developer)
-      project.add_developer(other_developer)
-      project.add_reporter(reporter)
-    end
-
-    context 'when there is one approver required' do
-      before do
-        project.update(approvals_before_merge: 1)
-      end
-
-      context 'when that approver is the MR author' do
-        before do
-          create(:approver, user: author, target: merge_request)
-        end
-
-        it_behaves_like 'authors self-approval authorization'
-
-        it 'requires one approval' do
-          expect(merge_request.approvals_left).to eq(1)
-        end
-
-        it 'allows any other project member with write access to approve the MR' do
-          expect(merge_request.can_approve?(developer)).to be_truthy
-
-          expect(merge_request.can_approve?(reporter)).to be_falsey
-          expect(merge_request.can_approve?(stranger)).to be_falsey
-        end
-
-        it 'does not allow a logged-out user to approve the MR' do
-          expect(merge_request.can_approve?(nil)).to be_falsey
-        end
-      end
-
-      context 'when that approver is not the MR author' do
-        before do
-          create(:approver, user: approver, target: merge_request)
-        end
-
-        it 'requires one approval' do
-          expect(merge_request.approvals_left).to eq(1)
-        end
-
-        it 'only allows the approver to approve the MR' do
-          expect(merge_request.can_approve?(approver)).to be_truthy
-
-          expect(merge_request.can_approve?(author)).to be_falsey
-          expect(merge_request.can_approve?(developer)).to be_falsey
-          expect(merge_request.can_approve?(reporter)).to be_falsey
-          expect(merge_request.can_approve?(stranger)).to be_falsey
-          expect(merge_request.can_approve?(nil)).to be_falsey
-        end
-      end
-    end
-
-    context 'when there are multiple approvers required' do
-      before do
-        project.update(approvals_before_merge: 3)
-      end
-
-      context 'when one of those approvers is the MR author' do
-        before do
-          create(:approver, user: author, target: merge_request)
-          create(:approver, user: approver, target: merge_request)
-          create(:approver, user: approver_2, target: merge_request)
-        end
-
-        it_behaves_like 'authors self-approval authorization'
-
-        it 'requires the original number of approvals' do
-          expect(merge_request.approvals_left).to eq(3)
-        end
-
-        it 'allows any other other approver to approve the MR' do
-          expect(merge_request.can_approve?(approver)).to be_truthy
-        end
-
-        it 'does not allow a logged-out user to approve the MR' do
-          expect(merge_request.can_approve?(nil)).to be_falsey
-        end
-
-        context 'when self-approval is disabled and all of the valid approvers have approved the MR' do
-          before do
-            create(:approval, user: approver, merge_request: merge_request)
-            create(:approval, user: approver_2, merge_request: merge_request)
-          end
-
-          it 'requires the original number of approvals' do
-            expect(merge_request.approvals_left).to eq(1)
-          end
-
-          it 'does not allow the author to approve the MR' do
-            expect(merge_request.can_approve?(author)).to be_falsey
-          end
-
-          it 'does not allow the approvers to approve the MR again' do
-            expect(merge_request.can_approve?(approver)).to be_falsey
-            expect(merge_request.can_approve?(approver_2)).to be_falsey
-          end
-
-          it 'allows any other project member with write access to approve the MR' do
-            expect(merge_request.can_approve?(developer)).to be_truthy
-
-            expect(merge_request.can_approve?(reporter)).to be_falsey
-            expect(merge_request.can_approve?(stranger)).to be_falsey
-            expect(merge_request.can_approve?(nil)).to be_falsey
-          end
-        end
-
-        context 'when self-approval is enabled and all of the valid approvers have approved the MR' do
-          before do
-            project.update!(merge_requests_author_approval: true)
-            create(:approval, user: author, merge_request: merge_request)
-            create(:approval, user: approver_2, merge_request: merge_request)
-          end
-
-          it 'requires the original number of approvals' do
-            expect(merge_request.approvals_left).to eq(1)
-          end
-
-          it 'does not allow the approvers to approve the MR again' do
-            expect(merge_request.can_approve?(author)).to be_falsey
-            expect(merge_request.can_approve?(approver_2)).to be_falsey
-          end
-
-          it 'allows any other project member with write access to approve the MR' do
-            expect(merge_request.can_approve?(reporter)).to be_falsey
-            expect(merge_request.can_approve?(stranger)).to be_falsey
-            expect(merge_request.can_approve?(nil)).to be_falsey
-          end
-        end
-
-        context 'when more than the number of approvers have approved the MR' do
-          before do
-            create(:approval, user: approver, merge_request: merge_request)
-            create(:approval, user: approver_2, merge_request: merge_request)
-            create(:approval, user: developer, merge_request: merge_request)
-          end
-
-          it 'marks the MR as approved' do
-            expect(merge_request).to be_approved
-          end
-
-          it 'clamps the approvals left at zero' do
-            expect(merge_request.approvals_left).to eq(0)
-          end
-        end
-      end
-
-      context 'when the approvers do not contain the MR author' do
-        before do
-          create(:approver, user: developer, target: merge_request)
-          create(:approver, user: approver, target: merge_request)
-          create(:approver, user: approver_2, target: merge_request)
-        end
-
-        it 'requires the original number of approvals' do
-          expect(merge_request.approvals_left).to eq(3)
-        end
-
-        it 'only allows the approvers to approve the MR' do
-          expect(merge_request.can_approve?(developer)).to be_truthy
-          expect(merge_request.can_approve?(approver)).to be_truthy
-          expect(merge_request.can_approve?(approver_2)).to be_truthy
-
-          expect(merge_request.can_approve?(author)).to be_falsey
-          expect(merge_request.can_approve?(reporter)).to be_falsey
-          expect(merge_request.can_approve?(stranger)).to be_falsey
-          expect(merge_request.can_approve?(nil)).to be_falsey
-        end
-
-        context 'when only 1 approval approved' do
-          it 'only allows the approvers to approve the MR' do
-            create(:approval, user: approver, merge_request: merge_request)
-
-            expect(merge_request.can_approve?(developer)).to be_truthy
-            expect(merge_request.can_approve?(approver)).to be_falsey
-            expect(merge_request.can_approve?(approver_2)).to be_truthy
-
-            expect(merge_request.can_approve?(author)).to be_falsey
-            expect(merge_request.can_approve?(reporter)).to be_falsey
-            expect(merge_request.can_approve?(other_developer)).to be_falsey
-            expect(merge_request.can_approve?(stranger)).to be_falsey
-            expect(merge_request.can_approve?(nil)).to be_falsey
-          end
-        end
-
-        context 'when all approvals received' do
-          it 'allows anyone with write access except for author to approve the MR' do
-            create(:approval, user: approver, merge_request: merge_request)
-            create(:approval, user: approver_2, merge_request: merge_request)
-            create(:approval, user: developer, merge_request: merge_request)
-
-            expect(merge_request.can_approve?(author)).to be_falsey
-            expect(merge_request.can_approve?(reporter)).to be_falsey
-            expect(merge_request.can_approve?(other_developer)).to be_truthy
-            expect(merge_request.can_approve?(stranger)).to be_falsey
-            expect(merge_request.can_approve?(nil)).to be_falsey
-          end
-        end
-      end
-    end
-  end
-
   describe '#branch_merge_base_commit' do
     context 'source and target branch exist' do
       it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
@@ -2950,22 +2556,6 @@ describe MergeRequest do
           expect(merge_request.mergeable_with_quick_action?(developer, last_diff_sha: mr_sha)).to be_truthy
         end
       end
-
-      context 'with approvals' do
-        before do
-          merge_request.target_project.update(approvals_before_merge: 1)
-        end
-
-        it 'is not mergeable when not approved' do
-          expect(merge_request.mergeable_with_quick_action?(developer, last_diff_sha: mr_sha)).to be_falsey
-        end
-
-        it 'is mergeable when approved' do
-          merge_request.approvals.create(user: user)
-
-          expect(merge_request.mergeable_with_quick_action?(developer, last_diff_sha: mr_sha)).to be_truthy
-        end
-      end
     end
   end
 

spec/services/merge_requests/update_service_spec.rb

@@ -19,8 +19,6 @@ describe MergeRequests::UpdateService, :mailer do
   end
 
   before do
-    stub_feature_flags(approval_rule: false)
-
     project.add_maintainer(user)
     project.add_developer(user2)
     project.add_developer(user3)
@@ -260,35 +258,6 @@ describe MergeRequests::UpdateService, :mailer do
 
         it { expect(@merge_request.state).to eq('opened') }
       end
-
-      context 'when not approved' do
-        before do
-          merge_request.update(approvals_before_merge: 1)
-
-          perform_enqueued_jobs do
-            service.execute(merge_request)
-            @merge_request = MergeRequest.find(merge_request.id)
-          end
-        end
-
-        it { expect(@merge_request).to be_valid }
-        it { expect(@merge_request.state).to eq('opened') }
-      end
-
-      context 'when approved' do
-        before do
-          merge_request.update(approvals_before_merge: 1)
-          merge_request.approvals.create(user: user)
-
-          perform_enqueued_jobs do
-            service.execute(merge_request)
-            @merge_request = MergeRequest.find(merge_request.id)
-          end
-        end
-
-        it { expect(@merge_request).to be_valid }
-        it { expect(@merge_request.state).to eq('merged') }
-      end
     end
 
     context 'todos' do
@@ -485,58 +454,6 @@ describe MergeRequests::UpdateService, :mailer do
       end
     end
 
-    context 'when the approvers change' do
-      let(:existing_approver) { create(:user) }
-      let(:removed_approver) { create(:user) }
-      let(:new_approver) { create(:user) }
-
-      before do
-        perform_enqueued_jobs do
-          update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
-        end
-
-        Todo.where(action: Todo::APPROVAL_REQUIRED).destroy_all # rubocop: disable DestroyAll
-        ActionMailer::Base.deliveries.clear
-      end
-
-      context 'when an approver is added and an approver is removed' do
-        before do
-          perform_enqueued_jobs do
-            update_merge_request(approver_ids: [new_approver, existing_approver].map(&:id).join(','))
-          end
-        end
-
-        it 'adds todos for and sends emails to the new approvers' do
-          expect(Todo.where(user: new_approver, action: Todo::APPROVAL_REQUIRED)).not_to be_empty
-          should_email(new_approver)
-        end
-
-        it 'does not add todos for or send emails to the existing approvers' do
-          expect(Todo.where(user: existing_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
-          should_not_email(existing_approver)
-        end
-
-        it 'does not add todos for or send emails to the removed approvers' do
-          expect(Todo.where(user: removed_approver, action: Todo::APPROVAL_REQUIRED)).to be_empty
-          should_not_email(removed_approver)
-        end
-      end
-
-      context 'when the approvers are set to the same values' do
-        it 'does not create any todos' do
-          expect do
-            update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
-          end.not_to change { Todo.count }
-        end
-
-        it 'does not send any emails' do
-          expect do
-            update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
-          end.not_to change { ActionMailer::Base.deliveries.count }
-        end
-      end
-    end
-
     context 'updating mentions' do
       let(:mentionable) { merge_request }
       include_examples 'updating mentions', described_class
@@ -622,17 +539,6 @@ describe MergeRequests::UpdateService, :mailer do
       end
     end
 
-    context 'updating target_branch' do
-      it 'resets approvals when target_branch is changed' do
-        merge_request.target_project.update(reset_approvals_on_push: true)
-        merge_request.approvals.create(user_id: user2.id)
-
-        update_merge_request(target_branch: 'video')
-
-        expect(merge_request.reload.approvals).to be_empty
-      end
-    end
-
     context 'updating asssignee_id' do
       it 'does not update assignee when assignee_id is invalid' do
         merge_request.update(assignee_id: user.id)

spec/services/notification_service_spec.rb

@@ -1304,49 +1304,6 @@ describe NotificationService, :mailer do
         should_email(user_4)
       end
 
-      context 'when the target project has approvers set' do
-        let(:project_approvers) { create_list(:user, 3) }
-
-        before do
-          stub_feature_flags(approval_rule: false)
-          merge_request.target_project.update(approvals_before_merge: 1)
-          project_approvers.each { |approver| create(:approver, user: approver, target: merge_request.target_project) }
-        end
-
-        it 'emails the approvers' do
-          notification.new_merge_request(merge_request, @u_disabled)
-
-          project_approvers.each { |approver| should_email(approver) }
-        end
-
-        it 'does not email the approvers when approval is not necessary' do
-          merge_request.target_project.update(approvals_before_merge: 0)
-          notification.new_merge_request(merge_request, @u_disabled)
-
-          project_approvers.each { |approver| should_not_email(approver) }
-        end
-
-        context 'when the merge request has approvers set' do
-          let(:mr_approvers) { create_list(:user, 3) }
-
-          before do
-            mr_approvers.each { |approver| create(:approver, user: approver, target: merge_request) }
-          end
-
-          it 'emails the MR approvers' do
-            notification.new_merge_request(merge_request, @u_disabled)
-
-            mr_approvers.each { |approver| should_email(approver) }
-          end
-
-          it 'does not email approvers set on the project who are not approvers of this MR' do
-            notification.new_merge_request(merge_request, @u_disabled)
-
-            project_approvers.each { |approver| should_not_email(approver) }
-          end
-        end
-      end
-
       context 'participating' do
         it_should_behave_like 'participating by assignee notification' do
           let(:participant) { create(:user, username: 'user-participant')}