diff options
19 files changed, 132 insertions, 60 deletions
diff --git a/app/assets/javascripts/issuable.js.es6 b/app/assets/javascripts/issuable.js.es6 index 53faaa38a0c..81d89a48227 100644 --- a/app/assets/javascripts/issuable.js.es6 +++ b/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) { diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index b5e79099e39..4a447735fa7 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/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 diff --git a/app/controllers/concerns/issues_action.rb b/app/controllers/concerns/issues_action.rb index b89fb94be6e..eced9d9d678 100644 --- a/app/controllers/concerns/issues_action.rb +++ b/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 } diff --git a/app/controllers/concerns/merge_requests_action.rb b/app/controllers/concerns/merge_requests_action.rb index a1b0eee37f9..729763169e2 100644 --- a/app/controllers/concerns/merge_requests_action.rb +++ b/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 diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index de02e28e384..19b8b1576c4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/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 } diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0288ee87717..e972376df4c 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/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 diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 60996b181f2..8f9ef8f725c 100644 --- a/app/finders/issuable_finder.rb +++ b/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 diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index c2befa5a5b3..be00a219205 100644 --- a/app/finders/issues_finder.rb +++ b/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 diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index b258216d0d4..3b254e7d9d5 100644 --- a/app/finders/merge_requests_finder.rb +++ b/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 diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5f3765cad0d..ed41bf04fc0 100644 --- a/app/helpers/application_helper.rb +++ b/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 += " " diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 93c4d5c3d30..cf26197f7d7 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/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 diff --git a/app/views/shared/issuable/_nav.html.haml b/app/views/shared/issuable/_nav.html.haml index 1d9b09a5ef1..fb592c2b1e2 100644 --- a/app/views/shared/issuable/_nav.html.haml +++ b/app/views/shared/issuable/_nav.html.haml @@ -1,25 +1,27 @@ %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)} diff --git a/app/views/shared/issuable/_search_form.html.haml b/app/views/shared/issuable/_search_form.html.haml index 186963b32b8..2c89217cadd 100644 --- a/app/views/shared/issuable/_search_form.html.haml +++ b/app/views/shared/issuable/_search_form.html.haml @@ -1,2 +1,2 @@ -= 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 } diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index e21f76d00d9..ed7241679ee 100644 --- a/features/steps/project/issues/issues.rb +++ b/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 diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index df17b5626c6..4a67cf06fba 100644 --- a/features/steps/project/merge_requests.rb +++ b/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 diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb index 3fb1cb37544..fc914022a59 100644 --- a/spec/features/dashboard_issues_spec.rb +++ b/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 diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 69fda27cc61..72f39e2fbca 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/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 diff --git a/spec/features/issues/reset_filters_spec.rb b/spec/features/issues/reset_filters_spec.rb index 41f218eaa8b..f4d0f13c3d5 100644 --- a/spec/features/issues/reset_filters_spec.rb +++ b/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 diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index ec8809e6926..40bccb8e50b 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/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 } |