task import: handle errors during import better (#1439)

* task import: handle errors during import better
* fix flash messages, bump proforma version

app/assets/javascripts/flash_message.coffee

@@ -1,7 +1,7 @@
 show_ajax_message = (msg, type) ->
   cls = 'alert-success' if type == 'notice'
   cls = 'alert-danger' if type == 'alert'
-  $("#flash-message").html "<div id='flash-#{type}' class='alert #{cls}'>#{msg}</div>"
+  $("#flash-message").html "<div id='flash-#{type}' class='alert flash #{cls} my-2 alert-dismissible fade show'><p class='mb-0' id='flash-notice'>#{decodeURIComponent(msg)}</p><button aria-label='Close' class='btn-close' data-bs-dismiss='alert' type='button'></button></div>"
   $("#flash-#{type}").delay(6000).slideUp 'medium'
 
 $(document).ajaxComplete (event, request) ->
@@ -12,5 +12,4 @@ $(document).ajaxComplete (event, request) ->
 ready = ->
   $('#flash-message').children().first().delay(5000).slideUp 'medium'
 
-
 $(document).on('turbolinks:load', ready)

app/assets/javascripts/tasks_import.coffee

@@ -34,9 +34,6 @@ importStart = () ->
     data: formData,
     processData: false,
     contentType: false,
-    success: (response) ->
-      if response.status == 'failure'
-        alert(response.message)
     error: (_xhr, _textStatus, message) ->
       alert("#{I18n.t('common.javascripts.error')}: #{message}")
   })

app/controllers/tasks_controller.rb

@@ -89,7 +89,7 @@ def download
     send_data(zip_file.string, type: 'application/zip', filename: "task_#{@task.id}.zip", disposition: 'attachment')
   end
 
-  def import_start
+  def import_start # rubocop:disable Metric/AbcSize, Metrics/MethodLength
     zip_file = params[:zip_file]
     unless zip_file.is_a?(ActionDispatch::Http::UploadedFile)
       return render json: {status: 'failure', message: t('.choose_file_error')}
@@ -100,9 +100,24 @@ def import_start
     respond_to do |format|
       format.js { render layout: false }
     end
+  rescue ProformaXML::ProformaError => e
+    messages = prettify_import_errors(e)
+    flash[:alert] = messages
+    render json: {
+      status: 'failure',
+      message: t('.error', error: messages),
+      actions: '',
+    }
+  rescue StandardError => e
+    Sentry.capture_exception(e)
+    render json: {
+      status: 'failure',
+      message: t('tasks.import.internal_error'),
+      actions: '',
+    }
   end
 
-  def import_confirm
+  def import_confirm # rubocop:disable Metric/AbcSize
     proforma_task = ProformaService::ProformaTaskFromCachedFile.call(**import_confirm_params.to_hash.symbolize_keys)
 
     task = ProformaService::ImportTask.call(proforma_task:, user: current_user)
@@ -117,6 +132,13 @@ def import_confirm
       message: t('.error', title: proforma_task.title, error: e.message),
       actions: '',
     }
+  rescue StandardError => e
+    Sentry.capture_exception(e)
+    render json: {
+      status: 'failure',
+      message: t('tasks.import.internal_error'),
+      actions: '',
+    }
   end
 
   def import_uuid_check
@@ -137,7 +159,7 @@ def import_external
     render json: t('.invalid'), status: :bad_request
   rescue StandardError => e
     Sentry.capture_exception(e)
-    render json: t('.internal_error'), status: :internal_server_error
+    render json: t('tasks.import.internal_error'), status: :internal_server_error
   end
 
   def export_external_start
@@ -199,6 +221,13 @@ def generate_test
 
   private
 
+  def prettify_import_errors(error)
+    message = "#{t('proforma_errors.import')}<br>"
+    message + JSON.parse(error.message).map do |msg|
+                t("proforma_errors.#{msg}", default: msg)
+              end.join('<br>')
+  end
+
   def load_and_authorize_task
     @task = Task.find(params[:id])
     authorize @task

config/locales/de/controllers/tasks.yml

@@ -9,15 +9,17 @@ de:
     export_external_confirm:
       error: 'Der Export der Aufgabe (%{title}) ist fehlgeschlagen. <br> Fehler: %{error}'
       success: Aufgabe (%{title}) erfolgreich exportiert.
+    import:
+      internal_error: Beim Import dieser Aufgabe ist auf CodeHarbor ein interner Fehler aufgetreten.
     import_confirm:
       error: 'Der Import der Aufgabe (%{title}) ist fehlgeschlagen. <br> Fehler: %{error}'
       success: Aufgabe (%{title}) erfolgreich importiert.
     import_external:
-      internal_error: Bei der Importierung der Aufgabe ist ein interner Fehler aufgetreten.
       invalid: Ungültige Aufgabe
       success: Erfolg
     import_start:
       choose_file_error: Sie müssen eine Datei auswählen.
+      error: 'Der Import konnte nicht gestartet werden. <br> Fehler: %{error}'
     proforma_service:
       convert_zip_to_proforma_tasks:
         nested_too_deep: Die ZIP-Datei ist zu tief verschachtelt.

config/locales/de/proforma_errors.yml

@@ -0,0 +1,6 @@
+---
+de:
+  proforma_errors:
+    import: Es ist ein Fehler beim Importieren der ProformaXML ZIP-Datei aufgetreten.
+    no task_xml found: Es konnte keine task.xml in der ZIP-Datei gefunden werden.
+    version not supported: Die Version dieses ProformaXML Dokuments wird nicht unterstützt.

config/locales/en/controllers/tasks.yml

@@ -9,15 +9,17 @@ en:
     export_external_confirm:
       error: 'Export of task (%{title}) failed. <br> Error: %{error}'
       success: Task (%{title}) successfully exported.
+    import:
+      internal_error: An internal error occurred on CodeHarbor while importing the exercise.
     import_confirm:
       error: 'Import of task (%{title}) failed. <br> Error: %{error}'
       success: Task (%{title}) successfully imported.
     import_external:
-      internal_error: An internal error occurred on CodeHarbor while importing the exercise.
       invalid: Invalid exercise
       success: success
     import_start:
       choose_file_error: You need to choose a file.
+      error: 'Import of task could not be started. <br> Error: %{error}'
     proforma_service:
       convert_zip_to_proforma_tasks:
         nested_too_deep: ZIP is nested too deep.

config/locales/en/proforma_errors.yml

@@ -0,0 +1,6 @@
+---
+en:
+  proforma_errors:
+    import: An error occurred while importing your ProformaXML ZIP file.
+    no task_xml found: No task.xml could be found in the supplied ZIP file.
+    version not supported: The version of this ProformaXML document is not supported.

spec/controllers/tasks_controller_spec.rb

@@ -722,7 +722,7 @@
     context 'when zip_file is submitted' do
       let(:zip_file) {}
 
-      it 'renders correct json' do
+      it 'renders correct JSON' do
         post_request
         expect(JSON.parse(response.body, symbolize_names: true)).to eql({status: 'failure', message: 'You need to choose a file.'})
       end
@@ -731,11 +731,33 @@
     context "when zip_file is 'undefined'" do
       let(:zip_file) { 'undefined' }
 
-      it 'renders correct json' do
+      it 'renders correct JSON' do
         post_request
         expect(JSON.parse(response.body, symbolize_names: true)).to eql({status: 'failure', message: 'You need to choose a file.'})
       end
     end
+
+    context 'when service throws version not supported error' do
+      before do
+        allow(ProformaService::CacheImportFile).to receive(:call).and_raise(ProformaXML::ProformaError.new(['version not supported']))
+      end
+
+      it 'renders correct JSON' do
+        post_request
+        expect(JSON.parse(response.body, symbolize_names: true)).to eql({status: 'failure', message: 'Import of task could not be started. <br> Error: An error occurred while importing your ProformaXML ZIP file.<br>The version of this ProformaXML document is not supported.', actions: ''})
+      end
+    end
+
+    context 'when service throws an unexpected error' do
+      before do
+        allow(ProformaService::CacheImportFile).to receive(:call).and_raise(StandardError)
+      end
+
+      it 'renders correct JSON' do
+        post_request
+        expect(JSON.parse(response.body, symbolize_names: true)).to eql({status: 'failure', message: 'An internal error occurred on CodeHarbor while importing the exercise.', actions: ''})
+      end
+    end
   end
 
   describe 'POST #import_confirm' do
@@ -757,19 +779,30 @@
         expect { post_request }.to change(Task, :count).by(1)
       end
 
-      it 'renders correct json' do
+      it 'renders correct JSON' do
         post_request
         expect(response.body).to include('successfully imported').and(include(I18n.t('tasks.import_actions.button.show_task')).and(include('Hide')))
       end
 
       context 'when import raises a validation error' do
         before { allow(ProformaService::ImportTask).to receive(:call).and_raise(ActiveRecord::RecordInvalid) }
 
-        it 'renders correct json' do
+        it 'renders correct JSON' do
           post_request
           expect(response.body).to include('failed').and(include('Record invalid').and(include('"actions":""')))
         end
       end
+
+      context 'when service throws an unexpected error' do
+        before do
+          allow(ProformaService::ImportTask).to receive(:call).and_raise(StandardError)
+        end
+
+        it 'renders correct JSON' do
+          post_request
+          expect(JSON.parse(response.body, symbolize_names: true)).to eql({status: 'failure', message: 'An internal error occurred on CodeHarbor while importing the exercise.', actions: ''})
+        end
+      end
     end
 
     context 'when signed out' do
@@ -910,7 +943,7 @@
     let(:uuid_found) { true }
     let(:error) { nil }
 
-    it 'renders the correct contents as json' do
+    it 'renders the correct contents as JSON' do
       post_request
       expect(response.parsed_body.symbolize_keys[:message]).to eq('message')
       expect(response.parsed_body.symbolize_keys[:actions]).to(
@@ -924,7 +957,7 @@
     context 'when there is an error' do
       let(:error) { 'error' }
 
-      it 'renders the correct contents as json' do
+      it 'renders the correct contents as JSON' do
         post_request
         expect(response.parsed_body.symbolize_keys[:message]).to eq('message')
         expect(response.parsed_body.symbolize_keys[:actions]).to(
@@ -939,7 +972,7 @@
     context 'when uuid_found is false' do
       let(:uuid_found) { false }
 
-      it 'renders the correct contents as json' do
+      it 'renders the correct contents as JSON' do
         post_request
         expect(response.parsed_body.symbolize_keys[:message]).to eq('message')
         expect(response.parsed_body.symbolize_keys[:actions]).to(