Merge branch 'update_issues_mr_counter' into 'master'

Fix issues with wrong issues/merge request counts when filters are selected

Closes #15356 plus counter for issues and MR are now displayed for the these paths `https://gitlab.com/groups/group-name/issues`  `https://gitlab.com/groups/group-name/merge_requests`  `https://gitlab.com/dashboard/issues` and  `https://gitlab.com/dashboard/merge_requests`

See merge request !4960

app/assets/javascripts/issuable.js.es6

@@ -16,11 +16,11 @@
     },
     initSearch: function() {
       this.timer = null;
-      return $('#issue_search').off('keyup').on('keyup', function() {
+      return $('#issuable_search').off('keyup').on('keyup', function() {
         clearTimeout(this.timer);
         return this.timer = setTimeout(function() {
           var $form, $input, $search;
-          $search = $('#issue_search');
+          $search = $('#issuable_search');
           $form = $('.js-filter-form');
           $input = $("input[name='" + ($search.attr('name')) + "']", $form);
           if ($input.length === 0) {

app/controllers/concerns/issuable_collections.rb

@@ -13,10 +13,18 @@ module IssuableCollections
     issues_finder.execute
   end
 
+  def all_issues_collection
+    IssuesFinder.new(current_user, filter_params_all).execute
+  end
+
   def merge_requests_collection
     merge_requests_finder.execute
   end
 
+  def all_merge_requests_collection
+    MergeRequestsFinder.new(current_user, filter_params_all).execute
+  end
+
   def issues_finder
     @issues_finder ||= issuable_finder_for(IssuesFinder)
   end
@@ -54,6 +62,10 @@ module IssuableCollections
     @filter_params
   end
 
+  def filter_params_all
+    @filter_params_all ||= filter_params.merge(state: 'all', sort: nil)
+  end
+
   def set_default_scope
     params[:scope] = 'all' if params[:scope].blank?
   end

app/controllers/concerns/issues_action.rb

@@ -10,6 +10,8 @@ module IssuesAction
               .preload(:author, :project)
               .page(params[:page])
 
+    @all_issues = all_issues_collection.non_archived
+
     respond_to do |format|
       format.html
       format.atom { render layout: false }

app/controllers/concerns/merge_requests_action.rb

@@ -9,5 +9,7 @@ module MergeRequestsAction
                       .non_archived
                       .preload(:author, :target_project)
                       .page(params[:page])
+
+    @all_merge_requests = all_merge_requests_collection.non_archived
   end
 end

app/controllers/projects/issues_controller.rb

@@ -23,20 +23,13 @@ class Projects::IssuesController < Projects::ApplicationController
   respond_to :html
 
   def index
-    terms = params['issue_search']
     @issues = issues_collection
-
-    if terms.present?
-      if terms =~ /\A#(\d+)\z/
-        @issues = @issues.where(iid: $1)
-      else
-        @issues = @issues.full_search(terms)
-      end
-    end
-
     @issues = @issues.page(params[:page])
+
     @labels = @project.labels.where(title: params[:label_name])
 
+    @all_issues = all_issues_collection
+
     respond_to do |format|
       format.html
       format.atom { render layout: false }

app/controllers/projects/merge_requests_controller.rb

@@ -31,22 +31,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts]
 
   def index
-    terms = params['issue_search']
     @merge_requests = merge_requests_collection
-
-    if terms.present?
-      if terms =~ /\A[#!](\d+)\z/
-        @merge_requests = @merge_requests.where(iid: $1)
-      else
-        @merge_requests = @merge_requests.full_search(terms)
-      end
-    end
-
     @merge_requests = @merge_requests.page(params[:page])
     @merge_requests = @merge_requests.preload(:target_project)
 
     @labels = @project.labels.where(title: params[:label_name])
 
+    @all_merge_requests = all_merge_requests_collection
+
     respond_to do |format|
       format.html
       format.json do

app/finders/issuable_finder.rb

@@ -216,7 +216,14 @@ class IssuableFinder
   end
 
   def by_search(items)
-    items = items.search(search) if search
+    if search
+      items =
+        if search =~ iid_pattern
+          items.where(iid: $~[:iid])
+        else
+          items.full_search(search)
+        end
+    end
 
     items
   end

app/finders/issues_finder.rb

@@ -25,4 +25,8 @@ class IssuesFinder < IssuableFinder
   def init_collection
     Issue.visible_to_user(current_user)
   end
+
+  def iid_pattern
+    @iid_pattern ||= %r{\A#{Regexp.escape(Issue.reference_prefix)}(?<iid>\d+)\z}
+  end
 end

app/finders/merge_requests_finder.rb

@@ -19,4 +19,14 @@ class MergeRequestsFinder < IssuableFinder
   def klass
     MergeRequest
   end
+
+  private
+
+  def iid_pattern
+    @iid_pattern ||= %r{\A[
+      #{Regexp.escape(MergeRequest.reference_prefix)}
+      #{Regexp.escape(Issue.reference_prefix)}
+      ](?<iid>\d+)\z
+    }x
+  end
 end

app/helpers/application_helper.rb

@@ -249,7 +249,7 @@ module ApplicationHelper
       milestone_title: params[:milestone_title],
       assignee_id: params[:assignee_id],
       author_id: params[:author_id],
-      issue_search: params[:issue_search],
+      search: params[:search],
       label_name: params[:label_name]
     }
 
@@ -280,23 +280,14 @@ module ApplicationHelper
     end
   end
 
-  def state_filters_text_for(entity, project)
+  def state_filters_text_for(state, records)
     titles = {
       opened: "Open"
     }
 
-    entity_title = titles[entity] || entity.to_s.humanize
-
-    count =
-      if project.nil?
-        nil
-      elsif current_controller?(:issues)
-        project.issues.visible_to_user(current_user).send(entity).count
-      elsif current_controller?(:merge_requests)
-        project.merge_requests.send(entity).count
-      end
-
-    html = content_tag :span, entity_title
+    state_title = titles[state] || state.to_s.humanize
+    count       = records.public_send(state).size
+    html        = content_tag :span, state_title
 
     if count.present?
       html += " "

app/views/shared/issuable/_filter.html.haml

@@ -2,9 +2,9 @@
 
 .issues-filters
   .issues-details-filters.row-content-block.second-block
-    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :issue_search]), method: :get, class: 'filter-form js-filter-form' do
-      - if params[:issue_search].present?
-        = hidden_field_tag :issue_search, params[:issue_search]
+    = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :search]), method: :get, class: 'filter-form js-filter-form' do
+      - if params[:search].present?
+        = hidden_field_tag :search, params[:search]
       - if @bulk_edit
         .check-all-holder
           = check_box_tag "check_all_issues", nil, false,
@@ -29,7 +29,7 @@
           = render "shared/issuable/label_dropdown"
 
         .filter-item.inline.reset-filters
-          %a{href: page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :issue_search])} Reset filters
+          %a{href: page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name, :search])} Reset filters
 
         .pull-right
           - if boards_page

app/views/shared/issuable/_nav.html.haml

 %ul.nav-links.issues-state-filters
   - if defined?(type) && type == :merge_requests
     - page_context_word = 'merge requests'
+    - records = @all_merge_requests
   - else
     - page_context_word = 'issues'
+    - records = @all_issues
   %li{class: ("active" if params[:state] == 'opened')}
     = link_to page_filter_path(state: 'opened', label: true), title: "Filter by #{page_context_word} that are currently opened." do
-      #{state_filters_text_for(:opened, @project)}
+      #{state_filters_text_for(:opened, records)}
 
   - if defined?(type) && type == :merge_requests
     %li{class: ("active" if params[:state] == 'merged')}
       = link_to page_filter_path(state: 'merged', label: true), title: 'Filter by merge requests that are currently merged.' do
-        #{state_filters_text_for(:merged, @project)}
+        #{state_filters_text_for(:merged, records)}
 
     %li{class: ("active" if params[:state] == 'closed')}
       = link_to page_filter_path(state: 'closed', label: true), title: 'Filter by merge requests that are currently closed and unmerged.' do
-        #{state_filters_text_for(:closed, @project)}
+        #{state_filters_text_for(:closed, records)}
   - else
     %li{class: ("active" if params[:state] == 'closed')}
       = link_to page_filter_path(state: 'closed', label: true), title: 'Filter by issues that are currently closed.' do
-        #{state_filters_text_for(:closed, @project)}
+        #{state_filters_text_for(:closed, records)}
 
   %li{class: ("active" if params[:state] == 'all')}
     = link_to page_filter_path(state: 'all', label: true), title: "Show all #{page_context_word}." do
-      #{state_filters_text_for(:all, @project)}
+      #{state_filters_text_for(:all, records)}

app/views/shared/issuable/_search_form.html.haml

-= form_tag(path, method: :get, id: "issue_search_form", class: 'issue-search-form') do
-  = search_field_tag :issue_search, params[:issue_search], { placeholder: 'Filter by name ...', class: 'form-control issue_search search-text-input input-short', spellcheck: false }
+= form_tag(path, method: :get, id: "issuable_search_form", class: 'issuable-search-form') do
+  = search_field_tag :search, params[:search], { id: 'issuable_search', placeholder: 'Filter by name ...', class: 'form-control issuable_search search-text-input input-short', spellcheck: false }

features/steps/project/issues/issues.rb

@@ -356,6 +356,6 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
   end
 
   def filter_issue(text)
-    fill_in 'issue_search', with: text
+    fill_in 'issuable_search', with: text
   end
 end

features/steps/project/merge_requests.rb

@@ -493,7 +493,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I fill in merge request search with "Fe"' do
-    fill_in 'issue_search', with: "Fe"
+    fill_in 'issuable_search', with: "Fe"
   end
 
   step 'I click the "Target branch" dropdown' do

spec/features/dashboard_issues_spec.rb

@@ -21,6 +21,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do
 
       click_link 'No Milestone'
 
+      page.within '.issues-state-filters' do
+        expect(page).to have_selector('.active .badge', text: '1')
+      end
       expect(page).to have_selector('.issue', count: 1)
     end
 
@@ -29,6 +32,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do
 
       click_link 'Any Milestone'
 
+      page.within '.issues-state-filters' do
+        expect(page).to have_selector('.active .badge', text: '2')
+      end
       expect(page).to have_selector('.issue', count: 2)
     end
 
@@ -39,6 +45,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do
         click_link milestone.title
       end
 
+      page.within '.issues-state-filters' do
+        expect(page).to have_selector('.active .badge', text: '1')
+      end
       expect(page).to have_selector('.issue', count: 1)
     end
   end

spec/features/issues/filter_issues_spec.rb

@@ -206,7 +206,7 @@ describe 'Filter issues', feature: true do
 
     context 'only text', js: true do
       it 'filters issues by searched text' do
-        fill_in 'issue_search', with: 'Bug'
+        fill_in 'issuable_search', with: 'Bug'
 
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
@@ -214,7 +214,7 @@ describe 'Filter issues', feature: true do
       end
 
       it 'does not show any issues' do
-        fill_in 'issue_search', with: 'testing'
+        fill_in 'issuable_search', with: 'testing'
 
         page.within '.issues-list' do
           expect(page).not_to have_selector('.issue')
@@ -224,12 +224,16 @@ describe 'Filter issues', feature: true do
 
     context 'text and dropdown options', js: true do
       it 'filters by text and label' do
-        fill_in 'issue_search', with: 'Bug'
+        fill_in 'issuable_search', with: 'Bug'
 
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
 
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '2')
+        end
+
         click_button 'Label'
         page.within '.labels-filter' do
           click_link 'bug'
@@ -239,15 +243,23 @@ describe 'Filter issues', feature: true do
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
+
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '1')
+        end
       end
 
       it 'filters by text and milestone' do
-        fill_in 'issue_search', with: 'Bug'
+        fill_in 'issuable_search', with: 'Bug'
 
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
 
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '2')
+        end
+
         click_button 'Milestone'
         page.within '.milestone-filter' do
           click_link '8'
@@ -256,15 +268,23 @@ describe 'Filter issues', feature: true do
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
+
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '1')
+        end
       end
 
       it 'filters by text and assignee' do
-        fill_in 'issue_search', with: 'Bug'
+        fill_in 'issuable_search', with: 'Bug'
 
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
 
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '2')
+        end
+
         click_button 'Assignee'
         page.within '.dropdown-menu-assignee' do
           click_link user.name
@@ -273,15 +293,23 @@ describe 'Filter issues', feature: true do
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
+
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '1')
+        end
       end
 
       it 'filters by text and author' do
-        fill_in 'issue_search', with: 'Bug'
+        fill_in 'issuable_search', with: 'Bug'
 
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 2)
         end
 
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '2')
+        end
+
         click_button 'Author'
         page.within '.dropdown-menu-author' do
           click_link user.name
@@ -290,6 +318,10 @@ describe 'Filter issues', feature: true do
         page.within '.issues-list' do
           expect(page).to have_selector('.issue', count: 1)
         end
+
+        page.within '.issues-state-filters' do
+          expect(page).to have_selector('.active .badge', text: '1')
+        end
       end
     end
   end

spec/features/issues/reset_filters_spec.rb

@@ -37,7 +37,7 @@ feature 'Issues filter reset button', feature: true, js: true do
 
   context 'when a text search has been conducted' do
     it 'resets the text search filter' do
-      visit_issues(project, issue_search: 'Bug')
+      visit_issues(project, search: 'Bug')
       expect(page).to have_css('.issue', count: 1)
 
       reset_filters
@@ -67,7 +67,7 @@ feature 'Issues filter reset button', feature: true, js: true do
 
   context 'when all filters have been applied' do
     it 'resets all filters' do
-      visit_issues(project, assignee_id: user.id, author_id: user.id, milestone_title: milestone.title, label_name: bug.name, issue_search: 'Bug')
+      visit_issues(project, assignee_id: user.id, author_id: user.id, milestone_title: milestone.title, label_name: bug.name, search: 'Bug')
       expect(page).to have_css('.issue', count: 0)
 
       reset_filters

spec/finders/issues_finder_spec.rb

@@ -7,8 +7,8 @@ describe IssuesFinder do
   let(:project2) { create(:empty_project) }
   let(:milestone) { create(:milestone, project: project1) }
   let(:label) { create(:label, project: project2) }
-  let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone) }
-  let(:issue2) { create(:issue, author: user, assignee: user, project: project2) }
+  let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') }
+  let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') }
   let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) }
   let!(:label_link) { create(:label_link, label: label, target: issue2) }
 
@@ -127,6 +127,22 @@ describe IssuesFinder do
         end
       end
 
+      context 'filtering by issue term' do
+        let(:params) { { search: 'git' } }
+
+        it 'returns issues with title and description match for search term' do
+          expect(issues).to contain_exactly(issue1, issue2)
+        end
+      end
+
+      context 'filtering by issue iid' do
+        let(:params) { { search: issue3.to_reference } }
+
+        it 'returns issue with iid match' do
+          expect(issues).to contain_exactly(issue3)
+        end
+      end
+
       context 'when the user is unauthorized' do
         let(:search_user) { nil }