Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTimothy Andrew <mail@timothyandrew.net>2016-06-24 08:07:04 +0300
committerTimothy Andrew <mail@timothyandrew.net>2016-07-13 10:54:56 +0300
commit60245bbe228014a9f689adafd64b571883ef6eb3 (patch)
tree2af0d7785fb1e9c1f168b01d704e788d42d89503 /app/services/commits
parent495db09653bafb0371e5d5a5f12d5bc33cdb584b (diff)
Refactor `Gitlab::GitAccess`
1. Don't use case statements for dispatch anymore. This leads to a lot of duplication, and makes the logic harder to follow. 2. Remove duplicated logic. - For example, the `can_push_to_branch?` exists, but we also have a different way of checking the same condition within `change_access_check`. - This kind of duplication is removed, and the `can_push_to_branch?` method is used in both places. 3. Move checks returning true/false to `UserAccess`. - All public methods in `GitAccess` now return an instance of `GitAccessStatus`. Previously, some methods would return true/false as well, which was confusing. - It makes sense for these kinds of checks to be at the level of a user, so the `UserAccess` class was repurposed for this. The prior `UserAccess.allowed?` classmethod is converted into an instance method. - All external uses of these checks have been migrated to use the `UserAccess` class 4. Move the "change_access_check" into a separate class. - Create the `GitAccess::ChangeAccessCheck` class to run these checks, which are quite substantial. - `ChangeAccessCheck` returns an instance of `GitAccessStatus` as well. 5. Break out the boolean logic in `ChangeAccessCheck` into `if/else` chains - this seems more readable. 6. I can understand that this might look like overkill for !4892, but I think this is a good opportunity to clean it up. - http://martinfowler.com/bliki/OpportunisticRefactoring.html
Diffstat (limited to 'app/services/commits')
-rw-r--r--app/services/commits/change_service.rb4
1 files changed, 2 insertions, 2 deletions
diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index c578097376a..ed73d8cb8c2 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -23,7 +23,7 @@ module Commits
private
def check_push_permissions
- allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch)
+ allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
unless allowed
raise ValidationError.new('You are not allowed to push into this branch')
@@ -31,7 +31,7 @@ module Commits
true
end
-
+
def create_target_branch(new_branch)
# Temporary branch exists and contains the change commit
return success if repository.find_branch(new_branch)