diff options
author | Felipe Artur <felipefac@gmail.com> | 2016-03-18 01:42:46 +0300 |
---|---|---|
committer | Felipe Artur <felipefac@gmail.com> | 2016-03-18 01:42:46 +0300 |
commit | 0a7f7161198feaa9a4cae7c16669a0e6187aed33 (patch) | |
tree | 445dcbd9f473be2af293a299b634525725a70666 /app | |
parent | a18ac62756573a2da2c42ca50b6f30033be6fa63 (diff) |
Code fixes
Diffstat (limited to 'app')
-rw-r--r-- | app/assets/stylesheets/framework/blocks.scss | 18 | ||||
-rw-r--r-- | app/assets/stylesheets/framework/common.scss | 22 | ||||
-rw-r--r-- | app/controllers/groups_controller.rb | 4 | ||||
-rw-r--r-- | app/finders/contributed_projects_finder.rb | 7 | ||||
-rw-r--r-- | app/finders/groups_finder.rb | 10 | ||||
-rw-r--r-- | app/finders/joined_groups_finder.rb | 4 | ||||
-rw-r--r-- | app/finders/personal_projects_finder.rb | 17 | ||||
-rw-r--r-- | app/models/ability.rb | 7 | ||||
-rw-r--r-- | app/services/groups/base_service.rb | 16 | ||||
-rw-r--r-- | app/services/groups/create_service.rb | 19 | ||||
-rw-r--r-- | app/services/groups/update_service.rb | 19 | ||||
-rw-r--r-- | app/views/groups/show.html.haml | 10 |
12 files changed, 73 insertions, 80 deletions
diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index d20b77ffae9..31084872367 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -115,6 +115,24 @@ color: #4c4e54; font-size: 23px; line-height: 1.1; + + h1 { + color: #313236; + margin-bottom: 6px; + font-size: 23px; + } + + .visibility-icon { + display: inline-block; + margin-left: 5px; + font-size: 18px; + color: $gray; + } + + p { + padding: 0 $gl-padding; + color: #5c5d5e; + } } .cover-desc { diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index f4608cd80bb..ff551f151f1 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -385,25 +385,3 @@ table { margin-right: -$gl-padding; border-top: 1px solid $border-color; } - -.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; - } - - p { - padding: 0 $gl-padding; - color: #5c5d5e; - } -} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 8243946c852..ba2057eb2c8 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -28,9 +28,9 @@ class GroupsController < Groups::ApplicationController end def create - @group = Group.new(group_params) + @group = Groups::CreateService.new(current_user, group_params).execute - if Groups::CreateService.new(@group, current_user, group_params).execute + if @group.persisted? redirect_to @group, notice: "Group '#{@group.name}' was successfully created." else render action: "new" diff --git a/app/finders/contributed_projects_finder.rb b/app/finders/contributed_projects_finder.rb index 4f7fe1c748b..f8b04dfa2aa 100644 --- a/app/finders/contributed_projects_finder.rb +++ b/app/finders/contributed_projects_finder.rb @@ -10,8 +10,9 @@ class ContributedProjectsFinder # visible by this user. # # Returns an ActiveRecord::Relation. + def execute(current_user = nil) - if current_user && !current_user.external? + if current_user relation = projects_visible_to_user(current_user) else relation = public_projects @@ -24,9 +25,7 @@ class ContributedProjectsFinder def projects_visible_to_user(current_user) authorized = @user.contributed_projects.visible_to_user(current_user) - - union = Gitlab::SQL::Union. - new([authorized.select(:id), public_projects.select(:id)]) + union = Gitlab::SQL::Union.new([authorized.select(:id), public_projects.select(:id)]) Project.where("projects.id IN (#{union.to_sql})") end diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index ce62f5e762f..30698f80231 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -14,9 +14,17 @@ class GroupsFinder def all_groups(current_user) if current_user - [current_user.authorized_groups, Group.unscoped.public_and_internal_only] + user_groups(current_user) else [Group.unscoped.public_only] end end + + def user_groups(current_user) + if current_user.external? + [current_user.authorized_groups, Group.unscoped.public_only] + else + [current_user.authorized_groups, Group.unscoped.public_and_internal_only] + end + end end diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb index ff744689e3d..867eb661682 100644 --- a/app/finders/joined_groups_finder.rb +++ b/app/finders/joined_groups_finder.rb @@ -12,7 +12,7 @@ class JoinedGroupsFinder # # Returns an ActiveRecord::Relation. def execute(current_user = nil) - if current_user && !current_user.external? + if current_user relation = groups_visible_to_user(current_user) else relation = public_groups @@ -29,7 +29,7 @@ class JoinedGroupsFinder # "@user" that "current_user" also has access to. def groups_visible_to_user(current_user) base = @user.authorized_groups.visible_to_user(current_user) - extra = public_and_internal_groups + extra = current_user.external? ? public_groups : public_and_internal_groups union = Gitlab::SQL::Union.new([base.select(:id), extra.select(:id)]) Group.where("namespaces.id IN (#{union.to_sql})") diff --git a/app/finders/personal_projects_finder.rb b/app/finders/personal_projects_finder.rb index 0e2d915da54..34f33e2353b 100644 --- a/app/finders/personal_projects_finder.rb +++ b/app/finders/personal_projects_finder.rb @@ -11,7 +11,7 @@ class PersonalProjectsFinder # # Returns an ActiveRecord::Relation. def execute(current_user = nil) - if current_user && !current_user.external? + if current_user relation = projects_visible_to_user(current_user) else relation = public_projects @@ -23,10 +23,7 @@ class PersonalProjectsFinder private def projects_visible_to_user(current_user) - authorized = @user.personal_projects.visible_to_user(current_user) - - union = Gitlab::SQL::Union. - new([authorized.select(:id), public_and_internal_projects.select(:id)]) + union = Gitlab::SQL::Union.new(projects_for_user_ids(current_user)) Project.where("projects.id IN (#{union.to_sql})") end @@ -38,4 +35,14 @@ class PersonalProjectsFinder def public_and_internal_projects @user.personal_projects.public_and_internal_only end + + def projects_for_user_ids(current_user) + authorized = @user.personal_projects.visible_to_user(current_user) + + if current_user.external? + [authorized.select(:id), public_projects.select(:id)] + else + [authorized.select(:id), public_and_internal_projects.select(:id)] + end + end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 134ae440c9c..ffcf05dcd33 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -295,11 +295,8 @@ class Ability end def can_read_group?(user, group) - if user.external? - group.public? || ProjectsFinder.new.execute(user, group: group).any? - else - user.admin? || group.public? || group.internal? || group.users.include?(user) || ProjectsFinder.new.execute(user, group: group).any? - end + user.admin? || group.public? || (group.internal? && !user.external?) || 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 644ec7c013e..053b6a05281 100644 --- a/app/services/groups/base_service.rb +++ b/app/services/groups/base_service.rb @@ -6,8 +6,20 @@ module Groups @group, @current_user, @params = group, user, params.dup end - def add_error_message(message) - group.errors.add(:visibility_level, message) + private + + def visibility_allowed_for_user?(level) + allowed_by_user = Gitlab::VisibilityLevel.allowed_for?(current_user, level) + @group.errors.add(:visibility_level, "You are not authorized to set this permission level.") unless allowed_by_user + allowed_by_user + end + + def visibility_allowed_for_project?(level) + projects_visibility = group.projects.pluck(:visibility_level) + + allowed_by_projects = !projects_visibility.any? { |project_visibility| level.to_i < project_visibility } + @group.errors.add(:visibility_level, "Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects + allowed_by_projects end end end diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index e2875aafb94..38742369d82 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -1,17 +1,16 @@ 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) + def initialize(user, params = {}) + @current_user, @params = user, params.dup + @group = Group.new(@params) 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 + def execute + return @group unless visibility_allowed_for_user?(@params[:visibility_level]) + @group.name = @group.path.dup unless @group.name + @group.save + @group.add_owner(@current_user) + @group end end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index a7382c1e07c..b910e0fde98 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -14,24 +14,7 @@ module Groups def visibility_level_allowed?(level) return true unless level.present? - allowed_by_projects = visibility_by_project(level) - allowed_by_user = visibility_by_user(level) - - allowed_by_projects && allowed_by_user - end - - 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 } - add_error_message("Cannot be changed. There are projects with higher visibility permissions.") unless allowed_by_projects - allowed_by_projects - end - - def visibility_by_user(level) - allowed_by_user = Gitlab::VisibilityLevel.allowed_for?(current_user, level) - add_error_message("You are not authorized to set this permission level.") unless allowed_by_user - allowed_by_user + visibility_allowed_for_project?(level) && visibility_allowed_for_user?(level) end end end diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 2653a03059f..4be117667db 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -18,7 +18,7 @@ %h1 = @group.name - %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{visibility_level_label(@group.visibility_level)} - #{group_visibility_description(@group)}"} + %span.visibility-icon.has_tooltip{data: { container: 'body', placement: 'left' }, title: "#{group_visibility_description(@group)}"} = visibility_level_icon(@group.visibility_level, fw: false) .cover-desc.username @@ -28,14 +28,6 @@ .cover-desc.description = markdown(@group.description, pipeline: :description) - %ul.nav-links - %li.active - = link_to "#activity", 'data-toggle' => 'tab' do - Activity - %li - = link_to "#projects", 'data-toggle' => 'tab' do - Projects - - if can?(current_user, :read_group, @group) %div{ class: container_class } .top-area |