From a3fdd6acd27f5aa98f13e7a0083d0c3208003ccb Mon Sep 17 00:00:00 2001
From: Toon Claes <toon@gitlab.com>
Date: Wed, 1 Mar 2017 21:23:00 +0100
Subject: [PATCH] Use string based `visibility` getter & setter

Add `visibility` & `visibility=` methods to the
`Gitlab::VisibilityLevel` module so the `visibility_level` can be
get/set with a string value.
---
 app/models/group.rb                     |  2 +-
 app/models/project.rb                   |  2 +-
 app/models/snippet.rb                   |  2 +-
 app/services/projects/create_service.rb |  2 +-
 app/services/system_hooks_service.rb    |  4 ++--
 lib/api/entities.rb                     |  6 ++----
 lib/api/helpers.rb                      |  8 --------
 lib/api/project_snippets.rb             |  4 ++--
 lib/api/projects.rb                     |  8 ++++----
 lib/api/snippets.rb                     |  4 ++--
 lib/gitlab/visibility_level.rb          | 23 ++++++++++++++++++++---
 11 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index 240a17f1dc1..7d23f655225 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -93,7 +93,7 @@ class Group < Namespace
   end
 
   def visibility_level_field
-    visibility_level
+    :visibility_level
   end
 
   def visibility_level_allowed_by_projects
diff --git a/app/models/project.rb b/app/models/project.rb
index 472736c48c4..fa031b061d8 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1003,7 +1003,7 @@ class Project < ActiveRecord::Base
   end
 
   def visibility_level_field
-    visibility_level
+    :visibility_level
   end
 
   def archive!
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 2665a7249a3..dbd564e5e7d 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -120,7 +120,7 @@ class Snippet < ActiveRecord::Base
   end
 
   def visibility_level_field
-    visibility_level
+    :visibility_level
   end
 
   def no_highlighting?
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 6dc3d8c2416..fbdaa455651 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -12,7 +12,7 @@ module Projects
       @project = Project.new(params)
 
       # Make sure that the user is allowed to use the specified visibility level
-      unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level])
+      unless Gitlab::VisibilityLevel.allowed_for?(current_user, @project.visibility_level)
         deny_visibility_level(@project)
         return @project
       end
diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb
index 9b6dd013e3a..868fa7b3f21 100644
--- a/app/services/system_hooks_service.rb
+++ b/app/services/system_hooks_service.rb
@@ -84,7 +84,7 @@ class SystemHooksService
       project_id: model.id,
       owner_name: owner.name,
       owner_email: owner.respond_to?(:email) ? owner.email : "",
-      project_visibility: Project.visibility_levels.key(model.visibility_level_field).downcase
+      project_visibility: Project.visibility_levels.key(model.visibility_level_value).downcase
     }
   end
 
@@ -101,7 +101,7 @@ class SystemHooksService
       user_email:                   model.user.email,
       user_id:                      model.user.id,
       access_level:                 model.human_access,
-      project_visibility:           Project.visibility_levels.key(project.visibility_level_field).downcase
+      project_visibility:           Project.visibility_levels.key(project.visibility_level_value).downcase
     }
   end
 
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index ca18eec2f39..d2d21f5d03a 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -70,8 +70,7 @@ module API
     class Project < Grape::Entity
       expose :id, :description, :default_branch, :tag_list
       expose :archived?, as: :archived
-      expose :ssh_url_to_repo, :http_url_to_repo, :web_url
-      expose(:visibility) { |project, _options| Gitlab::VisibilityLevel.string_level(project.visibility_level) }
+      expose :visibility, :ssh_url_to_repo, :http_url_to_repo, :web_url
       expose :owner, using: Entities::UserBasic, unless: ->(project, options) { project.group }
       expose :name, :name_with_namespace
       expose :path, :path_with_namespace
@@ -132,8 +131,7 @@ module API
     end
 
     class Group < Grape::Entity
-      expose :id, :name, :path, :description
-      expose(:visibility) { |group, _options| Gitlab::VisibilityLevel.string_level(group.visibility_level) }
+      expose :id, :name, :path, :description, :visibility
       expose :lfs_enabled?, as: :lfs_enabled
       expose :avatar_url
       expose :web_url
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 7a39c640c5a..72d2b320077 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -268,14 +268,6 @@ module API
       projects.reorder(params[:order_by] => params[:sort])
     end
 
-    def map_visibility_level(attrs)
-      visibility = attrs.delete(:visibility)
-      if visibility
-        attrs[:visibility_level] = Gitlab::VisibilityLevel.string_options[visibility]
-      end
-      attrs
-    end
-
     # file helpers
 
     def uploaded_file(field, uploads_path)
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index 31919a6270d..f57e7ea4032 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -56,7 +56,7 @@ module API
       end
       post ":id/snippets" do
         authorize! :create_project_snippet, user_project
-        snippet_params = map_visibility_level(declared_params).merge(request: request, api: true)
+        snippet_params = declared_params.merge(request: request, api: true)
         snippet_params[:content] = snippet_params.delete(:code)
 
         snippet = CreateSnippetService.new(user_project, current_user, snippet_params).execute
@@ -89,7 +89,7 @@ module API
 
         authorize! :update_project_snippet, snippet
 
-        snippet_params = map_visibility_level(declared_params(include_missing: false))
+        snippet_params = declared_params(include_missing: false)
           .merge(request: request, api: true)
 
         snippet_params[:content] = snippet_params.delete(:code) if snippet_params[:code].present?
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 743dcac41f9..f302496c12b 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -97,7 +97,7 @@ module API
         use :create_params
       end
       post do
-        attrs = map_visibility_level(declared_params(include_missing: false))
+        attrs = declared_params(include_missing: false)
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?
@@ -126,7 +126,7 @@ module API
         user = User.find_by(id: params.delete(:user_id))
         not_found!('User') unless user
 
-        attrs = map_visibility_level(declared_params(include_missing: false))
+        attrs = declared_params(include_missing: false)
         project = ::Projects::CreateService.new(user, attrs).execute
 
         if project.saved?
@@ -211,9 +211,9 @@ module API
       end
       put ':id' do
         authorize_admin_project
-        attrs = map_visibility_level(declared_params(include_missing: false))
+        attrs = declared_params(include_missing: false)
         authorize! :rename_project, user_project if attrs[:name].present?
-        authorize! :change_visibility_level, user_project if attrs[:visibility_level].present?
+        authorize! :change_visibility_level, user_project if attrs[:visibility].present?
 
         result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute
 
diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb
index c5e55a5df4f..b93fdc62808 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -64,7 +64,7 @@ module API
                               desc: 'The visibility of the snippet'
       end
       post do
-        attrs = map_visibility_level(declared_params(include_missing: false)).merge(request: request, api: true)
+        attrs = declared_params(include_missing: false).merge(request: request, api: true)
         snippet = CreateSnippetService.new(nil, current_user, attrs).execute
 
         render_spam_error! if snippet.spam?
@@ -95,7 +95,7 @@ module API
         return not_found!('Snippet') unless snippet
         authorize! :update_personal_snippet, snippet
 
-        attrs = map_visibility_level(declared_params(include_missing: false).merge(request: request, api: true))
+        attrs = declared_params(include_missing: false).merge(request: request, api: true)
 
         UpdateSnippetService.new(nil, current_user, snippet, attrs).execute
 
diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb
index a3047533ea9..2248763c106 100644
--- a/lib/gitlab/visibility_level.rb
+++ b/lib/gitlab/visibility_level.rb
@@ -95,21 +95,38 @@ module Gitlab
         level_name
       end
 
+      def level_value(level)
+        return string_options[level] if level.is_a? String
+        level
+      end
+
       def string_level(level)
         string_options.key(level)
       end
     end
 
     def private?
-      visibility_level_field == PRIVATE
+      visibility_level_value == PRIVATE
     end
 
     def internal?
-      visibility_level_field == INTERNAL
+      visibility_level_value == INTERNAL
     end
 
     def public?
-      visibility_level_field == PUBLIC
+      visibility_level_value == PUBLIC
+    end
+
+    def visibility_level_value
+      self[visibility_level_field]
+    end
+
+    def visibility
+      Gitlab::VisibilityLevel.string_level(visibility_level_value)
+    end
+
+    def visibility=(level)
+      self[visibility_level_field] = Gitlab::VisibilityLevel.level_value(level)
     end
   end
 end
-- 
GitLab