From 692f4b734f1976b690dccb5458c198b5205c51b5 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 3 Sep 2020 21:08:18 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .rubocop.yml | 3 + .rubocop_todo.yml | 19 +- .../javascripts/behaviors/shortcuts/shortcuts.js | 6 +- .../behaviors/shortcuts/shortcuts_find_file.js | 17 +- app/assets/javascripts/pages/search/show/index.js | 6 +- .../state_filter/components/state_filter.vue | 91 +++++ .../javascripts/search/state_filter/constants.js | 20 ++ .../javascripts/search/state_filter/index.js | 34 ++ .../vue_shared/components/file_finder/index.vue | 27 +- .../filtered_search_bar_root.vue | 39 +- .../filtered_search_bar/filtered_search_utils.js | 32 +- .../projects/merge_requests/diffs_controller.rb | 1 - .../projects/merge_requests_controller.rb | 2 + app/finders/issues_finder.rb | 2 +- app/helpers/lazy_image_tag_helper.rb | 2 +- app/helpers/search_helper.rb | 2 +- app/models/operations/feature_flag.rb | 101 ++++++ app/models/operations/feature_flag_scope.rb | 62 ++++ app/models/operations/feature_flags/scope.rb | 13 + app/models/operations/feature_flags/strategy.rb | 94 +++++ .../operations/feature_flags/strategy_user_list.rb | 12 + app/models/operations/feature_flags/user_list.rb | 36 ++ app/models/operations/feature_flags_client.rb | 25 ++ app/models/project.rb | 4 + app/services/search/global_service.rb | 3 +- app/services/search/group_service.rb | 3 +- app/services/search/project_service.rb | 3 +- .../feature_flag_strategies_validator.rb | 95 +++++ app/validators/feature_flag_user_xids_validator.rb | 31 ++ app/views/search/_results.html.haml | 2 + ...plement-issue-scope-results-filter-by-state.yml | 5 + changelogs/unreleased/access-modifier-cop.yml | 5 + config/dependency_decisions.yml | 6 + config/routes/project.rb | 8 + .../geo/disaster_recovery/planned_failover.md | 2 +- doc/api/graphql/reference/gitlab_schema.graphql | 5 + doc/api/graphql/reference/gitlab_schema.json | 10 + doc/api/groups.md | 22 ++ doc/api/markdown.md | 4 +- doc/ci/yaml/README.md | 3 +- doc/development/application_limits.md | 88 ++--- doc/development/documentation/feature_flags.md | 8 +- doc/development/documentation/styleguide.md | 5 +- .../merge_requests/browser_performance_testing.md | 8 +- .../img/browser_performance_testing.png | Bin 26201 -> 40417 bytes lib/backup/database.rb | 11 +- lib/gitlab/cache/request_cache.rb | 2 +- .../Jobs/Browser-Performance-Testing.gitlab-ci.yml | 8 +- .../Verify/Browser-Performance.gitlab-ci.yml | 6 +- lib/gitlab/group_search_results.rb | 4 +- lib/gitlab/middleware/multipart.rb | 4 +- lib/gitlab/project_search_results.rb | 4 +- lib/gitlab/regex.rb | 11 +- lib/gitlab/request_profiler.rb | 8 +- lib/gitlab/search_results.rb | 7 +- lib/uploaded_file.rb | 3 +- locale/gitlab.pot | 3 + package.json | 4 +- .../merge_requests/diffs_controller_spec.rb | 35 -- .../projects/merge_requests_controller_spec.rb | 10 + spec/factories/clusters/kubernetes_namespaces.rb | 15 +- spec/factories/draft_note.rb | 2 +- spec/factories/file_uploaders.rb | 2 +- spec/factories/operations/feature_flag_scopes.rb | 10 + spec/factories/operations/feature_flags.rb | 17 + spec/factories/operations/feature_flags/scope.rb | 8 + .../factories/operations/feature_flags/strategy.rb | 9 + .../operations/feature_flags/user_list.rb | 9 + spec/factories/operations/feature_flags_clients.rb | 7 + .../search/components/state_filter_spec.js | 81 +++++ .../components/file_finder/index_spec.js | 18 +- .../filtered_search_bar_root_spec.js | 114 +++--- .../filtered_search_utils_spec.js | 33 ++ .../components/filtered_search_bar/mock_data.js | 60 ++-- .../tokens/milestone_token_spec.js | 2 +- spec/lib/gitlab/group_search_results_spec.rb | 17 +- .../gitlab/middleware/multipart/handler_spec.rb | 53 +++ spec/lib/gitlab/middleware/multipart_spec.rb | 347 +++++------------- spec/lib/gitlab/project_search_results_spec.rb | 25 +- spec/lib/gitlab/search_results_spec.rb | 24 +- spec/lib/uploaded_file_spec.rb | 2 +- spec/models/operations/feature_flag_scope_spec.rb | 391 +++++++++++++++++++++ spec/models/operations/feature_flag_spec.rb | 258 ++++++++++++++ .../operations/feature_flags/strategy_spec.rb | 323 +++++++++++++++++ .../operations/feature_flags/user_list_spec.rb | 102 ++++++ .../models/operations/feature_flags_client_spec.rb | 21 ++ .../error_tracking/list_projects_service_spec.rb | 2 +- spec/support/forgery_protection.rb | 2 +- spec/support/helpers/feature_flag_helpers.rb | 95 +++++ spec/support/helpers/multipart_helpers.rb | 82 +++++ .../gitlab/middleware/multipart_shared_contexts.rb | 106 ++++-- .../gitlab/middleware/multipart_shared_examples.rb | 145 ++++++++ .../search_issue_state_filter_shared_examples.rb | 48 +++ spec/views/search/_results.html.haml_spec.rb | 6 + yarn.lock | 16 +- 95 files changed, 2950 insertions(+), 588 deletions(-) create mode 100644 app/assets/javascripts/search/state_filter/components/state_filter.vue create mode 100644 app/assets/javascripts/search/state_filter/constants.js create mode 100644 app/assets/javascripts/search/state_filter/index.js create mode 100644 app/models/operations/feature_flag.rb create mode 100644 app/models/operations/feature_flag_scope.rb create mode 100644 app/models/operations/feature_flags/scope.rb create mode 100644 app/models/operations/feature_flags/strategy.rb create mode 100644 app/models/operations/feature_flags/strategy_user_list.rb create mode 100644 app/models/operations/feature_flags/user_list.rb create mode 100644 app/models/operations/feature_flags_client.rb create mode 100644 app/validators/feature_flag_strategies_validator.rb create mode 100644 app/validators/feature_flag_user_xids_validator.rb create mode 100644 changelogs/unreleased/237932-search-ui-implement-issue-scope-results-filter-by-state.yml create mode 100644 changelogs/unreleased/access-modifier-cop.yml create mode 100644 spec/factories/operations/feature_flag_scopes.rb create mode 100644 spec/factories/operations/feature_flags.rb create mode 100644 spec/factories/operations/feature_flags/scope.rb create mode 100644 spec/factories/operations/feature_flags/strategy.rb create mode 100644 spec/factories/operations/feature_flags/user_list.rb create mode 100644 spec/factories/operations/feature_flags_clients.rb create mode 100644 spec/frontend/search/components/state_filter_spec.js create mode 100644 spec/lib/gitlab/middleware/multipart/handler_spec.rb create mode 100644 spec/models/operations/feature_flag_scope_spec.rb create mode 100644 spec/models/operations/feature_flag_spec.rb create mode 100644 spec/models/operations/feature_flags/strategy_spec.rb create mode 100644 spec/models/operations/feature_flags/user_list_spec.rb create mode 100644 spec/models/operations/feature_flags_client_spec.rb create mode 100644 spec/support/helpers/feature_flag_helpers.rb create mode 100644 spec/support/helpers/multipart_helpers.rb create mode 100644 spec/support/shared_examples/lib/gitlab/middleware/multipart_shared_examples.rb create mode 100644 spec/support/shared_examples/lib/gitlab/search_issue_state_filter_shared_examples.rb diff --git a/.rubocop.yml b/.rubocop.yml index 30046ac1b90..2d3afe3a8aa 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -60,6 +60,9 @@ Style/MutableConstant: Style/SafeNavigation: Enabled: false +Style/AccessModifierDeclarations: + AllowModifiersOnSymbols: true + # Frozen String Literal Style/FrozenStringLiteralComment: Enabled: true diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4776936ccce..8c43f0c1d6e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -498,17 +498,6 @@ Security/YAMLLoad: - 'spec/initializers/secret_token_spec.rb' - 'spec/lib/gitlab/prometheus/additional_metrics_parser_spec.rb' -# Offense count: 10 -# Configuration parameters: EnforcedStyle, AllowModifiersOnSymbols. -# SupportedStyles: inline, group -Style/AccessModifierDeclarations: - Exclude: - - 'app/helpers/issues_helper.rb' - - 'app/helpers/lazy_image_tag_helper.rb' - - 'lib/gitlab/cache/request_cache.rb' - - 'lib/gitlab/request_profiler.rb' - - 'spec/support/forgery_protection.rb' - # Offense count: 148 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. @@ -741,10 +730,6 @@ Rails/SaveBang: - 'ee/spec/models/license_spec.rb' - 'ee/spec/models/merge_request_spec.rb' - 'ee/spec/models/merge_train_spec.rb' - - 'ee/spec/models/operations/feature_flag_scope_spec.rb' - - 'ee/spec/models/operations/feature_flag_spec.rb' - - 'ee/spec/models/operations/feature_flags/strategy_spec.rb' - - 'ee/spec/models/operations/feature_flags/user_list_spec.rb' - 'spec/models/packages/package_spec.rb' - 'ee/spec/models/project_ci_cd_setting_spec.rb' - 'ee/spec/models/project_services/github_service_spec.rb' @@ -1124,6 +1109,10 @@ Rails/SaveBang: - 'spec/models/namespace_spec.rb' - 'spec/models/note_spec.rb' - 'spec/models/notification_setting_spec.rb' + - 'spec/models/operations/feature_flag_scope_spec.rb' + - 'spec/models/operations/feature_flag_spec.rb' + - 'spec/models/operations/feature_flags/strategy_spec.rb' + - 'spec/models/operations/feature_flags/user_list_spec.rb' - 'spec/models/pages_domain_spec.rb' - 'spec/models/project_auto_devops_spec.rb' - 'spec/models/project_feature_spec.rb' diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts.js index e4e5f16927b..f820396d05b 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts.js +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts.js @@ -9,13 +9,13 @@ import { refreshCurrentPage, visitUrl } from '../../lib/utils/url_utility'; import findAndFollowLink from '../../lib/utils/navigation_utility'; import { parseBoolean, getCspNonceValue } from '~/lib/utils/common_utils'; -const defaultStopCallback = Mousetrap.stopCallback; -Mousetrap.stopCallback = (e, element, combo) => { +const defaultStopCallback = Mousetrap.prototype.stopCallback; +Mousetrap.prototype.stopCallback = function customStopCallback(e, element, combo) { if (['ctrl+shift+p', 'command+shift+p'].indexOf(combo) !== -1) { return false; } - return defaultStopCallback(e, element, combo); + return defaultStopCallback.call(this, e, element, combo); }; function initToggleButton() { diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts_find_file.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts_find_file.js index 8658081c6c2..f0d2ecfd210 100644 --- a/app/assets/javascripts/behaviors/shortcuts/shortcuts_find_file.js +++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts_find_file.js @@ -5,12 +5,11 @@ export default class ShortcutsFindFile extends ShortcutsNavigation { constructor(projectFindFile) { super(); - const oldStopCallback = Mousetrap.stopCallback; - this.projectFindFile = projectFindFile; + const oldStopCallback = Mousetrap.prototype.stopCallback; - Mousetrap.stopCallback = (e, element, combo) => { + Mousetrap.prototype.stopCallback = function customStopCallback(e, element, combo) { if ( - element === this.projectFindFile.inputElement[0] && + element === projectFindFile.inputElement[0] && (combo === 'up' || combo === 'down' || combo === 'esc' || combo === 'enter') ) { // when press up/down key in textbox, cursor prevent to move to home/end @@ -18,12 +17,12 @@ export default class ShortcutsFindFile extends ShortcutsNavigation { return false; } - return oldStopCallback(e, element, combo); + return oldStopCallback.call(this, e, element, combo); }; - Mousetrap.bind('up', this.projectFindFile.selectRowUp); - Mousetrap.bind('down', this.projectFindFile.selectRowDown); - Mousetrap.bind('esc', this.projectFindFile.goToTree); - Mousetrap.bind('enter', this.projectFindFile.goToBlob); + Mousetrap.bind('up', projectFindFile.selectRowUp); + Mousetrap.bind('down', projectFindFile.selectRowDown); + Mousetrap.bind('esc', projectFindFile.goToTree); + Mousetrap.bind('enter', projectFindFile.goToBlob); } } diff --git a/app/assets/javascripts/pages/search/show/index.js b/app/assets/javascripts/pages/search/show/index.js index 85aaaa2c9da..92d01343bd5 100644 --- a/app/assets/javascripts/pages/search/show/index.js +++ b/app/assets/javascripts/pages/search/show/index.js @@ -1,3 +1,7 @@ import Search from './search'; +import initStateFilter from '~/search/state_filter'; -document.addEventListener('DOMContentLoaded', () => new Search()); +document.addEventListener('DOMContentLoaded', () => { + initStateFilter(); + return new Search(); +}); diff --git a/app/assets/javascripts/search/state_filter/components/state_filter.vue b/app/assets/javascripts/search/state_filter/components/state_filter.vue new file mode 100644 index 00000000000..5245c23843e --- /dev/null +++ b/app/assets/javascripts/search/state_filter/components/state_filter.vue @@ -0,0 +1,91 @@ + + + diff --git a/app/assets/javascripts/search/state_filter/constants.js b/app/assets/javascripts/search/state_filter/constants.js new file mode 100644 index 00000000000..25728486360 --- /dev/null +++ b/app/assets/javascripts/search/state_filter/constants.js @@ -0,0 +1,20 @@ +import { __ } from '~/locale'; + +export const FILTER_HEADER = __('Status'); + +export const FILTER_TEXT = __('Any Status'); + +export const FILTER_STATES = { + ANY: { + label: __('Any'), + value: 'all', + }, + OPEN: { + label: __('Open'), + value: 'opened', + }, + CLOSED: { + label: __('Closed'), + value: 'closed', + }, +}; diff --git a/app/assets/javascripts/search/state_filter/index.js b/app/assets/javascripts/search/state_filter/index.js new file mode 100644 index 00000000000..13708574cfb --- /dev/null +++ b/app/assets/javascripts/search/state_filter/index.js @@ -0,0 +1,34 @@ +import Vue from 'vue'; +import Translate from '~/vue_shared/translate'; +import StateFilter from './components/state_filter.vue'; + +Vue.use(Translate); + +export default () => { + const el = document.getElementById('js-search-filter-by-state'); + + if (!el) return false; + + return new Vue({ + el, + components: { + StateFilter, + }, + data() { + const { dataset } = this.$options.el; + return { + scope: dataset.scope, + state: dataset.state, + }; + }, + + render(createElement) { + return createElement('state-filter', { + props: { + scope: this.scope, + state: this.state, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/vue_shared/components/file_finder/index.vue b/app/assets/javascripts/vue_shared/components/file_finder/index.vue index d6f591ccca1..71d38ad4c42 100644 --- a/app/assets/javascripts/vue_shared/components/file_finder/index.vue +++ b/app/assets/javascripts/vue_shared/components/file_finder/index.vue @@ -9,7 +9,7 @@ export const MAX_FILE_FINDER_RESULTS = 40; export const FILE_FINDER_ROW_HEIGHT = 55; export const FILE_FINDER_EMPTY_ROW_HEIGHT = 33; -const originalStopCallback = Mousetrap.stopCallback; +const originalStopCallback = Mousetrap.prototype.stopCallback; export default { components: { @@ -134,7 +134,18 @@ export default { this.toggle(!this.visible); }); - Mousetrap.stopCallback = (e, el, combo) => this.mousetrapStopCallback(e, el, combo); + Mousetrap.prototype.stopCallback = function customStopCallback(e, el, combo) { + if ( + (combo === 't' && el.classList.contains('dropdown-input-field')) || + el.classList.contains('inputarea') + ) { + return true; + } else if (combo === 'command+p' || combo === 'ctrl+p') { + return false; + } + + return originalStopCallback.call(this, e, el, combo); + }; }, methods: { toggle(visible) { @@ -199,18 +210,6 @@ export default { this.cancelMouseOver = false; this.onMouseOver(index); }, - mousetrapStopCallback(e, el, combo) { - if ( - (combo === 't' && el.classList.contains('dropdown-input-field')) || - el.classList.contains('inputarea') - ) { - return true; - } else if (combo === 'command+p' || combo === 'ctrl+p') { - return false; - } - - return originalStopCallback(e, el, combo); - }, }, }; diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue b/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue index ee293d37b66..dae7c921988 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue @@ -15,7 +15,7 @@ import { deprecatedCreateFlash as createFlash } from '~/flash'; import RecentSearchesStore from '~/filtered_search/stores/recent_searches_store'; import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; -import { stripQuotes } from './filtered_search_utils'; +import { stripQuotes, uniqueTokens } from './filtered_search_utils'; import { SortDirection } from './constants'; export default { @@ -120,10 +120,31 @@ export default { ? __('Sort direction: Ascending') : __('Sort direction: Descending'); }, + /** + * This prop fixes a behaviour affecting GlFilteredSearch + * where selecting duplicate token values leads to history + * dropdown also showing that selection. + */ filteredRecentSearches() { - return this.recentSearchesStorageKey - ? this.recentSearches.filter(item => typeof item !== 'string') - : undefined; + if (this.recentSearchesStorageKey) { + const knownItems = []; + return this.recentSearches.reduce((historyItems, item) => { + // Only include non-string history items (discard items from legacy search) + if (typeof item !== 'string') { + const sanitizedItem = uniqueTokens(item); + const itemString = JSON.stringify(sanitizedItem); + // Only include items which aren't already part of history + if (!knownItems.includes(itemString)) { + historyItems.push(sanitizedItem); + // We're storing string for comparision as doing direct object compare + // won't work due to object reference not being the same. + knownItems.push(itemString); + } + } + return historyItems; + }, []); + } + return undefined; }, }, watch: { @@ -245,12 +266,14 @@ export default { this.recentSearchesService.save(resultantSearches); this.recentSearches = []; }, - handleFilterSubmit(filters) { + handleFilterSubmit() { + const filterTokens = uniqueTokens(this.filterValue); + this.filterValue = filterTokens; if (this.recentSearchesStorageKey) { this.recentSearchesPromise .then(() => { - if (filters.length) { - const resultantSearches = this.recentSearchesStore.addRecentSearch(filters); + if (filterTokens.length) { + const resultantSearches = this.recentSearchesStore.addRecentSearch(filterTokens); this.recentSearchesService.save(resultantSearches); this.recentSearches = resultantSearches; } @@ -260,7 +283,7 @@ export default { }); } this.blurSearchInput(); - this.$emit('onFilter', this.removeQuotesEnclosure(filters)); + this.$emit('onFilter', this.removeQuotesEnclosure(filterTokens)); }, }, }; diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js b/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js index 4a5b8668198..a981c67e7be 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/filtered_search_utils.js @@ -1,3 +1,31 @@ -export const stripQuotes = value => { - return value.includes(' ') ? value.slice(1, -1) : value; +/** + * Strips enclosing quotations from a string if it has one. + * + * @param {String} value String to strip quotes from + * + * @returns {String} String without any enclosure + */ +export const stripQuotes = value => value.replace(/^('|")(.*)('|")$/, '$2'); + +/** + * This method removes duplicate tokens from tokens array. + * + * @param {Array} tokens Array of tokens as defined by `GlFilteredSearch` + * + * @returns {Array} Unique array of tokens + */ +export const uniqueTokens = tokens => { + const knownTokens = []; + return tokens.reduce((uniques, token) => { + if (typeof token === 'object' && token.type !== 'filtered-search-term') { + const tokenString = `${token.type}${token.value.operator}${token.value.data}`; + if (!knownTokens.includes(tokenString)) { + uniques.push(token); + knownTokens.push(tokenString); + } + } else { + uniques.push(token); + } + return uniques; + }, []); }; diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index b8f14a82d96..1b18e5c80be 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -4,7 +4,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic include DiffHelper include RendersNotes - before_action :apply_diff_view_cookie! before_action :commit before_action :define_diff_vars before_action :define_diff_comment_vars, except: [:diffs_batch, :diffs_metadata] diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 35f79f721f5..14a8a4c5961 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -10,8 +10,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo include IssuableCollections include RecordUserLastActivity include SourcegraphDecorator + include DiffHelper skip_before_action :merge_request, only: [:index, :bulk_update] + before_action :apply_diff_view_cookie!, only: [:show] before_action :whitelist_query_limiting, only: [:assign_related_issues, :update] before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort] before_action :authorize_read_actual_head_pipeline!, only: [ diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index bbb624f543b..263cd245436 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -8,7 +8,7 @@ # current_user - which user use # params: # scope: 'created_by_me' or 'assigned_to_me' or 'all' -# state: 'open' or 'closed' or 'all' +# state: 'opened' or 'closed' or 'all' # group_id: integer # project_id: integer # milestone_title: string diff --git a/app/helpers/lazy_image_tag_helper.rb b/app/helpers/lazy_image_tag_helper.rb index ac987a04895..0c5744b46ae 100644 --- a/app/helpers/lazy_image_tag_helper.rb +++ b/app/helpers/lazy_image_tag_helper.rb @@ -25,5 +25,5 @@ module LazyImageTagHelper end # Required for Banzai::Filter::ImageLazyLoadFilter - module_function :placeholder_image + module_function :placeholder_image # rubocop: disable Style/AccessModifierDeclarations end diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 61e0fe19c77..9f3623ad511 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module SearchHelper - SEARCH_PERMITTED_PARAMS = [:search, :scope, :project_id, :group_id, :repository_ref, :snippets].freeze + SEARCH_PERMITTED_PARAMS = [:search, :scope, :project_id, :group_id, :repository_ref, :snippets, :state].freeze def search_autocomplete_opts(term) return unless current_user diff --git a/app/models/operations/feature_flag.rb b/app/models/operations/feature_flag.rb new file mode 100644 index 00000000000..586e9d689a1 --- /dev/null +++ b/app/models/operations/feature_flag.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +module Operations + class FeatureFlag < ApplicationRecord + include AtomicInternalId + include IidRoutes + + self.table_name = 'operations_feature_flags' + + belongs_to :project + + has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.operations_feature_flags&.maximum(:iid) } + + default_value_for :active, true + + # scopes exists only for the first version + has_many :scopes, class_name: 'Operations::FeatureFlagScope' + # strategies exists only for the second version + has_many :strategies, class_name: 'Operations::FeatureFlags::Strategy' + has_many :feature_flag_issues + has_many :issues, through: :feature_flag_issues + has_one :default_scope, -> { where(environment_scope: '*') }, class_name: 'Operations::FeatureFlagScope' + + validates :project, presence: true + validates :name, + presence: true, + length: 2..63, + format: { + with: Gitlab::Regex.feature_flag_regex, + message: Gitlab::Regex.feature_flag_regex_message + } + validates :name, uniqueness: { scope: :project_id } + validates :description, allow_blank: true, length: 0..255 + validate :first_default_scope, on: :create, if: :has_scopes? + validate :version_associations + + before_create :build_default_scope, if: -> { legacy_flag? && scopes.none? } + + accepts_nested_attributes_for :scopes, allow_destroy: true + accepts_nested_attributes_for :strategies, allow_destroy: true + + scope :ordered, -> { order(:name) } + + scope :enabled, -> { where(active: true) } + scope :disabled, -> { where(active: false) } + + enum version: { + legacy_flag: 1, + new_version_flag: 2 + } + + class << self + def preload_relations + preload(:scopes, strategies: :scopes) + end + + def for_unleash_client(project, environment) + includes(strategies: [:scopes, :user_list]) + .where(project: project) + .merge(Operations::FeatureFlags::Scope.on_environment(environment)) + .reorder(:id) + .references(:operations_scopes) + end + end + + def related_issues(current_user, preload:) + issues = ::Issue + .select('issues.*, operations_feature_flags_issues.id AS link_id') + .joins(:feature_flag_issues) + .where('operations_feature_flags_issues.feature_flag_id = ?', id) + .order('operations_feature_flags_issues.id ASC') + .includes(preload) + + Ability.issues_readable_by_user(issues, current_user) + end + + private + + def version_associations + if new_version_flag? && scopes.any? + errors.add(:version_associations, 'version 2 feature flags may not have scopes') + elsif legacy_flag? && strategies.any? + errors.add(:version_associations, 'version 1 feature flags may not have strategies') + end + end + + def first_default_scope + unless scopes.first.environment_scope == '*' + errors.add(:default_scope, 'has to be the first element') + end + end + + def build_default_scope + scopes.build(environment_scope: '*', active: self.active) + end + + def has_scopes? + scopes.any? + end + end +end diff --git a/app/models/operations/feature_flag_scope.rb b/app/models/operations/feature_flag_scope.rb new file mode 100644 index 00000000000..78be29f2531 --- /dev/null +++ b/app/models/operations/feature_flag_scope.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module Operations + class FeatureFlagScope < ApplicationRecord + prepend HasEnvironmentScope + include Gitlab::Utils::StrongMemoize + + self.table_name = 'operations_feature_flag_scopes' + + belongs_to :feature_flag + + validates :environment_scope, uniqueness: { + scope: :feature_flag, + message: "(%{value}) has already been taken" + } + + validates :environment_scope, + if: :default_scope?, on: :update, + inclusion: { in: %w(*), message: 'cannot be changed from default scope' } + + validates :strategies, feature_flag_strategies: true + + before_destroy :prevent_destroy_default_scope, if: :default_scope? + + scope :ordered, -> { order(:id) } + scope :enabled, -> { where(active: true) } + scope :disabled, -> { where(active: false) } + + def self.with_name_and_description + joins(:feature_flag) + .select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description]) + end + + def self.for_unleash_client(project, environment) + select_columns = [ + 'DISTINCT ON (operations_feature_flag_scopes.feature_flag_id) operations_feature_flag_scopes.id', + '(operations_feature_flags.active AND operations_feature_flag_scopes.active) AS active', + 'operations_feature_flag_scopes.strategies', + 'operations_feature_flag_scopes.environment_scope', + 'operations_feature_flag_scopes.created_at', + 'operations_feature_flag_scopes.updated_at' + ] + + select(select_columns) + .with_name_and_description + .where(feature_flag_id: project.operations_feature_flags.select(:id)) + .order(:feature_flag_id) + .on_environment(environment) + .reverse_order + end + + private + + def default_scope? + environment_scope_was == '*' + end + + def prevent_destroy_default_scope + raise ActiveRecord::ReadOnlyRecord, "default scope cannot be destroyed" + end + end +end diff --git a/app/models/operations/feature_flags/scope.rb b/app/models/operations/feature_flags/scope.rb new file mode 100644 index 00000000000..d70101b5e0d --- /dev/null +++ b/app/models/operations/feature_flags/scope.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Operations + module FeatureFlags + class Scope < ApplicationRecord + prepend HasEnvironmentScope + + self.table_name = 'operations_scopes' + + belongs_to :strategy, class_name: 'Operations::FeatureFlags::Strategy' + end + end +end diff --git a/app/models/operations/feature_flags/strategy.rb b/app/models/operations/feature_flags/strategy.rb new file mode 100644 index 00000000000..ff68af9741e --- /dev/null +++ b/app/models/operations/feature_flags/strategy.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Operations + module FeatureFlags + class Strategy < ApplicationRecord + STRATEGY_DEFAULT = 'default' + STRATEGY_GITLABUSERLIST = 'gitlabUserList' + STRATEGY_GRADUALROLLOUTUSERID = 'gradualRolloutUserId' + STRATEGY_USERWITHID = 'userWithId' + STRATEGIES = { + STRATEGY_DEFAULT => [].freeze, + STRATEGY_GITLABUSERLIST => [].freeze, + STRATEGY_GRADUALROLLOUTUSERID => %w[groupId percentage].freeze, + STRATEGY_USERWITHID => ['userIds'].freeze + }.freeze + USERID_MAX_LENGTH = 256 + + self.table_name = 'operations_strategies' + + belongs_to :feature_flag + has_many :scopes, class_name: 'Operations::FeatureFlags::Scope' + has_one :strategy_user_list + has_one :user_list, through: :strategy_user_list + + validates :name, + inclusion: { + in: STRATEGIES.keys, + message: 'strategy name is invalid' + } + + validate :parameters_validations, if: -> { errors[:name].blank? } + validates :user_list, presence: true, if: -> { name == STRATEGY_GITLABUSERLIST } + validates :user_list, absence: true, if: -> { name != STRATEGY_GITLABUSERLIST } + validate :same_project_validation, if: -> { user_list.present? } + + accepts_nested_attributes_for :scopes, allow_destroy: true + + def user_list_id=(user_list_id) + self.user_list = ::Operations::FeatureFlags::UserList.find(user_list_id) + end + + private + + def same_project_validation + unless user_list.project_id == feature_flag.project_id + errors.add(:user_list, 'must belong to the same project') + end + end + + def parameters_validations + validate_parameters_type && + validate_parameters_keys && + validate_parameters_values + end + + def validate_parameters_type + parameters.is_a?(Hash) || parameters_error('parameters are invalid') + end + + def validate_parameters_keys + actual_keys = parameters.keys.sort + expected_keys = STRATEGIES[name].sort + expected_keys == actual_keys || parameters_error('parameters are invalid') + end + + def validate_parameters_values + case name + when STRATEGY_GRADUALROLLOUTUSERID + gradual_rollout_user_id_parameters_validation + when STRATEGY_USERWITHID + FeatureFlagUserXidsValidator.validate_user_xids(self, :parameters, parameters['userIds'], 'userIds') + end + end + + def gradual_rollout_user_id_parameters_validation + percentage = parameters['percentage'] + group_id = parameters['groupId'] + + unless percentage.is_a?(String) && percentage.match(/\A[1-9]?[0-9]\z|\A100\z/) + parameters_error('percentage must be a string between 0 and 100 inclusive') + end + + unless group_id.is_a?(String) && group_id.match(/\A[a-z]{1,32}\z/) + parameters_error('groupId parameter is invalid') + end + end + + def parameters_error(message) + errors.add(:parameters, message) + false + end + end + end +end diff --git a/app/models/operations/feature_flags/strategy_user_list.rb b/app/models/operations/feature_flags/strategy_user_list.rb new file mode 100644 index 00000000000..813b632dd67 --- /dev/null +++ b/app/models/operations/feature_flags/strategy_user_list.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Operations + module FeatureFlags + class StrategyUserList < ApplicationRecord + self.table_name = 'operations_strategies_user_lists' + + belongs_to :strategy + belongs_to :user_list + end + end +end diff --git a/app/models/operations/feature_flags/user_list.rb b/app/models/operations/feature_flags/user_list.rb new file mode 100644 index 00000000000..b9bdcb59d5f --- /dev/null +++ b/app/models/operations/feature_flags/user_list.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Operations + module FeatureFlags + class UserList < ApplicationRecord + include AtomicInternalId + include IidRoutes + + self.table_name = 'operations_user_lists' + + belongs_to :project + has_many :strategy_user_lists + has_many :strategies, through: :strategy_user_lists + + has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.operations_feature_flags_user_lists&.maximum(:iid) }, presence: true + + validates :project, presence: true + validates :name, + presence: true, + uniqueness: { scope: :project_id }, + length: 1..255 + validates :user_xids, feature_flag_user_xids: true + + before_destroy :ensure_no_associated_strategies + + private + + def ensure_no_associated_strategies + if strategies.present? + errors.add(:base, 'User list is associated with a strategy') + throw :abort # rubocop: disable Cop/BanCatchThrow + end + end + end + end +end diff --git a/app/models/operations/feature_flags_client.rb b/app/models/operations/feature_flags_client.rb new file mode 100644 index 00000000000..1c65c3f096e --- /dev/null +++ b/app/models/operations/feature_flags_client.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Operations + class FeatureFlagsClient < ApplicationRecord + include TokenAuthenticatable + + self.table_name = 'operations_feature_flags_clients' + + belongs_to :project + + validates :project, presence: true + validates :token, presence: true + + add_authentication_token_field :token, encrypted: :required + + before_validation :ensure_token! + + def self.find_for_project_and_token(project, token) + return unless project + return unless token + + where(project_id: project).find_by_token(token) + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 89dda183788..a5317d14dd8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -344,6 +344,10 @@ class Project < ApplicationRecord # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/228637 has_many :product_analytics_events, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :operations_feature_flags, class_name: 'Operations::FeatureFlag' + has_one :operations_feature_flags_client, class_name: 'Operations::FeatureFlagsClient' + has_many :operations_feature_flags_user_lists, class_name: 'Operations::FeatureFlags::UserList' + accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature, update_only: true accepts_nested_attributes_for :project_setting, update_only: true diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb index 89f1ec6863b..fab02697cf0 100644 --- a/app/services/search/global_service.rb +++ b/app/services/search/global_service.rb @@ -13,7 +13,8 @@ module Search def execute Gitlab::SearchResults.new(current_user, params[:search], - projects) + projects, + filters: { state: params[:state] }) end def projects diff --git a/app/services/search/group_service.rb b/app/services/search/group_service.rb index 924716b8012..68778aa2768 100644 --- a/app/services/search/group_service.rb +++ b/app/services/search/group_service.rb @@ -15,7 +15,8 @@ module Search current_user, params[:search], projects, - group: group + group: group, + filters: { state: params[:state] } ) end diff --git a/app/services/search/project_service.rb b/app/services/search/project_service.rb index 6e52d59b038..5eba909c23b 100644 --- a/app/services/search/project_service.rb +++ b/app/services/search/project_service.rb @@ -12,7 +12,8 @@ module Search Gitlab::ProjectSearchResults.new(current_user, params[:search], project: project, - repository_ref: params[:repository_ref]) + repository_ref: params[:repository_ref], + filters: { state: params[:state] }) end def scope diff --git a/app/validators/feature_flag_strategies_validator.rb b/app/validators/feature_flag_strategies_validator.rb new file mode 100644 index 00000000000..e542d52c50a --- /dev/null +++ b/app/validators/feature_flag_strategies_validator.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +class FeatureFlagStrategiesValidator < ActiveModel::EachValidator + STRATEGY_DEFAULT = 'default'.freeze + STRATEGY_GRADUALROLLOUTUSERID = 'gradualRolloutUserId'.freeze + STRATEGY_USERWITHID = 'userWithId'.freeze + # Order key names alphabetically + STRATEGIES = { + STRATEGY_DEFAULT => [].freeze, + STRATEGY_GRADUALROLLOUTUSERID => %w[groupId percentage].freeze, + STRATEGY_USERWITHID => ['userIds'].freeze + }.freeze + USERID_MAX_LENGTH = 256 + + def validate_each(record, attribute, value) + return unless value + + if value.is_a?(Array) && value.all? { |s| s.is_a?(Hash) } + value.each do |strategy| + strategy_validations(record, attribute, strategy) + end + else + error(record, attribute, 'must be an array of strategy hashes') + end + end + + private + + def strategy_validations(record, attribute, strategy) + validate_name(record, attribute, strategy) && + validate_parameters_type(record, attribute, strategy) && + validate_parameters_keys(record, attribute, strategy) && + validate_parameters_values(record, attribute, strategy) + end + + def validate_name(record, attribute, strategy) + STRATEGIES.key?(strategy['name']) || error(record, attribute, 'strategy name is invalid') + end + + def validate_parameters_type(record, attribute, strategy) + strategy['parameters'].is_a?(Hash) || error(record, attribute, 'parameters are invalid') + end + + def validate_parameters_keys(record, attribute, strategy) + name, parameters = strategy.values_at('name', 'parameters') + actual_keys = parameters.keys.sort + expected_keys = STRATEGIES[name] + expected_keys == actual_keys || error(record, attribute, 'parameters are invalid') + end + + def validate_parameters_values(record, attribute, strategy) + case strategy['name'] + when STRATEGY_GRADUALROLLOUTUSERID + gradual_rollout_user_id_parameters_validation(record, attribute, strategy) + when STRATEGY_USERWITHID + user_with_id_parameters_validation(record, attribute, strategy) + end + end + + def gradual_rollout_user_id_parameters_validation(record, attribute, strategy) + percentage = strategy.dig('parameters', 'percentage') + group_id = strategy.dig('parameters', 'groupId') + + unless percentage.is_a?(String) && percentage.match(/\A[1-9]?[0-9]\z|\A100\z/) + error(record, attribute, 'percentage must be a string between 0 and 100 inclusive') + end + + unless group_id.is_a?(String) && group_id.match(/\A[a-z]{1,32}\z/) + error(record, attribute, 'groupId parameter is invalid') + end + end + + def user_with_id_parameters_validation(record, attribute, strategy) + user_ids = strategy.dig('parameters', 'userIds') + unless user_ids.is_a?(String) && !user_ids.match(/[\n\r\t]|,,/) && valid_ids?(user_ids.split(",")) + error(record, attribute, "userIds must be a string of unique comma separated values each #{USERID_MAX_LENGTH} characters or less") + end + end + + def valid_ids?(user_ids) + user_ids.uniq.length == user_ids.length && + user_ids.all? { |id| valid_id?(id) } + end + + def valid_id?(user_id) + user_id.present? && + user_id.strip == user_id && + user_id.length <= USERID_MAX_LENGTH + end + + def error(record, attribute, msg) + record.errors.add(attribute, msg) + false + end +end diff --git a/app/validators/feature_flag_user_xids_validator.rb b/app/validators/feature_flag_user_xids_validator.rb new file mode 100644 index 00000000000..a840993a94b --- /dev/null +++ b/app/validators/feature_flag_user_xids_validator.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class FeatureFlagUserXidsValidator < ActiveModel::EachValidator + USERXID_MAX_LENGTH = 256 + + def validate_each(record, attribute, value) + self.class.validate_user_xids(record, attribute, value, attribute) + end + + class << self + def validate_user_xids(record, attribute, user_xids, error_message_attribute_name) + unless user_xids.is_a?(String) && !user_xids.match(/[\n\r\t]|,,/) && valid_xids?(user_xids.split(",")) + record.errors.add(attribute, + "#{error_message_attribute_name} must be a string of unique comma separated values each #{USERXID_MAX_LENGTH} characters or less") + end + end + + private + + def valid_xids?(user_xids) + user_xids.uniq.length == user_xids.length && + user_xids.all? { |xid| valid_xid?(xid) } + end + + def valid_xid?(user_xid) + user_xid.present? && + user_xid.strip == user_xid && + user_xid.length <= USERXID_MAX_LENGTH + end + end +end diff --git a/app/views/search/_results.html.haml b/app/views/search/_results.html.haml index 79f01c61833..e0dbb5135e9 100644 --- a/app/views/search/_results.html.haml +++ b/app/views/search/_results.html.haml @@ -22,6 +22,8 @@ = _("in group %{link_to_group}").html_safe % { link_to_group: link_to_group } = render_if_exists 'shared/promotions/promote_advanced_search' + #js-search-filter-by-state{ 'v-cloak': true, data: { scope: @scope, state: params[:state] } } + .results.gl-mt-3 - if @scope == 'commits' %ul.content-list.commit-list diff --git a/changelogs/unreleased/237932-search-ui-implement-issue-scope-results-filter-by-state.yml b/changelogs/unreleased/237932-search-ui-implement-issue-scope-results-filter-by-state.yml new file mode 100644 index 00000000000..148bcab1524 --- /dev/null +++ b/changelogs/unreleased/237932-search-ui-implement-issue-scope-results-filter-by-state.yml @@ -0,0 +1,5 @@ +--- +title: Search UI Allow issue scope results filtering by state +merge_request: 39881 +author: +type: changed diff --git a/changelogs/unreleased/access-modifier-cop.yml b/changelogs/unreleased/access-modifier-cop.yml new file mode 100644 index 00000000000..b02825c2f7f --- /dev/null +++ b/changelogs/unreleased/access-modifier-cop.yml @@ -0,0 +1,5 @@ +--- +title: Fix Style/AccessModifierDeclarations co cop +merge_request: 41252 +author: Rajendra Kadam +type: fixed diff --git a/config/dependency_decisions.yml b/config/dependency_decisions.yml index 9256b902634..2f0d9066a7a 100644 --- a/config/dependency_decisions.yml +++ b/config/dependency_decisions.yml @@ -313,3 +313,9 @@ :why: "https://github.com/cure53/DOMPurify/blob/main/LICENSE and https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31928#note_346604841" :versions: [] :when: 2020-08-13 13:42:46.508082000 Z +- - :whitelist + - Apache-2.0 WITH LLVM-exception + - :who: Nathan Friend + :why: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40670#note_403946372 + :versions: [] + :when: 2020-08-28 15:01:59.329048917 Z diff --git a/config/routes/project.rb b/config/routes/project.rb index 8c9b1f7f5cd..4ba1f14222e 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -367,6 +367,14 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do post :mark_as_spam end end + + resources :feature_flags, param: :iid do + resources :feature_flag_issues, only: [:index, :create, :destroy], as: 'issues', path: 'issues' + end + resource :feature_flags_client, only: [] do + post :reset_token + end + resources :feature_flags_user_lists, param: :iid, only: [:new, :edit, :show] end # End of the /-/ scope. diff --git a/doc/administration/geo/disaster_recovery/planned_failover.md b/doc/administration/geo/disaster_recovery/planned_failover.md index a0cf263a762..366659ee892 100644 --- a/doc/administration/geo/disaster_recovery/planned_failover.md +++ b/doc/administration/geo/disaster_recovery/planned_failover.md @@ -54,7 +54,7 @@ gitlab-ctl promotion-preflight-checks You can run this command in `force` mode to promote to primary even if preflight checks fail: ```shell -sudo gitlab-ctl promotion-preflight-checks --force +sudo gitlab-ctl promote-to-primary-node --force ``` Each step is described in more detail below. diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 86bc0b57344..c1bc231a1e7 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -2913,6 +2913,11 @@ input DastOnDemandScanCreateInput { """ clientMutationId: String + """ + ID of the scanner profile to be used for the scan. + """ + dastScannerProfileId: DastScannerProfileID + """ ID of the site profile to be used for the scan. """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index b526f0f7361..eb8cad5c44c 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -7889,6 +7889,16 @@ }, "defaultValue": null }, + { + "name": "dastScannerProfileId", + "description": "ID of the scanner profile to be used for the scan.", + "type": { + "kind": "SCALAR", + "name": "DastScannerProfileID", + "ofType": null + }, + "defaultValue": null + }, { "name": "clientMutationId", "description": "A unique identifier for the client performing the mutation.", diff --git a/doc/api/groups.md b/doc/api/groups.md index f2dd9ab81b6..c1556fb8e79 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1170,10 +1170,14 @@ DELETE /groups/:id/share/:group_id ## Push Rules **(STARTER)** +> Introduced in [GitLab Starter](https://about.gitlab.com/pricing/) 13.4. + ### Get group push rules **(STARTER)** Get the [push rules](../user/group/index.md#group-push-rules-starter) of a group. +Only available to group owners and administrators. + ```plaintext GET /groups/:id/push_rule ``` @@ -1215,6 +1219,8 @@ the `commit_committer_check` and `reject_unsigned_commits` parameters: Adds [push rules](../user/group/index.md#group-push-rules-starter) to the specified group. +Only available to group owners and administrators. + ```plaintext POST /groups/:id/push_rule ``` @@ -1260,6 +1266,8 @@ Response: Edit push rules for a specified group. +Only available to group owners and administrators. + ```plaintext PUT /groups/:id/push_rule ``` @@ -1300,3 +1308,17 @@ Response: "max_file_size": 100 } ``` + +### Delete group push rule **(STARTER)** + +Deletes the [push rules](../user/group/index.md#group-push-rules-starter) of a group. + +Only available to group owners and administrators. + +```plaintext +DELETE /groups/:id/push_rule +``` + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) | diff --git a/doc/api/markdown.md b/doc/api/markdown.md index 4e5c8515126..e382ca6b7c8 100644 --- a/doc/api/markdown.md +++ b/doc/api/markdown.md @@ -20,8 +20,8 @@ POST /api/v4/markdown | Attribute | Type | Required | Description | | --------- | ------- | ------------- | ------------------------------------------ | | `text` | string | yes | The Markdown text to render | -| `gfm` | boolean | no (optional) | Render text using GitLab Flavored Markdown. Default is `false` | -| `project` | string | no (optional) | Use `project` as a context when creating references using GitLab Flavored Markdown. [Authentication](README.md#authentication) is required if a project is not public. | +| `gfm` | boolean | no | Render text using GitLab Flavored Markdown. Default is `false` | +| `project` | string | no | Use `project` as a context when creating references using GitLab Flavored Markdown. [Authentication](README.md#authentication) is required if a project is not public. | ```shell curl --header Content-Type:application/json --data '{"text":"Hello world! :tada:", "gfm":true, "project":"group_example/project_example"}' "https://gitlab.example.com/api/v4/markdown" diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 5fc4e85bbf8..bedf4cfa530 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -2975,7 +2975,8 @@ skip the download step. attached to the job when it [succeeds, fails, or always](#artifactswhen). The artifacts will be sent to GitLab after the job finishes and will -be available for download in the GitLab UI. +be available for download in the GitLab UI provided that the size is not +larger than the [maximum artifact size](../../user/gitlab_com/index.md#gitlab-cicd). [Read more about artifacts](../pipelines/job_artifacts.md). diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index 4d296451add..f96ed2e7f57 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -17,50 +17,50 @@ limits](https://about.gitlab.com/handbook/product/product-processes/#introducing ### Insert database plan limits -In the `plan_limits` table, you have to create a new column and insert the -limit values. It's recommended to create separate migration script files. - -1. Add new column to the `plan_limits` table with non-null default value - that represents desired limit, such as: - - ```ruby - add_column(:plan_limits, :project_hooks, :integer, default: 100, null: false) - ``` - - NOTE: **Note:** - Plan limits entries set to `0` mean that limits are not - enabled. You should use this setting only in special and documented circumstances. - -1. (Optionally) Create the database migration that fine-tunes each level with - a desired limit using `create_or_update_plan_limit` migration helper, such as: - - ```ruby - class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2] - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - def up - create_or_update_plan_limit('project_hooks', 'default', 0) - create_or_update_plan_limit('project_hooks', 'free', 10) - create_or_update_plan_limit('project_hooks', 'bronze', 20) - create_or_update_plan_limit('project_hooks', 'silver', 30) - create_or_update_plan_limit('project_hooks', 'gold', 100) - end - - def down - create_or_update_plan_limit('project_hooks', 'default', 0) - create_or_update_plan_limit('project_hooks', 'free', 0) - create_or_update_plan_limit('project_hooks', 'bronze', 0) - create_or_update_plan_limit('project_hooks', 'silver', 0) - create_or_update_plan_limit('project_hooks', 'gold', 0) - end - end - ``` - -NOTE: **Note:** -Some plans exist only on GitLab.com. This will be no-op -for plans that do not exist. +In the `plan_limits` table, create a new column and insert the limit values. +It's recommended to create two separate migration script files. + +1. Add a new column to the `plan_limits` table with non-null default value that + represents desired limit, such as: + + ```ruby + add_column(:plan_limits, :project_hooks, :integer, default: 100, null: false) + ``` + + NOTE: **Note:** + Plan limits entries set to `0` mean that limits are not enabled. You should + use this setting only in special and documented circumstances. + +1. (Optionally) Create the database migration that fine-tunes each level with a + desired limit using `create_or_update_plan_limit` migration helper, such as: + + ```ruby + class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + create_or_update_plan_limit('project_hooks', 'default', 0) + create_or_update_plan_limit('project_hooks', 'free', 10) + create_or_update_plan_limit('project_hooks', 'bronze', 20) + create_or_update_plan_limit('project_hooks', 'silver', 30) + create_or_update_plan_limit('project_hooks', 'gold', 100) + end + + def down + create_or_update_plan_limit('project_hooks', 'default', 0) + create_or_update_plan_limit('project_hooks', 'free', 0) + create_or_update_plan_limit('project_hooks', 'bronze', 0) + create_or_update_plan_limit('project_hooks', 'silver', 0) + create_or_update_plan_limit('project_hooks', 'gold', 0) + end + end + ``` + + NOTE: **Note:** + Some plans exist only on GitLab.com. This will be a no-op for plans + that do not exist. ### Plan limits validation diff --git a/doc/development/documentation/feature_flags.md b/doc/development/documentation/feature_flags.md index 7d6a99d1623..392f478273e 100644 --- a/doc/development/documentation/feature_flags.md +++ b/doc/development/documentation/feature_flags.md @@ -66,7 +66,7 @@ be enabled for a single project, and is not ready for production use: > - It's not recommended for production use. > - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#anchor-to-section). **(CORE ONLY)** -CAUTION: **Caution:** +CAUTION: **Warning:** This feature might not be available to you. Check the **version history** note above for details. (...Regular content goes here...) @@ -125,7 +125,7 @@ use: > - It's recommended for production use. > - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(CORE ONLY)** -CAUTION: **Caution:** +CAUTION: **Warning:** This feature might not be available to you. Check the **version history** note above for details. (...Regular content goes here...) @@ -181,7 +181,7 @@ cannot be enabled for a single project, and is ready for production use: > - It's recommended for production use. > - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(CORE ONLY)** -CAUTION: **Caution:** +CAUTION: **Warning:** This feature might not be available to you. Check the **version history** note above for details. (...Regular content goes here...) @@ -254,7 +254,7 @@ be enabled by project, and is ready for production use: > - It's recommended for production use. > - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(CORE ONLY)** -CAUTION: **Caution:** +CAUTION: **Warning:** This feature might not be available to you. Check the **version history** note above for details. (...Regular content goes here...) diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 81f6ef43372..b099d589b0f 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -874,9 +874,8 @@ For other punctuation rules, please refer to the someone in the Merge Request. - [Avoid using symbols and special characters](https://gitlab.com/gitlab-org/gitlab-docs/-/issues/84) in headers. Whenever possible, they should be plain and short text. -- Avoid adding things that show ephemeral statuses. For example, if a feature is - considered beta or experimental, put this information in a note, not in the - heading. +- When possible, avoid including words that might change in the future. Changing + a heading changes its anchor URL, which affects other linked pages. - When introducing a new document, be careful for the headings to be grammatically and syntactically correct. Mention an [assigned technical writer (TW)](https://about.gitlab.com/handbook/product/product-categories/) for review. diff --git a/doc/user/project/merge_requests/browser_performance_testing.md b/doc/user/project/merge_requests/browser_performance_testing.md index 10457e40e0b..6b16d907fb2 100644 --- a/doc/user/project/merge_requests/browser_performance_testing.md +++ b/doc/user/project/merge_requests/browser_performance_testing.md @@ -93,7 +93,7 @@ you can view the report directly in your browser. You can also customize the jobs with environment variables: - `SITESPEED_IMAGE`: Configure the Docker image to use for the job (default `sitespeedio/sitespeed.io`), but not the image version. -- `SITESPEED_VERSION`: Configure the version of the Docker image to use for the job (default `13.3.0`). +- `SITESPEED_VERSION`: Configure the version of the Docker image to use for the job (default `14.1.0`). - `SITESPEED_OPTIONS`: Configure any additional sitespeed.io options as required (default `nil`). Refer to the [sitespeed.io documentation](https://www.sitespeed.io/documentation/sitespeed.io/configuration/) for more details. For example, you can override the number of runs sitespeed.io @@ -196,13 +196,13 @@ performance: image: docker:git variables: URL: https://example.com - SITESPEED_VERSION: 13.3.0 + SITESPEED_VERSION: 14.1.0 SITESPEED_OPTIONS: '' services: - docker:stable-dind script: - mkdir gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/1.1.0/index.js - mkdir sitespeed-results - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL $SITESPEED_OPTIONS - mv sitespeed-results/data/performance.json performance.json @@ -226,7 +226,7 @@ performance: - docker:stable-dind script: - mkdir gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/1.1.0/index.js - mkdir sitespeed-results - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io sitespeedio/sitespeed.io:6.3.1 --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL - mv sitespeed-results/data/performance.json performance.json diff --git a/doc/user/project/merge_requests/img/browser_performance_testing.png b/doc/user/project/merge_requests/img/browser_performance_testing.png index 016abb89f7c..a3d7022bcfc 100644 Binary files a/doc/user/project/merge_requests/img/browser_performance_testing.png and b/doc/user/project/merge_requests/img/browser_performance_testing.png differ diff --git a/lib/backup/database.rb b/lib/backup/database.rb index 6c0029df27f..e3686947164 100644 --- a/lib/backup/database.rb +++ b/lib/backup/database.rb @@ -70,9 +70,9 @@ module Backup success = $?.success? && status.success? if errors.present? - progress.print "------ BEGIN ERRORS -----".color(:yellow) + progress.print "------ BEGIN ERRORS -----\n".color(:yellow) progress.print errors.join.color(:yellow) - progress.print "------ END ERRORS -------".color(:yellow) + progress.print "------ END ERRORS -------\n".color(:yellow) end report_success(success) @@ -89,12 +89,12 @@ module Backup Open3.popen3(ENV, *cmd) do |stdin, stdout, stderr, thread| stdin.binmode - Thread.new do + out_reader = Thread.new do data = stdout.read $stdout.write(data) end - Thread.new do + err_reader = Thread.new do until (raw_line = stderr.gets).nil? warn(raw_line) # Recent database dumps will use --if-exists with pg_dump @@ -108,8 +108,7 @@ module Backup end stdin.close - - thread.join + [thread, out_reader, err_reader].each(&:join) [thread.value, errors] end end diff --git a/lib/gitlab/cache/request_cache.rb b/lib/gitlab/cache/request_cache.rb index 6e48ca90054..3ad919fbba8 100644 --- a/lib/gitlab/cache/request_cache.rb +++ b/lib/gitlab/cache/request_cache.rb @@ -55,7 +55,7 @@ module Gitlab .join(':') end - private cache_key_method_name + private cache_key_method_name # rubocop: disable Style/AccessModifierDeclarations end end end diff --git a/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml index 8553a940bd7..5edb26a0b56 100644 --- a/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml @@ -7,7 +7,7 @@ performance: variables: DOCKER_TLS_CERTDIR: "" SITESPEED_IMAGE: sitespeedio/sitespeed.io - SITESPEED_VERSION: 13.3.0 + SITESPEED_VERSION: 14.1.0 SITESPEED_OPTIONS: '' services: - docker:19.03.12-dind @@ -20,15 +20,15 @@ performance: fi - export CI_ENVIRONMENT_URL=$(cat environment_url.txt) - mkdir gitlab-exporter - - wget -O gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/1.0.1/index.js + - wget -O gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/1.1.0/index.js - mkdir sitespeed-results - | if [ -f .gitlab-urls.txt ] then sed -i -e 's@^@'"$CI_ENVIRONMENT_URL"'@' .gitlab-urls.txt - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io $SITESPEED_IMAGE:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --outputFolder sitespeed-results .gitlab-urls.txt $SITESPEED_OPTIONS + docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io $SITESPEED_IMAGE:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --cpu --outputFolder sitespeed-results .gitlab-urls.txt $SITESPEED_OPTIONS else - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io $SITESPEED_IMAGE:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --outputFolder sitespeed-results "$CI_ENVIRONMENT_URL" $SITESPEED_OPTIONS + docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io $SITESPEED_IMAGE:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --cpu --outputFolder sitespeed-results "$CI_ENVIRONMENT_URL" $SITESPEED_OPTIONS fi - mv sitespeed-results/data/performance.json browser-performance.json artifacts: diff --git a/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml b/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml index 9dbd9b679a8..e591e3cc1e2 100644 --- a/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Verify/Browser-Performance.gitlab-ci.yml @@ -12,15 +12,15 @@ performance: variables: URL: '' SITESPEED_IMAGE: sitespeedio/sitespeed.io - SITESPEED_VERSION: 13.3.0 + SITESPEED_VERSION: 14.1.0 SITESPEED_OPTIONS: '' services: - docker:stable-dind script: - mkdir gitlab-exporter - - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js + - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/1.1.0/index.js - mkdir sitespeed-results - - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io $SITESPEED_IMAGE:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --outputFolder sitespeed-results $URL $SITESPEED_OPTIONS + - docker run --shm-size=1g --rm -v "$(pwd)":/sitespeed.io $SITESPEED_IMAGE:$SITESPEED_VERSION --plugins.add ./gitlab-exporter --cpu --outputFolder sitespeed-results $URL $SITESPEED_OPTIONS - mv sitespeed-results/data/performance.json browser-performance.json artifacts: paths: diff --git a/lib/gitlab/group_search_results.rb b/lib/gitlab/group_search_results.rb index 27b2307077d..0cc3de297ba 100644 --- a/lib/gitlab/group_search_results.rb +++ b/lib/gitlab/group_search_results.rb @@ -4,10 +4,10 @@ module Gitlab class GroupSearchResults < SearchResults attr_reader :group - def initialize(current_user, query, limit_projects = nil, group:, default_project_filter: false) + def initialize(current_user, query, limit_projects = nil, group:, default_project_filter: false, filters: {}) @group = group - super(current_user, query, limit_projects, default_project_filter: default_project_filter) + super(current_user, query, limit_projects, default_project_filter: default_project_filter, filters: filters) end # rubocop:disable CodeReuse/ActiveRecord diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index 733214a7cce..215a1ff383b 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -57,7 +57,8 @@ module Gitlab yield ensure - @open_files.each(&:close) + @open_files.compact + .each(&:close) end # This function calls itself recursively @@ -122,6 +123,7 @@ module Gitlab def allowed_paths [ + Dir.tmpdir, ::FileUploader.root, ::Gitlab.config.uploads.storage_path, ::JobArtifactUploader.workhorse_upload_path, diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 833065547c6..ded8d4ade3f 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -4,11 +4,11 @@ module Gitlab class ProjectSearchResults < SearchResults attr_reader :project, :repository_ref - def initialize(current_user, query, project:, repository_ref: nil) + def initialize(current_user, query, project:, repository_ref: nil, filters: {}) @project = project @repository_ref = repository_ref.presence - super(current_user, query, [project]) + super(current_user, query, [project], filters: filters) end def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE, preload_method: nil) diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 06e8a9ec649..8e23ac6aca5 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -292,7 +292,14 @@ module Gitlab def base64_regex @base64_regex ||= /(?:[A-Za-z0-9+\/]{4})*(?:[A-Za-z0-9+\/]{2}==|[A-Za-z0-9+\/]{3}=)?/.freeze end + + def feature_flag_regex + /\A[a-z]([-_a-z0-9]*[a-z0-9])?\z/ + end + + def feature_flag_regex_message + "can contain only lowercase letters, digits, '_' and '-'. " \ + "Must start with a letter, and cannot end with '-' or '_'" + end end end - -Gitlab::Regex.prepend_if_ee('EE::Gitlab::Regex') diff --git a/lib/gitlab/request_profiler.rb b/lib/gitlab/request_profiler.rb index dd1482da40d..541d505e735 100644 --- a/lib/gitlab/request_profiler.rb +++ b/lib/gitlab/request_profiler.rb @@ -11,7 +11,7 @@ module Gitlab Profile.new(File.basename(path)) end.select(&:valid?) end - module_function :all + module_function :all # rubocop: disable Style/AccessModifierDeclarations def find(name) file_path = File.join(PROFILES_DIR, name) @@ -19,18 +19,18 @@ module Gitlab Profile.new(name) end - module_function :find + module_function :find # rubocop: disable Style/AccessModifierDeclarations def profile_token Rails.cache.fetch('profile-token') do Devise.friendly_token end end - module_function :profile_token + module_function :profile_token # rubocop: disable Style/AccessModifierDeclarations def remove_all_profiles FileUtils.rm_rf(PROFILES_DIR) end - module_function :remove_all_profiles + module_function :remove_all_profiles # rubocop: disable Style/AccessModifierDeclarations end end diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb index 33f28efe284..06d8dca2f70 100644 --- a/lib/gitlab/search_results.rb +++ b/lib/gitlab/search_results.rb @@ -7,7 +7,7 @@ module Gitlab DEFAULT_PAGE = 1 DEFAULT_PER_PAGE = 20 - attr_reader :current_user, :query + attr_reader :current_user, :query, :filters # Limit search results by passed projects # It allows us to search only for projects user has access to @@ -19,11 +19,12 @@ module Gitlab # query attr_reader :default_project_filter - def initialize(current_user, query, limit_projects = nil, default_project_filter: false) + def initialize(current_user, query, limit_projects = nil, default_project_filter: false, filters: {}) @current_user = current_user @query = query @limit_projects = limit_projects || Project.all @default_project_filter = default_project_filter + @filters = filters end def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE, without_count: true, preload_method: nil) @@ -190,6 +191,8 @@ module Gitlab else params[:search] = query end + + params[:state] = filters[:state] if filters.key?(:state) end end diff --git a/lib/uploaded_file.rb b/lib/uploaded_file.rb index 73029c934f4..9c06daa6c5a 100644 --- a/lib/uploaded_file.rb +++ b/lib/uploaded_file.rb @@ -52,8 +52,7 @@ class UploadedFile elsif path.present? file_path = File.realpath(path) - paths = Array(upload_paths) << Dir.tmpdir - unless self.allowed_path?(file_path, paths.compact) + unless self.allowed_path?(file_path, Array(upload_paths).compact) raise InvalidPathError, "insecure path used '#{file_path}'" end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c300275746c..12ca5ace02f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2957,6 +2957,9 @@ msgstr "" msgid "Any Author" msgstr "" +msgid "Any Status" +msgstr "" + msgid "Any branch" msgstr "" diff --git a/package.json b/package.json index f6583335136..ffdd715fb19 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ "@babel/preset-env": "^7.10.1", "@gitlab/at.js": "1.5.5", "@gitlab/svgs": "1.161.0", - "@gitlab/ui": "20.18.0", + "@gitlab/ui": "20.18.1", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "^6.0.3-1", "@sentry/browser": "^5.10.2", @@ -116,7 +116,7 @@ "monaco-editor": "^0.20.0", "monaco-editor-webpack-plugin": "^1.9.0", "monaco-yaml": "^2.4.1", - "mousetrap": "^1.4.6", + "mousetrap": "1.6.5", "pdfjs-dist": "^2.0.943", "pikaday": "^1.8.0", "popper.js": "^1.16.1", diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 09e5196cb52..91770a00081 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -58,39 +58,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do end end - shared_examples 'persisted preferred diff view cookie' do - context 'with view param' do - before do - go(view: 'parallel') - end - - it 'saves the preferred diff view in a cookie' do - expect(response.cookies['diff_view']).to eq('parallel') - end - - it 'only renders the required view', :aggregate_failures do - diff_files_without_deletions = json_response['diff_files'].reject { |f| f['deleted_file'] } - have_no_inline_diff_lines = satisfy('have no inline diff lines') do |diff_file| - !diff_file.has_key?('highlighted_diff_lines') - end - - expect(diff_files_without_deletions).to all(have_key('parallel_diff_lines')) - expect(diff_files_without_deletions).to all(have_no_inline_diff_lines) - end - end - - context 'when the user cannot view the merge request' do - before do - project.team.truncate - go - end - - it 'returns a 404' do - expect(response).to have_gitlab_http_status(:not_found) - end - end - end - shared_examples "diff note on-demand position creation" do it "updates diff discussion positions" do service = double("service") @@ -155,7 +122,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'forked project with submodules' end - it_behaves_like 'persisted preferred diff view cookie' it_behaves_like 'cached diff collection' it_behaves_like 'diff note on-demand position creation' end @@ -511,7 +477,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do end it_behaves_like 'forked project with submodules' - it_behaves_like 'persisted preferred diff view cookie' it_behaves_like 'cached diff collection' context 'diff unfolding' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 8e1b250cd3c..14d9858318a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -44,6 +44,16 @@ RSpec.describe Projects::MergeRequestsController do get :show, params: params.merge(extra_params) end + context 'with view param' do + before do + go(view: 'parallel') + end + + it 'saves the preferred diff view in a cookie' do + expect(response.cookies['diff_view']).to eq('parallel') + end + end + context 'when merge request is unchecked' do before do merge_request.mark_as_unchecked! diff --git a/spec/factories/clusters/kubernetes_namespaces.rb b/spec/factories/clusters/kubernetes_namespaces.rb index c820bf4da60..efcb3abcb90 100644 --- a/spec/factories/clusters/kubernetes_namespaces.rb +++ b/spec/factories/clusters/kubernetes_namespaces.rb @@ -10,15 +10,18 @@ FactoryBot.define do if cluster.project_type? cluster_project = cluster.cluster_project - kubernetes_namespace.project = cluster_project.project + kubernetes_namespace.project = cluster_project&.project kubernetes_namespace.cluster_project = cluster_project end - kubernetes_namespace.namespace ||= - Gitlab::Kubernetes::DefaultNamespace.new( - cluster, - project: kubernetes_namespace.project - ).from_environment_slug(kubernetes_namespace.environment&.slug) + if kubernetes_namespace.project + kubernetes_namespace.namespace ||= + Gitlab::Kubernetes::DefaultNamespace.new( + cluster, + project: kubernetes_namespace.project + ).from_environment_slug(kubernetes_namespace.environment&.slug) + end + kubernetes_namespace.service_account_name ||= "#{kubernetes_namespace.namespace}-service-account" end diff --git a/spec/factories/draft_note.rb b/spec/factories/draft_note.rb index 24563dc92b7..67a3377a39f 100644 --- a/spec/factories/draft_note.rb +++ b/spec/factories/draft_note.rb @@ -25,7 +25,7 @@ FactoryBot.define do factory :draft_note_on_discussion, traits: [:on_discussion] trait :on_discussion do - discussion_id { create(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion_id } + discussion_id { association(:discussion_note_on_merge_request, noteable: merge_request, project: project).discussion_id } end end end diff --git a/spec/factories/file_uploaders.rb b/spec/factories/file_uploaders.rb index dc888fdd535..f7ceb800f14 100644 --- a/spec/factories/file_uploaders.rb +++ b/spec/factories/file_uploaders.rb @@ -14,7 +14,7 @@ FactoryBot.define do end after(:build) do |uploader, evaluator| - uploader.store!(evaluator.file) + uploader.store!(evaluator.file) if evaluator.project&.persisted? end initialize_with do diff --git a/spec/factories/operations/feature_flag_scopes.rb b/spec/factories/operations/feature_flag_scopes.rb new file mode 100644 index 00000000000..a98c397b8b5 --- /dev/null +++ b/spec/factories/operations/feature_flag_scopes.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :operations_feature_flag_scope, class: 'Operations::FeatureFlagScope' do + association :feature_flag, factory: :operations_feature_flag + active { true } + strategies { [{ name: "default", parameters: {} }] } + sequence(:environment_scope) { |n| "review/patch-#{n}" } + end +end diff --git a/spec/factories/operations/feature_flags.rb b/spec/factories/operations/feature_flags.rb new file mode 100644 index 00000000000..7e43d38a04f --- /dev/null +++ b/spec/factories/operations/feature_flags.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :operations_feature_flag, class: 'Operations::FeatureFlag' do + sequence(:name) { |n| "feature_flag_#{n}" } + project + active { true } + + trait :legacy_flag do + version { Operations::FeatureFlag.versions['legacy_flag'] } + end + + trait :new_version_flag do + version { Operations::FeatureFlag.versions['new_version_flag'] } + end + end +end diff --git a/spec/factories/operations/feature_flags/scope.rb b/spec/factories/operations/feature_flags/scope.rb new file mode 100644 index 00000000000..ef0097c6d08 --- /dev/null +++ b/spec/factories/operations/feature_flags/scope.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :operations_scope, class: 'Operations::FeatureFlags::Scope' do + association :strategy, factory: :operations_strategy + sequence(:environment_scope) { |n| "review/patch-#{n}" } + end +end diff --git a/spec/factories/operations/feature_flags/strategy.rb b/spec/factories/operations/feature_flags/strategy.rb new file mode 100644 index 00000000000..bdb5d9f0f3c --- /dev/null +++ b/spec/factories/operations/feature_flags/strategy.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :operations_strategy, class: 'Operations::FeatureFlags::Strategy' do + association :feature_flag, factory: :operations_feature_flag + name { "default" } + parameters { {} } + end +end diff --git a/spec/factories/operations/feature_flags/user_list.rb b/spec/factories/operations/feature_flags/user_list.rb new file mode 100644 index 00000000000..e87598f0d7c --- /dev/null +++ b/spec/factories/operations/feature_flags/user_list.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :operations_feature_flag_user_list, class: 'Operations::FeatureFlags::UserList' do + association :project, factory: :project + name { 'My User List' } + user_xids { 'user1,user2,user3' } + end +end diff --git a/spec/factories/operations/feature_flags_clients.rb b/spec/factories/operations/feature_flags_clients.rb new file mode 100644 index 00000000000..ca9a28dcfed --- /dev/null +++ b/spec/factories/operations/feature_flags_clients.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :operations_feature_flags_client, class: 'Operations::FeatureFlagsClient' do + project + end +end diff --git a/spec/frontend/search/components/state_filter_spec.js b/spec/frontend/search/components/state_filter_spec.js new file mode 100644 index 00000000000..8d203b3946a --- /dev/null +++ b/spec/frontend/search/components/state_filter_spec.js @@ -0,0 +1,81 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import StateFilter from '~/search/state_filter/components/state_filter.vue'; +import { FILTER_STATES } from '~/search/state_filter/constants'; +import * as urlUtils from '~/lib/utils/url_utility'; + +jest.mock('~/lib/utils/url_utility', () => ({ + visitUrl: jest.fn(), + setUrlParams: jest.fn(), +})); + +function createComponent(props = { scope: 'issues' }) { + return shallowMount(StateFilter, { + propsData: { + ...props, + }, + }); +} + +describe('StateFilter', () => { + let wrapper; + + beforeEach(() => { + wrapper = createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + + const findGlDropdown = () => wrapper.find(GlDropdown); + const findGlDropdownItems = () => findGlDropdown().findAll(GlDropdownItem); + const findDropdownItemsText = () => findGlDropdownItems().wrappers.map(w => w.text()); + const firstDropDownItem = () => findGlDropdownItems().at(0); + + describe('template', () => { + describe.each` + scope | showStateDropdown + ${'issues'} | ${true} + ${'projects'} | ${false} + ${'milestones'} | ${false} + ${'users'} | ${false} + ${'merge_requests'} | ${false} + ${'notes'} | ${false} + ${'wiki_blobs'} | ${false} + ${'blobs'} | ${false} + `(`state dropdown`, ({ scope, showStateDropdown }) => { + beforeEach(() => { + wrapper = createComponent({ scope }); + }); + + it(`does${showStateDropdown ? '' : ' not'} render when scope is ${scope}`, () => { + expect(findGlDropdown().exists()).toBe(showStateDropdown); + }); + }); + + describe('Filter options', () => { + it('renders a dropdown item for each filterOption', () => { + expect(findDropdownItemsText()).toStrictEqual( + Object.keys(FILTER_STATES).map(key => { + return FILTER_STATES[key].label; + }), + ); + }); + + it('clicking a dropdown item calls setUrlParams', () => { + const state = FILTER_STATES[Object.keys(FILTER_STATES)[0]].value; + firstDropDownItem().vm.$emit('click'); + + expect(urlUtils.setUrlParams).toHaveBeenCalledWith({ state }); + }); + + it('clicking a dropdown item calls visitUrl', () => { + firstDropDownItem().vm.$emit('click'); + + expect(urlUtils.visitUrl).toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/spec/frontend/vue_shared/components/file_finder/index_spec.js b/spec/frontend/vue_shared/components/file_finder/index_spec.js index f9e56774526..43208568ac1 100644 --- a/spec/frontend/vue_shared/components/file_finder/index_spec.js +++ b/spec/frontend/vue_shared/components/file_finder/index_spec.js @@ -343,26 +343,36 @@ describe('File finder item spec', () => { it('always allows `command+p` to trigger toggle', () => { expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'command+p'), + Mousetrap.prototype.stopCallback( + null, + vm.$el.querySelector('.dropdown-input-field'), + 'command+p', + ), ).toBe(false); }); it('always allows `ctrl+p` to trigger toggle', () => { expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 'ctrl+p'), + Mousetrap.prototype.stopCallback( + null, + vm.$el.querySelector('.dropdown-input-field'), + 'ctrl+p', + ), ).toBe(false); }); it('onlys handles `t` when focused in input-field', () => { expect( - vm.mousetrapStopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), + Mousetrap.prototype.stopCallback(null, vm.$el.querySelector('.dropdown-input-field'), 't'), ).toBe(true); }); it('stops callback in monaco editor', () => { setFixtures('
'); - expect(vm.mousetrapStopCallback(null, document.querySelector('.inputarea'), 't')).toBe(true); + expect( + Mousetrap.prototype.stopCallback(null, document.querySelector('.inputarea'), 't'), + ).toBe(true); }); }); }); diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_bar_root_spec.js b/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_bar_root_spec.js index 440a93eaf64..2681488f76a 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_bar_root_spec.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_bar_root_spec.js @@ -8,12 +8,27 @@ import { } from '@gitlab/ui'; import FilteredSearchBarRoot from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue'; +import { uniqueTokens } from '~/vue_shared/components/filtered_search_bar/filtered_search_utils'; import { SortDirection } from '~/vue_shared/components/filtered_search_bar/constants'; import RecentSearchesStore from '~/filtered_search/stores/recent_searches_store'; import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; -import { mockAvailableTokens, mockSortOptions, mockHistoryItems } from './mock_data'; +import { + mockAvailableTokens, + mockSortOptions, + mockHistoryItems, + tokenValueAuthor, + tokenValueLabel, + tokenValueMilestone, +} from './mock_data'; + +jest.mock('~/vue_shared/components/filtered_search_bar/filtered_search_utils', () => ({ + uniqueTokens: jest.fn().mockImplementation(tokens => tokens), + stripQuotes: jest.requireActual( + '~/vue_shared/components/filtered_search_bar/filtered_search_utils', + ).stripQuotes, +})); const createComponent = ({ shallow = true, @@ -73,13 +88,21 @@ describe('FilteredSearchBarRoot', () => { describe('computed', () => { describe('tokenSymbols', () => { it('returns a map containing type and symbols from `tokens` prop', () => { - expect(wrapper.vm.tokenSymbols).toEqual({ author_username: '@', label_name: '~' }); + expect(wrapper.vm.tokenSymbols).toEqual({ + author_username: '@', + label_name: '~', + milestone_title: '%', + }); }); }); describe('tokenTitles', () => { it('returns a map containing type and title from `tokens` prop', () => { - expect(wrapper.vm.tokenTitles).toEqual({ author_username: 'Author', label_name: 'Label' }); + expect(wrapper.vm.tokenTitles).toEqual({ + author_username: 'Author', + label_name: 'Label', + milestone_title: 'Milestone', + }); }); }); @@ -131,6 +154,20 @@ describe('FilteredSearchBarRoot', () => { expect(wrapper.vm.filteredRecentSearches[0]).toEqual({ foo: 'bar' }); }); + it('returns array of recent searches sanitizing any duplicate token values', async () => { + wrapper.setData({ + recentSearches: [ + [tokenValueAuthor, tokenValueLabel, tokenValueMilestone, tokenValueLabel], + [tokenValueAuthor, tokenValueMilestone], + ], + }); + + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.filteredRecentSearches).toHaveLength(2); + expect(uniqueTokens).toHaveBeenCalled(); + }); + it('returns undefined when recentSearchesStorageKey prop is not set on component', async () => { wrapper.setProps({ recentSearchesStorageKey: '', @@ -182,40 +219,12 @@ describe('FilteredSearchBarRoot', () => { }); describe('removeQuotesEnclosure', () => { - const mockFilters = [ - { - type: 'author_username', - value: { - data: 'root', - operator: '=', - }, - }, - { - type: 'label_name', - value: { - data: '"Documentation Update"', - operator: '=', - }, - }, - 'foo', - ]; + const mockFilters = [tokenValueAuthor, tokenValueLabel, 'foo']; it('returns filter array with unescaped strings for values which have spaces', () => { expect(wrapper.vm.removeQuotesEnclosure(mockFilters)).toEqual([ - { - type: 'author_username', - value: { - data: 'root', - operator: '=', - }, - }, - { - type: 'label_name', - value: { - data: 'Documentation Update', - operator: '=', - }, - }, + tokenValueAuthor, + tokenValueLabel, 'foo', ]); }); @@ -277,21 +286,26 @@ describe('FilteredSearchBarRoot', () => { }); describe('handleFilterSubmit', () => { - const mockFilters = [ - { - type: 'author_username', - value: { - data: 'root', - operator: '=', - }, - }, - 'foo', - ]; + const mockFilters = [tokenValueAuthor, 'foo']; + + beforeEach(async () => { + wrapper.setData({ + filterValue: mockFilters, + }); + + await wrapper.vm.$nextTick(); + }); + + it('calls `uniqueTokens` on `filterValue` prop to remove duplicates', () => { + wrapper.vm.handleFilterSubmit(); + + expect(uniqueTokens).toHaveBeenCalledWith(wrapper.vm.filterValue); + }); it('calls `recentSearchesStore.addRecentSearch` with serialized value of provided `filters` param', () => { jest.spyOn(wrapper.vm.recentSearchesStore, 'addRecentSearch'); - wrapper.vm.handleFilterSubmit(mockFilters); + wrapper.vm.handleFilterSubmit(); return wrapper.vm.recentSearchesPromise.then(() => { expect(wrapper.vm.recentSearchesStore.addRecentSearch).toHaveBeenCalledWith(mockFilters); @@ -301,7 +315,7 @@ describe('FilteredSearchBarRoot', () => { it('calls `recentSearchesService.save` with array of searches', () => { jest.spyOn(wrapper.vm.recentSearchesService, 'save'); - wrapper.vm.handleFilterSubmit(mockFilters); + wrapper.vm.handleFilterSubmit(); return wrapper.vm.recentSearchesPromise.then(() => { expect(wrapper.vm.recentSearchesService.save).toHaveBeenCalledWith([mockFilters]); @@ -311,7 +325,7 @@ describe('FilteredSearchBarRoot', () => { it('sets `recentSearches` data prop with array of searches', () => { jest.spyOn(wrapper.vm.recentSearchesService, 'save'); - wrapper.vm.handleFilterSubmit(mockFilters); + wrapper.vm.handleFilterSubmit(); return wrapper.vm.recentSearchesPromise.then(() => { expect(wrapper.vm.recentSearches).toEqual([mockFilters]); @@ -329,7 +343,7 @@ describe('FilteredSearchBarRoot', () => { it('emits component event `onFilter` with provided filters param', () => { jest.spyOn(wrapper.vm, 'removeQuotesEnclosure'); - wrapper.vm.handleFilterSubmit(mockFilters); + wrapper.vm.handleFilterSubmit(); expect(wrapper.emitted('onFilter')[0]).toEqual([mockFilters]); expect(wrapper.vm.removeQuotesEnclosure).toHaveBeenCalledWith(mockFilters); @@ -366,7 +380,9 @@ describe('FilteredSearchBarRoot', () => { '.gl-search-box-by-click-menu .gl-search-box-by-click-history-item', ); - expect(searchHistoryItemsEl.at(0).text()).toBe('Author := @tobyLabel := ~Bug"duo"'); + expect(searchHistoryItemsEl.at(0).text()).toBe( + 'Author := @rootLabel := ~bugMilestone := %v1.0"duo"', + ); wrapperFullMount.destroy(); }); diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js b/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js index a857f84adf1..14ffd7b2d85 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/filtered_search_utils_spec.js @@ -1,5 +1,12 @@ import * as filteredSearchUtils from '~/vue_shared/components/filtered_search_bar/filtered_search_utils'; +import { + tokenValueAuthor, + tokenValueLabel, + tokenValueMilestone, + tokenValuePlain, +} from './mock_data'; + describe('Filtered Search Utils', () => { describe('stripQuotes', () => { it.each` @@ -9,6 +16,7 @@ describe('Filtered Search Utils', () => { ${'FooBar'} | ${'FooBar'} ${"Foo'Bar"} | ${"Foo'Bar"} ${'Foo"Bar'} | ${'Foo"Bar'} + ${'Foo Bar'} | ${'Foo Bar'} `( 'returns string $outputValue when called with string $inputValue', ({ inputValue, outputValue }) => { @@ -16,4 +24,29 @@ describe('Filtered Search Utils', () => { }, ); }); + + describe('uniqueTokens', () => { + it('returns tokens array with duplicates removed', () => { + expect( + filteredSearchUtils.uniqueTokens([ + tokenValueAuthor, + tokenValueLabel, + tokenValueMilestone, + tokenValueLabel, + tokenValuePlain, + ]), + ).toHaveLength(4); // Removes 2nd instance of tokenValueLabel + }); + + it('returns tokens array as it is if it does not have duplicates', () => { + expect( + filteredSearchUtils.uniqueTokens([ + tokenValueAuthor, + tokenValueLabel, + tokenValueMilestone, + tokenValuePlain, + ]), + ).toHaveLength(4); + }); + }); }); diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/mock_data.js b/spec/frontend/vue_shared/components/filtered_search_bar/mock_data.js index dcccb1f49b6..0eb90f5529d 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/mock_data.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/mock_data.js @@ -89,36 +89,40 @@ export const mockMilestoneToken = { fetchMilestones: () => Promise.resolve({ data: mockMilestones }), }; -export const mockAvailableTokens = [mockAuthorToken, mockLabelToken]; +export const mockAvailableTokens = [mockAuthorToken, mockLabelToken, mockMilestoneToken]; + +export const tokenValueAuthor = { + type: 'author_username', + value: { + data: 'root', + operator: '=', + }, +}; + +export const tokenValueLabel = { + type: 'label_name', + value: { + operator: '=', + data: 'bug', + }, +}; + +export const tokenValueMilestone = { + type: 'milestone_title', + value: { + operator: '=', + data: 'v1.0', + }, +}; + +export const tokenValuePlain = { + type: 'filtered-search-term', + value: { data: 'foo' }, +}; export const mockHistoryItems = [ - [ - { - type: 'author_username', - value: { - data: 'toby', - operator: '=', - }, - }, - { - type: 'label_name', - value: { - data: 'Bug', - operator: '=', - }, - }, - 'duo', - ], - [ - { - type: 'author_username', - value: { - data: 'root', - operator: '=', - }, - }, - 'si', - ], + [tokenValueAuthor, tokenValueLabel, tokenValueMilestone, 'duo'], + [tokenValueAuthor, 'si'], ]; export const mockSortOptions = [ diff --git a/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js b/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js index e9bc482a8fc..c7eabaf3e8d 100644 --- a/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js +++ b/spec/frontend/vue_shared/components/filtered_search_bar/tokens/milestone_token_spec.js @@ -157,7 +157,7 @@ describe('MilestoneToken', () => { const tokenSegments = wrapper.findAll(GlFilteredSearchTokenSegment); expect(tokenSegments).toHaveLength(3); // Milestone, =, '%"4.0"' - expect(tokenSegments.at(2).text()).toBe(`%"${mockRegularMilestone.title}"`); // "4.0 RC1" + expect(tokenSegments.at(2).text()).toBe(`%${mockRegularMilestone.title}`); // "4.0 RC1" }); it('renders provided defaultMilestones as suggestions', async () => { diff --git a/spec/lib/gitlab/group_search_results_spec.rb b/spec/lib/gitlab/group_search_results_spec.rb index 3c3410c41bf..726df37e3aa 100644 --- a/spec/lib/gitlab/group_search_results_spec.rb +++ b/spec/lib/gitlab/group_search_results_spec.rb @@ -6,9 +6,22 @@ RSpec.describe Gitlab::GroupSearchResults do # group creation calls GroupFinder, so need to create the group # before so expect(GroupsFinder) check works let_it_be(:group) { create(:group) } - let(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let(:filters) { {} } + let(:limit_projects) { Project.all } + let(:query) { 'gob' } - subject(:results) { described_class.new(user, 'gob', anything, group: group) } + subject(:results) { described_class.new(user, query, limit_projects, group: group, filters: filters) } + + describe 'issues search' do + let_it_be(:project) { create(:project, :public, group: group) } + let_it_be(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') } + let_it_be(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') } + let(:query) { 'foo' } + let(:filters) { { state: 'opened' } } + + include_examples 'search issues scope filters by state' + end describe 'user search' do subject(:objects) { results.objects('users') } diff --git a/spec/lib/gitlab/middleware/multipart/handler_spec.rb b/spec/lib/gitlab/middleware/multipart/handler_spec.rb new file mode 100644 index 00000000000..aac3f00defe --- /dev/null +++ b/spec/lib/gitlab/middleware/multipart/handler_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Middleware::Multipart::Handler do + using RSpec::Parameterized::TableSyntax + + let_it_be(:env) { Rack::MockRequest.env_for('/', method: 'post', params: {}) } + let_it_be(:message) { { 'rewritten_fields' => {} } } + + describe '#allowed_paths' do + let_it_be(:expected_allowed_paths) do + [ + Dir.tmpdir, + ::FileUploader.root, + ::Gitlab.config.uploads.storage_path, + ::JobArtifactUploader.workhorse_upload_path, + ::LfsObjectUploader.workhorse_upload_path, + File.join(Rails.root, 'public/uploads/tmp') + ] + end + + let_it_be(:expected_with_packages_path) { expected_allowed_paths + [::Packages::PackageFileUploader.workhorse_upload_path] } + + subject { described_class.new(env, message).send(:allowed_paths) } + + where(:package_features_enabled, :object_storage_enabled, :direct_upload_enabled, :expected_paths) do + false | false | true | :expected_allowed_paths + false | false | false | :expected_allowed_paths + false | true | true | :expected_allowed_paths + false | true | false | :expected_allowed_paths + true | false | true | :expected_with_packages_path + true | false | false | :expected_with_packages_path + true | true | true | :expected_allowed_paths + true | true | false | :expected_with_packages_path + end + + with_them do + before do + stub_config(packages: { + enabled: package_features_enabled, + object_store: { + enabled: object_storage_enabled, + direct_upload: direct_upload_enabled + }, + storage_path: '/any/dir' + }) + end + + it { is_expected.to eq(send(expected_paths)) } + end + end +end diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb index 3b64fe335e8..781f3e0289b 100644 --- a/spec/lib/gitlab/middleware/multipart_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_spec.rb @@ -2,311 +2,138 @@ require 'spec_helper' -require 'tempfile' - RSpec.describe Gitlab::Middleware::Multipart do - include_context 'multipart middleware context' - - RSpec.shared_examples_for 'multipart upload files' do - it 'opens top-level files' do - Tempfile.open('top-level') do |tempfile| - rewritten = { 'file' => tempfile.path } - in_params = { 'file.name' => original_filename, 'file.path' => file_path, 'file.remote_id' => remote_id, 'file.size' => file_size } - env = post_env(rewritten, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') - - expect_uploaded_file(tempfile, %w(file)) + include MultipartHelpers - middleware.call(env) - end + describe '#call' do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:secret) { Gitlab::Workhorse.secret } + let(:issuer) { 'gitlab-workhorse' } + + subject do + env = post_env( + rewritten_fields: rewritten_fields, + params: params, + secret: secret, + issuer: issuer + ) + middleware.call(env) end - it 'opens files one level deep' do - Tempfile.open('one-level') do |tempfile| - rewritten = { 'user[avatar]' => tempfile.path } - in_params = { 'user' => { 'avatar' => { '.name' => original_filename, '.path' => file_path, '.remote_id' => remote_id, '.size' => file_size } } } - env = post_env(rewritten, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') + context 'with remote file mode params' do + let(:mode) { :remote } - expect_uploaded_file(tempfile, %w(user avatar)) + it_behaves_like 'handling all upload parameters conditions' - middleware.call(env) - end - end + context 'and a path set' do + include_context 'with one temporary file for multipart' - it 'opens files two levels deep' do - Tempfile.open('two-levels') do |tempfile| - in_params = { 'project' => { 'milestone' => { 'themesong' => { '.name' => original_filename, '.path' => file_path, '.remote_id' => remote_id, '.size' => file_size } } } } - rewritten = { 'project[milestone][themesong]' => tempfile.path } - env = post_env(rewritten, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') + let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } + let(:params) { upload_parameters_for(key: 'file', filename: filename, remote_id: remote_id).merge('file.path' => '/should/not/be/read') } - expect_uploaded_file(tempfile, %w(project milestone themesong)) - - middleware.call(env) - end - end + it 'builds an UploadedFile' do + expect_uploaded_files(original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file)) - def expect_uploaded_file(tempfile, path) - expect(app).to receive(:call) do |env| - file = get_params(env).dig(*path) - expect(file).to be_a(::UploadedFile) - expect(file.original_filename).to eq(original_filename) - - if remote_id - expect(file.remote_id).to eq(remote_id) - expect(file.path).to be_nil - else - expect(file.path).to eq(File.realpath(tempfile.path)) - expect(file.remote_id).to be_nil + subject end end end - end - RSpec.shared_examples_for 'handling CI artifact upload' do - it 'uploads both file and metadata' do - Tempfile.open('file') do |file| - Tempfile.open('metadata') do |metadata| - rewritten = { 'file' => file.path, 'metadata' => metadata.path } - in_params = { 'file.name' => 'file.txt', 'file.path' => file_path, 'file.remote_id' => file_remote_id, 'file.size' => file_size, 'metadata.name' => 'metadata.gz' } - env = post_env(rewritten, in_params, Gitlab::Workhorse.secret, 'gitlab-workhorse') - - with_expected_uploaded_artifact_files(file, metadata) do |uploaded_file, uploaded_metadata| - expect(uploaded_file).to be_a(::UploadedFile) - expect(uploaded_file.original_filename).to eq('file.txt') - - if file_remote_id - expect(uploaded_file.remote_id).to eq(file_remote_id) - expect(uploaded_file.size).to eq(file_size) - expect(uploaded_file.path).to be_nil - else - expect(uploaded_file.path).to eq(File.realpath(file.path)) - expect(uploaded_file.remote_id).to be_nil - end - - expect(uploaded_metadata).to be_a(::UploadedFile) - expect(uploaded_metadata.original_filename).to eq('metadata.gz') - expect(uploaded_metadata.path).to eq(File.realpath(metadata.path)) - expect(uploaded_metadata.remote_id).to be_nil - end - - middleware.call(env) - end - end - end + context 'local file mode' do + let(:mode) { :local } - def with_expected_uploaded_artifact_files(file, metadata) - expect(app).to receive(:call) do |env| - file = get_params(env).dig('file') - metadata = get_params(env).dig('metadata') + it_behaves_like 'handling all upload parameters conditions' - yield file, metadata - end - end - end + context 'when file is' do + include_context 'with one temporary file for multipart' - it 'rejects headers signed with the wrong secret' do - env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, 'x' * 32, 'gitlab-workhorse') + let(:allowed_paths) { [Dir.tmpdir] } - expect { middleware.call(env) }.to raise_error(JWT::VerificationError) - end - - it 'rejects headers signed with the wrong issuer' do - env = post_env({ 'file' => '/var/empty/nonesuch' }, {}, Gitlab::Workhorse.secret, 'acme-inc') - - expect { middleware.call(env) }.to raise_error(JWT::InvalidIssuerError) - end - - context 'with invalid rewritten field' do - invalid_field_names = [ - '[file]', - ';file', - 'file]', - ';file]', - 'file]]', - 'file;;' - ] - - invalid_field_names.each do |invalid_field_name| - it "rejects invalid rewritten field name #{invalid_field_name}" do - env = post_env({ invalid_field_name => nil }, {}, Gitlab::Workhorse.secret, 'gitlab-workhorse') - - expect { middleware.call(env) }.to raise_error(RuntimeError, "invalid field: \"#{invalid_field_name}\"") - end - end - end - - context 'with remote file' do - let(:remote_id) { 'someid' } - let(:file_size) { 300 } - let(:file_path) { '' } - - it_behaves_like 'multipart upload files' - end - - context 'with remote file and a file path set' do - let(:remote_id) { 'someid' } - let(:file_size) { 300 } - let(:file_path) { 'not_a_valid_file_path' } # file path will come from the rewritten_fields - - it_behaves_like 'multipart upload files' - end + before do + expect_next_instance_of(::Gitlab::Middleware::Multipart::Handler) do |handler| + expect(handler).to receive(:allowed_paths).and_return(allowed_paths) + end + end - context 'with local file' do - let(:remote_id) { nil } - let(:file_size) { nil } - let(:file_path) { 'not_a_valid_file_path' } # file path will come from the rewritten_fields + context 'in allowed paths' do + let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename, remote_id: remote_id) } - it_behaves_like 'multipart upload files' - end + it 'builds an UploadedFile' do + expect_uploaded_files(filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file)) - context 'with remote CI artifact upload' do - let(:file_remote_id) { 'someid' } - let(:file_size) { 300 } - let(:file_path) { 'not_a_valid_file_path' } # file path will come from the rewritten_fields + subject + end + end - it_behaves_like 'handling CI artifact upload' - end + context 'not in allowed paths' do + let(:allowed_paths) { [] } - context 'with local CI artifact upload' do - let(:file_remote_id) { nil } - let(:file_size) { nil } - let(:file_path) { 'not_a_valid_file_path' } # file path will come from the rewritten_fields + let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file') } - it_behaves_like 'handling CI artifact upload' - end + it 'returns an error' do + result = subject - it 'allows files in uploads/tmp directory' do - with_tmp_dir('public/uploads/tmp') do |dir, env| - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) + expect(result[0]).to eq(400) + expect(result[2]).to include('insecure path used') + end + end end - - middleware.call(env) end - end - it 'allows files in the job artifact upload path' do - with_tmp_dir('artifacts') do |dir, env| - expect(JobArtifactUploader).to receive(:workhorse_upload_path).and_return(File.join(dir, 'artifacts')) - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) - end + context 'with dummy params in remote mode' do + let(:rewritten_fields) { { 'file' => 'should/not/be/read' } } + let(:params) { upload_parameters_for(key: 'file') } + let(:mode) { :remote } - middleware.call(env) - end - end + context 'with an invalid secret' do + let(:secret) { 'INVALID_SECRET' } - it 'allows files in the lfs upload path' do - with_tmp_dir('lfs-objects') do |dir, env| - expect(LfsObjectUploader).to receive(:workhorse_upload_path).and_return(File.join(dir, 'lfs-objects')) - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) + it { expect { subject }.to raise_error(JWT::VerificationError) } end - middleware.call(env) - end - end + context 'with an invalid issuer' do + let(:issuer) { 'INVALID_ISSUER' } - it 'allows symlinks for uploads dir' do - Tempfile.open('two-levels') do |tempfile| - symlinked_dir = '/some/dir/uploads' - symlinked_path = File.join(symlinked_dir, File.basename(tempfile.path)) - env = post_env({ 'file' => symlinked_path }, { 'file.name' => original_filename, 'file.path' => symlinked_path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') - - allow(FileUploader).to receive(:root).and_return(symlinked_dir) - allow(UploadedFile).to receive(:allowed_paths).and_return([symlinked_dir, Gitlab.config.uploads.storage_path]) - allow(File).to receive(:realpath).and_call_original - allow(File).to receive(:realpath).with(symlinked_dir).and_return(Dir.tmpdir) - allow(File).to receive(:realpath).with(symlinked_path).and_return(tempfile.path) - allow(File).to receive(:exist?).and_call_original - allow(File).to receive(:exist?).with(symlinked_dir).and_return(true) - - # override Dir.tmpdir because this dir is in the list of allowed paths - # and it would match FileUploader.root path (which in this test is linked - # to /tmp too) - allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) - - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) + it { expect { subject }.to raise_error(JWT::InvalidIssuerError) } end - middleware.call(env) - end - end - - describe '#call' do - context 'with packages storage' do - using RSpec::Parameterized::TableSyntax - - let(:storage_path) { 'shared/packages' } - - RSpec.shared_examples 'allowing the multipart upload' do - it 'allows files to be uploaded' do - with_tmp_dir('tmp/uploads', storage_path) do |dir, env| - allow(Packages::PackageFileUploader).to receive(:root).and_return(File.join(dir, storage_path)) + context 'with invalid rewritten field key' do + invalid_keys = [ + '[file]', + ';file', + 'file]', + ';file]', + 'file]]', + 'file;;' + ] - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) - end + invalid_keys.each do |invalid_key| + context invalid_key do + let(:rewritten_fields) { { invalid_key => 'should/not/be/read' } } - middleware.call(env) + it { expect { subject }.to raise_error(RuntimeError, "invalid field: \"#{invalid_key}\"") } end end end - RSpec.shared_examples 'not allowing the multipart upload when package upload path is used' do - it 'does not allow files to be uploaded' do - with_tmp_dir('tmp/uploads', storage_path) do |dir, env| - # with_tmp_dir sets the same workhorse_upload_path for all Uploaders, - # so we have to prevent JobArtifactUploader and LfsObjectUploader to - # allow the tested path - allow(JobArtifactUploader).to receive(:workhorse_upload_path).and_return(Dir.tmpdir) - allow(LfsObjectUploader).to receive(:workhorse_upload_path).and_return(Dir.tmpdir) + context 'with invalid key in parameters' do + include_context 'with one temporary file for multipart' - status, headers, body = middleware.call(env) + let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'wrong_key', filename: filename, remote_id: remote_id) } - expect(status).to eq(400) - expect(headers).to eq({ 'Content-Type' => 'text/plain' }) - expect(body).to start_with('insecure path used') + it 'builds no UploadedFile' do + expect(app).to receive(:call) do |env| + received_params = get_params(env) + expect(received_params['file']).to be_nil + expect(received_params['wrong_key']).to be_nil end - end - end - RSpec.shared_examples 'adding package storage to multipart allowed paths' do - before do - expect(::Packages::PackageFileUploader).to receive(:workhorse_upload_path).and_call_original + subject end - - it_behaves_like 'allowing the multipart upload' - end - - RSpec.shared_examples 'not adding package storage to multipart allowed paths' do - before do - expect(::Packages::PackageFileUploader).not_to receive(:workhorse_upload_path) - end - - it_behaves_like 'not allowing the multipart upload when package upload path is used' - end - - where(:object_storage_enabled, :direct_upload_enabled, :example_name) do - false | true | 'adding package storage to multipart allowed paths' - false | false | 'adding package storage to multipart allowed paths' - true | true | 'not adding package storage to multipart allowed paths' - true | false | 'adding package storage to multipart allowed paths' - end - - with_them do - before do - stub_config(packages: { - enabled: true, - object_store: { - enabled: object_storage_enabled, - direct_upload: direct_upload_enabled - }, - storage_path: storage_path - }) - end - - it_behaves_like params[:example_name] end end end diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index ea66363469a..22383cd993c 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -5,12 +5,13 @@ require 'spec_helper' RSpec.describe Gitlab::ProjectSearchResults do include SearchHelpers - let(:user) { create(:user) } - let(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } let(:query) { 'hello world' } let(:repository_ref) { nil } + let(:filters) { {} } - subject(:results) { described_class.new(user, query, project: project, repository_ref: repository_ref) } + subject(:results) { described_class.new(user, query, project: project, repository_ref: repository_ref, filters: filters) } context 'with a repository_ref' do context 'when empty' do @@ -258,6 +259,24 @@ RSpec.describe Gitlab::ProjectSearchResults do describe "confidential issues" do include_examples "access restricted confidential issues" end + + context 'filtering' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') } + let_it_be(:opened_issue) { create(:issue, :opened, project: project, title: 'foo opened') } + let(:query) { 'foo' } + + include_examples 'search issues scope filters by state' + end + + it 'filters issues when state is provided', :aggregate_failures do + closed_issue = create(:issue, :closed, project: project, title: "Revert: #{issue.title}") + + results = described_class.new(project.creator, query, project: project, filters: { state: 'opened' }) + + expect(results.objects('issues')).not_to include closed_issue + expect(results.objects('issues')).to include issue + end end describe 'notes search' do diff --git a/spec/lib/gitlab/search_results_spec.rb b/spec/lib/gitlab/search_results_spec.rb index c5563027a84..13942493cc5 100644 --- a/spec/lib/gitlab/search_results_spec.rb +++ b/spec/lib/gitlab/search_results_spec.rb @@ -6,13 +6,14 @@ RSpec.describe Gitlab::SearchResults do include ProjectForksHelper include SearchHelpers - let(:user) { create(:user) } - let!(:project) { create(:project, name: 'foo') } - let!(:issue) { create(:issue, project: project, title: 'foo') } - let!(:merge_request) { create(:merge_request, source_project: project, title: 'foo') } - let!(:milestone) { create(:milestone, project: project, title: 'foo') } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, name: 'foo') } + let_it_be(:issue) { create(:issue, project: project, title: 'foo') } + let_it_be(:milestone) { create(:milestone, project: project, title: 'foo') } + let(:merge_request) { create(:merge_request, source_project: project, title: 'foo') } + let(:filters) { {} } - subject(:results) { described_class.new(user, 'foo', Project.all) } + subject(:results) { described_class.new(user, 'foo', Project.all, filters: filters) } context 'as a user with access' do before do @@ -105,10 +106,10 @@ RSpec.describe Gitlab::SearchResults do describe '#limited_issues_count' do it 'runs single SQL query to get the limited amount of issues' do - create(:milestone, project: project, title: 'foo2') + create(:issue, project: project, title: 'foo2') expect(results).to receive(:issues).with(public_only: true).and_call_original - expect(results).not_to receive(:issues).with(no_args).and_call_original + expect(results).not_to receive(:issues).with(no_args) expect(results.limited_issues_count).to eq(1) end @@ -165,6 +166,13 @@ RSpec.describe Gitlab::SearchResults do results.objects('issues') end + + context 'filtering' do + let_it_be(:closed_issue) { create(:issue, :closed, project: project, title: 'foo closed') } + let_it_be(:opened_issue) { create(:issue, :opened, project: project, title: 'foo open') } + + include_examples 'search issues scope filters by state' + end end describe '#users' do diff --git a/spec/lib/uploaded_file_spec.rb b/spec/lib/uploaded_file_spec.rb index 5ff46193b4f..cf2ab04b457 100644 --- a/spec/lib/uploaded_file_spec.rb +++ b/spec/lib/uploaded_file_spec.rb @@ -23,7 +23,7 @@ RSpec.describe UploadedFile do end subject do - described_class.from_params(params, :file, upload_path, file_path_override) + described_class.from_params(params, :file, [upload_path, Dir.tmpdir], file_path_override) end context 'when valid file is specified' do diff --git a/spec/models/operations/feature_flag_scope_spec.rb b/spec/models/operations/feature_flag_scope_spec.rb new file mode 100644 index 00000000000..29d338d8b29 --- /dev/null +++ b/spec/models/operations/feature_flag_scope_spec.rb @@ -0,0 +1,391 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Operations::FeatureFlagScope do + describe 'associations' do + it { is_expected.to belong_to(:feature_flag) } + end + + describe 'validations' do + context 'when duplicate environment scope is going to be created' do + let!(:existing_feature_flag_scope) do + create(:operations_feature_flag_scope) + end + + let(:new_feature_flag_scope) do + build(:operations_feature_flag_scope, + feature_flag: existing_feature_flag_scope.feature_flag, + environment_scope: existing_feature_flag_scope.environment_scope) + end + + it 'validates uniqueness of environment scope' do + new_feature_flag_scope.save + + expect(new_feature_flag_scope.errors[:environment_scope]) + .to include("(#{existing_feature_flag_scope.environment_scope})" \ + " has already been taken") + end + end + + context 'when environment scope of a default scope is updated' do + let!(:feature_flag) { create(:operations_feature_flag) } + let!(:scope_default) { feature_flag.default_scope } + + it 'keeps default scope intact' do + scope_default.update(environment_scope: 'review/*') + + expect(scope_default.errors[:environment_scope]) + .to include("cannot be changed from default scope") + end + end + + context 'when a default scope is destroyed' do + let!(:feature_flag) { create(:operations_feature_flag) } + let!(:scope_default) { feature_flag.default_scope } + + it 'prevents from destroying the default scope' do + expect { scope_default.destroy! }.to raise_error(ActiveRecord::ReadOnlyRecord) + end + end + + describe 'strategy validations' do + it 'handles null strategies which can occur while adding the column during migration' do + scope = create(:operations_feature_flag_scope, active: true) + allow(scope).to receive(:strategies).and_return(nil) + + scope.active = false + scope.save + + expect(scope.errors[:strategies]).to be_empty + end + + it 'validates multiple strategies' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: "default", parameters: {} }, + { name: "invalid", parameters: {} }]) + + expect(scope.errors[:strategies]).not_to be_empty + end + + where(:invalid_value) do + [{}, 600, "bad", [{ name: 'default', parameters: {} }, 300]] + end + with_them do + it 'must be an array of strategy hashes' do + scope = create(:operations_feature_flag_scope) + + scope.strategies = invalid_value + scope.save + + expect(scope.errors[:strategies]).to eq(['must be an array of strategy hashes']) + end + end + + describe 'name' do + using RSpec::Parameterized::TableSyntax + + where(:name, :params, :expected) do + 'default' | {} | [] + 'gradualRolloutUserId' | { groupId: 'mygroup', percentage: '50' } | [] + 'userWithId' | { userIds: 'sam' } | [] + 5 | nil | ['strategy name is invalid'] + nil | nil | ['strategy name is invalid'] + "nothing" | nil | ['strategy name is invalid'] + "" | nil | ['strategy name is invalid'] + 40.0 | nil | ['strategy name is invalid'] + {} | nil | ['strategy name is invalid'] + [] | nil | ['strategy name is invalid'] + end + with_them do + it 'must be one of "default", "gradualRolloutUserId", or "userWithId"' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: name, parameters: params }]) + + expect(scope.errors[:strategies]).to eq(expected) + end + end + end + + describe 'parameters' do + context 'when the strategy name is gradualRolloutUserId' do + it 'must have parameters' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId' }]) + + expect(scope.errors[:strategies]).to eq(['parameters are invalid']) + end + + where(:invalid_parameters) do + [nil, {}, { percentage: '40', groupId: 'mygroup', userIds: '4' }, { percentage: '40' }, + { percentage: '40', groupId: 'mygroup', extra: nil }, { groupId: 'mygroup' }] + end + with_them do + it 'must have valid parameters for the strategy' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId', + parameters: invalid_parameters }]) + + expect(scope.errors[:strategies]).to eq(['parameters are invalid']) + end + end + + it 'allows the parameters in any order' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId', + parameters: { percentage: '10', groupId: 'mygroup' } }]) + + expect(scope.errors[:strategies]).to be_empty + end + + describe 'percentage' do + where(:invalid_value) do + [50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100", + "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", + "\n10", "20\n", "\n100", "100\n", "\n ", nil] + end + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId', + parameters: { groupId: 'mygroup', percentage: invalid_value } }]) + + expect(scope.errors[:strategies]).to eq(['percentage must be a string between 0 and 100 inclusive']) + end + end + + where(:valid_value) do + %w[0 1 10 38 100 93] + end + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId', + parameters: { groupId: 'mygroup', percentage: valid_value } }]) + + expect(scope.errors[:strategies]).to eq([]) + end + end + end + + describe 'groupId' do + where(:invalid_value) do + [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '', '!bad', + '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"] + end + with_them do + it 'must be a string value of up to 32 lowercase characters' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId', + parameters: { groupId: invalid_value, percentage: '40' } }]) + + expect(scope.errors[:strategies]).to eq(['groupId parameter is invalid']) + end + end + + where(:valid_value) do + ["somegroup", "anothergroup", "okay", "g", "a" * 32] + end + with_them do + it 'must be a string value of up to 32 lowercase characters' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'gradualRolloutUserId', + parameters: { groupId: valid_value, percentage: '40' } }]) + + expect(scope.errors[:strategies]).to eq([]) + end + end + end + end + + context 'when the strategy name is userWithId' do + it 'must have parameters' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'userWithId' }]) + + expect(scope.errors[:strategies]).to eq(['parameters are invalid']) + end + + where(:invalid_parameters) do + [nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}] + end + with_them do + it 'must have valid parameters for the strategy' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'userWithId', parameters: invalid_parameters }]) + + expect(scope.errors[:strategies]).to eq(['parameters are invalid']) + end + end + + describe 'userIds' do + where(:valid_value) do + ["", "sam", "1", "a", "uuid-of-some-kind", "sam,fred,tom,jane,joe,mike", + "gitlab@example.com", "123,4", "UPPER,Case,charActeRS", "0", + "$valid$email#2345#$%..{}+=-)?\\/@example.com", "spaces allowed", + "a" * 256, "a,#{'b' * 256},ccc", "many spaces"] + end + with_them do + it 'is valid with a string of comma separated values' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'userWithId', parameters: { userIds: valid_value } }]) + + expect(scope.errors[:strategies]).to be_empty + end + end + + where(:invalid_value) do + [1, 2.5, {}, [], nil, "123\n456", "1,2,3,12\t3", "\n", "\n\r", + "joe\r,sam", "1,2,2", "1,,2", "1,2,,,,", "b" * 257, "1, ,2", "tim, ,7", " ", + " ", " ,1", "1, ", " leading,1", "1,trailing ", "1, both ,2"] + end + with_them do + it 'is invalid' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'userWithId', parameters: { userIds: invalid_value } }]) + + expect(scope.errors[:strategies]).to include( + 'userIds must be a string of unique comma separated values each 256 characters or less' + ) + end + end + end + end + + context 'when the strategy name is default' do + it 'must have parameters' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'default' }]) + + expect(scope.errors[:strategies]).to eq(['parameters are invalid']) + end + + where(:invalid_value) do + [{ groupId: "hi", percentage: "7" }, "", "nothing", 7, nil, [], 2.5] + end + with_them do + it 'must be empty' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'default', + parameters: invalid_value }]) + + expect(scope.errors[:strategies]).to eq(['parameters are invalid']) + end + end + + it 'must be empty' do + feature_flag = create(:operations_feature_flag) + scope = described_class.create(feature_flag: feature_flag, + environment_scope: 'production', active: true, + strategies: [{ name: 'default', + parameters: {} }]) + + expect(scope.errors[:strategies]).to be_empty + end + end + end + end + end + + describe '.enabled' do + subject { described_class.enabled } + + let!(:feature_flag_scope) do + create(:operations_feature_flag_scope, active: active) + end + + context 'when scope is active' do + let(:active) { true } + + it 'returns the scope' do + is_expected.to include(feature_flag_scope) + end + end + + context 'when scope is inactive' do + let(:active) { false } + + it 'returns an empty array' do + is_expected.not_to include(feature_flag_scope) + end + end + end + + describe '.disabled' do + subject { described_class.disabled } + + let!(:feature_flag_scope) do + create(:operations_feature_flag_scope, active: active) + end + + context 'when scope is active' do + let(:active) { true } + + it 'returns an empty array' do + is_expected.not_to include(feature_flag_scope) + end + end + + context 'when scope is inactive' do + let(:active) { false } + + it 'returns the scope' do + is_expected.to include(feature_flag_scope) + end + end + end + + describe '.for_unleash_client' do + it 'returns scopes for the specified project' do + project1 = create(:project) + project2 = create(:project) + expected_feature_flag = create(:operations_feature_flag, project: project1) + create(:operations_feature_flag, project: project2) + + scopes = described_class.for_unleash_client(project1, 'sandbox').to_a + + expect(scopes).to contain_exactly(*expected_feature_flag.scopes) + end + + it 'returns a scope that matches exactly over a match with a wild card' do + project = create(:project) + feature_flag = create(:operations_feature_flag, project: project) + create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production*') + expected_scope = create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production') + + scopes = described_class.for_unleash_client(project, 'production').to_a + + expect(scopes).to contain_exactly(expected_scope) + end + end +end diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb new file mode 100644 index 00000000000..83d6c6b95a3 --- /dev/null +++ b/spec/models/operations/feature_flag_spec.rb @@ -0,0 +1,258 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Operations::FeatureFlag do + include FeatureFlagHelpers + + subject { create(:operations_feature_flag) } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_many(:scopes) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } + it { is_expected.to define_enum_for(:version).with_values(legacy_flag: 1, new_version_flag: 2) } + + context 'a version 1 feature flag' do + it 'is valid if associated with Operations::FeatureFlagScope models' do + project = create(:project) + feature_flag = described_class.create({ name: 'test', project: project, version: 1, + scopes_attributes: [{ environment_scope: '*', active: false }] }) + + expect(feature_flag).to be_valid + end + + it 'is invalid if associated with Operations::FeatureFlags::Strategy models' do + project = create(:project) + feature_flag = described_class.create({ name: 'test', project: project, version: 1, + strategies_attributes: [{ name: 'default', parameters: {} }] }) + + expect(feature_flag.errors.messages).to eq({ + version_associations: ["version 1 feature flags may not have strategies"] + }) + end + end + + context 'a version 2 feature flag' do + it 'is invalid if associated with Operations::FeatureFlagScope models' do + project = create(:project) + feature_flag = described_class.create({ name: 'test', project: project, version: 2, + scopes_attributes: [{ environment_scope: '*', active: false }] }) + + expect(feature_flag.errors.messages).to eq({ + version_associations: ["version 2 feature flags may not have scopes"] + }) + end + + it 'is valid if associated with Operations::FeatureFlags::Strategy models' do + project = create(:project) + feature_flag = described_class.create({ name: 'test', project: project, version: 2, + strategies_attributes: [{ name: 'default', parameters: {} }] }) + + expect(feature_flag).to be_valid + end + end + + it_behaves_like 'AtomicInternalId', validate_presence: true do + let(:internal_id_attribute) { :iid } + let(:instance) { build(:operations_feature_flag) } + let(:scope) { :project } + let(:scope_attrs) { { project: instance.project } } + let(:usage) { :operations_feature_flags } + end + end + + describe 'feature flag version' do + it 'defaults to 1 if unspecified' do + project = create(:project) + + feature_flag = described_class.create(name: 'my_flag', project: project, active: true) + + expect(feature_flag).to be_valid + expect(feature_flag.version_before_type_cast).to eq(1) + end + end + + describe 'Scope creation' do + subject { described_class.new(**params) } + + let(:project) { create(:project) } + + let(:params) do + { name: 'test', project: project, scopes_attributes: scopes_attributes } + end + + let(:scopes_attributes) do + [{ environment_scope: '*', active: false }, + { environment_scope: 'review/*', active: true }] + end + + it { is_expected.to be_valid } + + context 'when the first scope is not wildcard' do + let(:scopes_attributes) do + [{ environment_scope: 'review/*', active: true }, + { environment_scope: '*', active: false }] + end + + it { is_expected.not_to be_valid } + end + end + + describe 'the default scope' do + let_it_be(:project) { create(:project) } + + context 'with a version 1 feature flag' do + it 'creates a default scope' do + feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 1 }) + + expect(feature_flag.scopes.count).to eq(1) + expect(feature_flag.scopes.first.environment_scope).to eq('*') + end + + it 'allows specifying the default scope in the parameters' do + feature_flag = described_class.create({ name: 'test', project: project, + scopes_attributes: [{ environment_scope: '*', active: false }, + { environment_scope: 'review/*', active: true }], version: 1 }) + + expect(feature_flag.scopes.count).to eq(2) + expect(feature_flag.scopes.first.environment_scope).to eq('*') + end + end + + context 'with a version 2 feature flag' do + it 'does not create a default scope' do + feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 2 }) + + expect(feature_flag.scopes).to eq([]) + end + end + end + + describe '.enabled' do + subject { described_class.enabled } + + context 'when the feature flag is active' do + let!(:feature_flag) { create(:operations_feature_flag, active: true) } + + it 'returns the flag' do + is_expected.to eq([feature_flag]) + end + end + + context 'when the feature flag is active and all scopes are inactive' do + let!(:feature_flag) { create(:operations_feature_flag, active: true) } + + it 'returns the flag' do + feature_flag.default_scope.update!(active: false) + + is_expected.to eq([feature_flag]) + end + end + + context 'when the feature flag is inactive' do + let!(:feature_flag) { create(:operations_feature_flag, active: false) } + + it 'does not return the flag' do + is_expected.to be_empty + end + end + + context 'when the feature flag is inactive and all scopes are active' do + let!(:feature_flag) { create(:operations_feature_flag, active: false) } + + it 'does not return the flag' do + feature_flag.default_scope.update!(active: true) + + is_expected.to be_empty + end + end + end + + describe '.disabled' do + subject { described_class.disabled } + + context 'when the feature flag is active' do + let!(:feature_flag) { create(:operations_feature_flag, active: true) } + + it 'does not return the flag' do + is_expected.to be_empty + end + end + + context 'when the feature flag is active and all scopes are inactive' do + let!(:feature_flag) { create(:operations_feature_flag, active: true) } + + it 'does not return the flag' do + feature_flag.default_scope.update!(active: false) + + is_expected.to be_empty + end + end + + context 'when the feature flag is inactive' do + let!(:feature_flag) { create(:operations_feature_flag, active: false) } + + it 'returns the flag' do + is_expected.to eq([feature_flag]) + end + end + + context 'when the feature flag is inactive and all scopes are active' do + let!(:feature_flag) { create(:operations_feature_flag, active: false) } + + it 'returns the flag' do + feature_flag.default_scope.update!(active: true) + + is_expected.to eq([feature_flag]) + end + end + end + + describe '.for_unleash_client' do + let_it_be(:project) { create(:project) } + let!(:feature_flag) do + create(:operations_feature_flag, project: project, + name: 'feature1', active: true, version: 2) + end + + let!(:strategy) do + create(:operations_strategy, feature_flag: feature_flag, + name: 'default', parameters: {}) + end + + it 'matches wild cards in the scope' do + create(:operations_scope, strategy: strategy, environment_scope: 'review/*') + + flags = described_class.for_unleash_client(project, 'review/feature-branch') + + expect(flags).to eq([feature_flag]) + end + + it 'matches wild cards case sensitively' do + create(:operations_scope, strategy: strategy, environment_scope: 'Staging/*') + + flags = described_class.for_unleash_client(project, 'staging/feature') + + expect(flags).to eq([]) + end + + it 'returns feature flags ordered by id' do + create(:operations_scope, strategy: strategy, environment_scope: 'production') + feature_flag_b = create(:operations_feature_flag, project: project, + name: 'feature2', active: true, version: 2) + strategy_b = create(:operations_strategy, feature_flag: feature_flag_b, + name: 'default', parameters: {}) + create(:operations_scope, strategy: strategy_b, environment_scope: '*') + + flags = described_class.for_unleash_client(project, 'production') + + expect(flags.map(&:id)).to eq([feature_flag.id, feature_flag_b.id]) + end + end +end diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb new file mode 100644 index 00000000000..04e3ef26e9d --- /dev/null +++ b/spec/models/operations/feature_flags/strategy_spec.rb @@ -0,0 +1,323 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Operations::FeatureFlags::Strategy do + let_it_be(:project) { create(:project) } + + describe 'validations' do + it do + is_expected.to validate_inclusion_of(:name) + .in_array(%w[default gradualRolloutUserId userWithId gitlabUserList]) + .with_message('strategy name is invalid') + end + + describe 'parameters' do + context 'when the strategy name is invalid' do + where(:invalid_name) do + [nil, {}, [], 'nothing', 3] + end + with_them do + it 'skips parameters validation' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: invalid_name, parameters: { bad: 'params' }) + + expect(strategy.errors[:name]).to eq(['strategy name is invalid']) + expect(strategy.errors[:parameters]).to be_empty + end + end + end + + context 'when the strategy name is gradualRolloutUserId' do + where(:invalid_parameters) do + [nil, {}, { percentage: '40', groupId: 'mygroup', userIds: '4' }, { percentage: '40' }, + { percentage: '40', groupId: 'mygroup', extra: nil }, { groupId: 'mygroup' }] + end + with_them do + it 'must have valid parameters for the strategy' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', parameters: invalid_parameters) + + expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) + end + end + + it 'allows the parameters in any order' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + parameters: { percentage: '10', groupId: 'mygroup' }) + + expect(strategy.errors[:parameters]).to be_empty + end + + describe 'percentage' do + where(:invalid_value) do + [50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100", + "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", + "\n10", "20\n", "\n100", "100\n", "\n ", nil] + end + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + parameters: { groupId: 'mygroup', percentage: invalid_value }) + + expect(strategy.errors[:parameters]).to eq(['percentage must be a string between 0 and 100 inclusive']) + end + end + + where(:valid_value) do + %w[0 1 10 38 100 93] + end + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + parameters: { groupId: 'mygroup', percentage: valid_value }) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + + describe 'groupId' do + where(:invalid_value) do + [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '', '!bad', + '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"] + end + with_them do + it 'must be a string value of up to 32 lowercase characters' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + parameters: { groupId: invalid_value, percentage: '40' }) + + expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid']) + end + end + + where(:valid_value) do + ["somegroup", "anothergroup", "okay", "g", "a" * 32] + end + with_them do + it 'must be a string value of up to 32 lowercase characters' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + parameters: { groupId: valid_value, percentage: '40' }) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + end + + context 'when the strategy name is userWithId' do + where(:invalid_parameters) do + [nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}] + end + with_them do + it 'must have valid parameters for the strategy' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'userWithId', parameters: invalid_parameters) + + expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) + end + end + + describe 'userIds' do + where(:valid_value) do + ["", "sam", "1", "a", "uuid-of-some-kind", "sam,fred,tom,jane,joe,mike", + "gitlab@example.com", "123,4", "UPPER,Case,charActeRS", "0", + "$valid$email#2345#$%..{}+=-)?\\/@example.com", "spaces allowed", + "a" * 256, "a,#{'b' * 256},ccc", "many spaces"] + end + with_them do + it 'is valid with a string of comma separated values' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: valid_value }) + + expect(strategy.errors[:parameters]).to be_empty + end + end + + where(:invalid_value) do + [1, 2.5, {}, [], nil, "123\n456", "1,2,3,12\t3", "\n", "\n\r", + "joe\r,sam", "1,2,2", "1,,2", "1,2,,,,", "b" * 257, "1, ,2", "tim, ,7", " ", + " ", " ,1", "1, ", " leading,1", "1,trailing ", "1, both ,2"] + end + with_them do + it 'is invalid' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'userWithId', parameters: { userIds: invalid_value }) + + expect(strategy.errors[:parameters]).to include( + 'userIds must be a string of unique comma separated values each 256 characters or less' + ) + end + end + end + end + + context 'when the strategy name is default' do + where(:invalid_value) do + [{ groupId: "hi", percentage: "7" }, "", "nothing", 7, nil, [], 2.5] + end + with_them do + it 'must be empty' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'default', + parameters: invalid_value) + + expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) + end + end + + it 'must be empty' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'default', + parameters: {}) + + expect(strategy.errors[:parameters]).to be_empty + end + end + + context 'when the strategy name is gitlabUserList' do + where(:invalid_value) do + [{ groupId: "default", percentage: "7" }, "", "nothing", 7, nil, [], 2.5, { userIds: 'user1' }] + end + with_them do + it 'must be empty' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gitlabUserList', + parameters: invalid_value) + + expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) + end + end + + it 'must be empty' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gitlabUserList', + parameters: {}) + + expect(strategy.errors[:parameters]).to be_empty + end + end + end + + describe 'associations' do + context 'when name is gitlabUserList' do + it 'is valid when associated with a user list' do + feature_flag = create(:operations_feature_flag, project: project) + user_list = create(:operations_feature_flag_user_list, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gitlabUserList', + user_list: user_list, + parameters: {}) + + expect(strategy.errors[:user_list]).to be_empty + end + + it 'is invalid without a user list' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gitlabUserList', + parameters: {}) + + expect(strategy.errors[:user_list]).to eq(["can't be blank"]) + end + + it 'is invalid when associated with a user list from another project' do + other_project = create(:project) + feature_flag = create(:operations_feature_flag, project: project) + user_list = create(:operations_feature_flag_user_list, project: other_project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gitlabUserList', + user_list: user_list, + parameters: {}) + + expect(strategy.errors[:user_list]).to eq(['must belong to the same project']) + end + end + + context 'when name is default' do + it 'is invalid when associated with a user list' do + feature_flag = create(:operations_feature_flag, project: project) + user_list = create(:operations_feature_flag_user_list, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'default', + user_list: user_list, + parameters: {}) + + expect(strategy.errors[:user_list]).to eq(['must be blank']) + end + + it 'is valid without a user list' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'default', + parameters: {}) + + expect(strategy.errors[:user_list]).to be_empty + end + end + + context 'when name is userWithId' do + it 'is invalid when associated with a user list' do + feature_flag = create(:operations_feature_flag, project: project) + user_list = create(:operations_feature_flag_user_list, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'userWithId', + user_list: user_list, + parameters: { userIds: 'user1' }) + + expect(strategy.errors[:user_list]).to eq(['must be blank']) + end + + it 'is valid without a user list' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'userWithId', + parameters: { userIds: 'user1' }) + + expect(strategy.errors[:user_list]).to be_empty + end + end + + context 'when name is gradualRolloutUserId' do + it 'is invalid when associated with a user list' do + feature_flag = create(:operations_feature_flag, project: project) + user_list = create(:operations_feature_flag_user_list, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + user_list: user_list, + parameters: { groupId: 'default', percentage: '10' }) + + expect(strategy.errors[:user_list]).to eq(['must be blank']) + end + + it 'is valid without a user list' do + feature_flag = create(:operations_feature_flag, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'gradualRolloutUserId', + parameters: { groupId: 'default', percentage: '10' }) + + expect(strategy.errors[:user_list]).to be_empty + end + end + end + end +end diff --git a/spec/models/operations/feature_flags/user_list_spec.rb b/spec/models/operations/feature_flags/user_list_spec.rb new file mode 100644 index 00000000000..020416aa7bc --- /dev/null +++ b/spec/models/operations/feature_flags/user_list_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Operations::FeatureFlags::UserList do + subject { create(:operations_feature_flag_user_list) } + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } + it { is_expected.to validate_length_of(:name).is_at_least(1).is_at_most(255) } + + describe 'user_xids' do + where(:valid_value) do + ["", "sam", "1", "a", "uuid-of-some-kind", "sam,fred,tom,jane,joe,mike", + "gitlab@example.com", "123,4", "UPPER,Case,charActeRS", "0", + "$valid$email#2345#$%..{}+=-)?\\/@example.com", "spaces allowed", + "a" * 256, "a,#{'b' * 256},ccc", "many spaces"] + end + with_them do + it 'is valid with a string of comma separated values' do + user_list = described_class.create(user_xids: valid_value) + + expect(user_list.errors[:user_xids]).to be_empty + end + end + + where(:typecast_value) do + [1, 2.5, {}, []] + end + with_them do + it 'automatically casts values of other types' do + user_list = described_class.create(user_xids: typecast_value) + + expect(user_list.errors[:user_xids]).to be_empty + expect(user_list.user_xids).to eq(typecast_value.to_s) + end + end + + where(:invalid_value) do + [nil, "123\n456", "1,2,3,12\t3", "\n", "\n\r", + "joe\r,sam", "1,2,2", "1,,2", "1,2,,,,", "b" * 257, "1, ,2", "tim, ,7", " ", + " ", " ,1", "1, ", " leading,1", "1,trailing ", "1, both ,2"] + end + with_them do + it 'is invalid' do + user_list = described_class.create(user_xids: invalid_value) + + expect(user_list.errors[:user_xids]).to include( + 'user_xids must be a string of unique comma separated values each 256 characters or less' + ) + end + end + end + end + + describe 'url_helpers' do + it 'generates paths based on the internal id' do + create(:operations_feature_flag_user_list) + project_b = create(:project) + list_b = create(:operations_feature_flag_user_list, project: project_b) + + path = ::Gitlab::Routing.url_helpers.project_feature_flags_user_list_path(project_b, list_b) + + expect(path).to eq("/#{project_b.full_path}/-/feature_flags_user_lists/#{list_b.iid}") + end + end + + describe '#destroy' do + it 'deletes the model if it is not associated with any feature flag strategies' do + project = create(:project) + user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2') + + user_list.destroy + + expect(described_class.count).to eq(0) + end + + it 'does not delete the model if it is associated with a feature flag strategy' do + project = create(:project) + user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2') + feature_flag = create(:operations_feature_flag, :new_version_flag, project: project) + strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: user_list) + + user_list.destroy + + expect(described_class.count).to eq(1) + expect(::Operations::FeatureFlags::StrategyUserList.count).to eq(1) + expect(strategy.reload.user_list).to eq(user_list) + expect(strategy.valid?).to eq(true) + end + end + + it_behaves_like 'AtomicInternalId' do + let(:internal_id_attribute) { :iid } + let(:instance) { build(:operations_feature_flag_user_list) } + let(:scope) { :project } + let(:scope_attrs) { { project: instance.project } } + let(:usage) { :operations_user_lists } + end +end diff --git a/spec/models/operations/feature_flags_client_spec.rb b/spec/models/operations/feature_flags_client_spec.rb new file mode 100644 index 00000000000..05988d676f3 --- /dev/null +++ b/spec/models/operations/feature_flags_client_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Operations::FeatureFlagsClient do + subject { create(:operations_feature_flags_client) } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + end + + describe '#token' do + it "ensures that token is always set" do + expect(subject.token).not_to be_empty + end + end +end diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb index 8bc632349fa..ce391bd1ca0 100644 --- a/spec/services/error_tracking/list_projects_service_spec.rb +++ b/spec/services/error_tracking/list_projects_service_spec.rb @@ -121,7 +121,7 @@ RSpec.describe ErrorTracking::ListProjectsService do end context 'error_tracking_setting is nil' do - let(:error_tracking_setting) { build(:project_error_tracking_setting) } + let(:error_tracking_setting) { build(:project_error_tracking_setting, project: project) } let(:new_api_url) { new_api_host + 'api/0/projects/org/proj/' } before do diff --git a/spec/support/forgery_protection.rb b/spec/support/forgery_protection.rb index 1d6ea013292..d12e99b17c4 100644 --- a/spec/support/forgery_protection.rb +++ b/spec/support/forgery_protection.rb @@ -8,7 +8,7 @@ module ForgeryProtection ActionController::Base.allow_forgery_protection = false end - module_function :with_forgery_protection + module_function :with_forgery_protection # rubocop: disable Style/AccessModifierDeclarations end RSpec.configure do |config| diff --git a/spec/support/helpers/feature_flag_helpers.rb b/spec/support/helpers/feature_flag_helpers.rb new file mode 100644 index 00000000000..93cd915879b --- /dev/null +++ b/spec/support/helpers/feature_flag_helpers.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module FeatureFlagHelpers + def create_flag(project, name, active = true, description: nil, version: Operations::FeatureFlag.versions['legacy_flag']) + create(:operations_feature_flag, name: name, active: active, version: version, + description: description, project: project) + end + + def create_scope(feature_flag, environment_scope, active = true, strategies = [{ name: "default", parameters: {} }]) + create(:operations_feature_flag_scope, + feature_flag: feature_flag, + environment_scope: environment_scope, + active: active, + strategies: strategies) + end + + def within_feature_flag_row(index) + within ".gl-responsive-table-row:nth-child(#{index + 1})" do + yield + end + end + + def within_feature_flag_scopes + within '.js-feature-flag-environments' do + yield + end + end + + def within_scope_row(index) + within ".gl-responsive-table-row:nth-child(#{index + 1})" do + yield + end + end + + def within_strategy_row(index) + within ".feature-flags-form > fieldset > div[data-testid='feature-flag-strategies'] > div:nth-child(#{index})" do + yield + end + end + + def within_environment_spec + within '.table-section:nth-child(1)' do + yield + end + end + + def within_status + within '.table-section:nth-child(2)' do + yield + end + end + + def within_delete + within '.table-section:nth-child(4)' do + yield + end + end + + def edit_feature_flag_button + find('.js-feature-flag-edit-button') + end + + def delete_strategy_button + find("button[data-testid='delete-strategy-button']") + end + + def add_linked_issue_button + find('.js-issue-count-badge-add-button') + end + + def remove_linked_issue_button + find('.js-issue-item-remove-button') + end + + def status_toggle_button + find('[data-testid="feature-flag-status-toggle"] button') + end + + def expect_status_toggle_button_to_be_checked + expect(page).to have_css('[data-testid="feature-flag-status-toggle"] button.is-checked') + end + + def expect_status_toggle_button_not_to_be_checked + expect(page).to have_css('[data-testid="feature-flag-status-toggle"] button:not(.is-checked)') + end + + def expect_status_toggle_button_to_be_disabled + expect(page).to have_css('[data-testid="feature-flag-status-toggle"] button.is-disabled') + end + + def expect_user_to_see_feature_flags_index_page + expect(page).to have_text('Feature Flags') + expect(page).to have_text('Lists') + end +end diff --git a/spec/support/helpers/multipart_helpers.rb b/spec/support/helpers/multipart_helpers.rb new file mode 100644 index 00000000000..f068d5e102d --- /dev/null +++ b/spec/support/helpers/multipart_helpers.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module MultipartHelpers + def post_env(rewritten_fields:, params:, secret:, issuer:) + token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') + Rack::MockRequest.env_for( + '/', + method: 'post', + params: params, + described_class::RACK_ENV_KEY => token + ) + end + + # This function assumes a `mode` variable to be set + def upload_parameters_for(filepath: nil, key: nil, filename: 'filename', remote_id: 'remote_id') + result = { + "#{key}.name" => filename, + "#{key}.type" => "application/octet-stream", + "#{key}.sha256" => "1234567890" + } + + case mode + when :local + result["#{key}.path"] = filepath + when :remote + result["#{key}.remote_id"] = remote_id + result["#{key}.size"] = 3.megabytes + else + raise ArgumentError, "can't handle #{mode} mode" + end + + result + end + + # This function assumes a `mode` variable to be set + def rewritten_fields_hash(hash) + if mode == :remote + # For remote uploads, workhorse still submits rewritten_fields, + # but all the values are empty strings. + hash.keys.each { |k| hash[k] = '' } + end + + hash + end + + def expect_uploaded_files(uploaded_file_expectations) + expect(app).to receive(:call) do |env| + Array.wrap(uploaded_file_expectations).each do |expectation| + file = get_params(env).dig(*expectation[:params_path]) + expect_uploaded_file(file, expectation) + end + end + end + + # This function assumes a `mode` variable to be set + def expect_uploaded_file(file, expectation) + expect(file).to be_a(::UploadedFile) + expect(file.original_filename).to eq(expectation[:original_filename]) + expect(file.sha256).to eq('1234567890') + + case mode + when :local + expect(file.path).to eq(File.realpath(expectation[:filepath])) + expect(file.remote_id).to be_nil + expect(file.size).to eq(expectation[:size]) + when :remote + expect(file.remote_id).to eq(expectation[:remote_id]) + expect(file.path).to be_nil + expect(file.size).to eq(3.megabytes) + else + raise ArgumentError, "can't handle #{mode} mode" + end + end + + # Rails doesn't combine the GET/POST parameters in + # ActionDispatch::HTTP::Parameters if action_dispatch.request.parameters is set: + # https://github.com/rails/rails/blob/aea6423f013ca48f7704c70deadf2cd6ac7d70a1/actionpack/lib/action_dispatch/http/parameters.rb#L41 + def get_params(env) + req = ActionDispatch::Request.new(env) + req.GET.merge(req.POST) + end +end diff --git a/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb b/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb index f1554ea8e9f..ec5bea34e8b 100644 --- a/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb +++ b/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb @@ -1,42 +1,88 @@ # frozen_string_literal: true -RSpec.shared_context 'multipart middleware context' do - let(:app) { double(:app) } - let(:middleware) { described_class.new(app) } - let(:original_filename) { 'filename' } - - # Rails 5 doesn't combine the GET/POST parameters in - # ActionDispatch::HTTP::Parameters if action_dispatch.request.parameters is set: - # https://github.com/rails/rails/blob/aea6423f013ca48f7704c70deadf2cd6ac7d70a1/actionpack/lib/action_dispatch/http/parameters.rb#L41 - def get_params(env) - req = ActionDispatch::Request.new(env) - req.GET.merge(req.POST) +# This context provides one temporary file for the multipart spec +# +# Here are the available variables: +# - uploaded_file +# - uploaded_filepath +# - filename +# - remote_id +RSpec.shared_context 'with one temporary file for multipart' do |within_tmp_sub_dir: false| + let(:uploaded_filepath) { uploaded_file.path } + + around do |example| + Tempfile.open('uploaded_file2') do |tempfile| + @uploaded_file = tempfile + @filename = 'test_file.png' + @remote_id = 'remote_id' + + example.run + end end - def post_env(rewritten_fields, params, secret, issuer) - token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') - Rack::MockRequest.env_for( - '/', - method: 'post', - params: params, - described_class::RACK_ENV_KEY => token - ) + attr_reader :uploaded_file, :filename, :remote_id +end + +# This context provides two temporary files for the multipart spec +# +# Here are the available variables: +# - uploaded_file +# - uploaded_filepath +# - filename +# - remote_id +# - tmp_sub_dir (only when using within_tmp_sub_dir: true) +# - uploaded_file2 +# - uploaded_filepath2 +# - filename2 +# - remote_id2 +RSpec.shared_context 'with two temporary files for multipart' do + include_context 'with one temporary file for multipart' + + let(:uploaded_filepath2) { uploaded_file2.path } + + around do |example| + Tempfile.open('uploaded_file2') do |tempfile| + @uploaded_file2 = tempfile + @filename2 = 'test_file2.png' + @remote_id2 = 'remote_id2' + + example.run + end end - def with_tmp_dir(uploads_sub_dir, storage_path = '') - Dir.mktmpdir do |dir| - upload_dir = File.join(dir, storage_path, uploads_sub_dir) - FileUtils.mkdir_p(upload_dir) + attr_reader :uploaded_file2, :filename2, :remote_id2 +end + +# This context provides three temporary files for the multipart spec +# +# Here are the available variables: +# - uploaded_file +# - uploaded_filepath +# - filename +# - remote_id +# - tmp_sub_dir (only when using within_tmp_sub_dir: true) +# - uploaded_file2 +# - uploaded_filepath2 +# - filename2 +# - remote_id2 +# - uploaded_file3 +# - uploaded_filepath3 +# - filename3 +# - remote_id3 +RSpec.shared_context 'with three temporary files for multipart' do + include_context 'with two temporary files for multipart' - allow(Rails).to receive(:root).and_return(dir) - allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) - allow(GitlabUploader).to receive(:root).and_return(File.join(dir, storage_path)) + let(:uploaded_filepath3) { uploaded_file3.path } - Tempfile.open('top-level', upload_dir) do |tempfile| - env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') + around do |example| + Tempfile.open('uploaded_file3') do |tempfile| + @uploaded_file3 = tempfile + @filename3 = 'test_file3.png' + @remote_id3 = 'remote_id3' - yield dir, env - end + example.run end end + + attr_reader :uploaded_file3, :filename3, :remote_id3 end diff --git a/spec/support/shared_examples/lib/gitlab/middleware/multipart_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/middleware/multipart_shared_examples.rb new file mode 100644 index 00000000000..6327367fcc2 --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/middleware/multipart_shared_examples.rb @@ -0,0 +1,145 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'handling all upload parameters conditions' do + context 'one root parameter' do + include_context 'with one temporary file for multipart' + + let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'file', filename: filename, remote_id: remote_id) } + + it 'builds an UploadedFile' do + expect_uploaded_files(filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file)) + + subject + end + end + + context 'two root parameters' do + include_context 'with two temporary files for multipart' + + let(:rewritten_fields) { rewritten_fields_hash('file1' => uploaded_filepath, 'file2' => uploaded_filepath2) } + let(:params) do + upload_parameters_for(filepath: uploaded_filepath, key: 'file1', filename: filename, remote_id: remote_id).merge( + upload_parameters_for(filepath: uploaded_filepath2, key: 'file2', filename: filename2, remote_id: remote_id2) + ) + end + + it 'builds UploadedFiles' do + expect_uploaded_files([ + { filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file1) }, + { filepath: uploaded_filepath2, original_filename: filename2, remote_id: remote_id2, size: uploaded_file2.size, params_path: %w(file2) } + ]) + + subject + end + end + + context 'one nested parameter' do + include_context 'with one temporary file for multipart' + + let(:rewritten_fields) { rewritten_fields_hash('user[avatar]' => uploaded_filepath) } + let(:params) { { 'user' => { 'avatar' => upload_parameters_for(filepath: uploaded_filepath, filename: filename, remote_id: remote_id) } } } + + it 'builds an UploadedFile' do + expect_uploaded_files(filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(user avatar)) + + subject + end + end + + context 'two nested parameters' do + include_context 'with two temporary files for multipart' + + let(:rewritten_fields) { rewritten_fields_hash('user[avatar]' => uploaded_filepath, 'user[screenshot]' => uploaded_filepath2) } + let(:params) do + { + 'user' => { + 'avatar' => upload_parameters_for(filepath: uploaded_filepath, filename: filename, remote_id: remote_id), + 'screenshot' => upload_parameters_for(filepath: uploaded_filepath2, filename: filename2, remote_id: remote_id2) + } + } + end + + it 'builds UploadedFiles' do + expect_uploaded_files([ + { filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(user avatar) }, + { filepath: uploaded_filepath2, original_filename: filename2, remote_id: remote_id2, size: uploaded_file2.size, params_path: %w(user screenshot) } + ]) + + subject + end + end + + context 'one deeply nested parameter' do + include_context 'with one temporary file for multipart' + + let(:rewritten_fields) { rewritten_fields_hash('user[avatar][bananas]' => uploaded_filepath) } + let(:params) { { 'user' => { 'avatar' => { 'bananas' => upload_parameters_for(filepath: uploaded_filepath, filename: filename, remote_id: remote_id) } } } } + + it 'builds an UploadedFile' do + expect_uploaded_files(filepath: uploaded_file, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(user avatar bananas)) + + subject + end + end + + context 'two deeply nested parameters' do + include_context 'with two temporary files for multipart' + + let(:rewritten_fields) { rewritten_fields_hash('user[avatar][bananas]' => uploaded_filepath, 'user[friend][ananas]' => uploaded_filepath2) } + let(:params) do + { + 'user' => { + 'avatar' => { + 'bananas' => upload_parameters_for(filepath: uploaded_filepath, filename: filename, remote_id: remote_id) + }, + 'friend' => { + 'ananas' => upload_parameters_for(filepath: uploaded_filepath2, filename: filename2, remote_id: remote_id2) + } + } + } + end + + it 'builds UploadedFiles' do + expect_uploaded_files([ + { filepath: uploaded_file, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(user avatar bananas) }, + { filepath: uploaded_file2, original_filename: filename2, remote_id: remote_id2, size: uploaded_file2.size, params_path: %w(user friend ananas) } + ]) + + subject + end + end + + context 'three parameters nested at different levels' do + include_context 'with three temporary files for multipart' + + let(:rewritten_fields) do + rewritten_fields_hash( + 'file' => uploaded_filepath, + 'user[avatar]' => uploaded_filepath2, + 'user[friend][avatar]' => uploaded_filepath3 + ) + end + + let(:params) do + upload_parameters_for(filepath: uploaded_filepath, filename: filename, key: 'file', remote_id: remote_id).merge( + 'user' => { + 'avatar' => upload_parameters_for(filepath: uploaded_filepath2, filename: filename2, remote_id: remote_id2), + 'friend' => { + 'avatar' => upload_parameters_for(filepath: uploaded_filepath3, filename: filename3, remote_id: remote_id3) + } + } + ) + end + + it 'builds UploadedFiles' do + expect_uploaded_files([ + { filepath: uploaded_filepath, original_filename: filename, remote_id: remote_id, size: uploaded_file.size, params_path: %w(file) }, + { filepath: uploaded_filepath2, original_filename: filename2, remote_id: remote_id2, size: uploaded_file2.size, params_path: %w(user avatar) }, + { filepath: uploaded_filepath3, original_filename: filename3, remote_id: remote_id3, size: uploaded_file3.size, params_path: %w(user friend avatar) } + ]) + + subject + end + end +end diff --git a/spec/support/shared_examples/lib/gitlab/search_issue_state_filter_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/search_issue_state_filter_shared_examples.rb new file mode 100644 index 00000000000..e0e41aca331 --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/search_issue_state_filter_shared_examples.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'search issues scope filters by state' do + context 'state not provided' do + let(:filters) { {} } + + it 'returns opened and closed issues', :aggregate_failures do + expect(results.objects('issues')).to include opened_issue + expect(results.objects('issues')).to include closed_issue + end + end + + context 'all state' do + let(:filters) { { state: 'all' } } + + it 'returns opened and closed issues', :aggregate_failures do + expect(results.objects('issues')).to include opened_issue + expect(results.objects('issues')).to include closed_issue + end + end + + context 'closed state' do + let(:filters) { { state: 'closed' } } + + it 'returns only closed issues', :aggregate_failures do + expect(results.objects('issues')).not_to include opened_issue + expect(results.objects('issues')).to include closed_issue + end + end + + context 'opened state' do + let(:filters) { { state: 'opened' } } + + it 'returns only opened issues', :aggregate_failures do + expect(results.objects('issues')).to include opened_issue + expect(results.objects('issues')).not_to include closed_issue + end + end + + context 'unsupported state' do + let(:filters) { { state: 'hello' } } + + it 'returns only opened issues', :aggregate_failures do + expect(results.objects('issues')).to include opened_issue + expect(results.objects('issues')).to include closed_issue + end + end +end diff --git a/spec/views/search/_results.html.haml_spec.rb b/spec/views/search/_results.html.haml_spec.rb index cbd639c6a20..9e95dc40ff8 100644 --- a/spec/views/search/_results.html.haml_spec.rb +++ b/spec/views/search/_results.html.haml_spec.rb @@ -54,6 +54,12 @@ RSpec.describe 'search/_results' do expect(rendered).to have_selector('[data-track-event=click_text]') expect(rendered).to have_selector('[data-track-property=search_result]') end + + it 'renders the state filter drop down' do + render + + expect(rendered).to have_selector('#js-search-filter-by-state') + end end end end diff --git a/yarn.lock b/yarn.lock index 964fa2e46eb..027cd147c9c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -848,10 +848,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.161.0.tgz#661e8d19862dfba0e4c558e2eb6d64b402c1453e" integrity sha512-qsbboEICn08ZoEoAX/TuYygsFaXlzsCY+CfmdOzqvJbOdfHhVXmrJBxd2hP2qqjTZm2PkbRRmn+03+ce1jvatQ== -"@gitlab/ui@20.18.0": - version "20.18.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-20.18.0.tgz#f308c444bcd2d0f09fb9ca358c97dd8817ea5598" - integrity sha512-JSIK7qHyQf0jAALUn9igOPSi6fIPNZyen7C0L2HFBzi5WIwYNIrV/4/uFfwdG/5fHvtPvTCbjFRRwOrP2IwCNA== +"@gitlab/ui@20.18.1": + version "20.18.1" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-20.18.1.tgz#c8a1e0830b63c056999b9417a499677fd46659af" + integrity sha512-WbLBP6Ni8YxKqlKOZChmedc8uS7MRm5CYg/k3mRUELydF/LoW4/M0CsKwgplW4OJfEQRJr8bvmjiTLAyKAky4g== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" @@ -8371,10 +8371,10 @@ monaco-yaml@^2.4.1: optionalDependencies: prettier "^1.19.1" -mousetrap@^1.4.6: - version "1.4.6" - resolved "https://registry.yarnpkg.com/mousetrap/-/mousetrap-1.4.6.tgz#eaca72e22e56d5b769b7555873b688c3332e390a" - integrity sha1-6spy4i5W1bdpt1VYc7aIwzMuOQo= +mousetrap@1.6.5: + version "1.6.5" + resolved "https://registry.yarnpkg.com/mousetrap/-/mousetrap-1.6.5.tgz#8a766d8c272b08393d5f56074e0b5ec183485bf9" + integrity sha512-QNo4kEepaIBwiT8CDhP98umTetp+JNfQYBWvC1pc6/OAibuXtRcxZ58Qz8skvEHYvURne/7R8T5VoOI7rDsEUA== move-concurrently@^1.0.1: version "1.0.1" -- cgit v1.2.3