diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-04 00:08:18 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-09-04 00:08:18 +0300 |
commit | 692f4b734f1976b690dccb5458c198b5205c51b5 (patch) | |
tree | c6af56b7127850615b9dc5626cefbe665fd96ea9 /spec | |
parent | 592223823c8ebf6e32d98e4b12620ba8ff043cca (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
36 files changed, 2050 insertions, 431 deletions
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('<div class="inputarea"></div>'); - 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', + '.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', + '.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 |