Tweaks, refactoring, and specs

8 files changed, 94 insertions, 157 deletions

diff --git a/app/finders/contributed_projects_finder.rb b/app/finders/contributed_projects_finder.rb
index f8b04dfa2aa..a685719555c 100644
--- a/app/finders/contributed_projects_finder.rb
+++ b/app/finders/contributed_projects_finder.rb
@@ -1,4 +1,4 @@
-class ContributedProjectsFinder
+class ContributedProjectsFinder < UnionFinder
   def initialize(user)
     @user = user
   end
@@ -10,27 +10,20 @@ class ContributedProjectsFinder
   #                visible by this user.
   #
   # Returns an ActiveRecord::Relation.
-
   def execute(current_user = nil)
-    if current_user
-      relation = projects_visible_to_user(current_user)
-    else
-      relation = public_projects
-    end
+    segments = all_projects(current_user)
 
-    relation.includes(:namespace).order_id_desc
+    find_union(segments, Project).includes(:namespace).order_id_desc
   end
 
   private
 
-  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)])
+  def all_projects(current_user)
+    projects = []
 
-    Project.where("projects.id IN (#{union.to_sql})")
-  end
+    projects << @user.contributed_projects.visible_to_user(current_user) if current_user
+    projects << @user.contributed_projects.public_to_user(current_user)
 
-  def public_projects
-    @user.contributed_projects.public_only
+    projects
   end
 end
diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb
new file mode 100644
index 00000000000..84fe468ae5d
--- /dev/null
+++ b/app/finders/group_projects_finder.rb
@@ -0,0 +1,43 @@
+class GroupProjectsFinder < UnionFinder
+  def initialize(group, options = {})
+    @group = group
+    @options = options
+  end
+
+  def execute(current_user = nil)
+    segments = group_projects(current_user)
+
+    find_union(segments, Project)
+  end
+
+  private
+
+  def group_projects(current_user)
+    include_owned = @options.fetch(:owned, true)
+    include_shared = @options.fetch(:shared, true)
+
+    projects = []
+
+    if current_user
+      if @group.users.include?(current_user)
+        projects << @group.projects if include_owned
+        projects << @group.shared_projects if include_shared
+      else
+        if include_owned
+          projects << @group.projects.visible_to_user(current_user)
+          projects << @group.projects.public_to_user(current_user)
+        end
+
+        if include_shared
+          projects << @group.shared_projects.visible_to_user(current_user)
+          projects << @group.shared_projects.public_to_user(current_user)
+        end
+      end
+    else
+      projects << @group.projects.public_only if include_owned
+      projects << @group.shared_projects.public_only if include_shared
+    end
+
+    projects
+  end
+end
diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index 30698f80231..4e43f42e9e1 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -1,30 +1,18 @@
-class GroupsFinder
+class GroupsFinder < UnionFinder
   def execute(current_user = nil)
     segments = all_groups(current_user)
 
-    if segments.length > 1
-      union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) })
-      Group.where("namespaces.id IN (#{union.to_sql})").order_id_desc
-    else
-      segments.first
-    end
+    find_union(segments, Group).order_id_desc
   end
 
   private
 
   def all_groups(current_user)
-    if current_user
-      user_groups(current_user)
-    else
-      [Group.unscoped.public_only]
-    end
-  end
+    groups = []
+
+    groups << current_user.authorized_groups if current_user
+    groups << Group.unscoped.public_to_user(current_user)
 
-  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
+    groups
   end
 end
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 19e8c7a92be..7510f6e9e29 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -81,7 +81,7 @@ class IssuableFinder
     elsif current_user && params[:authorized_only].presence && !current_user_related?
       @projects = current_user.authorized_projects.reorder(nil)
     else
-      @projects = ProjectsFinder.new.execute(current_user, group: group).
+      @projects = GroupProjectsFinder.new(group).execute(current_user).
         reorder(nil)
     end
   end
@@ -170,7 +170,7 @@ class IssuableFinder
   end
 
   def by_scope(items)
-    case params[:scope]
+    case params[:scope] || 'all'
     when 'created-by-me', 'authored' then
       items.where(author_id: current_user.id)
     when 'all' then
diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb
index 867eb661682..2a3f0296d37 100644
--- a/app/finders/joined_groups_finder.rb
+++ b/app/finders/joined_groups_finder.rb
@@ -1,5 +1,4 @@
-#Shows only authorized groups of a user
-class JoinedGroupsFinder
+class JoinedGroupsFinder < UnionFinder
   def initialize(user)
     @user = user
   end
@@ -12,34 +11,19 @@ class JoinedGroupsFinder
   #
   # Returns an ActiveRecord::Relation.
   def execute(current_user = nil)
-    if current_user
-      relation = groups_visible_to_user(current_user)
-    else
-      relation = public_groups
-    end
+    segments = all_groups(current_user)
 
-    relation.order_id_desc
+    find_union(segments, Group).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 = 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})")
-  end
+  def all_groups(current_user)
+    groups = []
 
-  def public_groups
-    @user.authorized_groups.public_only
-  end
+    groups << @user.authorized_groups.visible_to_user(current_user) if current_user
+    groups << @user.authorized_groups.public_to_user(current_user)
 
-  def public_and_internal_groups
-    @user.authorized_groups.public_and_internal_only
+    groups
   end
 end
diff --git a/app/finders/personal_projects_finder.rb b/app/finders/personal_projects_finder.rb
index 34f33e2353b..3ad4bd5f066 100644
--- a/app/finders/personal_projects_finder.rb
+++ b/app/finders/personal_projects_finder.rb
@@ -1,4 +1,4 @@
-class PersonalProjectsFinder
+class PersonalProjectsFinder < UnionFinder
   def initialize(user)
     @user = user
   end
@@ -11,38 +11,19 @@ class PersonalProjectsFinder
   #
   # Returns an ActiveRecord::Relation.
   def execute(current_user = nil)
-    if current_user
-      relation = projects_visible_to_user(current_user)
-    else
-      relation = public_projects
-    end
+    segments = all_projects(current_user)
 
-    relation.includes(:namespace).order_id_desc
+    find_union(segments, Project).includes(:namespace).order_id_desc
   end
 
   private
 
-  def projects_visible_to_user(current_user)
-    union = Gitlab::SQL::Union.new(projects_for_user_ids(current_user))
+  def all_projects(current_user)
+    projects = []
 
-    Project.where("projects.id IN (#{union.to_sql})")
-  end
-
-  def public_projects
-    @user.personal_projects.public_only
-  end
-
-  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)
+    projects << @user.personal_projects.visible_to_user(current_user) if current_user
+    projects << @user.personal_projects.public_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
+    projects
   end
 end
diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index 3a5fc5b5907..2f0a9659d15 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -1,81 +1,18 @@
-class ProjectsFinder
-  # Returns all projects, optionally including group projects a user has access
-  # to.
-  #
-  # ## Examples
-  #
-  # Retrieving all public projects:
-  #
-  #     ProjectsFinder.new.execute
-  #
-  # Retrieving all public/internal projects and those the given user has access
-  # to:
-  #
-  #     ProjectsFinder.new.execute(some_user)
-  #
-  # Retrieving all public/internal projects as well as the group's projects the
-  # user has access to:
-  #
-  #     ProjectsFinder.new.execute(some_user, group: some_group)
-  #
-  # Returns an ActiveRecord::Relation.
+class ProjectsFinder < UnionFinder
   def execute(current_user = nil, options = {})
-    group = options[:group]
+    segments = all_projects(current_user)
 
-    if group
-      segments = group_projects(current_user, group)
-    else
-      segments = all_projects(current_user)
-    end
-
-    if segments.length > 1
-      union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) })
-
-      Project.where("projects.id IN (#{union.to_sql})")
-    else
-      segments.first
-    end
+    find_union(segments, Project)
   end
 
   private
 
-  def group_projects(current_user, group)
-    return [group.projects.public_only] unless current_user
-
-    user_group_projects = [
-       group_projects_for_user(current_user, group),
-       group.shared_projects.visible_to_user(current_user)
-    ]
-    if current_user.external?
-      user_group_projects << group.projects.public_only
-    else
-      user_group_projects << group.projects.public_and_internal_only
-    end
-  end
-
   def all_projects(current_user)
-    return [public_projects] unless current_user
+    projects = []
 
-    if current_user.external?
-      [current_user.authorized_projects, public_projects]
-    else
-      [current_user.authorized_projects, public_and_internal_projects]
-    end
-  end
-
-  def group_projects_for_user(current_user, group)
-    if group.users.include?(current_user)
-      group.projects
-    else
-      group.projects.visible_to_user(current_user)
-    end
-  end
-
-  def public_projects
-    Project.unscoped.public_only
-  end
+    projects << current_user.authorized_projects if current_user
+    projects << Project.unscoped.public_to_user(current_user)
 
-  def public_and_internal_projects
-    Project.unscoped.public_and_internal_only
+    projects
   end
 end
diff --git a/app/finders/union_finder.rb b/app/finders/union_finder.rb
new file mode 100644
index 00000000000..33cd1a491f3
--- /dev/null
+++ b/app/finders/union_finder.rb
@@ -0,0 +1,11 @@
+class UnionFinder
+  def find_union(segments, klass)
+    if segments.length > 1
+      union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) })
+
+      klass.where("#{klass.table_name}.id IN (#{union.to_sql})")
+    else
+      segments.first
+    end
+  end
+end