diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2019-07-12 12:10:54 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2019-07-17 14:56:12 +0300 |
commit | 6c27c0d394b70be4f2a2e0fa047f6844199c2661 (patch) | |
tree | 6f874386333b58409a02c8a883b1b12aefcaa6ab /app/services/merge_requests | |
parent | 9c3dfd2085369005305260bdaec872cb078a758e (diff) |
Filter params in MR build service
Reusing the existing `IssuableBaseService#filter_params` which uses
the policies to determine what params a user can set, and which values
it can be set to.
This also removed the need for the seperate call to
`IssuableBaseService#ensure_milestone_available`.
The `Issues::BuildService` does not suffer from this because it limits
the params that are assignable to the `title`, `description` and
`milestone_id`.
Diffstat (limited to 'app/services/merge_requests')
-rw-r--r-- | app/services/merge_requests/build_service.rb | 28 |
1 files changed, 22 insertions, 6 deletions
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 109c964e577..b28f80939ae 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -11,15 +11,18 @@ module MergeRequests # https://gitlab.com/gitlab-org/gitlab-ce/issues/53658 merge_quick_actions_into_params!(merge_request, only: [:target_branch]) merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) if params.has_key?(:force_remove_source_branch) - merge_request.assign_attributes(params) + # Assign the projects first so we can use policies for `filter_params` merge_request.author = current_user + merge_request.source_project = find_source_project + merge_request.target_project = find_target_project + + filter_params(merge_request) + merge_request.assign_attributes(params.to_h.compact) + merge_request.compare_commits = [] - merge_request.source_project = find_source_project - merge_request.target_project = find_target_project - merge_request.target_branch = find_target_branch - merge_request.can_be_created = projects_and_branches_valid? - ensure_milestone_available(merge_request) + merge_request.target_branch = find_target_branch + merge_request.can_be_created = projects_and_branches_valid? # compare branches only if branches are valid, otherwise # compare_branches may raise an error @@ -50,12 +53,14 @@ module MergeRequests to: :merge_request def find_source_project + source_project = project_from_params(:source_project) return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project) project end def find_target_project + target_project = project_from_params(:target_project) return target_project if target_project.present? && can?(current_user, :create_merge_request_in, target_project) target_project = project.default_merge_request_target @@ -65,6 +70,17 @@ module MergeRequests project end + def project_from_params(param_name) + project_from_params = params.delete(param_name) + + id_param_name = :"#{param_name}_id" + if project_from_params.nil? && params[id_param_name] + project_from_params = Project.find_by_id(params.delete(id_param_name)) + end + + project_from_params + end + def find_target_branch target_branch || target_project.default_branch end |