Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix task transform #409

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Fix task transform #409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 47 additions & 13 deletions lib/proformaxml/services/transform_task.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require 'proformaxml/helpers/export_helpers'
require 'active_support/core_ext/array/wrap'

module ProformaXML
class TransformTask < ServiceBase
Expand All @@ -23,26 +23,60 @@ 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'] = if fr['@required'].nil? || fr.delete('@required') == 'true'
'required'
else
'optional'
end
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)
Array.wrap(data)
end

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

Expand Down
124 changes: 124 additions & 0 deletions spec/factories/task.rb
View file
Open in desktop
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,16 @@
'@pattern-format' => 'none',
'$1' => 'restriction1',
},
{
'@@order' => %w[$1],
'@required' => 'false',
'@pattern-format' => 'none',
'$1' => 'restriction2',
},
{
'@@order' => %w[$1],
'$1' => 'restriction3',
},
],
},
}
Expand Down Expand Up @@ -143,6 +191,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
Open in desktop
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
Dome-GER marked this conversation as resolved.
Show resolved Hide resolved
end
end

Expand Down
94 changes: 91 additions & 3 deletions spec/proformaxml/transform_task_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,39 @@
let(:to_version) { '2.1' }
let(:task) { build(:task, :with_2_0_file_restrictions) }

it 'transforms file_restriction' do
it_behaves_like 'validatable task'

it 'transforms file_restriction required -> true to required' 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 required -> false to optional' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'][1] }
.from({'$1' => 'restriction2', '@@order' => ['$1'], '@pattern-format' => 'none', '@required' => 'false'})
.to({'$1' => 'restriction2', '@@order' => ['$1'], '@pattern-format' => 'none', '@use' => 'optional'})
end

it 'transforms file_restriction with default values' do
expect { call }.to change { task.submission_restrictions['submission-restrictions']['file-restriction'][2] }
.from({'$1' => 'restriction3', '@@order' => ['$1']})
.to({'$1' => 'restriction3', '@@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 +92,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 +126,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 +164,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