Applies the CE backport of EE#657

10 files changed, 50 insertions, 24 deletions

diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 2ccb3896857..92e7d7fdebd 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -103,6 +103,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
     @source_project = @merge_request.source_project
     @commits = set_commits_for_rendering(@merge_request.commits)
     @commit = @merge_request.diff_head_commit
+    @mr_presenter = @merge_request.present(current_user: current_user)
 
     @labels = LabelsFinder.new(current_user, project_id: @project.id).execute
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 75a85fafa3f..dbe710f58e6 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -331,6 +331,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     @target_project = @merge_request.target_project
     @target_branches = @merge_request.target_project.repository.branch_names
     @noteable = @merge_request
+    @mr_presenter = @merge_request.present(current_user: current_user)
   end
 
   def finder_type
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 7352c5e9bec..a9417369ca2 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -16,6 +16,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :tree, only: [:show], if: [:repo_exists?, :project_view_files?]
   before_action :lfs_blob_ids, only: [:show], if: [:repo_exists?, :project_view_files?]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
+  before_action :present_project, only: [:edit]
 
   # Authorize
   before_action :authorize_admin_project!, only: [:edit, :update, :housekeeping, :download_export, :export, :remove_export, :generate_new_export]
@@ -433,4 +434,8 @@ class ProjectsController < Projects::ApplicationController
   def whitelist_query_limiting
     Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42440')
   end
+
+  def present_project
+    @project = @project.present(current_user: current_user)
+  end
 end
diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb
index 18cc6891ca4..4d8128dd824 100644
--- a/app/finders/joined_groups_finder.rb
+++ b/app/finders/joined_groups_finder.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class JoinedGroupsFinder < UnionFinder
+class JoinedGroupsFinder
   def initialize(user)
     @user = user
   end
@@ -8,19 +8,8 @@ class JoinedGroupsFinder < UnionFinder
   # Finds the groups of the source user, optionally limited to those visible to
   # the current user.
   def execute(current_user = nil)
-    segments = all_groups(current_user)
-
-    find_union(segments, Group).order_id_desc
-  end
-
-  private
-
-  def all_groups(current_user)
-    groups = []
-
-    groups << @user.authorized_groups.visible_to_user(current_user) if current_user
-    groups << @user.authorized_groups.public_to_user(current_user)
-
-    groups
+    @user.authorized_groups
+      .public_or_visible_to_user(current_user)
+      .order_id_desc
   end
 end
diff --git a/app/models/group.rb b/app/models/group.rb
index 62af20d2142..612c546ca57 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -82,8 +82,17 @@ class Group < Namespace
       User.reference_pattern
     end
 
-    def visible_to_user(user)
-      where(id: user.authorized_groups.select(:id).reorder(nil))
+    # WARNING: This method should never be used on its own
+    # please do make sure the number of rows you are filtering is small
+    # enough for this query
+    def public_or_visible_to_user(user)
+      return public_to_user unless user
+
+      public_for_user = public_to_user_arel(user)
+      visible_for_user = visible_to_user_arel(user)
+      public_or_visible = public_for_user.or(visible_for_user)
+
+      where(public_or_visible)
     end
 
     def select_for_project_authorization
@@ -95,6 +104,23 @@ class Group < Namespace
         super
       end
     end
+
+    private
+
+    def public_to_user_arel(user)
+      self.arel_table[:visibility_level]
+        .in(Gitlab::VisibilityLevel.levels_for_user(user))
+    end
+
+    def visible_to_user_arel(user)
+      groups_table = self.arel_table
+      authorized_groups = user.authorized_groups.as('authorized')
+
+      groups_table.project(1)
+        .from(authorized_groups)
+        .where(authorized_groups[:id].eq(groups_table[:id]))
+        .exists
+    end
   end
 
   # Overrides notification_settings has_many association
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index dd5d494997d..0481a4a3d28 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -6,6 +6,7 @@ class MergeRequest < ActiveRecord::Base
   include Issuable
   include Noteable
   include Referable
+  include Presentable
   include IgnorableColumn
   include TimeTrackable
   include ManualInverseAssociation
diff --git a/app/models/user.rb b/app/models/user.rb
index eeac87e2e52..cd3b1c95b7e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -674,10 +674,12 @@ class User < ActiveRecord::Base
 
   # Returns the groups a user has access to, either through a membership or a project authorization
   def authorized_groups
-    Group.from_union([
-      groups,
-      authorized_projects.joins(:namespace).select('namespaces.*')
-    ])
+    Group.unscoped do
+      Group.from_union([
+        groups,
+        authorized_projects.joins(:namespace).select('namespaces.*')
+      ])
+    end
   end
 
   # Returns the groups a user is a member of, either directly or through a parent group
diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml
index 5a59f956cb5..13b967beba1 100644
--- a/app/views/projects/merge_requests/_form.html.haml
+++ b/app/views/projects/merge_requests/_form.html.haml
@@ -1,4 +1,4 @@
 = form_for [@project.namespace.becomes(Namespace), @project, @merge_request],
            html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' },
            data: { markdown_version: @merge_request.cached_markdown_version } do |f|
-  = render 'shared/issuable/form', f: f, issuable: @merge_request
+  = render 'shared/issuable/form', f: f, issuable: @merge_request, presenter: @mr_presenter
diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml
index d5c4134dee2..464f8fa65e9 100644
--- a/app/views/projects/merge_requests/creations/_new_submit.html.haml
+++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml
@@ -11,7 +11,7 @@
     = link_to 'Change branches', mr_change_branches_path(@merge_request)
 %hr
 = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form common-note-form js-requires-input js-quick-submit' } do |f|
-  = render 'shared/issuable/form', f: f, issuable: @merge_request, commits: @commits
+  = render 'shared/issuable/form', f: f, issuable: @merge_request, commits: @commits, presenter: @mr_presenter
   = f.hidden_field :source_project_id
   = f.hidden_field :source_branch
   = f.hidden_field :target_project_id
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 5b28a43a361..b33c758b464 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -1,6 +1,7 @@
 - form = local_assigns.fetch(:f)
 - commits = local_assigns[:commits]
 - project = @target_project || @project
+- presenter = local_assigns.fetch(:presenter, nil)
 
 = form_errors(issuable)
 
@@ -29,7 +30,7 @@
 
 = render 'shared/issuable/form/metadata', issuable: issuable, form: form
 
-= render_if_exists 'shared/issuable/approvals', issuable: issuable, form: form
+= render_if_exists 'shared/issuable/approvals', issuable: issuable, presenter: presenter, form: form
 
 = render 'shared/issuable/form/branch_chooser', issuable: issuable, form: form