Show a more helpful error for import status

Importing a project from GitHub for a project namespace that already exists
would show an unhelpful error, "An error occurred while importing project."
We now add the base message from Projects::CreateService when this fails.

Closes #47365
This commit is contained in:
Stan Hu 2018-06-05 22:34:06 -07:00
parent af07c490b2
commit 3a722ff53f
10 changed files with 68 additions and 8 deletions

View file

@ -67,7 +67,15 @@ class ImporterStatus {
false,
));
})
.catch(() => flash(__('An error occurred while importing project')));
.catch((error) => {
let details = error;
if (error.response && error.response.data && error.response.data.errors) {
details = error.response.data.errors;
}
flash(__(`An error occurred while importing project: ${details}`));
});
}
autoUpdate() {

View file

@ -25,4 +25,13 @@ class Import::BaseController < ApplicationController
current_user.namespace
end
def project_save_error(project)
# Projects::CreateService will set base message if unable to save
if project.errors[:base].present?
project.errors[:base].last
else
project.errors.full_messages.join(', ')
end
end
end

View file

@ -55,7 +55,7 @@ class Import::BitbucketController < Import::BaseController
if project.persisted?
render json: ProjectSerializer.new.represent(project)
else
render json: { errors: project.errors.full_messages }, status: :unprocessable_entity
render json: { errors: project_save_error(project) }, status: :unprocessable_entity
end
else
render json: { errors: 'This namespace has already been taken! Please choose another one.' }, status: :unprocessable_entity

View file

@ -66,7 +66,7 @@ class Import::FogbugzController < Import::BaseController
if project.persisted?
render json: ProjectSerializer.new.represent(project)
else
render json: { errors: project.errors.full_messages }, status: :unprocessable_entity
render json: { errors: project_save_error(project) }, status: :unprocessable_entity
end
end

View file

@ -48,7 +48,7 @@ class Import::GithubController < Import::BaseController
if project.persisted?
render json: ProjectSerializer.new.represent(project)
else
render json: { errors: project.errors.full_messages }, status: :unprocessable_entity
render json: { errors: project_save_error(project) }, status: :unprocessable_entity
end
else
render json: { errors: 'This namespace has already been taken! Please choose another one.' }, status: :unprocessable_entity

View file

@ -32,7 +32,7 @@ class Import::GitlabController < Import::BaseController
if project.persisted?
render json: ProjectSerializer.new.represent(project)
else
render json: { errors: project.errors.full_messages }, status: :unprocessable_entity
render json: { errors: project_save_error(project) }, status: :unprocessable_entity
end
else
render json: { errors: 'This namespace has already been taken! Please choose another one.' }, status: :unprocessable_entity

View file

@ -92,7 +92,7 @@ class Import::GoogleCodeController < Import::BaseController
if project.persisted?
render json: ProjectSerializer.new.represent(project)
else
render json: { errors: project.errors.full_messages }, status: :unprocessable_entity
render json: { errors: project_save_error(project) }, status: :unprocessable_entity
end
end

View file

@ -0,0 +1,5 @@
---
title: Show a more helpful error for import status
merge_request:
author:
type: other

View file

@ -50,6 +50,24 @@ describe('Importer Status', () => {
})
.catch(done.fail);
});
it('shows error message after failed POST request', (done) => {
appendSetFixtures('<div class="flash-container"></div>');
mock.onPost(importUrl).reply(422, {
errors: 'You forgot your lunch',
});
instance.addToImport({
currentTarget: document.querySelector('.js-add-to-import'),
})
.then(() => {
const flashMessage = document.querySelector('.flash-text');
expect(flashMessage.textContent.trim()).toEqual('An error occurred while importing project: You forgot your lunch');
done();
})
.catch(done.fail);
});
});
describe('autoUpdate', () => {

View file

@ -118,14 +118,34 @@ shared_examples 'a GitHub-ish import controller: POST create' do
expect(response).to have_gitlab_http_status(200)
end
it 'returns 422 response when the project could not be imported' do
it 'returns 422 response with the base error when the project could not be imported' do
project = build(:project)
error_message = 'This is an error'
project.errors.add(:base, error_message)
allow(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider)
.and_return(double(execute: build(:project)))
.and_return(double(execute: project))
post :create, format: :json
expect(response).to have_gitlab_http_status(422)
expect(json_response['errors']).to eq(error_message)
end
it 'returns 422 response with a combined error when the project could not be imported' do
project = build(:project)
project.errors.add(:name, 'is invalid')
project.errors.add(:path, 'is old')
allow(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider)
.and_return(double(execute: project))
post :create, format: :json
expect(response).to have_gitlab_http_status(422)
expect(json_response['errors']).to eq('Name is invalid, Path is old')
end
context "when the repository owner is the provider user" do