summaryrefslogtreecommitdiff
path: root/app/models
diff options
context:
space:
mode:
authorTimothy Andrew <mail@timothyandrew.net>2016-07-29 11:43:07 +0530
committerTimothy Andrew <mail@timothyandrew.net>2016-07-29 15:20:39 +0530
commit0a8aeb46dc187cc309ddbe23d8624f5d24b6218c (patch)
treefc19d99c449d91ea56c65120f45d773dfdf77dcb /app/models
parentc93a895abc434b9b78aa7cf4d285ce309cfd868a (diff)
downloadgitlab-ce-0a8aeb46dc187cc309ddbe23d8624f5d24b6218c.tar.gz
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')
-rw-r--r--app/models/protected_branch.rb11
-rw-r--r--app/models/protected_branch/merge_access_level.rb16
-rw-r--r--app/models/protected_branch/push_access_level.rb21
3 files changed, 16 insertions, 32 deletions
diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb
index c0bee72b4d7..226b3f54342 100644
--- a/app/models/protected_branch.rb
+++ b/app/models/protected_branch.rb
@@ -8,18 +8,13 @@ class ProtectedBranch < ActiveRecord::Base
has_one :merge_access_level, dependent: :destroy
has_one :push_access_level, dependent: :destroy
+ accepts_nested_attributes_for :push_access_level
+ accepts_nested_attributes_for :merge_access_level
+
def commit
project.commit(self.name)
end
- def allowed_to_push
- self.push_access_level && self.push_access_level.access_level
- end
-
- def allowed_to_merge
- self.merge_access_level && self.merge_access_level.access_level
- end
-
# Returns all protected branches that match the given branch name.
# This realizes all records from the scope built up so far, and does
# _not_ return a relation.
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