Move restricted visibility settings to the UI

Add checkboxes to the application settings page for restricted
visibility levels, and remove those settings from gitlab.yml.

CHANGELOG

@@ -11,6 +11,7 @@ v 7.9.0 (unreleased)
   - Improve error messages for file edit failures
   - Improve UI for commits, issues and merge request lists
   - Fix commit comments on first line of diff not rendering in Merge Request Discussion view.
+  - Move restricted visibility settings from gitlab.yml into the web UI.
   - Improve trigger merge request hook when source project branch has been updated (Kirill Zaitsev)
   - Save web edit in new branch
   - Fix ordering of imported but unchanged projects (Marco Wessel)

app/assets/stylesheets/generic/forms.scss

@@ -97,3 +97,8 @@ label {
 .wiki-content {
   margin-top: 35px;
 }
+
+.btn-group .btn.active {
+  text-shadow: 0 0 0.2em #D9534F, 0 0 0.2em #D9534F, 0 0 0.2em #D9534F;
+  background-color: #5487bf;
+}

app/controllers/admin/application_settings_controller.rb

@@ -20,6 +20,13 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   end
 
   def application_setting_params
+    restricted_levels = params[:application_setting][:restricted_visibility_levels]
+    unless restricted_levels.nil?
+      restricted_levels.map! do |level|
+        level.to_i
+      end
+    end
+
     params.require(:application_setting).permit(
       :default_projects_limit,
       :default_branch_protection,
@@ -28,7 +35,8 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
       :gravatar_enabled,
       :twitter_sharing_enabled,
       :sign_in_text,
-      :home_page_url
+      :home_page_url,
+      restricted_visibility_levels: []
     )
   end
 end

app/helpers/application_settings_helper.rb

@@ -18,4 +18,20 @@ module ApplicationSettingsHelper
   def extra_sign_in_text
     current_application_settings.sign_in_text
   end
+
+  # Return a group of checkboxes that use Bootstrap's button plugin for a
+  # toggle button effect.
+  def restricted_level_checkboxes(help_block_id)
+    Gitlab::VisibilityLevel.options.map do |name, level|
+      checked = restricted_visibility_levels(true).include?(level)
+      css_class = 'btn btn-primary'
+      css_class += ' active' if checked
+      checkbox_name = 'application_setting[restricted_visibility_levels][]'
+
+      label_tag(checkbox_name, class: css_class) do
+        check_box_tag(checkbox_name, level, checked, autocomplete: 'off',
+                      'aria-describedby' => help_block_id) + name
+      end
+    end
+  end
 end

app/helpers/visibility_level_helper.rb

@@ -60,7 +60,8 @@ module VisibilityLevelHelper
     Project.visibility_levels.key(level)
   end
 
-  def restricted_visibility_levels
-    current_user.is_admin? ? [] : gitlab_config.restricted_visibility_levels
+  def restricted_visibility_levels(show_all = false)
+    return [] if current_user.is_admin? && !show_all
+    current_application_settings.restricted_visibility_levels
   end
 end

app/models/application_setting.rb

@@ -2,25 +2,38 @@
 #
 # Table name: application_settings
 #
-#  id                        :integer          not null, primary key
-#  default_projects_limit    :integer
-#  signup_enabled            :boolean
-#  signin_enabled            :boolean
-#  gravatar_enabled          :boolean
-#  sign_in_text              :text
-#  created_at                :datetime
-#  updated_at                :datetime
-#  home_page_url             :string(255)
-#  default_branch_protection :integer          default(2)
-#  twitter_sharing_enabled   :boolean          default(TRUE)
+#  id                           :integer        not null, primary key
+#  default_projects_limit       :integer
+#  default_branch_protection    :integer
+#  signup_enabled               :boolean
+#  signin_enabled               :boolean
+#  gravatar_enabled             :boolean
+#  twitter_sharing_enabled      :boolean
+#  sign_in_text                 :text
+#  created_at                   :datetime
+#  updated_at                   :datetime
+#  home_page_url                :string(255)
+#  default_branch_protection    :integer          default(2)
+#  twitter_sharing_enabled      :boolean          default(TRUE)
+#  restricted_visibility_levels :text
 #
 
 class ApplicationSetting < ActiveRecord::Base
+  serialize :restricted_visibility_levels
+
   validates :home_page_url,
     allow_blank: true,
     format: { with: URI::regexp(%w(http https)), message: "should be a valid url" },
     if: :home_page_url_column_exist
 
+  validates_each :restricted_visibility_levels do |record, attr, value|
+    value.each do |level|
+      unless Gitlab::VisibilityLevel.options.has_value?(level)
+        record.errors.add(attr, "'#{level}' is not a valid visibility level")
+      end
+    end
+  end
+
   def self.current
     ApplicationSetting.last
   end
@@ -34,6 +47,7 @@ class ApplicationSetting < ActiveRecord::Base
       twitter_sharing_enabled: Settings.gitlab['twitter_sharing_enabled'],
       gravatar_enabled: Settings.gravatar['enabled'],
       sign_in_text: Settings.extra['sign_in_text'],
+      restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels']
     )
   end
 

app/models/project.rb

@@ -34,6 +34,8 @@ require 'file_size_validator'
 
 class Project < ActiveRecord::Base
   include Sortable
+  include Gitlab::CurrentSettings
+  extend Gitlab::CurrentSettings
   include Gitlab::ShellAdapter
   include Gitlab::VisibilityLevel
   include Gitlab::ConfigHelper
@@ -132,8 +134,8 @@ class Project < ActiveRecord::Base
   validates :issues_enabled, :merge_requests_enabled,
             :wiki_enabled, inclusion: { in: [true, false] }
   validates :visibility_level,
-    exclusion: { in: gitlab_config.restricted_visibility_levels },
-    if: -> { gitlab_config.restricted_visibility_levels.any? }
+    exclusion: { in: current_application_settings.restricted_visibility_levels },
+    if: -> { current_application_settings.restricted_visibility_levels.any? }
   validates :issues_tracker_id, length: { maximum: 255 }, allow_blank: true
   validates :namespace, presence: true
   validates_uniqueness_of :name, scope: :namespace_id

app/services/base_service.rb

@@ -31,10 +31,6 @@ class BaseService
     SystemHooksService.new
   end
 
-  def current_application_settings
-    ApplicationSetting.current
-  end
-
   private
 
   def error(message, http_status = nil)

app/services/update_snippet_service.rb

@@ -0,0 +1,22 @@
+class UpdateSnippetService < BaseService
+  attr_accessor :snippet
+
+  def initialize(project = nil, user, snippet, params = {})
+    super(project, user, params)
+    @snippet = snippet
+  end
+
+  def execute
+    # check that user is allowed to set specified visibility_level
+    new_visibility = params[:visibility_level]
+    if new_visibility && new_visibility != snippet.visibility_level
+      unless can?(current_user, :change_visibility_level, snippet) &&
+        Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
+        deny_visibility_level(snippet, new_visibility_level)
+        return snippet
+      end
+    end
+
+    snippet.update_attributes(params)
+  end
+end

app/views/admin/application_settings/_form.html.haml

@@ -34,6 +34,14 @@
       = f.label :default_branch_protection, class: 'control-label col-sm-2'
       .col-sm-10
         = f.select :default_branch_protection, options_for_select(Gitlab::Access.protection_options, @application_setting.default_branch_protection), {}, class: 'form-control'
+    .form-group
+      = f.label :restricted_visibility_levels, class: 'control-label col-sm-2'
+      .col-sm-10
+        - data_attrs = { toggle: 'buttons' }
+        .btn-group{ data: data_attrs }
+          - restricted_level_checkboxes('restricted-visibility-help').each do |level|
+            = level
+        %span.help-block#restricted-visibility-help Selected levels cannot be used by non-admin users for projects or snippets
     .form-group
       = f.label :home_page_url, class: 'control-label col-sm-2'
       .col-sm-10

config/gitlab.yml.example

@@ -56,10 +56,6 @@ production: &base
     ##   COLOR  = 5
     # default_theme: 2 # default: 2
 
-    # Restrict setting visibility levels for non-admin users.
-    # The default is to allow all levels.
-    # restricted_visibility_levels: [ "public" ]
-
     ## Automatic issue closing
     # If a commit message matches this regular expression, all issues referenced from the matched text will be closed.
     # This happens when the commit is pushed or merged into the default branch of a project.

db/migrate/20150301014758_add_restricted_visibility_levels_to_application_settings.rb

@@ -0,0 +1,5 @@
+class AddRestrictedVisibilityLevelsToApplicationSettings < ActiveRecord::Migration
+  def change
+    add_column :application_settings, :restricted_visibility_levels, :text
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150225065047) do
+ActiveRecord::Schema.define(version: 20150301014758) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -25,8 +25,9 @@ ActiveRecord::Schema.define(version: 20150225065047) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "home_page_url"
-    t.integer  "default_branch_protection", default: 2
-    t.boolean  "twitter_sharing_enabled",   default: true
+    t.integer  "default_branch_protection",    default: 2
+    t.boolean  "twitter_sharing_enabled",      default: true
+    t.text     "restricted_visibility_levels"
   end
 
   create_table "broadcast_messages", force: true do |t|

lib/gitlab/current_settings.rb

@@ -5,8 +5,7 @@ module Gitlab
 
       RequestStore.store[key] ||= begin
         if ActiveRecord::Base.connected? && ActiveRecord::Base.connection.table_exists?('application_settings')
-          RequestStore.store[:current_application_settings] =
-            (ApplicationSetting.current || ApplicationSetting.create_from_defaults)
+          ApplicationSetting.current || ApplicationSetting.create_from_defaults
         else
           fake_application_settings
         end
@@ -21,6 +20,7 @@ module Gitlab
         signin_enabled: Settings.gitlab['signin_enabled'],
         gravatar_enabled: Settings.gravatar['enabled'],
         sign_in_text: Settings.extra['sign_in_text'],
+        restricted_visibility_levels: Settings.gitlab['restricted_visibility_levels']
       )
     end
   end

lib/gitlab/visibility_level.rb

@@ -5,6 +5,8 @@
 #
 module Gitlab
   module VisibilityLevel
+    extend CurrentSettings
+
     PRIVATE  = 0 unless const_defined?(:PRIVATE)
     INTERNAL = 10 unless const_defined?(:INTERNAL)
     PUBLIC   = 20 unless const_defined?(:PUBLIC)
@@ -23,21 +25,21 @@ module Gitlab
       end
 
       def allowed_for?(user, level)
-        user.is_admin? || allowed_level?(level)
+        user.is_admin? || allowed_level?(level.to_i)
       end
 
-      # Level can be a string `"public"` or a value `20`, first check if valid,
-      # then check if the corresponding string appears in the config
+      # Return true if the specified level is allowed for the current user.
+      # Level should be a numeric value, e.g. `20`.
       def allowed_level?(level)
-        if options.has_key?(level.to_s)
-          non_restricted_level?(level)
-        elsif options.has_value?(level.to_i)
-          non_restricted_level?(options.key(level.to_i).downcase)
-        end
+        valid_level?(level) && non_restricted_level?(level)
       end
 
       def non_restricted_level?(level)
-        ! Gitlab.config.gitlab.restricted_visibility_levels.include?(level)
+        ! current_application_settings.restricted_visibility_levels.include?(level)
+      end
+
+      def valid_level?(level)
+        options.has_value?(level)
       end
     end
 

spec/models/application_setting_spec.rb

@@ -2,17 +2,19 @@
 #
 # Table name: application_settings
 #
-#  id                        :integer          not null, primary key
-#  default_projects_limit    :integer
-#  signup_enabled            :boolean
-#  signin_enabled            :boolean
-#  gravatar_enabled          :boolean
-#  sign_in_text              :text
-#  created_at                :datetime
-#  updated_at                :datetime
-#  home_page_url             :string(255)
-#  default_branch_protection :integer          default(2)
-#  twitter_sharing_enabled   :boolean          default(TRUE)
+#  id                           :integer          not null, primary key
+#  default_projects_limit       :integer
+#  default_branch_protection    :integer
+#  signup_enabled               :boolean
+#  signin_enabled               :boolean
+#  gravatar_enabled             :boolean
+#  sign_in_text                 :text
+#  created_at                   :datetime
+#  updated_at                   :datetime
+#  home_page_url                :string(255)
+#  default_branch_protection    :integer          default(2)
+#  twitter_sharing_enabled      :boolean          default(TRUE)
+#  restricted_visibility_levels :text
 #
 
 require 'spec_helper'