From 5551ccd7201ea6b45a2e2721502ba55e8f525d8f Mon Sep 17 00:00:00 2001
From: Felipe Artur <felipefac@gmail.com>
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