diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-07-29 09:13:07 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-07-29 12:50:39 +0300 |
commit | 0a8aeb46dc187cc309ddbe23d8624f5d24b6218c (patch) | |
tree | fc19d99c449d91ea56c65120f45d773dfdf77dcb /app/models/protected_branch | |
parent | c93a895abc434b9b78aa7cf4d285ce309cfd868a (diff) |
Use `Gitlab::Access` to protected branch access levels.
1. It makes sense to reuse these constants since we had them duplicated
in the previous enum implementation. This also simplifies our
`check_access` implementation, because we can use
`project.team.max_member_access` directly.
2. Use `accepts_nested_attributes_for` to create push/merge access
levels. This was a bit fiddly to set up, but this simplifies our code
by quite a large amount. We can even get rid of
`ProtectedBranches::BaseService`.
3. Move API handling back into the API (previously in
`ProtectedBranches::BaseService#translate_api_params`.
4. The protected branch services now return a `ProtectedBranch` rather
than `true/false`.
5. Run `load_protected_branches` on-demand in the `create` action, to
prevent it being called unneccessarily.
6. "Masters" is pre-selected as the default option for "Allowed to Push"
and "Allowed to Merge".
7. These changes were based on a review from @rymai in !5081.
Diffstat (limited to 'app/models/protected_branch')
-rw-r--r-- | app/models/protected_branch/merge_access_level.rb | 16 | ||||
-rw-r--r-- | app/models/protected_branch/push_access_level.rb | 21 |
2 files changed, 13 insertions, 24 deletions
diff --git a/app/models/protected_branch/merge_access_level.rb b/app/models/protected_branch/merge_access_level.rb index 17a3a86c3e1..25a6ca6a8ee 100644 --- a/app/models/protected_branch/merge_access_level.rb +++ b/app/models/protected_branch/merge_access_level.rb @@ -2,25 +2,19 @@ class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base belongs_to :protected_branch delegate :project, to: :protected_branch - enum access_level: [:masters, :developers] + validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER, + Gitlab::Access::DEVELOPER] } def self.human_access_levels { - "masters" => "Masters", - "developers" => "Developers + Masters" + Gitlab::Access::MASTER => "Masters", + Gitlab::Access::DEVELOPER => "Developers + Masters" }.with_indifferent_access end def check_access(user) return true if user.is_admin? - - min_member_access = if masters? - Gitlab::Access::MASTER - elsif developers? - Gitlab::Access::DEVELOPER - end - - project.team.max_member_access(user.id) >= min_member_access + project.team.max_member_access(user.id) >= access_level end def humanize diff --git a/app/models/protected_branch/push_access_level.rb b/app/models/protected_branch/push_access_level.rb index 22096b13300..1999316aa26 100644 --- a/app/models/protected_branch/push_access_level.rb +++ b/app/models/protected_branch/push_access_level.rb @@ -2,27 +2,22 @@ class ProtectedBranch::PushAccessLevel < ActiveRecord::Base belongs_to :protected_branch delegate :project, to: :protected_branch - enum access_level: [:masters, :developers, :no_one] + validates :access_level, presence: true, inclusion: { in: [Gitlab::Access::MASTER, + Gitlab::Access::DEVELOPER, + Gitlab::Access::NO_ACCESS] } def self.human_access_levels { - "masters" => "Masters", - "developers" => "Developers + Masters", - "no_one" => "No one" + Gitlab::Access::MASTER => "Masters", + Gitlab::Access::DEVELOPER => "Developers + Masters", + Gitlab::Access::NO_ACCESS => "No one" }.with_indifferent_access end def check_access(user) - return false if no_one? + return false if access_level == Gitlab::Access::NO_ACCESS return true if user.is_admin? - - min_member_access = if masters? - Gitlab::Access::MASTER - elsif developers? - Gitlab::Access::DEVELOPER - end - - project.team.max_member_access(user.id) >= min_member_access + project.team.max_member_access(user.id) >= access_level end def humanize |