refactor code based on feedback

2 files changed, 30 insertions, 15 deletions

diff --git a/lib/api/project_import.rb b/lib/api/project_import.rb
index b0511342b63..88fe1d2b5f5 100644
--- a/lib/api/project_import.rb
+++ b/lib/api/project_import.rb
@@ -10,19 +10,24 @@ module API
       def file_is_valid?
         import_params[:file] && import_params[:file]['tempfile'].respond_to?(:read)
       end
+
+      def validate_file!
+        render_api_error!('The file is invalid', 400) unless file_is_valid?
+      end
     end
 
     before do
       forbidden! unless Gitlab::CurrentSettings.import_sources.include?('gitlab_project')
     end
 
-    resource :projects, requirements: { id: %r{[^/]+} } do
+    resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
       params do
         requires :path, type: String, desc: 'The new project path and name'
         requires :file, type: File, desc: 'The project export file to be imported'
         optional :namespace, type: String, desc: 'The ID or name of the namespace that the project will be imported into. Defaults to the user namespace.'
       end
       desc 'Create a new project import' do
+        detail 'This feature was introduced in GitLab 10.6.'
         success Entities::ProjectImportStatus
       end
       post 'import' do
@@ -30,13 +35,10 @@ module API
 
         Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42437')
 
-        namespace = import_params[:namespace]
-        namespace = if namespace.blank?
-                      current_user.namespace
-                    elsif namespace =~ /^\d+$/
-                      Namespace.find_by(id: namespace)
+        namespace = if import_params[:namespace]
+                      find_namespace!(import_params[:namespace])
                     else
-                      Namespace.find_by_path_or_name(namespace)
+                      current_user.namespace
                     end
 
         project_params = import_params.merge(namespace_id: namespace.id,
@@ -52,6 +54,7 @@ module API
         requires :id, type: String, desc: 'The ID of a project'
       end
       desc 'Get a project export status' do
+        detail 'This feature was introduced in GitLab 10.6.'
         success Entities::ProjectImportStatus
       end
       get ':id/import' do
diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb
index 47c1edc7919..2fb103c6510 100644
--- a/spec/requests/api/project_import_spec.rb
+++ b/spec/requests/api/project_import_spec.rb
@@ -17,8 +17,7 @@ describe API::ProjectImport do
 
   describe 'POST /projects/import' do
     it 'schedules an import using a namespace' do
-      expect_any_instance_of(Project).to receive(:import_schedule)
-      expect(Gitlab::ImportExport::ProjectCreator).to receive(:new).with(namespace.id, any_args).and_call_original
+      stub_import(user.namespace)
 
       post api('/projects/import', user), path: 'test-import', file: fixture_file_upload(file), namespace: namespace.id
 
@@ -26,8 +25,7 @@ describe API::ProjectImport do
     end
 
     it 'schedules an import using the namespace path' do
-      expect_any_instance_of(Project).to receive(:import_schedule)
-      expect(Gitlab::ImportExport::ProjectCreator).to receive(:new).with(namespace.id, any_args).and_call_original
+      stub_import(unamespace)
 
       post api('/projects/import', user), path: 'test-import', file: fixture_file_upload(file), namespace: namespace.full_path
 
@@ -35,14 +33,23 @@ describe API::ProjectImport do
     end
 
     it 'schedules an import at the user namespace level' do
-      expect_any_instance_of(Project).to receive(:import_schedule)
-      expect(Gitlab::ImportExport::ProjectCreator).to receive(:new).with(user.namespace.id, any_args).and_call_original
+      stub_import(user.namespace)
 
       post api('/projects/import', user), path: 'test-import2', file: fixture_file_upload(file)
 
       expect(response).to have_gitlab_http_status(201)
     end
 
+    it 'schedules an import at the user namespace level' do
+      expect_any_instance_of(Project).not_to receive(:import_schedule)
+      expect(Gitlab::ImportExport::ProjectCreator).not_to receive(:new)
+
+      post api('/projects/import', user), namespace: 'nonexistent', path: 'test-import2', file: fixture_file_upload(file)
+
+      expect(response).to have_gitlab_http_status(404)
+      expect(json_response['message']).to eq('404 Namespace Not Found')
+    end
+
     it 'does not schedule an import if the user has no permission to the namespace' do
       expect_any_instance_of(Project).not_to receive(:import_schedule)
 
@@ -51,8 +58,8 @@ describe API::ProjectImport do
            file: fixture_file_upload(file),
            namespace: namespace.full_path)
 
-      expect(response).to have_gitlab_http_status(400)
-      expect(json_response['message']).to eq('Namespace is not valid')
+      expect(response).to have_gitlab_http_status(404)
+      expect(json_response['message']).to eq('404 Namespace Not Found')
     end
 
     it 'does not schedule an import if the user uploads no valid file' do
@@ -63,6 +70,11 @@ describe API::ProjectImport do
       expect(response).to have_gitlab_http_status(400)
       expect(json_response['error']).to eq('file is invalid')
     end
+
+    def stub_import(namespace)
+      expect_any_instance_of(Project).to receive(:import_schedule)
+      expect(Gitlab::ImportExport::ProjectCreator).to receive(:new).with(namespace.id, any_args).and_call_original
+    end
   end
 
   describe 'GET /projects/:id/import' do