fixed missing (edge) cases for the transform from 2.0 to 2.1

openHPI · Jul 29, 2024 · a866663 · a866663
1 parent 413e332
commit a866663
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 24 deletions.
diff --git a/lib/proformaxml/services/transform_task.rb b/lib/proformaxml/services/transform_task.rb
@@ -24,8 +24,9 @@ def perform
 
     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'
+        # when only one file-restriction is present, dachsfisch does not create an array
+        [@task.submission_restrictions['submission-restrictions']['file-restriction']].flatten.each do |file_restriction|
+          file_restriction['@use'] = file_restriction.delete('@required') == 'true' ? 'required' : 'optional'
         end
       end
 
@@ -40,21 +41,13 @@ def transform_from_2_1_to_2_0
 
     def transform_external_resources_from_2_1_to_2_0
       # when only one external-resource is present, dachsfisch does not create an array
-      if @task.external_resources['external-resources']['external-resource'].is_a? Hash
-        remove_attributes_in_external_resource @task.external_resources['external-resources']['external-resource']
-      else
-        @task.external_resources['external-resources']['external-resource'].each do |external_resource|
-          remove_attributes_in_external_resource external_resource
-        end
+      [@task.external_resources['external-resources']['external-resource']].flatten.each do |external_resource|
+        external_resource.delete('@visible')
+        external_resource.delete('@usage-by-lms')
+        external_resource.delete('@used-by-grader')
       end
     end
 
-    def remove_attributes_in_external_resource(external_resource)
-      external_resource.delete('@visible')
-      external_resource.delete('@usage-by-lms')
-      external_resource.delete('@used-by-grader')
-    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')
@@ -63,19 +56,11 @@ def transform_submission_restrictions_from_2_1_to_2_0
 
     def transform_file_restrictions_from_2_1_to_2_0
       # when only one file-restriction is present, dachsfisch does not create an array
-      if @task.submission_restrictions['submission-restrictions']['file-restriction'].is_a? Hash
-        replace_use_with_required_in_file_restriction @task.submission_restrictions['submission-restrictions']['file-restriction']
-      else
-        @task.submission_restrictions['submission-restrictions']['file-restriction'].each do |file_restriction|
-          replace_use_with_required_in_file_restriction file_restriction
-        end
+      [@task.submission_restrictions['submission-restrictions']['file-restriction']].flatten.each do |file_restriction|
+        file_restriction['@required'] = (file_restriction['@use'].nil? || file_restriction.delete('@use') == 'required').to_s
       end
     end
 
-    def replace_use_with_required_in_file_restriction(file_restriction)
-      file_restriction['@required'] = (file_restriction['@use'].nil? || file_restriction.delete('@use') == 'required').to_s
-    end
-
     def add_model_solution_placeholder
       return if @task.model_solutions&.any?
 

diff --git a/spec/factories/task.rb b/spec/factories/task.rb
@@ -74,6 +74,7 @@
         }
       end
     end
+
     trait(:with_submission_restrictions_with_single_file_restriction) do
       submission_restrictions do
         {
@@ -88,6 +89,25 @@
         }
       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
         {

diff --git a/spec/proformaxml/transform_task_spec.rb b/spec/proformaxml/transform_task_spec.rb
@@ -56,6 +56,18 @@
           .to({'$1' => 'restriction1', '@@order' => ['$1'], '@pattern-format' => 'none', '@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) }