Skip to content

Commit

Permalink
basic functionality of reusing files based on xml_id.
Browse files Browse the repository at this point in the history
Use xml_id_path to recreate proforma structure with tests and model_solutions containing multiple files
  • Loading branch information
kkoehn committed Sep 24, 2024
1 parent 472ef0b commit ac0fe42
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 81 deletions.
95 changes: 61 additions & 34 deletions app/services/proforma_service/convert_exercise_to_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,62 +77,86 @@ def uuid
end

def model_solutions
@exercise.files.filter {|file| file.role == 'reference_implementation' }.map do |file|
model_solutions_files = @exercise.files.filter {|file| file.role == 'reference_implementation' }.group_by {|file| xml_id_from_file(file).first }
model_solutions_files.map do |xml_id, files|
ProformaXML::ModelSolution.new(
id: "ms-#{file.id}",
files: model_solution_file(file)
id: xml_id,
files: files.map {|file| model_solution_file(file) }
)
end
end

def model_solution_file(file)
[
task_file(file).tap do |ms_file|
ms_file.used_by_grader = false
ms_file.usage_by_lms = 'display'
end,
]
task_file(file).tap do |ms_file|
ms_file.used_by_grader = false
ms_file.usage_by_lms = 'display'
end
end

def tests
@exercise.files.filter(&:teacher_defined_assessment?).map do |file|
@exercise.files.filter(&:teacher_defined_assessment?).group_by {|file| xml_id_from_file(file).first }.map do |xml_id, files|
ProformaXML::Test.new(
id: file.id,
title: file.name,
files: test_file(file),
meta_data: test_meta_data(file)
id: xml_id,
title: files.first.name,
files: files.map {|file| test_file(file) },
meta_data: test_meta_data(files)
)
end
end

def test_meta_data(file)
def xml_id_from_file(file)
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
end

def test_meta_data(files)
{
'@@order' => %w[test-meta-data],
'test-meta-data' => {
'@@order' => %w[CodeOcean:feedback-message CodeOcean:weight CodeOcean:hidden-feedback],
'@@order' => %w[CodeOcean:test-file],
'@xmlns' => {'CodeOcean' => 'codeocean.openhpi.de'},
'CodeOcean:feedback-message' => {
'@@order' => %w[$1],
'$1' => file.feedback_message,
},
'CodeOcean:weight' => {
'@@order' => %w[$1],
'$1' => file.weight,
},
'CodeOcean:hidden-feedback' => {
'@@order' => %w[$1],
'$1' => file.hidden_feedback,
},
'CodeOcean:test-file' => files.to_h do |file|
[
"CodeOcean:#{xml_id_from_file(file).last}", {
'@@order' => %w[CodeOcean:feedback-message CodeOcean:weight CodeOcean:hidden-feedback],
'@xmlns' => {'CodeOcean' => 'codeocean.openhpi.de'},
'@id' => xml_id_from_file(file).last,
'@name' => file.name,
'CodeOcean:feedback-message' => {
'@@order' => %w[$1],
'$1' => file.feedback_message,
},
'CodeOcean:weight' => {
'@@order' => %w[$1],
'$1' => file.weight,
},
'CodeOcean:hidden-feedback' => {
'@@order' => %w[$1],
'$1' => file.hidden_feedback,
},
}
]
end,
},
}
end

def test_file(file)
[
task_file(file).tap do |t_file|
t_file.used_by_grader = true
end,
]
task_file(file).tap do |t_file|
t_file.used_by_grader = true
end
end

def exercise_files
Expand Down Expand Up @@ -168,8 +192,11 @@ 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?

xml_id = xml_id_from_file(file).last
task_file = ProformaXML::TaskFile.new(
id: file.id,
id: xml_id,
filename: filename(file),
usage_by_lms: file.read_only ? 'display' : 'edit',
visible: file.hidden ? 'no' : 'yes'
Expand Down
39 changes: 24 additions & 15 deletions app/services/proforma_service/convert_task_to_exercise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,44 +60,53 @@ def string_to_bool(str)
end

def files
model_solution_files + test_files + task_files.values.tap {|array| array.each {|file| file.role ||= 'regular_file' } }
model_solution_files + test_files + task_files
end

def test_files
@task.tests.map do |test_object|
task_files.delete(test_object.files.first.id).tap do |file|
file.weight = extract_meta_data(test_object.meta_data&.dig('test-meta-data'), 'weight').presence || 1.0
file.feedback_message = extract_meta_data(test_object.meta_data&.dig('test-meta-data'), 'feedback-message').presence || 'Feedback'
file.hidden_feedback = extract_meta_data(test_object.meta_data&.dig('test-meta-data'), 'hidden-feedback').presence || false
file.role ||= 'teacher_defined_test'
@task.tests.flat_map do |test|
test.files.map do |task_file|
codeocean_file_from_task_file(task_file, test).tap do |file|
file.weight = extract_meta_data(test.meta_data&.dig('test-meta-data'), 'test-file', task_file.id, 'weight').presence || 1.0
file.feedback_message = extract_meta_data(test.meta_data&.dig('test-meta-data'), 'test-file', task_file.id, 'feedback-message').presence || 'Feedback'
file.hidden_feedback = extract_meta_data(test.meta_data&.dig('test-meta-data'), 'test-file', task_file.id, 'hidden-feedback').presence || false
file.role = 'teacher_defined_test' unless file.teacher_defined_assessment?
end
end
end
end

def model_solution_files
@task.model_solutions.map do |model_solution_object|
task_files.delete(model_solution_object.files.first.id).tap do |file|
file.role ||= 'reference_implementation'
@task.model_solutions.flat_map do |model_solution|
model_solution.files.map do |task_file|
codeocean_file_from_task_file(task_file, model_solution).tap do |file|
file.role ||= 'reference_implementation'
end
end
end
end

def task_files
@task_files ||= @task.all_files.reject {|file| file.id == 'ms-placeholder-file' }.to_h do |task_file|
[task_file.id, codeocean_file_from_task_file(task_file)]
@task.files.reject {|file| file.id == 'ms-placeholder-file' }.map do |task_file|
codeocean_file_from_task_file(task_file).tap do |file|
file.role ||= 'regular_file'
end
end
end

def codeocean_file_from_task_file(file)
def codeocean_file_from_task_file(file, parent_object = nil)
extension = File.extname(file.filename)
codeocean_file = CodeOcean::File.new(

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.assign_attributes(
context: @exercise,
file_type: file_type(extension),
hidden: file.visible != 'yes', # hides 'delayed' and 'no'
name: File.basename(file.filename, '.*'),
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)
path: File.dirname(file.filename).in?(['.', '']) ? nil : File.dirname(file.filename),
xml_id_path: (parent_object.nil? ? '' : "#{parent_object.id}/") + file.id.to_s
)
if file.binary
codeocean_file.native_file = FileIO.new(file.content.dup.force_encoding('UTF-8'), File.basename(file.filename))
Expand Down
8 changes: 1 addition & 7 deletions app/services/proforma_service/import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,7 @@ def execute
import_result = importer.perform
@task = import_result

exercise = base_exercise
exercise_files = exercise&.files&.to_a

exercise = ConvertTaskToExercise.call(task: @task, user: @user, exercise:)
exercise_files&.each(&:destroy) # feels suboptimal

exercise
ConvertTaskToExercise.call(task: @task, user: @user, exercise: base_exercise)
else
import_multi
end
Expand Down
7 changes: 7 additions & 0 deletions db/migrate/20240903204319_add_xml_path_to_files.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class AddXmlPathToFiles < ActiveRecord::Migration[7.1]
def change
add_column :files, :xml_id_path, :string, null: true, default: nil
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2023_12_08_194632) do
ActiveRecord::Schema[7.1].define(version: 2024_09_03_204319) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
enable_extension "pgcrypto"
Expand Down Expand Up @@ -315,6 +315,7 @@
t.string "path"
t.integer "file_template_id"
t.boolean "hidden_feedback", default: false, null: false
t.string "xml_id_path"
t.index ["context_id", "context_type"], name: "index_files_on_context_id_and_context_type"
end

Expand Down
79 changes: 69 additions & 10 deletions spec/services/proforma_service/convert_exercise_to_task_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,11 @@

context 'when exercise has a mainfile' do
let(:files) { [file] }
let(:file) { build(:file) }
let(:file) { create(:file) }

it 'creates a task-file with the correct attributes' do
expect(task.files.first).to have_attributes(
id: file.id,
id: file.id.to_s,
content: file.content,
filename: file.name_with_extension,
used_by_grader: true,
Expand All @@ -109,6 +109,18 @@
)
)
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)
end

context 'when xml_id_path is set for file' do
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)
end
end
end

context 'when exercise has a regular file' do
Expand All @@ -119,7 +131,7 @@

it 'creates a task-file with the correct attributes' do
expect(task.files.first).to have_attributes(
id: file.id,
id: file.id.to_s,
content: file.content,
filename: file.name_with_extension,
used_by_grader: true,
Expand Down Expand Up @@ -185,14 +197,14 @@

it 'creates a model-solution with one file' do
expect(task.model_solutions.first).to have_attributes(
id: "ms-#{file.id}",
id: "co-ms-#{file.id}",
files: have(1).item
)
end

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,
id: file.id.to_s,
content: file.content,
filename: file.name_with_extension,
used_by_grader: false,
Expand All @@ -210,6 +222,18 @@
it 'creates a task with two model-solutions' do
expect(task.model_solutions).to have(2).items
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}" } }

it 'creates a task with one model-solution' do
expect(task.model_solutions).to have(1).items
end

it 'creates a task with a model-solution with two files' do
expect(task.model_solutions.first.files).to have(2).items
end
end
end

context 'when exercise has a test' do
Expand All @@ -223,22 +247,27 @@

it 'creates a test with one file' do
expect(task.tests.first).to have_attributes(
id: test_file.id,
id: "co-test-#{test_file.id}",
title: test_file.name,
files: have(1).item,
meta_data: a_hash_including(
'test-meta-data' => a_hash_including(
'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
'CodeOcean:weight' => {'$1' => test_file.weight, '@@order' => ['$1']},
'CodeOcean:hidden-feedback' => {'$1' => test_file.hidden_feedback, '@@order' => ['$1']}
'CodeOcean:test-file' => a_hash_including(
"CodeOcean:#{test_file.id}" => a_hash_including(
'@name' => test_file.name,
'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
'CodeOcean:weight' => {'$1' => test_file.weight, '@@order' => ['$1']},
'CodeOcean:hidden-feedback' => {'$1' => test_file.hidden_feedback, '@@order' => ['$1']}
)
)
)
)
)
end

it 'creates a test with one file with correct attributes' do
expect(task.tests.first.files.first).to have_attributes(
id: test_file.id,
id: test_file.id.to_s,
content: test_file.content,
filename: test_file.name_with_extension,
used_by_grader: true,
Expand All @@ -263,6 +292,36 @@
it 'creates a task with two tests' do
expect(task.tests).to have(2).items
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}" } }

it 'creates a test with two file' do
expect(task.tests.first).to have_attributes(
id: 'proforma_test',
title: tests.first.name,
files: have(2).item,
meta_data: a_hash_including(
'test-meta-data' => a_hash_including(
'CodeOcean:test-file' => a_hash_including(
'CodeOcean:xml_id_0' => a_hash_including(
'@name' => tests.first.name,
'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
'CodeOcean:weight' => {'$1' => 1, '@@order' => ['$1']},
'CodeOcean:hidden-feedback' => {'$1' => tests.first.hidden_feedback, '@@order' => ['$1']}
),
'CodeOcean:xml_id_1' => a_hash_including(
'@name' => tests.last.name,
'CodeOcean:feedback-message' => {'$1' => 'feedback_message', '@@order' => ['$1']},
'CodeOcean:weight' => {'$1' => 1, '@@order' => ['$1']},
'CodeOcean:hidden-feedback' => {'$1' => tests.last.hidden_feedback, '@@order' => ['$1']}
)
)
)
)
)
end
end
end
end
end
Loading

0 comments on commit ac0fe42

Please sign in to comment.