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 new file mode 100644 index 0000000000000000000000000000000000000000..54ea936252e4e79128ae6eae6093050308bc8f33 --- /dev/null +++ b/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js @@ -0,0 +1,16 @@ +export default IssuableTokenKeys => { + const wipToken = { + key: 'wip', + type: 'string', + param: '', + symbol: '', + icon: 'admin', + tag: 'Yes or No', + lowercaseValueOnSubmit: true, + uppercaseTokenName: true, + capitalizeTokenValue: true, + }; + + IssuableTokenKeys.tokenKeys.push(wipToken); + IssuableTokenKeys.tokenKeysWithAlternative.push(wipToken); +}; diff --git a/app/assets/javascripts/filtered_search/available_dropdown_mappings.js b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js new file mode 100644 index 0000000000000000000000000000000000000000..e2f9c03ee650ecf99386a7cdfb04376a3805a340 --- /dev/null +++ b/app/assets/javascripts/filtered_search/available_dropdown_mappings.js @@ -0,0 +1,133 @@ +import DropdownHint from './dropdown_hint'; +import DropdownUser from './dropdown_user'; +import DropdownNonUser from './dropdown_non_user'; +import DropdownEmoji from './dropdown_emoji'; +import NullDropdown from './null_dropdown'; +import DropdownAjaxFilter from './dropdown_ajax_filter'; +import DropdownUtils from './dropdown_utils'; + +export default class AvailableDropdownMappings { + constructor(container, baseEndpoint, groupsOnly, includeAncestorGroups, includeDescendantGroups) { + this.container = container; + this.baseEndpoint = baseEndpoint; + this.groupsOnly = groupsOnly; + this.includeAncestorGroups = includeAncestorGroups; + this.includeDescendantGroups = includeDescendantGroups; + } + + getAllowedMappings(supportedTokens) { + return this.buildMappings(supportedTokens, this.getMappings()); + } + + buildMappings(supportedTokens, availableMappings) { + const allowedMappings = { + hint: { + reference: null, + gl: DropdownHint, + element: this.container.querySelector('#js-dropdown-hint'), + }, + }; + + supportedTokens.forEach(type => { + if (availableMappings[type]) { + allowedMappings[type] = availableMappings[type]; + } + }); + + return allowedMappings; + } + + getMappings() { + return { + author: { + reference: null, + gl: DropdownUser, + element: this.container.querySelector('#js-dropdown-author'), + }, + assignee: { + reference: null, + gl: DropdownUser, + element: this.container.querySelector('#js-dropdown-assignee'), + }, + milestone: { + reference: null, + gl: DropdownNonUser, + extraArguments: { + endpoint: this.getMilestoneEndpoint(), + symbol: '%', + }, + element: this.container.querySelector('#js-dropdown-milestone'), + }, + label: { + reference: null, + gl: DropdownNonUser, + extraArguments: { + endpoint: this.getLabelsEndpoint(), + symbol: '~', + preprocessing: DropdownUtils.duplicateLabelPreprocessing, + }, + element: this.container.querySelector('#js-dropdown-label'), + }, + 'my-reaction': { + reference: null, + gl: DropdownEmoji, + element: this.container.querySelector('#js-dropdown-my-reaction'), + }, + wip: { + reference: null, + gl: DropdownNonUser, + element: this.container.querySelector('#js-dropdown-wip'), + }, + confidential: { + reference: null, + gl: DropdownNonUser, + element: this.container.querySelector('#js-dropdown-confidential'), + }, + status: { + reference: null, + gl: NullDropdown, + element: this.container.querySelector('#js-dropdown-admin-runner-status'), + }, + type: { + reference: null, + gl: NullDropdown, + element: this.container.querySelector('#js-dropdown-admin-runner-type'), + }, + tag: { + reference: null, + gl: DropdownAjaxFilter, + extraArguments: { + endpoint: this.getRunnerTagsEndpoint(), + symbol: '~', + }, + element: this.container.querySelector('#js-dropdown-runner-tag'), + }, + }; + } + + getMilestoneEndpoint() { + return `${this.baseEndpoint}/milestones.json`; + } + + getLabelsEndpoint() { + let endpoint = `${this.baseEndpoint}/labels.json?`; + + if (this.groupsOnly) { + endpoint = `${endpoint}only_group_labels=true&`; + } + + if (this.includeAncestorGroups) { + endpoint = `${endpoint}include_ancestor_groups=true&`; + } + + if (this.includeDescendantGroups) { + endpoint = `${endpoint}include_descendant_groups=true`; + } + + return endpoint; + } + + getRunnerTagsEndpoint() { + return `${this.baseEndpoint}/admin/runners/tag_list.json`; + } +} 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 0b21914a805986cd3bd0d4c81a3ffa1eaac68db0..cb0a84b490be50721ab9826822e8f13f240e5e62 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js @@ -1,15 +1,9 @@ +import AvailableDropdownMappings from 'ee_else_ce/filtered_search/available_dropdown_mappings'; import _ from 'underscore'; import DropLab from '~/droplab/drop_lab'; -import DropdownWeight from 'ee/filtered_search/dropdown_weight'; import FilteredSearchContainer from './container'; import FilteredSearchTokenKeys from './filtered_search_token_keys'; import DropdownUtils from './dropdown_utils'; -import DropdownHint from './dropdown_hint'; -import DropdownEmoji from './dropdown_emoji'; -import DropdownNonUser from './dropdown_non_user'; -import DropdownUser from './dropdown_user'; -import DropdownAjaxFilter from './dropdown_ajax_filter'; -import NullDropdown from './null_dropdown'; import FilteredSearchVisualTokens from './filtered_search_visual_tokens'; export default class FilteredSearchDropdownManager { @@ -51,127 +45,15 @@ export default class FilteredSearchDropdownManager { setupMapping() { const supportedTokens = this.filteredSearchTokenKeys.getKeys(); - const allowedMappings = { - hint: { - reference: null, - gl: DropdownHint, - element: this.container.querySelector('#js-dropdown-hint'), - }, - }; - const availableMappings = { - author: { - reference: null, - gl: DropdownUser, - element: this.container.querySelector('#js-dropdown-author'), - }, - assignee: { - reference: null, - gl: DropdownUser, - element: this.container.querySelector('#js-dropdown-assignee'), - }, - milestone: { - reference: null, - gl: DropdownNonUser, - extraArguments: { - endpoint: this.getMilestoneEndpoint(), - symbol: '%', - }, - element: this.container.querySelector('#js-dropdown-milestone'), - }, - label: { - reference: null, - gl: DropdownNonUser, - extraArguments: { - endpoint: this.getLabelsEndpoint(), - symbol: '~', - preprocessing: DropdownUtils.duplicateLabelPreprocessing, - }, - element: this.container.querySelector('#js-dropdown-label'), - }, - 'my-reaction': { - reference: null, - gl: DropdownEmoji, - element: this.container.querySelector('#js-dropdown-my-reaction'), - }, - wip: { - reference: null, - gl: DropdownNonUser, - element: this.container.querySelector('#js-dropdown-wip'), - }, - confidential: { - reference: null, - gl: DropdownNonUser, - element: this.container.querySelector('#js-dropdown-confidential'), - }, - status: { - reference: null, - gl: NullDropdown, - element: this.container.querySelector('#js-dropdown-admin-runner-status'), - }, - type: { - reference: null, - gl: NullDropdown, - element: this.container.querySelector('#js-dropdown-admin-runner-type'), - }, - tag: { - reference: null, - gl: DropdownAjaxFilter, - extraArguments: { - endpoint: this.getRunnerTagsEndpoint(), - symbol: '~', - }, - element: this.container.querySelector('#js-dropdown-runner-tag'), - }, - - // EE-only start - weight: { - reference: null, - gl: DropdownWeight, - element: this.container.querySelector('#js-dropdown-weight'), - }, - // EE-only end - }; - - supportedTokens.forEach(type => { - if (availableMappings[type]) { - allowedMappings[type] = availableMappings[type]; - } - }); - - this.mapping = allowedMappings; - } - - getMilestoneEndpoint() { - let endpoint = `${this.baseEndpoint}/milestones.json`; - - // EE-only - if (this.groupsOnly) { - endpoint = `${endpoint}?only_group_milestones=true`; - } - - return endpoint; - } - - getLabelsEndpoint() { - let endpoint = `${this.baseEndpoint}/labels.json?`; - - if (this.groupsOnly) { - endpoint = `${endpoint}only_group_labels=true&`; - } - - if (this.includeAncestorGroups) { - endpoint = `${endpoint}include_ancestor_groups=true&`; - } - - if (this.includeDescendantGroups) { - endpoint = `${endpoint}include_descendant_groups=true`; - } - - return endpoint; - } + const availableMappings = new AvailableDropdownMappings( + this.container, + this.baseEndpoint, + this.groupsOnly, + this.includeAncestorGroups, + this.includeDescendantGroups, + ); - getRunnerTagsEndpoint() { - return `${this.baseEndpoint}/admin/runners/tag_list.json`; + this.mapping = availableMappings.getAllowedMappings(supportedTokens); } static addWordToInput(tokenName, tokenValue = '', clicked = false, options = {}) { 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 48534bdf8150467328554d9e6ff64ad05d561706..11ed85504ec6624742191d8cf0f781d8ecd6957e 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_token_keys.js +++ b/app/assets/javascripts/filtered_search/filtered_search_token_keys.js @@ -88,21 +88,4 @@ export default class FilteredSearchTokenKeys { this.tokenKeys.push(confidentialToken); this.tokenKeysWithAlternative.push(confidentialToken); } - - 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 addf1ad94dff3d3ed559dfcab14e055dc0e54330..7746908714e2ab513fb40f3cde4ce88edd9dd1d2 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -1,10 +1,6 @@ -import _ from 'underscore'; -import AjaxCache from '~/lib/utils/ajax_cache'; +import VisualTokenValue from 'ee_else_ce/filtered_search/visual_token_value'; import { objectToQueryString } from '~/lib/utils/common_utils'; -import Flash from '../flash'; import FilteredSearchContainer from './container'; -import UsersCache from '../lib/utils/users_cache'; -import DropdownUtils from './dropdown_utils'; export default class FilteredSearchVisualTokens { static getLastVisualTokenBeforeInput() { @@ -20,21 +16,6 @@ export default class FilteredSearchVisualTokens { }; } - /** - * Returns a computed API endpoint - * and query string composed of values from endpointQueryParams - * @param {String} endpoint - * @param {String} endpointQueryParams - */ - static getEndpointWithQueryParams(endpoint, endpointQueryParams) { - if (!endpointQueryParams) { - return endpoint; - } - - const queryString = objectToQueryString(JSON.parse(endpointQueryParams)); - return `${endpoint}?${queryString}`; - } - static unselectTokens() { const otherTokens = FilteredSearchContainer.container.querySelectorAll( '.js-visual-token .selectable.selected', @@ -76,124 +57,15 @@ export default class FilteredSearchVisualTokens { `; } - static setTokenStyle(tokenContainer, backgroundColor, textColor) { - const token = tokenContainer; - - token.style.backgroundColor = backgroundColor; - token.style.color = textColor; - - if (textColor === '#FFFFFF') { - const removeToken = token.querySelector('.remove-token'); - removeToken.classList.add('inverted'); - } - - return token; - } - - static updateLabelTokenColor(tokenValueContainer, tokenValue) { - const filteredSearchInput = FilteredSearchContainer.container.querySelector('.filtered-search'); - const { baseEndpoint } = filteredSearchInput.dataset; - const labelsEndpoint = FilteredSearchVisualTokens.getEndpointWithQueryParams( - `${baseEndpoint}/labels.json`, - filteredSearchInput.dataset.endpointQueryParams, - ); - - return AjaxCache.retrieve(labelsEndpoint) - .then(labels => { - const matchingLabel = (labels || []).find( - label => `~${DropdownUtils.getEscapedText(label.title)}` === tokenValue, - ); - - if (!matchingLabel) { - return; - } - - FilteredSearchVisualTokens.setTokenStyle( - tokenValueContainer, - matchingLabel.color, - matchingLabel.text_color, - ); - }) - .catch(() => new Flash('An error occurred while fetching label colors.')); - } - - static updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) { - const username = tokenValue.replace(/^@/, ''); - return ( - UsersCache.retrieve(username) - .then(user => { - if (!user) { - return; - } - - /* eslint-disable no-param-reassign */ - tokenValueContainer.dataset.originalValue = tokenValue; - tokenValueElement.innerHTML = ` - <img class="avatar s20" src="${user.avatar_url}" alt=""> - ${_.escape(user.name)} - `; - /* eslint-enable no-param-reassign */ - }) - // ignore error and leave username in the search bar - .catch(() => {}) - ); - } - - static updateEmojiTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) { - const container = tokenValueContainer; - const element = tokenValueElement; - const value = tokenValue; - - return ( - import(/* webpackChunkName: 'emoji' */ '../emoji') - .then(Emoji => { - Emoji.initEmojiMap() - .then(() => { - if (!Emoji.isEmojiNameValid(value)) { - return; - } - - container.dataset.originalValue = value; - element.innerHTML = Emoji.glEmojiTag(value); - }) - // ignore error and leave emoji name in the search bar - .catch(err => { - throw err; - }); - }) - // ignore error and leave emoji name in the search bar - .catch(importError => { - throw importError; - }) - ); - } - static renderVisualTokenValue(parentElement, tokenName, tokenValue) { + const tokenType = tokenName.toLowerCase(); const tokenValueContainer = parentElement.querySelector('.value-container'); const tokenValueElement = tokenValueContainer.querySelector('.value'); tokenValueElement.innerText = tokenValue; - if (['none', 'any'].includes(tokenValue.toLowerCase())) { - return; - } + const visualTokenValue = new VisualTokenValue(tokenValue, tokenType); - const tokenType = tokenName.toLowerCase(); - - if (tokenType === 'label') { - FilteredSearchVisualTokens.updateLabelTokenColor(tokenValueContainer, tokenValue); - } else if (tokenType === 'author' || tokenType === 'assignee') { - FilteredSearchVisualTokens.updateUserTokenAppearance( - tokenValueContainer, - tokenValueElement, - tokenValue, - ); - } else if (tokenType === 'my-reaction') { - FilteredSearchVisualTokens.updateEmojiTokenAppearance( - tokenValueContainer, - tokenValueElement, - tokenValue, - ); - } + visualTokenValue.render(tokenValueContainer, tokenValueElement); } static addVisualTokenElement(name, value, options = {}) { @@ -328,6 +200,21 @@ export default class FilteredSearchVisualTokens { } } + /** + * Returns a computed API endpoint + * and query string composed of values from endpointQueryParams + * @param {String} endpoint + * @param {String} endpointQueryParams + */ + static getEndpointWithQueryParams(endpoint, endpointQueryParams) { + if (!endpointQueryParams) { + return endpoint; + } + + const queryString = objectToQueryString(JSON.parse(endpointQueryParams)); + return `${endpoint}?${queryString}`; + } + static editToken(token) { const input = FilteredSearchContainer.container.querySelector('.filtered-search'); diff --git a/app/assets/javascripts/filtered_search/visual_token_value.js b/app/assets/javascripts/filtered_search/visual_token_value.js new file mode 100644 index 0000000000000000000000000000000000000000..7f6f41c18f790aea9ab2cf821ac5193483e7a7bb --- /dev/null +++ b/app/assets/javascripts/filtered_search/visual_token_value.js @@ -0,0 +1,125 @@ +import _ from 'underscore'; +import FilteredSearchContainer from '~/filtered_search/container'; +import FilteredSearchVisualTokens from '~/filtered_search/filtered_search_visual_tokens'; +import AjaxCache from '~/lib/utils/ajax_cache'; +import DropdownUtils from '~/filtered_search/dropdown_utils'; +import Flash from '~/flash'; +import UsersCache from '~/lib/utils/users_cache'; + +export default class VisualTokenValue { + constructor(tokenValue, tokenType) { + this.tokenValue = tokenValue; + this.tokenType = tokenType; + } + + render(tokenValueContainer, tokenValueElement) { + const { tokenType } = this; + + if (['none', 'any'].includes(tokenType)) { + return; + } + + if (tokenType === 'label') { + this.updateLabelTokenColor(tokenValueContainer); + } else if (tokenType === 'author' || tokenType === 'assignee') { + this.updateUserTokenAppearance(tokenValueContainer, tokenValueElement); + } else if (tokenType === 'my-reaction') { + this.updateEmojiTokenAppearance(tokenValueContainer, tokenValueElement); + } + } + + updateUserTokenAppearance(tokenValueContainer, tokenValueElement) { + const { tokenValue } = this; + const username = this.tokenValue.replace(/^@/, ''); + + return ( + UsersCache.retrieve(username) + .then(user => { + if (!user) { + return; + } + + /* eslint-disable no-param-reassign */ + tokenValueContainer.dataset.originalValue = tokenValue; + tokenValueElement.innerHTML = ` + <img class="avatar s20" src="${user.avatar_url}" alt=""> + ${_.escape(user.name)} + `; + /* eslint-enable no-param-reassign */ + }) + // ignore error and leave username in the search bar + .catch(() => {}) + ); + } + + updateLabelTokenColor(tokenValueContainer) { + const { tokenValue } = this; + const filteredSearchInput = FilteredSearchContainer.container.querySelector('.filtered-search'); + const { baseEndpoint } = filteredSearchInput.dataset; + const labelsEndpoint = FilteredSearchVisualTokens.getEndpointWithQueryParams( + `${baseEndpoint}/labels.json`, + filteredSearchInput.dataset.endpointQueryParams, + ); + + return AjaxCache.retrieve(labelsEndpoint) + .then(labels => { + const matchingLabel = (labels || []).find( + label => `~${DropdownUtils.getEscapedText(label.title)}` === tokenValue, + ); + + if (!matchingLabel) { + return; + } + + VisualTokenValue.setTokenStyle( + tokenValueContainer, + matchingLabel.color, + matchingLabel.text_color, + ); + }) + .catch(() => new Flash('An error occurred while fetching label colors.')); + } + + static setTokenStyle(tokenValueContainer, backgroundColor, textColor) { + const token = tokenValueContainer; + + token.style.backgroundColor = backgroundColor; + token.style.color = textColor; + + if (textColor === '#FFFFFF') { + const removeToken = token.querySelector('.remove-token'); + removeToken.classList.add('inverted'); + } + + return token; + } + + updateEmojiTokenAppearance(tokenValueContainer, tokenValueElement) { + const container = tokenValueContainer; + const element = tokenValueElement; + const value = this.tokenValue; + + return ( + import(/* webpackChunkName: 'emoji' */ '../emoji') + .then(Emoji => { + Emoji.initEmojiMap() + .then(() => { + if (!Emoji.isEmojiNameValid(value)) { + return; + } + + container.dataset.originalValue = value; + element.innerHTML = Emoji.glEmojiTag(value); + }) + // ignore error and leave emoji name in the search bar + .catch(err => { + throw err; + }); + }) + // ignore error and leave emoji name in the search bar + .catch(importError => { + throw importError; + }) + ); + } +} diff --git a/app/assets/javascripts/pages/dashboard/merge_requests/index.js b/app/assets/javascripts/pages/dashboard/merge_requests/index.js index 260484726f3cef9491aa6093325428c841d494db..ff758fcb4fe14730d357a55923341e337688fe05 100644 --- a/app/assets/javascripts/pages/dashboard/merge_requests/index.js +++ b/app/assets/javascripts/pages/dashboard/merge_requests/index.js @@ -1,10 +1,11 @@ import projectSelect from '~/project_select'; import initFilteredSearch from '~/pages/search/init_filtered_search'; +import addExtraTokensForMergeRequests from 'ee_else_ce/filtered_search/add_extra_tokens_for_merge_requests'; import IssuableFilteredSearchTokenKeys from '~/filtered_search/issuable_filtered_search_token_keys'; import { FILTERED_SEARCH } from '~/pages/constants'; document.addEventListener('DOMContentLoaded', () => { - IssuableFilteredSearchTokenKeys.addExtraTokensForMergeRequests(); + addExtraTokensForMergeRequests(IssuableFilteredSearchTokenKeys); initFilteredSearch({ page: FILTERED_SEARCH.MERGE_REQUESTS, diff --git a/app/assets/javascripts/pages/groups/merge_requests/index.js b/app/assets/javascripts/pages/groups/merge_requests/index.js index 339ce67438a05ce5365fcfc1b79823c4c20b0dd0..12a26fd88fa92e456f0b6281843edb85b10d2c45 100644 --- a/app/assets/javascripts/pages/groups/merge_requests/index.js +++ b/app/assets/javascripts/pages/groups/merge_requests/index.js @@ -1,10 +1,11 @@ import projectSelect from '~/project_select'; import initFilteredSearch from '~/pages/search/init_filtered_search'; import IssuableFilteredSearchTokenKeys from '~/filtered_search/issuable_filtered_search_token_keys'; +import addExtraTokensForMergeRequests from 'ee_else_ce/filtered_search/add_extra_tokens_for_merge_requests'; import { FILTERED_SEARCH } from '~/pages/constants'; document.addEventListener('DOMContentLoaded', () => { - IssuableFilteredSearchTokenKeys.addExtraTokensForMergeRequests(); + addExtraTokensForMergeRequests(IssuableFilteredSearchTokenKeys); initFilteredSearch({ page: FILTERED_SEARCH.MERGE_REQUESTS, 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 ec39db12e74ad683d5291c47256c5edcae2cd398..0bcca22e40f54d84fbe650c63812a318f078d932 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/index/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/index/index.js @@ -2,12 +2,13 @@ import IssuableIndex from '~/issuable_index'; import ShortcutsNavigation from '~/behaviors/shortcuts/shortcuts_navigation'; import UsersSelect from '~/users_select'; import initFilteredSearch from '~/pages/search/init_filtered_search'; +import addExtraTokensForMergeRequests from 'ee_else_ce/filtered_search/add_extra_tokens_for_merge_requests'; import IssuableFilteredSearchTokenKeys from '~/filtered_search/issuable_filtered_search_token_keys'; import { FILTERED_SEARCH } from '~/pages/constants'; import { ISSUABLE_INDEX } from '~/pages/projects/constants'; document.addEventListener('DOMContentLoaded', () => { - IssuableFilteredSearchTokenKeys.addExtraTokensForMergeRequests(); + addExtraTokensForMergeRequests(IssuableFilteredSearchTokenKeys); initFilteredSearch({ page: FILTERED_SEARCH.MERGE_REQUESTS, diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 93bee3f14880b8217a8fa9b1f53d243e5bda7a77..3fc4a281fb6bce720c586448dce5e52c2263b60f 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -90,3 +90,5 @@ def wip_match(table) .or(table[:title].matches('[WIP]%')) end end + +MergeRequestsFinder.prepend(EE::MergeRequestsFinder) diff --git a/app/views/projects/merge_requests/_merge_requests.html.haml b/app/views/projects/merge_requests/_merge_requests.html.haml index bd6f1c05949b1f2d932a4b6abdb0191304688941..57fbd360d465113d7f1f177b87ad475b38ed25d2 100644 --- a/app/views/projects/merge_requests/_merge_requests.html.haml +++ b/app/views/projects/merge_requests/_merge_requests.html.haml @@ -1,5 +1,5 @@ %ul.content-list.mr-list.issuable-list - - if @merge_requests.exists? + - if @merge_requests.present? = render @merge_requests - else = render 'shared/empty_states/merge_requests' diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index bdba47ed14ddcb2818532b4b67d3ab8ea8867855..f43be304e6b0cce0a5382d58faa9a56568e757e5 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -71,6 +71,7 @@ = render 'shared/issuable/user_dropdown_item', user: User.new(username: '{{username}}', name: '{{name}}'), avatar: { lazy: true, url: '{{avatar_url}}' } + = render_if_exists 'shared/issuable/approver_dropdown' #js-dropdown-milestone.filtered-search-input-dropdown-menu.dropdown-menu %ul{ data: { dropdown: true } } %li.filter-dropdown-item{ data: { value: 'None' } } diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 3cd66b8b956b0a07955b50a14993f40d8fa09d23..023133eaee1c8f12307220a365da161aa757d3f3 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -29,27 +29,28 @@ GET /merge_requests?search=foo&in=title Parameters: -| Attribute | Type | Required | Description | -| ------------------- | -------- | -------- | ---------------------------------------------------------------------------------------------------------------------- | -| `state` | string | no | Return all merge requests or just those that are `opened`, `closed`, `locked`, or `merged` | -| `order_by` | string | no | Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` | -| `sort` | string | no | Return requests sorted in `asc` or `desc` order. Default is `desc` | -| `milestone` | string | no | Return merge requests for a specific milestone. `None` returns merge requests with no milestone. `Any` returns merge requests that have an assigned milestone. | -| `view` | string | no | If `simple`, returns the `iid`, URL, title, description, and basic state of merge request | -| `labels` | string | no | Return merge requests matching a comma separated list of labels. `None` lists all merge requests with no labels. `Any` lists all merge requests with at least one label. `No+Label` (Deprecated) lists all merge requests with no labels. Predefined names are case-insensitive. | -| `created_after` | datetime | no | Return merge requests created on or after the given time | -| `created_before` | datetime | no | Return merge requests created on or before the given time | -| `updated_after` | datetime | no | Return merge requests updated on or after the given time | -| `updated_before` | datetime | no | Return merge requests updated on or before the given time | -| `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead. | -| `author_id` | integer | no | Returns merge requests created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me` | -| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. | -| `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | -| `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` | -| `in` | string | no | Modify the scope of the `search` attribute. `title`, `description`, or a string joining them with comma. Default is `title,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 | +| Attribute | Type | Required | Description | +| ------------------- | -------- | -------- | ---------------------------------------------------------------------------------------------------------------------- | +| `state` | string | no | Return all merge requests or just those that are `opened`, `closed`, `locked`, or `merged` | +| `order_by` | string | no | Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` | +| `sort` | string | no | Return requests sorted in `asc` or `desc` order. Default is `desc` | +| `milestone` | string | no | Return merge requests for a specific milestone. `None` returns merge requests with no milestone. `Any` returns merge requests that have an assigned milestone. | +| `view` | string | no | If `simple`, returns the `iid`, URL, title, description, and basic state of merge request | +| `labels` | string | no | Return merge requests matching a comma separated list of labels. `None` lists all merge requests with no labels. `Any` lists all merge requests with at least one label. `No+Label` (Deprecated) lists all merge requests with no labels. Predefined names are case-insensitive. | +| `created_after` | datetime | no | Return merge requests created on or after the given time | +| `created_before` | datetime | no | Return merge requests created on or before the given time | +| `updated_after` | datetime | no | Return merge requests updated on or after the given time | +| `updated_before` | datetime | no | Return merge requests updated on or before the given time | +| `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`. Defaults to `created_by_me`<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead. | +| `author_id` | integer | no | Returns merge requests created by the given user `id`. Combine with `scope=all` or `scope=assigned_to_me` | +| `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. | +| `approver_ids` | Array[integer] | no | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. | +| `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | +| `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` | +| `in` | string | no | Modify the scope of the `search` attribute. `title`, `description`, or a string joining them with comma. Default is `title,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 [ @@ -180,6 +181,7 @@ Parameters: | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> For versions before 11.0, use the now deprecated `created-by-me` or `assigned-to-me` scopes instead.<br> _([Introduced][ce-13060] in GitLab 9.5. [Changed to snake_case][ce-18935] in GitLab 11.0)_ | | `author_id` | integer | no | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_ | | `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ | +| `approver_ids` | Array[integer] | no | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. | | `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | @@ -306,6 +308,7 @@ Parameters: | `scope` | string | no | Return merge requests for the given scope: `created_by_me`, `assigned_to_me` or `all`.<br> | | `author_id` | integer | no | Returns merge requests created by the given user `id` _([Introduced][ce-13060] in GitLab 9.5)_ | | `assignee_id` | integer | no | Returns merge requests assigned to the given user `id`. `None` returns unassigned merge requests. `Any` returns merge requests with an assignee. _([Introduced][ce-13060] in GitLab 9.5)_ | +| `approver_ids` | Array[integer] | no | Returns merge requests which have specified all the users with the given `id`s as individual approvers. `None` returns merge requests without approvers. `Any` returns merge requests with an approver. | | `my_reaction_emoji` | string | no | Return merge requests reacted by the authenticated user by the given `emoji`. `None` returns issues not given a reaction. `Any` returns issues given at least one reaction. _([Introduced][ce-14016] in GitLab 10.0)_ | | `source_branch` | string | no | Return merge requests with the given source branch | | `target_branch` | string | no | Return merge requests with the given target branch | diff --git a/doc/user/project/merge_requests/img/filter_approver_merge_requests.png b/doc/user/project/merge_requests/img/filter_approver_merge_requests.png new file mode 100644 index 0000000000000000000000000000000000000000..9c386391a4f62c3050a618017c6a1ba17fa117c5 Binary files /dev/null and b/doc/user/project/merge_requests/img/filter_approver_merge_requests.png differ diff --git a/doc/user/project/merge_requests/merge_request_approvals.md b/doc/user/project/merge_requests/merge_request_approvals.md index 2c2c3fa1972f900c680ba2068c3d07b4ca3c0621..b91e07b98129d3fac599122d00b00483a27a31ab 100644 --- a/doc/user/project/merge_requests/merge_request_approvals.md +++ b/doc/user/project/merge_requests/merge_request_approvals.md @@ -290,3 +290,10 @@ request itself. It belongs to the target branch's project. ## Approver suggestions Approvers are suggested for merge requests based on the previous authors of the files affected by the merge request. + +## Filtering merge requests by approvers + +To filter merge requests by an individual approver, you can type (or select from +the dropdown) `approver` and select the user. + + diff --git a/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js b/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js new file mode 100644 index 0000000000000000000000000000000000000000..8e31670a7e4ffc474855028905ab9efd73d0afec --- /dev/null +++ b/ee/app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js @@ -0,0 +1,32 @@ +import addExtraTokensForMergeRequests from '~/filtered_search/add_extra_tokens_for_merge_requests'; + +export default IssuableTokenKeys => { + addExtraTokensForMergeRequests(IssuableTokenKeys); + + const approversConditions = [ + { + url: 'approver_usernames[]=None', + tokenKey: 'approver', + value: 'None', + }, + { + url: 'approver_usernames[]=Any', + tokenKey: 'approver', + value: 'Any', + }, + ]; + + const approversToken = { + key: 'approver', + type: 'array', + param: 'usernames[]', + symbol: '@', + icon: 'approval', + tag: '@approver', + }; + const approversTokenPosition = 2; + + IssuableTokenKeys.tokenKeys.splice(approversTokenPosition, 0, approversToken); + IssuableTokenKeys.tokenKeysWithAlternative.splice(approversTokenPosition, 0, approversToken); + IssuableTokenKeys.conditions.push(...approversConditions); +}; diff --git a/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js b/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js new file mode 100644 index 0000000000000000000000000000000000000000..3bbea89b5cc128f5337fdfa000bb3d513389c373 --- /dev/null +++ b/ee/app/assets/javascripts/filtered_search/available_dropdown_mappings.js @@ -0,0 +1,60 @@ +import DropdownUser from '~/filtered_search/dropdown_user'; +import DropdownNonUser from '~/filtered_search/dropdown_non_user'; +import DropdownWeight from './dropdown_weight'; +import AvailableDropdownMappingsCE from '~/filtered_search/available_dropdown_mappings'; + +export default class AvailableDropdownMappings { + constructor(container, baseEndpoint, groupsOnly, includeAncestorGroups, includeDescendantGroups) { + this.container = container; + this.baseEndpoint = baseEndpoint; + this.groupsOnly = groupsOnly; + this.includeAncestorGroups = includeAncestorGroups; + this.includeDescendantGroups = includeDescendantGroups; + + this.ceAvailableMappings = new AvailableDropdownMappingsCE( + container, + baseEndpoint, + groupsOnly, + includeAncestorGroups, + includeDescendantGroups, + ); + } + + getAllowedMappings(supportedTokens) { + const ceMappings = this.ceAvailableMappings.getMappings(); + + ceMappings.milestone = { + reference: null, + gl: DropdownNonUser, + extraArguments: { + endpoint: this.getMilestoneEndpoint(), + symbol: '%', + }, + element: this.container.querySelector('#js-dropdown-milestone'), + }; + + ceMappings.approver = { + reference: null, + gl: DropdownUser, + element: this.container.querySelector('#js-dropdown-approver'), + }; + + ceMappings.weight = { + reference: null, + gl: DropdownWeight, + element: this.container.querySelector('#js-dropdown-weight'), + }; + + return this.ceAvailableMappings.buildMappings(supportedTokens, ceMappings); + } + + getMilestoneEndpoint() { + let endpoint = `${this.baseEndpoint}/milestones.json`; + + if (this.groupsOnly) { + endpoint = `${endpoint}?only_group_milestones=true`; + } + + return endpoint; + } +} diff --git a/ee/app/assets/javascripts/filtered_search/visual_token_value.js b/ee/app/assets/javascripts/filtered_search/visual_token_value.js new file mode 100644 index 0000000000000000000000000000000000000000..9d5c1a652086dd8be778400c7469594516874d70 --- /dev/null +++ b/ee/app/assets/javascripts/filtered_search/visual_token_value.js @@ -0,0 +1,18 @@ +import VisualTokenValueCE from '~/filtered_search/visual_token_value'; + +export default class VisualTokenValue { + constructor(tokenValue, tokenType) { + this.tokenValue = tokenValue; + this.tokenType = tokenType; + + this.visualTokenValueCE = new VisualTokenValueCE(tokenValue, tokenType); + } + + render(tokenValueContainer, tokenValueElement) { + if (this.tokenType === 'approver') { + this.visualTokenValueCE.updateUserTokenAppearance(tokenValueContainer, tokenValueElement); + } else { + this.visualTokenValueCE.render(tokenValueContainer, tokenValueElement); + } + } +} diff --git a/ee/app/finders/ee/merge_requests_finder.rb b/ee/app/finders/ee/merge_requests_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..dfe34482cf033036fd554502c1a558446581bfe7 --- /dev/null +++ b/ee/app/finders/ee/merge_requests_finder.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module EE + module MergeRequestsFinder + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + override :filter_items + def filter_items(items) + items = super(items) + + by_approvers(items) + end + + def by_approvers(items) + ::MergeRequests::ByApproversFinder + .new(params[:approver_usernames], params[:approver_ids]) + .execute(items) + end + + class_methods do + extend ::Gitlab::Utils::Override + + override :scalar_params + def scalar_params + @scalar_params ||= super + [:approver_ids] + end + + override :array_params + def array_params + @array_params ||= super.merge(approver_usernames: []) + end + end + end +end diff --git a/ee/app/finders/merge_requests/by_approvers_finder.rb b/ee/app/finders/merge_requests/by_approvers_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..896be79efd579debddb143baa23a290fc62d0f75 --- /dev/null +++ b/ee/app/finders/merge_requests/by_approvers_finder.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +# MergeRequests::ByApprovers class +# +# Used to filter MergeRequests collections by approvers + +module MergeRequests + class ByApproversFinder + attr_reader :usernames, :ids + + def initialize(usernames, ids) + @usernames = usernames.to_a.map(&:to_s) + @ids = ids + end + + def execute(items) + if by_no_approvers? + without_approvers(items) + elsif by_any_approvers? + with_any_approvers(items) + elsif ids.present? + find_approvers_by_ids(items) + elsif usernames.present? + find_approvers_by_names(items) + else + items + end + end + + private + + def by_no_approvers? + includes_custom_label?(IssuableFinder::FILTER_NONE) + end + + def by_any_approvers? + includes_custom_label?(IssuableFinder::FILTER_ANY) + end + + def includes_custom_label?(label) + ids.to_s.downcase == label || usernames.map(&:downcase).include?(label) + end + + # rubocop: disable CodeReuse/ActiveRecord + def without_approvers(items) + items + .left_outer_joins(:approval_rules) + .joins('LEFT OUTER JOIN approval_project_rules ON approval_project_rules.project_id = merge_requests.target_project_id') + .where(approval_merge_request_rules: { id: nil }) + .where(approval_project_rules: { id: nil }) + end + + def with_any_approvers(items) + items.select_from_union([ + items.joins(:approval_rules), + items.joins('INNER JOIN approval_project_rules ON approval_project_rules.project_id = merge_requests.target_project_id') + ]) + end + + def find_approvers_by_names(items) + with_users_filtered_by_criteria(items) do |items_with_users| + find_approvers_by_query(items_with_users, :username, usernames) + end + end + + def find_approvers_by_ids(items) + with_users_filtered_by_criteria(items) do |items_with_users| + find_approvers_by_query(items_with_users, :id, ids) + end + end + + def find_approvers_by_query(items, field, values) + items + .where(users: { field => values }) + .group('merge_requests.id') + .having("COUNT(users.id) = ?", values.size) + end + + def with_users_filtered_by_criteria(items) + users_mrs = yield(items.joins(approval_rules: :users)) + group_users_mrs = yield(items.joins(approval_rules: { groups: :users })) + + mrs_without_overriden_rules = items.left_outer_joins(:approval_rules).where(approval_merge_request_rules: { id: nil }) + project_users_mrs = yield(mrs_without_overriden_rules.joins(target_project: { approval_rules: :users })) + project_group_users_mrs = yield(mrs_without_overriden_rules.joins(target_project: { approval_rules: { groups: :users } })) + + items.select_from_union([users_mrs, group_users_mrs, project_users_mrs, project_group_users_mrs]) + end + # rubocop: enable CodeReuse/ActiveRecord + end +end diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 11132baf232735b3ce78d129b45a58b4dd62999a..2b0f69a9c40491bc23c984c1c08d3feefc934f20 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -7,6 +7,7 @@ module MergeRequest include ::Approvable include ::Gitlab::Utils::StrongMemoize + include FromUnion prepend ApprovableForRule prepended do @@ -33,6 +34,12 @@ module MergeRequest accepts_nested_attributes_for :approval_rules, allow_destroy: true end + class_methods do + def select_from_union(relations) + from_union(relations, remove_duplicates: true) + end + end + override :mergeable? def mergeable?(skip_ci_check: false) return false unless approved? diff --git a/ee/app/views/shared/issuable/_approver_dropdown.html.haml b/ee/app/views/shared/issuable/_approver_dropdown.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..ab0f081b72d94addac249dea44c9ff6aac1d465d --- /dev/null +++ b/ee/app/views/shared/issuable/_approver_dropdown.html.haml @@ -0,0 +1,16 @@ +#js-dropdown-approver.filtered-search-input-dropdown-menu.dropdown-menu + %ul{ data: { dropdown: true } } + %li.filter-dropdown-item{ data: { value: 'None' } } + %button.btn.btn-link{ type: 'button' } + = _('None') + %li.filter-dropdown-item{ data: { value: 'Any' } } + %button.btn.btn-link{ type: 'button' } + = _('Any') + %li.divider.droplab-item-ignore + - if current_user + = render 'shared/issuable/user_dropdown_item', + user: current_user + %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } } + = render 'shared/issuable/user_dropdown_item', + user: User.new(username: '{{username}}', name: '{{name}}'), + avatar: { lazy: true, url: '{{avatar_url}}' } diff --git a/ee/changelogs/unreleased/id-1951-filter-merge-requests-by-approvers.yml b/ee/changelogs/unreleased/id-1951-filter-merge-requests-by-approvers.yml new file mode 100644 index 0000000000000000000000000000000000000000..66a804c2eff44773911a31dd52473ac2385b7d57 --- /dev/null +++ b/ee/changelogs/unreleased/id-1951-filter-merge-requests-by-approvers.yml @@ -0,0 +1,5 @@ +--- +title: Add filtering merge requests by approvers +merge_request: 9468 +author: +type: added diff --git a/ee/lib/ee/api/merge_requests.rb b/ee/lib/ee/api/merge_requests.rb index 61cdff1d6e9fb0f753ee07f33fc8b50064992542..d9a6b3e9b5224c5c9687158299a858e7d9996428 100644 --- a/ee/lib/ee/api/merge_requests.rb +++ b/ee/lib/ee/api/merge_requests.rb @@ -14,6 +14,11 @@ module MergeRequests optional :approvals_required, type: Integer, desc: 'Total number of approvals required' end end + + params :optional_merge_requests_search_params do + optional :approver_ids, types: [String, Array], array_none_any: true, + desc: 'Return merge requests which have specified the users with the given IDs as an individual approver' + end end params do diff --git a/ee/spec/factories/merge_requests.rb b/ee/spec/factories/merge_requests.rb index 02b7667ba4ae33bbace579ec41d7d7e7e7d45d4b..f17e4b5295232067b425548101866e814b4a2460 100644 --- a/ee/spec/factories/merge_requests.rb +++ b/ee/spec/factories/merge_requests.rb @@ -7,6 +7,19 @@ create :approver, target: merge_request end end + + transient do + approval_groups [] + approval_users [] + end + + after :create do |merge_request, evaluator| + next if evaluator.approval_users.blank? && evaluator.approval_groups.blank? + + rule = merge_request.approval_rules.first_or_create(attributes_for(:approval_merge_request_rule)) + rule.users = evaluator.approval_users if evaluator.approval_users.present? + rule.groups = evaluator.approval_groups if evaluator.approval_groups.present? + end end end diff --git a/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb b/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2dd50e5d258524544434d7c9239870deefb12952 --- /dev/null +++ b/ee/spec/features/merge_requests/user_filters_by_approvers_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'Merge Requests > User filters by approvers', :js do + include FilteredSearchHelpers + + let(:project) { create(:project, :public, :repository) } + let(:user) { project.creator } + let(:group_user) { create(:user) } + let(:first_user) { create(:user) } + + let!(:merge_request_with_approver) do + create(:merge_request, approval_users: [first_user], title: 'Bugfix1', source_project: project, source_branch: 'bugfix1') + end + + let!(:merge_request_with_two_approvers) do + create(:merge_request, title: 'Bugfix2', approval_users: [user, first_user], source_project: project, source_branch: 'bugfix2') + end + let!(:merge_request) { create(:merge_request, title: 'Bugfix3', source_project: project, source_branch: 'bugfix3') } + let!(:merge_request_with_group_approver) do + group = create(:group) + group.add_developer(group_user) + + create(:merge_request, approval_groups: [group], title: 'Bugfix4', source_project: project, source_branch: 'bugfix4') + end + + before do + sign_in(user) + visit project_merge_requests_path(project) + end + + context 'filtering by approver:none' do + it 'applies the filter' do + input_filtered_search('approver:none') + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + + expect(page).not_to have_content 'Bugfix1' + expect(page).not_to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix4' + expect(page).to have_content 'Bugfix3' + end + end + + context 'filtering by approver:any' do + it 'applies the filter' do + input_filtered_search('approver:any') + + expect(page).to have_issuable_counts(open: 3, closed: 0, all: 3) + + expect(page).to have_content 'Bugfix1' + expect(page).to have_content 'Bugfix2' + expect(page).to have_content 'Bugfix4' + expect(page).not_to have_content 'Bugfix3' + end + end + + context 'filtering by approver:@username' do + it 'applies the filter' do + input_filtered_search("approver:@#{first_user.username}") + + expect(page).to have_issuable_counts(open: 2, closed: 0, all: 2) + + expect(page).to have_content 'Bugfix1' + expect(page).to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix3' + expect(page).not_to have_content 'Bugfix4' + end + end + + context 'filtering by multiple approvers' do + it 'applies the filter' do + input_filtered_search("approver:@#{first_user.username} approver:@#{user.username}") + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + + expect(page).to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix1' + expect(page).not_to have_content 'Bugfix3' + expect(page).not_to have_content 'Bugfix4' + end + end + + context 'filtering by an approver from a group' do + it 'applies the filter' do + input_filtered_search("approver:@#{group_user.username}") + + expect(page).to have_issuable_counts(open: 1, closed: 0, all: 1) + + expect(page).to have_content 'Bugfix4' + expect(page).not_to have_content 'Bugfix1' + expect(page).not_to have_content 'Bugfix2' + expect(page).not_to have_content 'Bugfix3' + end + end +end diff --git a/ee/spec/finders/merge_requests/by_approvers_finder_spec.rb b/ee/spec/finders/merge_requests/by_approvers_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6bbb281dbe9f37200e4811d43c41e1971bd45c81 --- /dev/null +++ b/ee/spec/finders/merge_requests/by_approvers_finder_spec.rb @@ -0,0 +1,136 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequests::ByApproversFinder do + let(:group_user) { create(:user) } + let(:second_group_user) { create(:user) } + let(:group) do + create(:group).tap do |group| + group.add_developer(group_user) + group.add_developer(second_group_user) + end + end + + let!(:merge_request) { create(:merge_request) } + let!(:merge_request_with_approver) { create(:merge_request_with_approver) } + + let(:project_user) { create(:user) } + let(:first_user) { merge_request_with_approver.approvers.first.user } + let(:second_user) { create(:user) } + let(:second_project_user) { create(:user) } + + let!(:merge_request_with_project_approver) do + rule = create(:approval_project_rule, users: [project_user, second_project_user]) + create(:merge_request, source_project: create(:project, approval_rules: [rule])) + end + + let!(:merge_request_with_two_approvers) { create(:merge_request, approval_users: [first_user, second_user]) } + let!(:merge_request_with_group_approver) do + create(:merge_request).tap do |merge_request| + rule = create(:approval_merge_request_rule, merge_request: merge_request, groups: [group]) + merge_request.approval_rules << rule + end + end + let!(:merge_request_with_project_group_approver) do + rule = create(:approval_project_rule, groups: [group]) + create(:merge_request, source_project: create(:project, approval_rules: [rule])) + end + + def merge_requests(ids: nil, names: []) + described_class.new(names, ids).execute(MergeRequest.all) + end + + context 'filter by no approvers' do + it 'returns merge requests without approvers' do + expect(merge_requests(ids: 'None')).to eq([merge_request]) + expect(merge_requests(names: ['None'])).to eq([merge_request]) + end + end + + context 'filter by any approver' do + it 'returns only merge requests with approvers' do + expect(merge_requests(ids: 'Any')).to match_array([ + merge_request_with_approver, merge_request_with_two_approvers, + merge_request_with_group_approver, merge_request_with_project_approver, + merge_request_with_project_group_approver + ]) + expect(merge_requests(names: ['Any'])).to match_array([ + merge_request_with_approver, merge_request_with_two_approvers, + merge_request_with_group_approver, merge_request_with_project_approver, + merge_request_with_project_group_approver + ]) + end + end + + context 'filter by second approver' do + it 'returns only merge requests with the second approver' do + expect(merge_requests(ids: [second_user.id])).to eq( + [merge_request_with_two_approvers] + ) + expect(merge_requests(names: [second_user.username])).to eq( + [merge_request_with_two_approvers] + ) + end + end + + context 'filter by both approvers' do + it 'returns only merge requests with both approvers' do + expect(merge_requests(ids: [first_user.id, second_user.id])).to eq( + [merge_request_with_two_approvers] + ) + expect(merge_requests(names: [first_user.username, second_user.username])).to eq( + [merge_request_with_two_approvers] + ) + end + end + + context 'pass empty params' do + it 'returns all merge requests' do + expect(merge_requests(ids: [])).to match_array([ + merge_request, merge_request_with_approver, + merge_request_with_two_approvers, merge_request_with_group_approver, + merge_request_with_project_approver, merge_request_with_project_group_approver + ]) + expect(merge_requests(names: [])).to match_array([ + merge_request, merge_request_with_approver, + merge_request_with_two_approvers, merge_request_with_group_approver, + merge_request_with_project_approver, merge_request_with_project_group_approver + ]) + end + end + + context 'filter by an approver from group' do + it 'returns only merge requests with the approver from group' do + expect(merge_requests(ids: [group_user.id])).to match_array( + [merge_request_with_project_group_approver, merge_request_with_group_approver] + ) + expect(merge_requests(names: [group_user.username])).to match_array( + [merge_request_with_project_group_approver, merge_request_with_group_approver] + ) + expect(merge_requests(names: [first_user.username, group_user.username])).to match_array([]) + expect(merge_requests(names: [group_user.username, second_group_user.username])).to match_array( + [merge_request_with_project_group_approver, merge_request_with_group_approver] + ) + end + end + + context 'filter by an overridden approver from project' do + it 'returns only merge requests with the project approver' do + expect(merge_requests(ids: [project_user.id])).to eq( + [merge_request_with_project_approver] + ) + expect(merge_requests(ids: [first_user.id, project_user.id])).to eq([]) + expect(merge_requests(ids: [project_user.id, second_project_user.id])).to eq( + [merge_request_with_project_approver] + ) + expect(merge_requests(names: [project_user.username])).to eq( + [merge_request_with_project_approver] + ) + expect(merge_requests(names: [first_user.username, project_user.username])).to eq([]) + expect(merge_requests(names: [project_user.username, second_project_user.username])).to eq( + [merge_request_with_project_approver] + ) + end + end +end diff --git a/ee/spec/requests/api/merge_requests_spec.rb b/ee/spec/requests/api/merge_requests_spec.rb index 2fb50ea4580c202986297c40ee3b8259d785ff1e..81d17d5267e98d7c361d22042b77cfa9fdbde421 100644 --- a/ee/spec/requests/api/merge_requests_spec.rb +++ b/ee/spec/requests/api/merge_requests_spec.rb @@ -130,4 +130,57 @@ def create_merge_request(args) end end end + + context 'when authenticated' do + def expect_response_contain_exactly(*items) + expect(response).to have_gitlab_http_status(200) + expect(json_response.length).to eq(items.size) + expect(json_response.map { |element| element['id'] }).to contain_exactly(*items.map(&:id)) + end + + let!(:merge_request_with_approver) do + create(:merge_request_with_approver, :simple, author: user, source_project: project, target_project: project, source_branch: 'other-branch') + end + + let(:another_user) {} + + context 'request merge requests' do + before do + get api('/merge_requests', user), params: { approver_ids: approvers_param, scope: :all } + end + + context 'with specified approver id' do + let(:approvers_param) { [merge_request_with_approver.approvers.first.user_id] } + + it 'returns an array of merge requests which have specified the user as an approver' do + expect_response_contain_exactly(merge_request_with_approver) + end + end + + context 'with specified None as a param' do + let(:approvers_param) { 'None' } + + it 'returns an array of merge requests with no approvers' do + expect_response_contain_exactly(merge_request) + end + end + + context 'with specified Any as a param' do + let(:approvers_param) { 'Any' } + + it 'returns an array of merge requests with any approver' do + expect_response_contain_exactly(merge_request_with_approver) + end + end + + context 'with any other string as a param' do + let(:approvers_param) { 'any-other-string' } + + it 'returns a validation error' do + expect(response).to have_gitlab_http_status(400) + expect(json_response['error']).to eq("approver_ids should be an array, 'None' or 'Any'") + end + end + end + end end diff --git a/lib/api/helpers/custom_validators.rb b/lib/api/helpers/custom_validators.rb index 1058f4e8a5e0c69a77275221a8baef6bdb411fd9..c86eae6f2dab5bcd0628959a7f3281d0e7aa8d79 100644 --- a/lib/api/helpers/custom_validators.rb +++ b/lib/api/helpers/custom_validators.rb @@ -22,9 +22,22 @@ def validate_param!(attr_name, params) message: "should be an integer, 'None' or 'Any'" end end + + class ArrayNoneAny < Grape::Validations::Base + def validate_param!(attr_name, params) + value = params[attr_name] + + return if value.is_a?(Array) || + [IssuableFinder::FILTER_NONE, IssuableFinder::FILTER_ANY].include?(value.to_s.downcase) + + raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], + message: "should be an array, 'None' or 'Any'" + end + end end end end Grape::Validations.register_validator(:absence, ::API::Helpers::CustomValidators::Absence) Grape::Validations.register_validator(:integer_none_any, ::API::Helpers::CustomValidators::IntegerNoneAny) +Grape::Validations.register_validator(:array_none_any, ::API::Helpers::CustomValidators::ArrayNoneAny) diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 5dc45ebaec4c7e56f3ae4eca602a0ca7e97bb243..0f757b38a853e1e102c27607c749967785d858a8 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -12,6 +12,9 @@ class MergeRequests < Grape::API helpers do params :optional_params_ee do end + + params :optional_merge_requests_search_params do + end end def self.update_params_at_least_one_of @@ -114,6 +117,8 @@ def authorize_push_to_merge_request!(merge_request) optional :search, type: String, desc: 'Search merge requests for text present in the title, description, or any combination of these' optional :in, type: String, desc: '`title`, `description`, or a string joining them with comma' optional :wip, type: String, values: %w[yes no], desc: 'Search merge requests for WIP in the title' + + use :optional_merge_requests_search_params use :pagination end end 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 6230da77f49f248b7f2e5e921a994fc137180f4b..f3dc35552d5d9e7fa83eb35fd9e3f81f7cc1d06d 100644 --- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js @@ -1,9 +1,4 @@ -import _ from 'underscore'; -import AjaxCache from '~/lib/utils/ajax_cache'; -import UsersCache from '~/lib/utils/users_cache'; - import FilteredSearchVisualTokens from '~/filtered_search/filtered_search_visual_tokens'; -import DropdownUtils from '~/filtered_search//dropdown_utils'; import FilteredSearchSpecHelper from '../helpers/filtered_search_spec_helper'; describe('Filtered Search Visual Tokens', () => { @@ -685,349 +680,21 @@ describe('Filtered Search Visual Tokens', () => { }); describe('renderVisualTokenValue', () => { - const keywordToken = FilteredSearchSpecHelper.createFilterVisualToken('search'); - const milestoneToken = FilteredSearchSpecHelper.createFilterVisualToken( - 'milestone', - 'upcoming', - ); - - let updateLabelTokenColorSpy; - let updateUserTokenAppearanceSpy; - beforeEach(() => { tokensContainer.innerHTML = FilteredSearchSpecHelper.createTokensContainerHTML(` ${authorToken.outerHTML} ${bugLabelToken.outerHTML} - ${keywordToken.outerHTML} - ${milestoneToken.outerHTML} `); - - spyOn(subject, 'updateLabelTokenColor'); - updateLabelTokenColorSpy = subject.updateLabelTokenColor; - - spyOn(subject, 'updateUserTokenAppearance'); - updateUserTokenAppearanceSpy = subject.updateUserTokenAppearance; }); it('renders a author token value element', () => { - const { tokenNameElement, tokenValueContainer, tokenValueElement } = findElements( - authorToken, - ); + const { tokenNameElement, tokenValueElement } = findElements(authorToken); const tokenName = tokenNameElement.innerText; const tokenValue = 'new value'; subject.renderVisualTokenValue(authorToken, tokenName, tokenValue); expect(tokenValueElement.innerText).toBe(tokenValue); - expect(updateUserTokenAppearanceSpy.calls.count()).toBe(1); - const expectedArgs = [tokenValueContainer, tokenValueElement, tokenValue]; - - expect(updateUserTokenAppearanceSpy.calls.argsFor(0)).toEqual(expectedArgs); - expect(updateLabelTokenColorSpy.calls.count()).toBe(0); - }); - - it('renders a label token value element', () => { - const { tokenNameElement, tokenValueContainer, tokenValueElement } = findElements( - bugLabelToken, - ); - const tokenName = tokenNameElement.innerText; - const tokenValue = 'new value'; - - subject.renderVisualTokenValue(bugLabelToken, tokenName, tokenValue); - - expect(tokenValueElement.innerText).toBe(tokenValue); - expect(updateLabelTokenColorSpy.calls.count()).toBe(1); - const expectedArgs = [tokenValueContainer, tokenValue]; - - expect(updateLabelTokenColorSpy.calls.argsFor(0)).toEqual(expectedArgs); - expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); - }); - - it('renders a milestone token value element', () => { - const { tokenNameElement, tokenValueElement } = findElements(milestoneToken); - const tokenName = tokenNameElement.innerText; - const tokenValue = 'new value'; - - subject.renderVisualTokenValue(milestoneToken, tokenName, tokenValue); - - expect(tokenValueElement.innerText).toBe(tokenValue); - expect(updateLabelTokenColorSpy.calls.count()).toBe(0); - expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); - }); - - it('does not update user token appearance for `None` filter', () => { - const { tokenNameElement } = findElements(authorToken); - - const tokenName = tokenNameElement.innerText; - const tokenValue = 'None'; - - subject.renderVisualTokenValue(authorToken, tokenName, tokenValue); - - expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); - }); - - it('does not update user token appearance for `none` filter', () => { - const { tokenNameElement } = findElements(authorToken); - - const tokenName = tokenNameElement.innerText; - const tokenValue = 'none'; - - subject.renderVisualTokenValue(authorToken, tokenName, tokenValue); - - expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); - }); - - it('does not update user token appearance for `any` filter', () => { - const { tokenNameElement } = findElements(authorToken); - - const tokenName = tokenNameElement.innerText; - const tokenValue = 'any'; - - subject.renderVisualTokenValue(authorToken, tokenName, tokenValue); - - expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); - }); - - it('does not update label token color for `none` filter', () => { - const { tokenNameElement } = findElements(bugLabelToken); - - const tokenName = tokenNameElement.innerText; - const tokenValue = 'none'; - - subject.renderVisualTokenValue(bugLabelToken, tokenName, tokenValue); - - expect(updateLabelTokenColorSpy.calls.count()).toBe(0); - }); - - it('does not update label token color for `any` filter', () => { - const { tokenNameElement } = findElements(bugLabelToken); - - const tokenName = tokenNameElement.innerText; - const tokenValue = 'any'; - - subject.renderVisualTokenValue(bugLabelToken, tokenName, tokenValue); - - expect(updateLabelTokenColorSpy.calls.count()).toBe(0); - }); - }); - - describe('updateUserTokenAppearance', () => { - let usersCacheSpy; - - beforeEach(() => { - spyOn(UsersCache, 'retrieve').and.callFake(username => usersCacheSpy(username)); - }); - - it('ignores error if UsersCache throws', done => { - spyOn(window, 'Flash'); - const dummyError = new Error('Earth rotated backwards'); - const { tokenValueContainer, tokenValueElement } = findElements(authorToken); - const tokenValue = tokenValueElement.innerText; - usersCacheSpy = username => { - expect(`@${username}`).toBe(tokenValue); - return Promise.reject(dummyError); - }; - - subject - .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) - .then(() => { - expect(window.Flash.calls.count()).toBe(0); - }) - .then(done) - .catch(done.fail); - }); - - it('does nothing if user cannot be found', done => { - const { tokenValueContainer, tokenValueElement } = findElements(authorToken); - const tokenValue = tokenValueElement.innerText; - usersCacheSpy = username => { - expect(`@${username}`).toBe(tokenValue); - return Promise.resolve(undefined); - }; - - subject - .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) - .then(() => { - expect(tokenValueElement.innerText).toBe(tokenValue); - }) - .then(done) - .catch(done.fail); - }); - - it('replaces author token with avatar and display name', done => { - const dummyUser = { - name: 'Important Person', - avatar_url: 'https://host.invalid/mypics/avatar.png', - }; - const { tokenValueContainer, tokenValueElement } = findElements(authorToken); - const tokenValue = tokenValueElement.innerText; - usersCacheSpy = username => { - expect(`@${username}`).toBe(tokenValue); - return Promise.resolve(dummyUser); - }; - - subject - .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) - .then(() => { - expect(tokenValueContainer.dataset.originalValue).toBe(tokenValue); - expect(tokenValueElement.innerText.trim()).toBe(dummyUser.name); - const avatar = tokenValueElement.querySelector('img.avatar'); - - expect(avatar.src).toBe(dummyUser.avatar_url); - expect(avatar.alt).toBe(''); - }) - .then(done) - .catch(done.fail); - }); - - it('escapes user name when creating token', done => { - const dummyUser = { - name: '<script>', - avatar_url: `${gl.TEST_HOST}/mypics/avatar.png`, - }; - const { tokenValueContainer, tokenValueElement } = findElements(authorToken); - const tokenValue = tokenValueElement.innerText; - usersCacheSpy = username => { - expect(`@${username}`).toBe(tokenValue); - return Promise.resolve(dummyUser); - }; - - subject - .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) - .then(() => { - expect(tokenValueElement.innerText.trim()).toBe(dummyUser.name); - tokenValueElement.querySelector('.avatar').remove(); - - expect(tokenValueElement.innerHTML.trim()).toBe(_.escape(dummyUser.name)); - }) - .then(done) - .catch(done.fail); - }); - }); - - describe('setTokenStyle', () => { - let originalTextColor; - - beforeEach(() => { - originalTextColor = bugLabelToken.style.color; - }); - - it('should set backgroundColor', () => { - const originalBackgroundColor = bugLabelToken.style.backgroundColor; - const token = subject.setTokenStyle(bugLabelToken, 'blue', 'white'); - - expect(token.style.backgroundColor).toEqual('blue'); - expect(token.style.backgroundColor).not.toEqual(originalBackgroundColor); - }); - - it('should set textColor', () => { - const token = subject.setTokenStyle(bugLabelToken, 'white', 'black'); - - expect(token.style.color).toEqual('black'); - expect(token.style.color).not.toEqual(originalTextColor); - }); - - it('should add inverted class when textColor is #FFFFFF', () => { - const token = subject.setTokenStyle(bugLabelToken, 'black', '#FFFFFF'); - - expect(token.style.color).toEqual('rgb(255, 255, 255)'); - expect(token.style.color).not.toEqual(originalTextColor); - expect(token.querySelector('.remove-token').classList.contains('inverted')).toEqual(true); - }); - }); - - describe('updateLabelTokenColor', () => { - const jsonFixtureName = 'labels/project_labels.json'; - const dummyEndpoint = '/dummy/endpoint'; - - preloadFixtures(jsonFixtureName); - - let labelData; - - beforeAll(() => { - labelData = getJSONFixture(jsonFixtureName); - }); - - const missingLabelToken = FilteredSearchSpecHelper.createFilterVisualToken( - 'label', - '~doesnotexist', - ); - const spaceLabelToken = FilteredSearchSpecHelper.createFilterVisualToken( - 'label', - '~"some space"', - ); - - beforeEach(() => { - tokensContainer.innerHTML = FilteredSearchSpecHelper.createTokensContainerHTML(` - ${bugLabelToken.outerHTML} - ${missingLabelToken.outerHTML} - ${spaceLabelToken.outerHTML} - `); - - const filteredSearchInput = document.querySelector('.filtered-search'); - filteredSearchInput.dataset.baseEndpoint = dummyEndpoint; - - AjaxCache.internalStorage = {}; - AjaxCache.internalStorage[`${dummyEndpoint}/labels.json`] = labelData; - }); - - const parseColor = color => { - const dummyElement = document.createElement('div'); - dummyElement.style.color = color; - return dummyElement.style.color; - }; - - const expectValueContainerStyle = (tokenValueContainer, label) => { - expect(tokenValueContainer.getAttribute('style')).not.toBe(null); - expect(tokenValueContainer.style.backgroundColor).toBe(parseColor(label.color)); - expect(tokenValueContainer.style.color).toBe(parseColor(label.text_color)); - }; - - const findLabel = tokenValue => - labelData.find(label => tokenValue === `~${DropdownUtils.getEscapedText(label.title)}`); - - it('updates the color of a label token', done => { - const { tokenValueContainer, tokenValueElement } = findElements(bugLabelToken); - const tokenValue = tokenValueElement.innerText; - const matchingLabel = findLabel(tokenValue); - - subject - .updateLabelTokenColor(tokenValueContainer, tokenValue) - .then(() => { - expectValueContainerStyle(tokenValueContainer, matchingLabel); - }) - .then(done) - .catch(done.fail); - }); - - it('updates the color of a label token with spaces', done => { - const { tokenValueContainer, tokenValueElement } = findElements(spaceLabelToken); - const tokenValue = tokenValueElement.innerText; - const matchingLabel = findLabel(tokenValue); - - subject - .updateLabelTokenColor(tokenValueContainer, tokenValue) - .then(() => { - expectValueContainerStyle(tokenValueContainer, matchingLabel); - }) - .then(done) - .catch(done.fail); - }); - - it('does not change color of a missing label', done => { - const { tokenValueContainer, tokenValueElement } = findElements(missingLabelToken); - const tokenValue = tokenValueElement.innerText; - const matchingLabel = findLabel(tokenValue); - - expect(matchingLabel).toBe(undefined); - - subject - .updateLabelTokenColor(tokenValueContainer, tokenValue) - .then(() => { - expect(tokenValueContainer.getAttribute('style')).toBe(null); - }) - .then(done) - .catch(done.fail); }); }); }); diff --git a/spec/javascripts/filtered_search/visual_token_value_spec.js b/spec/javascripts/filtered_search/visual_token_value_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..f52dc26a7bbb250b0387f284004f5440c4d7bf0c --- /dev/null +++ b/spec/javascripts/filtered_search/visual_token_value_spec.js @@ -0,0 +1,361 @@ +import VisualTokenValue from '~/filtered_search/visual_token_value'; +import _ from 'underscore'; +import AjaxCache from '~/lib/utils/ajax_cache'; +import UsersCache from '~/lib/utils/users_cache'; +import DropdownUtils from '~/filtered_search//dropdown_utils'; +import FilteredSearchSpecHelper from '../helpers/filtered_search_spec_helper'; + +describe('Filtered Search Visual Tokens', () => { + const findElements = tokenElement => { + const tokenNameElement = tokenElement.querySelector('.name'); + const tokenValueContainer = tokenElement.querySelector('.value-container'); + const tokenValueElement = tokenValueContainer.querySelector('.value'); + const tokenType = tokenNameElement.innerText.toLowerCase(); + const tokenValue = tokenValueElement.innerText; + const subject = new VisualTokenValue(tokenValue, tokenType); + return { subject, tokenValueContainer, tokenValueElement }; + }; + + let tokensContainer; + let authorToken; + let bugLabelToken; + + beforeEach(() => { + setFixtures(` + <ul class="tokens-container"> + ${FilteredSearchSpecHelper.createInputHTML()} + </ul> + `); + tokensContainer = document.querySelector('.tokens-container'); + + authorToken = FilteredSearchSpecHelper.createFilterVisualToken('author', '@user'); + bugLabelToken = FilteredSearchSpecHelper.createFilterVisualToken('label', '~bug'); + }); + + describe('updateUserTokenAppearance', () => { + let usersCacheSpy; + + beforeEach(() => { + spyOn(UsersCache, 'retrieve').and.callFake(username => usersCacheSpy(username)); + }); + + it('ignores error if UsersCache throws', done => { + spyOn(window, 'Flash'); + const dummyError = new Error('Earth rotated backwards'); + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + const tokenValue = tokenValueElement.innerText; + usersCacheSpy = username => { + expect(`@${username}`).toBe(tokenValue); + return Promise.reject(dummyError); + }; + + subject + .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) + .then(() => { + expect(window.Flash.calls.count()).toBe(0); + }) + .then(done) + .catch(done.fail); + }); + + it('does nothing if user cannot be found', done => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + const tokenValue = tokenValueElement.innerText; + usersCacheSpy = username => { + expect(`@${username}`).toBe(tokenValue); + return Promise.resolve(undefined); + }; + + subject + .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) + .then(() => { + expect(tokenValueElement.innerText).toBe(tokenValue); + }) + .then(done) + .catch(done.fail); + }); + + it('replaces author token with avatar and display name', done => { + const dummyUser = { + name: 'Important Person', + avatar_url: 'https://host.invalid/mypics/avatar.png', + }; + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + const tokenValue = tokenValueElement.innerText; + usersCacheSpy = username => { + expect(`@${username}`).toBe(tokenValue); + return Promise.resolve(dummyUser); + }; + + subject + .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) + .then(() => { + expect(tokenValueContainer.dataset.originalValue).toBe(tokenValue); + expect(tokenValueElement.innerText.trim()).toBe(dummyUser.name); + const avatar = tokenValueElement.querySelector('img.avatar'); + + expect(avatar.src).toBe(dummyUser.avatar_url); + expect(avatar.alt).toBe(''); + }) + .then(done) + .catch(done.fail); + }); + + it('escapes user name when creating token', done => { + const dummyUser = { + name: '<script>', + avatar_url: `${gl.TEST_HOST}/mypics/avatar.png`, + }; + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + const tokenValue = tokenValueElement.innerText; + usersCacheSpy = username => { + expect(`@${username}`).toBe(tokenValue); + return Promise.resolve(dummyUser); + }; + + subject + .updateUserTokenAppearance(tokenValueContainer, tokenValueElement, tokenValue) + .then(() => { + expect(tokenValueElement.innerText.trim()).toBe(dummyUser.name); + tokenValueElement.querySelector('.avatar').remove(); + + expect(tokenValueElement.innerHTML.trim()).toBe(_.escape(dummyUser.name)); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('updateLabelTokenColor', () => { + const jsonFixtureName = 'labels/project_labels.json'; + const dummyEndpoint = '/dummy/endpoint'; + + preloadFixtures(jsonFixtureName); + + let labelData; + + beforeAll(() => { + labelData = getJSONFixture(jsonFixtureName); + }); + + const missingLabelToken = FilteredSearchSpecHelper.createFilterVisualToken( + 'label', + '~doesnotexist', + ); + const spaceLabelToken = FilteredSearchSpecHelper.createFilterVisualToken( + 'label', + '~"some space"', + ); + + beforeEach(() => { + tokensContainer.innerHTML = FilteredSearchSpecHelper.createTokensContainerHTML(` + ${bugLabelToken.outerHTML} + ${missingLabelToken.outerHTML} + ${spaceLabelToken.outerHTML} + `); + + const filteredSearchInput = document.querySelector('.filtered-search'); + filteredSearchInput.dataset.baseEndpoint = dummyEndpoint; + + AjaxCache.internalStorage = {}; + AjaxCache.internalStorage[`${dummyEndpoint}/labels.json`] = labelData; + }); + + const parseColor = color => { + const dummyElement = document.createElement('div'); + dummyElement.style.color = color; + return dummyElement.style.color; + }; + + const expectValueContainerStyle = (tokenValueContainer, label) => { + expect(tokenValueContainer.getAttribute('style')).not.toBe(null); + expect(tokenValueContainer.style.backgroundColor).toBe(parseColor(label.color)); + expect(tokenValueContainer.style.color).toBe(parseColor(label.text_color)); + }; + + const findLabel = tokenValue => + labelData.find(label => tokenValue === `~${DropdownUtils.getEscapedText(label.title)}`); + + it('updates the color of a label token', done => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(bugLabelToken); + const tokenValue = tokenValueElement.innerText; + const matchingLabel = findLabel(tokenValue); + + subject + .updateLabelTokenColor(tokenValueContainer, tokenValue) + .then(() => { + expectValueContainerStyle(tokenValueContainer, matchingLabel); + }) + .then(done) + .catch(done.fail); + }); + + it('updates the color of a label token with spaces', done => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(spaceLabelToken); + const tokenValue = tokenValueElement.innerText; + const matchingLabel = findLabel(tokenValue); + + subject + .updateLabelTokenColor(tokenValueContainer, tokenValue) + .then(() => { + expectValueContainerStyle(tokenValueContainer, matchingLabel); + }) + .then(done) + .catch(done.fail); + }); + + it('does not change color of a missing label', done => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(missingLabelToken); + const tokenValue = tokenValueElement.innerText; + const matchingLabel = findLabel(tokenValue); + + expect(matchingLabel).toBe(undefined); + + subject + .updateLabelTokenColor(tokenValueContainer, tokenValue) + .then(() => { + expect(tokenValueContainer.getAttribute('style')).toBe(null); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('setTokenStyle', () => { + let originalTextColor; + + beforeEach(() => { + originalTextColor = bugLabelToken.style.color; + }); + + it('should set backgroundColor', () => { + const originalBackgroundColor = bugLabelToken.style.backgroundColor; + const token = VisualTokenValue.setTokenStyle(bugLabelToken, 'blue', 'white'); + + expect(token.style.backgroundColor).toEqual('blue'); + expect(token.style.backgroundColor).not.toEqual(originalBackgroundColor); + }); + + it('should set textColor', () => { + const token = VisualTokenValue.setTokenStyle(bugLabelToken, 'white', 'black'); + + expect(token.style.color).toEqual('black'); + expect(token.style.color).not.toEqual(originalTextColor); + }); + + it('should add inverted class when textColor is #FFFFFF', () => { + const token = VisualTokenValue.setTokenStyle(bugLabelToken, 'black', '#FFFFFF'); + + expect(token.style.color).toEqual('rgb(255, 255, 255)'); + expect(token.style.color).not.toEqual(originalTextColor); + expect(token.querySelector('.remove-token').classList.contains('inverted')).toEqual(true); + }); + }); + + describe('render', () => { + const setupSpies = subject => { + spyOn(subject, 'updateLabelTokenColor'); // eslint-disable-line jasmine/no-unsafe-spy + const updateLabelTokenColorSpy = subject.updateLabelTokenColor; + + spyOn(subject, 'updateUserTokenAppearance'); // eslint-disable-line jasmine/no-unsafe-spy + const updateUserTokenAppearanceSpy = subject.updateUserTokenAppearance; + + return { updateLabelTokenColorSpy, updateUserTokenAppearanceSpy }; + }; + + const keywordToken = FilteredSearchSpecHelper.createFilterVisualToken('search'); + const milestoneToken = FilteredSearchSpecHelper.createFilterVisualToken( + 'milestone', + 'upcoming', + ); + + beforeEach(() => { + tokensContainer.innerHTML = FilteredSearchSpecHelper.createTokensContainerHTML(` + ${authorToken.outerHTML} + ${bugLabelToken.outerHTML} + ${keywordToken.outerHTML} + ${milestoneToken.outerHTML} + `); + }); + + it('renders a author token value element', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + + const { updateLabelTokenColorSpy, updateUserTokenAppearanceSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(1); + const expectedArgs = [tokenValueContainer, tokenValueElement]; + + expect(updateUserTokenAppearanceSpy.calls.argsFor(0)).toEqual(expectedArgs); + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + }); + + it('renders a label token value element', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(bugLabelToken); + + const { updateLabelTokenColorSpy, updateUserTokenAppearanceSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateLabelTokenColorSpy.calls.count()).toBe(1); + const expectedArgs = [tokenValueContainer]; + + expect(updateLabelTokenColorSpy.calls.argsFor(0)).toEqual(expectedArgs); + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); + }); + + it('renders a milestone token value element', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(milestoneToken); + + const { updateLabelTokenColorSpy, updateUserTokenAppearanceSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); + }); + + it('does not update user token appearance for `none` filter', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + + subject.tokenType = 'none'; + + const { updateUserTokenAppearanceSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); + }); + + it('does not update user token appearance for `any` filter', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(authorToken); + + subject.tokenType = 'any'; + + const { updateUserTokenAppearanceSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateUserTokenAppearanceSpy.calls.count()).toBe(0); + }); + + it('does not update label token color for `none` filter', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(bugLabelToken); + + subject.tokenType = 'none'; + + const { updateLabelTokenColorSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + }); + + it('does not update label token color for `any` filter', () => { + const { subject, tokenValueContainer, tokenValueElement } = findElements(bugLabelToken); + + subject.tokenType = 'any'; + + const { updateLabelTokenColorSpy } = setupSpies(subject); + subject.render(tokenValueContainer, tokenValueElement); + + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + }); + }); +}); diff --git a/spec/lib/api/helpers/custom_validators_spec.rb b/spec/lib/api/helpers/custom_validators_spec.rb index 41e6fb47b11d1fb3e54fa0e8e6e144ada885ae37..9945d598a14747e390fd4a8b4a02201d0806967f 100644 --- a/spec/lib/api/helpers/custom_validators_spec.rb +++ b/spec/lib/api/helpers/custom_validators_spec.rb @@ -50,6 +50,29 @@ def full_name(attr_name) end end + describe API::Helpers::CustomValidators::ArrayNoneAny do + subject do + described_class.new(['test'], {}, false, scope.new) + end + + context 'valid parameters' do + it 'does not raise a validation error' do + expect_no_validation_error({ 'test' => [] }) + expect_no_validation_error({ 'test' => [1, 2, 3] }) + expect_no_validation_error({ 'test' => 'None' }) + expect_no_validation_error({ 'test' => 'Any' }) + expect_no_validation_error({ 'test' => 'none' }) + expect_no_validation_error({ 'test' => 'any' }) + end + end + + context 'invalid parameters' do + it 'should raise a validation error' do + expect_validation_error({ 'test' => 'some_other_string' }) + end + end + end + def expect_no_validation_error(params) expect { validate_test_param!(params) }.not_to raise_error end