Skip to content

Commit

Permalink
refactor xml_id_path to use array
Browse files Browse the repository at this point in the history
  • Loading branch information
kkoehn committed Sep 24, 2024
1 parent 4c88cf5 commit 20a2705
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 18 deletions.
11 changes: 6 additions & 5 deletions app/services/proforma_service/convert_exercise_to_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,19 @@ 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?
'ms'
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
Expand Down Expand Up @@ -190,7 +191,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(
Expand Down
7 changes: 5 additions & 2 deletions app/services/proforma_service/convert_task_to_exercise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/20240903204319_add_xml_path_to_files.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions spec/services/proforma_service/convert_exercise_to_task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 20a2705

Please sign in to comment.