Commit 52fcb2e7 authored by Rémy Coutable's avatar Rémy Coutable

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
parents 422c9025 418e95bd
...@@ -16,11 +16,11 @@ ...@@ -16,11 +16,11 @@
}, },
initSearch: function() { initSearch: function() {
this.timer = null; this.timer = null;
return $('#issue_search').off('keyup').on('keyup', function() { return $('#issuable_search').off('keyup').on('keyup', function() {
clearTimeout(this.timer); clearTimeout(this.timer);
return this.timer = setTimeout(function() { return this.timer = setTimeout(function() {
var $form, $input, $search; var $form, $input, $search;
$search = $('#issue_search'); $search = $('#issuable_search');
$form = $('.js-filter-form'); $form = $('.js-filter-form');
$input = $("input[name='" + ($search.attr('name')) + "']", $form); $input = $("input[name='" + ($search.attr('name')) + "']", $form);
if ($input.length === 0) { if ($input.length === 0) {
......
...@@ -13,10 +13,18 @@ module IssuableCollections ...@@ -13,10 +13,18 @@ module IssuableCollections
issues_finder.execute issues_finder.execute
end end
def all_issues_collection
IssuesFinder.new(current_user, filter_params_all).execute
end
def merge_requests_collection def merge_requests_collection
merge_requests_finder.execute merge_requests_finder.execute
end end
def all_merge_requests_collection
MergeRequestsFinder.new(current_user, filter_params_all).execute
end
def issues_finder def issues_finder
@issues_finder ||= issuable_finder_for(IssuesFinder) @issues_finder ||= issuable_finder_for(IssuesFinder)
end end
...@@ -54,6 +62,10 @@ module IssuableCollections ...@@ -54,6 +62,10 @@ module IssuableCollections
@filter_params @filter_params
end end
def filter_params_all
@filter_params_all ||= filter_params.merge(state: 'all', sort: nil)
end
def set_default_scope def set_default_scope
params[:scope] = 'all' if params[:scope].blank? params[:scope] = 'all' if params[:scope].blank?
end end
......
...@@ -10,6 +10,8 @@ module IssuesAction ...@@ -10,6 +10,8 @@ module IssuesAction
.preload(:author, :project) .preload(:author, :project)
.page(params[:page]) .page(params[:page])
@all_issues = all_issues_collection.non_archived
respond_to do |format| respond_to do |format|
format.html format.html
format.atom { render layout: false } format.atom { render layout: false }
......
...@@ -9,5 +9,7 @@ module MergeRequestsAction ...@@ -9,5 +9,7 @@ module MergeRequestsAction
.non_archived .non_archived
.preload(:author, :target_project) .preload(:author, :target_project)
.page(params[:page]) .page(params[:page])
@all_merge_requests = all_merge_requests_collection.non_archived
end end
end end
...@@ -23,20 +23,13 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -23,20 +23,13 @@ class Projects::IssuesController < Projects::ApplicationController
respond_to :html respond_to :html
def index def index
terms = params['issue_search']
@issues = issues_collection @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]) @issues = @issues.page(params[:page])
@labels = @project.labels.where(title: params[:label_name]) @labels = @project.labels.where(title: params[:label_name])
@all_issues = all_issues_collection
respond_to do |format| respond_to do |format|
format.html format.html
format.atom { render layout: false } format.atom { render layout: false }
......
...@@ -31,22 +31,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -31,22 +31,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController
before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts] before_action :authorize_can_resolve_conflicts!, only: [:conflicts, :resolve_conflicts]
def index def index
terms = params['issue_search']
@merge_requests = merge_requests_collection @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.page(params[:page])
@merge_requests = @merge_requests.preload(:target_project) @merge_requests = @merge_requests.preload(:target_project)
@labels = @project.labels.where(title: params[:label_name]) @labels = @project.labels.where(title: params[:label_name])
@all_merge_requests = all_merge_requests_collection
respond_to do |format| respond_to do |format|
format.html format.html
format.json do format.json do
......
...@@ -216,7 +216,14 @@ class IssuableFinder ...@@ -216,7 +216,14 @@ class IssuableFinder
end end
def by_search(items) 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 items
end end
......
...@@ -25,4 +25,8 @@ class IssuesFinder < IssuableFinder ...@@ -25,4 +25,8 @@ class IssuesFinder < IssuableFinder
def init_collection def init_collection
Issue.visible_to_user(current_user) Issue.visible_to_user(current_user)
end end
def iid_pattern
@iid_pattern ||= %r{\A#{Regexp.escape(Issue.reference_prefix)}(?<iid>\d+)\z}
end
end end
...@@ -19,4 +19,14 @@ class MergeRequestsFinder < IssuableFinder ...@@ -19,4 +19,14 @@ class MergeRequestsFinder < IssuableFinder
def klass def klass
MergeRequest MergeRequest
end 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 end
...@@ -249,7 +249,7 @@ module ApplicationHelper ...@@ -249,7 +249,7 @@ module ApplicationHelper
milestone_title: params[:milestone_title], milestone_title: params[:milestone_title],
assignee_id: params[:assignee_id], assignee_id: params[:assignee_id],
author_id: params[:author_id], author_id: params[:author_id],
issue_search: params[:issue_search], search: params[:search],
label_name: params[:label_name] label_name: params[:label_name]
} }
...@@ -280,23 +280,14 @@ module ApplicationHelper ...@@ -280,23 +280,14 @@ module ApplicationHelper
end end
end end
def state_filters_text_for(entity, project) def state_filters_text_for(state, records)
titles = { titles = {
opened: "Open" opened: "Open"
} }
entity_title = titles[entity] || entity.to_s.humanize state_title = titles[state] || state.to_s.humanize
count = records.public_send(state).size
count = html = content_tag :span, state_title
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
if count.present? if count.present?
html += " " html += " "
......
...@@ -2,9 +2,9 @@ ...@@ -2,9 +2,9 @@
.issues-filters .issues-filters
.issues-details-filters.row-content-block.second-block .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 = 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[:issue_search].present? - if params[:search].present?
= hidden_field_tag :issue_search, params[:issue_search] = hidden_field_tag :search, params[:search]
- if @bulk_edit - if @bulk_edit
.check-all-holder .check-all-holder
= check_box_tag "check_all_issues", nil, false, = check_box_tag "check_all_issues", nil, false,
...@@ -29,7 +29,7 @@ ...@@ -29,7 +29,7 @@
= render "shared/issuable/label_dropdown" = render "shared/issuable/label_dropdown"
.filter-item.inline.reset-filters .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 .pull-right
- if boards_page - if boards_page
......
%ul.nav-links.issues-state-filters %ul.nav-links.issues-state-filters
- if defined?(type) && type == :merge_requests - if defined?(type) && type == :merge_requests
- page_context_word = 'merge requests' - page_context_word = 'merge requests'
- records = @all_merge_requests
- else - else
- page_context_word = 'issues' - page_context_word = 'issues'
- records = @all_issues
%li{class: ("active" if params[:state] == 'opened')} %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 = 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 - if defined?(type) && type == :merge_requests
%li{class: ("active" if params[:state] == 'merged')} %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 = 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')} %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 = 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 - else
%li{class: ("active" if params[:state] == 'closed')} %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 = 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')} %li{class: ("active" if params[:state] == 'all')}
= link_to page_filter_path(state: 'all', label: true), title: "Show all #{page_context_word}." do = 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)}
= form_tag(path, method: :get, id: "issue_search_form", class: 'issue-search-form') do = form_tag(path, method: :get, id: "issuable_search_form", class: 'issuable-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 } = 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 }
...@@ -356,6 +356,6 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps ...@@ -356,6 +356,6 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
end end
def filter_issue(text) def filter_issue(text)
fill_in 'issue_search', with: text fill_in 'issuable_search', with: text
end end
end end
...@@ -493,7 +493,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -493,7 +493,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
end end
step 'I fill in merge request search with "Fe"' do step 'I fill in merge request search with "Fe"' do
fill_in 'issue_search', with: "Fe" fill_in 'issuable_search', with: "Fe"
end end
step 'I click the "Target branch" dropdown' do step 'I click the "Target branch" dropdown' do
......
...@@ -21,6 +21,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do ...@@ -21,6 +21,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do
click_link 'No Milestone' 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) expect(page).to have_selector('.issue', count: 1)
end end
...@@ -29,6 +32,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do ...@@ -29,6 +32,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do
click_link 'Any Milestone' 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) expect(page).to have_selector('.issue', count: 2)
end end
...@@ -39,6 +45,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do ...@@ -39,6 +45,9 @@ describe "Dashboard Issues filtering", feature: true, js: true do
click_link milestone.title click_link milestone.title
end 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) expect(page).to have_selector('.issue', count: 1)
end end
end end
......
...@@ -206,7 +206,7 @@ describe 'Filter issues', feature: true do ...@@ -206,7 +206,7 @@ describe 'Filter issues', feature: true do
context 'only text', js: true do context 'only text', js: true do
it 'filters issues by searched text' 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 page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 2) expect(page).to have_selector('.issue', count: 2)
...@@ -214,7 +214,7 @@ describe 'Filter issues', feature: true do ...@@ -214,7 +214,7 @@ describe 'Filter issues', feature: true do
end end
it 'does not show any issues' do it 'does not show any issues' do
fill_in 'issue_search', with: 'testing' fill_in 'issuable_search', with: 'testing'
page.within '.issues-list' do page.within '.issues-list' do
expect(page).not_to have_selector('.issue') expect(page).not_to have_selector('.issue')
...@@ -224,12 +224,16 @@ describe 'Filter issues', feature: true do ...@@ -224,12 +224,16 @@ describe 'Filter issues', feature: true do
context 'text and dropdown options', js: true do context 'text and dropdown options', js: true do
it 'filters by text and label' 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 page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 2) expect(page).to have_selector('.issue', count: 2)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '2')
end
click_button 'Label' click_button 'Label'
page.within '.labels-filter' do page.within '.labels-filter' do
click_link 'bug' click_link 'bug'
...@@ -239,15 +243,23 @@ describe 'Filter issues', feature: true do ...@@ -239,15 +243,23 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1) expect(page).to have_selector('.issue', count: 1)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '1')
end
end end
it 'filters by text and milestone' do it 'filters by text and milestone' do
fill_in 'issue_search', with: 'Bug' fill_in 'issuable_search', with: 'Bug'
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 2) expect(page).to have_selector('.issue', count: 2)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '2')
end
click_button 'Milestone' click_button 'Milestone'
page.within '.milestone-filter' do page.within '.milestone-filter' do
click_link '8' click_link '8'
...@@ -256,15 +268,23 @@ describe 'Filter issues', feature: true do ...@@ -256,15 +268,23 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1) expect(page).to have_selector('.issue', count: 1)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '1')
end
end end
it 'filters by text and assignee' do it 'filters by text and assignee' do
fill_in 'issue_search', with: 'Bug' fill_in 'issuable_search', with: 'Bug'
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 2) expect(page).to have_selector('.issue', count: 2)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '2')
end
click_button 'Assignee' click_button 'Assignee'
page.within '.dropdown-menu-assignee' do page.within '.dropdown-menu-assignee' do
click_link user.name click_link user.name
...@@ -273,15 +293,23 @@ describe 'Filter issues', feature: true do ...@@ -273,15 +293,23 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1) expect(page).to have_selector('.issue', count: 1)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '1')
end
end end
it 'filters by text and author' do it 'filters by text and author' do
fill_in 'issue_search', with: 'Bug' fill_in 'issuable_search', with: 'Bug'
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 2) expect(page).to have_selector('.issue', count: 2)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '2')
end
click_button 'Author' click_button 'Author'
page.within '.dropdown-menu-author' do page.within '.dropdown-menu-author' do
click_link user.name click_link user.name
...@@ -290,6 +318,10 @@ describe 'Filter issues', feature: true do ...@@ -290,6 +318,10 @@ describe 'Filter issues', feature: true do
page.within '.issues-list' do page.within '.issues-list' do
expect(page).to have_selector('.issue', count: 1) expect(page).to have_selector('.issue', count: 1)
end end
page.within '.issues-state-filters' do
expect(page).to have_selector('.active .badge', text: '1')
end
end end
end end
end end
......
...@@ -37,7 +37,7 @@ feature 'Issues filter reset button', feature: true, js: true do ...@@ -37,7 +37,7 @@ feature 'Issues filter reset button', feature: true, js: true do
context 'when a text search has been conducted' do context 'when a text search has been conducted' do
it 'resets the text search filter' 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) expect(page).to have_css('.issue', count: 1)
reset_filters reset_filters
...@@ -67,7 +67,7 @@ feature 'Issues filter reset button', feature: true, js: true do ...@@ -67,7 +67,7 @@ feature 'Issues filter reset button', feature: true, js: true do
context 'when all filters have been applied' do context 'when all filters have been applied' do
it 'resets all filters' 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) expect(page).to have_css('.issue', count: 0)
reset_filters reset_filters
......
...@@ -7,8 +7,8 @@ describe IssuesFinder do ...@@ -7,8 +7,8 @@ describe IssuesFinder do
let(:project2) { create(:empty_project) } let(:project2) { create(:empty_project) }
let(:milestone) { create(:milestone, project: project1) } let(:milestone) { create(:milestone, project: project1) }
let(:label) { create(:label, project: project2) } let(:label) { create(:label, project: project2) }
let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone) } let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') }
let(:issue2) { create(:issue, author: user, assignee: user, project: project2) } let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') }
let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) } let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2) }
let!(:label_link) { create(:label_link, label: label, target: issue2) } let!(:label_link) { create(:label_link, label: label, target: issue2) }
...@@ -127,6 +127,22 @@ describe IssuesFinder do ...@@ -127,6 +127,22 @@ describe IssuesFinder do
end end
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 context 'when the user is unauthorized' do
let(:search_user) { nil } let(:search_user) { nil }
......
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