diff --git a/app/assets/javascripts/import_entities/import_projects/components/provider_repo_table_row.vue b/app/assets/javascripts/import_entities/import_projects/components/provider_repo_table_row.vue index ab166b8fba197d5b303e050ad427563d4116c088..447bed7f9d0300403f8354c91c1f99b7ff349d58 100644 --- a/app/assets/javascripts/import_entities/import_projects/components/provider_repo_table_row.vue +++ b/app/assets/javascripts/import_entities/import_projects/components/provider_repo_table_row.vue @@ -60,6 +60,8 @@ export default { return { isSelectedForReimport: false, showMembershipsModal: false, + userHasSelectedNamespace: false, + showNamespaceRequiredError: false, }; }, @@ -72,8 +74,13 @@ export default { }, showMembershipsWarning() { - const userNamespaceSelected = this.importTarget.targetNamespace === this.userNamespace; - return (this.isImportNotStarted || this.isSelectedForReimport) && userNamespaceSelected; + const usersNamespaceIsSelected = this.importTarget.targetNamespace === this.userNamespace; + + return this.isNotImporting && usersNamespaceIsSelected; + }, + + isNotImporting() { + return this.isImportNotStarted || this.isSelectedForReimport; }, isFinished() { @@ -128,6 +135,15 @@ export default { this.updateImportTarget({ newName: value }); }, }, + shouldBlockImportForNamespace() { + if (this.importTarget.targetNamespace) { + return false; + } + + return ( + !this.repo.importSource.target && this.isNotImporting && !this.userHasSelectedNamespace + ); + }, }, methods: { @@ -156,7 +172,9 @@ export default { }, onImportClick() { - if (this.showMembershipsWarning) { + if (this.shouldBlockImportForNamespace) { + this.showNamespaceRequiredError = true; + } else if (this.showMembershipsWarning) { this.showMembershipsModal = true; } else { this.handleImportRepo(); @@ -164,6 +182,8 @@ export default { }, onSelect(value) { + this.userHasSelectedNamespace = true; + this.showNamespaceRequiredError = false; this.updateImportTarget({ targetNamespace: value }); }, }, @@ -209,6 +229,7 @@ export default {
@@ -224,6 +245,14 @@ export default { data-testid="project-path-field" />
+ @@ -264,9 +293,11 @@ export default { (repoId) => { return { newName: repo.importSource.sanitizedName, - targetNamespace: state.defaultTargetNamespace, + targetNamespace: null, }; }; diff --git a/app/views/import/_githubish_status.html.haml b/app/views/import/_githubish_status.html.haml index ad68a0587f2fb24ce1dd30ed2292a09427396f5b..b6d4dc6b9c915ba6f5236e34bf74181b7289e611 100644 --- a/app/views/import/_githubish_status.html.haml +++ b/app/views/import/_githubish_status.html.haml @@ -3,7 +3,7 @@ - extra_data = local_assigns.fetch(:extra_data, {}) - filterable = local_assigns.fetch(:filterable, true) - paginatable = local_assigns.fetch(:paginatable, false) -- default_namespace_path = (local_assigns[:default_namespace] || current_user.namespace).full_path +- default_namespace_path = local_assigns[:default_namespace]&.full_path - cancel_path = local_assigns.fetch(:cancel_path, nil) - details_path = local_assigns.fetch(:details_path, nil) - provider_title = Gitlab::ImportSources.title(local_assigns.fetch(:provider)) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6b7ec2c5c142b0938d5a3f81ed1f22b97fa6f6ad..9e1167be57a16edd83d8a3f66ca25a0975d45c3e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -32005,6 +32005,12 @@ msgstr "" msgid "ImportProjects|Requesting your %{provider} repositories failed" msgstr "" +msgid "ImportProjects|Select a destination namespace." +msgstr "" + +msgid "ImportProjects|Select namespace" +msgstr "" + msgid "ImportProjects|Select the repositories you want to import" msgstr "" diff --git a/spec/features/import/manifest_import_spec.rb b/spec/features/import/manifest_import_spec.rb index 175ae2fee8c00bc4087900364ba03de5a45d4b32..35c25966e878b3b5e7971432556843a3505ce50a 100644 --- a/spec/features/import/manifest_import_spec.rb +++ b/spec/features/import/manifest_import_spec.rb @@ -34,7 +34,6 @@ page.within(second_row) do click_on 'Import' end - click_on 'Continue import' wait_for_requests diff --git a/spec/frontend/import_entities/import_projects/components/provider_repo_table_row_spec.js b/spec/frontend/import_entities/import_projects/components/provider_repo_table_row_spec.js index a135570656693cba56402a8f7a9612e975e283b6..ffe3953a4052263a66afeba7ed19047a9c2d18f5 100644 --- a/spec/frontend/import_entities/import_projects/components/provider_repo_table_row_spec.js +++ b/spec/frontend/import_entities/import_projects/components/provider_repo_table_row_spec.js @@ -1,4 +1,4 @@ -import { GlBadge, GlButton, GlModal } from '@gitlab/ui'; +import { GlBadge, GlButton } from '@gitlab/ui'; import Vue, { nextTick } from 'vue'; // eslint-disable-next-line no-restricted-imports import Vuex from 'vuex'; @@ -15,7 +15,7 @@ describe('ProviderRepoTableRow', () => { const cancelImport = jest.fn(); const setImportTarget = jest.fn(); const groupImportTarget = { - targetNamespace: 'target', + targetNamespace: null, newName: 'newName', }; @@ -45,7 +45,8 @@ describe('ProviderRepoTableRow', () => { const findImportStatus = () => wrapper.findComponent(ImportStatus); const findProviderLink = () => wrapper.findByTestId('provider-link'); const findMembershipsWarning = () => wrapper.findByTestId('memberships-warning'); - const findGlModal = () => wrapper.findComponent(GlModal); + const findMembershipsWarningModal = () => wrapper.findByTestId('memberships-warning-modal'); + const findNamespaceRequiredWarning = () => wrapper.findByTestId('namespace-required-warning'); const findCancelButton = () => { const buttons = wrapper @@ -97,12 +98,37 @@ describe('ProviderRepoTableRow', () => { expect(findImportTargetDropdown().exists()).toBe(true); }); + it('renders the dropdown without a default selected', () => { + expect(findImportTargetDropdown().props('selected')).toBe(null); + expect(findImportTargetDropdown().props().toggleText).toBe('Select namespace'); + }); + + describe('when no namespace is selected', () => { + it('shows namespace required warning when import button is clicked', async () => { + findImportButton().vm.$emit('click'); + await nextTick(); + + expect(findNamespaceRequiredWarning().text()).toBe('Select a destination namespace.'); + }); + + it('does not trigger import when clicking import button', async () => { + findImportButton().vm.$emit('click'); + await nextTick(); + + expect(fetchImport).not.toHaveBeenCalled(); + }); + }); + describe('when user namespace is selected as import target', () => { - beforeEach(() => { + beforeEach(async () => { mountComponent( { repo }, { storeOptions: { importTarget: { targetNamespace: userNamespace } } }, ); + + const dropdown = findImportTargetDropdown(); + dropdown.vm.$emit('select', userNamespace); + await nextTick(); }); it('shows memberships warning', () => { @@ -113,7 +139,7 @@ describe('ProviderRepoTableRow', () => { findImportButton().vm.$emit('click'); await nextTick(); - const modal = findGlModal(); + const modal = findMembershipsWarningModal(); expect(modal.props('title')).toBe( 'Are you sure you want to import the project to a personal namespace?', ); @@ -122,11 +148,15 @@ describe('ProviderRepoTableRow', () => { ); }); + it('does not show `missing namespace` warning', () => { + expect(findNamespaceRequiredWarning().exists()).toBe(false); + }); + it('triggers import when clicking modal primary button', async () => { findImportButton().vm.$emit('click'); await nextTick(); - findGlModal().vm.$emit('primary'); + findMembershipsWarningModal().vm.$emit('primary'); expect(fetchImport).toHaveBeenCalledWith(expect.anything(), { repoId: repo.importSource.id, @@ -136,53 +166,68 @@ describe('ProviderRepoTableRow', () => { }); describe('when group namespace is selected as import target', () => { + beforeEach(async () => { + const dropdown = findImportTargetDropdown(); + dropdown.vm.$emit('select', 'target'); + await nextTick(); + }); + it('does not show memberships warning', () => { expect(findMembershipsWarning().isVisible()).toBe(false); }); - it('does not show modal when import button is clicked', async () => { + it('does not show memberships modal when import button is clicked', async () => { findImportButton().vm.$emit('click'); await nextTick(); - expect(findGlModal().exists()).toBe(false); + expect(findMembershipsWarningModal().exists()).toBe(false); }); - }); - it('renders import button', () => { - expect(findImportButton().exists()).toBe(true); - }); + it('does not show `missing namespace` warning when import button is clicked', async () => { + findImportButton().vm.$emit('click'); + await nextTick(); + expect(findNamespaceRequiredWarning().exists()).toBe(false); + }); + + it('renders import button', () => { + expect(findImportButton().exists()).toBe(true); + }); - it('imports repo when clicking import button', async () => { - findImportButton().vm.$emit('click'); + it('imports repo when clicking import button', async () => { + findImportButton().vm.$emit('click'); - await nextTick(); + await nextTick(); - expect(fetchImport).toHaveBeenCalledWith(expect.anything(), { - repoId: repo.importSource.id, - optionalStages: {}, + expect(fetchImport).toHaveBeenCalledWith(expect.anything(), { + repoId: repo.importSource.id, + optionalStages: {}, + }); }); - }); - it('includes optionalStages to import', async () => { - const OPTIONAL_STAGES = { stage1: true, stage2: false }; + it('includes optionalStages to import', async () => { + const OPTIONAL_STAGES = { stage1: true, stage2: false }; - mountComponent({ - repo, - optionalStages: OPTIONAL_STAGES, - }); + mountComponent({ + repo, + optionalStages: OPTIONAL_STAGES, + }); - findImportButton().vm.$emit('click'); + const dropdown = findImportTargetDropdown(); + dropdown.vm.$emit('select', 'target'); + await nextTick(); - await nextTick(); + findImportButton().vm.$emit('click'); + await nextTick(); - expect(fetchImport).toHaveBeenCalledWith(expect.anything(), { - repoId: repo.importSource.id, - optionalStages: OPTIONAL_STAGES, + expect(fetchImport).toHaveBeenCalledWith(expect.anything(), { + repoId: repo.importSource.id, + optionalStages: OPTIONAL_STAGES, + }); }); - }); - it('does not render re-import button', () => { - expect(findReimportButton().exists()).toBe(false); + it('does not render re-import button', () => { + expect(findReimportButton().exists()).toBe(false); + }); }); }); @@ -290,7 +335,12 @@ describe('ProviderRepoTableRow', () => { await nextTick(); + findImportTargetDropdown().vm.$emit('select', 'some-namespace'); + await nextTick(); + findReimportButton().vm.$emit('click'); + await nextTick(); + expect(findNamespaceRequiredWarning().exists()).toBe(false); expect(fetchImport).toHaveBeenCalledWith(expect.anything(), { repoId: repo.importSource.id, diff --git a/spec/frontend/import_entities/import_projects/store/actions_spec.js b/spec/frontend/import_entities/import_projects/store/actions_spec.js index 918821dfa59816e4658b8f59819d5f950ef4804e..aa57ae9f0413713e26c77e431f47324f5237a38c 100644 --- a/spec/frontend/import_entities/import_projects/store/actions_spec.js +++ b/spec/frontend/import_entities/import_projects/store/actions_spec.js @@ -55,7 +55,7 @@ describe('import_projects store actions', () => { let localState; const importRepoId = 1; const otherImportRepoId = 2; - const defaultTargetNamespace = 'default'; + const defaultTargetNamespace = null; const sanitizedName = 'sanitizedName'; const defaultImportTarget = { newName: sanitizedName, targetNamespace: defaultTargetNamespace }; diff --git a/spec/frontend/import_entities/import_projects/store/getters_spec.js b/spec/frontend/import_entities/import_projects/store/getters_spec.js index fced5670f25b3edbc202242e53662efa998cbc8d..e25a6e099287ded186789b689a3a8805c3bd1c69 100644 --- a/spec/frontend/import_entities/import_projects/store/getters_spec.js +++ b/spec/frontend/import_entities/import_projects/store/getters_spec.js @@ -99,7 +99,7 @@ describe('import_projects store getters', () => { expect(getImportTarget(localState)(IMPORTABLE_REPO.importSource.id)).toStrictEqual({ newName: IMPORTABLE_REPO.importSource.sanitizedName, - targetNamespace: localState.defaultTargetNamespace, + targetNamespace: null, }); });