From 5551ccd7201ea6b45a2e2721502ba55e8f525d8f Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Wed, 2 Mar 2016 19:13:50 -0300 Subject: [PATCH] Code improvements --- app/controllers/groups_controller.rb | 4 ++-- app/controllers/users_controller.rb | 10 ++++++++++ app/finders/groups_finder.rb | 13 ++----------- app/helpers/groups_helper.rb | 2 +- app/models/ability.rb | 18 +++++++++++------- app/models/concerns/shared_scopes.rb | 8 ++++++++ app/models/group.rb | 10 +++++----- app/models/project.rb | 3 +-- ...301124843_add_visibility_level_to_groups.rb | 5 +---- db/schema.rb | 2 +- 10 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 app/models/concerns/shared_scopes.rb diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 13de19bc141..6532eee1602 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -9,7 +9,7 @@ class GroupsController < Groups::ApplicationController before_action :group, except: [:index, :new, :create] # Authorize - before_action :authorize_read_group!, except: [:index, :show, :new, :create, :autocomplete] + before_action :authorize_read_group!, except: [:index, :new, :create] before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects] before_action :authorize_create_group!, only: [:new, :create] @@ -105,7 +105,7 @@ class GroupsController < Groups::ApplicationController # Dont allow unauthorized access to group def authorize_read_group! - unless @group and (@projects.present? or can?(current_user, :read_group, @group)) + unless can?(current_user, :read_group, @group) if current_user.nil? return authenticate_user! else diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e10c633690f..d26a1ce6737 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,6 +3,16 @@ class UsersController < ApplicationController before_action :set_user def show +<<<<<<< HEAD +======= + @contributed_projects = contributed_projects.joined(@user).reject(&:forked?) + + @projects = PersonalProjectsFinder.new(@user).execute(current_user) + @projects = @projects.page(params[:page]).per(PER_PAGE) + + @groups = @user.groups.order_id_desc + +>>>>>>> Code improvements respond_to do |format| format.html diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb index a3a8cd541de..ce62f5e762f 100644 --- a/app/finders/groups_finder.rb +++ b/app/finders/groups_finder.rb @@ -1,6 +1,5 @@ class GroupsFinder def execute(current_user = nil) - segments = all_groups(current_user) if segments.length > 1 @@ -15,17 +14,9 @@ class GroupsFinder def all_groups(current_user) if current_user - [current_user.authorized_groups, public_and_internal_groups] + [current_user.authorized_groups, Group.unscoped.public_and_internal_only] else - [Group.public_only] + [Group.unscoped.public_only] end end - - def public_groups - Group.unscoped.public_only - end - - def public_and_internal_groups - Group.unscoped.public_and_internal_only - end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index b1f0a765bb9..d918d8acd22 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -28,7 +28,7 @@ module GroupsHelper group = Group.find_by(path: group) end - if group && group.avatar.present? + if group && can?(current_user, :read_group, group) && group.avatar.present? group.avatar.url else 'no_group_avatar.png' diff --git a/app/models/ability.rb b/app/models/ability.rb index c84ded61606..ec5587d8fa5 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -83,7 +83,7 @@ class Ability subject.group end - if group && group.projects.public_only.any? + if group && group.public? [:read_group] else [] @@ -271,16 +271,13 @@ class Ability def group_abilities(user, group) rules = [] - if user.admin? || group.users.include?(user) || ProjectsFinder.new.execute(user, group: group).any? - rules << :read_group - end + rules << :read_group if can_read_group?(user, group) # Only group masters and group owners can create new projects and change permission level if group.has_master?(user) || group.has_owner?(user) || user.admin? rules += [ :create_projects, - :admin_milestones, - :change_visibility_level + :admin_milestones ] end @@ -289,13 +286,20 @@ class Ability rules += [ :admin_group, :admin_namespace, - :admin_group_member + :admin_group_member, + :change_visibility_level ] end rules.flatten end + def can_read_group?(user, group) + is_project_member = ProjectsFinder.new.execute(user, group: group).any? + internal_group_allowed = group.internal? && user.present? + user.admin? || group.users.include?(user) || is_project_member || group.public? || internal_group_allowed + end + def namespace_abilities(user, namespace) rules = [] diff --git a/app/models/concerns/shared_scopes.rb b/app/models/concerns/shared_scopes.rb new file mode 100644 index 00000000000..f576d2c0821 --- /dev/null +++ b/app/models/concerns/shared_scopes.rb @@ -0,0 +1,8 @@ +module SharedScopes + extend ActiveSupport::Concern + + included do + scope :public_only, -> { where(visibility_level: Group::PUBLIC) } + scope :public_and_internal_only, -> { where(visibility_level: [Group::PUBLIC, Group::INTERNAL] ) } + end +end diff --git a/app/models/group.rb b/app/models/group.rb index 26914f55541..d1a1817f0fa 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -21,7 +21,7 @@ class Group < Namespace include Gitlab::ConfigHelper include Gitlab::VisibilityLevel include Referable - + include SharedScopes has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' alias_method :members, :group_members @@ -35,10 +35,6 @@ class Group < Namespace after_create :post_create_hook after_destroy :post_destroy_hook - scope :public_only, -> { where(visibility_level: Group::PUBLIC) } - scope :public_and_internal_only, -> { where(visibility_level: [Group::PUBLIC, Group::INTERNAL] ) } - - class << self def search(query) where("LOWER(namespaces.name) LIKE :query or LOWER(namespaces.path) LIKE :query", query: "%#{query.downcase}%") @@ -69,6 +65,10 @@ class Group < Namespace name end + def visibility_level_field + visibility_level + end + def avatar_url(size = nil) if avatar.present? [gitlab_config.url, avatar.url].join diff --git a/app/models/project.rb b/app/models/project.rb index 426464dee81..6e86c7cc883 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -52,6 +52,7 @@ class Project < ActiveRecord::Base include AfterCommitQueue include CaseSensitivity include TokenAuthenticatable + include SharedScopes extend Gitlab::ConfigHelper @@ -213,8 +214,6 @@ class Project < ActiveRecord::Base scope :in_group_namespace, -> { joins(:group) } scope :personal, ->(user) { where(namespace_id: user.namespace_id) } scope :joined, ->(user) { where('namespace_id != ?', user.namespace_id) } - scope :public_only, -> { where(visibility_level: Project::PUBLIC) } - scope :public_and_internal_only, -> { where(visibility_level: Project.public_and_internal_levels) } scope :non_archived, -> { where(archived: false) } scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct } diff --git a/db/migrate/20160301124843_add_visibility_level_to_groups.rb b/db/migrate/20160301124843_add_visibility_level_to_groups.rb index b0afe795f42..86dc07f4333 100644 --- a/db/migrate/20160301124843_add_visibility_level_to_groups.rb +++ b/db/migrate/20160301124843_add_visibility_level_to_groups.rb @@ -1,9 +1,6 @@ class AddVisibilityLevelToGroups < ActiveRecord::Migration def change #All groups will be private when created - add_column :namespaces, :visibility_level, :integer, null: false, default: 0 - - #Set all existing groups to public - Group.update_all(visibility_level: 20) + add_column :namespaces, :visibility_level, :integer, null: false, default: 20 end end diff --git a/db/schema.rb b/db/schema.rb index d0e3bb769d2..d2f311e42da 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: 20160305220806) do +ActiveRecord::Schema.define(version: 20160301124843) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" -- GitLab