diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-09-14 05:34:29 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-10-24 09:03:38 +0300 |
commit | cef10ef7d7a20a78d377f711867e361bb51fbaf2 (patch) | |
tree | 98e53a12a546e0a1afdc1cd861e96c231ab8c4c4 /app/services | |
parent | b116df7e9146baf70f04eb6ec1d19091278c14d5 (diff) |
Implement review comments from @dbalexandre.
1. Don't have any EE-only code in CE. Ok to have CE-only code in EE.
2. Use `case` instead of `if/elsif`
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/protected_branches/api_update_service.rb | 45 |
1 files changed, 8 insertions, 37 deletions
diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb index 41d3d413caa..1c27d32aad8 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/api_update_service.rb @@ -14,25 +14,19 @@ module ProtectedBranches @protected_branch = protected_branch protected_branch.transaction do - # If a protected branch can have only a single access level (CE), delete it to - # make room for the new access level. If a protected branch can have more than - # one access level (EE), only remove the relevant access levels. If we don't do this, - # we'll have a failed validation. - if protected_branch_restricted_to_single_access_level? - delete_redundant_ce_access_levels - else - delete_redundant_ee_access_levels - end + delete_redundant_access_levels - if @developers_can_push + case @developers_can_push + when true params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) - elsif @developers_can_push == false + when false params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) end - if @developers_can_merge + case @developers_can_merge + when true params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) - elsif @developers_can_merge == false + when false params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) end @@ -43,7 +37,7 @@ module ProtectedBranches private - def delete_redundant_ce_access_levels + def delete_redundant_access_levels if @developers_can_merge || @developers_can_merge == false @protected_branch.merge_access_levels.destroy_all end @@ -52,28 +46,5 @@ module ProtectedBranches @protected_branch.push_access_levels.destroy_all end end - - def delete_redundant_ee_access_levels - if @developers_can_merge - @protected_branch.merge_access_levels.developer.destroy_all - elsif @developers_can_merge == false - @protected_branch.merge_access_levels.developer.destroy_all - @protected_branch.merge_access_levels.master.destroy_all - end - - if @developers_can_push - @protected_branch.push_access_levels.developer.destroy_all - elsif @developers_can_push == false - @protected_branch.push_access_levels.developer.destroy_all - @protected_branch.push_access_levels.master.destroy_all - end - end - - def protected_branch_restricted_to_single_access_level? - length_validator = ProtectedBranch.validators_on(:push_access_levels).find do |validator| - validator.is_a? ActiveModel::Validations::LengthValidator - end - length_validator.options[:is] == 1 if length_validator - end end end |