Merge pull request #5278 from tchak/clean-up-options

Store drop_down_list values as jsonb

app/models/champ.rb

@@ -11,7 +11,22 @@ class Champ < ApplicationRecord
   belongs_to :etablissement, dependent: :destroy
   has_many :champs, -> { ordered }, foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy
 
-  delegate :libelle, :type_champ, :procedure, :order_place, :mandatory?, :description, :drop_down_list, :exclude_from_export?, :exclude_from_view?, :repetition?, :dossier_link?, to: :type_de_champ
+  delegate :libelle,
+    :type_champ,
+    :procedure,
+    :order_place,
+    :mandatory?,
+    :description,
+    :drop_down_list,
+    :drop_down_list_options,
+    :drop_down_list_options?,
+    :drop_down_list_disabled_options,
+    :drop_down_list_enabled_non_empty_options,
+    :exclude_from_export?,
+    :exclude_from_view?,
+    :repetition?,
+    :dossier_link?,
+    to: :type_de_champ
 
   scope :updated_since?, -> (date) { where('champs.updated_at > ?', date) }
   scope :public_only, -> { where(private: false) }

app/models/champs/drop_down_list_champ.rb

@@ -2,6 +2,22 @@ class Champs::DropDownListChamp < Champ
   THRESHOLD_NB_OPTIONS_AS_RADIO = 5
 
   def render_as_radios?
-    drop_down_list.enabled_non_empty_options.size <= THRESHOLD_NB_OPTIONS_AS_RADIO
+    enabled_non_empty_options.size <= THRESHOLD_NB_OPTIONS_AS_RADIO
+  end
+
+  def options?
+    drop_down_list_options?
+  end
+
+  def options
+    drop_down_list_options
+  end
+
+  def disabled_options
+    drop_down_list_disabled_options
+  end
+
+  def enabled_non_empty_options
+    drop_down_list_enabled_non_empty_options
   end
 end

app/models/champs/linked_drop_down_list_champ.rb

@@ -1,6 +1,10 @@
 class Champs::LinkedDropDownListChamp < Champ
   delegate :primary_options, :secondary_options, to: 'type_de_champ.dynamic_type'
 
+  def options?
+    drop_down_list_options?
+  end
+
   def primary_value
     if value.present?
       JSON.parse(value)[0]

app/models/champs/multiple_drop_down_list_champ.rb

@@ -1,6 +1,22 @@
 class Champs::MultipleDropDownListChamp < Champ
   before_save :format_before_save
 
+  def options?
+    drop_down_list_options?
+  end
+
+  def options
+    drop_down_list_options
+  end
+
+  def disabled_options
+    drop_down_list_disabled_options
+  end
+
+  def enabled_non_empty_options
+    drop_down_list_enabled_non_empty_options
+  end
+
   THRESHOLD_NB_OPTIONS_AS_CHECKBOX = 5
 
   def search_terms
@@ -24,7 +40,7 @@ class Champs::MultipleDropDownListChamp < Champ
   end
 
   def render_as_checkboxes?
-    drop_down_list.enabled_non_empty_options.size <= THRESHOLD_NB_OPTIONS_AS_CHECKBOX
+    enabled_non_empty_options.size <= THRESHOLD_NB_OPTIONS_AS_CHECKBOX
   end
 
   private

app/models/drop_down_list.rb

@@ -1,30 +1,8 @@
 class DropDownList < ApplicationRecord
   belongs_to :type_de_champ
 
-  before_validation :clean_value
-
   def options
     result = value.split(/[\r\n]|[\r]|[\n]|[\n\r]/).reject(&:empty?)
     result.blank? ? [] : [''] + result
   end
-
-  def enabled_non_empty_options
-    (options - disabled_options).reject(&:empty?)
-  end
-
-  def disabled_options
-    options.filter { |v| (v =~ /^--.*--$/).present? }
-  end
-
-  def multiple
-    type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:multiple_drop_down_list)
-  end
-
-  private
-
-  def clean_value
-    value = read_attribute(:value)
-    value = value ? value.split("\r\n").map(&:strip).join("\r\n") : ''
-    write_attribute(:value, value)
-  end
 end

app/models/type_de_champ.rb

@@ -35,7 +35,7 @@ class TypeDeChamp < ApplicationRecord
   belongs_to :parent, class_name: 'TypeDeChamp'
   has_many :types_de_champ, -> { ordered }, foreign_key: :parent_id, class_name: 'TypeDeChamp', inverse_of: :parent, dependent: :destroy
 
-  store_accessor :options, :cadastres, :quartiers_prioritaires, :parcelles_agricoles, :old_pj
+  store_accessor :options, :cadastres, :quartiers_prioritaires, :parcelles_agricoles, :old_pj, :drop_down_options
   delegate :tags_for_template, to: :dynamic_type
 
   class WithIndifferentAccess
@@ -53,7 +53,6 @@ class TypeDeChamp < ApplicationRecord
   after_initialize :set_dynamic_type
   after_create :populate_stable_id
   before_save :setup_procedure
-  before_validation :set_default_drop_down_list
 
   attr_reader :dynamic_type
 
@@ -83,6 +82,7 @@ class TypeDeChamp < ApplicationRecord
 
   before_validation :check_mandatory
   before_save :remove_piece_justificative_template, if: -> { type_champ_changed? }
+  before_validation :remove_drop_down_list, if: -> { type_champ_changed? }
 
   def valid?(context = nil)
     super
@@ -146,6 +146,10 @@ class TypeDeChamp < ApplicationRecord
     ])
   end
 
+  def linked_drop_down_list?
+    type_champ == TypeDeChamp.type_champs.fetch(:linked_drop_down_list)
+  end
+
   def exclude_from_view?
     type_champ == TypeDeChamp.type_champs.fetch(:explication)
   end
@@ -158,6 +162,10 @@ class TypeDeChamp < ApplicationRecord
     type_champ == TypeDeChamp.type_champs.fetch(:dossier_link)
   end
 
+  def piece_justificative?
+    type_champ == TypeDeChamp.type_champs.fetch(:piece_justificative)
+  end
+
   def legacy_number?
     type_champ == TypeDeChamp.type_champs.fetch(:number)
   end
@@ -183,11 +191,31 @@ class TypeDeChamp < ApplicationRecord
   end
 
   def drop_down_list_value
-    drop_down_list&.value
+    if drop_down_list_options.present?
+      drop_down_list_options.reject(&:empty?).join("\r\n")
+    else
+      ''
+    end
   end
 
   def drop_down_list_value=(value)
-    self.drop_down_list_attributes = { value: value }
+    self.drop_down_options = parse_drop_down_list_value(value)
+  end
+
+  def drop_down_list_options?
+    drop_down_list_options.any?
+  end
+
+  def drop_down_list_options
+    drop_down_options.presence || drop_down_list&.options || []
+  end
+
+  def drop_down_list_disabled_options
+    drop_down_list_options.filter { |v| (v =~ /^--.*--$/).present? }
+  end
+
+  def drop_down_list_enabled_non_empty_options
+    (drop_down_list_options - drop_down_list_disabled_options).reject(&:empty?)
   end
 
   def to_typed_id
@@ -244,10 +272,10 @@ class TypeDeChamp < ApplicationRecord
 
   private
 
-  def set_default_drop_down_list
-    if drop_down_list? && !drop_down_list
-      self.drop_down_list_attributes = { value: '' }
-    end
+  def parse_drop_down_list_value(value)
+    value = value ? value.split("\r\n").map(&:strip).join("\r\n") : ''
+    result = value.split(/[\r\n]|[\r]|[\n]|[\n\r]/).reject(&:empty?)
+    result.blank? ? [] : [''] + result
   end
 
   def setup_procedure
@@ -263,8 +291,15 @@ class TypeDeChamp < ApplicationRecord
   end
 
   def remove_piece_justificative_template
-    if type_champ != TypeDeChamp.type_champs.fetch(:piece_justificative) && piece_justificative_template.attached?
+    if !piece_justificative? && piece_justificative_template.attached?
       piece_justificative_template.purge_later
     end
   end
+
+  def remove_drop_down_list
+    if !drop_down_list?
+      self.drop_down_list = nil
+      self.drop_down_options = nil
+    end
+  end
 end

app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb

@@ -1,26 +1,9 @@
 class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBase
   PRIMARY_PATTERN = /^--(.*)--$/
 
-  delegate :drop_down_list, to: :@type_de_champ
-
+  delegate :drop_down_list_options, to: :@type_de_champ
   validate :check_presence_of_primary_options
 
-  def primary_options
-    primary_options = unpack_options.map(&:first)
-    if primary_options.present?
-      primary_options.unshift('')
-    end
-    primary_options
-  end
-
-  def secondary_options
-    secondary_options = unpack_options.to_h
-    if secondary_options.present?
-      secondary_options[''] = []
-    end
-    secondary_options
-  end
-
   def tags_for_template
     tags = super
     tdc = @type_de_champ
@@ -49,16 +32,26 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
     tags
   end
 
-  private
-
-  def check_presence_of_primary_options
-    if !PRIMARY_PATTERN.match?(drop_down_list.options.second)
-      errors.add(libelle.presence || "La liste", "doit commencer par une entrée de menu primaire de la forme <code style='white-space: pre-wrap;'>--texte--</code>")
+  def primary_options
+    primary_options = unpack_options.map(&:first)
+    if primary_options.present?
+      primary_options.unshift('')
     end
+    primary_options
   end
 
+  def secondary_options
+    secondary_options = unpack_options.to_h
+    if secondary_options.present?
+      secondary_options[''] = []
+    end
+    secondary_options
+  end
+
+  private
+
   def unpack_options
-    _, *options = drop_down_list.options
+    _, *options = drop_down_list_options
     chunked = options.slice_before(PRIMARY_PATTERN)
     chunked.map do |chunk|
       primary, *secondary = chunk
@@ -66,4 +59,10 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas
       [PRIMARY_PATTERN.match(primary)&.[](1), secondary]
     end
   end
+
+  def check_presence_of_primary_options
+    if !PRIMARY_PATTERN.match?(drop_down_list_options.second)
+      errors.add(libelle.presence || "La liste", "doit commencer par une entrée de menu primaire de la forme <code style='white-space: pre-wrap;'>--texte--</code>")
+    end
+  end
 end

app/views/dossiers/dossier_vide.pdf.prawn

@@ -129,7 +129,7 @@ def render_single_champ(pdf, champ)
     add_libelle(pdf, champ)
     add_optionnal_description(pdf, champ)
     add_explanation(pdf, 'Cochez la mention applicable, une seule valeur possible')
-    champ.drop_down_list.options.reject(&:blank?).each do |option|
+    champ.options.reject(&:blank?).each do |option|
       format_with_checkbox(pdf, option)
     end
     pdf.text "\n"
@@ -137,7 +137,7 @@ def render_single_champ(pdf, champ)
     add_libelle(pdf, champ)
     add_optionnal_description(pdf, champ)
     add_explanation(pdf, 'Cochez la mention applicable, plusieurs valeurs possibles')
-    champ.drop_down_list.options.reject(&:blank?).each do |option|
+    champ.options.reject(&:blank?).each do |option|
       format_with_checkbox(pdf, option)
     end
     pdf.text "\n"

app/views/shared/dossiers/editable_champs/_drop_down_list.html.haml

@@ -1,14 +1,14 @@
-- if champ.drop_down_list && champ.drop_down_list.options.any?
+- if champ.options?
   - if champ.render_as_radios?
     %fieldset.radios
       %legend.mandatory-explanation
         Sélectionnez une des valeurs
-      - champ.drop_down_list.enabled_non_empty_options.each do |option|
+      - champ.enabled_non_empty_options.each do |option|
         %label
           = form.radio_button :value, option
           = option
   - else
     = form.select :value,
-      champ.drop_down_list.options,
-      disabled: champ.drop_down_list.disabled_options,
+      champ.options,
+      disabled: champ.disabled_options,
       required: champ.mandatory?

app/views/shared/dossiers/editable_champs/_linked_drop_down_list.html.haml

@@ -1,4 +1,4 @@
-- if champ.drop_down_list && champ.drop_down_list.options.any?
+- if champ.options?
   = form.select :primary_value,
     champ.primary_options,
     { required: champ.mandatory? },

app/views/shared/dossiers/editable_champs/_multiple_drop_down_list.html.haml

@@ -1,15 +1,15 @@
-- if champ.drop_down_list && champ.drop_down_list.options.any?
+- if champ.options?
   - if champ.render_as_checkboxes?
-    - champ.drop_down_list.enabled_non_empty_options.each do |option|
+    - champ.enabled_non_empty_options.each do |option|
       .editable-champ.editable-champ-checkbox
         %label
           = form.check_box :value, { multiple: true, checked: champ&.value&.include?(option) }, option, nil
           = option
   - else
     = form.select :value,
-        champ.drop_down_list.options,
+        champ.options,
         { selected: champ.selected_options,
-          disabled: champ.drop_down_list.disabled_options },
+          disabled: champ.disabled_options },
         multiple: true,
         class: 'select2'
 

lib/tasks/deployment/20200618121241_drop_down_list_options_to_json.rake

@@ -0,0 +1,25 @@
+namespace :after_party do
+  desc 'Deployment task: drop_down_list_options_to_json'
+  task drop_down_list_options_to_json: :environment do
+    puts "Running deploy task 'drop_down_list_options_to_json'"
+
+    types_de_champ = TypeDeChamp.joins(:drop_down_list).where(type_champ: [
+      TypeDeChamp.type_champs.fetch(:drop_down_list),
+      TypeDeChamp.type_champs.fetch(:multiple_drop_down_list),
+      TypeDeChamp.type_champs.fetch(:linked_drop_down_list)
+    ])
+    progress = ProgressReport.new(types_de_champ.count)
+    types_de_champ.find_each do |type_de_champ|
+      type_de_champ.drop_down_list_value = type_de_champ.drop_down_list_value
+      if type_de_champ.save
+        type_de_champ.drop_down_list.destroy
+      end
+      progress.inc
+    end
+    progress.finish
+
+    # Update task as completed.  If you remove the line below, the task will
+    # run with every deploy (or every time you call after_party:run).
+    AfterParty::TaskRecord.create version: '20200618121241'
+  end
+end

spec/factories/drop_down_list.rb

@@ -1,9 +0,0 @@
-FactoryBot.define do
-  factory :drop_down_list do
-    value { "val1\r\nval2\r\n--separateur--\r\nval3" }
-
-    trait :long do
-      value { "alpha\r\nbravo\r\n--separateur--\r\ncharly\r\ndelta\r\necho\r\nfox-trot\r\ngolf" }
-    end
-  end
-end

spec/factories/type_de_champ.rb

@@ -52,21 +52,21 @@ FactoryBot.define do
     factory :type_de_champ_drop_down_list do
       libelle { 'Menu déroulant' }
       type_champ { TypeDeChamp.type_champs.fetch(:drop_down_list) }
-      drop_down_list { create(:drop_down_list) }
+      drop_down_list_value { "val1\r\nval2\r\n--separateur--\r\nval3" }
       trait :long do
-        drop_down_list { create(:drop_down_list, :long) }
+        drop_down_list_value { "alpha\r\nbravo\r\n--separateur--\r\ncharly\r\ndelta\r\necho\r\nfox-trot\r\ngolf" }
       end
     end
     factory :type_de_champ_multiple_drop_down_list do
       type_champ { TypeDeChamp.type_champs.fetch(:multiple_drop_down_list) }
-      drop_down_list { create(:drop_down_list) }
+      drop_down_list_value { "val1\r\nval2\r\n--separateur--\r\nval3" }
       trait :long do
-        drop_down_list { create(:drop_down_list, :long) }
+        drop_down_list_value { "alpha\r\nbravo\r\n--separateur--\r\ncharly\r\ndelta\r\necho\r\nfox-trot\r\ngolf" }
       end
     end
     factory :type_de_champ_linked_drop_down_list do
       type_champ { TypeDeChamp.type_champs.fetch(:linked_drop_down_list) }
-      drop_down_list { create(:drop_down_list, value: "--primary--\nsecondary\n") }
+      drop_down_list_value { "--primary--\nsecondary\n" }
     end
     factory :type_de_champ_pays do
       type_champ { TypeDeChamp.type_champs.fetch(:pays) }

spec/features/new_administrateur/types_de_champ_spec.rb

@@ -139,7 +139,7 @@ feature 'As an administrateur I can edit types de champ', js: true do
     fill_in 'champ-0-libelle', with: 'Libellé de champ menu déroulant', fill_options: { clear: :backspace }
     fill_in 'champ-0-drop_down_list_value', with: 'Un menu', fill_options: { clear: :backspace }
 
-    wait_until { procedure.types_de_champ.first.drop_down_list&.value == 'Un menu' }
+    wait_until { procedure.types_de_champ.first.drop_down_list_options == ['', 'Un menu'] }
     expect(page).to have_content('Formulaire enregistré')
 
     page.refresh

spec/features/users/linked_dropdown_spec.rb

@@ -13,8 +13,7 @@ feature 'linked dropdown lists' do
       Secondary 2.3
     END_OF_LIST
   end
-  let(:drop_down_list) { create(:drop_down_list, value: list_items) }
-  let(:type_de_champ) { create(:type_de_champ_linked_drop_down_list, libelle: 'linked dropdown', drop_down_list: drop_down_list) }
+  let(:type_de_champ) { create(:type_de_champ_linked_drop_down_list, libelle: 'linked dropdown', drop_down_list_value: list_items) }
 
   let!(:procedure) do
     p = create(:procedure, :published, :for_individual)

spec/models/champs/linked_drop_down_list_champ_spec.rb

@@ -62,12 +62,12 @@ describe Champs::LinkedDropDownListChamp do
   end
 
   describe '#mandatory_and_blank' do
-    let(:drop_down_list) { build(:drop_down_list, value: "--Primary--\nSecondary") }
+    let(:value) { "--Primary--\nSecondary" }
 
     subject { described_class.new(type_de_champ: type_de_champ) }
 
     context 'when the champ is not mandatory' do
-      let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, drop_down_list: drop_down_list) }
+      let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, drop_down_list_value: value) }
 
       it 'blank is fine' do
         is_expected.not_to be_mandatory_and_blank
@@ -75,7 +75,7 @@ describe Champs::LinkedDropDownListChamp do
     end
 
     context 'when the champ is mandatory' do
-      let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, mandatory: true, drop_down_list: drop_down_list) }
+      let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, mandatory: true, drop_down_list_value: value) }
 
       context 'when there is no value' do
         it { is_expected.to be_mandatory_and_blank }
@@ -95,7 +95,7 @@ describe Champs::LinkedDropDownListChamp do
         end
 
         context 'when there is nothing to select for the secondary value' do
-          let(:drop_down_list) { build(:drop_down_list, value: "--A--\nAbbott\nAbelard\n--B--\n--C--\nCynthia") }
+          let(:value) { "--A--\nAbbott\nAbelard\n--B--\n--C--\nCynthia" }
           before { subject.primary_value = 'B' }
 
           it { is_expected.not_to be_mandatory_and_blank }

spec/models/drop_down_list_spec.rb

@@ -1,41 +0,0 @@
-describe DropDownList do
-  let(:dropdownlist) { create :drop_down_list, value: value }
-
-  describe '#options' do
-    let(:value) do
-      <<~EOS
-        Cohésion sociale
-        Dév.Eco / Emploi
-        Cadre de vie / Urb.
-        Pilotage / Ingénierie
-      EOS
-    end
-
-    it { expect(dropdownlist.options).to eq ['', 'Cohésion sociale', 'Dév.Eco / Emploi', 'Cadre de vie / Urb.', 'Pilotage / Ingénierie'] }
-
-    context 'when one value is empty' do
-      let(:value) do
-        <<~EOS
-          Cohésion sociale
-          Cadre de vie / Urb.
-          Pilotage / Ingénierie
-        EOS
-      end
-
-      it { expect(dropdownlist.options).to eq ['', 'Cohésion sociale', 'Cadre de vie / Urb.', 'Pilotage / Ingénierie'] }
-    end
-  end
-
-  describe 'disabled_options' do
-    let(:value) do
-      <<~EOS
-        tip
-        --top--
-        --troupt--
-        ouaich
-      EOS
-    end
-
-    it { expect(dropdownlist.disabled_options).to match(['--top--', '--troupt--']) }
-  end
-end

spec/models/type_de_champ_shared_example.rb

@@ -187,4 +187,44 @@ shared_examples 'type_de_champ_spec' do
       end
     end
   end
+
+  describe '#drop_down_list_options' do
+    let(:value) do
+      <<~EOS
+        Cohésion sociale
+        Dév.Eco / Emploi
+        Cadre de vie / Urb.
+        Pilotage / Ingénierie
+      EOS
+    end
+    let(:type_de_champ) { create(:type_de_champ_drop_down_list, drop_down_list_value: value) }
+
+    it { expect(type_de_champ.drop_down_list_options).to eq ['', 'Cohésion sociale', 'Dév.Eco / Emploi', 'Cadre de vie / Urb.', 'Pilotage / Ingénierie'] }
+
+    context 'when one value is empty' do
+      let(:value) do
+        <<~EOS
+          Cohésion sociale
+          Cadre de vie / Urb.
+          Pilotage / Ingénierie
+        EOS
+      end
+
+      it { expect(type_de_champ.drop_down_list_options).to eq ['', 'Cohésion sociale', 'Cadre de vie / Urb.', 'Pilotage / Ingénierie'] }
+    end
+  end
+
+  describe 'disabled_options' do
+    let(:value) do
+      <<~EOS
+        tip
+        --top--
+        --troupt--
+        ouaich
+      EOS
+    end
+    let(:type_de_champ) { create(:type_de_champ_drop_down_list, drop_down_list_value: value) }
+
+    it { expect(type_de_champ.drop_down_list_disabled_options).to match(['--top--', '--troupt--']) }
+  end
 end

spec/models/types_de_champ/linked_drop_down_list_type_de_champ_spec.rb

@@ -1,6 +1,5 @@
 describe TypesDeChamp::LinkedDropDownListTypeDeChamp do
-  let(:drop_down_list) { build(:drop_down_list, value: menu_options) }
-  let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, drop_down_list: drop_down_list) }
+  let(:type_de_champ) { build(:type_de_champ_linked_drop_down_list, drop_down_list_value: menu_options) }
 
   subject { type_de_champ.dynamic_type }