Commit bb81f2af authored by Timothy Andrew's avatar Timothy Andrew

Implement last round of review comments from !4892.

1. Fix typos, minor styling errors.

2. Use single quotes rather than double quotes in `user_access_spec`.

3. Test formatting.
parent ea9e8f46
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
.table-responsive .table-responsive
%table.table.protected-branches-list %table.table.protected-branches-list
%colgroup %colgroup
%col{ width: "27%" } %col{ width: "20%" }
%col{ width: "30%" } %col{ width: "30%" }
%col{ width: "25%" } %col{ width: "25%" }
%col{ width: "25%" } %col{ width: "25%" }
...@@ -19,7 +19,7 @@ ...@@ -19,7 +19,7 @@
%th Protected Branch %th Protected Branch
%th Commit %th Commit
%th Developers Can Push %th Developers Can Push
%th Developers can Merge %th Developers Can Merge
- if can_admin_project - if can_admin_project
%th %th
%tbody %tbody
......
...@@ -28,7 +28,7 @@ module Gitlab ...@@ -28,7 +28,7 @@ module Gitlab
if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches) 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." 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) 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 end
if matching_merge_request? if matching_merge_request?
...@@ -47,7 +47,9 @@ module Gitlab ...@@ -47,7 +47,9 @@ module Gitlab
end end
def tag_checks 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." "You are not allowed to change existing tags on this project."
end end
end end
......
...@@ -7,40 +7,38 @@ describe Gitlab::UserAccess, lib: true do ...@@ -7,40 +7,38 @@ describe Gitlab::UserAccess, lib: true do
describe 'can_push_to_branch?' do describe 'can_push_to_branch?' do
describe 'push to none protected 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] 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 end
it "returns true if user is a developer" do it 'returns true if user is a developer' do
project.team << [user, :developer] 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 end
it "returns false if user is a reporter" do it 'returns false if user is a reporter' do
project.team << [user, :reporter] 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
end end
describe 'push to protected branch' do describe 'push to protected branch' do
before do let(:branch) { create :protected_branch, project: project }
@branch = create :protected_branch, project: project
end
it "returns true if user is a master" do it 'returns true if user is a master' do
project.team << [user, :master] 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 end
it "returns false if user is a developer" do it 'returns false if user is a developer' do
project.team << [user, :developer] 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 end
it "returns false if user is a reporter" do it 'returns false if user is a reporter' do
project.team << [user, :reporter] 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
end end
...@@ -49,17 +47,17 @@ describe Gitlab::UserAccess, lib: true do ...@@ -49,17 +47,17 @@ describe Gitlab::UserAccess, lib: true do
@branch = create :protected_branch, project: project, developers_can_push: true @branch = create :protected_branch, project: project, developers_can_push: true
end end
it "returns true if user is a master" do it 'returns true if user is a master' do
project.team << [user, :master] 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 end
it "returns true if user is a developer" do it 'returns true if user is a developer' do
project.team << [user, :developer] project.team << [user, :developer]
expect(access.can_push_to_branch?(@branch.name)).to be_truthy expect(access.can_push_to_branch?(@branch.name)).to be_truthy
end end
it "returns false if user is a reporter" do it 'returns false if user is a reporter' do
project.team << [user, :reporter] 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
...@@ -70,17 +68,17 @@ describe Gitlab::UserAccess, lib: true do ...@@ -70,17 +68,17 @@ describe Gitlab::UserAccess, lib: true do
@branch = create :protected_branch, project: project, developers_can_merge: true @branch = create :protected_branch, project: project, developers_can_merge: true
end end
it "returns true if user is a master" do it 'returns true if user is a master' do
project.team << [user, :master] project.team << [user, :master]
expect(access.can_merge_to_branch?(@branch.name)).to be_truthy expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
end end
it "returns true if user is a developer" do it 'returns true if user is a developer' do
project.team << [user, :developer] project.team << [user, :developer]
expect(access.can_merge_to_branch?(@branch.name)).to be_truthy expect(access.can_merge_to_branch?(@branch.name)).to be_truthy
end end
it "returns false if user is a reporter" do it 'returns false if user is a reporter' do
project.team << [user, :reporter] project.team << [user, :reporter]
expect(access.can_merge_to_branch?(@branch.name)).to be_falsey expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
end end
......
...@@ -243,7 +243,8 @@ describe GitPushService, services: true do ...@@ -243,7 +243,8 @@ describe GitPushService, services: true do
expect(project).to receive(:execute_hooks) expect(project).to receive(:execute_hooks)
expect(project.default_branch).to eq("master") 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 }) 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 end
it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do
......
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