Commit 60a38e5c authored by Timothy Andrew's avatar Timothy Andrew

Implement changes from @DouweM's review of !927

- Move `ProctedBranchAccessEe` to `EE::ProtectedBranchAccess`. This is a lot
  cleaner, and has a precedent (`EE::Group`).

- Include `EE::ProtectedBranchAccess` inside `ProtectedBranchAccess`, instead of
  including both in the model classes. `EE::ProtectedBranchAccess` depends on
  `ProtectedBranchAccess` - this is a good way to model that dependency.

- Remove author credit from CHANGELOG entry
parent 2a3b3545
......@@ -2,6 +2,8 @@ module ProtectedBranchAccess
extend ActiveSupport::Concern
included do
include EE::ProtectedBranchAccess
belongs_to :protected_branch
delegate :project, to: :protected_branch
......@@ -18,10 +20,4 @@ module ProtectedBranchAccess
project.team.max_member_access(user.id) >= access_level
end
def check_access(user)
return true if user.is_admin?
project.team.max_member_access(user.id) >= access_level
end
end
# EE-specific code related to protected branch access levels.
#
# Note: Include `ProtectedBranchAccess` _before_ including this module, since
# a number of methods here override methods in `ProtectedBranchAccess`
module ProtectedBranchAccessEe
extend ActiveSupport::Concern
included do
belongs_to :user
belongs_to :group
validates :group_id, uniqueness: { scope: :protected_branch, allow_nil: true }
validates :user_id, uniqueness: { scope: :protected_branch, allow_nil: true }
validates :access_level, uniqueness: { scope: :protected_branch, if: :role?,
conditions: -> { where(user_id: nil, group_id: nil) } }
scope :by_user, -> (user) { where(user: user ) }
scope :by_group, -> (group) { where(group: group ) }
end
def type
if self.user.present?
:user
elsif self.group.present?
:group
else
:role
end
end
# Is this a role-based access level?
def role?
type == :role
end
def humanize
return self.user.name if self.user.present?
return self.group.name if self.group.present?
super
end
def check_access(user)
return true if user.is_admin?
return user.id == self.user_id if self.user.present?
return group.users.exists?(user.id) if self.group.present?
super
end
end
# EE-specific code related to protected branch access levels.
#
# Note: Don't directly include this concern into a model class.
# Instead, include `ProtectedBranchAccess`, which in turn includes
# this concern. A number of methods here depend on `ProtectedBranchAccess`
# being next up in the ancestor chain.
module EE
module ProtectedBranchAccess
extend ActiveSupport::Concern
included do
belongs_to :user
belongs_to :group
validates :group_id, uniqueness: { scope: :protected_branch, allow_nil: true }
validates :user_id, uniqueness: { scope: :protected_branch, allow_nil: true }
validates :access_level, uniqueness: { scope: :protected_branch, if: :role?,
conditions: -> { where(user_id: nil, group_id: nil) } }
scope :by_user, -> (user) { where(user: user ) }
scope :by_group, -> (group) { where(group: group ) }
end
def type
if self.user.present?
:user
elsif self.group.present?
:group
else
:role
end
end
# Is this a role-based access level?
def role?
type == :role
end
def humanize
return self.user.name if self.user.present?
return self.group.name if self.group.present?
super
end
def check_access(user)
return true if user.is_admin?
return user.id == self.user_id if self.user.present?
return group.users.exists?(user.id) if self.group.present?
super
end
end
end
class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base
include ProtectedBranchAccess
include ProtectedBranchAccessEe
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER] }
......
class ProtectedBranch::PushAccessLevel < ActiveRecord::Base
include ProtectedBranchAccess
include ProtectedBranchAccessEe
validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER,
Gitlab::Access::DEVELOPER,
......
---
title: Technical debt follow-up from restricting pushes / merges by group
merge_request: 927
author: Timothy Andrew
author:
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