refactor(champs): remove orphaned and invisible champs after submit

app/models/concerns/dossier_state_concern.rb

@@ -11,9 +11,7 @@ module DossierStateConcern
 
     resolve_pending_correction!
     process_sva_svr!
-    remove_piece_justificative_file_not_visible!
+    clean_champs_after_submit!
-
-    editing_forks.each(&:destroy_editing_fork!)
   end
 
   def after_passer_en_construction
@@ -41,7 +39,8 @@ module DossierStateConcern
     groupe_instructeur.instructeurs.with_instant_email_dossier_notifications.each do |instructeur|
       DossierMailer.notify_new_dossier_depose_to_instructeur(self, instructeur.email).deliver_later
     end
-    remove_piece_justificative_file_not_visible!
+
+    clean_champs_after_submit!
   end
 
   def after_passer_en_instruction(h)
@@ -154,7 +153,7 @@ module DossierStateConcern
     end
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_accepter_automatiquement
@@ -189,7 +188,7 @@ module DossierStateConcern
     NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers?
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_refuser(h)
@@ -226,7 +225,7 @@ module DossierStateConcern
     end
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_refuser_automatiquement
@@ -254,7 +253,7 @@ module DossierStateConcern
     NotificationMailer.send_notification_for_tiers(self).deliver_later if self.for_tiers?
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_classer_sans_suite(h)
@@ -291,7 +290,7 @@ module DossierStateConcern
     end
 
     send_dossier_decision_to_experts(self)
-    remove_titres_identite!
+    clean_champs_after_instruction!
   end
 
   def after_repasser_en_instruction(h)
@@ -329,4 +328,58 @@ module DossierStateConcern
 
     rebase_later
   end
+
+  def clean_champs_after_submit!
+    remove_discarded_rows!
+    remove_not_visible_rows!
+    remove_not_visible_or_empty_champs!
+    editing_forks.each(&:destroy_editing_fork!)
+  end
+
+  def clean_champs_after_instruction!
+    remove_discarded_rows!
+    remove_titres_identite!
+  end
+
+  private
+
+  def remove_discarded_rows!
+    row_to_remove_ids = champs.filter { _1.row? && _1.discarded? }.map(&:row_id)
+
+    return if row_to_remove_ids.empty?
+    champs.where(row_id: row_to_remove_ids).destroy_all
+  end
+
+  def remove_not_visible_or_empty_champs!
+    repetition_to_keep_stable_ids, champ_to_keep_public_ids = project_champs_public_all
+      .reject { _1.blank? || !_1.visible? }
+      .partition(&:repetition?)
+      .then { |(repetitions, champs)| [repetitions.to_set(&:stable_id), champs.to_set(&:public_id)] }
+
+    rows_public, champs_public = champs
+      .filter(&:public?)
+      .partition(&:row?)
+
+    champs_to_remove = champs_public.reject { champ_to_keep_public_ids.member?(_1.public_id) }
+    champs_to_remove += rows_public.reject { repetition_to_keep_stable_ids.member?(_1.stable_id) }
+
+    return if champs_to_remove.empty?
+    champs.where(id: champs_to_remove).destroy_all
+  end
+
+  def remove_not_visible_rows!
+    row_to_remove_ids = project_champs_public
+      .filter { _1.repetition? && !_1.visible? }
+      .flat_map(&:row_ids)
+
+    return if row_to_remove_ids.empty?
+    champs.where(row_id: row_to_remove_ids).destroy_all
+  end
+
+  def remove_titres_identite!
+    champ_to_remove_ids = filled_champs.filter(&:titre_identite?).map(&:id)
+
+    return if champ_to_remove_ids.empty?
+    champs.where(id: champ_to_remove_ids).destroy_all
+  end
 end

app/models/dossier.rb

@@ -905,19 +905,6 @@ class Dossier < ApplicationRecord
     traitements.any?(&:termine?)
   end
 
-  def remove_titres_identite!
-    champs.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later)
-  end
-
-  def remove_piece_justificative_file_not_visible!
-    champs.each do |champ|
-      next unless champ.piece_justificative_file.attached?
-      next if champ.visible?
-
-      champ.piece_justificative_file.purge_later
-    end
-  end
-
   def check_mandatory_and_visible_champs
     project_champs_public.filter(&:visible?).each do |champ|
       if champ.mandatory_blank?

app/tasks/maintenance/remove_piece_justificative_file_not_visible_task.rb

@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-module Maintenance
-  class RemovePieceJustificativeFileNotVisibleTask < MaintenanceTasks::Task
-    attribute :procedure_id, :string
-    validates :procedure_id, presence: true
-
-    def collection
-      procedure = Procedure.with_discarded.find(procedure_id.strip.to_i)
-      procedure.dossiers.state_not_brouillon
-    end
-
-    def process(dossier)
-      dossier.remove_piece_justificative_file_not_visible!
-    end
-  end
-end

spec/models/concerns/dossier_state_concern_spec.rb

@@ -0,0 +1,149 @@
+# frozen_string_literal: true
+
+RSpec.describe DossierStateConcern do
+  include Logic
+
+  let(:procedure) { create(:procedure, :published, :for_individual, types_de_champ_public:, declarative_with_state:) }
+  let(:types_de_champ_public) do
+    [
+      { type: :text, stable_id: 90 },
+      { type: :text, stable_id: 91 },
+      { type: :piece_justificative, stable_id: 92, condition: ds_eq(constant(true), constant(false)) },
+      { type: :titre_identite, stable_id: 93, condition: ds_eq(constant(true), constant(false)) },
+      { type: :repetition, stable_id: 94, children: [{ type: :text, stable_id: 941 }, { type: :text, stable_id: 942 }] },
+      { type: :repetition, stable_id: 95, children: [{ type: :text, stable_id: 951 }] },
+      { type: :repetition, stable_id: 96, children: [{ type: :text, stable_id: 961 }], condition: ds_eq(constant(true), constant(false)) },
+      { type: :text, stable_id: 97, condition: ds_eq(constant(true), constant(false)) },
+      { type: :titre_identite, stable_id: 98 }
+    ]
+  end
+  let(:declarative_with_state) { nil }
+  let(:dossier_state) { :brouillon }
+  let(:dossier) do
+    create(:dossier, dossier_state, :with_individual, :with_populated_champs, procedure:).tap do |dossier|
+      procedure.draft_revision.remove_type_de_champ(91)
+      procedure.draft_revision.remove_type_de_champ(95)
+      procedure.draft_revision.remove_type_de_champ(942)
+      procedure.publish_revision!
+      perform_enqueued_jobs
+      dossier.reload
+      champ_repetition = dossier.project_champs_public.find { _1.stable_id == 94 }
+      row_id = champ_repetition.row_ids.first
+      champ_repetition.remove_row(row_id, updated_by: 'test')
+    end
+  end
+
+  describe 'submit brouillon' do
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(4)
+      expect(dossier.champs.filter { _1.stable_id.in?([90, 92, 93, 97, 961, 951]) }.size).to eq(8)
+
+      champ_text = dossier.project_champs_public.find { _1.stable_id == 90 }
+      champ_text.update(value: '')
+
+      dossier.passer_en_construction!
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(3)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(0)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(0)
+      expect(dossier.champs.filter { _1.stable_id.in?([90, 92, 93, 97, 961, 951]) }.size).to eq(0)
+    end
+  end
+
+  describe 'submit en construction' do
+    let(:dossier_state) { :en_construction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(4)
+      expect(dossier.champs.filter { _1.stable_id.in?([92, 93, 97, 961, 951]) }.size).to eq(7)
+
+      dossier.submit_en_construction!
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(4)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.row? && _1.discarded? }.size).to eq(0)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id.in?([95, 96]) }.size).to eq(0)
+      expect(dossier.champs.filter { _1.stable_id.in?([92, 93, 97, 961, 951]) }.size).to eq(0)
+    end
+  end
+
+  describe 'accepter' do
+    let(:dossier_state) { :en_instruction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+      dossier.accepter!(motivation: 'test')
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(15)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+    end
+  end
+
+  describe 'refuser' do
+    let(:dossier_state) { :en_instruction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+      dossier.refuser!(motivation: 'test')
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(15)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+    end
+  end
+
+  describe 'classer_sans_suite' do
+    let(:dossier_state) { :en_instruction }
+
+    it do
+      expect(dossier.champs.size).to eq(20)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+      dossier.classer_sans_suite!(motivation: 'test')
+      dossier.reload
+
+      expect(dossier.champs.size).to eq(15)
+      expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+      expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+    end
+  end
+
+  describe 'automatiquement' do
+    let(:dossier_state) { :en_construction }
+    let(:declarative_with_state) { Dossier.states.fetch(:accepte) }
+
+    describe 'accepter' do
+      it do
+        expect(dossier.champs.size).to eq(20)
+        expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(2)
+        expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(2)
+
+        dossier.accepter_automatiquement!
+        dossier.reload
+
+        expect(dossier.champs.size).to eq(15)
+        expect(dossier.champs.filter { _1.row? && _1.stable_id == 94 }.size).to eq(1)
+        expect(dossier.champs.filter { _1.stable_id.in?([93, 98]) }.size).to eq(0)
+      end
+    end
+  end
+end

spec/models/dossier_spec.rb

@@ -501,15 +501,13 @@ describe Dossier, type: :model do
           let(:procedure) { create(:procedure, types_de_champ_public:) }
           let(:dossier) { create(:dossier, :brouillon, :with_populated_champs, procedure:) }
 
-          before { expect(champ).to receive(:visible?).and_return(visible) }
-
           context 'when piece_justificative' do
-            let(:types_de_champ_public) { [{ type: :piece_justificative }] }
+            let(:types_de_champ_public) { [{ type: :piece_justificative, condition: ds_eq(constant(true), constant(visible)) }] }
             let(:champ) { dossier.project_champs_public.find(&:piece_justificative?) }
 
             context 'when not visible' do
               let(:visible) { false }
-              it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } }
+              it { expect { subject }.to change { Champ.exists?(champ.id) } }
             end
 
             context 'when visible' do
@@ -519,12 +517,12 @@ describe Dossier, type: :model do
           end
 
           context 'when titre identite' do
-            let(:types_de_champ_public) { [{ type: :titre_identite }] }
+            let(:types_de_champ_public) { [{ type: :titre_identite, condition: ds_eq(constant(true), constant(visible)) }] }
             let(:champ) { dossier.project_champs_public.find(&:titre_identite?) }
 
             context 'when not visible' do
               let(:visible) { false }
-              it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } }
+              it { expect { subject }.to change { Champ.exists?(champ.id) } }
             end
 
             context 'when visible' do

spec/system/users/brouillon_spec.rb

@@ -569,6 +569,16 @@ describe 'The user', js: true do
         expect(page).to have_no_css('legend', text: 'permis de conduire', visible: true)
         expect(page).to have_no_css('label', text: 'tonnage', visible: true)
 
+        fill_in('age du candidat', with: '18')
+        wait_for_autosave
+
+        # the champ keeps their previous value so they are all displayed
+        expect(page).to have_css('legend', text: 'permis de conduire', visible: true)
+        expect(page).to have_css('label', text: 'tonnage', visible: true)
+
+        fill_in('age du candidat', with: '2')
+        wait_for_autosave
+
         click_on 'Déposer le dossier'
         click_on 'Accéder à votre dossier'
         click_on 'Modifier mon dossier'
@@ -576,13 +586,6 @@ describe 'The user', js: true do
         expect(page).to have_css('label', text: 'age du candidat', visible: true)
         expect(page).to have_no_css('legend', text: 'permis de conduire', visible: true)
         expect(page).to have_no_css('label', text: 'tonnage', visible: true)
-
-        fill_in('age du candidat', with: '18')
-        wait_for_autosave
-
-        # the champ keeps their previous value so they are all displayed
-        expect(page).to have_css('legend', text: 'permis de conduire', visible: true)
-        expect(page).to have_css('label', text: 'tonnage', visible: true)
       end
     end
   end

spec/tasks/maintenance/remove_piece_justificative_file_not_visible_task_spec.rb

@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-require "rails_helper"
-
-module Maintenance
-  RSpec.describe RemovePieceJustificativeFileNotVisibleTask do
-    describe "#process" do
-      subject(:process) { described_class.process(dossier) }
-
-      let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :piece_justificative }]) }
-      let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure:) }
-
-      before { expect(champ).to receive(:visible?).and_return(visible) }
-
-      context 'when piece_justificative' do
-        let(:champ) { dossier.filled_champs_public.find(&:piece_justificative?) }
-
-        context 'when not visible' do
-          let(:visible) { false }
-          it { expect { subject }.to change { champ.reload.piece_justificative_file.attached? } }
-        end
-
-        context 'when visible' do
-          let(:visible) { true }
-          it { expect { subject }.not_to change { champ.reload.piece_justificative_file.attached? } }
-        end
-      end
-    end
-  end
-end