diff --git a/app/assets/javascripts/exercises.js b/app/assets/javascripts/exercises.js index ba5225829..941565bf4 100644 --- a/app/assets/javascripts/exercises.js +++ b/app/assets/javascripts/exercises.js @@ -344,7 +344,6 @@ $(document).on('turbolinks:load', function () { var observeExportButtons = function () { $('.export-start').on('click', function (e) { e.preventDefault(); - new bootstrap.Modal($('#export-modal')).show(); exportExerciseStart($(this).data().exerciseId); }); body_selector.on('click', '.export-retry-button', function () { @@ -356,11 +355,11 @@ $(document).on('turbolinks:load', function () { } var exportExerciseStart = function (exerciseID) { - const $exerciseDiv = $('#export-exercise'); - const $messageDiv = $exerciseDiv.children('.export-message'); - const $actionsDiv = $exerciseDiv.children('.export-exercise-actions'); + const $exerciseDiv = $('#exercise-transfer'); + const $messageDiv = $exerciseDiv.children('.transfer-message'); + const $actionsDiv = $exerciseDiv.children('.transfer-exercise-actions'); - $messageDiv.removeClass('export-failure'); + $messageDiv.removeClass('transfer-failure'); $messageDiv.html(I18n.t('exercises.export_codeharbor.checking_codeharbor')); $actionsDiv.html('
'); @@ -380,9 +379,9 @@ $(document).on('turbolinks:load', function () { }; var exportExerciseConfirm = function (exerciseID) { - const $exerciseDiv = $('#export-exercise'); - const $messageDiv = $exerciseDiv.children('.export-message'); - const $actionsDiv = $exerciseDiv.children('.export-exercise-actions'); + const $exerciseDiv = $('#exercise-transfer'); + const $messageDiv = $exerciseDiv.children('.transfer-message'); + const $actionsDiv = $exerciseDiv.children('.transfer-exercise-actions'); return $.ajax({ type: 'POST', @@ -395,11 +394,11 @@ $(document).on('turbolinks:load', function () { if (response.status === 'success') { $messageDiv.addClass('export-success'); setTimeout((function () { - bootstrap.Modal.getInstance($('#export-modal'))?.hide(); + bootstrap.Modal.getInstance($('#transfer-modal'))?.hide(); $messageDiv.html('').removeClass('export-success'); }), 3000); } else { - $messageDiv.addClass('export-failure'); + $messageDiv.addClass('transfer-failure'); } }, error: function (a, b, c) { @@ -408,6 +407,76 @@ $(document).on('turbolinks:load', function () { }); }; + var observeImportButtons = function () { + const $exerciseDiv = $('#exercise-transfer'); + const $messageDiv = $exerciseDiv.children('.transfer-message'); + const $actionsDiv = $exerciseDiv.children('.transfer-exercise-actions'); + + $('.import-start').on('click', function (e) { + e.preventDefault(); + importExerciseStart(); + }); + body_selector.on('change', '#proforma-file', async function () { + const file = event.target.files[0]; + const formData = new FormData(); + formData.append('file', file); + + return $.ajax({ + type: 'POST', + url: Routes.import_start_exercises_path(), + data: formData, + processData: false, + contentType: false, + + success: function (response) { + if(response.status === 'failure') + $messageDiv.addClass('transfer-failure'); + else + $messageDiv.removeClass('transfer-failure'); + $messageDiv.html(response.message); + return $actionsDiv.html(response.actions); + }, + error: function (a, b, c) { + return alert(`error: ${c}`); + } + }); + }); + body_selector.on('click', '.import-action', async function () { + let fileId = $(this).attr('data-file-id') + let importType = $(this).attr('data-import-type') + importExerciseConfirm(fileId, importType) + }); + } + var importExerciseStart = function () { + const $exerciseDiv = $('#exercise-transfer'); + const $messageDiv = $exerciseDiv.children('.transfer-message'); + const $actionsDiv = $exerciseDiv.children('.transfer-exercise-actions'); + + $messageDiv.removeClass('transfer-failure'); + $messageDiv.html(I18n.t('exercises.import_proforma.dialog.start')); + $actionsDiv.html(``); + } + + var importExerciseConfirm = function (fileId, importType) { + const $exerciseDiv = $('#exercise-transfer'); + const $messageDiv = $exerciseDiv.children('.transfer-message'); + const $actionsDiv = $exerciseDiv.children('.transfer-exercise-actions'); + + $.ajax({ + type: 'POST', + url: Routes.import_confirm_exercises_path(), + data: {file_id: fileId, import_type: importType}, + dataType: 'json', + + success: function (response) { + $messageDiv.html(response.message); + return $actionsDiv.html(response.actions); + }, + error: function (a, b, c) { + return alert(`error: ${c}`); + } + }); + } var overrideTextareaTabBehavior = function () { $('.mb-3 textarea[name$="[content]"]').on('keydown', function (event) { if (event.which === TAB_KEY_CODE) { @@ -463,6 +532,7 @@ $(document).on('turbolinks:load', function () { if ($('table:not(#tags-table)').isPresent()) { enableBatchUpdate(); observeExportButtons(); + observeImportButtons(); } else if ($('.edit_exercise, .new_exercise').isPresent()) { const form_selector = $('form'); execution_environments = form_selector.data('execution-environments'); @@ -499,4 +569,4 @@ $(document).on('turbolinks:load', function () { } -}); +}); \ No newline at end of file diff --git a/app/assets/stylesheets/exercises.css.scss b/app/assets/stylesheets/exercises.css.scss index 94a9a11c0..e5c3a0900 100644 --- a/app/assets/stylesheets/exercises.css.scss +++ b/app/assets/stylesheets/exercises.css.scss @@ -179,7 +179,7 @@ a.file-heading { } } -#export-modal { +#transfer-modal { .modal-content { min-height: 300px; } @@ -189,27 +189,27 @@ a.file-heading { } } -#export-exercise{ +#exercise-transfer{ display: flex; } -.export-message { +.transfer-message { flex-grow: 1; font-size: 12px; padding-right: 5px; word-wrap: break-word; } -.export-message + :empty { +.transfer-message + :empty { max-width: 100%; } -.export-exercise-actions:empty { +.transfer-exercise-actions:empty { display: none; } -.export-exercise-actions { - max-width: 110px; - min-width: 110px; +.transfer-exercise-actions { + max-width: 140px; + min-width: 140px; } .export-button { @@ -223,6 +223,6 @@ a.file-heading { font-weight: 600; } -.export-failure { +.transfer-failure { color: var(--bs-danger); } diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 79f49e00d..b8ab2dbfc 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -20,7 +20,7 @@ class ExercisesController < ApplicationController skip_before_action :verify_authenticity_token, only: %i[import_task import_uuid_check] skip_before_action :require_fully_authenticated_user!, only: %i[import_task import_uuid_check] - skip_after_action :verify_authorized, only: %i[import_task import_uuid_check] + skip_after_action :verify_authorized, only: %i[import_task import_uuid_check import_start import_confirm] skip_after_action :verify_policy_scoped, only: %i[import_task import_uuid_check], raise: false rescue_from Pundit::NotAuthorizedError, with: :not_authorized_for_exercise @@ -150,13 +150,66 @@ def import_uuid_check user = user_from_api_key return render json: {}, status: :unauthorized if user.nil? - uuid = params[:uuid].presence - exercise = Exercise.find_by(uuid:) + render json: uuid_check(user:, uuid: params[:uuid].presence) + end + + def import_start + zip_file = params[:file] + unless zip_file.is_a?(ActionDispatch::Http::UploadedFile) + return render json: {status: 'failure', message: t('.choose_file_error')} + end + + uuid = ProformaService::UuidFromZip.call(zip: zip_file) + exists, updatable = uuid_check(user: current_user, uuid:).values_at(:uuid_found, :update_right) + + uploader = ProformaZipUploader.new + uploader.cache!(params[:file]) + + message = if exists && updatable + t('.exercise_exists_and_is_updatable') + elsif exists + t('.exercise_exists_and_is_not_updatable') + else + t('.exercise_is_importable') + end - return render json: {uuid_found: false} if uuid.blank? || exercise.nil? - return render json: {uuid_found: true, update_right: false} unless ExercisePolicy.new(user, exercise).update? + render json: { + status: 'success', + message:, + actions: render_to_string(partial: 'import_actions', + locals: {exercise: @exercise, imported: false, exists:, updatable:, file_id: uploader.cache_name}), + } + rescue ProformaXML::InvalidZip => e + render json: { + status: 'failure', + message: t('.error', message: e.message), + } + end + + def import_confirm + uploader = ProformaZipUploader.new + uploader.retrieve_from_cache!(params[:file_id]) + exercise = ::ProformaService::Import.call(zip: uploader.file, user: current_user) + exercise.save! - render json: {uuid_found: true, update_right: true} + render json: { + status: 'success', + message: t('.success'), + actions: render_to_string(partial: 'import_actions', locals: {exercise:, imported: true}), + } + rescue ProformaXML::ProformaError, ActiveRecord::RecordInvalid => e + render json: { + status: 'failure', + message: t('.error', error: e.message), + actions: '', + } + rescue StandardError => e + Sentry.capture_exception(e) + render json: { + status: 'failure', + message: t('exercises.import_proforma.import_errors.internal_error'), + actions: '', + } end def import_task @@ -175,10 +228,10 @@ def import_task rescue ProformaXML::ExerciseNotOwned render json: {}, status: :unauthorized rescue ProformaXML::ProformaError - render json: t('exercises.import_codeharbor.import_errors.invalid'), status: :bad_request + render json: t('exercises.import_proforma.import_errors.invalid'), status: :bad_request rescue StandardError => e Sentry.capture_exception(e) - render json: t('exercises.import_codeharbor.import_errors.internal_error'), status: :internal_server_error + render json: t('exercises.import_proforma.import_errors.internal_error'), status: :internal_server_error end def download_proforma @@ -579,4 +632,17 @@ def study_group_dashboard @graph_data = @exercise.get_working_times_for_study_group(@study_group_id) end + + private + + def uuid_check(user:, uuid:) + return {uuid_found: false} if uuid.blank? + + exercise = Exercise.find_by(uuid:) + + return {uuid_found: false} if exercise.nil? + return {uuid_found: true, update_right: false} unless ExercisePolicy.new(user, exercise).update? + + {uuid_found: true, update_right: true} + end end diff --git a/app/errors/proformaxml/invalid_zip.rb b/app/errors/proformaxml/invalid_zip.rb new file mode 100644 index 000000000..43b434619 --- /dev/null +++ b/app/errors/proformaxml/invalid_zip.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +module ProformaXML + class InvalidZip < ApplicationError; end +end diff --git a/app/services/proforma_service/convert_task_to_exercise.rb b/app/services/proforma_service/convert_task_to_exercise.rb index 8f64a3ade..238ce4eb3 100644 --- a/app/services/proforma_service/convert_task_to_exercise.rb +++ b/app/services/proforma_service/convert_task_to_exercise.rb @@ -23,7 +23,7 @@ def import_task @exercise.assign_attributes( user: @user, title: @task.title, - description: @task.description, + description: @task.description.presence || @task.title, public: string_to_bool(extract_meta_data(@task.meta_data&.dig('meta-data'), 'public')) || false, hide_file_tree: string_to_bool(extract_meta_data(@task.meta_data&.dig('meta-data'), 'hide_file_tree')) || false, allow_file_creation: string_to_bool(extract_meta_data(@task.meta_data&.dig('meta-data'), 'allow_file_creation')) || false, diff --git a/app/services/proforma_service/import.rb b/app/services/proforma_service/import.rb index 86e11d5fd..156a70189 100644 --- a/app/services/proforma_service/import.rb +++ b/app/services/proforma_service/import.rb @@ -2,10 +2,11 @@ module ProformaService class Import < ServiceBase - def initialize(zip:, user:) + def initialize(zip:, user:, import_type: 'import') super() @zip = zip @user = user + @import_type = import_type end def execute @@ -23,6 +24,8 @@ def execute private def base_exercise + return Exercise.new(uuid: SecureRandom.uuid, unpublished: true) if @import_type == 'create_new' + exercise = Exercise.find_by(uuid: @task.uuid) if exercise raise ProformaXML::ExerciseNotOwned unless ExercisePolicy.new(@user, exercise).update? diff --git a/app/services/proforma_service/uuid_from_zip.rb b/app/services/proforma_service/uuid_from_zip.rb new file mode 100644 index 000000000..1fbe986f3 --- /dev/null +++ b/app/services/proforma_service/uuid_from_zip.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module ProformaService + class UuidFromZip < ServiceBase + def initialize(zip:) + super() + @zip = zip + end + + def execute + if xml_exists_in_zip? + importer = ProformaXML::Importer.new(zip: @zip) + import_result = importer.perform + task = import_result + task.uuid + end + rescue Zip::Error + raise ProformaXML::InvalidZip.new I18n.t('exercises.import_proforma.import_errors.invalid_zip') + end + + private + + def xml_exists_in_zip? + filenames = Zip::File.open(@zip.path) do |zip_file| + zip_file.map(&:name) + end + + return true if filenames.any? {|f| f[/\.xml$/] } + + raise ProformaXML::InvalidZip.new I18n.t('exercises.import_proforma.import_errors.no_xml_found') + end + end +end diff --git a/app/uploaders/proforma_zip_uploader.rb b/app/uploaders/proforma_zip_uploader.rb new file mode 100644 index 000000000..51136e476 --- /dev/null +++ b/app/uploaders/proforma_zip_uploader.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class ProformaZipUploader < CarrierWave::Uploader::Base + def filename + SecureRandom.uuid + end +end diff --git a/app/views/exercises/_export_dialogcontent.html.slim b/app/views/exercises/_export_dialogcontent.html.slim deleted file mode 100644 index bae048800..000000000 --- a/app/views/exercises/_export_dialogcontent.html.slim +++ /dev/null @@ -1,3 +0,0 @@ -#export-exercise - .export-message - .export-exercise-actions diff --git a/app/views/exercises/_import_actions.html.slim b/app/views/exercises/_import_actions.html.slim new file mode 100644 index 000000000..e7bcf8b11 --- /dev/null +++ b/app/views/exercises/_import_actions.html.slim @@ -0,0 +1,14 @@ +- if imported + = link_to t('exercises.import_proforma.button.show_exercise'), exercise, class: 'btn btn-light btn-sm float-end show-action import-export-button', target: '_blank', rel: 'noopener noreferrer' +- elsif exists && updatable + = button_tag type: 'button', class: 'btn btn-success btn-sm float-end import-action import-export-button', data: {'import-type' => 'import', 'file-id' => file_id} do + i.fa-solid.fa-check.confirm-icon.export-button-icon + = t('exercises.import_proforma.button.overwrite') +- elsif exists + = button_tag type: 'button', class: 'btn btn-success btn-sm float-end import-action import-export-button', data: {'import-type' => 'create_new', 'file-id' => file_id} do + i.fa-solid.fa-check.confirm-icon-alt.export-button-icon + = t('exercises.import_proforma.button.import_copy') +- else + = button_tag type: 'button', class: 'btn btn-success btn-sm float-end import-action import-export-button', data: {'import-type' => 'import', 'file-id' => file_id} do + i.fa-solid.fa-check.confirm-icon.export-button-icon + = t('exercises.import_proforma.button.import') diff --git a/app/views/exercises/_transfer_dialogcontent.html.slim b/app/views/exercises/_transfer_dialogcontent.html.slim new file mode 100644 index 000000000..759b88c56 --- /dev/null +++ b/app/views/exercises/_transfer_dialogcontent.html.slim @@ -0,0 +1,3 @@ +#exercise-transfer + .transfer-message + .transfer-exercise-actions diff --git a/app/views/exercises/index.html.slim b/app/views/exercises/index.html.slim index 8b78290a1..6b13278db 100644 --- a/app/views/exercises/index.html.slim +++ b/app/views/exercises/index.html.slim @@ -57,9 +57,14 @@ h1 = Exercise.model_name.human(count: :other) li = link_to(t('exercises.download_proforma.label'), download_proforma_exercise_path(exercise), class: 'dropdown-item', target: '_blank', rel: 'noopener noreferrer') if policy(exercise).download_proforma? = render('shared/pagination', collection: @exercises) -p = render('shared/new_button', model: Exercise) +p + = render('shared/new_button', model: Exercise) + - if policy(Exercise).new? + button.btn.btn-success.import-start type='button' data-bs-toggle='modal' data-bs-target='#transfer-modal' + i.fa-solid.fa-upload + = t('exercises.import_proforma.label') = render 'shared/modal', title: t('exercises.export_codeharbor.dialogtitle'), - modal_root_attributes: {id: 'export-modal'}, - template: 'exercises/_export_dialogcontent' + modal_root_attributes: {id: 'transfer-modal'}, + template: 'exercises/_transfer_dialogcontent' diff --git a/app/views/exercises/show.html.slim b/app/views/exercises/show.html.slim index 7e03e478d..0650457c2 100644 --- a/app/views/exercises/show.html.slim +++ b/app/views/exercises/show.html.slim @@ -68,5 +68,5 @@ ul.list-unstyled#files - if policy(@exercise).export_external_confirm? = render 'shared/modal', title: t('exercises.export_codeharbor.dialogtitle'), - modal_root_attributes: {id: 'export-modal'}, - template: 'exercises/_export_dialogcontent' + modal_root_attributes: {id: 'transfer-modal'}, + template: 'exercises/_transfer_dialogcontent' diff --git a/config/locales/de/exercise.yml b/config/locales/de/exercise.yml index 74a944d77..cccc7916a 100644 --- a/config/locales/de/exercise.yml +++ b/config/locales/de/exercise.yml @@ -197,10 +197,30 @@ de: tips_intervention: text: Falls Sie bei dieser Aufgabe momentan nicht weiterkommen, können Sie die verfügbaren Tipps vielleicht unterstützen. Diese finden Sie jederzeit in der linken Seitenleiste und werden auch direkt hier angezeigt. unpublished: Die gewählte Aufgabe wurde deaktiviert und kann daher derzeit nicht bearbeitet werden. - import_codeharbor: + import_confirm: + error: Import von ProFormA Task ist fehlgeschlagen. + success: Import von ProFormA Task war erfolgreich. + import_proforma: + button: + import: Importieren + import_copy: Importiere Kopie + overwrite: Überschreiben + show_exercise: Zeige Aufgabe + dialog: + start: Klicke auf Datei hochladen und wähle eine ProFormAXML ZIP Datei aus, um das Importieren zu starten. import_errors: internal_error: Beim Import der Aufgabe ist ein interner Fehler aufgetreten. invalid: Fehlerhafte Aufgabe + invalid_zip: Ungültige ZIP Datei + no_xml_found: Keine XML Datei in ZIP Datei gefunden + label: ProFormAXML ZIP importieren + import_start: + choose_file_error: Sie müssen eine Datei auswählen. + error: 'Der folgende Fehler ist aufgetreten: %{message}.' + exercise_exists_and_is_not_updatable: Die Übung existiert bereits in CodeOcean. Sie haben nicht die erforderlichen Rechte, um die bestehende Übung zu aktualisieren. Sie können sie als Kopie importieren. + exercise_exists_and_is_updatable: Die Aufgabe existiert bereits in CodeOcean. Die Übung existiert bereits in CodeOcean. Sie kann importiert werden und überschreibt die bestehende Version. + exercise_is_importable: Diese Übung kann importiert werden. + upload_file: Datei hochladen index: clone: Duplizieren external_user_statistics: Statistik für externe Personen diff --git a/config/locales/en/exercise.yml b/config/locales/en/exercise.yml index 12c14f65d..f8a50412a 100644 --- a/config/locales/en/exercise.yml +++ b/config/locales/en/exercise.yml @@ -197,10 +197,30 @@ en: tips_intervention: text: If you are struggling with this exercise, the available tips may help. You can find them at any time in the left sidebar and are also displayed below for your convenience. unpublished: The selected exercise has been deactivated. Hence, you cannot implement this exercise at the moment. - import_codeharbor: + import_confirm: + error: Import of ProFormA task failed. + success: Import of ProFormA Task was successful. + import_proforma: + button: + import: Import + import_copy: Import copy + overwrite: Overwrite + show_exercise: Show exercise + dialog: + start: Press Upload and select and ProFormAXML ZIP file to start importing. import_errors: internal_error: An internal error occurred on CodeOcean while importing the exercise. invalid: Invalid exercise + invalid_zip: Invalid ZIP file + no_xml_found: No XML file found in ZIP + label: Import ProFormAXML ZIP + import_start: + choose_file_error: You need to choose a file. + error: 'The following error occured: %{message}.' + exercise_exists_and_is_not_updatable: Exercise already exists in CodeOcean. You have insufficient right to update the existing exercise. You can import it as a copy. + exercise_exists_and_is_updatable: The exercise already exists in CodeOcean. The exercise already exists in CodeOcean. It can be imported and will overwrite the existing one. + exercise_is_importable: This exercise can be imported. + upload_file: Upload file index: clone: Duplicate external_user_statistics: External User Statistics diff --git a/config/routes.rb b/config/routes.rb index e996728ec..156a449f8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -77,6 +77,8 @@ resources :exercises do collection do + post :import_start + post :import_confirm match '', to: 'exercises#batch_update', via: %i[patch put] end diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 1c8d5c5cf..555cf093f 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -584,4 +584,161 @@ end end end + + describe 'POST #import_start' do + let(:valid_file) { fixture_file_upload('proforma_import/testfile.zip', 'application/zip') } + let(:invalid_file) { 'invalid_file' } + let(:mock_uploader) { instance_double(ProformaZipUploader) } + let(:uuid) { 'mocked-uuid' } + let(:post_request) { post :import_start, params: {file: file} } + let(:file) { valid_file } + + before do + allow(controller).to receive(:current_user).and_return(user) + allow(ProformaZipUploader).to receive(:new).and_return(mock_uploader) + end + + context 'when the file is valid' do + before do + allow(ProformaService::UuidFromZip).to receive(:call).and_return(uuid) + allow(mock_uploader).to receive(:cache!) + allow(mock_uploader).to receive(:cache_name).and_return('mocked-cache-name') + end + + context 'when the exercise exists and is updatable' do + before do + allow(controller).to receive(:uuid_check).with(user: user, uuid: uuid) + .and_return(uuid_found: true, update_right: true) + end + + it 'renders success JSON with updatable message' do + post_request + + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('success') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_start.exercise_exists_and_is_updatable')) + end + end + + context 'when the exercise exists but is not updatable' do + before do + allow(controller).to receive(:uuid_check).with(user: user, uuid: uuid) + .and_return(uuid_found: true, update_right: false) + end + + it 'renders success JSON with not updatable message' do + post_request + + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('success') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_start.exercise_exists_and_is_not_updatable')) + end + end + + context 'when the exercise does not exist' do + before do + allow(controller).to receive(:uuid_check).with(user: user, uuid: uuid) + .and_return(uuid_found: false, update_right: false) + end + + it 'renders success JSON with importable message' do + post_request + + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('success') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_start.exercise_is_importable')) + end + end + end + + context 'when the file is invalid' do + let(:file) { invalid_file } + + it 'renders failure JSON with correct error' do + post_request + + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('failure') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_start.choose_file_error')) + end + end + + context 'when the uploaded zip file is invalid' do + it 'renders failure JSON with correct error' do + error_message = I18n.t('exercises.import_proforma.import_errors.invalid_zip') + allow(ProformaService::UuidFromZip).to receive(:call).and_raise(ProformaXML::InvalidZip.new(error_message)) + + post_request + expect(response).to have_http_status(:ok) + + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('failure') + expect(parsed_response['message']).to include(error_message) + end + end + end + + describe 'POST #import_confirm' do + let(:file_id) { 'file_id' } + let(:mock_uploader) { instance_double(ProformaZipUploader, file: 'mocked_file') } + let(:post_request) { post :import_confirm, params: {file_id: file_id} } + + before do + allow(ProformaZipUploader).to receive(:new).and_return(mock_uploader) + end + + context 'when the import is successful' do + before do + allow(mock_uploader).to receive(:retrieve_from_cache!).with(file_id) + allow(ProformaService::Import).to receive(:call).with(zip: 'mocked_file', user: user).and_return(exercise) + allow(exercise).to receive(:save!).and_return(true) + end + + it 'renders success JSON' do + post_request + + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('success') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_confirm.success')) + end + end + + context 'when ProformaError or validation error occurs' do + before do + allow(mock_uploader).to receive(:retrieve_from_cache!).with(file_id) + allow(ProformaService::Import).to receive(:call).and_raise(ProformaXML::ProformaError, 'Proforma error') + end + + it 'renders failure JSON' do + post_request + + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('failure') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_confirm.error', error: 'Proforma error message')) + end + end + + context 'when StandardError occurs' do + before do + allow(mock_uploader).to receive(:retrieve_from_cache!).and_raise(StandardError, 'Unexpected error') + allow(Sentry).to receive(:capture_exception) + end + + it 'logs the error and renders internal error JSON' do + post_request + + expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError)) + expect(response).to have_http_status(:ok) + parsed_response = response.parsed_body + expect(parsed_response['status']).to eq('failure') + expect(parsed_response['message']).to eq(I18n.t('exercises.import_proforma.import_errors.internal_error')) + end + end + end end diff --git a/spec/fixtures/files/proforma_import/corrupt.zip b/spec/fixtures/files/proforma_import/corrupt.zip new file mode 100644 index 000000000..b43a4b260 Binary files /dev/null and b/spec/fixtures/files/proforma_import/corrupt.zip differ diff --git a/spec/fixtures/files/proforma_import/empty.zip b/spec/fixtures/files/proforma_import/empty.zip new file mode 100644 index 000000000..a0220a87a Binary files /dev/null and b/spec/fixtures/files/proforma_import/empty.zip differ diff --git a/spec/fixtures/files/proforma_import/testfile.zip b/spec/fixtures/files/proforma_import/testfile.zip new file mode 100644 index 000000000..c7d2d45f5 Binary files /dev/null and b/spec/fixtures/files/proforma_import/testfile.zip differ diff --git a/spec/services/proforma_service/convert_task_to_exercise_spec.rb b/spec/services/proforma_service/convert_task_to_exercise_spec.rb index ba7a9011e..f60cea59b 100644 --- a/spec/services/proforma_service/convert_task_to_exercise_spec.rb +++ b/spec/services/proforma_service/convert_task_to_exercise_spec.rb @@ -31,7 +31,7 @@ let(:task) do ProformaXML::Task.new( title: 'title', - description: 'description', + description:, proglang: {name: 'python', version: '3.4'}, uuid: 'uuid', parent_uuid: 'parent_uuid', @@ -49,6 +49,7 @@ let(:model_solutions) { [] } let(:exercise) { nil } + let(:description) { 'description' } let(:meta_data) { {} } let(:public) { 'true' } let(:hide_file_tree) { 'true' } @@ -76,6 +77,16 @@ it { is_expected.to be_valid } + context 'when task has no description' do + let(:description) { nil } + + it 'creates an exercise with the correct description fallback' do + expect(convert_to_exercise_service).to have_attributes( + description: 'title' + ) + end + end + context 'when meta_data is set' do let(:meta_data) do { diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb index 53290f889..e5239c2da 100644 --- a/spec/services/proforma_service/import_spec.rb +++ b/spec/services/proforma_service/import_spec.rb @@ -16,11 +16,26 @@ it 'assigns user' do expect(import_service.instance_variable_get(:@user)).to be user end + + it 'assigns import_type' do + expect(import_service.instance_variable_get(:@import_type)).to be 'import' + end + + context 'when import_type is supplied' do + subject(:import_service) { described_class.new(zip:, user:, import_type:) } + + let(:import_type) { 'create_new' } + + it 'assigns import_type' do + expect(import_service.instance_variable_get(:@import_type)).to be import_type + end + end end describe '#execute' do - subject(:import_service) { described_class.call(zip: zip_file, user: import_user) } + subject(:import_service) { described_class.call(zip: zip_file, user: import_user, import_type:) } + let(:import_type) { 'import' } let(:user) { create(:teacher) } let(:import_user) { user } let(:zip_file) { Tempfile.new('proforma_test_zip_file', encoding: 'ascii-8bit') } @@ -58,6 +73,16 @@ expect(import_service.uuid).not_to be_blank end + context 'when import_type is create_new' do + let(:import_type) { 'create_new' } + + it { is_expected.not_to be_an_equal_exercise_as exercise } + + it 'creates a new task' do + expect(import_service).to be_new_record + end + end + context 'when no exercise exists' do before { exercise.destroy } diff --git a/spec/services/proforma_service/uuid_from_zip_spec.rb b/spec/services/proforma_service/uuid_from_zip_spec.rb new file mode 100644 index 000000000..540fca92a --- /dev/null +++ b/spec/services/proforma_service/uuid_from_zip_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'zip' + +RSpec.describe ProformaService::UuidFromZip do + describe '.new' do + subject(:service) { described_class.new(zip:) } + + let(:zip) { Tempfile.new('proforma_test_zip_file') } + + it 'assigns zip' do + expect(service.instance_variable_get(:@zip)).to be zip + end + end + + describe '#execute' do + subject(:service) { described_class.call(zip: service_input) } + + let(:valid_zip_file) { Rails.root.join('spec/fixtures/files/proforma_import/testfile.zip').open } + let(:empty_zip_file) { Rails.root.join('spec/fixtures/files/proforma_import/empty.zip').open } + let(:corrupt_zip_file) { Rails.root.join('spec/fixtures/files/proforma_import/corrupt.zip').open } + + context 'when the ZIP contains a valid XML file' do + let(:service_input) { valid_zip_file } + let(:importer_double) { instance_double(ProformaXML::Importer, perform: instance_double(ProformaXML::Task, uuid: '12345')) } + + it 'returns the task UUID' do + allow(ProformaXML::Importer).to receive(:new).with(zip: valid_zip_file).and_return(importer_double) + + expect(service).to eq('12345') + end + end + + context 'when the ZIP does not contain an XML file' do + let(:service_input) { empty_zip_file } + + it 'raises a ProformaXML::InvalidZip error with correct message' do + expect { service }.to raise_error(ProformaXML::InvalidZip, I18n.t('exercises.import_proforma.import_errors.no_xml_found')) + end + end + + context 'when the ZIP file cannot be opened' do + let(:service_input) { corrupt_zip_file } + + it 'raises a ProformaXML::InvalidZip with correct message' do + expect { service }.to raise_error(ProformaXML::InvalidZip, I18n.t('exercises.import_proforma.import_errors.invalid_zip')) + end + end + end +end diff --git a/spec/uploaders/proforma_zip_uploader_spec.rb b/spec/uploaders/proforma_zip_uploader_spec.rb new file mode 100644 index 000000000..67700ca45 --- /dev/null +++ b/spec/uploaders/proforma_zip_uploader_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ProformaZipUploader, type: :uploader do + subject(:uploader) { described_class.new } + + let(:file) { Rails.root.join('spec/fixtures/files/proforma_import/testfile.zip').open('r') } + + describe '#filename' do + before do + uploader.cache!(file) + end + + after do + uploader.remove! + end + + it 'generates a unique filename using SecureRandom.uuid' do + expect(uploader.filename).to match(/\b[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}\b/) + end + end +end