Refactor User#authorized_groups/projects

These methods no longer include public groups/projects (that don't
belong to the actual user) as this is handled by the various finder
classes now. This also removes the need for passing extra arguments.

Note that memoizing was removed _explicitly_. For whatever reason doing
so messes up the users controller to a point where it claims a certain
user does _not_ have access to certain groups/projects when it does have
access. Existing code shouldn't be affected as these methods are only
called in ways that they'd run queries anyway (e.g. a combination of
"any?" and "each" which would run 2 queries regardless of memoizing).
This commit is contained in:
Yorick Peterse 2015-11-18 12:30:24 +01:00
parent a4fc8112df
commit e116a356b8
2 changed files with 10 additions and 51 deletions

View File

@ -389,40 +389,17 @@ class User < ActiveRecord::Base
end end
end end
# Returns the groups a user has access to, optionally including any public # Returns the groups a user has access to
# groups. def authorized_groups
#
# public_internal - When set to "true" all public groups and groups of public
# projects are also included.
#
# Returns an ActiveRecord::Relation
def authorized_groups(public_internal = false)
union = Gitlab::SQL::Union. union = Gitlab::SQL::Union.
new([groups.select(:id), authorized_projects(public_internal). new([groups.select(:id), authorized_projects.select(:namespace_id)])
select(:namespace_id)])
sql = "namespaces.id IN (#{union.to_sql})" Group.where("namespaces.id IN (#{union.to_sql})")
if public_internal
sql << ' OR public IS TRUE'
end
Group.where(sql)
end end
# Returns the groups a user is authorized to access. # Returns the groups a user is authorized to access.
# def authorized_projects
# public_internal - When set to "true" all public/internal projects will also Project.where("projects.id IN (#{projects_union.to_sql})")
# be included.
def authorized_projects(public_internal = false)
base = "projects.id IN (#{projects_union.to_sql})"
if public_internal
Project.where("#{base} OR projects.visibility_level IN (?)",
Project.public_and_internal_levels)
else
Project.where(base)
end
end end
def owned_projects def owned_projects

View File

@ -762,44 +762,26 @@ describe User do
describe '#authorized_groups' do describe '#authorized_groups' do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:private_group) { create(:group) } let!(:private_group) { create(:group) }
let!(:public_group) { create(:group, public: true) }
before do before do
private_group.add_user(user, Gitlab::Access::MASTER) private_group.add_user(user, Gitlab::Access::MASTER)
end end
describe 'excluding public groups' do subject { user.authorized_groups }
subject { user.authorized_groups }
it { is_expected.to eq([private_group]) } it { is_expected.to eq([private_group]) }
end
describe 'including public groups' do
subject { user.authorized_groups(true) }
it { is_expected.to eq([public_group, private_group]) }
end
end end
describe '#authorized_projects' do describe '#authorized_projects' do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:private_project) { create(:project, :private) } let!(:private_project) { create(:project, :private) }
let!(:public_project) { create(:project, :public) }
before do before do
private_project.team << [user, Gitlab::Access::MASTER] private_project.team << [user, Gitlab::Access::MASTER]
end end
describe 'excluding public projects' do subject { user.authorized_projects }
subject { user.authorized_projects }
it { is_expected.to eq([private_project]) } it { is_expected.to eq([private_project]) }
end
describe 'including public projects' do
subject { user.authorized_projects(true) }
it { is_expected.to eq([public_project, private_project]) }
end
end end
end end