diff --git a/app/services/proforma_service/convert_exercise_to_task.rb b/app/services/proforma_service/convert_exercise_to_task.rb index 03bad6d8c..8907d3549 100644 --- a/app/services/proforma_service/convert_exercise_to_task.rb +++ b/app/services/proforma_service/convert_exercise_to_task.rb @@ -105,6 +105,9 @@ def tests end def xml_id_from_file(file) + xml_id_path = file.xml_id_path || [] + return xml_id_path if xml_id_path&.any? + type = if file.teacher_defined_assessment? 'test' elsif file.reference_implementation? @@ -112,11 +115,9 @@ def xml_id_from_file(file) else 'file' end - xml_id_path_parts = file.xml_id_path&.split('/') - xml_id_path = [] - xml_id_path << (xml_id_path_parts&.first || "co-#{type}-#{file.id}") unless type == 'file' - xml_id_path << (xml_id_path_parts&.last || file.id) + xml_id_path << "co-#{type}-#{file.id}" unless type == 'file' + xml_id_path << file.id xml_id_path end @@ -192,7 +193,7 @@ def task_files end def task_file(file) - file.update(xml_id_path: xml_id_from_file(file).join('/')) if file.xml_id_path.blank? + file.update(xml_id_path: xml_id_from_file(file)) if file.xml_id_path.blank? xml_id = xml_id_from_file(file).last task_file = ProformaXML::TaskFile.new( diff --git a/app/services/proforma_service/convert_task_to_exercise.rb b/app/services/proforma_service/convert_task_to_exercise.rb index 880915e36..8364fb9bd 100644 --- a/app/services/proforma_service/convert_task_to_exercise.rb +++ b/app/services/proforma_service/convert_task_to_exercise.rb @@ -97,7 +97,10 @@ def task_files def codeocean_file_from_task_file(file, parent_object = nil) extension = File.extname(file.filename) - codeocean_file = CodeOcean::File.where(context: @exercise).where('xml_id_path = ? OR xml_id_path LIKE ?', file.id, "%/#{file.id}").first_or_initialize(context: @exercise) + codeocean_file = CodeOcean::File + .where(context: @exercise) + .where('array_length(xml_id_path, 1) IS NOT NULL AND xml_id_path[array_length(xml_id_path, 1)] = ?', file.id) + .first_or_initialize(context: @exercise) codeocean_file.assign_attributes( context: @exercise, file_type: file_type(extension), @@ -106,7 +109,7 @@ def codeocean_file_from_task_file(file, parent_object = nil) read_only: file.usage_by_lms != 'edit', role: extract_meta_data(@task.meta_data&.dig('meta-data'), 'files', "CO-#{file.id}", 'role'), path: File.dirname(file.filename).in?(['.', '']) ? nil : File.dirname(file.filename), - xml_id_path: (parent_object.nil? ? '' : "#{parent_object.id}/") + file.id.to_s + xml_id_path: (parent_object.nil? ? [file.id] : [parent_object.id, file.id]) ) if file.binary codeocean_file.native_file = FileIO.new(file.content.dup.force_encoding('UTF-8'), File.basename(file.filename)) diff --git a/db/migrate/20240903204319_add_xml_path_to_files.rb b/db/migrate/20240903204319_add_xml_path_to_files.rb index 680624c15..80c492b6b 100644 --- a/db/migrate/20240903204319_add_xml_path_to_files.rb +++ b/db/migrate/20240903204319_add_xml_path_to_files.rb @@ -2,6 +2,6 @@ class AddXmlPathToFiles < ActiveRecord::Migration[7.1] def change - add_column :files, :xml_id_path, :string, null: true, default: nil + add_column :files, :xml_id_path, :string, array: true, default: [], null: true end end diff --git a/db/schema.rb b/db/schema.rb index 85221e599..b53e9f9db 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -315,7 +315,7 @@ t.string "path" t.integer "file_template_id" t.boolean "hidden_feedback", default: false, null: false - t.string "xml_id_path" + t.string "xml_id_path", default: [], array: true t.index ["context_id", "context_type"], name: "index_files_on_context_id_and_context_type" end diff --git a/spec/services/proforma_service/convert_exercise_to_task_spec.rb b/spec/services/proforma_service/convert_exercise_to_task_spec.rb index c832c832c..bf5e49b61 100644 --- a/spec/services/proforma_service/convert_exercise_to_task_spec.rb +++ b/spec/services/proforma_service/convert_exercise_to_task_spec.rb @@ -111,11 +111,11 @@ end it 'sets xml_id_path to default' do - expect { convert_to_task.execute }.to change(file.reload, :xml_id_path).from(nil).to(file.id.to_s) + expect { convert_to_task.execute }.to change(file.reload, :xml_id_path).from([]).to([file.id.to_s]) end context 'when xml_id_path is set for file' do - let(:file) { create(:file, xml_id_path: 'foobar') } + let(:file) { create(:file, xml_id_path: ['foobar']) } it 'does not change xml_path_id' do expect { convert_to_task.execute }.not_to change(file.reload, :xml_id_path) @@ -204,7 +204,7 @@ it 'creates a model-solution with one file with correct attributes' do expect(task.model_solutions.first.files.first).to have_attributes( - id: file.id.to_s, + id: file.id, content: file.content, filename: file.name_with_extension, used_by_grader: false, @@ -224,7 +224,7 @@ end context 'when reference_implementations belong to the same proforma model_solution' do - let(:files) { build_list(:file, 2, role: 'reference_implementation') {|file, i| file.xml_id_path = "proforma_ms/xml_id_#{i}" } } + let(:files) { build_list(:file, 2, role: 'reference_implementation') {|file, i| file.xml_id_path = ['proforma_ms', "xml_id_#{i}"] } } it 'creates a task with one model-solution' do expect(task.model_solutions).to have(1).items @@ -267,7 +267,7 @@ it 'creates a test with one file with correct attributes' do expect(task.tests.first.files.first).to have_attributes( - id: test_file.id.to_s, + id: test_file.id, content: test_file.content, filename: test_file.name_with_extension, used_by_grader: true, @@ -294,7 +294,7 @@ end context 'when test_files belong to the same proforma test' do - let(:tests) { build_list(:test_file, 2) {|file, i| file.xml_id_path = "proforma_test/xml_id_#{i}" } } + let(:tests) { build_list(:test_file, 2) {|file, i| file.xml_id_path = ['proforma_test', "xml_id_#{i}"] } } it 'creates a test with two file' do expect(task.tests.first).to have_attributes( 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 400d35820..5a7e36aa0 100644 --- a/spec/services/proforma_service/convert_task_to_exercise_spec.rb +++ b/spec/services/proforma_service/convert_task_to_exercise_spec.rb @@ -563,9 +563,9 @@ description: 'exercise-description') end let(:co_files) do - [create(:file, xml_id_path: 'id', role: 'regular_file'), - create(:file, xml_id_path: 'ms-id/ms-file-id', role: 'reference_implementation'), - create(:test_file, xml_id_path: 'test-id/test-file-id')] + [create(:file, xml_id_path: ['id'], role: 'regular_file'), + create(:file, xml_id_path: %w[ms-id ms-file-id], role: 'reference_implementation'), + create(:test_file, xml_id_path: %w[test-id test-file-id])] end it 'reuses existing file' do