Skip to content

Commit

Permalink
Move restricted visibility settings to the UI
Browse files Browse the repository at this point in the history
Add checkboxes to the application settings page for restricted
visibility levels, and remove those settings from gitlab.yml.
  • Loading branch information
mr-vinn committed Mar 7, 2015
1 parent 3cf4359 commit cacac14
Show file tree
Hide file tree
Showing 16 changed files with 128 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions app/assets/stylesheets/generic/forms.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
10 changes: 9 additions & 1 deletion app/controllers/admin/application_settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ def set_application_setting
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,
Expand All @@ -28,7 +35,8 @@ def application_setting_params
:gravatar_enabled,
:twitter_sharing_enabled,
:sign_in_text,
:home_page_url
:home_page_url,
restricted_visibility_levels: []
)
end
end
16 changes: 16 additions & 0 deletions app/helpers/application_settings_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,20 @@ def signin_enabled?
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
5 changes: 3 additions & 2 deletions app/helpers/visibility_level_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ def visibility_level_label(level)
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
36 changes: 25 additions & 11 deletions app/models/application_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -34,6 +47,7 @@ def self.create_from_defaults
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

Expand Down
6 changes: 4 additions & 2 deletions app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

class Project < ActiveRecord::Base
include Sortable
include Gitlab::CurrentSettings
extend Gitlab::CurrentSettings
include Gitlab::ShellAdapter
include Gitlab::VisibilityLevel
include Gitlab::ConfigHelper
Expand Down Expand Up @@ -132,8 +134,8 @@ def set_last_activity_at
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
Expand Down
4 changes: 0 additions & 4 deletions app/services/base_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ def system_hook_service
SystemHooksService.new
end

def current_application_settings
ApplicationSetting.current
end

private

def error(message, http_status = nil)
Expand Down
22 changes: 22 additions & 0 deletions app/services/update_snippet_service.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions app/views/admin/application_settings/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions config/gitlab.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddRestrictedVisibilityLevelsToApplicationSettings < ActiveRecord::Migration
def change
add_column :application_settings, :restricted_visibility_levels, :text
end
end
7 changes: 4 additions & 3 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -25,8 +25,9 @@
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|
Expand Down
4 changes: 2 additions & 2 deletions lib/gitlab/current_settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ def current_application_settings

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
Expand All @@ -21,6 +20,7 @@ def fake_application_settings
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
Expand Down
20 changes: 11 additions & 9 deletions lib/gitlab/visibility_level.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -23,21 +25,21 @@ def options
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

Expand Down
24 changes: 13 additions & 11 deletions spec/models/application_setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit cacac14

Please sign in to comment.