From 010c9a0dcbe14fb5ffadf7602701d9030106e0e7 Mon Sep 17 00:00:00 2001
From: Colin Darie <colin@darie.eu>
Date: Sat, 29 Oct 2022 12:43:37 +0200
Subject: [PATCH] feat(procedure): includes read duration of explication/non
 fillable champs

Long explications can significantly increase global fill duration,
so we can't ignore them.

Closes #7963
---
 app/models/procedure_revision.rb                | 11 +++++++----
 app/models/types_de_champ/type_de_champ_base.rb | 10 ++++++++--
 spec/helpers/procedure_helper_spec.rb           |  6 +++---
 spec/models/procedure_revision_spec.rb          | 14 ++++++++++++++
 4 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb
index 6f95fa0e3..ca5b85733 100644
--- a/app/models/procedure_revision.rb
+++ b/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|
-      duration = tdc.estimated_fill_duration(self) + tdc.estimated_read_duration
-      tdc.mandatory ? duration : duration / 2
+    types_de_champ_public.sum do |tdc|
+      next tdc.estimated_read_duration unless tdc.fillable?
+
+      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)
diff --git a/app/models/types_de_champ/type_de_champ_base.rb b/app/models/types_de_champ/type_de_champ_base.rb
index 73a7e98f3..d21014c55 100644
--- a/app/models/types_de_champ/type_de_champ_base.rb
+++ b/app/models/types_de_champ/type_de_champ_base.rb
@@ -1,8 +1,9 @@
 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
   FILL_DURATION_MEDIUM = 1.minute.in_seconds
   FILL_DURATION_LONG   = 3.minutes.in_seconds
   READ_WORDS_PER_SECOND = 140.0 / 60 # 140 words per minute
@@ -32,7 +33,12 @@ 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?
diff --git a/spec/helpers/procedure_helper_spec.rb b/spec/helpers/procedure_helper_spec.rb
index f8dfbc9e2..6a2a92020 100644
--- a/spec/helpers/procedure_helper_spec.rb
+++ b/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)
diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb
index f7e1b4720..3867106d7 100644
--- a/spec/models/procedure_revision_spec.rb
+++ b/spec/models/procedure_revision_spec.rb
@@ -731,6 +731,20 @@ describe ProcedureRevision do
         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