Commit 06288cb2 authored by Timothy Andrew's avatar Timothy Andrew Committed by Alfredo Sumaran

Implement final round of review comments from @dbalexandre.

- Primarly whitespace and code style changes.

- Improve feature spec dealing with removing a user from a protected
  branch. Previously, we were only asserting on a "Deleted" flash
  notice. Now the assertion looks at the database to make sure that the
  right access level was removed.
parent 3e88873f
......@@ -14,6 +14,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
def create
@protected_branch = ::ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute
if @protected_branch.persisted?
redirect_to namespace_project_protected_branches_path(@project.namespace, @project)
else
......
......@@ -14,19 +14,13 @@ class ProtectedBranch < ActiveRecord::Base
accepts_nested_attributes_for :push_access_levels
accepts_nested_attributes_for :merge_access_levels
# Scopes
# Returns all merge access levels (for protected branches in scope) that grant merge
# access to the given user.
def self.merge_access_by_user(user)
MergeAccessLevel.joins(:protected_branch).where(protected_branch_id: self.ids).merge(MergeAccessLevel.by_user(user))
end
scope :merge_access_by_user, -> (user) { MergeAccessLevel.joins(:protected_branch).where(protected_branch_id: self.ids).merge(MergeAccessLevel.by_user(user)) }
# Returns all push access levels (for protected branches in scope) that grant push
# access to the given user.
def self.push_access_by_user(user)
PushAccessLevel.joins(:protected_branch).where(protected_branch_id: self.ids).merge(PushAccessLevel.by_user(user))
end
scope :push_access_by_user, -> (user) { PushAccessLevel.joins(:protected_branch).where(protected_branch_id: self.ids).merge(PushAccessLevel.by_user(user)) }
def commit
project.commit(self.name)
......
......@@ -3,6 +3,7 @@ class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base
belongs_to :protected_branch
belongs_to :user
delegate :project, to: :protected_branch
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
......
......@@ -3,6 +3,7 @@ class ProtectedBranch::PushAccessLevel < ActiveRecord::Base
belongs_to :protected_branch
belongs_to :user
delegate :project, to: :protected_branch
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
......
......@@ -202,8 +202,13 @@ feature 'Projected Branches', feature: true, js: true do
end
it "allows deleting a user-specific access level" do
other_authorized_user = create(:user)
project.team << [other_authorized_user, :developer]
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('master')
# First authorized user has access
within('.new_protected_branch') do
find(".allowed-to-#{git_operation_type}").click
click_on authorized_user.name
......@@ -211,11 +216,24 @@ feature 'Projected Branches', feature: true, js: true do
click_on "Protect"
within(".protected-branches-list") { click_on "Settings" }
# Second authorized user has access
within(".allowed-to-#{git_operation_type}-container") do
find(".js-allowed-to-#{git_operation_type}").click
perform_enqueued_jobs { click_on other_authorized_user.name }
end
# Remove first user's access
within(".protected-branch-#{git_operation_type}-access-list") do
perform_enqueued_jobs { click_link "Delete" }
perform_enqueued_jobs { click_link "Delete", match: :first }
end
expect(page).to have_content("Successfully deleted.")
access_levels = ProtectedBranch.first.send("#{git_operation_type}_access_levels".to_sym)
expect(access_levels.count).to eq(1)
expect(access_levels.first.user).to eq(other_authorized_user)
end
end
end
......
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