Improve group visibility level feature

3 files changed, 16 insertions, 24 deletions

diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb
index 053b6a05281..1db81216084 100644
--- a/app/services/groups/base_service.rb
+++ b/app/services/groups/base_service.rb
@@ -8,18 +8,13 @@ module Groups
 
     private
 
-    def visibility_allowed_for_user?(level)
+    def visibility_allowed_for_user?
+      level = group.visibility_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
+      group.errors.add(:visibility_level, "#{level} has been restricted by your GitLab administrator.") unless allowed_by_user
+      
+      allowed_by_user
     end
   end
 end
diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb
index 38742369d82..f605ccca81b 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -2,14 +2,16 @@ module Groups
   class CreateService < Groups::BaseService
     def initialize(user, params = {})
       @current_user, @params = user, params.dup
-      @group = Group.new(@params)
     end
 
     def execute
-      return @group unless visibility_allowed_for_user?(@params[:visibility_level])
+      @group = Group.new(params)
+
+      return @group unless visibility_allowed_for_user?
+
       @group.name = @group.path.dup unless @group.name
       @group.save
-      @group.add_owner(@current_user)
+      @group.add_owner(current_user)
       @group
     end
   end
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb
index b910e0fde98..0b0c5a35d37 100644
--- a/app/services/groups/update_service.rb
+++ b/app/services/groups/update_service.rb
@@ -1,20 +1,15 @@
-#Checks visibility level permission check before updating a group
-#Do not allow to put Group visibility level smaller than its projects
-#Do not allow unauthorized permission levels
+# Checks visibility level permission check before updating a group
+# Do not allow to put Group visibility level smaller than its projects
+# Do not allow unauthorized permission levels
 
 module Groups
   class UpdateService < Groups::BaseService
     def execute
-      return false unless visibility_level_allowed?(params[:visibility_level])
-      group.update_attributes(params)
-    end
-
-    private
+      group.assign_attributes(params)
 
-    def visibility_level_allowed?(level)
-      return true unless level.present?
+      return false unless visibility_allowed_for_user?
 
-      visibility_allowed_for_project?(level) && visibility_allowed_for_user?(level)
+      group.save
     end
   end
 end