Merge pull request #7967 from colinux/improve-estimated-fill-duration

feat: improve estimated fill duration with text reading time

app/models/procedure_revision.rb

@@ -203,11 +203,14 @@ class ProcedureRevision < ApplicationRecord
   private
 
   def compute_estimated_fill_duration
-    tdc_durations = types_de_champ_public.fillable.map do |tdc|
+    types_de_champ_public.sum do |tdc|
-      duration = tdc.estimated_fill_duration(self)
+      next tdc.estimated_read_duration unless tdc.fillable?
-      tdc.mandatory ? duration : duration / 2
+
+      duration = tdc.estimated_read_duration + tdc.estimated_fill_duration(self)
+      duration /= 2 unless tdc.mandatory?
+
+      duration
     end
-    tdc_durations.sum
   end
 
   def children_types_de_champ_as_json(tdcs_as_json, parent_tdcs)

app/models/type_de_champ.rb

@@ -113,7 +113,7 @@ class TypeDeChamp < ApplicationRecord
   has_one :revision, through: :revision_type_de_champ
   has_one :procedure, through: :revision
 
-  delegate :estimated_fill_duration, :tags_for_template, :libelle_for_export, to: :dynamic_type
+  delegate :estimated_fill_duration, :estimated_read_duration, :tags_for_template, :libelle_for_export, to: :dynamic_type
 
   class WithIndifferentAccess
     def self.load(options)

app/models/types_de_champ/repetition_type_de_champ.rb

@@ -8,11 +8,14 @@ class TypesDeChamp::RepetitionTypeDeChamp < TypesDeChamp::TypeDeChampBase
 
   def estimated_fill_duration(revision)
     estimated_rows_in_repetition = 2.5
-    estimated_row_duration = revision
+
-      .children_of(@type_de_champ)
+    children = revision.children_of(@type_de_champ)
-      .map { |child_tdc| child_tdc.estimated_fill_duration(revision) }
+
-      .sum
+    estimated_row_duration = children.map { _1.estimated_fill_duration(revision) }.sum
-    estimated_row_duration * estimated_rows_in_repetition
+    estimated_children_read_duration = children.map(&:estimated_read_duration).sum
+
+    # Count only once children read time for all rows
+    estimated_row_duration * estimated_rows_in_repetition + estimated_children_read_duration
   end
 
   # We have to truncate the label here as spreadsheets have a (30 char) limit on length.

app/models/types_de_champ/type_de_champ_base.rb

@@ -1,11 +1,12 @@
 class TypesDeChamp::TypeDeChampBase
   include ActiveModel::Validations
 
-  delegate :description, :libelle, :mandatory, :stable_id, to: :@type_de_champ
+  delegate :description, :libelle, :mandatory, :stable_id, :fillable?, to: :@type_de_champ
 
-  FILL_DURATION_SHORT  = 10.seconds.in_seconds
+  FILL_DURATION_SHORT  = 10.seconds
-  FILL_DURATION_MEDIUM = 1.minute.in_seconds
+  FILL_DURATION_MEDIUM = 1.minute
-  FILL_DURATION_LONG   = 3.minutes.in_seconds
+  FILL_DURATION_LONG   = 3.minutes
+  READ_WORDS_PER_SECOND = 140.0 / 60 # 140 words per minute
 
   def initialize(type_de_champ)
     @type_de_champ = type_de_champ
@@ -32,7 +33,22 @@ class TypesDeChamp::TypeDeChampBase
   # Default estimated duration to fill the champ in a form, in seconds.
   # May be overridden by subclasses.
   def estimated_fill_duration(revision)
-    FILL_DURATION_SHORT
+    if fillable?
+      FILL_DURATION_SHORT
+    else
+      0.seconds
+    end
+  end
+
+  def estimated_read_duration
+    return 0.seconds if description.blank?
+
+    sanitizer = Rails::Html::Sanitizer.full_sanitizer.new
+    content = sanitizer.sanitize(description)
+
+    words = content.split(/\s+/).size
+
+    (words / READ_WORDS_PER_SECOND).round.seconds
   end
 
   def build_champ(params)

spec/helpers/procedure_helper_spec.rb

@@ -11,10 +11,10 @@ RSpec.describe ProcedureHelper, type: :helper do
   end
 
   describe '#estimated_fill_duration_minutes' do
-    subject { estimated_fill_duration_minutes(procedure) }
+    subject { estimated_fill_duration_minutes(procedure.reload) }
 
     context 'with champs' do
-      let(:procedure) { build(:procedure, :with_yes_no, :with_piece_justificative) }
+      let(:procedure) { create(:procedure, :with_yes_no, :with_piece_justificative) }
 
       it 'rounds up the duration to the minute' do
         expect(subject).to eq(2)
@@ -22,7 +22,7 @@ RSpec.describe ProcedureHelper, type: :helper do
     end
 
     context 'without champs' do
-      let(:procedure) { build(:procedure) }
+      let(:procedure) { create(:procedure) }
 
       it 'never displays ‘zero minutes’' do
         expect(subject).to eq(1)

spec/models/procedure_revision_spec.rb

@@ -661,11 +661,14 @@ describe ProcedureRevision do
 
   describe '#estimated_fill_duration' do
     let(:mandatory) { true }
+    let(:description) { nil }
+    let(:description_read_time) { ((description || "").split.size / TypesDeChamp::TypeDeChampBase::READ_WORDS_PER_SECOND).round }
+
     let(:types_de_champ_public) do
       [
-        { mandatory: true },
+        { mandatory: true, description: },
-        { type: :siret, mandatory: true },
+        { type: :siret, mandatory: true, description: },
-        { type: :piece_justificative, mandatory: mandatory }
+        { type: :piece_justificative, mandatory:, description: }
       ]
     end
     let(:procedure) { create(:procedure, types_de_champ_public: types_de_champ_public) }
@@ -676,7 +679,8 @@ describe ProcedureRevision do
       expect(subject).to eq \
           TypesDeChamp::TypeDeChampBase::FILL_DURATION_SHORT \
         + TypesDeChamp::TypeDeChampBase::FILL_DURATION_MEDIUM \
-        + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG
+        + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG \
+        + 3 * description_read_time
     end
 
     context 'when some champs are optional' do
@@ -684,9 +688,22 @@ describe ProcedureRevision do
 
       it 'estimates that half of optional champs will be filled' do
         expect(subject).to eq \
-             TypesDeChamp::TypeDeChampBase::FILL_DURATION_SHORT \
+            TypesDeChamp::TypeDeChampBase::FILL_DURATION_SHORT \
           + TypesDeChamp::TypeDeChampBase::FILL_DURATION_MEDIUM \
-          + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG / 2
+          + 2 * description_read_time \
+          + (description_read_time + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG) / 2
+      end
+    end
+
+    context 'when some champs have a description' do
+      let(:description) { "some four words description" }
+
+      it 'estimates that duration includes description reading time' do
+        expect(subject).to eq \
+            TypesDeChamp::TypeDeChampBase::FILL_DURATION_SHORT \
+          + TypesDeChamp::TypeDeChampBase::FILL_DURATION_MEDIUM \
+          + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG \
+          + 3 * description_read_time
       end
     end
 
@@ -696,17 +713,38 @@ describe ProcedureRevision do
           {
             type: :repetition,
             mandatory: true,
+            description:,
             children: [
-              { mandatory: true },
+              { mandatory: true, description: "word " * 10 },
-              { type: :piece_justificative, position: 2, mandatory: true }
+              { type: :piece_justificative, position: 2, mandatory: true, description: nil }
             ]
           }
         ]
       end
 
       it 'estimates that between 2 and 3 rows will be filled for each repetition' do
+        repetable_block_read_duration = description_read_time
+
         row_duration = TypesDeChamp::TypeDeChampBase::FILL_DURATION_SHORT + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG
-        expect(subject).to eq row_duration * 2.5
+        children_read_duration = (10 / TypesDeChamp::TypeDeChampBase::READ_WORDS_PER_SECOND).round
+
+        expect(subject).to eq repetable_block_read_duration + row_duration * 2.5 + children_read_duration
+      end
+    end
+
+    context 'when there are non fillable champs' do
+      let(:types_de_champ_public) do
+        [
+          {
+            type: :explication,
+            description: "5 words description <strong>containing html</strong> " * 20
+          },
+          { mandatory: true, description: nil }
+        ]
+      end
+
+      it 'estimates duration based on content reading' do
+        expect(subject).to eq((100 / TypesDeChamp::TypeDeChampBase::READ_WORDS_PER_SECOND).round + TypesDeChamp::TypeDeChampBase::FILL_DURATION_SHORT)
       end
     end
 
@@ -722,6 +760,7 @@ describe ProcedureRevision do
         before do
           draft_revision.estimated_fill_duration
           draft_revision.types_de_champ.first.update!(type_champ: TypeDeChamp.type_champs.fetch(:piece_justificative))
+          draft_revision.reload
         end
 
         it 'returns an up-to-date estimate' do
@@ -729,6 +768,7 @@ describe ProcedureRevision do
               TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG \
             + TypesDeChamp::TypeDeChampBase::FILL_DURATION_MEDIUM \
             + TypesDeChamp::TypeDeChampBase::FILL_DURATION_LONG \
+            + 3 * description_read_time
         end
       end