Code fixes

3 files changed, 24 insertions, 30 deletions

diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb
index 644ec7c013e..053b6a05281 100644
--- a/app/services/groups/base_service.rb
+++ b/app/services/groups/base_service.rb
@@ -6,8 +6,20 @@ module Groups
       @group, @current_user, @params = group, user, params.dup
     end
 
-    def add_error_message(message)
-      group.errors.add(:visibility_level, message)
+    private
+
+    def visibility_allowed_for_user?(level)
+      allowed_by_user  = Gitlab::VisibilityLevel.allowed_for?(current_user, level)
+      @group.errors.add(:visibility_level, "You are not authorized to set this permission level.") unless allowed_by_user
+      allowed_by_user
+    end
+
+    def visibility_allowed_for_project?(level)
+      projects_visibility = group.projects.pluck(:visibility_level)
+
+      allowed_by_projects = !projects_visibility.any? { |project_visibility| level.to_i < project_visibility }
+      @group.errors.add(:visibility_level, "Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects
+      allowed_by_projects
     end
   end
 end
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index e2875aafb94..38742369d82 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -1,17 +1,16 @@
 module Groups
   class CreateService < Groups::BaseService
-    def execute
-      return false unless visibility_level_allowed?(params[:visibility_level])
-      @group.name = @group.path.dup unless @group.name
-      @group.save(params) && @group.add_owner(current_user)
+    def initialize(user, params = {})
+      @current_user, @params = user, params.dup
+      @group = Group.new(@params)
     end
 
-    private
-
-    def visibility_level_allowed?(level)
-      allowed = Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
-      add_error_message("Visibility level restricted by admin.") unless allowed
-      allowed
+    def execute
+      return @group unless visibility_allowed_for_user?(@params[:visibility_level])
+      @group.name = @group.path.dup unless @group.name
+      @group.save
+      @group.add_owner(@current_user)
+      @group
     end
   end
 end
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb
index a7382c1e07c..b910e0fde98 100644
--- a/app/services/groups/update_service.rb
+++ b/app/services/groups/update_service.rb
@@ -14,24 +14,7 @@ module Groups
     def visibility_level_allowed?(level)
       return true unless level.present?
 
-      allowed_by_projects = visibility_by_project(level)
-      allowed_by_user     = visibility_by_user(level)
-
-      allowed_by_projects && allowed_by_user
-    end
-
-    def visibility_by_project(level)
-      projects_visibility = group.projects.pluck(:visibility_level)
-
-      allowed_by_projects = !projects_visibility.any?{ |project_visibility| level.to_i < project_visibility }
-      add_error_message("Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects
-      allowed_by_projects
-    end
-
-    def visibility_by_user(level)
-      allowed_by_user  = Gitlab::VisibilityLevel.allowed_for?(current_user, level)
-      add_error_message("You are not authorized to set this permission level.") unless allowed_by_user
-      allowed_by_user
+      visibility_allowed_for_project?(level) && visibility_allowed_for_user?(level)
     end
   end
 end