From e64ac79f05b502ba954db219121dcd38c47f976a Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 14 Apr 2023 10:55:36 +0200 Subject: [PATCH] tech(refactor): much nicer code, thx LeSim root -> rooted_tree depth_cach -> walk smal refactor comment remove form for header_section remove seen_at from header section header_section: champ -> header_section champ_subree: remove if not remove root_depth use header_section_level_value instead remove unused include remove ChampTreeComponent rename ChampsSubtreeComponent to SectionComponent use TreeableConcern only in section component remove fields_for_champ_component champs -> tail add split_section_champ helper refactor(editable_champ::header_section): keep same interface everywhere fix(repetition): add spec for SectionComponent on repetitions --- .../champs_subtree_component.rb | 47 -------------- .../champs_subtree_component.html.haml | 17 ----- .../editable_champ/champs_tree_component.rb | 7 -- .../champs_tree_component.html.haml | 1 - .../fields_for_champ_component.rb | 5 -- .../fields_for_champ_component.html.haml | 2 - .../header_section_component.rb | 7 +- .../repetition_row_component.html.haml | 4 +- .../editable_champ/section_component.rb | 64 +++++++++++++++++++ .../section_component.html.haml | 19 ++++++ .../types_de_champ_controller.rb | 2 +- app/models/concerns/treeable_concern.rb | 28 ++++---- app/views/shared/dossiers/_edit.html.haml | 2 +- .../dossiers/_edit_annotations.html.haml | 2 +- ...nent_spec.rb => section_component_spec.rb} | 35 ++++++++-- spec/models/concern/treeable_concern_spec.rb | 6 +- 16 files changed, 139 insertions(+), 109 deletions(-) delete mode 100644 app/components/editable_champ/champs_subtree_component.rb delete mode 100644 app/components/editable_champ/champs_subtree_component/champs_subtree_component.html.haml delete mode 100644 app/components/editable_champ/champs_tree_component.rb delete mode 100644 app/components/editable_champ/champs_tree_component/champs_tree_component.html.haml delete mode 100644 app/components/editable_champ/fields_for_champ_component.rb delete mode 100644 app/components/editable_champ/fields_for_champ_component/fields_for_champ_component.html.haml create mode 100644 app/components/editable_champ/section_component.rb create mode 100644 app/components/editable_champ/section_component/section_component.html.haml rename spec/components/editable_champ/{champs_subtree_component_spec.rb => section_component_spec.rb} (72%) diff --git a/app/components/editable_champ/champs_subtree_component.rb b/app/components/editable_champ/champs_subtree_component.rb deleted file mode 100644 index 028aeee81..000000000 --- a/app/components/editable_champ/champs_subtree_component.rb +++ /dev/null @@ -1,47 +0,0 @@ -class EditableChamp::ChampsSubtreeComponent < ApplicationComponent - include ApplicationHelper - include TreeableConcern - - def initialize(nodes:) - @nodes = to_fieldset(nodes:) - end - - def render_within_fieldset? - first_champ_is_an_header_section? && any_champ_fillable? - end - - def header_section - first_champ = @nodes.first - return first_champ if first_champ.is_a?(Champs::HeaderSectionChamp) - nil - end - - def champs - return @nodes if !first_champ_is_an_header_section? - _, *rest_of_champ = @nodes - - rest_of_champ - end - - def tag_for_depth - "h#{header_section.level + 1}" - end - - def fillable? - false - end - - private - - def to_fieldset(nodes:) - nodes.map { _1.is_a?(Array) ? EditableChamp::ChampsSubtreeComponent.new(nodes: _1) : _1 } - end - - def first_champ_is_an_header_section? - header_section.present? - end - - def any_champ_fillable? - champs.any? { _1&.fillable? } - end -end diff --git a/app/components/editable_champ/champs_subtree_component/champs_subtree_component.html.haml b/app/components/editable_champ/champs_subtree_component/champs_subtree_component.html.haml deleted file mode 100644 index d935cc765..000000000 --- a/app/components/editable_champ/champs_subtree_component/champs_subtree_component.html.haml +++ /dev/null @@ -1,17 +0,0 @@ -- if render_within_fieldset? - = tag.fieldset(class: "reset-#{tag_for_depth}") do - = tag.legend do - = render EditableChamp::HeaderSectionComponent.new(champ: header_section, form: nil) - - champs.each do |champ_or_section| - - if !champ_or_section.is_a?(EditableChamp::ChampsSubtreeComponent) - = render EditableChamp::FieldsForChampComponent.new(champ: champ_or_section, seen_at: nil) - - else - = render champ_or_section -- else - - if header_section - = render EditableChamp::HeaderSectionComponent.new(champ: header_section, form: nil) - - champs.each do |champ_or_section| - - if !champ_or_section.is_a?(EditableChamp::ChampsSubtreeComponent) - = render EditableChamp::FieldsForChampComponent.new(champ: champ_or_section, seen_at: nil) - - else - = render champ_or_section diff --git a/app/components/editable_champ/champs_tree_component.rb b/app/components/editable_champ/champs_tree_component.rb deleted file mode 100644 index f9677006e..000000000 --- a/app/components/editable_champ/champs_tree_component.rb +++ /dev/null @@ -1,7 +0,0 @@ -class EditableChamp::ChampsTreeComponent < ApplicationComponent - include TreeableConcern - - def initialize(champs:, root_depth:) - @tree = to_tree(champs:, root_depth:) - end -end diff --git a/app/components/editable_champ/champs_tree_component/champs_tree_component.html.haml b/app/components/editable_champ/champs_tree_component/champs_tree_component.html.haml deleted file mode 100644 index 6e1f1d58b..000000000 --- a/app/components/editable_champ/champs_tree_component/champs_tree_component.html.haml +++ /dev/null @@ -1 +0,0 @@ -= render EditableChamp::ChampsSubtreeComponent.new(nodes: @tree) diff --git a/app/components/editable_champ/fields_for_champ_component.rb b/app/components/editable_champ/fields_for_champ_component.rb deleted file mode 100644 index 86cf1aea3..000000000 --- a/app/components/editable_champ/fields_for_champ_component.rb +++ /dev/null @@ -1,5 +0,0 @@ -class EditableChamp::FieldsForChampComponent < ApplicationComponent - def initialize(champ:, seen_at: nil) - @champ, @seen_at = champ, seen_at - end -end diff --git a/app/components/editable_champ/fields_for_champ_component/fields_for_champ_component.html.haml b/app/components/editable_champ/fields_for_champ_component/fields_for_champ_component.html.haml deleted file mode 100644 index 97b59eff8..000000000 --- a/app/components/editable_champ/fields_for_champ_component/fields_for_champ_component.html.haml +++ /dev/null @@ -1,2 +0,0 @@ -= fields_for @champ.input_name, @champ do |form| - = render EditableChamp::EditableChampComponent.new form: form, champ: @champ, seen_at: @seen_at diff --git a/app/components/editable_champ/header_section_component.rb b/app/components/editable_champ/header_section_component.rb index 212320bc5..253e32ff0 100644 --- a/app/components/editable_champ/header_section_component.rb +++ b/app/components/editable_champ/header_section_component.rb @@ -1,7 +1,6 @@ class EditableChamp::HeaderSectionComponent < ApplicationComponent - def initialize(form:, champ:, seen_at: nil) + def initialize(form: nil, champ:, seen_at: nil) @champ = champ - @form = form end def level @@ -13,9 +12,9 @@ class EditableChamp::HeaderSectionComponent < ApplicationComponent end def header_section_classnames - class_names = ["fr-h#{level}", 'header-section'] + class_names = ["fr-h#{level}"] - class_names << 'header-section-counter' if @champ.dossier.auto_numbering_section_headers_for?(@champ) + class_names << 'header-section' if @champ.dossier.auto_numbering_section_headers_for?(@champ) class_names end diff --git a/app/components/editable_champ/repetition_row_component/repetition_row_component.html.haml b/app/components/editable_champ/repetition_row_component/repetition_row_component.html.haml index 24068e02f..addb1c152 100644 --- a/app/components/editable_champ/repetition_row_component/repetition_row_component.html.haml +++ b/app/components/editable_champ/repetition_row_component/repetition_row_component.html.haml @@ -3,9 +3,9 @@ - if @row.size > 1 %fieldset %legend.block-id= "#{@champ.libelle} " - = render EditableChamp::ChampsTreeComponent.new(champs: @row, root_depth: @champ.current_section_level) + = render EditableChamp::SectionComponent.new(champs: @row) - else - = render EditableChamp::ChampsTreeComponent.new(champs: @row, root_depth: @champ.current_section_level) + = render EditableChamp::SectionComponent.new(champs: @row) .flex.row-reverse{ 'data-turbo': 'true' } = render NestedForms::OwnedButtonComponent.new(formaction: champs_repetition_path(@champ.id, row_id: @row.first.row_id), http_method: :delete, opt: { class: "fr-btn fr-btn--sm fr-btn--tertiary fr-text-action-high--red-marianne", title: t(".delete_title", row_number: @champ.rows.find_index(@row))}) do diff --git a/app/components/editable_champ/section_component.rb b/app/components/editable_champ/section_component.rb new file mode 100644 index 000000000..c8ad567a1 --- /dev/null +++ b/app/components/editable_champ/section_component.rb @@ -0,0 +1,64 @@ +class EditableChamp::SectionComponent < ApplicationComponent + include ApplicationHelper + include TreeableConcern + + def initialize(nodes: nil, champs: nil) + if (nodes.nil?) + nodes = to_tree(champs:) + end + @nodes = to_fieldset(nodes:) + end + + def render_within_fieldset? + first_champ_is_an_header_section? && any_champ_fillable? + end + + def header_section + return @nodes.first if @nodes.first.is_a?(Champs::HeaderSectionChamp) + end + + def splitted_tail + tail.map { split_section_champ(_1) } + end + + def tail + return @nodes if !first_champ_is_an_header_section? + _, *rest_of_champ = @nodes + + rest_of_champ + end + + def tag_for_depth + "h#{header_section.level + 1}" + end + + # if two headers follows each others [h1, [h2, c]] + # the first one must not be contained in fieldset + # so we make the tree not fillable + def fillable? + false + end + + def split_section_champ(node) + case node + when EditableChamp::SectionComponent + [node, nil] + else + [nil, node] + end + end + + private + + def to_fieldset(nodes:) + nodes.map { _1.is_a?(Array) ? EditableChamp::SectionComponent.new(nodes: _1) : _1 } + end + + def first_champ_is_an_header_section? + header_section.present? + end + + def any_champ_fillable? + tail.any? { _1&.fillable? } + end +end diff --git a/app/components/editable_champ/section_component/section_component.html.haml b/app/components/editable_champ/section_component/section_component.html.haml new file mode 100644 index 000000000..b6ecc682c --- /dev/null +++ b/app/components/editable_champ/section_component/section_component.html.haml @@ -0,0 +1,19 @@ +- if render_within_fieldset? + = tag.fieldset(class: "reset-#{tag_for_depth}") do + = tag.legend do + = render EditableChamp::HeaderSectionComponent.new(champ: header_section) + - splitted_tail.each do |section, champ| + - if section.present? + = render section + - else + = fields_for champ.input_name, champ do |form| + = render EditableChamp::EditableChampComponent.new form: ,champ: +- else + - if header_section + = render EditableChamp::HeaderSectionComponent.new(champ: header_section) + - splitted_tail.each do |section, champ| + - if section.present? + = render section + - else + = fields_for champ.input_name, champ do |form| + = render EditableChamp::EditableChampComponent.new form: ,champ: diff --git a/app/controllers/administrateurs/types_de_champ_controller.rb b/app/controllers/administrateurs/types_de_champ_controller.rb index 48dbf9d72..c77310dbd 100644 --- a/app/controllers/administrateurs/types_de_champ_controller.rb +++ b/app/controllers/administrateurs/types_de_champ_controller.rb @@ -128,7 +128,7 @@ module Administrateurs :drop_down_secondary_description, :collapsible_explanation_enabled, :collapsible_explanation_text, - :header_section_level + :header_section_level, editable_options: [ :cadastres, :unesco, diff --git a/app/models/concerns/treeable_concern.rb b/app/models/concerns/treeable_concern.rb index 3f269e11e..81592eff7 100644 --- a/app/models/concerns/treeable_concern.rb +++ b/app/models/concerns/treeable_concern.rb @@ -6,32 +6,32 @@ module TreeableConcern included do # as we progress in the list of ordered champs - # we keep a reference to each level of nesting (depth_cache) + # we keep a reference to each level of nesting (walk) # when we encounter an header_section, it depends of its own depth of nesting minus 1, ie: - # h1 belongs to prior (root) + # h1 belongs to prior (rooted_tree) # h2 belongs to prior h1 # h3 belongs to prior h2 - # h1 belongs to prior (root) + # h1 belongs to prior (rooted_tree) # then, each and every champs which are not an header_section - # are added to the most_recent_subtree + # are added to the current_tree # given a root_depth at 0, we build a full tree # given a root_depth > 0, we build a partial tree (aka, a repetition) - def to_tree(champs:, root_depth:) - root = [] - depth_cache = Array.new(MAX_DEPTH) - depth_cache[root_depth] = root - most_recent_subtree = root + def to_tree(champs:) + rooted_tree = [] + walk = Array.new(MAX_DEPTH) + walk[0] = rooted_tree + current_tree = rooted_tree champs.each do |champ| if champ.header_section? - champs_subtree = [champ] - depth_cache[champ.level - 1].push(champs_subtree) - most_recent_subtree = depth_cache[champ.level] = champs_subtree + new_tree = [champ] + walk[champ.header_section_level_value - 1].push(new_tree) + current_tree = walk[champ.header_section_level_value] = new_tree else - most_recent_subtree.push(champ) + current_tree.push(champ) end end - root + rooted_tree end end end diff --git a/app/views/shared/dossiers/_edit.html.haml b/app/views/shared/dossiers/_edit.html.haml index e3a4563bc..647d9ba16 100644 --- a/app/views/shared/dossiers/_edit.html.haml +++ b/app/views/shared/dossiers/_edit.html.haml @@ -42,5 +42,5 @@ = f.select :groupe_instructeur_id, dossier.procedure.groupe_instructeurs.active.map { |gi| [gi.label, gi.id] }, { include_blank: dossier.brouillon? } - = render EditableChamp::ChampsTreeComponent.new(champs: dossier.champs_public, root_depth: 0) + = render EditableChamp::SectionComponent.new(champs: dossier.champs_public) = render Dossiers::EditFooterComponent.new(dossier: dossier, annotation: false) diff --git a/app/views/shared/dossiers/_edit_annotations.html.haml b/app/views/shared/dossiers/_edit_annotations.html.haml index 7f69a1390..5dfaefff0 100644 --- a/app/views/shared/dossiers/_edit_annotations.html.haml +++ b/app/views/shared/dossiers/_edit_annotations.html.haml @@ -3,7 +3,7 @@ %section.counter-start-header-section = render NestedForms::FormOwnerComponent.new = form_for dossier, url: annotations_instructeur_dossier_path(dossier.procedure, dossier), html: { class: 'form', multipart: true } do |f| - = render EditableChamp::ChampsTreeComponent.new(champs: dossier.champs_private, root_depth: 0) + = render EditableChamp::SectionComponent.new(champs: dossier.champs_private) = render Dossiers::EditFooterComponent.new(dossier: dossier, annotation: true) - else diff --git a/spec/components/editable_champ/champs_subtree_component_spec.rb b/spec/components/editable_champ/section_component_spec.rb similarity index 72% rename from spec/components/editable_champ/champs_subtree_component_spec.rb rename to spec/components/editable_champ/section_component_spec.rb index 792aa7140..511fa7898 100644 --- a/spec/components/editable_champ/champs_subtree_component_spec.rb +++ b/spec/components/editable_champ/section_component_spec.rb @@ -1,8 +1,6 @@ -describe EditableChamp::ChampsSubtreeComponent, type: :component do +describe EditableChamp::SectionComponent, type: :component do include TreeableConcern - let(:component) { described_class.new(nodes: nodes) } - let(:nodes) { to_tree(champs:, root_depth:) } - let(:root_depth) { 0 } + let(:component) { described_class.new(champs: champs) } before { render_inline(component).to_html } context 'list of champs without an header_section' do @@ -83,4 +81,33 @@ describe EditableChamp::ChampsSubtreeComponent, type: :component do expect(page).to have_selector("fieldset fieldset textarea", count: 1) end end + + context 'with repetition' do + let(:procedure) do + create(:procedure, types_de_champ_public: [ + { type: :header_section, header_section_level: 1 }, + { + type: :repetition, + libelle: 'repetition', + children: [ + { type: :header_section, header_section_level: 1, libelle: 'child_1' }, + { type: :text, libelle: 'child_2' } + ] + } + ]) + end + let(:dossier) { create(:dossier, :with_populated_champs, procedure: procedure) } + let(:champs) { dossier.champs_public } + + it 'render nested fieldsets, increase heading level for repetition header_section' do + expect(page).to have_selector("fieldset") + expect(page).to have_selector("legend h2") + expect(page).to have_selector("fieldset fieldset") + expect(page).to have_selector("fieldset fieldset legend h3") + end + + it 'contains as many text champ as repetition.rows' do + expect(page).to have_selector("fieldset fieldset input[type=text]", count: dossier.champs_public.find(&:repetition?).rows.size) + end + end end diff --git a/spec/models/concern/treeable_concern_spec.rb b/spec/models/concern/treeable_concern_spec.rb index 9887df4f3..d3f12d5d9 100644 --- a/spec/models/concern/treeable_concern_spec.rb +++ b/spec/models/concern/treeable_concern_spec.rb @@ -3,12 +3,12 @@ describe TreeableConcern do include TreeableConcern attr_reader :root - def initialize(champs:, root_depth:) - @root = to_tree(champs:, root_depth:) + def initialize(champs:) + @root = to_tree(champs:) end end - subject { ChampsToTree.new(champs: champs, root_depth: 0).root } + subject { ChampsToTree.new(champs: champs).root } describe "to_tree" do let(:header_1) { build(:champ_header_section_level_1) } let(:header_1_2) { build(:champ_header_section_level_2) }