From 1051087ac4efc3dbf45bd075e36af647d2b66d62 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 30 Sep 2016 13:14:46 +0530 Subject: Implement second round of review comments from @DouweM. - Pass `developers_and_merge` and `developers_can_push` in `params` instead of using keyword arguments. - Refactor a slightly complex boolean check to a simple `nil?` check. --- app/services/protected_branches/api_create_service.rb | 6 +++--- app/services/protected_branches/api_update_service.rb | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'app/services/protected_branches') diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb index cbb99ede9f3..d714a8aaf01 100644 --- a/app/services/protected_branches/api_create_service.rb +++ b/app/services/protected_branches/api_create_service.rb @@ -4,10 +4,10 @@ # lives in this service. module ProtectedBranches class ApiCreateService < BaseService - def initialize(project, user, params, developers_can_push:, developers_can_merge:) + def initialize(project, user, params) + @developers_can_merge = params.delete(:developers_can_merge) + @developers_can_push = params.delete(:developers_can_push) super(project, user, params) - @developers_can_merge = developers_can_merge - @developers_can_push = developers_can_push end def execute diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb index 1c27d32aad8..c28bffee2f4 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/api_update_service.rb @@ -4,10 +4,10 @@ # lives in this service. module ProtectedBranches class ApiUpdateService < BaseService - def initialize(project, user, params, developers_can_push:, developers_can_merge:) + def initialize(project, user, params) + @developers_can_merge = params.delete(:developers_can_merge) + @developers_can_push = params.delete(:developers_can_push) super(project, user, params) - @developers_can_merge = developers_can_merge - @developers_can_push = developers_can_push end def execute(protected_branch) @@ -38,11 +38,11 @@ module ProtectedBranches private def delete_redundant_access_levels - if @developers_can_merge || @developers_can_merge == false + unless @developers_can_merge.nil? @protected_branch.merge_access_levels.destroy_all end - if @developers_can_push || @developers_can_push == false + unless @developers_can_push.nil? @protected_branch.push_access_levels.destroy_all end end -- cgit v1.2.3