From 832d7fa3ce3e97113dd783600de57b8d2276d8a1 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 13 Apr 2016 13:11:44 +0100 Subject: [PATCH 01/10] Issuable filtering improvements This improves the filtering of issues and merge requests by creating a single file that encapsulates all the filtering. Previously this was done with a file for issues and a file for merge requests. Created the ability for the text search to be done alongside other filterables. Previously because this was outside the filterable form, this wasn't possible and would instead do either filter dropdown or text filter - not both. --- app/assets/javascripts/dispatcher.js.coffee | 1 - app/assets/javascripts/issuable.js.coffee | 23 +++++++++++++++++-- .../javascripts/lib/url_utility.js.coffee | 11 ++++++++- app/assets/stylesheets/framework/nav.scss | 2 +- app/assets/stylesheets/pages/issues.scss | 5 ---- app/helpers/application_helper.rb | 17 ++++++++++---- app/views/shared/issuable/_filter.html.haml | 6 +++-- .../shared/issuable/_search_form.html.haml | 6 ----- 8 files changed, 48 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index f91aa3c5ad..5dc1f2cbd6 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -16,7 +16,6 @@ class Dispatcher shortcut_handler = null switch page when 'projects:issues:index' - Issues.init() Issuable.init() shortcut_handler = new ShortcutsNavigation() when 'projects:issues:show' diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index afffed63ac..b999500d0e 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -1,7 +1,10 @@ +issuable_created = false @Issuable = init: -> - Issuable.initTemplates() - Issuable.initSearch() + if not issuable_created + issuable_created = true + Issuable.initTemplates() + Issuable.initSearch() initTemplates: -> Issuable.labelRow = _.template( @@ -64,6 +67,7 @@ $('#filter_issue_search').val($('#issue_search').val()) + updateStateFilters: -> stateFilters = $('.issues-state-filters') newParams = {} @@ -82,3 +86,18 @@ else newUrl = gl.utils.mergeUrlParams(newParams, initialUrl) $(this).attr 'href', newUrl + + checkChanged: -> + checked_issues = $('.selected_issue:checked') + if checked_issues.length > 0 + ids = [] + $.each checked_issues, (index, value) -> + ids.push $(value).data('id') + + $('#update_issues_ids').val ids + $('.issues-other-filters').hide() + $('.issues_bulk_update').show() + else + $('#update_issues_ids').val [] + $('.issues_bulk_update').hide() + $('.issues-other-filters').show() diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee index 6a00932c02..3d51744614 100644 --- a/app/assets/javascripts/lib/url_utility.js.coffee +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -26,10 +26,19 @@ newUrl = decodeURIComponent(url) for paramName, paramValue of params pattern = new RegExp "\\b(#{paramName}=).*?(&|$)" - if url.search(pattern) >= 0 + if !paramValue? + newUrl = newUrl.replace pattern, '' + else if url.search(pattern) >= 0 newUrl = newUrl.replace pattern, "$1#{paramValue}$2" else newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" + + # Remove a trailing ampersand + lastChar = newUrl[newUrl.length - 1] + + if lastChar is '&' + newUrl = newUrl.slice 0, -1 + newUrl # removes parameter query string from url. returns the modified url diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index a81fcb1c6b..89bff150c5 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -119,7 +119,7 @@ } input { - height: 34px; + height: 35px; display: inline-block; position: relative; top: 2px; diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index fc9db97132..59f72cd8b6 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -40,11 +40,6 @@ } } -.issue-search-form { - margin: 0; - height: 24px; -} - form.edit-issue { margin: 0; } diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3e0074da39..da4dee443a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -263,6 +263,7 @@ module ApplicationHelper assignee_id: params[:assignee_id], author_id: params[:author_id], sort: params[:sort], + issue_search: params[:issue_search] } options = exist_opts.merge(options) @@ -273,15 +274,21 @@ module ApplicationHelper end end + params = options.compact.to_param + path = request.path - path << "?#{options.to_param}" - if add_label - if params[:label_name].present? and params[:label_name].respond_to?('any?') - params[:label_name].each do |label| - path << "&label_name[]=#{label}" + + if params != nil + path << "?#{options.to_param}" + if add_label + if params[:label_name].present? and params[:label_name].respond_to?('any?') + params[:label_name].each do |label| + path << "&label_name[]=#{label}" + end end end end + path end diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 9474462cbd..c4aa57e0ac 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -1,6 +1,8 @@ .issues-filters - .issues-details-filters.row-content-block.second-block - = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name]), method: :get, class: 'filter-form' do + .issues-details-filters.gray-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] - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project) .check-all-holder = check_box_tag "check_all_issues", nil, false, diff --git a/app/views/shared/issuable/_search_form.html.haml b/app/views/shared/issuable/_search_form.html.haml index afad48499b..186963b32b 100644 --- a/app/views/shared/issuable/_search_form.html.haml +++ b/app/views/shared/issuable/_search_form.html.haml @@ -1,8 +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 } - = hidden_field_tag :state, params['state'] - = hidden_field_tag :scope, params['scope'] - = hidden_field_tag :assignee_id, params['assignee_id'] - = hidden_field_tag :author_id, params['author_id'] - = hidden_field_tag :milestone_id, params['milestone_id'] - = hidden_field_tag :label_id, params['label_id'] From 3a76cc5c411433d39b8ac5e9af037a839767152c Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 13 Apr 2016 16:41:52 +0100 Subject: [PATCH 02/10] Fixed Ruby issues --- app/helpers/application_helper.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index da4dee443a..e6f11f95ec 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -279,7 +279,7 @@ module ApplicationHelper path = request.path if params != nil - path << "?#{options.to_param}" + path << "?#{params}" if add_label if params[:label_name].present? and params[:label_name].respond_to?('any?') params[:label_name].each do |label| @@ -288,7 +288,6 @@ module ApplicationHelper end end end - path end From 471d7a5b32c9be9d925ae262165c0917605a846b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 15 Apr 2016 08:54:28 +0100 Subject: [PATCH 03/10] Added tests --- spec/features/issues/filter_issues_spec.rb | 106 +++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 192e361937..9f4e6e5665 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -154,4 +154,110 @@ describe 'Filter issues', feature: true do end end end + + describe 'filter issues by text' do + before do + create(:issue, title: "Bug", project: project) + + create(:label, project: project, title: 'bug') + milestone = create(:milestone, title: "8", project: project) + + issue = create(:issue, + title: "Bug 2", + project: project, + milestone: milestone, + author: user, + assignee: user + ) + issue.labels << project.labels.find_by(title: 'bug') + + visit namespace_project_issues_path(project.namespace, project) + end + + context 'only text', js: true do + it 'should filter issues by searched text' do + fill_in 'issue_search', with: 'Bug' + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 2) + end + end + + it 'should not show any issues' do + fill_in 'issue_search', with: 'testing' + + page.within '.issues-list' do + expect(page).to_not have_selector('.issue') + end + end + + it 'should filter by text and label' do + fill_in 'issue_search', with: 'Bug' + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 2) + end + + click_button 'Label' + page.within '.labels-filter' do + click_link 'bug' + end + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 1) + end + end + + it 'should filter by text and milestone' do + fill_in 'issue_search', with: 'Bug' + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 2) + end + + click_button 'Milestone' + page.within '.milestone-filter' do + click_link '8' + end + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 1) + end + end + + it 'should filter by text and assignee' do + fill_in 'issue_search', with: 'Bug' + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 2) + end + + click_button 'Assignee' + page.within '.dropdown-menu-assignee' do + click_link user.name + end + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 1) + end + end + + it 'should filter by text and author' do + fill_in 'issue_search', with: 'Bug' + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 2) + end + + click_button 'Author' + page.within '.dropdown-menu-author' do + click_link user.name + end + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 1) + end + end + end + end end From fdc949073ceabac7f573c1c1e0fcf06e4603ce11 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 15 Apr 2016 09:24:19 +0100 Subject: [PATCH 04/10] Fixed issue with not being able to sort and filter --- app/assets/javascripts/issuable.js.coffee | 2 +- app/views/shared/_sort_dropdown.html.haml | 13 ++++++-- spec/features/issues/filter_issues_spec.rb | 35 ++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index b999500d0e..b40fa32294 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -69,7 +69,7 @@ issuable_created = false updateStateFilters: -> - stateFilters = $('.issues-state-filters') + stateFilters = $('.issues-state-filters, .dropdown-menu-sort') newParams = {} paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] diff --git a/app/views/shared/_sort_dropdown.html.haml b/app/views/shared/_sort_dropdown.html.haml index d327bd0a96..154d9e3085 100644 --- a/app/views/shared/_sort_dropdown.html.haml +++ b/app/views/shared/_sort_dropdown.html.haml @@ -6,26 +6,35 @@ - else = sort_title_recently_created %b.caret - %ul.dropdown-menu.dropdown-menu-align-right + %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-sort %li = link_to page_filter_path(sort: sort_value_recently_created) do = sort_title_recently_created + %li = link_to page_filter_path(sort: sort_value_oldest_created) do = sort_title_oldest_created + %li = link_to page_filter_path(sort: sort_value_recently_updated) do = sort_title_recently_updated + %li = link_to page_filter_path(sort: sort_value_oldest_updated) do = sort_title_oldest_updated + %li = link_to page_filter_path(sort: sort_value_milestone_soon) do = sort_title_milestone_soon + %li = link_to page_filter_path(sort: sort_value_milestone_later) do = sort_title_milestone_later - - if controller.controller_name == 'issues' || controller.action_name == 'issues' + - if controller.controller_name == 'issues' || controller.action_name == 'issues' + %li = link_to page_filter_path(sort: sort_value_due_date_soon) do = sort_title_due_date_soon + %li = link_to page_filter_path(sort: sort_value_due_date_later) do = sort_title_due_date_later + %li = link_to page_filter_path(sort: sort_value_upvotes) do = sort_title_upvotes + %li = link_to page_filter_path(sort: sort_value_downvotes) do = sort_title_downvotes diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 9f4e6e5665..c479e43b01 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -190,7 +190,9 @@ describe 'Filter issues', feature: true do expect(page).to_not have_selector('.issue') end end + end + context 'text and dropdown options', js: true do it 'should filter by text and label' do fill_in 'issue_search', with: 'Bug' @@ -260,4 +262,37 @@ describe 'Filter issues', feature: true do end end end + + describe 'filter issues and sort', js: true do + before do + label = create(:label, project: project, title: 'bug') + bug_one = create(:issue, title: "Frontend", project: project) + bug_two = create(:issue, title: "Bug 2", project: project) + + bug_one.labels << project.labels.find_by(title: 'bug') + bug_two.labels << project.labels.find_by(title: 'bug') + + visit namespace_project_issues_path(project.namespace, project) + end + + it 'should be able to filter and sort issues' do + click_button 'Label' + page.within '.labels-filter' do + click_link 'bug' + end + + page.within '.issues-list' do + expect(page).to have_selector('.issue', count: 2) + end + + click_button 'Last created' + page.within '.dropdown-menu-sort' do + click_link 'Oldest created' + end + + page.within '.issues-list' do + expect(first('.issue')).to have_content('Frontend') + end + end + end end From 1a086745fc9a3e1ad74a7156f7f8bbf826a46b83 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 15 Apr 2016 15:18:55 +0100 Subject: [PATCH 05/10] Fixed tests --- app/helpers/application_helper.rb | 4 ++-- app/views/shared/_sort_dropdown.html.haml | 11 +---------- spec/features/issues/filter_issues_spec.rb | 5 ++--- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index e6f11f95ec..27fae71621 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -274,12 +274,12 @@ module ApplicationHelper end end - params = options.compact.to_param + params = options.compact path = request.path if params != nil - path << "?#{params}" + path << "?#{params.to_param}" if add_label if params[:label_name].present? and params[:label_name].respond_to?('any?') params[:label_name].each do |label| diff --git a/app/views/shared/_sort_dropdown.html.haml b/app/views/shared/_sort_dropdown.html.haml index 154d9e3085..1e0f075b30 100644 --- a/app/views/shared/_sort_dropdown.html.haml +++ b/app/views/shared/_sort_dropdown.html.haml @@ -10,31 +10,22 @@ %li = link_to page_filter_path(sort: sort_value_recently_created) do = sort_title_recently_created - %li = link_to page_filter_path(sort: sort_value_oldest_created) do = sort_title_oldest_created - %li = link_to page_filter_path(sort: sort_value_recently_updated) do = sort_title_recently_updated - %li = link_to page_filter_path(sort: sort_value_oldest_updated) do = sort_title_oldest_updated - %li = link_to page_filter_path(sort: sort_value_milestone_soon) do = sort_title_milestone_soon - %li = link_to page_filter_path(sort: sort_value_milestone_later) do = sort_title_milestone_later - - if controller.controller_name == 'issues' || controller.action_name == 'issues' - %li + - if controller.controller_name == 'issues' || controller.action_name == 'issues' = link_to page_filter_path(sort: sort_value_due_date_soon) do = sort_title_due_date_soon - %li = link_to page_filter_path(sort: sort_value_due_date_later) do = sort_title_due_date_later - %li = link_to page_filter_path(sort: sort_value_upvotes) do = sort_title_upvotes - %li = link_to page_filter_path(sort: sort_value_downvotes) do = sort_title_downvotes diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index c479e43b01..c7ffee7d09 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -167,8 +167,7 @@ describe 'Filter issues', feature: true do project: project, milestone: milestone, author: user, - assignee: user - ) + assignee: user) issue.labels << project.labels.find_by(title: 'bug') visit namespace_project_issues_path(project.namespace, project) @@ -265,7 +264,7 @@ describe 'Filter issues', feature: true do describe 'filter issues and sort', js: true do before do - label = create(:label, project: project, title: 'bug') + create(:label, project: project, title: 'bug') bug_one = create(:issue, title: "Frontend", project: project) bug_two = create(:issue, title: "Bug 2", project: project) From 7eb97ef4025f831fa5c922fb69661caf50aa990e Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 25 Apr 2016 13:52:14 +0100 Subject: [PATCH 06/10] Added issue_search parameter --- app/assets/javascripts/issuable.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index b40fa32294..eb785bf96e 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -71,7 +71,7 @@ issuable_created = false updateStateFilters: -> stateFilters = $('.issues-state-filters, .dropdown-menu-sort') newParams = {} - paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search'] + paramKeys = ['author_id', 'milestone_title', 'assignee_id', 'issue_search', 'issue_search'] for paramKey in paramKeys newParams[paramKey] = gl.utils.getParameterValues(paramKey)[0] or '' From 31b1dc3c31a4da59acd2deff2ded0d6d2aed8557 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 26 Apr 2016 11:10:39 +0100 Subject: [PATCH 07/10] Fixed failing rubocop tests Fixed issue with issuable checkboxs not being init'd --- app/assets/javascripts/issuable.js.coffee | 11 +++++-- app/assets/javascripts/issues.js.coffee | 38 ----------------------- app/helpers/application_helper.rb | 2 +- 3 files changed, 10 insertions(+), 41 deletions(-) delete mode 100644 app/assets/javascripts/issues.js.coffee diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index eb785bf96e..012c3e4801 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -5,6 +5,7 @@ issuable_created = false issuable_created = true Issuable.initTemplates() Issuable.initSearch() + Issuable.initChecks() initTemplates: -> Issuable.labelRow = _.template( @@ -62,11 +63,17 @@ issuable_created = false dataType: "json" reload: -> - if Issues.created - Issues.initChecks() + if Issuable.created + Issuable.initChecks() $('#filter_issue_search').val($('#issue_search').val()) + initChecks: -> + $('.check_all_issues').on 'click', -> + $('.selected_issue').prop('checked', @checked) + Issuable.checkChanged() + + $('.selected_issue').on 'change', Issuable.checkChanged updateStateFilters: -> stateFilters = $('.issues-state-filters, .dropdown-menu-sort') diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee deleted file mode 100644 index 3330e6c68a..0000000000 --- a/app/assets/javascripts/issues.js.coffee +++ /dev/null @@ -1,38 +0,0 @@ -@Issues = - init: -> - Issues.created = true - Issues.initChecks() - - $("body").on "ajax:success", ".close_issue, .reopen_issue", -> - t = $(this) - totalIssues = undefined - reopen = t.hasClass("reopen_issue") - $(".issue_counter").each -> - issue = $(this) - totalIssues = parseInt($(this).html(), 10) - if reopen and issue.closest(".main_menu").length - $(this).html totalIssues + 1 - else - $(this).html totalIssues - 1 - - initChecks: -> - $(".check_all_issues").click -> - $(".selected_issue").prop("checked", @checked) - Issues.checkChanged() - - $(".selected_issue").bind "change", Issues.checkChanged - - checkChanged: -> - checked_issues = $(".selected_issue:checked") - if checked_issues.length > 0 - ids = [] - $.each checked_issues, (index, value) -> - ids.push $(value).attr("data-id") - - $("#update_issues_ids").val ids - $(".issues-other-filters").hide() - $(".issues_bulk_update").show() - else - $("#update_issues_ids").val [] - $(".issues_bulk_update").hide() - $(".issues-other-filters").show() diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 27fae71621..59a04578c5 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -278,7 +278,7 @@ module ApplicationHelper path = request.path - if params != nil + if !params.nil? path << "?#{params.to_param}" if add_label if params[:label_name].present? and params[:label_name].respond_to?('any?') From 7df4b16b56f52eafccd080253b60a4cc15c66149 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 26 Apr 2016 13:41:17 +0100 Subject: [PATCH 08/10] Fixed issue with not being able to search text & filter --- app/assets/javascripts/issuable.js.coffee | 11 ++++++++++- app/views/shared/issuable/_filter.html.haml | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index 012c3e4801..4c4f67f510 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -23,7 +23,16 @@ issuable_created = false .on 'keyup', -> clearTimeout(@timer) @timer = setTimeout( -> - Issuable.filterResults $('#issue_search_form') + $search = $('#issue_search') + $form = $('.js-filter-form') + $input = $("input[name='#{$search.attr('name')}']", $form) + + if $input.length is 0 + $form.append "" + else + $input.val $search.val() + + Issuable.filterResults $form , 500) toggleLabelFilters: -> diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index c4aa57e0ac..323d563cd4 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -1,5 +1,5 @@ .issues-filters - .issues-details-filters.gray-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 - if params[:issue_search].present? = hidden_field_tag :issue_search, params[:issue_search] From fac08c3bcd7316794bcbcfb5cec32ac05cb104fe Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 20 May 2016 09:20:35 +0100 Subject: [PATCH 09/10] Fixed up JS comments --- app/assets/javascripts/issuable.js.coffee | 5 ++--- app/assets/javascripts/lib/url_utility.js.coffee | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index 4c4f67f510..6cb5d86fb9 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -106,9 +106,8 @@ issuable_created = false checkChanged: -> checked_issues = $('.selected_issue:checked') if checked_issues.length > 0 - ids = [] - $.each checked_issues, (index, value) -> - ids.push $(value).data('id') + ids = $.map checked_issues, (value) -> + $(value).data('id') $('#update_issues_ids').val ids $('.issues-other-filters').hide() diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee index 3d51744614..e8085e1c2e 100644 --- a/app/assets/javascripts/lib/url_utility.js.coffee +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -26,9 +26,9 @@ newUrl = decodeURIComponent(url) for paramName, paramValue of params pattern = new RegExp "\\b(#{paramName}=).*?(&|$)" - if !paramValue? + if not paramValue? newUrl = newUrl.replace pattern, '' - else if url.search(pattern) >= 0 + else if url.search(pattern) isnt -1 newUrl = newUrl.replace pattern, "$1#{paramValue}$2" else newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" From 5cca2d3bb171c260ad4b07f2a69962d3856c4d99 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 23 May 2016 09:49:13 +0100 Subject: [PATCH 10/10] Updated Ruby based on feedback --- app/assets/javascripts/issuable.js.coffee | 4 ++-- app/helpers/application_helper.rb | 17 ++++------------- spec/features/issues/filter_issues_spec.rb | 10 +++++----- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/issuable.js.coffee b/app/assets/javascripts/issuable.js.coffee index 6cb5d86fb9..6504e48110 100644 --- a/app/assets/javascripts/issuable.js.coffee +++ b/app/assets/javascripts/issuable.js.coffee @@ -1,7 +1,7 @@ issuable_created = false @Issuable = init: -> - if not issuable_created + unless issuable_created issuable_created = true Issuable.initTemplates() Issuable.initSearch() @@ -28,7 +28,7 @@ issuable_created = false $input = $("input[name='#{$search.attr('name')}']", $form) if $input.length is 0 - $form.append "" + $form.append "" else $input.val $search.val() diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 59a04578c5..a3fe1d0129 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -263,7 +263,8 @@ module ApplicationHelper assignee_id: params[:assignee_id], author_id: params[:author_id], sort: params[:sort], - issue_search: params[:issue_search] + issue_search: params[:issue_search], + label_name: params[:label_name] } options = exist_opts.merge(options) @@ -276,19 +277,9 @@ module ApplicationHelper params = options.compact - path = request.path + params.delete(:label_name) unless add_label - if !params.nil? - path << "?#{params.to_param}" - if add_label - if params[:label_name].present? and params[:label_name].respond_to?('any?') - params[:label_name].each do |label| - path << "&label_name[]=#{label}" - end - end - end - end - path + "#{request.path}?#{params.to_param}" end def outdated_browser? diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index c7ffee7d09..bfbd06a29e 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -159,7 +159,7 @@ describe 'Filter issues', feature: true do before do create(:issue, title: "Bug", project: project) - create(:label, project: project, title: 'bug') + bug_label = create(:label, project: project, title: 'bug') milestone = create(:milestone, title: "8", project: project) issue = create(:issue, @@ -168,7 +168,7 @@ describe 'Filter issues', feature: true do milestone: milestone, author: user, assignee: user) - issue.labels << project.labels.find_by(title: 'bug') + issue.labels << bug_label visit namespace_project_issues_path(project.namespace, project) end @@ -264,12 +264,12 @@ describe 'Filter issues', feature: true do describe 'filter issues and sort', js: true do before do - create(:label, project: project, title: 'bug') + bug_label = create(:label, project: project, title: 'bug') bug_one = create(:issue, title: "Frontend", project: project) bug_two = create(:issue, title: "Bug 2", project: project) - bug_one.labels << project.labels.find_by(title: 'bug') - bug_two.labels << project.labels.find_by(title: 'bug') + bug_one.labels << bug_label + bug_two.labels << bug_label visit namespace_project_issues_path(project.namespace, project) end