diff options
author | Tim Zallmann <tzallmann@gitlab.com> | 2018-10-03 08:15:00 +0000 |
---|---|---|
committer | Tim Zallmann <tzallmann@gitlab.com> | 2018-10-03 08:15:00 +0000 |
commit | 09f38f8d13b19c48e52ba6771cd26f218feddab2 (patch) | |
tree | ce34ad0daceb0e9aed942cfbb84425b39b66ae08 | |
parent | 82ece8ad4584278ce437270470b54ff9b42c653b (diff) | |
parent | 38f3d59fd0b2dd4eef5c94512ea6216a0e5d56b5 (diff) | |
download | gitlab-ce-09f38f8d13b19c48e52ba6771cd26f218feddab2.tar.gz |
Merge branch 'ccr/wip_filter' into 'master'
#13650 added wip search functionality and tests
See merge request gitlab-org/gitlab-ce!18119
22 files changed, 300 insertions, 56 deletions
diff --git a/app/assets/javascripts/filtered_search/dropdown_hint.js b/app/assets/javascripts/filtered_search/dropdown_hint.js index 8aecf9725e6..c568f4e4ebf 100644 --- a/app/assets/javascripts/filtered_search/dropdown_hint.js +++ b/app/assets/javascripts/filtered_search/dropdown_hint.js @@ -51,7 +51,11 @@ export default class DropdownHint extends FilteredSearchDropdown { FilteredSearchVisualTokens.addSearchVisualToken(searchTerms.join(' ')); } - FilteredSearchDropdownManager.addWordToInput(token.replace(':', ''), '', false, this.container); + const key = token.replace(':', ''); + const { uppercaseTokenName } = this.tokenKeys.searchByKey(key); + FilteredSearchDropdownManager.addWordToInput(key, '', false, { + uppercaseTokenName, + }); } this.dismissDropdown(); this.dispatchInputEvent(); diff --git a/app/assets/javascripts/filtered_search/dropdown_utils.js b/app/assets/javascripts/filtered_search/dropdown_utils.js index 27fff488603..6da6ca10008 100644 --- a/app/assets/javascripts/filtered_search/dropdown_utils.js +++ b/app/assets/javascripts/filtered_search/dropdown_utils.js @@ -143,7 +143,9 @@ export default class DropdownUtils { const dataValue = selected.getAttribute('data-value'); if (dataValue) { - FilteredSearchDropdownManager.addWordToInput(filter, dataValue, true); + FilteredSearchDropdownManager.addWordToInput(filter, dataValue, true, { + capitalizeTokenValue: selected.hasAttribute('data-capitalize'), + }); } // Return boolean based on whether it was set diff --git a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js index 207616b9de2..cd3d532c958 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js @@ -91,6 +91,11 @@ export default class FilteredSearchDropdownManager { gl: DropdownEmoji, element: this.container.querySelector('#js-dropdown-my-reaction'), }, + wip: { + reference: null, + gl: DropdownNonUser, + element: this.container.querySelector('#js-dropdown-wip'), + }, status: { reference: null, gl: NullDropdown, @@ -136,10 +141,16 @@ export default class FilteredSearchDropdownManager { return endpoint; } - static addWordToInput(tokenName, tokenValue = '', clicked = false) { + static addWordToInput(tokenName, tokenValue = '', clicked = false, options = {}) { + const { + uppercaseTokenName = false, + capitalizeTokenValue = false, + } = options; const input = FilteredSearchContainer.container.querySelector('.filtered-search'); - - FilteredSearchVisualTokens.addFilterVisualToken(tokenName, tokenValue); + FilteredSearchVisualTokens.addFilterVisualToken(tokenName, tokenValue, { + uppercaseTokenName, + capitalizeTokenValue, + }); input.value = ''; if (clicked) { diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index d25f6f95b22..54533ebb70d 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -405,7 +405,10 @@ export default class FilteredSearchManager { if (isLastVisualTokenValid) { tokens.forEach((t) => { input.value = input.value.replace(`${t.key}:${t.symbol}${t.value}`, ''); - FilteredSearchVisualTokens.addFilterVisualToken(t.key, `${t.symbol}${t.value}`); + FilteredSearchVisualTokens.addFilterVisualToken(t.key, `${t.symbol}${t.value}`, { + uppercaseTokenName: this.filteredSearchTokenKeys.shouldUppercaseTokenName(t.key), + capitalizeTokenValue: this.filteredSearchTokenKeys.shouldCapitalizeTokenValue(t.key), + }); }); const fragments = searchToken.split(':'); @@ -421,7 +424,10 @@ export default class FilteredSearchManager { FilteredSearchVisualTokens.addSearchVisualToken(searchTerms); } - FilteredSearchVisualTokens.addFilterVisualToken(tokenKey); + FilteredSearchVisualTokens.addFilterVisualToken(tokenKey, null, { + uppercaseTokenName: this.filteredSearchTokenKeys.shouldUppercaseTokenName(tokenKey), + capitalizeTokenValue: this.filteredSearchTokenKeys.shouldCapitalizeTokenValue(tokenKey), + }); input.value = input.value.replace(`${tokenKey}:`, ''); } } else { @@ -429,7 +435,10 @@ export default class FilteredSearchManager { const valueCompletedRegex = /([~%@]{0,1}".+")|([~%@]{0,1}'.+')|^((?![~%@]')(?![~%@]")(?!')(?!")).*/g; if (searchToken.match(valueCompletedRegex) && input.value[input.value.length - 1] === ' ') { - FilteredSearchVisualTokens.addFilterVisualToken(searchToken); + const tokenKey = FilteredSearchVisualTokens.getLastTokenPartial(); + FilteredSearchVisualTokens.addFilterVisualToken(searchToken, null, { + capitalizeTokenValue: this.filteredSearchTokenKeys.shouldCapitalizeTokenValue(tokenKey), + }); // Trim the last space as seen in the if statement above input.value = input.value.replace(searchToken, '').trim(); @@ -480,7 +489,7 @@ export default class FilteredSearchManager { FilteredSearchVisualTokens.addFilterVisualToken( condition.tokenKey, condition.value, - canEdit, + { canEdit }, ); } else { // Sanitize value since URL converts spaces into + @@ -506,10 +515,15 @@ export default class FilteredSearchManager { hasFilteredSearch = true; const canEdit = this.canEdit && this.canEdit(sanitizedKey, sanitizedValue); + const { uppercaseTokenName, capitalizeTokenValue } = match; FilteredSearchVisualTokens.addFilterVisualToken( sanitizedKey, `${symbol}${quotationsToUse}${sanitizedValue}${quotationsToUse}`, - canEdit, + { + canEdit, + uppercaseTokenName, + capitalizeTokenValue, + }, ); } else if (!match && keyParam === 'assignee_id') { const id = parseInt(value, 10); @@ -517,7 +531,7 @@ export default class FilteredSearchManager { hasFilteredSearch = true; const tokenName = 'assignee'; const canEdit = this.canEdit && this.canEdit(tokenName); - FilteredSearchVisualTokens.addFilterVisualToken(tokenName, `@${usernameParams[id]}`, canEdit); + FilteredSearchVisualTokens.addFilterVisualToken(tokenName, `@${usernameParams[id]}`, { canEdit }); } } else if (!match && keyParam === 'author_id') { const id = parseInt(value, 10); @@ -525,7 +539,7 @@ export default class FilteredSearchManager { hasFilteredSearch = true; const tokenName = 'author'; const canEdit = this.canEdit && this.canEdit(tokenName); - FilteredSearchVisualTokens.addFilterVisualToken(tokenName, `@${usernameParams[id]}`, canEdit); + FilteredSearchVisualTokens.addFilterVisualToken(tokenName, `@${usernameParams[id]}`, { canEdit }); } } else if (!match && keyParam === 'search') { hasFilteredSearch = true; @@ -561,15 +575,17 @@ export default class FilteredSearchManager { this.saveCurrentSearchQuery(); - const { tokens, searchToken } - = this.tokenizer.processTokens(searchQuery, this.filteredSearchTokenKeys.getKeys()); + const tokenKeys = this.filteredSearchTokenKeys.getKeys(); + const { tokens, searchToken } = this.tokenizer.processTokens(searchQuery, tokenKeys); const currentState = state || getParameterByName('state') || 'opened'; paths.push(`state=${currentState}`); tokens.forEach((token) => { const condition = this.filteredSearchTokenKeys .searchByConditionKeyValue(token.key, token.value.toLowerCase()); - const { param } = this.filteredSearchTokenKeys.searchByKey(token.key) || {}; + const tokenConfig = this.filteredSearchTokenKeys.searchByKey(token.key) || {}; + const { param } = tokenConfig; + // Replace hyphen with underscore to use as request parameter // e.g. 'my-reaction' => 'my_reaction' const underscoredKey = token.key.replace('-', '_'); @@ -581,6 +597,10 @@ export default class FilteredSearchManager { } else { let tokenValue = token.value; + if (tokenConfig.lowercaseValueOnSubmit) { + tokenValue = tokenValue.toLowerCase(); + } + if ((tokenValue[0] === '\'' && tokenValue[tokenValue.length - 1] === '\'') || (tokenValue[0] === '"' && tokenValue[tokenValue.length - 1] === '"')) { tokenValue = tokenValue.slice(1, tokenValue.length - 1); diff --git a/app/assets/javascripts/filtered_search/filtered_search_token_keys.js b/app/assets/javascripts/filtered_search/filtered_search_token_keys.js index 5d131b396a0..a09ad3e4758 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/filtered_search_token_keys.js @@ -23,6 +23,16 @@ export default class FilteredSearchTokenKeys { return this.conditions; } + shouldUppercaseTokenName(tokenKey) { + const token = this.searchByKey(tokenKey.toLowerCase()); + return token && token.uppercaseTokenName; + } + + shouldCapitalizeTokenValue(tokenKey) { + const token = this.searchByKey(tokenKey.toLowerCase()); + return token && token.capitalizeTokenValue; + } + searchByKey(key) { return this.tokenKeys.find(tokenKey => tokenKey.key === key) || null; } @@ -55,4 +65,21 @@ export default class FilteredSearchTokenKeys { return this.conditions .find(condition => condition.tokenKey === key && condition.value === value) || null; } + + addExtraTokensForMergeRequests() { + const wipToken = { + key: 'wip', + type: 'string', + param: '', + symbol: '', + icon: 'admin', + tag: 'Yes or No', + lowercaseValueOnSubmit: true, + uppercaseTokenName: true, + capitalizeTokenValue: true, + }; + + this.tokenKeys.push(wipToken); + this.tokenKeysWithAlternative.push(wipToken); + } } 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 56fe1ab4e90..0854c1822fb 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -55,12 +55,18 @@ export default class FilteredSearchVisualTokens { } } - static createVisualTokenElementHTML(canEdit = true) { + static createVisualTokenElementHTML(options = {}) { + const { + canEdit = true, + uppercaseTokenName = false, + capitalizeTokenValue = false, + } = options; + return ` <div class="${canEdit ? 'selectable' : 'hidden'}" role="button"> - <div class="name"></div> + <div class="${uppercaseTokenName ? 'text-uppercase' : ''} name"></div> <div class="value-container"> - <div class="value"></div> + <div class="${capitalizeTokenValue ? 'text-capitalize' : ''} value"></div> <div class="remove-token" role="button"> <i class="fa fa-close"></i> </div> @@ -182,16 +188,26 @@ export default class FilteredSearchVisualTokens { } } - static addVisualTokenElement(name, value, isSearchTerm, canEdit) { + static addVisualTokenElement(name, value, options = {}) { + const { + isSearchTerm = false, + canEdit, + uppercaseTokenName, + capitalizeTokenValue, + } = options; const li = document.createElement('li'); li.classList.add('js-visual-token'); li.classList.add(isSearchTerm ? 'filtered-search-term' : 'filtered-search-token'); if (value) { - li.innerHTML = FilteredSearchVisualTokens.createVisualTokenElementHTML(canEdit); + li.innerHTML = FilteredSearchVisualTokens.createVisualTokenElementHTML({ + canEdit, + uppercaseTokenName, + capitalizeTokenValue, + }); FilteredSearchVisualTokens.renderVisualTokenValue(li, name, value); } else { - li.innerHTML = '<div class="name"></div>'; + li.innerHTML = `<div class="${uppercaseTokenName ? 'text-uppercase' : ''} name"></div>`; } li.querySelector('.name').innerText = name; @@ -212,20 +228,32 @@ export default class FilteredSearchVisualTokens { } } - static addFilterVisualToken(tokenName, tokenValue, canEdit) { + static addFilterVisualToken(tokenName, tokenValue, { + canEdit, + uppercaseTokenName = false, + capitalizeTokenValue = false, + } = {}) { const { lastVisualToken, isLastVisualTokenValid } = FilteredSearchVisualTokens.getLastVisualTokenBeforeInput(); const { addVisualTokenElement } = FilteredSearchVisualTokens; if (isLastVisualTokenValid) { - addVisualTokenElement(tokenName, tokenValue, false, canEdit); + addVisualTokenElement(tokenName, tokenValue, { + canEdit, + uppercaseTokenName, + capitalizeTokenValue, + }); } else { const previousTokenName = lastVisualToken.querySelector('.name').innerText; const tokensContainer = FilteredSearchContainer.container.querySelector('.tokens-container'); tokensContainer.removeChild(lastVisualToken); const value = tokenValue || tokenName; - addVisualTokenElement(previousTokenName, value, false, canEdit); + addVisualTokenElement(previousTokenName, value, { + canEdit, + uppercaseTokenName, + capitalizeTokenValue, + }); } } @@ -235,7 +263,9 @@ export default class FilteredSearchVisualTokens { if (lastVisualToken && lastVisualToken.classList.contains('filtered-search-term')) { lastVisualToken.querySelector('.name').innerText += ` ${searchTerm}`; } else { - FilteredSearchVisualTokens.addVisualTokenElement(searchTerm, null, true); + FilteredSearchVisualTokens.addVisualTokenElement(searchTerm, null, { + isSearchTerm: true, + }); } } @@ -306,7 +336,9 @@ export default class FilteredSearchVisualTokens { let value; if (token.classList.contains('filtered-search-token')) { - FilteredSearchVisualTokens.addFilterVisualToken(nameElement.innerText); + FilteredSearchVisualTokens.addFilterVisualToken(nameElement.innerText, null, { + uppercaseTokenName: nameElement.classList.contains('text-uppercase'), + }); const valueContainerElement = token.querySelector('.value-container'); value = valueContainerElement.dataset.originalValue; diff --git a/app/assets/javascripts/pages/groups/merge_requests/index.js b/app/assets/javascripts/pages/groups/merge_requests/index.js index b798a254459..339ce67438a 100644 --- a/app/assets/javascripts/pages/groups/merge_requests/index.js +++ b/app/assets/javascripts/pages/groups/merge_requests/index.js @@ -4,6 +4,8 @@ import IssuableFilteredSearchTokenKeys from '~/filtered_search/issuable_filtered import { FILTERED_SEARCH } from '~/pages/constants'; document.addEventListener('DOMContentLoaded', () => { + IssuableFilteredSearchTokenKeys.addExtraTokensForMergeRequests(); + initFilteredSearch({ page: FILTERED_SEARCH.MERGE_REQUESTS, isGroupDecendent: true, diff --git a/app/assets/javascripts/pages/projects/merge_requests/index/index.js b/app/assets/javascripts/pages/projects/merge_requests/index/index.js index 3647048a872..ec39db12e74 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/index/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/index/index.js @@ -7,10 +7,13 @@ import { FILTERED_SEARCH } from '~/pages/constants'; import { ISSUABLE_INDEX } from '~/pages/projects/constants'; document.addEventListener('DOMContentLoaded', () => { + IssuableFilteredSearchTokenKeys.addExtraTokensForMergeRequests(); + initFilteredSearch({ page: FILTERED_SEARCH.MERGE_REQUESTS, filteredSearchTokenKeys: IssuableFilteredSearchTokenKeys, }); + new IssuableIndex(ISSUABLE_INDEX.MERGE_REQUEST); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new new UsersSelect(); // eslint-disable-line no-new diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index b698a3c7b09..50c051c3aa1 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -27,13 +27,17 @@ # updated_before: datetime # class MergeRequestsFinder < IssuableFinder + def self.scalar_params + @scalar_params ||= super + [:wip] + end + def klass MergeRequest end def filter_items(_items) items = by_source_branch(super) - + items = by_wip(items) by_target_branch(items) end @@ -61,5 +65,24 @@ class MergeRequestsFinder < IssuableFinder items.where(target_branch: target_branch) end - # rubocop: enable CodeReuse/ActiveRecord + + def item_project_ids(items) + items&.reorder(nil)&.select(:target_project_id) + end + + def by_wip(items) + if params[:wip] == 'yes' + items.where(wip_match(items.arel_table)) + elsif params[:wip] == 'no' + items.where.not(wip_match(items.arel_table)) + else + items + end + end + + def wip_match(table) + table[:title].matches('WIP:%') + .or(table[:title].matches('WIP %')) + .or(table[:title].matches('[WIP]%')) + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0481a4a3d28..6559f94a696 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -261,7 +261,7 @@ class MergeRequest < ActiveRecord::Base end end - WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze + WIP_REGEX = /\A*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze def self.work_in_progress?(title) !!(title =~ WIP_REGEX) diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index 659e03fd67d..c4d177361e7 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -33,13 +33,13 @@ #js-dropdown-hint.filtered-search-input-dropdown-menu.dropdown-menu.hint-dropdown %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { action: 'submit' } } - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } = sprite_icon('search') %span Press Enter or click to search %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } %li.filter-dropdown-item - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } -# Encapsulate static class name `{{icon}}` inside #{} to bypass -# haml lint's ClassAttributeWithStaticValue %svg @@ -60,7 +60,7 @@ #js-dropdown-assignee.filtered-search-input-dropdown-menu.dropdown-menu %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { value: 'none' } } - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } No Assignee %li.divider.droplab-item-ignore - if current_user @@ -73,38 +73,46 @@ #js-dropdown-milestone.filtered-search-input-dropdown-menu.dropdown-menu %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { value: 'none' } } - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } No Milestone %li.filter-dropdown-item{ data: { value: 'upcoming' } } - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } Upcoming %li.filter-dropdown-item{ 'data-value' => 'started' } - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } Started %li.divider.droplab-item-ignore %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } %li.filter-dropdown-item - %button.btn.btn-link.js-data-value + %button.btn.btn-link.js-data-value{ type: 'button' } {{title}} #js-dropdown-label.filtered-search-input-dropdown-menu.dropdown-menu %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { value: 'none' } } - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } No Label %li.divider.droplab-item-ignore %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } %li.filter-dropdown-item - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } %span.dropdown-label-box{ style: 'background: {{color}}' } %span.label-title.js-data-value {{title}} #js-dropdown-my-reaction.filtered-search-input-dropdown-menu.dropdown-menu %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } %li.filter-dropdown-item - %button.btn.btn-link + %button.btn.btn-link{ type: 'button' } %gl-emoji %span.js-data-value.prepend-left-10 {{name}} + #js-dropdown-wip.filtered-search-input-dropdown-menu.dropdown-menu + %ul.filter-dropdown{ data: { dropdown: true } } + %li.filter-dropdown-item{ data: { value: 'yes', capitalize: true } } + %button.btn.btn-link{ type: 'button' } + = _('Yes') + %li.filter-dropdown-item{ data: { value: 'no', capitalize: true } } + %button.btn.btn-link{ type: 'button' } + = _('No') = render_if_exists 'shared/issuable/filter_weight', type: type diff --git a/changelogs/unreleased/ccr-wip_filter.yml b/changelogs/unreleased/ccr-wip_filter.yml new file mode 100644 index 00000000000..07d85ec02ae --- /dev/null +++ b/changelogs/unreleased/ccr-wip_filter.yml @@ -0,0 +1,5 @@ +--- +title: Added search functionality for Work In Progress (WIP) merge requests +merge_request: 18119 +author: Chantal Rollison +type: added diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 4c099581f07..b37e7698ab4 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -47,6 +47,7 @@ Parameters: | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | | `search` | string | no | Search merge requests against their `title` and `description` | +| `wip` | string | no | Filter merge requests against their `wip` status. `yes` to return *only* WIP merge requests, `no` to return *non* WIP merge requests | ```json [ diff --git a/doc/user/project/merge_requests/img/filter_wip_merge_requests.png b/doc/user/project/merge_requests/img/filter_wip_merge_requests.png Binary files differnew file mode 100644 index 00000000000..40913718385 --- /dev/null +++ b/doc/user/project/merge_requests/img/filter_wip_merge_requests.png diff --git a/doc/user/project/merge_requests/work_in_progress_merge_requests.md b/doc/user/project/merge_requests/work_in_progress_merge_requests.md index f01da06fa6e..66ac7740157 100644 --- a/doc/user/project/merge_requests/work_in_progress_merge_requests.md +++ b/doc/user/project/merge_requests/work_in_progress_merge_requests.md @@ -7,7 +7,7 @@ have been marked a **Work In Progress**. ![Blocked Accept Button](img/wip_blocked_accept_button.png) To mark a merge request a Work In Progress, simply start its title with `[WIP]` -or `WIP:`. As an alternative, you're also able to do it by sending a commit +or `WIP:`. As an alternative, you're also able to do it by sending a commit with its title starting with `wip` or `WIP` to the merge request's source branch. ![Mark as WIP](img/wip_mark_as_wip.png) @@ -15,4 +15,11 @@ with its title starting with `wip` or `WIP` to the merge request's source branch To allow a Work In Progress merge request to be accepted again when it's ready, simply remove the `WIP` prefix. -![Unark as WIP](img/wip_unmark_as_wip.png) +![Unmark as WIP](img/wip_unmark_as_wip.png) + +## Filtering merge requests with WIP Status + +To filter merge requests with the `WIP` status, you can type `wip` +and select the value for your filter from the merge request search input. + +![Filter WIP MRs](img/filter_wip_merge_requests.png) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 764905ca00f..440d94ae186 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -33,7 +33,6 @@ module API # rubocop: disable CodeReuse/ActiveRecord def find_merge_requests(args = {}) args = declared_params.merge(args) - args[:milestone_title] = args.delete(:milestone) args[:label_name] = args.delete(:labels) args[:scope] = args[:scope].underscore if args[:scope] @@ -97,6 +96,7 @@ module API optional :source_branch, type: String, desc: 'Return merge requests with the given source branch' optional :target_branch, type: String, desc: 'Return merge requests with the given target branch' optional :search, type: String, desc: 'Search merge requests for text present in the title or description' + optional :wip, type: String, values: %w[yes no], desc: 'Search merge requests for WIP in the title' use :pagination end end diff --git a/spec/features/issues/filtered_search/dropdown_hint_spec.rb b/spec/features/issues/filtered_search/dropdown_hint_spec.rb index b99c5a7f4e3..0e296ab2109 100644 --- a/spec/features/issues/filtered_search/dropdown_hint_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_hint_spec.rb @@ -15,6 +15,7 @@ describe 'Dropdown hint', :js do before do project.add_maintainer(user) create(:issue, project: project) + create(:merge_request, source_project: project, target_project: project) end context 'when user not logged in' do @@ -224,4 +225,21 @@ describe 'Dropdown hint', :js do end end end + + context 'merge request page' do + before do + sign_in(user) + visit project_merge_requests_path(project) + filtered_search.click + end + + it 'shows the WIP menu item and opens the WIP options dropdown' do + click_hint('wip') + + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-wip', visible: true) + expect_tokens([{ name: 'wip' }]) + expect_filtered_search_input_empty + end + end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index 35d0eeda8f6..33d01697c75 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -16,12 +16,18 @@ describe MergeRequestsFinder do p end let(:project4) { create(:project, :public, group: subgroup) } + let(:project5) { create(:project, :public, group: subgroup) } + let(:project6) { create(:project, :public, group: subgroup) } let!(:merge_request1) { create(:merge_request, :simple, author: user, source_project: project2, target_project: project1) } 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') } - let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3) } - let!(:merge_request5) { create(:merge_request, :simple, author: user, source_project: project4, target_project: project4) } + 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') } + let!(:merge_request5) { create(:merge_request, :simple, author: user, source_project: project4, target_project: project4, title: '[WIP]') } + let!(:merge_request6) { create(:merge_request, :simple, author: user, source_project: project5, target_project: project5, title: 'WIP: thing') } + let!(:merge_request7) { create(:merge_request, :simple, author: user, source_project: project6, target_project: project6, title: 'wip thing') } + let!(:merge_request8) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project1, title: '[wip] thing') } + let!(:merge_request9) { create(:merge_request, :simple, author: user, source_project: project1, target_project: project2, title: 'wip: thing') } before do project1.add_maintainer(user) @@ -29,19 +35,21 @@ describe MergeRequestsFinder do project3.add_developer(user) project2.add_developer(user2) project4.add_developer(user) + project5.add_developer(user) + project6.add_developer(user) end describe "#execute" do it 'filters by scope' do params = { scope: 'authored', state: 'opened' } merge_requests = described_class.new(user, params).execute - expect(merge_requests.size).to eq(3) + expect(merge_requests.size).to eq(7) end it 'filters by project' do params = { project_id: project1.id, scope: 'authored', state: 'opened' } merge_requests = described_class.new(user, params).execute - expect(merge_requests.size).to eq(1) + expect(merge_requests.size).to eq(2) end it 'filters by group' do @@ -49,7 +57,7 @@ describe MergeRequestsFinder do merge_requests = described_class.new(user, params).execute - expect(merge_requests.size).to eq(2) + expect(merge_requests.size).to eq(3) end it 'filters by group including subgroups', :nested_groups do @@ -57,13 +65,13 @@ describe MergeRequestsFinder do merge_requests = described_class.new(user, params).execute - expect(merge_requests.size).to eq(3) + expect(merge_requests.size).to eq(6) end it 'filters by non_archived' do params = { non_archived: true } merge_requests = described_class.new(user, params).execute - expect(merge_requests.size).to eq(4) + expect(merge_requests.size).to eq(8) end it 'filters by iid' do @@ -98,6 +106,36 @@ describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request3) end + it 'filters by wip' do + params = { wip: 'yes' } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request4, merge_request5, merge_request6, merge_request7, merge_request8, merge_request9) + end + + it 'filters by not wip' do + params = { wip: 'no' } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3) + end + + it 'returns all items if no valid wip param exists' do + params = { wip: '' } + + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request4, merge_request5, merge_request6, merge_request7, merge_request8, merge_request9) + end + + it 'adds wip to scalar params' do + scalar_params = described_class.scalar_params + + expect(scalar_params).to include(:wip, :assignee_id) + end + context 'filtering by group milestone' do let!(:group) { create(:group, :public) } let(:group_milestone) { create(:milestone, group: group) } @@ -207,7 +245,7 @@ describe MergeRequestsFinder do it 'returns the number of rows for the default state' do finder = described_class.new(user) - expect(finder.row_count).to eq(3) + expect(finder.row_count).to eq(7) end it 'returns the number of rows for a given state' do diff --git a/spec/javascripts/filtered_search/dropdown_utils_spec.js b/spec/javascripts/filtered_search/dropdown_utils_spec.js index 8792e99d461..68bbbf838da 100644 --- a/spec/javascripts/filtered_search/dropdown_utils_spec.js +++ b/spec/javascripts/filtered_search/dropdown_utils_spec.js @@ -288,13 +288,13 @@ describe('Dropdown Utils', () => { describe('setDataValueIfSelected', () => { beforeEach(() => { - spyOn(FilteredSearchDropdownManager, 'addWordToInput') - .and.callFake(() => {}); + spyOn(FilteredSearchDropdownManager, 'addWordToInput').and.callFake(() => {}); }); it('calls addWordToInput when dataValue exists', () => { const selected = { getAttribute: () => 'value', + hasAttribute: () => false, }; DropdownUtils.setDataValueIfSelected(null, selected); @@ -304,6 +304,7 @@ describe('Dropdown Utils', () => { it('returns true when dataValue exists', () => { const selected = { getAttribute: () => 'value', + hasAttribute: () => false, }; const result = DropdownUtils.setDataValueIfSelected(null, selected); 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 756a654765b..53a6d1d62b0 100644 --- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js @@ -240,13 +240,17 @@ describe('Filtered Search Visual Tokens', () => { beforeEach(() => { setFixtures(` <div class="test-area"> - ${subject.createVisualTokenElementHTML()} + ${subject.createVisualTokenElementHTML('custom-token')} </div> `); tokenElement = document.querySelector('.test-area').firstElementChild; }); + it('should add class name to token element', () => { + expect(document.querySelector('.test-area .custom-token')).toBeDefined(); + }); + it('contains name div', () => { expect(tokenElement.querySelector('.name')).toEqual(jasmine.anything()); }); @@ -280,7 +284,7 @@ describe('Filtered Search Visual Tokens', () => { describe('addVisualTokenElement', () => { it('renders search visual tokens', () => { - subject.addVisualTokenElement('search term', null, true); + subject.addVisualTokenElement('search term', null, { isSearchTerm: true }); const token = tokensContainer.querySelector('.js-visual-token'); expect(token.classList.contains('filtered-search-term')).toEqual(true); diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 48f4e53b93e..666d7e69f89 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -746,7 +746,7 @@ describe MergeRequest do end describe "#wipless_title" do - ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix| + ['WIP ', 'WIP:', 'WIP: ', '[WIP]', '[WIP] ', '[WIP] WIP [WIP] WIP: WIP '].each do |wip_prefix| it "removes the '#{wip_prefix}' prefix" do wipless_title = subject.title subject.title = "#{wip_prefix}#{subject.title}" diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index e987eee6e91..07d19e3ad29 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -81,6 +81,35 @@ describe API::MergeRequests do let(:user2) { create(:user) } it 'returns an array of all merge requests except unauthorized ones' do + get api('/merge_requests', user), scope: :all + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |mr| mr['id'] }) + .to contain_exactly(merge_request.id, merge_request_closed.id, merge_request_merged.id, merge_request_locked.id, merge_request2.id) + end + + it "returns an array of no merge_requests when wip=yes" do + get api("/merge_requests", user), wip: 'yes' + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(0) + end + + it "returns an array of no merge_requests when wip=no" do + get api("/merge_requests", user), wip: 'no' + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |mr| mr['id'] }) + .to contain_exactly(merge_request.id, merge_request_closed.id, merge_request_merged.id, merge_request_locked.id, merge_request2.id) + end + + it 'does not return unauthorized merge requests' do private_project = create(:project, :private) merge_request3 = create(:merge_request, :simple, source_project: private_project, target_project: private_project, source_branch: 'other-branch') @@ -244,6 +273,15 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(404) end + it "returns an array of no merge_requests when wip=yes" do + get api("/projects/#{project.id}/merge_requests", user), wip: 'yes' + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(0) + end + it 'returns merge_request by "iids" array' do get api(endpoint_path, user), iids: [merge_request.iid, merge_request_closed.iid] |