fix set_specific_max_scores and each_form_with_document

patodevilla · patodevilla · commit 729438a16741 · 2023-03-20T12:57:55.000-06:00
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -8,7 +8,7 @@ GIT
 PATH
   remote: .
   specs:
-    json_schema_form (0.10.0)
+    json_schema_form (0.10.1)
       activesupport (~> 6)
       dry-schema (~> 1.10)
       json_schemer (= 0.2.17)
diff --git a/lib/jsf/forms/field/concerns/base.rb b/lib/jsf/forms/field/concerns/base.rb
@@ -98,7 +98,6 @@ def valid_for_locale?(locale = DEFAULT_LOCALE)
           #
           # @return [Boolean]
           def scored?
-            # raise NoMethodError.new('this field does not implement scored?')
             false
           end
 
diff --git a/lib/jsf/forms/form.rb b/lib/jsf/forms/form.rb
@@ -473,15 +473,24 @@ def properties
         self[:properties]
       end
     
+      # TODO: deprecate, use reduce_each_form
+      #
       # get own and dynamic properties
       #
       # @return [Hash{String => JSF::Forms::Field::*}]
       def merged_properties(**args)
-        properties = {}
+        reduce_each_form({}, **args) do |acum, form|
+          acum.merge!(form&.properties || {})
+        end
+      end
+
+      # similar to Array.reduce
+      #
+      def reduce_each_form(init_value, **args)
         each_form(**args) do |form|
-          properties.merge!(form&.properties || {})
+          init_value = yield(init_value, form)
         end
-        properties
+        init_value
       end
     
       # gets the property definition inside the properties key
@@ -819,8 +828,6 @@ def each_form(
       #   for each value in the document's array. If you want forms to only be yielded once,
       #   use each_form
       #
-      # @param skip_property_proc [Proc] skips a property if it returns true when called
-      #
       # @yieldparam [JSF::Forms::Form] form
       # @yieldparam [JSF::Forms::Condition] condition
       # @yieldparam [Proc] skip_branch_proc
@@ -829,7 +836,7 @@ def each_form(
       # @yieldparam [Array<String,Integer>] document_path
       #
       # @return [void]
-      def each_form_with_document(document, section_or_shared: nil, document_path: [], skip_property_proc: nil, skip_on_condition: false, condition_proc: nil, **kwargs, &block)
+      def each_form_with_document(document, section_or_shared: nil, document_path: [], skip_on_condition: false, condition_proc: nil, **kwargs, &block)
         empty_document = {}
 
         # since JSF::Forms::Field::Shared and JSF::Forms::Section are already
@@ -852,38 +859,37 @@ def each_form_with_document(document, section_or_shared: nil, document_path: [],
             section_or_shared
           )
 
-          # handle all properties that have a value that is a hash or an array because the document_path is modified
+          # handle all properties that have a value in which the document_path is modified (sections, shared)
           form[:properties].each do |key, property|
             next if kwargs[:skip_tree_when_hidden] && !property.visible(is_create: kwargs[:is_create])
-            next if skip_property_proc&.call(key, property)
-            
+
             # go recursive
-            case property
-            when JSF::Forms::Section
-              value = document[key]
+            if !kwargs[:ignore_sections] && property.is_a?(JSF::Forms::Section)
+              if !property.repeatable?
+                document[key] ||= []
+                document[key].push({}) if document[key].empty?
+              end
               empty_document[key] ||= []
-              value&.map&.with_index do |doc, i|
+              document[key]&.map&.with_index do |doc, i|
                 empty_document[key][i] = property
                   .form
                   .each_form_with_document(
                     doc,
                     document_path: document_path + [key, i],
-                    skip_property_proc: skip_property_proc,
                     skip_on_condition: skip_on_condition,
                     section_or_shared: property,
                     condition_proc: condition_proc,
                     **kwargs,
                     &block
                   )
               end
-            when JSF::Forms::Field::Shared
+            elsif !kwargs[:ignore_definitions] && property.is_a?(JSF::Forms::Field::Shared)
               value = document[key] || {}
               empty_document[key] = property
                 .shared_definition
                 .each_form_with_document(
                   value,
                   document_path: (document_path + [key]),
-                  skip_property_proc: skip_property_proc,
                   skip_on_condition: skip_on_condition,
                   section_or_shared: property,
                   condition_proc: condition_proc,
@@ -1015,7 +1021,7 @@ def set_specific_max_scores!(document, is_create: false, condition_proc: nil, **
           # iterate properties
           form[:properties].each do |k, prop|
             next unless prop.visible(is_create: is_create)
-            next unless JSF::Forms::Form::SCORABLE_FIELDS.include?(prop.class)
+            next unless (JSF::Forms::Form::SCORABLE_FIELDS.include?(prop.class) && prop.scored?)
 
             value = current_doc.dig(k)
 
@@ -1024,15 +1030,16 @@ def set_specific_max_scores!(document, is_create: false, condition_proc: nil, **
             else
 
               # check for any visible child field
-              visible_children = prop.dependent_conditions.any? do |cond|
+              visible_scorable_children = prop.dependent_conditions.any? do |cond|
                 next unless cond.evaluate(current_doc, &condition_proc)
 
-                cond.dig(:then, :properties)&.any? do |k,v|
-                  v.visible(is_create: is_create)
-                end
+                cond[:then]&.scored_with_document?(
+                  current_doc,
+                  is_create: is_create
+                ) # TODO: need to pass more arguments?
               end
         
-              if visible_children
+              if visible_scorable_children
                 prop.score_for_value(value)
               else
                 prop.max_score
@@ -1140,6 +1147,19 @@ def scored?(**args)
         has_scoring
       end
 
+      # @return [Boolean]
+      def scored_with_document?(document, is_create: false, **kwargs)
+        each_form_with_document(document) do |form, condition, skip_branch_proc, current_level, current_doc, current_empty_doc, document_path, section_or_shared|
+          form[:properties].each do |key, property|
+            next unless property.visible(is_create: is_create)
+            next if property.is_a?(JSF::Forms::Section)
+            
+            return true if property.scored?
+          end
+        end
+        false
+      end
+
       # Initial value when scoring a document
       #
       # @return [Float, NilClass]
diff --git a/lib/jsf/version.rb b/lib/jsf/version.rb
@@ -1,3 +1,3 @@
 module JSF
-  VERSION = "0.10.0"
+  VERSION = "0.10.1"
 end
diff --git a/test/spec/forms/form_spec.rb b/test/spec/forms/form_spec.rb
@@ -998,84 +998,120 @@ def test_set_specific_max_scores!
       end
     end
 
+    form = JSF::Forms::FormBuilder.build do
+      add_response_set(:response_set_1, example('response_set')).tap do |response_set|
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'score_0'
+          r[:score] = 0
+          r.set_translation('score 0', 'es')
+        end
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'score_1'
+          r[:score] = 1
+          r.set_translation('score 1', 'es')
+        end
+      end
+
+      append_property(:select_1, example('select')) do |f, key|
+        f.response_set_id = :response_set_1
+        append_conditional_property(:markdown_1_1, example('markdown'), dependent_on: key, type: :enum, value: ['score_0'])
+      end
+    end
+
+    [
+      {
+        doc: {'select_1' => nil},
+        hash: {"select_1"=>1},
+        total: 1,
+      },
+      {
+        doc: {'select_1' => 'score_1'},
+        hash: {"select_1"=>1},
+        total: 1,
+      },
+      {
+        doc: {'select_1' => 'score_0'},
+        hash: {"select_1"=>1},
+        total: 1,
+      }
+    ].each do |obj|
+      assert_equal obj[:total], form.set_specific_max_scores!(obj[:doc])
+      assert_equal obj[:doc]['meta']['specific_max_score_hash'], obj[:hash]
+    end
+
     ### SCORABLE FIELDS ###
 
-    scored_form_proc = Proc.new do
-      JSF::Forms::FormBuilder.build do
+    form = JSF::Forms::FormBuilder.build do
 
-        add_response_set(:response_set_1, example('response_set')).tap do |response_set|
-          response_set.add_response(example('response', :is_inspection)).tap do |r|
-            r[:const] = 'nil_score'
-            r[:score] = nil
-          end
-          response_set.add_response(example('response', :is_inspection)).tap do |r|
-            r[:const] = 'score_3'
-            r[:score] = 3
-          end
-          response_set.add_response(example('response', :is_inspection)).tap do |r|
-            r[:const] = 'score_6'
-            r[:score] = 6
-          end
+      add_response_set(:response_set_1, example('response_set')).tap do |response_set|
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'nil_score'
+          r[:score] = nil
         end
-  
-        add_response_set(:response_set_2, example('response_set')).tap do |response_set|
-          response_set.add_response(example('response', :is_inspection)).tap do |r|
-            r[:const] = 'nil_score'
-            r[:score] = nil
-          end
-          response_set.add_response(example('response', :is_inspection)).tap do |r|
-            r[:const] = 'score_1'
-            r[:score] = 1
-          end
-          response_set.add_response(example('response', :is_inspection)).tap do |r|
-            r[:const] = 'score_2'
-            r[:score] = 2
-          end
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'score_3'
+          r[:score] = 3
+        end
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'score_6'
+          r[:score] = 6
         end
+      end
+
+      add_response_set(:response_set_2, example('response_set')).tap do |response_set|
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'nil_score'
+          r[:score] = nil
+        end
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'score_1'
+          r[:score] = 1
+        end
+        response_set.add_response(example('response', :is_inspection)).tap do |r|
+          r[:const] = 'score_2'
+          r[:score] = 2
+        end
+      end
+
+      append_property(:markdown_1, example('markdown'))
+      append_property(:switch_1, example('switch'))
+
+      append_property(:select_1, example('select')) do |f, key|
+        f.response_set_id = :response_set_1
+
+        append_conditional_property(:select_1_1, example('select'), dependent_on: key, type: :enum, value: ['nil_score', 'score_3']) do |f, key, subform|
+          f.response_set_id = :response_set_2
   
-        append_property(:markdown_1, example('markdown'))
-        append_property(:switch_1, example('switch'))
+          subform.append_conditional_property(:select_1_1_1, example('select'), dependent_on: key, type: :const, value: 'nil_score') do |f, key, subform|
+            f.response_set_id = :response_set_1
+          end
   
-        append_property(:select_1, example('select')) do |f, key|
-          f.response_set_id = :response_set_1
-
-          append_conditional_property(:select_1_1, example('select'), dependent_on: key, type: :enum, value: ['nil_score', 'score_3']) do |f, key, subform|
+          subform.append_conditional_property(:checkbox_1_1_2, example('checkbox'), dependent_on: key, type: :enum, value: ['nil_score', 'score_1']) do |f, key, subform|
             f.response_set_id = :response_set_2
-    
-            subform.append_conditional_property(:select_1_1_1, example('select'), dependent_on: key, type: :const, value: 'nil_score') do |f, key, subform|
-              f.response_set_id = :response_set_1
-            end
-    
-            subform.append_conditional_property(:checkbox_1_1_2, example('checkbox'), dependent_on: key, type: :enum, value: ['nil_score', 'score_1']) do |f, key, subform|
-              f.response_set_id = :response_set_2
-            end
           end
-
         end
 
-        append_property(:section_1, example('section')) do |f, _|
-          f.form.instance_eval do
-            append_property(:slider_sec_1, example('slider'))
-            append_property(:number_input_sec_2, example('number_input')) do |f, key|
-              append_conditional_property(:select_sec_1_1, example('select'), dependent_on: key, type: :const, value: 1) do |f, key, subform|
-                f.response_set_id = :response_set_2
-  
-                subform.append_conditional_property(:switch_1_1_1, example('switch'), dependent_on: key, type: :const, value: 'score_2')
-                subform.append_conditional_property(:section_1_1_1, example('section'), dependent_on: key, type: :const, value: 'score_1') do |f, _|
-                  f.form.instance_eval do
-                    append_property(:switch_1_1_1_1, example('switch'))
-                  end
+      end
+
+      append_property(:section_1, example('section')) do |f, _|
+        f.form.instance_eval do
+          append_property(:slider_sec_1, example('slider'))
+          append_property(:number_input_sec_2, example('number_input')) do |f, key|
+            append_conditional_property(:select_sec_1_1, example('select'), dependent_on: key, type: :const, value: 1) do |f, key, subform|
+              f.response_set_id = :response_set_2
+
+              subform.append_conditional_property(:switch_1_1_1, example('switch'), dependent_on: key, type: :const, value: 'score_2')
+              subform.append_conditional_property(:section_1_1_1, example('section'), dependent_on: key, type: :const, value: 'score_1') do |f, _|
+                f.form.instance_eval do
+                  append_property(:switch_1_1_1_1, example('switch'))
                 end
               end
             end
           end
         end
-
       end
-    end
 
-    # test multiple documents for a form with no hidden fields
-    form = scored_form_proc.call
+    end
 
     values = [
       {
@@ -1150,8 +1186,8 @@ def test_set_specific_max_scores!
       },
       {
         doc: {'select_1' => 'nil_score', 'select_1_1' => 'nil_score', 'section_1' => [{'number_input_sec_2' => 1}, {'number_input_sec_2' => 1, 'select_sec_1_1' => 'score_1'}] },
-        hash: {"switch_1"=>1, "select_1"=>nil, "section_1"=>[{"slider_sec_1"=>10, "select_sec_1_1"=>2}, {"slider_sec_1"=>10, "select_sec_1_1"=>1, "section_1_1_1"=>[]}], "select_1_1"=>nil, "select_1_1_1"=>6, "checkbox_1_1_2"=>3},
-        total: 33.0,
+        hash: {"switch_1"=>1, "select_1"=>nil, "section_1"=>[{"slider_sec_1"=>10, "select_sec_1_1"=>2}, {"slider_sec_1"=>10, "select_sec_1_1"=>2, "section_1_1_1"=>[]}], "select_1_1"=>nil, "select_1_1_1"=>6, "checkbox_1_1_2"=>3},
+        total: 34.0,
       },
       {
         doc: {'select_1' => 'nil_score', 'select_1_1' => 'nil_score', 'section_1' => [{'number_input_sec_2' => 1}, {'number_input_sec_2' => 1, 'select_sec_1_1' => 'score_2'}] },

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,6 @@ def valid_for_locale?(locale = DEFAULT_LOCALE)`
`98`	`98`	`#`
`99`	`99`	`# @return [Boolean]`
`100`	`100`	`def scored?`
`101`		`- # raise NoMethodError.new('this field does not implement scored?')`
`102`	`101`	`false`
`103`	`102`	`end`
`104`	`103`