Skip to content

Commit

Permalink
fixed missing (edge) cases for the transforms
Browse files Browse the repository at this point in the history
  • Loading branch information
@kkoehn
kkoehn committed Jul 30, 2024
1 parent 49216a4 commit 961d744
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 15 deletions.
54 changes: 42 additions & 12 deletions lib/proformaxml/services/transform_task.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,56 @@ def perform
private

def transform_from_2_0_to_2_1
if @task.submission_restrictions.present?
@task.submission_restrictions['submission-restrictions']['file-restriction'].each do |fr|
fr['@use'] = fr.delete('@required') == 'true' ? 'required' : 'optional'
end
end
transform_submission_restrictions_from_2_0_to_2_1 unless @task.submission_restrictions.nil?
transform_external_resources_from_2_0_to_2_1 unless @task.external_resources.nil?

@task.model_solutions.filter! {|model_solution| model_solution.id != 'ms-placeholder' }
end

def transform_from_2_1_to_2_0
unless @task.submission_restrictions.nil?
@task.submission_restrictions['submission-restrictions']['file-restriction'].each do |fr|
fr['@required'] = (fr.delete('@use') == 'required').to_s
end
@task.submission_restrictions['submission-restrictions'].delete('description')
@task.submission_restrictions['submission-restrictions'].delete('internal-description')
def transform_external_resources_from_2_0_to_2_1
ensure_array(@task.external_resources['external-resources']['external-resource']).each do |external_resource|
external_resource['@used-by-grader'] = external_resource['@used-by-grader'] || 'false'
external_resource['@visible'] = external_resource['@visible'] || 'no'
end
end

def transform_submission_restrictions_from_2_0_to_2_1
ensure_array(@task.submission_restrictions['submission-restrictions']['file-restriction']).each do |fr|
fr['@use'] = fr['@required'].nil? || fr.delete('@required') == 'true' ? 'required' : 'optional'
end
end

def transform_from_2_1_to_2_0
transform_submission_restrictions_from_2_1_to_2_0 unless @task.submission_restrictions.nil?
transform_external_resources_from_2_1_to_2_0 unless @task.external_resources.nil?
add_model_solution_placeholder
end

def transform_external_resources_from_2_1_to_2_0
ensure_array(@task.external_resources['external-resources']['external-resource']).each do |external_resource|
external_resource.delete('@visible')
external_resource.delete('@usage-by-lms')
external_resource.delete('@used-by-grader')
end
end

def transform_submission_restrictions_from_2_1_to_2_0
transform_file_restrictions_from_2_1_to_2_0
@task.submission_restrictions['submission-restrictions'].delete('description')
@task.submission_restrictions['submission-restrictions'].delete('internal-description')
end

def transform_file_restrictions_from_2_1_to_2_0
ensure_array(@task.submission_restrictions['submission-restrictions']['file-restriction']).each do |file_restriction|
file_restriction['@required'] = (file_restriction['@use'].nil? || file_restriction.delete('@use') == 'required').to_s
end
end

# when only one field is present, dachsfisch does not create an array. This method ensure, that we can work with an array
def ensure_array(data)
[data].flatten
end

def add_model_solution_placeholder
return if @task.model_solutions&.any?

Expand Down
118 changes: 118 additions & 0 deletions spec/factories/task.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
'@pattern-format' => 'posix-ere',
'$1' => 'restriction2',
},
{
'@@order' => %w[$1],
'$1' => 'restriction3',
},
],
'description' => {
'@@order' => %w[$1],
Expand All @@ -70,6 +74,40 @@
}
end
end

trait(:with_submission_restrictions_with_single_file_restriction) do
submission_restrictions do
{
'@@order' => %w[submission-restrictions],
'submission-restrictions' => {
'@@order' => %w[file-restriction description internal-description],
'file-restriction' => {
'@@order' => %w[$1],
'$1' => 'restriction1',
},
},
}
end
end

trait(:with_2_0_file_restrictions_with_single_file_restriction) do
submission_restrictions do
{
'@@order' => %w[submission-restrictions],
'submission-restrictions' => {
'@@order' => %w[file-restriction description internal-description],
'@max-size' => '50',
'file-restriction' => {
'@@order' => %w[$1],
'@required' => 'true',
'@pattern-format' => 'none',
'$1' => 'restriction1',
},
},
}
end
end

trait(:with_2_0_file_restrictions) do
submission_restrictions do
{
Expand All @@ -84,6 +122,10 @@
'@pattern-format' => 'none',
'$1' => 'restriction1',
},
{
'@@order' => %w[$1],
'$1' => 'restriction2',
},
],
},
}
Expand Down Expand Up @@ -143,6 +185,82 @@
}
end
end
trait(:with_2_0_external_resources) do
external_resources do
{
'@@order' => %w[external-resources],
'external-resources' => {
'@@order' => %w[external-resource],
'@xmlns' => {'foo' => 'urn:custom:foobar'},
'external-resource' => [
{
'@@order' => %w[internal-description foo:bar],
'@id' => 'external-resource 1',
},
{
'@@order' => %w[internal-description foo:bar],
'@id' => 'external-resource 2',
'@reference' => '2',
'internal-description' => {
'@@order' => %w[$1],
'$1' => 'internal-desc',
},
'foo:bar' => {
'@version' => '5',
'@@order' => %w[foo:content],
'foo:content' => {
'@@order' => %w[$1],
'$1' => 'barfoo',
},
},
},
],
},
}
end
end
trait(:with_2_0_external_resources_with_single_external_resource) do
external_resources do
{
'@@order' => %w[external-resources],
'external-resources' => {
'@@order' => %w[external-resource],
'@xmlns' => {'foo' => 'urn:custom:foobar'},
'external-resource' => {
'@@order' => %w[internal-description foo:bar],
'@id' => 'external-resource 1',
},
},
}
end
end
trait(:with_external_resources_with_single_external_resource) do
external_resources do
{
'@@order' => %w[external-resources],
'external-resources' => {
'@@order' => %w[external-resource],
'external-resource' => {
'@@order' => %w[foobar],
'@id' => 'external-resource 1',
'@reference' => '1',
'@used-by-grader' => 'true',
'@visible' => 'delayed',
'@usage-by-lms' => 'download',
'foobar' => {
'@@order' => %w[content],
'@version' => '4',
'content' => {
'@@order' => %w[$1],
'$1' => 'foobar',
},
},
},

},
}
end
end

trait(:with_grading_hints) do
grading_hints do
Expand Down
2 changes: 1 addition & 1 deletion spec/proformaxml/exporter_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@
end

it 'add file-restrictions to submission-restrictions' do
expect(xml.xpath('/task/submission-restrictions/file-restriction')).to have(2).items
expect(xml.xpath('/task/submission-restrictions/file-restriction')).to have(3).items
end
end

Expand Down
86 changes: 84 additions & 2 deletions spec/proformaxml/transform_task_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,33 @@
let(:to_version) { '2.1' }
let(:task) { build(:task, :with_2_0_file_restrictions) }

it_behaves_like 'validatable task'

it 'transforms file_restriction' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'][0] }
.from({'$1' => 'restriction1', '@@order' => ['$1'], '@pattern-format' => 'none', '@required' => 'true'})
.to({'$1' => 'restriction1', '@@order' => ['$1'], '@pattern-format' => 'none', '@use' => 'required'})
end

context 'with model solution placehoder' do
it 'transforms file_restriction with default values' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'][1] }
.from({'$1' => 'restriction2', '@@order' => ['$1']})
.to({'$1' => 'restriction2', '@@order' => ['$1'], '@use' => 'required'})
end

context 'with single file_restriction' do
let(:task) { build(:task, :with_2_0_file_restrictions_with_single_file_restriction) }

it 'transforms file_restriction' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'] }
.from({'$1' => 'restriction1', '@@order' => ['$1'], '@pattern-format' => 'none', '@required' => 'true'})
.to({'$1' => 'restriction1', '@@order' => ['$1'], '@pattern-format' => 'none', '@use' => 'required'})
end

it_behaves_like 'validatable task'
end

context 'with model solution placeholder' do
let(:task) { build(:task, :with_placeholder_model_solution) }

it 'removes model solution placeholder' do
Expand All @@ -66,7 +86,27 @@
end
end

it_behaves_like 'validatable task'
context 'with external-resources' do
let(:task) { build(:task, :with_2_0_external_resources) }

it 'transforms external_resource' do
expect { call }.to change { task.external_resources['external-resources']['external-resource'][0].slice('@visible', '@usage-by-lms', '@used-by-grader') }
.from({})
.to({'@used-by-grader' => 'false', '@visible' => 'no'})
end

context 'with single external-resource' do
let(:task) { build(:task, :with_2_0_external_resources_with_single_external_resource) }

it 'transforms external_resource' do
expect { call }.to change { task.external_resources['external-resources']['external-resource'].slice('@visible', '@usage-by-lms', '@used-by-grader') }
.from({})
.to({'@used-by-grader' => 'false', '@visible' => 'no'})
end
end

it_behaves_like 'validatable task'
end
end

context 'with from 2.1 to 2.0' do
Expand All @@ -80,12 +120,28 @@
.to({'$1' => 'restriction1', '@@order' => ['$1'], '@pattern-format' => 'none', '@required' => 'true'})
end

it 'transforms file_restriction without attributes' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'][2] }
.from({'$1' => 'restriction3', '@@order' => ['$1']})
.to({'$1' => 'restriction3', '@@order' => ['$1'], '@required' => 'true'})
end

it 'removes description and internal-description from submission-restrictions' do
expect { call }.to change { task.submission_restrictions['submission-restrictions'].keys }
.from(include('description', 'internal-description'))
.to(not_include('description', 'internal-description'))
end

context 'when submission_restriction has only one file_restriction' do
let(:task) { build(:task, :with_submission_restrictions_with_single_file_restriction) }

it 'transforms file_restriction' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'] }
.from({'$1' => 'restriction1', '@@order' => ['$1']})
.to({'$1' => 'restriction1', '@@order' => ['$1'], '@required' => 'true'})
end
end

context 'without model_solutions' do
it 'adds a model solution placeholder' do
expect { call }.to change(task, :model_solutions)
Expand All @@ -102,6 +158,32 @@
end
end

context 'with external_resources' do
let(:task) { build(:task, :with_external_resources) }

it 'transforms external_resource' do
expect { call }.to change { task.external_resources['external-resources']['external-resource'][0].slice('@visible', '@usage-by-lms', '@used-by-grader') }
.from({'@usage-by-lms' => 'download', '@used-by-grader' => 'true', '@visible' => 'delayed'})
.to({})
end

context 'with external_resources with single external_resource' do
let(:task) { build(:task, :with_external_resources_with_single_external_resource) }

it 'transforms external_resource' do
expect { call }.to change { task.external_resources['external-resources']['external-resource'].slice('@visible', '@usage-by-lms', '@used-by-grader') }
.from({'@usage-by-lms' => 'download', '@used-by-grader' => 'true', '@visible' => 'delayed'})
.to({})
end
end
end

context 'with everything' do
let(:task) { build(:task, :with_everything) }

it_behaves_like 'validatable task'
end

it_behaves_like 'validatable task'
end
end
Expand Down

0 comments on commit 961d744

Please sign in to comment.