Merge remote-tracking branch 'upstream/master' into ce-to-ee-2017-06-30

app/assets/javascripts/group_name.js

-
+import Cookies from 'js-cookie';
 import _ from 'underscore';
 
 export default class GroupName {
   constructor() {
-    this.titleContainer = document.querySelector('.title-container');
-    this.title = document.querySelector('.title');
+    this.titleContainer = document.querySelector('.js-title-container');
+    this.title = this.titleContainer.querySelector('.title');
     this.titleWidth = this.title.offsetWidth;
-    this.groupTitle = document.querySelector('.group-title');
-    this.groups = document.querySelectorAll('.group-path');
+    this.groupTitle = this.titleContainer.querySelector('.group-title');
+    this.groups = this.titleContainer.querySelectorAll('.group-path');
     this.toggle = null;
     this.isHidden = false;
     this.init();
@@ -33,11 +33,20 @@ export default class GroupName {
 
   createToggle() {
     this.toggle = document.createElement('button');
+    this.toggle.setAttribute('type', 'button');
     this.toggle.className = 'text-expander group-name-toggle';
     this.toggle.setAttribute('aria-label', 'Toggle full path');
+    if (Cookies.get('new_nav') === 'true') {
+      this.toggle.innerHTML = '<i class="fa fa-ellipsis-h" aria-hidden="true"></i>';
+    } else {
       this.toggle.innerHTML = '...';
+    }
     this.toggle.addEventListener('click', this.toggleGroups.bind(this));
+    if (Cookies.get('new_nav') === 'true') {
+      this.title.insertBefore(this.toggle, this.groupTitle);
+    } else {
       this.titleContainer.insertBefore(this.toggle, this.title);
+    }
     this.toggleGroups();
   }
 

app/assets/javascripts/vue_merge_request_widget/ee/services/mr_widget_service.js

@@ -6,7 +6,13 @@ export default class MRWidgetService extends CEWidgetService {
   constructor(mr) {
     super(mr);
 
-    this.approvalsResource = Vue.resource(mr.approvalsPath);
+    // Set as a text/plain request so BE doesn't try to parse
+    // See https://gitlab.com/gitlab-org/gitlab-ce/issues/34534
+    this.approvalsResource = Vue.resource(mr.approvalsPath, {}, {}, {
+      headers: {
+        'Content-Type': 'text/plain',
+      },
+    });
     this.rebaseResource = Vue.resource(mr.rebasePath);
   }
 

app/assets/stylesheets/new_nav.scss

@@ -264,3 +264,127 @@ header.navbar-gitlab-new {
     }
   }
 }
+
+.breadcrumbs {
+  display: flex;
+  min-height: 60px;
+  padding-top: $gl-padding-top;
+  padding-bottom: $gl-padding-top;
+  color: $gl-text-color;
+  border-bottom: 1px solid $border-color;
+
+  .dropdown-toggle-caret {
+    position: relative;
+    top: -1px;
+    padding: 0 5px;
+    color: rgba($black, .65);
+    font-size: 10px;
+    line-height: 1;
+    background: none;
+    border: 0;
+
+    &:focus {
+      outline: 0;
+    }
+  }
+}
+
+.breadcrumbs-container {
+  display: flex;
+  width: 100%;
+  position: relative;
+
+  .dropdown-menu-projects {
+    margin-top: -$gl-padding;
+    margin-left: $gl-padding;
+  }
+}
+
+.breadcrumbs-links {
+  flex: 1;
+  align-self: center;
+  color: $black-transparent;
+
+  a {
+    color: rgba($black, .65);
+
+    &:not(:first-child),
+    &.group-path {
+      margin-left: 4px;
+    }
+
+    &:not(:last-of-type),
+    &.group-path {
+      margin-right: 3px;
+    }
+  }
+
+  .title {
+    white-space: nowrap;
+
+    > a {
+      &:last-of-type {
+        font-weight: 600;
+      }
+    }
+  }
+
+  .avatar-tile {
+    margin-right: 5px;
+    border: 1px solid $border-color;
+    border-radius: 50%;
+    vertical-align: sub;
+
+    &.identicon {
+      float: left;
+      width: 16px;
+      height: 16px;
+      margin-top: 2px;
+      font-size: 10px;
+    }
+  }
+
+  .text-expander {
+    margin-left: 4px;
+    margin-right: 4px;
+
+    > i {
+      position: relative;
+      top: 1px;
+    }
+  }
+}
+
+.breadcrumbs-extra {
+  flex: 0 0 auto;
+  margin-left: auto;
+}
+
+.breadcrumbs-sub-title {
+  margin: 2px 0 0;
+  font-size: 16px;
+  font-weight: normal;
+
+  ul {
+    margin: 0;
+  }
+
+  li {
+    display: inline-block;
+
+    &:not(:last-child) {
+      &::after {
+        content: "/";
+        margin: 0 2px 0 5px;
+      }
+    }
+
+    &:last-child a {
+      font-weight: 600;
+    }
+  }
+
+  a {
+    color: $gl-text-color;
+  }
+}

app/finders/issuable_finder.rb

@@ -20,6 +20,7 @@
 #
 class IssuableFinder
   NONE = '0'.freeze
+  IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page].freeze
 
   SCALAR_PARAMS = %i(scope state group_id project_id milestone_title assignee_id search label_name sort assignee_username author_id author_username authorized_only due_date iids non_archived weight).freeze
   ARRAY_PARAMS = { label_name: [], iids: [] }.freeze
@@ -67,7 +68,7 @@ class IssuableFinder
   # grouping and counting within that query.
   #
   def count_by_state
-    count_params = params.merge(state: nil, sort: nil)
+    count_params = params.merge(state: nil, sort: nil, for_counting: true)
     labels_count = label_names.any? ? label_names.count : 1
     finder = self.class.new(current_user, count_params)
     counts = Hash.new(0)
@@ -91,6 +92,10 @@ class IssuableFinder
     execute.find_by!(*params)
   end
 
+  def state_counter_cache_key(state)
+    Digest::SHA1.hexdigest(state_counter_cache_key_components(state).flatten.join('-'))
+  end
+
   def group
     return @group if defined?(@group)
 
@@ -448,4 +453,13 @@ class IssuableFinder
   def current_user_related?
     params[:scope] == 'created-by-me' || params[:scope] == 'authored' || params[:scope] == 'assigned-to-me'
   end
+
+  def state_counter_cache_key_components(state)
+    opts = params.with_indifferent_access
+    opts[:state] = state
+    opts.except!(*IRRELEVANT_PARAMS_FOR_CACHE_KEY)
+    opts.delete_if { |_, value| value.blank? }
+
+    ['issuables_count', klass.to_ability_name, opts.sort]
+  end
 end

app/finders/issues_finder.rb

@@ -16,14 +16,72 @@
 #     sort: string
 #
 class IssuesFinder < IssuableFinder
+  CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER
+
   def klass
     Issue
   end
 
+  def with_confidentiality_access_check
+    return Issue.all if user_can_see_all_confidential_issues?
+    return Issue.where('issues.confidential IS NOT TRUE') if user_cannot_see_confidential_issues?
+
+    Issue.where('
+      issues.confidential IS NOT TRUE
+      OR (issues.confidential = TRUE
+        AND (issues.author_id = :user_id
+          OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id)
+          OR issues.project_id IN(:project_ids)))',
+      user_id: current_user.id,
+      project_ids: current_user.authorized_projects(CONFIDENTIAL_ACCESS_LEVEL).select(:id))
+  end
+
   private
 
   def init_collection
-    IssuesFinder.not_restricted_by_confidentiality(current_user)
+    with_confidentiality_access_check
+  end
+
+  def user_can_see_all_confidential_issues?
+    return @user_can_see_all_confidential_issues if defined?(@user_can_see_all_confidential_issues)
+
+    return @user_can_see_all_confidential_issues = false if current_user.blank?
+    return @user_can_see_all_confidential_issues = true if current_user.full_private_access?
+
+    @user_can_see_all_confidential_issues =
+      project? &&
+      project &&
+      project.team.max_member_access(current_user.id) >= CONFIDENTIAL_ACCESS_LEVEL
+  end
+
+  # Anonymous users can't see any confidential issues.
+  #
+  # Users without access to see _all_ confidential issues (as in
+  # `user_can_see_all_confidential_issues?`) are more complicated, because they
+  # can see confidential issues where:
+  # 1. They are an assignee.
+  # 2. They are an author.
+  #
+  # That's fine for most cases, but if we're just counting, we need to cache
+  # effectively. If we cached this accurately, we'd have a cache key for every
+  # authenticated user without sufficient access to the project. Instead, when
+  # we are counting, we treat them as if they can't see any confidential issues.
+  #
+  # This does mean the counts may be wrong for those users, but avoids an
+  # explosion in cache keys.
+  def user_cannot_see_confidential_issues?(for_counting: false)
+    return false if user_can_see_all_confidential_issues?
+
+    current_user.blank? || for_counting || params[:for_counting]
+  end
+
+  def state_counter_cache_key_components(state)
+    extra_components = [
+      user_can_see_all_confidential_issues?,
+      user_cannot_see_confidential_issues?(for_counting: true)
+    ]
+
+    super + extra_components
   end
 
   def by_assignee(items)
@@ -38,21 +96,6 @@ class IssuesFinder < IssuableFinder
     end
   end
 
-  def self.not_restricted_by_confidentiality(user)
-    return Issue.where('issues.confidential IS NOT TRUE') if user.blank?
-
-    return Issue.all if user.full_private_access?
-
-    Issue.where('
-      issues.confidential IS NOT TRUE
-      OR (issues.confidential = TRUE
-        AND (issues.author_id = :user_id
-          OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id)
-          OR issues.project_id IN(:project_ids)))',
-      user_id: user.id,
-      project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
-  end
-
   def item_project_ids(items)
     items&.reorder(nil)&.select(:project_id)
   end

app/helpers/groups_helper.rb

@@ -16,11 +16,12 @@ module GroupsHelper
     full_title = ''
 
     group.ancestors.reverse.each do |parent|
-      full_title += link_to(simple_sanitize(parent.name), group_path(parent), class: 'group-path hidable')
+      full_title += group_title_link(parent, hidable: true)
+
       full_title += '<span class="hidable"> / </span>'.html_safe
     end
 
-    full_title += link_to(simple_sanitize(group.name), group_path(group), class: 'group-path')
+    full_title += group_title_link(group)
     full_title += ' &middot; '.html_safe + link_to(simple_sanitize(name), url, class: 'group-path') if name
 
     content_tag :span, class: 'group-title' do
@@ -62,4 +63,20 @@ module GroupsHelper
   def group_issues(group)
     IssuesFinder.new(current_user, group_id: group.id).execute
   end
+
+  private
+
+  def group_title_link(group, hidable: false)
+    link_to(group_path(group), class: "group-path #{'hidable' if hidable}") do
+      output =
+        if show_new_nav?
+          image_tag(group_icon(group), class: "avatar-tile", width: 16, height: 16)
+        else
+          ""
+        end
+
+      output << simple_sanitize(group.name)
+      output.html_safe
+    end
+  end
 end

app/helpers/issuables_helper.rb

@@ -166,7 +166,7 @@ module IssuablesHelper
 
     state_title = titles[state] || state.to_s.humanize
 
-    count = cached_issuables_count_for_state(issuable_type, state)
+    count = issuables_count_for_state(issuable_type, state)
 
     html = content_tag(:span, state_title)
     html << " " << content_tag(:span, number_with_delimiter(count), class: 'badge')
@@ -174,12 +174,6 @@ module IssuablesHelper
     html.html_safe
   end
 
-  def cached_issuables_count_for_state(issuable_type, state)
-    Rails.cache.fetch(issuables_state_counter_cache_key(issuable_type, state), expires_in: 2.minutes) do
-      issuables_count_for_state(issuable_type, state)
-    end
-  end
-
   def cached_assigned_issuables_count(assignee, issuable_type, state)
     cache_key = hexdigest(['assigned_issuables_count', assignee.id, issuable_type, state].join('-'))
     Rails.cache.fetch(cache_key, expires_in: 2.minutes) do
@@ -248,6 +242,18 @@ module IssuablesHelper
     }
   end
 
+  def issuables_count_for_state(issuable_type, state, finder: nil)
+    finder ||= public_send("#{issuable_type}_finder")
+    cache_key = finder.state_counter_cache_key(state)
+
+    @counts ||= {}
+    @counts[cache_key] ||= Rails.cache.fetch(cache_key, expires_in: 2.minutes) do
+      finder.count_by_state
+    end
+
+    @counts[cache_key][state]
+  end
+
   private
 
   def sidebar_gutter_collapsed?
@@ -266,24 +272,6 @@ module IssuablesHelper
     end
   end
 
-  def issuables_count_for_state(issuable_type, state)
-    @counts ||= {}
-    @counts[issuable_type] ||= public_send("#{issuable_type}_finder").count_by_state
-    @counts[issuable_type][state]
-  end
-
-  IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page].freeze
-  private_constant :IRRELEVANT_PARAMS_FOR_CACHE_KEY
-
-  def issuables_state_counter_cache_key(issuable_type, state)
-    opts = params.with_indifferent_access
-    opts[:state] = state
-    opts.except!(*IRRELEVANT_PARAMS_FOR_CACHE_KEY)
-    opts.delete_if { |_, value| value.blank? }
-
-    hexdigest(['issuables_count', issuable_type, opts.sort].flatten.join('-'))
-  end
-
   def issuable_templates(issuable)
     @issuable_templates ||=
       case issuable

app/helpers/projects_helper.rb

@@ -58,7 +58,17 @@ module ProjectsHelper
         link_to(simple_sanitize(owner.name), user_path(owner))
       end
 
-    project_link = link_to simple_sanitize(project.name), project_path(project), { class: "project-item-select-holder" }
+    project_link = link_to project_path(project), { class: "project-item-select-holder" } do
+      output =
+        if show_new_nav?
+          project_icon(project, alt: project.name, class: 'avatar-tile', width: 16, height: 16)
+        else
+          ""
+        end
+
+      output << simple_sanitize(project.name)
+      output.html_safe
+    end
 
     if current_user
       project_link << button_tag(type: 'button', class: 'dropdown-toggle-caret js-projects-dropdown-toggle', aria: { label: 'Toggle switch project dropdown' }, data: { target: '.js-dropdown-menu-projects', toggle: 'dropdown', order_by: 'last_activity_at' }) do

app/views/dashboard/activity.html.haml

+- @hide_top_links = true
 - @no_container = true
 
 = content_for :meta_tags do

app/views/dashboard/groups/index.html.haml

+- @hide_top_links = true
 - page_title "Groups"
 - header_title  "Groups", dashboard_groups_path
 = render 'dashboard/groups_head'

app/views/dashboard/milestones/index.html.haml

+- @hide_top_links = true
 - page_title 'Milestones'
 - header_title 'Milestones', dashboard_milestones_path
 

app/views/dashboard/projects/index.html.haml

 - @no_container = true
+- @hide_top_links = true
+- @breadcrumb_title = "Projects"
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, dashboard_projects_url(rss_url_options), title: "All activity")

app/views/dashboard/snippets/index.html.haml

+- @hide_top_links = true
 - page_title    "Snippets"
 - header_title  "Snippets", dashboard_snippets_path
 

app/views/layouts/_page.html.haml

@@ -14,6 +14,8 @@
       = render "layouts/broadcast"
       = render "layouts/flash"
       = yield :flash_message
+    - if show_new_nav?
+      = render "layouts/nav/breadcrumbs"
     %div{ class: "#{(container_class unless @no_container)} #{@content_class}" }
       .content{ id: "content-body" }
         = yield

app/views/layouts/header/_default.html.haml

@@ -17,7 +17,7 @@
         = link_to root_path, class: 'home', title: 'Dashboard', id: 'logo' do
           = brand_header_logo
 
-      .title-container
+      .title-container.js-title-container
         %h1.title{ class: ('initializing' if @has_group_title) }= title
 
       .navbar-collapse.collapse

app/views/layouts/header/_new.html.haml

@@ -83,8 +83,6 @@
         = icon('ellipsis-v', class: 'js-navbar-toggle-right')
         = icon('times', class: 'js-navbar-toggle-left', style: 'display: none;')
 
-      = yield :header_content
-
 = render 'shared/outdated_browser'
 
 - if @project && !@project.empty_repo?

app/views/layouts/nav/_breadcrumbs.html.haml

+- breadcrumb_title = @breadcrumb_title || controller.controller_name.humanize
+- hide_top_links = @hide_top_links || false
+
+%nav.breadcrumbs{ role: "navigation" }
+  .breadcrumbs-container{ class: container_class }
+    .breadcrumbs-links.js-title-container
+      - unless hide_top_links
+        .title
+          = link_to "GitLab", root_path
+          \/
+          = header_title
+      %h2.breadcrumbs-sub-title
+        %ul.list-unstyled
+          - if content_for?(:sub_title_before)
+            = yield :sub_title_before
+          %li= link_to breadcrumb_title, request.path
+    - if content_for?(:breadcrumbs_extra)
+      .breadcrumbs-extra.hidden-xs= yield :breadcrumbs_extra
+    = yield :header_content

app/views/layouts/nav/_project.html.haml

@@ -28,7 +28,7 @@
           %span
             Issues
             - if @project.default_issues_tracker?
-              %span.badge.count.issue_counter= number_with_delimiter(IssuesFinder.new(current_user, project_id: @project.id).execute.opened.count)
+              %span.badge.count.issue_counter= number_with_delimiter(issuables_count_for_state(:issues, :opened, finder: IssuesFinder.new(current_user, project_id: @project.id)))
 
     - if project_nav_tab? :merge_requests
       - controllers = [:merge_requests, 'projects/merge_requests/conflicts']
@@ -37,7 +37,7 @@
         = link_to namespace_project_merge_requests_path(@project.namespace, @project), title: 'Merge Requests', class: 'shortcuts-merge_requests' do
           %span
             Merge Requests
-            %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(MergeRequestsFinder.new(current_user, project_id: @project.id).execute.opened.count)
+            %span.badge.count.merge_counter.js-merge-counter= number_with_delimiter(issuables_count_for_state(:merge_requests, :opened, finder: MergeRequestsFinder.new(current_user, project_id: @project.id)))
 
     - if project_nav_tab? :pipelines
       = nav_link(controller: [:pipelines, :builds, :environments, :artifacts]) do

app/views/projects/boards/_show.html.haml

@@ -3,6 +3,10 @@
 - @content_class = "issue-boards-content js-focus-mode-board"
 - page_title "Boards"
 
+- if show_new_nav?
+  - content_for :sub_title_before do
+    %li= link_to "Issues", namespace_project_issues_path(@project.namespace, @project)
+
 - content_for :page_specific_javascripts do
   = webpack_bundle_tag 'common_vue'
   = webpack_bundle_tag 'filtered_search'

app/views/projects/issues/_nav_btns.html.haml

+= link_to params.merge(rss_url_options), class: 'btn btn-default append-right-10 has-tooltip', title: 'Subscribe' do
+  = icon('rss')
+= render 'projects/issues/export_issues/button'
+- if @can_bulk_update
+  = button_tag "Edit Issues", class: "btn btn-default append-right-10 js-bulk-update-toggle"
+= link_to "New issue", new_namespace_project_issue_path(@project.namespace,
+                                         @project,
+                                         issue: { assignee_id: issues_finder.assignee.try(:id),
+                                                  milestone_id: issues_finder.milestones.first.try(:id) }),
+                                         class: "btn btn-new",
+                                         title: "New issue",
+                                         id: "new_issue_link"

app/views/projects/issues/export_issues/_csv_download.html.haml

@@ -12,7 +12,7 @@
       .modal-header
         = icon('check', { class: 'export-checkmark' })
         %strong
-          #{pluralize(cached_issuables_count_for_state(:issues, params[:state]), 'issue')} selected
+          #{pluralize(issuables_count_for_state(:issues, params[:state]), 'issue')} selected
       .modal-body
         %div
           The CSV export will be created in the background. Once finished, it will be sent to

app/views/projects/issues/index.html.haml

@@ -14,6 +14,10 @@
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@project.name} issues")
 
+- if show_new_nav?
+  - content_for :breadcrumbs_extra do
+    = render "projects/issues/nav_btns"
+
 - if project_issues(@project).exists?
 
   = render 'projects/issues/export_issues/csv_download'
@@ -21,22 +25,8 @@
   %div{ class: (container_class) }
     .top-area
       = render 'shared/issuable/nav', type: :issues
-      .nav-controls.inline
-        = link_to params.merge(rss_url_options), class: 'btn append-right-10 has-tooltip', title: 'Subscribe' do
-          = icon('rss')
-
-        = render 'projects/issues/export_issues/button'
-
-        - if @can_bulk_update
-          = button_tag "Edit Issues", class: "btn btn-default js-bulk-update-toggle"
-        = link_to new_namespace_project_issue_path(@project.namespace,
-                                                   @project,
-                                                   issue: { assignee_id: issues_finder.assignee.try(:id),
-                                                            milestone_id: issues_finder.milestones.first.try(:id) }),
-                                                   class: "btn btn-new btn-full",
-                                                   title: "New issue",
-                                                   id: "new_issue_link" do
-          New issue
+      .nav-controls{ class: ("visible-xs" if show_new_nav?) }
+        = render "projects/issues/nav_btns"
     = render 'shared/issuable/search_bar', type: :issues
 
     - if @can_bulk_update

app/views/projects/merge_requests/_nav_btns.html.haml

+- if @can_bulk_update
+  = button_tag "Edit Merge Requests", class: "btn js-bulk-update-toggle"
+- if merge_project
+  = link_to new_merge_request_path, class: "btn btn-new", title: "New merge request" do
+    New merge request

app/views/projects/merge_requests/index.html.haml

 - @no_container = true
 - @can_bulk_update = can?(current_user, :admin_merge_request, @project)
+- merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
+- new_merge_request_path = namespace_project_new_merge_request_path(merge_project.namespace, merge_project) if merge_project
 
 - page_title "Merge Requests"
 - unless @project.default_issues_tracker?
@@ -10,6 +12,9 @@
   = webpack_bundle_tag 'common_vue'
   = webpack_bundle_tag 'filtered_search'
 
+- if show_new_nav?
+  - content_for :breadcrumbs_extra do
+    = render "projects/merge_requests/nav_btns", merge_project: merge_project, new_merge_request_path: new_merge_request_path
 
 = render 'projects/last_push'
 
@@ -20,12 +25,8 @@
   %div{ class: container_class }
     .top-area
       = render 'shared/issuable/nav', type: :merge_requests
-      .nav-controls
-        - if @can_bulk_update
-          = button_tag "Edit Merge Requests", class: "btn js-bulk-update-toggle"
-        - if merge_project
-          = link_to new_merge_request_path, class: "btn btn-new", title: "New merge request" do
-            New merge request
+      .nav-controls{ class: ("visible-xs" if show_new_nav?) }
+        = render "projects/merge_requests/nav_btns", merge_project: merge_project, new_merge_request_path: new_merge_request_path
 
     = render 'shared/issuable/search_bar', type: :merge_requests
 

changelogs/unreleased/speed-up-issue-counting-for-a-project.yml

+---
+title: Cache open issue and merge request counts for project tabs to speed up project
+  pages
+merge_request: 12457
+author:

spec/finders/issues_finder_spec.rb

@@ -311,32 +311,167 @@ describe IssuesFinder do
     end
   end
 
-  describe '.not_restricted_by_confidentiality' do
-    let(:authorized_user) { create(:user) }
-    let(:admin_user) { create(:admin) }
+  describe '#with_confidentiality_access_check' do
+    let(:guest) { create(:user) }
+    set(:authorized_user) { create(:user) }
+    let(:admin_user) { create(:user, :admin) }
     let(:auditor_user) { create(:user, :auditor) }
-    let(:project) { create(:empty_project, namespace: authorized_user.namespace) }
-    let!(:public_issue) { create(:issue, project: project) }
-    let!(:confidential_issue) { create(:issue, project: project, confidential: true) }
+    set(:project) { create(:empty_project, namespace: authorized_user.namespace) }
+    set(:public_issue) { create(:issue, project: project) }
+    set(:confidential_issue) { create(:issue, project: project, confidential: true) }
 
-    it 'returns non confidential issues for nil user' do
-      expect(described_class.send(:not_restricted_by_confidentiality, nil)).to include(public_issue)
+    context 'when no project filter is given' do
+      let(:params) { {} }
+
+      context 'for an anonymous user' do
+        subject { described_class.new(nil, params).with_confidentiality_access_check }
+
+        it 'returns only public issues' do
+          expect(subject).to include(public_issue)
+          expect(subject).not_to include(confidential_issue)
         end
+      end
+
+      context 'for a user without project membership' do
+        subject { described_class.new(user, params).with_confidentiality_access_check }
 
-    it 'returns non confidential issues for user not authorized for the issues projects' do
-      expect(described_class.send(:not_restricted_by_confidentiality, user)).to include(public_issue)
+        it 'returns only public issues' do
+          expect(subject).to include(public_issue)
+          expect(subject).not_to include(confidential_issue)
         end
+      end
+
+      context 'for a guest user' do
+        subject { described_class.new(guest, params).with_confidentiality_access_check }
 
-    it 'returns all issues for user authorized for the issues projects' do
-      expect(described_class.send(:not_restricted_by_confidentiality, authorized_user)).to include(public_issue, confidential_issue)
+        before do
+          project.add_guest(guest)
         end
 
-    it 'returns all issues for an admin user' do
-      expect(described_class.send(:not_restricted_by_confidentiality, admin_user)).to include(public_issue, confidential_issue)
+        it 'returns only public issues' do
+          expect(subject).to include(public_issue)
+          expect(subject).not_to include(confidential_issue)
+        end
+      end
+
+      context 'for a project member with access to view confidential issues' do
+        subject { described_class.new(authorized_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue)
+        end
+      end
+
+      context 'for an auditor' do
+        subject { described_class.new(auditor_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue)
+        end
       end
 
-    it 'returns all issues for an auditor user' do
-      expect(described_class.send(:not_restricted_by_confidentiality, auditor_user)).to include(public_issue, confidential_issue)
+      context 'for an admin' do
+        subject { described_class.new(admin_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue)
+        end
+      end
+    end
+
+    context 'when searching within a specific project' do
+      let(:params) { { project_id: project.id } }
+
+      context 'for an anonymous user' do
+        subject { described_class.new(nil, params).with_confidentiality_access_check }
+
+        it 'returns only public issues' do
+          expect(subject).to include(public_issue)
+          expect(subject).not_to include(confidential_issue)
+        end
+
+        it 'does not filter by confidentiality' do
+          expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
+      end
+
+      context 'for a user without project membership' do
+        subject { described_class.new(user, params).with_confidentiality_access_check }
+
+        it 'returns only public issues' do
+          expect(subject).to include(public_issue)
+          expect(subject).not_to include(confidential_issue)
+        end
+
+        it 'filters by confidentiality' do
+          expect(Issue).to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
+      end
+
+      context 'for a guest user' do
+        subject { described_class.new(guest, params).with_confidentiality_access_check }
+
+        before do
+          project.add_guest(guest)
+        end
+
+        it 'returns only public issues' do
+          expect(subject).to include(public_issue)
+          expect(subject).not_to include(confidential_issue)
+        end
+
+        it 'filters by confidentiality' do
+          expect(Issue).to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
+      end
+
+      context 'for a project member with access to view confidential issues' do
+        subject { described_class.new(authorized_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue)
+        end
+
+        it 'does not filter by confidentiality' do
+          expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
+      end
+
+      context 'for an auditor' do
+        subject { described_class.new(auditor_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue)
+        end
+
+        it 'does not filter by confidentiality' do
+          expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
+      end
+
+      context 'for an admin' do
+        subject { described_class.new(auditor_user, params).with_confidentiality_access_check }
+
+        it 'returns all issues' do
+          expect(subject).to include(public_issue, confidential_issue)
+        end
+
+        it 'does not filter by confidentiality' do
+          expect(Issue).not_to receive(:where).with(a_string_matching('confidential'), anything)
+
+          subject
+        end
+      end
     end
   end
 end

spec/helpers/groups_helper_spec.rb

@@ -110,7 +110,7 @@ describe GroupsHelper do
     let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
 
     it 'outputs the groups in the correct order', :postgresql do
-      expect(group_title(very_deep_nested_group)).to match(/>#{group.name}<\/a>.*>#{nested_group.name}<\/a>.*>#{deep_nested_group.name}<\/a>/)
+      expect(helper.group_title(very_deep_nested_group)).to match(/>#{group.name}<\/a>.*>#{nested_group.name}<\/a>.*>#{deep_nested_group.name}<\/a>/)
     end
   end
 end

spec/helpers/issuables_helper_spec.rb

@@ -77,54 +77,89 @@ describe IssuablesHelper do
         }.with_indifferent_access
       end
 
+      let(:issues_finder) { IssuesFinder.new(nil, params) }
+      let(:merge_requests_finder) { MergeRequestsFinder.new(nil, params) }
+
+      before do
+        allow(helper).to receive(:issues_finder).and_return(issues_finder)
+        allow(helper).to receive(:merge_requests_finder).and_return(merge_requests_finder)
+      end
+
       it 'returns the cached value when called for the same issuable type & with the same params' do
-        expect(helper).to receive(:params).twice.and_return(params)
-        expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42)
+        expect(issues_finder).to receive(:count_by_state).and_return(opened: 42)
 
         expect(helper.issuables_state_counter_text(:issues, :opened))
           .to eq('<span>Open</span> <span class="badge">42</span>')
 
-        expect(helper).not_to receive(:issuables_count_for_state)
+        expect(issues_finder).not_to receive(:count_by_state)
 
         expect(helper.issuables_state_counter_text(:issues, :opened))
           .to eq('<span>Open</span> <span class="badge">42</span>')
       end
 
+      it 'takes confidential status into account when searching for issues' do
+        expect(issues_finder).to receive(:count_by_state).and_return(opened: 42)
+
+        expect(helper.issuables_state_counter_text(:issues, :opened))
+          .to include('42')
+
+        expect(issues_finder).to receive(:user_cannot_see_confidential_issues?).twice.and_return(false)
+        expect(issues_finder).to receive(:count_by_state).and_return(opened: 40)
+
+        expect(helper.issuables_state_counter_text(:issues, :opened))
+          .to include('40')
+
+        expect(issues_finder).to receive(:user_can_see_all_confidential_issues?).and_return(true)
+        expect(issues_finder).to receive(:count_by_state).and_return(opened: 45)
+
+        expect(helper.issuables_state_counter_text(:issues, :opened))
+          .to include('45')
+      end
+
+      it 'does not take confidential status into account when searching for merge requests' do
+        expect(merge_requests_finder).to receive(:count_by_state).and_return(opened: 42)
+        expect(merge_requests_finder).not_to receive(:user_cannot_see_confidential_issues?)
+        expect(merge_requests_finder).not_to receive(:user_can_see_all_confidential_issues?)
+
+        expect(helper.issuables_state_counter_text(:merge_requests, :opened))
+          .to include('42')
+      end
+
       it 'does not take some keys into account in the cache key' do
-        expect(helper).to receive(:params).and_return({
+        expect(issues_finder).to receive(:count_by_state).and_return(opened: 42)
+        expect(issues_finder).to receive(:params).and_return({
           author_id: '11',
           state: 'foo',
           sort: 'foo',
           utf8: 'foo',
           page: 'foo'
         }.with_indifferent_access)
-        expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42)
 
         expect(helper.issuables_state_counter_text(:issues, :opened))
           .to eq('<span>Open</span> <span class="badge">42</span>')
 
-        expect(helper).to receive(:params).and_return({
+        expect(issues_finder).not_to receive(:count_by_state)
+        expect(issues_finder).to receive(:params).and_return({
           author_id: '11',
           state: 'bar',
           sort: 'bar',
           utf8: 'bar',
           page: 'bar'
         }.with_indifferent_access)
-        expect(helper).not_to receive(:issuables_count_for_state)
 
         expect(helper.issuables_state_counter_text(:issues, :opened))
           .to eq('<span>Open</span> <span class="badge">42</span>')
       end
 
       it 'does not take params order into account in the cache key' do
-        expect(helper).to receive(:params).and_return('author_id' => '11', 'state' => 'opened')
-        expect(helper).to receive(:issuables_count_for_state).with(:issues, :opened).and_return(42)
+        expect(issues_finder).to receive(:params).and_return('author_id' => '11', 'state' => 'opened')
+        expect(issues_finder).to receive(:count_by_state).and_return(opened: 42)
 
         expect(helper.issuables_state_counter_text(:issues, :opened))
           .to eq('<span>Open</span> <span class="badge">42</span>')
 
-        expect(helper).to receive(:params).and_return('state' => 'opened', 'author_id' => '11')
-        expect(helper).not_to receive(:issuables_count_for_state)
+        expect(issues_finder).to receive(:params).and_return('state' => 'opened', 'author_id' => '11')
+        expect(issues_finder).not_to receive(:count_by_state)
 
         expect(helper.issuables_state_counter_text(:issues, :opened))
           .to eq('<span>Open</span> <span class="badge">42</span>')