diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-07-12 10:47:36 +0530 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-07-13 14:18:05 +0530 |
commit | bb81f2afc1d82d6e88d7039025c8a8be0794aac6 (patch) | |
tree | e016e94d8e0d709ca4d614a0ecae81b4bfe48700 | |
parent | ea9e8f4609f46f9c5713a8346f91dc19d310c2e1 (diff) | |
download | gitlab-ce-bb81f2afc1d82d6e88d7039025c8a8be0794aac6.tar.gz |
Implement last round of review comments from !4892.18193-developers-can-merge
1. Fix typos, minor styling errors.
2. Use single quotes rather than double quotes in `user_access_spec`.
3. Test formatting.
-rw-r--r-- | app/views/projects/protected_branches/_branches_list.html.haml | 4 | ||||
-rw-r--r-- | lib/gitlab/checks/change_access.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/user_access_spec.rb | 40 | ||||
-rw-r--r-- | spec/services/git_push_service_spec.rb | 3 |
4 files changed, 27 insertions, 26 deletions
diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml index 181d1a27c73..720d67dff7c 100644 --- a/app/views/projects/protected_branches/_branches_list.html.haml +++ b/app/views/projects/protected_branches/_branches_list.html.haml @@ -8,7 +8,7 @@ .table-responsive %table.table.protected-branches-list %colgroup - %col{ width: "27%" } + %col{ width: "20%" } %col{ width: "30%" } %col{ width: "25%" } %col{ width: "25%" } @@ -19,7 +19,7 @@ %th Protected Branch %th Commit %th Developers Can Push - %th Developers can Merge + %th Developers Can Merge - if can_admin_project %th %tbody diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index cb8f4f2cbdd..5551fac4b8b 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -28,7 +28,7 @@ module Gitlab if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches) return "You are not allowed to force push code to a protected branch on this project." elsif Gitlab::Git.blank_ref?(@newrev) && user_access.cannot_do_action?(:remove_protected_branches) - return "You are not allowed to deleted protected branches from this project." + return "You are not allowed to delete protected branches from this project." end if matching_merge_request? @@ -47,7 +47,9 @@ module Gitlab end def tag_checks - if (tag_ref = tag_name(@ref)) && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) + tag_ref = tag_name(@ref) + + if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) "You are not allowed to change existing tags on this project." end end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index c77b746cc88..aa9ec243498 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -7,40 +7,38 @@ describe Gitlab::UserAccess, lib: true do describe 'can_push_to_branch?' do describe 'push to none protected branch' do - it "returns true if user is a master" 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 + expect(access.can_push_to_branch?('random_branch')).to be_truthy end - it "returns true if user is a developer" do + it 'returns true if user is a developer' do project.team << [user, :developer] - expect(access.can_push_to_branch?("random_branch")).to be_truthy + expect(access.can_push_to_branch?('random_branch')).to be_truthy end - it "returns false if user is a reporter" do + it 'returns false if user is a reporter' do project.team << [user, :reporter] - expect(access.can_push_to_branch?("random_branch")).to be_falsey + 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 + let(:branch) { create :protected_branch, project: project } - it "returns true if user is a master" do + it 'returns true if user is a master' do project.team << [user, :master] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy + expect(access.can_push_to_branch?(branch.name)).to be_truthy end - it "returns false if user is a developer" do + it 'returns false if user is a developer' do project.team << [user, :developer] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey + expect(access.can_push_to_branch?(branch.name)).to be_falsey end - it "returns false if user is a reporter" do + it 'returns false if user is a reporter' do project.team << [user, :reporter] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey + expect(access.can_push_to_branch?(branch.name)).to be_falsey end end @@ -49,17 +47,17 @@ describe Gitlab::UserAccess, lib: true do @branch = create :protected_branch, project: project, developers_can_push: true end - it "returns true if user is a master" do + 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 + 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 + 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 @@ -70,17 +68,17 @@ describe Gitlab::UserAccess, lib: true do @branch = create :protected_branch, project: project, developers_can_merge: true end - it "returns true if user is a master" do + it 'returns true if user is a master' do project.team << [user, :master] expect(access.can_merge_to_branch?(@branch.name)).to be_truthy end - it "returns true if user is a developer" do + it 'returns true if user is a developer' do project.team << [user, :developer] expect(access.can_merge_to_branch?(@branch.name)).to be_truthy end - it "returns false if user is a reporter" do + it 'returns false if user is a reporter' do project.team << [user, :reporter] expect(access.can_merge_to_branch?(@branch.name)).to be_falsey end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 0f30a84a2cf..47c0580e0f0 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -243,7 +243,8 @@ describe GitPushService, services: true do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true, developers_can_merge: false }) - execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) + + execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master') end it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do |