diff --git a/app/assets/javascripts/import/details/components/bulk_import_details_app.vue b/app/assets/javascripts/import/details/components/bulk_import_details_app.vue index a248dd3d2c46f778add0601958de8ee7e96c1d17..96dd3f461fcae55cf91cf25e22e23f91b6dd193f 100644 --- a/app/assets/javascripts/import/details/components/bulk_import_details_app.vue +++ b/app/assets/javascripts/import/details/components/bulk_import_details_app.vue @@ -1,6 +1,5 @@ <script> import { sprintf, s__, __ } from '~/locale'; -import { getParameterValues } from '~/lib/utils/url_utility'; import ImportDetailsTable from '~/import/details/components/import_details_table.vue'; @@ -35,11 +34,24 @@ export default { gitlabLogo: window.gon.gitlab_logo, + props: { + id: { + type: String, + required: false, + default: null, + }, + entityId: { + type: String, + required: false, + default: null, + }, + }, + computed: { title() { - const id = getParameterValues('entity_id')[0]; - - return sprintf(s__('BulkImport|Items that failed to be imported for %{id}'), { id }); + return sprintf(s__('BulkImport|Items that failed to be imported for %{id}'), { + id: this.entityId, + }); }, }, }; @@ -53,7 +65,9 @@ export default { </h1> <import-details-table + :id="id" bulk-import + :entity-id="entityId" :fields="$options.fields" :local-storage-key="$options.LOCAL_STORAGE_KEY" /> diff --git a/app/assets/javascripts/import/details/components/import_details_table.vue b/app/assets/javascripts/import/details/components/import_details_table.vue index 8b9bf14e3a339c02ed83378e87ee1bdf8a81b9bd..f35d905eeba26fc71f1b40365c413995d0c91d18 100644 --- a/app/assets/javascripts/import/details/components/import_details_table.vue +++ b/app/assets/javascripts/import/details/components/import_details_table.vue @@ -34,6 +34,18 @@ export default { }, props: { + id: { + type: String, + required: false, + default: null, + }, + + entityId: { + type: String, + required: false, + default: null, + }, + bulkImport: { type: Boolean, required: false, @@ -96,11 +108,7 @@ export default { fetchFn(params) { return this.bulkImport - ? getBulkImportFailures( - getParameterValues('id')[0], - getParameterValues('entity_id')[0], - params, - ) + ? getBulkImportFailures(this.id, this.entityId, params) : fetchImportFailures(this.failuresPath, { projectId: getParameterValues('project_id')[0], ...params, diff --git a/app/assets/javascripts/import_entities/import_groups/components/import_status.vue b/app/assets/javascripts/import_entities/import_groups/components/import_status.vue index e2c5a4aadb878d7d34b8c561de419fb8ff4a154e..e401d5ad90dc1f41079cc83fd407aecb0b53d6ff 100644 --- a/app/assets/javascripts/import_entities/import_groups/components/import_status.vue +++ b/app/assets/javascripts/import_entities/import_groups/components/import_status.vue @@ -1,6 +1,5 @@ <script> import { GlBadge, GlLink } from '@gitlab/ui'; -import { mergeUrlParams } from '~/lib/utils/url_utility'; import { STATUSES, STATUS_ICON_MAP } from '~/import_entities/constants'; export default { @@ -59,7 +58,9 @@ export default { return null; } - return mergeUrlParams({ id: this.id, entity_id: this.entityId }, this.detailsPath); + return this.detailsPath + .replace(':id', encodeURIComponent(this.id)) + .replace(':entity_id', encodeURIComponent(this.entityId)); }, }, }; diff --git a/app/assets/javascripts/pages/import/bulk_imports/details/index.js b/app/assets/javascripts/pages/import/bulk_imports/failures/index.js similarity index 70% rename from app/assets/javascripts/pages/import/bulk_imports/details/index.js rename to app/assets/javascripts/pages/import/bulk_imports/failures/index.js index 5c2571af60f89cfb9337533b0f5f4c32ce4ba4b7..97a5159e583e2947af463a033fa4273605195c73 100644 --- a/app/assets/javascripts/pages/import/bulk_imports/details/index.js +++ b/app/assets/javascripts/pages/import/bulk_imports/failures/index.js @@ -8,11 +8,18 @@ export const initBulkImportDetails = () => { return null; } + const { id, entityId } = el.dataset; + return new Vue({ el, name: 'BulkImportDetailsRoot', render(createElement) { - return createElement(BulkImportDetailsApp); + return createElement(BulkImportDetailsApp, { + props: { + id, + entityId, + }, + }); }, }); }; diff --git a/app/controllers/import/bulk_imports_controller.rb b/app/controllers/import/bulk_imports_controller.rb index 7ed264c3e4aa966184edf572b4e6c90cba9e65e3..a68056a879f7d86e350fa1d4a9abfc172b5b189d 100644 --- a/app/controllers/import/bulk_imports_controller.rb +++ b/app/controllers/import/bulk_imports_controller.rb @@ -5,7 +5,7 @@ class Import::BulkImportsController < ApplicationController before_action :ensure_bulk_import_enabled before_action :verify_blocked_uri, only: :status - before_action :bulk_import, only: [:history] + before_action :bulk_import, only: [:history, :failures] feature_category :importers urgency :low @@ -52,7 +52,9 @@ def status def history; end - def details; end + def failures + bulk_import_entity + end def create return render json: { success: false }, status: :too_many_requests if throttled_request? @@ -85,6 +87,10 @@ def bulk_import @bulk_import || render_404 end + def bulk_import_entity + @bulk_import_entity ||= @bulk_import.entities.find(params[:entity_id]) + end + def pagination_headers %w[x-next-page x-page x-per-page x-prev-page x-total x-total-pages] end diff --git a/app/views/import/bulk_imports/details.html.haml b/app/views/import/bulk_imports/failures.haml similarity index 83% rename from app/views/import/bulk_imports/details.html.haml rename to app/views/import/bulk_imports/failures.haml index 39eb0f74ca24733e41ec9c1bb3303a73dfa59457..054d17f59c3a0e65b0f712895454bb09199efc77 100644 --- a/app/views/import/bulk_imports/details.html.haml +++ b/app/views/import/bulk_imports/failures.haml @@ -5,4 +5,4 @@ - add_to_breadcrumbs params[:id], history_import_bulk_import_path(id: params[:id]) - page_title format(s_('Import|Failures for %{id}'), id: params[:entity_id]) -.js-bulk-import-details +.js-bulk-import-details{ data: { id: params[:id], entity_id: params[:entity_id] } } diff --git a/app/views/import/bulk_imports/history.html.haml b/app/views/import/bulk_imports/history.html.haml index d5e577b5afe7f72f60a500d9dd98cd18cdb6b471..00dec289a8367aae6b386fb1f9fc788a68d22376 100644 --- a/app/views/import/bulk_imports/history.html.haml +++ b/app/views/import/bulk_imports/history.html.haml @@ -6,4 +6,4 @@ - page_title s_('BulkImport|Direct transfer history') - add_page_specific_style 'page_bundles/import' -#import-history-mount-element{ data: { id: params[:id], details_path: details_import_bulk_imports_path, realtime_changes_path: realtime_changes_import_bulk_imports_path(format: :json) } } +#import-history-mount-element{ data: { id: params[:id], details_path: failures_import_bulk_import_path(':id', ':entity_id'), realtime_changes_path: realtime_changes_import_bulk_imports_path(format: :json) } } diff --git a/config/routes/import.rb b/config/routes/import.rb index 17ca96c24b87ca8487c6c3e2d3b4bf6d56fffd98..53cfb6c39955af1092fd39de259956c91cd94448 100644 --- a/config/routes/import.rb +++ b/config/routes/import.rb @@ -74,12 +74,12 @@ get :status get :realtime_changes get :history - get :details end resources :bulk_imports, only: [] do member do get :history + get '/history/:entity_id/failures', action: :failures, as: :failures end end diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index df803c9f8bcd179de68455b0de4bc283fcd228ac..ead24e648f80bd11e5b4a428e636a81765accae4 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -338,14 +338,41 @@ def get_status(params_override = {}, format = :json) end end - describe 'GET details' do - subject(:request) { get :details } + describe 'GET failures' do + let_it_be(:bulk_import) { create(:bulk_import, user: user) } + let_it_be(:bulk_import_entity) { create(:bulk_import_entity, bulk_import: bulk_import) } + let(:id) { bulk_import.id } + let(:entity_id) { bulk_import_entity.id } - it 'responds with a 200 and shows the template', :aggregate_failures do - request + subject(:request) { get :failures, params: { id: id, entity_id: entity_id } } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template(:details) + describe 'when bulk_import does not exist' do + let(:id) { 18231 } + + it 'responds with a 404' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'when bulk_import_entity does not exist' do + let(:entity_id) { 136485 } + + it 'responds with a 404' do + request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + describe 'when both bulk_import and bulk_import_entity exist' do + it 'responds with a 200 and shows the template', :aggregate_failures do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:failures) + end end end diff --git a/spec/frontend/import/details/components/bulk_import_details_app_spec.js b/spec/frontend/import/details/components/bulk_import_details_app_spec.js index 18b03ed98025bd0da590639a5a4c9e50f7e98a7b..1c6ef0f55c0d634fe67befd9d206340544575085 100644 --- a/spec/frontend/import/details/components/bulk_import_details_app_spec.js +++ b/spec/frontend/import/details/components/bulk_import_details_app_spec.js @@ -1,22 +1,27 @@ import { shallowMount } from '@vue/test-utils'; -import { getParameterValues } from '~/lib/utils/url_utility'; import BulkImportDetailsApp from '~/import/details/components/bulk_import_details_app.vue'; +import ImportDetailsTable from '~/import/details/components/import_details_table.vue'; jest.mock('~/lib/utils/url_utility'); describe('Bulk import details app', () => { let wrapper; - const mockId = 151; + const mockId = '151'; + const mockEntityId = '46584'; + const defaultProps = { + id: mockId, + entityId: mockEntityId, + }; const createComponent = () => { - wrapper = shallowMount(BulkImportDetailsApp); + wrapper = shallowMount(BulkImportDetailsApp, { + propsData: { ...defaultProps }, + }); }; - beforeEach(() => { - getParameterValues.mockReturnValueOnce([mockId]); - }); + const findImportTable = () => wrapper.findComponent(ImportDetailsTable); describe('template', () => { it('renders heading', () => { @@ -24,7 +29,17 @@ describe('Bulk import details app', () => { const headingText = wrapper.find('h1').text(); - expect(headingText).toBe(`Items that failed to be imported for ${mockId}`); + expect(headingText).toBe(`Items that failed to be imported for ${mockEntityId}`); + }); + + it('renders import table', () => { + createComponent(); + + expect(findImportTable().props()).toMatchObject({ + id: mockId, + bulkImport: true, + entityId: mockEntityId, + }); }); }); }); diff --git a/spec/frontend/import/details/components/import_details_table_spec.js b/spec/frontend/import/details/components/import_details_table_spec.js index e2ba0ddad17818abed21fcf49fac7eb07e850f11..702d2f4e1e5513fc46836fff1c19aaa25b089f32 100644 --- a/spec/frontend/import/details/components/import_details_table_spec.js +++ b/spec/frontend/import/details/components/import_details_table_spec.js @@ -2,7 +2,6 @@ import { mount, shallowMount } from '@vue/test-utils'; import { GlEmptyState, GlLoadingIcon, GlTable } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; -import { getParameterValues } from '~/lib/utils/url_utility'; import { HTTP_STATUS_OK, HTTP_STATUS_INTERNAL_SERVER_ERROR } from '~/lib/utils/http_status'; import { createAlert } from '~/alert'; import waitForPromises from 'helpers/wait_for_promises'; @@ -12,10 +11,6 @@ import ImportDetailsTable from '~/import/details/components/import_details_table import { mockImportFailures, mockHeaders } from '../mock_data'; jest.mock('~/alert'); -jest.mock('~/lib/utils/url_utility', () => ({ - ...jest.requireActual('~/lib/utils/url_utility'), - getParameterValues: jest.fn().mockReturnValue([]), -})); describe('Import details table', () => { let wrapper; @@ -131,13 +126,11 @@ describe('Import details table', () => { }); describe('when bulk_import is true', () => { - const mockId = 144; - const mockEntityId = 68; + const mockId = '144'; + const mockEntityId = '68'; beforeEach(() => { gon.api_version = 'v4'; - getParameterValues.mockReturnValueOnce([mockId]); - getParameterValues.mockReturnValueOnce([mockEntityId]); mock .onGet(`/api/v4/bulk_imports/${mockId}/entities/${mockEntityId}/failures`) @@ -147,6 +140,8 @@ describe('Import details table', () => { mountFn: mount, props: { bulkImport: true, + id: mockId, + entityId: mockEntityId, }, }); }); diff --git a/spec/frontend/import_entities/import_groups/components/import_status_spec.js b/spec/frontend/import_entities/import_groups/components/import_status_spec.js index e0cabb86dcfbf415b6a59d13e20744d98374127a..5cef7d694cefb3090e6331cc6962c3f18ce1c1a0 100644 --- a/spec/frontend/import_entities/import_groups/components/import_status_spec.js +++ b/spec/frontend/import_entities/import_groups/components/import_status_spec.js @@ -11,7 +11,7 @@ describe('Group import status component', () => { status: STATUSES.FINISHED, }; - const mockDetailsPath = '/details'; + const mockDetailsPath = '/:id/failures/:entity_id'; const createComponent = ({ props } = {}) => { wrapper = shallowMount(ImportStatus, { @@ -92,7 +92,7 @@ describe('Group import status component', () => { }, }); - expect(findGlLink().attributes('href')).toBe('/details?id=2&entity_id=11'); + expect(findGlLink().attributes('href')).toBe('/2/failures/11'); }); }); }); diff --git a/spec/frontend/pages/import/bulk_imports/details/index_spec.js b/spec/frontend/pages/import/bulk_imports/failures/index_spec.js similarity index 97% rename from spec/frontend/pages/import/bulk_imports/details/index_spec.js rename to spec/frontend/pages/import/bulk_imports/failures/index_spec.js index 0fefa3017f7cd66711572739993e0409fb6afc5b..bbc59230e19543671994c3a0e9b1dc78fd3d4b53 100644 --- a/spec/frontend/pages/import/bulk_imports/details/index_spec.js +++ b/spec/frontend/pages/import/bulk_imports/failures/index_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import { initBulkImportDetails } from '~/pages/import/bulk_imports/details/index'; +import { initBulkImportDetails } from '~/pages/import/bulk_imports/failures/index'; jest.mock('~/import/details/components/bulk_import_details_app.vue');