diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 009a1d9dd7c..996e32f79b2 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -1579,7 +1579,7 @@ changes: *code-qa-patterns when: manual allow_failure: true - - <<: *if-dot-com-gitlab-org-schedule-child-pipeline + - <<: *if-dot-com-gitlab-org-schedule allow_failure: true .review:rules:review-stop: diff --git a/app/assets/javascripts/search/sidebar/components/app.vue b/app/assets/javascripts/search/sidebar/components/app.vue index 99cf16c8350..40b6e11577a 100644 --- a/app/assets/javascripts/search/sidebar/components/app.vue +++ b/app/assets/javascripts/search/sidebar/components/app.vue @@ -13,7 +13,7 @@ export default { ConfidentialityFilter, }, computed: { - ...mapState(['query']), + ...mapState(['query', 'sidebarDirty']), showReset() { return this.query.state || this.query.confidential; }, @@ -32,7 +32,7 @@ export default {
- + {{ __('Apply') }} {{ diff --git a/app/assets/javascripts/search/sidebar/constants/state_filter_data.js b/app/assets/javascripts/search/sidebar/constants/state_filter_data.js index 7c9a029ffe4..2f9f8a7cb46 100644 --- a/app/assets/javascripts/search/sidebar/constants/state_filter_data.js +++ b/app/assets/javascripts/search/sidebar/constants/state_filter_data.js @@ -5,7 +5,7 @@ const header = __('Status'); const filters = { ANY: { label: __('Any'), - value: 'all', + value: null, }, OPEN: { label: __('Open'), diff --git a/app/assets/javascripts/search/store/actions.js b/app/assets/javascripts/search/store/actions.js index be64a9278e3..a6af5644681 100644 --- a/app/assets/javascripts/search/store/actions.js +++ b/app/assets/javascripts/search/store/actions.js @@ -2,9 +2,9 @@ import Api from '~/api'; import createFlash from '~/flash'; import { visitUrl, setUrlParams } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; -import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY } from './constants'; +import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY, SIDEBAR_PARAMS } from './constants'; import * as types from './mutation_types'; -import { loadDataFromLS, setFrequentItemToLS, mergeById } from './utils'; +import { loadDataFromLS, setFrequentItemToLS, mergeById, isSidebarDirty } from './utils'; export const fetchGroups = ({ commit }, search) => { commit(types.REQUEST_GROUPS); @@ -86,8 +86,12 @@ export const setFrequentProject = ({ state, commit }, item) => { commit(types.LOAD_FREQUENT_ITEMS, { key: PROJECTS_LOCAL_STORAGE_KEY, data: frequentItems }); }; -export const setQuery = ({ commit }, { key, value }) => { +export const setQuery = ({ state, commit }, { key, value }) => { commit(types.SET_QUERY, { key, value }); + + if (SIDEBAR_PARAMS.includes(key)) { + commit(types.SET_SIDEBAR_DIRTY, isSidebarDirty(state.query, state.urlQuery)); + } }; export const applyQuery = ({ state }) => { diff --git a/app/assets/javascripts/search/store/constants.js b/app/assets/javascripts/search/store/constants.js index 3abf7cac6ba..678bd82c7a6 100644 --- a/app/assets/javascripts/search/store/constants.js +++ b/app/assets/javascripts/search/store/constants.js @@ -1,3 +1,6 @@ +import { stateFilterData } from '~/search/sidebar/constants/state_filter_data'; +import { confidentialFilterData } from '~/search/sidebar/constants/confidential_filter_data'; + export const MAX_FREQUENT_ITEMS = 5; export const MAX_FREQUENCY = 5; @@ -5,3 +8,5 @@ export const MAX_FREQUENCY = 5; export const GROUPS_LOCAL_STORAGE_KEY = 'global-search-frequent-groups'; export const PROJECTS_LOCAL_STORAGE_KEY = 'global-search-frequent-projects'; + +export const SIDEBAR_PARAMS = [stateFilterData.filterParam, confidentialFilterData.filterParam]; diff --git a/app/assets/javascripts/search/store/mutation_types.js b/app/assets/javascripts/search/store/mutation_types.js index 5c1c29dc738..bf1e3e79cba 100644 --- a/app/assets/javascripts/search/store/mutation_types.js +++ b/app/assets/javascripts/search/store/mutation_types.js @@ -7,5 +7,6 @@ export const RECEIVE_PROJECTS_SUCCESS = 'RECEIVE_PROJECTS_SUCCESS'; export const RECEIVE_PROJECTS_ERROR = 'RECEIVE_PROJECTS_ERROR'; export const SET_QUERY = 'SET_QUERY'; +export const SET_SIDEBAR_DIRTY = 'SET_SIDEBAR_DIRTY'; export const LOAD_FREQUENT_ITEMS = 'LOAD_FREQUENT_ITEMS'; diff --git a/app/assets/javascripts/search/store/mutations.js b/app/assets/javascripts/search/store/mutations.js index 63156a89738..5d154fe3aa0 100644 --- a/app/assets/javascripts/search/store/mutations.js +++ b/app/assets/javascripts/search/store/mutations.js @@ -26,6 +26,9 @@ export default { [types.SET_QUERY](state, { key, value }) { state.query[key] = value; }, + [types.SET_SIDEBAR_DIRTY](state, value) { + state.sidebarDirty = value; + }, [types.LOAD_FREQUENT_ITEMS](state, { key, data }) { state.frequentItems[key] = data; }, diff --git a/app/assets/javascripts/search/store/state.js b/app/assets/javascripts/search/store/state.js index 5b1429ccc97..d4005697f35 100644 --- a/app/assets/javascripts/search/store/state.js +++ b/app/assets/javascripts/search/store/state.js @@ -1,6 +1,8 @@ +import { cloneDeep } from 'lodash'; import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY } from './constants'; const createState = ({ query }) => ({ + urlQuery: cloneDeep(query), query, groups: [], fetchingGroups: false, @@ -10,5 +12,6 @@ const createState = ({ query }) => ({ [GROUPS_LOCAL_STORAGE_KEY]: [], [PROJECTS_LOCAL_STORAGE_KEY]: [], }, + sidebarDirty: false, }); export default createState; diff --git a/app/assets/javascripts/search/store/utils.js b/app/assets/javascripts/search/store/utils.js index b00b9bb0f2e..6b56ff0b5e5 100644 --- a/app/assets/javascripts/search/store/utils.js +++ b/app/assets/javascripts/search/store/utils.js @@ -1,5 +1,5 @@ import AccessorUtilities from '../../lib/utils/accessor'; -import { MAX_FREQUENT_ITEMS, MAX_FREQUENCY } from './constants'; +import { MAX_FREQUENT_ITEMS, MAX_FREQUENCY, SIDEBAR_PARAMS } from './constants'; function extractKeys(object, keyList) { return Object.fromEntries(keyList.map((key) => [key, object[key]])); @@ -80,3 +80,13 @@ export const mergeById = (inflatedData, storedData) => { return { ...stored, ...data }; }); }; + +export const isSidebarDirty = (currentQuery, urlQuery) => { + return SIDEBAR_PARAMS.some((param) => { + // userAddParam ensures we don't get a false dirty from null !== undefined + const userAddedParam = !urlQuery[param] && currentQuery[param]; + const userChangedExistingParam = urlQuery[param] && urlQuery[param] !== currentQuery[param]; + + return userAddedParam || userChangedExistingParam; + }); +}; diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index 1c894550a14..4dc9bb8a414 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -341,9 +341,8 @@ Ensure your SAML identity provider sends an attribute statement named `Groups` o ``` NOTE: +The value for `Groups` or `groups` in the SAML response can be either the group name or the group ID. To inspect the SAML response, you can use one of these [SAML debugging tools](#saml-debugging-tools). -Also note that the value for `Groups` or `groups` in the SAML response can be either the group name or -the group ID depending what the IdP sends to GitLab. When SAML SSO is enabled for the top-level group, `Maintainer` and `Owner` level users see a new menu item in group **Settings > SAML Group Links**. You can configure one or more **SAML Group Links** to map diff --git a/doc/user/project/import/bitbucket_server.md b/doc/user/project/import/bitbucket_server.md index 2715804b37a..e7ee5742745 100644 --- a/doc/user/project/import/bitbucket_server.md +++ b/doc/user/project/import/bitbucket_server.md @@ -78,10 +78,7 @@ the author's: - `slug` - `displayName` -If the user is not found by any of these properties, the search falls back to the author's -`email` address. - -Alternatively, if there is also no email address, the project creator is set as the author. +If the user is not found by any of these properties, the project creator is set as the author. ##### Enable or disable User assignment by username diff --git a/doc/user/project/merge_requests/approvals/settings.md b/doc/user/project/merge_requests/approvals/settings.md index 1c56e91ed6b..3c67097e8c6 100644 --- a/doc/user/project/merge_requests/approvals/settings.md +++ b/doc/user/project/merge_requests/approvals/settings.md @@ -135,11 +135,11 @@ To learn more, see [Coverage check approval rule](../../../../ci/pipelines/setti ## Merge request approval settings cascading -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285410) in GitLab 14.4. [Deployed behind the `group_merge_request_approval_settings_feature_flag` flag](../../../../administration/feature_flags.md), disabled by default. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/285410) in GitLab 14.4. [Deployed behind the `group_merge_request_approval_settings_feature_flag` flag](../../../../administration/feature_flags.md), disabled by default. +> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/285410) in GitLab 14.5. FLAG: -On self-managed GitLab, by default this feature is not available. To make it available per group, ask an administrator to [enable the `group_merge_request_approval_settings_feature_flag` flag](../../../../administration/feature_flags.md). On GitLab.com, this feature is not available. -You should not use this feature for production environments +On self-managed GitLab, by default this feature is available. To hide the feature per group, ask an administrator to [disable the feature flag](../../../../administration/feature_flags.md) named `group_merge_request_approval_settings_feature_flag`. On GitLab.com, this feature is available. You can also enforce merge request approval settings: diff --git a/lib/gitlab/bitbucket_server_import/importer.rb b/lib/gitlab/bitbucket_server_import/importer.rb index e0eee64dc58..899e2e6c1c5 100644 --- a/lib/gitlab/bitbucket_server_import/importer.rb +++ b/lib/gitlab/bitbucket_server_import/importer.rb @@ -461,10 +461,14 @@ module Gitlab end def uid(rep_object) - find_user_id(by: :email, value: rep_object.author_email) unless Feature.enabled?(:bitbucket_server_user_mapping_by_username) - - find_user_id(by: :username, value: rep_object.author_username) || + # We want this explicit to only be username on the FF + # Otherwise, match email. + # There should be no default fall-through on username. Fall-through to import user + if Feature.enabled?(:bitbucket_server_user_mapping_by_username) + find_user_id(by: :username, value: rep_object.author_username) + else find_user_id(by: :email, value: rep_object.author_email) + end end end end diff --git a/spec/frontend/search/sidebar/components/app_spec.js b/spec/frontend/search/sidebar/components/app_spec.js index b93527c1fe9..58c5929f3de 100644 --- a/spec/frontend/search/sidebar/components/app_spec.js +++ b/spec/frontend/search/sidebar/components/app_spec.js @@ -1,13 +1,13 @@ import { GlButton, GlLink } from '@gitlab/ui'; -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; +import Vue from 'vue'; import Vuex from 'vuex'; import { MOCK_QUERY } from 'jest/search/mock_data'; import GlobalSearchSidebar from '~/search/sidebar/components/app.vue'; import ConfidentialityFilter from '~/search/sidebar/components/confidentiality_filter.vue'; import StatusFilter from '~/search/sidebar/components/status_filter.vue'; -const localVue = createLocalVue(); -localVue.use(Vuex); +Vue.use(Vuex); describe('GlobalSearchSidebar', () => { let wrapper; @@ -27,21 +27,19 @@ describe('GlobalSearchSidebar', () => { }); wrapper = shallowMount(GlobalSearchSidebar, { - localVue, store, }); }; afterEach(() => { wrapper.destroy(); - wrapper = null; }); const findSidebarForm = () => wrapper.find('form'); - const findStatusFilter = () => wrapper.find(StatusFilter); - const findConfidentialityFilter = () => wrapper.find(ConfidentialityFilter); - const findApplyButton = () => wrapper.find(GlButton); - const findResetLinkButton = () => wrapper.find(GlLink); + const findStatusFilter = () => wrapper.findComponent(StatusFilter); + const findConfidentialityFilter = () => wrapper.findComponent(ConfidentialityFilter); + const findApplyButton = () => wrapper.findComponent(GlButton); + const findResetLinkButton = () => wrapper.findComponent(GlLink); describe('template', () => { beforeEach(() => { @@ -61,6 +59,28 @@ describe('GlobalSearchSidebar', () => { }); }); + describe('ApplyButton', () => { + describe('when sidebarDirty is false', () => { + beforeEach(() => { + createComponent({ sidebarDirty: false }); + }); + + it('disables the button', () => { + expect(findApplyButton().attributes('disabled')).toBe('true'); + }); + }); + + describe('when sidebarDirty is true', () => { + beforeEach(() => { + createComponent({ sidebarDirty: true }); + }); + + it('enables the button', () => { + expect(findApplyButton().attributes('disabled')).toBe(undefined); + }); + }); + }); + describe('ResetLinkButton', () => { describe('with no filter selected', () => { beforeEach(() => { diff --git a/spec/frontend/search/store/actions_spec.js b/spec/frontend/search/store/actions_spec.js index b50248bb295..5f8cee8160f 100644 --- a/spec/frontend/search/store/actions_spec.js +++ b/spec/frontend/search/store/actions_spec.js @@ -5,7 +5,11 @@ import createFlash from '~/flash'; import axios from '~/lib/utils/axios_utils'; import * as urlUtils from '~/lib/utils/url_utility'; import * as actions from '~/search/store/actions'; -import { GROUPS_LOCAL_STORAGE_KEY, PROJECTS_LOCAL_STORAGE_KEY } from '~/search/store/constants'; +import { + GROUPS_LOCAL_STORAGE_KEY, + PROJECTS_LOCAL_STORAGE_KEY, + SIDEBAR_PARAMS, +} from '~/search/store/constants'; import * as types from '~/search/store/mutation_types'; import createState from '~/search/store/state'; import * as storeUtils from '~/search/store/utils'; @@ -153,15 +157,24 @@ describe('Global Search Store Actions', () => { }); }); - describe('setQuery', () => { - const payload = { key: 'key1', value: 'value1' }; + describe.each` + payload | isDirty | isDirtyMutation + ${{ key: SIDEBAR_PARAMS[0], value: 'test' }} | ${false} | ${[{ type: types.SET_SIDEBAR_DIRTY, payload: false }]} + ${{ key: SIDEBAR_PARAMS[0], value: 'test' }} | ${true} | ${[{ type: types.SET_SIDEBAR_DIRTY, payload: true }]} + ${{ key: SIDEBAR_PARAMS[1], value: 'test' }} | ${false} | ${[{ type: types.SET_SIDEBAR_DIRTY, payload: false }]} + ${{ key: SIDEBAR_PARAMS[1], value: 'test' }} | ${true} | ${[{ type: types.SET_SIDEBAR_DIRTY, payload: true }]} + ${{ key: 'non-sidebar', value: 'test' }} | ${false} | ${[]} + ${{ key: 'non-sidebar', value: 'test' }} | ${true} | ${[]} + `('setQuery', ({ payload, isDirty, isDirtyMutation }) => { + describe(`when filter param is ${payload.key} and utils.isSidebarDirty returns ${isDirty}`, () => { + const expectedMutations = [{ type: types.SET_QUERY, payload }].concat(isDirtyMutation); - it('calls the SET_QUERY mutation', () => { - return testAction({ - action: actions.setQuery, - payload, - state, - expectedMutations: [{ type: types.SET_QUERY, payload }], + beforeEach(() => { + storeUtils.isSidebarDirty = jest.fn().mockReturnValue(isDirty); + }); + + it(`should dispatch the correct mutations`, () => { + return testAction({ action: actions.setQuery, payload, state, expectedMutations }); }); }); }); diff --git a/spec/frontend/search/store/mutations_spec.js b/spec/frontend/search/store/mutations_spec.js index a60718a972d..25f9b692955 100644 --- a/spec/frontend/search/store/mutations_spec.js +++ b/spec/frontend/search/store/mutations_spec.js @@ -72,6 +72,16 @@ describe('Global Search Store Mutations', () => { }); }); + describe('SET_SIDEBAR_DIRTY', () => { + const value = true; + + it('sets sidebarDirty to the value', () => { + mutations[types.SET_SIDEBAR_DIRTY](state, value); + + expect(state.sidebarDirty).toBe(value); + }); + }); + describe('LOAD_FREQUENT_ITEMS', () => { it('sets frequentItems[key] to data', () => { const payload = { key: 'test-key', data: [1, 2, 3] }; diff --git a/spec/frontend/search/store/utils_spec.js b/spec/frontend/search/store/utils_spec.js index bcdad9f89dd..20d764190b1 100644 --- a/spec/frontend/search/store/utils_spec.js +++ b/spec/frontend/search/store/utils_spec.js @@ -1,6 +1,11 @@ import { useLocalStorageSpy } from 'helpers/local_storage_helper'; -import { MAX_FREQUENCY } from '~/search/store/constants'; -import { loadDataFromLS, setFrequentItemToLS, mergeById } from '~/search/store/utils'; +import { MAX_FREQUENCY, SIDEBAR_PARAMS } from '~/search/store/constants'; +import { + loadDataFromLS, + setFrequentItemToLS, + mergeById, + isSidebarDirty, +} from '~/search/store/utils'; import { MOCK_LS_KEY, MOCK_GROUPS, @@ -216,4 +221,24 @@ describe('Global Search Store Utils', () => { }); }); }); + + describe.each` + description | currentQuery | urlQuery | isDirty + ${'identical'} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default' }} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default' }} | ${false} + ${'different'} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'new' }} | ${{ [SIDEBAR_PARAMS[0]]: 'default', [SIDEBAR_PARAMS[1]]: 'default' }} | ${true} + ${'null/undefined'} | ${{ [SIDEBAR_PARAMS[0]]: null, [SIDEBAR_PARAMS[1]]: null }} | ${{ [SIDEBAR_PARAMS[0]]: undefined, [SIDEBAR_PARAMS[1]]: undefined }} | ${false} + ${'updated/undefined'} | ${{ [SIDEBAR_PARAMS[0]]: 'new', [SIDEBAR_PARAMS[1]]: 'new' }} | ${{ [SIDEBAR_PARAMS[0]]: undefined, [SIDEBAR_PARAMS[1]]: undefined }} | ${true} + `('isSidebarDirty', ({ description, currentQuery, urlQuery, isDirty }) => { + describe(`with ${description} sidebar query data`, () => { + let res; + + beforeEach(() => { + res = isSidebarDirty(currentQuery, urlQuery); + }); + + it(`returns ${isDirty}`, () => { + expect(res).toStrictEqual(isDirty); + }); + }); + }); }); diff --git a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb index 4e4d921d67f..a46b7db256f 100644 --- a/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb +++ b/spec/lib/gitlab/bitbucket_server_import/importer_spec.rb @@ -142,7 +142,7 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do expect { subject.execute }.to change { MergeRequest.count }.by(1) merge_request = MergeRequest.first - expect(merge_request.author).to eq(pull_request_author) + expect(merge_request.author).to eq(expected_author) end end @@ -151,7 +151,25 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do stub_feature_flags(bitbucket_server_user_mapping_by_username: false) end - include_examples 'imports pull requests' + context 'when email is not present' do + before do + allow(pull_request).to receive(:author_email).and_return(nil) + end + + let(:expected_author) { project_creator } + + include_examples 'imports pull requests' + end + + context 'when email is present' do + before do + allow(pull_request).to receive(:author_email).and_return(pull_request_author.email) + end + + let(:expected_author) { pull_request_author } + + include_examples 'imports pull requests' + end end context 'when bitbucket_server_user_mapping_by_username feature flag is enabled' do @@ -159,19 +177,24 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do stub_feature_flags(bitbucket_server_user_mapping_by_username: true) end - include_examples 'imports pull requests' do - context 'when username is not present' do - before do - allow(pull_request).to receive(:author_username).and_return(nil) - end - - it 'maps by email' do - expect { subject.execute }.to change { MergeRequest.count }.by(1) - - merge_request = MergeRequest.first - expect(merge_request.author).to eq(pull_request_author) - end + context 'when username is not present' do + before do + allow(pull_request).to receive(:author_username).and_return(nil) end + + let(:expected_author) { project_creator } + + include_examples 'imports pull requests' + end + + context 'when username is present' do + before do + allow(pull_request).to receive(:author_username).and_return(pull_request_author.username) + end + + let(:expected_author) { pull_request_author } + + include_examples 'imports pull requests' end end @@ -228,7 +251,23 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do allow(subject.client).to receive(:activities).and_return([pr_comment]) end - it 'maps by email' do + it 'defaults to import user' do + expect { subject.execute }.to change { MergeRequest.count }.by(1) + + merge_request = MergeRequest.first + expect(merge_request.notes.count).to eq(1) + note = merge_request.notes.first + expect(note.author).to eq(project_creator) + end + end + + context 'when username is present' do + before do + allow(pr_note).to receive(:author_username).and_return(note_author.username) + allow(subject.client).to receive(:activities).and_return([pr_comment]) + end + + it 'maps by username' do expect { subject.execute }.to change { MergeRequest.count }.by(1) merge_request = MergeRequest.first @@ -384,13 +423,13 @@ RSpec.describe Gitlab::BitbucketServerImport::Importer do allow(inline_note).to receive(:author_username).and_return(nil) end - it 'maps by email' do + it 'defaults to import user' do expect { subject.execute }.to change { MergeRequest.count }.by(1) notes = MergeRequest.first.notes.order(:id).to_a - expect(notes.first.author).to eq(inline_note_author) - expect(notes.last.author).to eq(reply_author) + expect(notes.first.author).to eq(project_creator) + expect(notes.last.author).to eq(project_creator) end end end