Tweaks, refactoring, and specs

3 files changed, 16 insertions, 15 deletions

diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb
index 1db81216084..1642115583d 100644
--- a/app/services/groups/base_service.rb
+++ b/app/services/groups/base_service.rb
@@ -1,20 +1,9 @@
 module Groups
-  class BaseService
+  class BaseService < BaseService
     attr_accessor :group, :current_user, :params
 
     def initialize(group, user, params = {})
       @group, @current_user, @params = group, user, params.dup
     end
-
-    private
-
-    def visibility_allowed_for_user?
-      level = group.visibility_level
-      allowed_by_user  = Gitlab::VisibilityLevel.allowed_for?(current_user, level)
-
-      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 f605ccca81b..46c2a53e1f6 100644
--- a/app/services/groups/create_service.rb
+++ b/app/services/groups/create_service.rb
@@ -7,7 +7,10 @@ module Groups
     def execute
       @group = Group.new(params)
 
-      return @group unless visibility_allowed_for_user?
+      unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
+        deny_visibility_level(@group)
+        return @group
+      end
 
       @group.name = @group.path.dup unless @group.name
       @group.save
diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb
index 0b0c5a35d37..b70e2e4aaa9 100644
--- a/app/services/groups/update_service.rb
+++ b/app/services/groups/update_service.rb
@@ -5,9 +5,18 @@
 module Groups
   class UpdateService < Groups::BaseService
     def execute
-      group.assign_attributes(params)
+      # check that user is allowed to set specified visibility_level
+      new_visibility = params[:visibility_level]
+      if new_visibility && new_visibility.to_i != group.visibility_level
+        unless can?(current_user, :change_visibility_level, group) &&
+          Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
+
+          deny_visibility_level(group, new_visibility)
+          return group
+        end
+      end
 
-      return false unless visibility_allowed_for_user?
+      group.assign_attributes(params)
 
       group.save
     end