From de784ac10516ec1e1c93d164f3d99b2ff09e5889 Mon Sep 17 00:00:00 2001 From: Hiroyuki Sato Date: Sat, 9 Mar 2019 12:22:58 +0000 Subject: Filter merge requests by target branch --- .../add_extra_tokens_for_merge_requests.js | 12 ++++++ .../filtered_search/available_dropdown_mappings.js | 31 +++++++++++++++ .../filtered_search/filtered_search_manager.js | 13 ++----- .../filtered_search_visual_tokens.js | 12 +++++- app/assets/stylesheets/framework/filters.scss | 11 +++++- app/controllers/autocomplete_controller.rb | 9 ++++- app/finders/merge_requests_finder.rb | 2 +- app/models/merge_request.rb | 16 ++++++++ app/views/shared/issuable/_search_bar.html.haml | 5 +++ .../filter-merge-requests-by-target-branch.yml | 5 +++ config/routes.rb | 1 + spec/controllers/autocomplete_controller_spec.rb | 31 +++++++++++++++ .../user_filters_by_target_branch_spec.rb | 45 ++++++++++++++++++++++ spec/finders/merge_requests_finder_spec.rb | 2 +- .../filtered_search_visual_tokens_spec.js | 4 ++ .../helpers/filtered_search_spec_helper.js | 2 +- spec/models/merge_request_spec.rb | 19 +++++++++ 17 files changed, 204 insertions(+), 16 deletions(-) create mode 100644 changelogs/unreleased/filter-merge-requests-by-target-branch.yml create mode 100644 spec/features/merge_requests/user_filters_by_target_branch_spec.rb diff --git a/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js b/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js index 54ea936252e..0b24d9fc920 100644 --- a/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js +++ b/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js @@ -13,4 +13,16 @@ export default IssuableTokenKeys => { IssuableTokenKeys.tokenKeys.push(wipToken); IssuableTokenKeys.tokenKeysWithAlternative.push(wipToken); + + const targetBranchToken = { + key: 'target-branch', + type: 'string', + param: '', + symbol: '', + icon: 'arrow-right', + tag: 'branch', + }; + + IssuableTokenKeys.tokenKeys.push(targetBranchToken); + IssuableTokenKeys.tokenKeysWithAlternative.push(targetBranchToken); }; diff --git a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js index e2f9c03ee65..be867a3838d 100644 --- a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js +++ b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js @@ -5,6 +5,7 @@ import DropdownEmoji from './dropdown_emoji'; import NullDropdown from './null_dropdown'; import DropdownAjaxFilter from './dropdown_ajax_filter'; import DropdownUtils from './dropdown_utils'; +import { mergeUrlParams } from '../lib/utils/url_utility'; export default class AvailableDropdownMappings { constructor(container, baseEndpoint, groupsOnly, includeAncestorGroups, includeDescendantGroups) { @@ -13,6 +14,7 @@ export default class AvailableDropdownMappings { this.groupsOnly = groupsOnly; this.includeAncestorGroups = includeAncestorGroups; this.includeDescendantGroups = includeDescendantGroups; + this.filteredSearchInput = this.container.querySelector('.filtered-search'); } getAllowedMappings(supportedTokens) { @@ -102,6 +104,15 @@ export default class AvailableDropdownMappings { }, element: this.container.querySelector('#js-dropdown-runner-tag'), }, + 'target-branch': { + reference: null, + gl: DropdownNonUser, + extraArguments: { + endpoint: this.getMergeRequestTargetBranchesEndpoint(), + symbol: '', + }, + element: this.container.querySelector('#js-dropdown-target-branch'), + }, }; } @@ -130,4 +141,24 @@ export default class AvailableDropdownMappings { getRunnerTagsEndpoint() { return `${this.baseEndpoint}/admin/runners/tag_list.json`; } + + getMergeRequestTargetBranchesEndpoint() { + const endpoint = `${gon.relative_url_root || + ''}/autocomplete/merge_request_target_branches.json`; + + const params = { + group_id: this.getGroupId(), + project_id: this.getProjectId(), + }; + + return mergeUrlParams(params, endpoint); + } + + getGroupId() { + return this.filteredSearchInput.getAttribute('data-group-id') || ''; + } + + getProjectId() { + return this.filteredSearchInput.getAttribute('data-project-id') || ''; + } } diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index 33c82778c79..0c2e87521d9 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -504,14 +504,7 @@ export default class FilteredSearchManager { const match = this.filteredSearchTokenKeys.searchByKeyParam(keyParam); if (match) { - // Use lastIndexOf because the token key is allowed to contain underscore - // e.g. 'my_reaction' is the token key of 'my_reaction_emoji' - const lastIndexOf = keyParam.lastIndexOf('_'); - let sanitizedKey = lastIndexOf !== -1 ? keyParam.slice(0, lastIndexOf) : keyParam; - // Replace underscore with hyphen in the sanitizedkey. - // e.g. 'my_reaction' => 'my-reaction' - sanitizedKey = sanitizedKey.replace('_', '-'); - const { symbol } = match; + const { key, symbol } = match; let quotationsToUse = ''; if (sanitizedValue.indexOf(' ') !== -1) { @@ -520,10 +513,10 @@ export default class FilteredSearchManager { } hasFilteredSearch = true; - const canEdit = this.canEdit && this.canEdit(sanitizedKey, sanitizedValue); + const canEdit = this.canEdit && this.canEdit(key, sanitizedValue); const { uppercaseTokenName, capitalizeTokenValue } = match; FilteredSearchVisualTokens.addFilterVisualToken( - sanitizedKey, + key, `${symbol}${quotationsToUse}${sanitizedValue}${quotationsToUse}`, { canEdit, diff --git a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js index 7746908714e..315cd6f64da 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -69,11 +69,21 @@ export default class FilteredSearchVisualTokens { } static addVisualTokenElement(name, value, options = {}) { - const { isSearchTerm = false, canEdit, uppercaseTokenName, capitalizeTokenValue } = options; + const { + isSearchTerm = false, + canEdit, + uppercaseTokenName, + capitalizeTokenValue, + tokenClass = `search-token-${name.toLowerCase()}`, + } = options; const li = document.createElement('li'); li.classList.add('js-visual-token'); li.classList.add(isSearchTerm ? 'filtered-search-term' : 'filtered-search-token'); + if (!isSearchTerm) { + li.classList.add(tokenClass); + } + if (value) { li.innerHTML = FilteredSearchVisualTokens.createVisualTokenElementHTML({ canEdit, diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index e5b529ae11d..5bcfd5d1322 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -108,6 +108,8 @@ } .value-container { + display: flex; + align-items: center; background-color: $white-normal; color: $filter-value-text-color; border-radius: 0 2px 2px 0; @@ -121,7 +123,7 @@ .remove-token { display: inline-block; - padding-left: 4px; + padding-left: 8px; padding-right: 0; .fa-close { @@ -412,3 +414,10 @@ padding: 8px 16px; text-align: center; } + +.search-token-target-branch { + .value { + font-family: $monospace-font; + font-size: 13px; + } +} diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index 0d5c8657c9e..091327931c2 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class AutocompleteController < ApplicationController - skip_before_action :authenticate_user!, only: [:users, :award_emojis] + skip_before_action :authenticate_user!, only: [:users, :award_emojis, :merge_request_target_branches] def users project = Autocomplete::ProjectFinder @@ -38,4 +38,11 @@ class AutocompleteController < ApplicationController def award_emojis render json: AwardedEmojiFinder.new(current_user).execute end + + def merge_request_target_branches + merge_requests = MergeRequestsFinder.new(current_user, params).execute + target_branches = merge_requests.recent_target_branches + + render json: target_branches.map { |target_branch| { title: target_branch } } + end end diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 93bee3f1488..84689ff5dc7 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -29,7 +29,7 @@ # class MergeRequestsFinder < IssuableFinder def self.scalar_params - @scalar_params ||= super + [:wip] + @scalar_params ||= super + [:wip, :target_branch] end def klass diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index acf80addc6a..af8cb37bfb6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -203,6 +203,22 @@ class MergeRequest < ActiveRecord::Base '!' end + # Returns the top 100 target branches + # + # The returned value is a Array containing branch names + # sort by updated_at of merge request: + # + # ['master', 'develop', 'production'] + # + # limit - The maximum number of target branch to return. + def self.recent_target_branches(limit: 100) + group(:target_branch) + .select(:target_branch) + .reorder('MAX(merge_requests.updated_at) DESC') + .limit(limit) + .pluck(:target_branch) + end + def rebase_in_progress? strong_memoize(:rebase_in_progress) do # The source project can be deleted diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index f43be304e6b..2bcfcb6fa7c 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -137,6 +137,11 @@ %li.filter-dropdown-item{ data: { value: 'no', capitalize: true } } %button.btn.btn-link{ type: 'button' } = _('No') + #js-dropdown-target-branch.filtered-search-input-dropdown-menu.dropdown-menu + %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } + %li.filter-dropdown-item + %button.btn.btn-link.js-data-value.monospace + {{title}} = render_if_exists 'shared/issuable/filter_weight', type: type diff --git a/changelogs/unreleased/filter-merge-requests-by-target-branch.yml b/changelogs/unreleased/filter-merge-requests-by-target-branch.yml new file mode 100644 index 00000000000..d0aba631c96 --- /dev/null +++ b/changelogs/unreleased/filter-merge-requests-by-target-branch.yml @@ -0,0 +1,5 @@ +--- +title: Add target branch filter to merge requests search bar +merge_request: 24380 +author: Hiroyuki Sato +type: added diff --git a/config/routes.rb b/config/routes.rb index 53c6225eff1..bbf00208545 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -43,6 +43,7 @@ Rails.application.routes.draw do get '/autocomplete/users/:id' => 'autocomplete#user' get '/autocomplete/projects' => 'autocomplete#projects' get '/autocomplete/award_emojis' => 'autocomplete#award_emojis' + get '/autocomplete/merge_request_target_branches' => 'autocomplete#merge_request_target_branches' # Search get 'search' => 'search#show' diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 4458a7223bf..d8b75c5151e 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -371,5 +371,36 @@ describe AutocompleteController do expect(json_response[3]).to match('name' => 'thumbsdown') end end + + context 'Get merge_request_target_branches' do + let(:user2) { create(:user) } + let!(:merge_request1) { create(:merge_request, source_project: project, target_branch: 'feature') } + + context 'unauthorized user' do + it 'returns empty json' do + get :merge_request_target_branches + + expect(json_response).to be_empty + end + end + + context 'sign in as user without any accesible merge requests' do + it 'returns empty json' do + sign_in(user2) + get :merge_request_target_branches + + expect(json_response).to be_empty + end + end + + context 'sign in as user with a accesible merge request' do + it 'returns json' do + sign_in(user) + get :merge_request_target_branches + + expect(json_response).to contain_exactly({ 'title' => 'feature' }) + end + end + end end end diff --git a/spec/features/merge_requests/user_filters_by_target_branch_spec.rb b/spec/features/merge_requests/user_filters_by_target_branch_spec.rb new file mode 100644 index 00000000000..ffbdacc68f6 --- /dev/null +++ b/spec/features/merge_requests/user_filters_by_target_branch_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +describe 'Merge Requests > User filters by target branch', :js do + include FilteredSearchHelpers + + let!(:project) { create(:project, :public, :repository) } + let!(:user) { project.creator } + let!(:mr1) { create(:merge_request, source_project: project, target_project: project, source_branch: 'feature', target_branch: 'master') } + let!(:mr2) { create(:merge_request, source_project: project, target_project: project, source_branch: 'feature', target_branch: 'merged-target') } + + before do + sign_in(user) + visit project_merge_requests_path(project) + end + + context 'filtering by target-branch:master' do + it 'applies the filter' do + input_filtered_search('target-branch:master') + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).to have_content mr1.title + expect(page).not_to have_content mr2.title + end + end + + context 'filtering by target-branch:merged-target' do + it 'applies the filter' do + input_filtered_search('target-branch:merged-target') + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + expect(page).not_to have_content mr1.title + expect(page).to have_content mr2.title + end + end + + context 'filtering by target-branch:feature' do + it 'applies the filter' do + input_filtered_search('target-branch:feature') + + expect(page).to have_issuable_counts(open: 0, closed: 0, all: 0) + expect(page).not_to have_content mr1.title + expect(page).not_to have_content mr2.title + end + end +end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 503b88fcbad..f1178b07eec 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -36,7 +36,7 @@ describe MergeRequestsFinder do let(:project5) { create_project_without_n_plus_1(group: subgroup) } let(:project6) { create_project_without_n_plus_1(group: subgroup) } - let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } + let!(:merge_request1) { create(:merge_request, author: user, source_project: project2, target_project: project1, target_branch: 'merged-target') } let!(:merge_request2) { create(:merge_request, :conflict, author: user, source_project: project2, target_project: project1, state: 'closed') } let!(:merge_request3) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project2, state: 'locked', title: 'thing WIP thing') } let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3, title: 'WIP thing') } diff --git a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js index f3dc35552d5..a72ea6ab547 100644 --- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js @@ -293,6 +293,7 @@ describe('Filtered Search Visual Tokens', () => { subject.addVisualTokenElement('milestone'); const token = tokensContainer.querySelector('.js-visual-token'); + expect(token.classList.contains('search-token-milestone')).toEqual(true); expect(token.classList.contains('filtered-search-token')).toEqual(true); expect(token.querySelector('.name').innerText).toEqual('milestone'); expect(token.querySelector('.value')).toEqual(null); @@ -302,6 +303,7 @@ describe('Filtered Search Visual Tokens', () => { subject.addVisualTokenElement('label', 'Frontend'); const token = tokensContainer.querySelector('.js-visual-token'); + expect(token.classList.contains('search-token-label')).toEqual(true); expect(token.classList.contains('filtered-search-token')).toEqual(true); expect(token.querySelector('.name').innerText).toEqual('label'); expect(token.querySelector('.value').innerText).toEqual('Frontend'); @@ -317,10 +319,12 @@ describe('Filtered Search Visual Tokens', () => { const labelToken = tokens[0]; const assigneeToken = tokens[1]; + expect(labelToken.classList.contains('search-token-label')).toEqual(true); expect(labelToken.classList.contains('filtered-search-token')).toEqual(true); expect(labelToken.querySelector('.name').innerText).toEqual('label'); expect(labelToken.querySelector('.value').innerText).toEqual('Frontend'); + expect(assigneeToken.classList.contains('search-token-assignee')).toEqual(true); expect(assigneeToken.classList.contains('filtered-search-token')).toEqual(true); expect(assigneeToken.querySelector('.name').innerText).toEqual('assignee'); expect(assigneeToken.querySelector('.value').innerText).toEqual('@root'); diff --git a/spec/javascripts/helpers/filtered_search_spec_helper.js b/spec/javascripts/helpers/filtered_search_spec_helper.js index 8933dd5def4..fd06bb1f324 100644 --- a/spec/javascripts/helpers/filtered_search_spec_helper.js +++ b/spec/javascripts/helpers/filtered_search_spec_helper.js @@ -5,7 +5,7 @@ export default class FilteredSearchSpecHelper { static createFilterVisualToken(name, value, isSelected = false) { const li = document.createElement('li'); - li.classList.add('js-visual-token', 'filtered-search-token'); + li.classList.add('js-visual-token', 'filtered-search-token', `search-token-${name}`); li.innerHTML = `
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 07cb4c9c1e3..a35d3f14df8 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -270,6 +270,25 @@ describe MergeRequest do end end + describe '.recent_target_branches' do + let(:project) { create(:project) } + let!(:merge_request1) { create(:merge_request, :opened, source_project: project, target_branch: 'feature') } + let!(:merge_request2) { create(:merge_request, :closed, source_project: project, target_branch: 'merge-test') } + let!(:merge_request3) { create(:merge_request, :opened, source_project: project, target_branch: 'fix') } + let!(:merge_request4) { create(:merge_request, :closed, source_project: project, target_branch: 'feature') } + + before do + merge_request1.update_columns(updated_at: 1.day.since) + merge_request2.update_columns(updated_at: 2.days.since) + merge_request3.update_columns(updated_at: 3.days.since) + merge_request4.update_columns(updated_at: 4.days.since) + end + + it 'returns target branches sort by updated at desc' do + expect(described_class.recent_target_branches).to match_array(['feature', 'merge-test', 'fix']) + end + end + describe '#target_branch_sha' do let(:project) { create(:project, :repository) } -- cgit v1.2.1