Commit 2110247f authored by Yorick Peterse's avatar Yorick Peterse

Refactoed GroupsFinder into two separate classes

In the previous setup the GroupsFinder class had two distinct tasks:

1. Finding the projects user A could see
2. Finding the projects of user A that user B could see

Task two was actually handled outside of the GroupsFinder (in the
UsersController) by restricting the returned list of groups to those the
viewed user was a member of. Moving all this logic into a single finder
proved to be far too complex and confusing, hence there are now two
finders:

* GroupsFinder: for finding groups a user can see
* JoinedGroupsFinder: for finding groups that user A is a member of,
  restricted to either public groups or groups user B can also see.
parent b4646391
class GroupsFinder
def execute(current_user, options = {})
all_groups(current_user)
# Finds the groups available to the given user.
#
# current_user - The user to find the groups for.
#
# Returns an ActiveRecord::Relation.
def execute(current_user = nil)
if current_user
relation = groups_visible_to_user(current_user)
else
relation = public_groups
end
relation.order_id_desc
end
private
def all_groups(current_user)
group_ids = if current_user
if current_user.authorized_groups.any?
# User has access to groups
#
# Return only:
# groups with public projects
# groups with internal projects
# groups with joined projects
#
Project.public_and_internal_only.pluck(:namespace_id) +
current_user.authorized_groups.pluck(:id)
else
# User has no group membership
#
# Return only:
# groups with public projects
# groups with internal projects
#
Project.public_and_internal_only.pluck(:namespace_id)
# This method returns the groups "current_user" can see.
def groups_visible_to_user(current_user)
base = groups_for_projects(public_and_internal_projects)
union = Gitlab::SQL::Union.
new([base.select(:id), current_user.authorized_groups.select(:id)])
Group.where("namespaces.id IN (#{union.to_sql})")
end
else
# Not authenticated
#
# Return only:
# groups with public projects
Project.public_only.pluck(:namespace_id)
def public_groups
groups_for_projects(public_projects)
end
def groups_for_projects(projects)
Group.public_and_given_groups(projects.select(:namespace_id))
end
def public_projects
Project.unscoped.public_only
end
Group.where("public IS TRUE OR id IN(?)", group_ids)
def public_and_internal_projects
Project.unscoped.public_and_internal_only
end
end
# Class for finding the groups a user is a member of.
class JoinedGroupsFinder
def initialize(user = nil)
@user = user
end
# Finds the groups of the source user, optionally limited to those visible to
# the current user.
#
# current_user - If given the groups of "@user" will only include the groups
# "current_user" can also see.
#
# Returns an ActiveRecord::Relation.
def execute(current_user = nil)
if current_user
relation = groups_visible_to_user(current_user)
else
relation = public_groups
end
relation.order_id_desc
end
private
# Returns the groups the user in "current_user" can see.
#
# This list includes all public/internal projects as well as the projects of
# "@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
union = Gitlab::SQL::Union.new([base.select(:id), extra.select(:id)])
Group.where("namespaces.id IN (#{union.to_sql})")
end
def public_groups
groups_for_projects(@user.authorized_projects.public_only)
end
def public_and_internal_groups
groups_for_projects(@user.authorized_projects.public_and_internal_only)
end
def groups_for_projects(projects)
@user.groups.public_and_given_groups(projects.select(:namespace_id))
end
end
require 'spec_helper'
describe GroupsFinder do
let(:user) { create :user }
let!(:group) { create :group }
let!(:public_group) { create :group, public: true }
describe :execute do
it 'finds public group' do
groups = GroupsFinder.new.execute(user)
expect(groups.size).to eq(1)
expect(groups.first).to eq(public_group)
describe '#execute' do
let(:user) { create(:user) }
let(:group1) { create(:group) }
let(:group2) { create(:group) }
let(:group3) { create(:group) }
let(:group4) { create(:group, public: true) }
let!(:public_project) { create(:project, :public, group: group1) }
let!(:internal_project) { create(:project, :internal, group: group2) }
let!(:private_project) { create(:project, :private, group: group3) }
let(:finder) { described_class.new }
describe 'with a user' do
subject { finder.execute(user) }
describe 'when the user is not a member of any groups' do
it { is_expected.to eq([group4, group2, group1]) }
end
describe 'when the user is a member of a group' do
before do
group3.add_user(user, Gitlab::Access::DEVELOPER)
end
it { is_expected.to eq([group4, group3, group2, group1]) }
end
describe 'when the user is a member of a private project' do
before do
private_project.team.add_user(user, Gitlab::Access::DEVELOPER)
end
it { is_expected.to eq([group4, group3, group2, group1]) }
end
end
describe 'without a user' do
subject { finder.execute }
it { is_expected.to eq([group4, group1]) }
end
end
end
require 'spec_helper'
describe JoinedGroupsFinder do
describe '#execute' do
let(:source_user) { create(:user) }
let(:current_user) { create(:user) }
let(:group1) { create(:group) }
let(:group2) { create(:group) }
let(:group3) { create(:group) }
let(:group4) { create(:group, public: true) }
let!(:public_project) { create(:project, :public, group: group1) }
let!(:internal_project) { create(:project, :internal, group: group2) }
let!(:private_project) { create(:project, :private, group: group3) }
let(:finder) { described_class.new(source_user) }
before do
[group1, group2, group3, group4].each do |group|
group.add_user(source_user, Gitlab::Access::MASTER)
end
end
describe 'with a current user' do
describe 'when the current user has access to the projects of the source user' do
before do
private_project.team.add_user(current_user, Gitlab::Access::DEVELOPER)
end
subject { finder.execute(current_user) }
it { is_expected.to eq([group4, group3, group2, group1]) }
end
describe 'when the current user does not have access to the projects of the source user' do
subject { finder.execute(current_user) }
it { is_expected.to eq([group4, group2, group1]) }
end
end
describe 'without a current user' do
subject { finder.execute }
it { is_expected.to eq([group4, group1]) }
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment