Revert "Merge branch '18193-developers-can-merge' into 'master' "

This reverts commit 9ca633eb4c62231e4ddff5466c723cf8e2bdb25d, reversing
changes made to fb229bbf7970ba908962b837b270adf56f14098f.

1 files changed, 133 insertions, 114 deletions

diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index db33c7a22bb..c79ba11f782 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -6,6 +6,67 @@ describe Gitlab::GitAccess, lib: true do
   let(:user) { create(:user) }
   let(:actor) { user }
 
+  describe 'can_push_to_branch?' do
+    describe 'push to none protected branch' do
+      it "returns true if user is a master" do
+        project.team << [user, :master]
+        expect(access.can_push_to_branch?("random_branch")).to be_truthy
+      end
+
+      it "returns true if user is a developer" do
+        project.team << [user, :developer]
+        expect(access.can_push_to_branch?("random_branch")).to be_truthy
+      end
+
+      it "returns false if user is a reporter" do
+        project.team << [user, :reporter]
+        expect(access.can_push_to_branch?("random_branch")).to be_falsey
+      end
+    end
+
+    describe 'push to protected branch' do
+      before do
+        @branch = create :protected_branch, project: project
+      end
+      
+      it "returns true if user is a master" do
+        project.team << [user, :master]
+        expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+      end
+
+      it "returns false if user is a developer" do
+        project.team << [user, :developer]
+        expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+      end
+
+      it "returns false if user is a reporter" do
+        project.team << [user, :reporter]
+        expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+      end
+    end
+
+    describe 'push to protected branch if allowed for developers' do
+      before do
+        @branch = create :protected_branch, project: project, developers_can_push: true
+      end
+      
+      it "returns true if user is a master" do
+        project.team << [user, :master]
+        expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+      end
+
+      it "returns true if user is a developer" do
+        project.team << [user, :developer]
+        expect(access.can_push_to_branch?(@branch.name)).to be_truthy
+      end
+
+      it "returns false if user is a reporter" do
+        project.team << [user, :reporter]
+        expect(access.can_push_to_branch?(@branch.name)).to be_falsey
+      end
+    end
+  end
+
   describe '#check with single protocols allowed' do
     def disable_protocol(protocol)
       settings = ::ApplicationSetting.create_from_defaults
@@ -99,145 +160,103 @@ describe Gitlab::GitAccess, lib: true do
   end
 
   describe 'push_access_check' do
-    before { merge_into_protected_branch }
-    let(:unprotected_branch) { FFaker::Internet.user_name }
+    def protect_feature_branch
+      create(:protected_branch, name: 'feature', project: project)
+    end
 
-    let(:changes) do
-      { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
+    def changes
+      {
+        push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow",
         push_master: '6f6d7e7ed 570e7b2ab refs/heads/master',
         push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature',
         push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\
                                       'refs/heads/feature',
         push_tag: '6f6d7e7ed 570e7b2ab refs/tags/v1.0.0',
         push_new_tag: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/tags/v7.8.9",
-        push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'],
-        merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" }
+        push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature']
+      }
     end
 
-    def stub_git_hooks
-      # Running the `pre-receive` hook is expensive, and not necessary for this test.
-      allow_any_instance_of(GitHooksService).to receive(:execute).and_yield
+    def self.permissions_matrix
+      {
+        master: {
+          push_new_branch: true,
+          push_master: true,
+          push_protected_branch: true,
+          push_remove_protected_branch: false,
+          push_tag: true,
+          push_new_tag: true,
+          push_all: true,
+        },
+
+        developer: {
+          push_new_branch: true,
+          push_master: true,
+          push_protected_branch: false,
+          push_remove_protected_branch: false,
+          push_tag: false,
+          push_new_tag: true,
+          push_all: false,
+        },
+
+        reporter: {
+          push_new_branch: false,
+          push_master: false,
+          push_protected_branch: false,
+          push_remove_protected_branch: false,
+          push_tag: false,
+          push_new_tag: false,
+          push_all: false,
+        },
+
+        guest: {
+          push_new_branch: false,
+          push_master: false,
+          push_protected_branch: false,
+          push_remove_protected_branch: false,
+          push_tag: false,
+          push_new_tag: false,
+          push_all: false,
+        }
+      }
     end
 
-    def merge_into_protected_branch
-      @protected_branch_merge_commit ||= begin
-        stub_git_hooks
-        project.repository.add_branch(user, unprotected_branch, 'feature')
-        target_branch = project.repository.lookup('feature')
-        source_branch = project.repository.commit_file(user, FFaker::InternetSE.login_user_name, FFaker::HipsterIpsum.paragraph, FFaker::HipsterIpsum.sentence, unprotected_branch, false)
-        rugged = project.repository.rugged
-        author = { email: "email@example.com", time: Time.now, name: "Example Git User" }
-
-        merge_index = rugged.merge_commits(target_branch, source_branch)
-        Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged))
-      end
+    def self.updated_permissions_matrix
+      updated_permissions_matrix = permissions_matrix.dup
+      updated_permissions_matrix[:developer][:push_protected_branch] = true
+      updated_permissions_matrix[:developer][:push_all] = true
+      updated_permissions_matrix
     end
 
-    def self.run_permission_checks(permissions_matrix)
-      permissions_matrix.keys.each do |role|
-        describe "#{role} access" do
-          before { project.team << [user, role] }
+    permissions_matrix.keys.each do |role|
+      describe "#{role} access" do
+        before { protect_feature_branch }
+        before { project.team << [user, role] }
 
-          permissions_matrix[role].each do |action, allowed|
-            context action do
-              subject { access.push_access_check(changes[action]) }
+        permissions_matrix[role].each do |action, allowed|
+          context action do
+            subject { access.push_access_check(changes[action]) }
 
-              it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
-            end
+            it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
           end
         end
       end
     end
 
-    permissions_matrix = {
-      master: {
-        push_new_branch: true,
-        push_master: true,
-        push_protected_branch: true,
-        push_remove_protected_branch: false,
-        push_tag: true,
-        push_new_tag: true,
-        push_all: true,
-        merge_into_protected_branch: true
-      },
-
-      developer: {
-        push_new_branch: true,
-        push_master: true,
-        push_protected_branch: false,
-        push_remove_protected_branch: false,
-        push_tag: false,
-        push_new_tag: true,
-        push_all: false,
-        merge_into_protected_branch: false
-      },
-
-      reporter: {
-        push_new_branch: false,
-        push_master: false,
-        push_protected_branch: false,
-        push_remove_protected_branch: false,
-        push_tag: false,
-        push_new_tag: false,
-        push_all: false,
-        merge_into_protected_branch: false
-      },
-
-      guest: {
-        push_new_branch: false,
-        push_master: false,
-        push_protected_branch: false,
-        push_remove_protected_branch: false,
-        push_tag: false,
-        push_new_tag: false,
-        push_all: false,
-        merge_into_protected_branch: false
-      }
-    }
-
-    [['feature', 'exact'], ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type|
-      context do
-        before { create(:protected_branch, name: protected_branch_name, project: project) }
-
-        run_permission_checks(permissions_matrix)
-      end
-
-      context "when 'developers can push' is turned on for the #{protected_branch_type} protected branch" do
-        before { create(:protected_branch, name: protected_branch_name, developers_can_push: true, project: project) }
-
-        run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
-      end
-
-      context "when 'developers can merge' is turned on for the #{protected_branch_type} protected branch" do
-        before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, project: project) }
-
-        context "when a merge request exists for the given source/target branch" do
-          context "when the merge request is in progress" do
-            before do
-              create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch)
-            end
+    context "with enabled developers push to protected branches " do
+      updated_permissions_matrix.keys.each do |role|
+        describe "#{role} access" do
+          before { create(:protected_branch, name: 'feature', developers_can_push: true, project: project) }
+          before { project.team << [user, role] }
 
-            run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true }))
-          end
+          updated_permissions_matrix[role].each do |action, allowed|
+            context action do
+              subject { access.push_access_check(changes[action]) }
 
-          context "when the merge request is not in progress" do
-            before do
-              create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil)
+              it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
             end
-
-            run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
           end
         end
-
-        context "when a merge request does not exist for the given source/target branch" do
-          run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false }))
-        end
-      end
-
-      context "when 'developers can merge' and 'developers can push' are turned on for the #{protected_branch_type} protected branch" do
-        before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, developers_can_push: true, project: project) }
-
-        run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
       end
     end
   end