diff options
| -rw-r--r-- | app/assets/stylesheets/framework/common.scss | 46 | ||||
| -rw-r--r-- | app/assets/stylesheets/framework/mobile.scss | 2 | ||||
| -rw-r--r-- | app/assets/stylesheets/pages/projects.scss | 22 | ||||
| -rw-r--r-- | app/controllers/groups_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/users_controller.rb | 9 | ||||
| -rw-r--r-- | app/finders/joined_groups_finder.rb | 2 | ||||
| -rw-r--r-- | app/models/ability.rb | 4 | ||||
| -rw-r--r-- | app/services/groups/base_service.rb | 4 | ||||
| -rw-r--r-- | app/services/groups/create_service.rb | 17 | ||||
| -rw-r--r-- | app/services/groups/update_service.rb | 13 | ||||
| -rw-r--r-- | app/services/projects/create_service.rb | 6 | ||||
| -rw-r--r-- | app/views/groups/show.html.haml | 4 | ||||
| -rw-r--r-- | app/views/projects/_home_panel.html.haml | 2 | ||||
| -rw-r--r-- | db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb | 20 | ||||
| -rw-r--r-- | db/schema.rb | 2 | ||||
| -rw-r--r-- | spec/features/security/group/internal_access_spec.rb | 2 | ||||
| -rw-r--r-- | spec/services/groups/create_service_spec.rb | 22 |
17 files changed, 97 insertions, 84 deletions
diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 18044b25b87..0931090b840 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -383,35 +383,25 @@ table { margin-right: -$gl-padding; border-top: 1px solid $border-color; } -.message { - border: 1px solid #ccc; - padding: 10px; - color: #333; -} -.message { - border: 1px solid #ccc; - padding: 10px; - color: #333; -} -.group-projects-show-title{ - h1 { - color: #313236; - margin: 0; - margin-bottom: 6px; - font-size: 23px; - font-weight: normal; - } +.cover-title{ + h1 { + color: #313236; + margin: 0; + margin-bottom: 6px; + font-size: 23px; + font-weight: normal; + } - .visibility-icon { - display: inline-block; - margin-left: 5px; - font-size: 18px; - color: $gray; - } + .visibility-icon { + display: inline-block; + margin-left: 5px; + font-size: 18px; + color: $gray; + } - p { - padding: 0 $gl-padding; - color: #5c5d5e; - } + p { + padding: 0 $gl-padding; + color: #5c5d5e; } +} diff --git a/app/assets/stylesheets/framework/mobile.scss b/app/assets/stylesheets/framework/mobile.scss index 3bfac2ad9b5..d088228fe4c 100644 --- a/app/assets/stylesheets/framework/mobile.scss +++ b/app/assets/stylesheets/framework/mobile.scss @@ -48,7 +48,7 @@ display: block; } - .project-home-desc { + #project-home-desc { font-size: 21px; } diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index b1b76edfb32..1a7b0c1e278 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -61,28 +61,6 @@ } } - .project-home-desc { - h1 { - color: #313236; - margin: 0; - margin-bottom: 6px; - font-size: 23px; - font-weight: normal; - } - - .visibility-icon { - display: inline-block; - margin-left: 5px; - font-size: 18px; - color: $gray; - } - - p { - padding: 0 $gl-padding; - color: #5c5d5e; - } - } - .project-repo-buttons { margin-top: 20px; margin-bottom: 0px; diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 54f14e62ead..5baeb3def08 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -29,10 +29,8 @@ class GroupsController < Groups::ApplicationController def create @group = Group.new(group_params) - @group.name = @group.path.dup unless @group.name - if @group.save - @group.add_owner(current_user) + if Groups::CreateService.new(@group, current_user, group_params).execute redirect_to @group, notice: "Group '#{@group.name}' was successfully created." else render action: "new" diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7b32572f822..481d00d6aae 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,13 +3,6 @@ class UsersController < ApplicationController before_action :set_user def show - @contributed_projects = contributed_projects.joined(@user).reject(&:forked?) - - @projects = PersonalProjectsFinder.new(@user).execute(current_user) - @projects = @projects.page(params[:page]).per(PER_PAGE) - - @groups = JoinedGroupsFinder.new(@user).execute(current_user) - respond_to do |format| format.html @@ -115,7 +108,7 @@ class UsersController < ApplicationController end def load_groups - @groups = @user.groups.order_id_desc + @groups = JoinedGroupsFinder.new(@user).execute(current_user) end def projects_for_current_user diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb index 131b518563e..fbdf492c965 100644 --- a/app/finders/joined_groups_finder.rb +++ b/app/finders/joined_groups_finder.rb @@ -1,6 +1,6 @@ #Shows only authorized groups of a user class JoinedGroupsFinder - def initialize(user = nil) + def initialize(user) @user = user end diff --git a/app/models/ability.rb b/app/models/ability.rb index fe460ccdaca..bd001ef1545 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -295,8 +295,8 @@ class Ability end def can_read_group?(user, group) - is_project_member = ProjectsFinder.new.execute(user, group: group).any? - user.admin? || group.public? || group.internal? || is_project_member || group.users.include?(user) + user.admin? || group.public? || group.internal? || group.users.include?(user) || + ProjectsFinder.new.execute(user, group: group).any? end def namespace_abilities(user, namespace) diff --git a/app/services/groups/base_service.rb b/app/services/groups/base_service.rb index 5becd475d3a..644ec7c013e 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -5,5 +5,9 @@ module Groups def initialize(group, user, params = {}) @group, @current_user, @params = group, user, params.dup end + + def add_error_message(message) + group.errors.add(:visibility_level, message) + end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb new file mode 100644 index 00000000000..e2875aafb94 --- /dev/null +++ b/app/services/groups/create_service.rb @@ -0,0 +1,17 @@ +module Groups + class CreateService < Groups::BaseService + def execute + return false unless visibility_level_allowed?(params[:visibility_level]) + @group.name = @group.path.dup unless @group.name + @group.save(params) && @group.add_owner(current_user) + end + + private + + def visibility_level_allowed?(level) + allowed = Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) + add_error_message("Visibility level restricted by admin.") unless allowed + allowed + end + end +end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index acb6c529c17..a7382c1e07c 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -5,7 +5,8 @@ module Groups class UpdateService < Groups::BaseService def execute - visibility_level_allowed?(params[:visibility_level]) ? group.update_attributes(params) : false + return false unless visibility_level_allowed?(params[:visibility_level]) + group.update_attributes(params) end private @@ -22,7 +23,7 @@ module Groups def visibility_by_project(level) projects_visibility = group.projects.pluck(:visibility_level) - allowed_by_projects = !projects_visibility.any?{|project_visibility| level.to_i < project_visibility } + allowed_by_projects = !projects_visibility.any?{ |project_visibility| level.to_i < project_visibility } add_error_message("Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects allowed_by_projects end @@ -32,13 +33,5 @@ module Groups add_error_message("You are not authorized to set this permission level.") unless allowed_by_user allowed_by_user end - - def add_error_message(message) - level_name = Gitlab::VisibilityLevel.level_name(params[:visibility_level]) - group.errors.add(:visibility_level, message) - end end end - - - diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 522fae79503..4c121106bda 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -12,7 +12,7 @@ module Projects # Make sure that the user is allowed to use the specified visibility # level - unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) && @project.visibility_level_allowed?(@project.visibility_level) + unless visibility_level_allowed? deny_visibility_level(@project) return @project end @@ -100,5 +100,9 @@ module Projects @project.import_start if @project.import? end + + def visibility_level_allowed? + Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) && @project.visibility_level_allowed?(@project.visibility_level) + end end end diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 0a8c2da7207..641191edf05 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -14,11 +14,11 @@ .avatar-holder = link_to group_icon(@group), target: '_blank' do = image_tag group_icon(@group), class: "avatar group-avatar s90" - .group-projects-show-title + .cover-title %h1 = @group.name - %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{project_visibility_level_description(@group.visibility_level)}"} + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{group_visibility_description(@group)}"} = visibility_level_icon(@group.visibility_level, fw: false) .cover-desc.username diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index dd16f504c92..e8434b5292c 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -2,7 +2,7 @@ .project-home-panel.cover-block.clearfix{:class => ("empty-project" if empty_repo)} .project-identicon-holder = project_icon(@project, alt: '', class: 'project-avatar avatar s90') - .group-projects-show-title + .cover-title#project-home-desc %h1 = @project.name %span.visibility-icon.has_tooltip{data: { container: 'body' }, diff --git a/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb b/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb index c2bdd7b31fa..b71322376fa 100644 --- a/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb +++ b/db/migrate/20160308212903_add_default_group_visibility_to_application_settings.rb @@ -1,11 +1,27 @@ +#Create visibility level field on DB +#Sets default_visibility_level to value on settings if not restricted +#If value is restricted takes higher visibility level allowed + class AddDefaultGroupVisibilityToApplicationSettings < ActiveRecord::Migration def up add_column :application_settings, :default_group_visibility, :integer - visibility = Settings.gitlab.default_groups_features['visibility_level'] - execute("update application_settings set default_group_visibility = #{visibility}") + execute("update application_settings set default_group_visibility = #{allowed_visibility_level}") end def down remove_column :application_settings, :default_group_visibility end + + private + def allowed_visibility_level + default_visibility = Settings.gitlab.default_groups_features['visibility_level'] + restricted_levels = current_application_settings.restricted_visibility_levels + return default_visibility unless restricted_levels.present? + + if restricted_levels.include?(default_visibility) + Gitlab::VisibilityLevel.values.select{ |vis_level| vis_level unless restricted_levels.include?(vis_level) }.last + else + default_visibility + end + end end diff --git a/db/schema.rb b/db/schema.rb index 082d681a176..292a9100d9c 100644 --- a/db/schema.rb +++ b/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: 20160308212903) do +ActiveRecord::Schema.define(version: 20160309140734) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/spec/features/security/group/internal_access_spec.rb b/spec/features/security/group/internal_access_spec.rb index 69a0fbb4468..4e781c23ee0 100644 --- a/spec/features/security/group/internal_access_spec.rb +++ b/spec/features/security/group/internal_access_spec.rb @@ -4,8 +4,6 @@ describe 'Internal group access', feature: true do include AccessMatchers include GroupAccessHelper - - describe 'GET /groups/:path' do subject { group_path(group(Gitlab::VisibilityLevel::INTERNAL)) } diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb new file mode 100644 index 00000000000..7dbc5297978 --- /dev/null +++ b/spec/services/groups/create_service_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Groups::CreateService, services: true do + let!(:user) { create(:user) } + let!(:private_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE) } + let!(:internal_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::INTERNAL) } + let!(:public_group) { create(:group, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } + + describe "execute" do + let!(:service) { described_class.new(public_group, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC ) } + subject { service.execute } + + context "create groups without restricted visibility level" do + it { is_expected.to be_truthy } + end + + context "cannot create group with restricted visibility level" do + before { allow(current_application_settings).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } + it { is_expected.to be_falsy } + end + end +end |