From 7731c6ad64cd404de88c7b42c95b2b8c0ef4cdf8 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Thu, 27 Oct 2022 20:13:21 +0200 Subject: [PATCH 1/3] feat(procedure): estimate fill duration takes account of libelle & description read time Sometimes long description on types de champ significantly increases global fill duration. 140 words per minute is a good estimate of attentive off-voice read time. (This is independent of characters per word). Closes #7963 --- app/models/procedure_revision.rb | 2 +- app/models/type_de_champ.rb | 2 +- .../repetition_type_de_champ.rb | 13 +++--- .../types_de_champ/type_de_champ_base.rb | 12 ++++- spec/models/procedure_revision_spec.rb | 44 +++++++++++++++---- 5 files changed, 56 insertions(+), 17 deletions(-) diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index 46ebed130..6f95fa0e3 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -204,7 +204,7 @@ class ProcedureRevision < ApplicationRecord def compute_estimated_fill_duration tdc_durations = types_de_champ_public.fillable.map do |tdc| - duration = tdc.estimated_fill_duration(self) + duration = tdc.estimated_fill_duration(self) + tdc.estimated_read_duration tdc.mandatory ? duration : duration / 2 end tdc_durations.sum diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index a77dcf53b..c884f0eeb 100644 --- a/app/models/type_de_champ.rb +++ b/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) diff --git a/app/models/types_de_champ/repetition_type_de_champ.rb b/app/models/types_de_champ/repetition_type_de_champ.rb index 9af53de27..b456f17cb 100644 --- a/app/models/types_de_champ/repetition_type_de_champ.rb +++ b/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) - .map { |child_tdc| child_tdc.estimated_fill_duration(revision) } - .sum - estimated_row_duration * estimated_rows_in_repetition + + children = revision.children_of(@type_de_champ) + + estimated_row_duration = children.map { _1.estimated_fill_duration(revision) }.sum + 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. 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 a8edcd20a..73a7e98f3 100644 --- a/app/models/types_de_champ/type_de_champ_base.rb +++ b/app/models/types_de_champ/type_de_champ_base.rb @@ -3,9 +3,9 @@ class TypesDeChamp::TypeDeChampBase delegate :description, :libelle, :mandatory, :stable_id, to: :@type_de_champ - FILL_DURATION_SHORT = 10.seconds.in_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 def initialize(type_de_champ) @type_de_champ = type_de_champ @@ -33,6 +33,16 @@ class TypesDeChamp::TypeDeChampBase # May be overridden by subclasses. def estimated_fill_duration(revision) FILL_DURATION_SHORT + + 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) diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb index 610dfe1bd..f7e1b4720 100644 --- a/spec/models/procedure_revision_spec.rb +++ b/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 }, - { type: :siret, mandatory: true }, - { type: :piece_justificative, mandatory: mandatory } + { mandatory: true, description: }, + { type: :siret, mandatory: true, description: }, + { 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,24 @@ describe ProcedureRevision do { type: :repetition, mandatory: true, + description:, children: [ - { mandatory: true }, - { type: :piece_justificative, position: 2, mandatory: true } + { mandatory: true, description: "word " * 10 }, + { 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 end end @@ -722,6 +746,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 +754,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 From 010c9a0dcbe14fb5ffadf7602701d9030106e0e7 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 29 Oct 2022 12:43:37 +0200 Subject: [PATCH 2/3] 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 containing html " * 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 From 3a06e7e0b3886cb2c646a82352a6d2050fb04c4d Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 29 Oct 2022 12:45:12 +0200 Subject: [PATCH 3/3] refactor(tdc): convert fill duration in Duration objects --- app/models/types_de_champ/type_de_champ_base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 d21014c55..2e9c5b1e6 100644 --- a/app/models/types_de_champ/type_de_champ_base.rb +++ b/app/models/types_de_champ/type_de_champ_base.rb @@ -4,8 +4,8 @@ class TypesDeChamp::TypeDeChampBase 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 + FILL_DURATION_MEDIUM = 1.minute + FILL_DURATION_LONG = 3.minutes READ_WORDS_PER_SECOND = 140.0 / 60 # 140 words per minute def initialize(type_de_champ)