summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTimothy Andrew <mail@timothyandrew.net>2016-07-12 10:47:36 +0530
committerTimothy Andrew <mail@timothyandrew.net>2016-07-13 14:18:05 +0530
commitbb81f2afc1d82d6e88d7039025c8a8be0794aac6 (patch)
treee016e94d8e0d709ca4d614a0ecae81b4bfe48700
parentea9e8f4609f46f9c5713a8346f91dc19d310c2e1 (diff)
downloadgitlab-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.haml4
-rw-r--r--lib/gitlab/checks/change_access.rb6
-rw-r--r--spec/lib/gitlab/user_access_spec.rb40
-rw-r--r--spec/services/git_push_service_spec.rb3
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