specs: avoid executing business logic in Procedure factory

Calling business logic in a factory is a code-smell, because it
usually requires the object to be saved into database, and may have
unintended consequences when the business logic is changed.

Also, this allows to just build a published procedure, without saving it
to the database.

spec/factories/dossier.rb

@@ -2,18 +2,20 @@ FactoryBot.define do
   factory :dossier do
     autorisation_donnees { true }
     state { Dossier.states.fetch(:brouillon) }
-    association :user
+
+    user { association :user }
 
     transient do
-      procedure { nil }
+      for_individual? { false }
+      # For now a dossier must use a `create`d procedure, even if the dossier is only built (and not created).
+      # This is because saving the dossier fails when the procedure has not been saved beforehand
+      # (due to some internal ActiveRecord error).
+      # TODO: find a way to find the issue and just `build` the procedure.
+      procedure { create(:procedure, :published, :with_type_de_champ, :with_type_de_champ_private, for_individual: for_individual?) }
     end
 
     after(:build) do |dossier, evaluator|
-      if evaluator.procedure.present?
+      procedure = evaluator.procedure
-        procedure = evaluator.procedure
-      else
-        procedure = create(:procedure, :published, :with_type_de_champ, :with_type_de_champ_private)
-      end
 
       dossier.revision = procedure.active_revision
 
@@ -22,7 +24,9 @@ FactoryBot.define do
         dossier.groupe_instructeur = procedure.routee? ? nil : procedure.defaut_groupe_instructeur
       end
 
-      dossier.build_default_individual
+      if procedure.for_individual? && dossier.individual.blank?
+        build(:individual, :empty, dossier: dossier)
+      end
     end
 
     trait :with_entreprise do
@@ -42,12 +46,11 @@ FactoryBot.define do
     end
 
     trait :with_individual do
-      after(:build) do |dossier, evaluator|
+      transient do
-        # If the procedure was implicitely created by the factory,
+        for_individual? { true }
-        # mark it automatically as for_individual.
+      end
-        if evaluator.procedure.nil?
+
-          dossier.procedure.update(for_individual: true)
+      after(:build) do |dossier, _evaluator|
-        end
         if !dossier.procedure.for_individual?
           raise 'Inconsistent factory: attempting to create a dossier :with_individual on a procedure that is not `for_individual?`'
         end

spec/factories/individual.rb

@@ -5,5 +5,12 @@ FactoryBot.define do
     prenom { 'Xavier' }
     birthdate { Date.new(1991, 11, 01) }
     association :dossier
+
+    trait :empty do
+      gender { nil }
+      nom { nil }
+      prenom { nil }
+      birthdate { nil }
+    end
   end
 end

spec/factories/procedure.rb

@@ -1,5 +1,6 @@
 FactoryBot.define do
   sequence(:published_path) { |n| "fake_path#{n}" }
+
   factory :procedure do
     sequence(:libelle) { |n| "Procedure #{n}" }
     description { "Demande de subvention à l'intention des associations" }
@@ -26,24 +27,17 @@ FactoryBot.define do
       elsif procedure.administrateurs.empty?
         procedure.administrateurs = [build(:administrateur)]
       end
-      procedure.draft_revision = build(:procedure_revision, procedure: procedure)
 
-      evaluator.types_de_champ.each do |type_de_champ|
+      initial_revision = build(:procedure_revision, procedure: procedure)
-        type_de_champ.revision = procedure.draft_revision
+      add_types_de_champs(evaluator.types_de_champ, to: initial_revision, scope: :public)
-        type_de_champ.private = false
+      add_types_de_champs(evaluator.types_de_champ_private, to: initial_revision, scope: :private)
-        type_de_champ.revision.revision_types_de_champ << build(:procedure_revision_type_de_champ,
-          revision: procedure.draft_revision,
-          position: type_de_champ.order_place,
-          type_de_champ: type_de_champ)
-      end
 
-      evaluator.types_de_champ_private.each do |type_de_champ|
+      if procedure.brouillon?
-        type_de_champ.revision = procedure.draft_revision
+        procedure.draft_revision = initial_revision
-        type_de_champ.private = true
+      else
-        type_de_champ.revision.revision_types_de_champ_private << build(:procedure_revision_type_de_champ,
+        procedure.published_revision = initial_revision
-          revision: procedure.draft_revision,
+        procedure.published_revision.published_at = Time.zone.now
-          position: type_de_champ.order_place,
+        procedure.draft_revision = build(:procedure_revision, from_original: initial_revision)
-          type_de_champ: type_de_champ)
       end
     end
 
@@ -71,11 +65,12 @@ FactoryBot.define do
     end
 
     factory :simple_procedure do
+      published
+
+      for_individual { true }
+
       after(:build) do |procedure, _evaluator|
-        procedure.for_individual = true
         build(:type_de_champ, libelle: 'Texte obligatoire', mandatory: true, procedure: procedure)
-        procedure.path = generate(:published_path)
-        procedure.publish!
       end
     end
 
@@ -218,26 +213,27 @@ FactoryBot.define do
     end
 
     trait :published do
-      after(:build) do |procedure, _evaluator|
+      aasm_state { :publiee }
-        procedure.path = generate(:published_path)
+      path { generate(:published_path) }
-        procedure.publish!
+      published_at { Time.zone.now }
-      end
+      unpublished_at { nil }
+      closed_at { nil }
     end
 
     trait :closed do
-      after(:build) do |procedure, _evaluator|
+      published
-        procedure.path = generate(:published_path)
+
-        procedure.publish!
+      aasm_state { :close }
-        procedure.close!
+      published_at { Time.zone.now - 1.second }
-      end
+      closed_at { Time.zone.now }
     end
 
     trait :unpublished do
-      after(:build) do |procedure, _evaluator|
+      published
-        procedure.path = generate(:published_path)
+
-        procedure.publish!
+      aasm_state { :depubliee }
-        procedure.unpublish!
+      published_at { Time.zone.now - 1.second }
-      end
+      unpublished_at { Time.zone.now }
     end
 
     trait :discarded do
@@ -308,3 +304,17 @@ FactoryBot.define do
     end
   end
 end
+
+def add_types_de_champs(types_de_champ, to: nil, scope: :public)
+  revision = to
+  association_name = scope == :private ? :revision_types_de_champ_private : :revision_types_de_champ
+
+  types_de_champ.each do |type_de_champ|
+    type_de_champ.revision = revision
+    type_de_champ.private = (scope == :private)
+    type_de_champ.revision.public_send(association_name) << build(:procedure_revision_type_de_champ,
+                                                                  revision: revision,
+                                                                  position: type_de_champ.order_place,
+                                                                  type_de_champ: type_de_champ)
+  end
+end

spec/factories/procedure_revision.rb

@@ -1,4 +1,21 @@
 FactoryBot.define do
   factory :procedure_revision do
+    transient do
+      from_original { nil }
+    end
+
+    after(:build) do |revision, evaluator|
+      if evaluator.from_original
+        original = evaluator.from_original
+
+        revision.procedure = original.procedure
+        original.revision_types_de_champ.each do |r_tdc|
+          revision.revision_types_de_champ << build(:procedure_revision_type_de_champ, from_original: r_tdc)
+        end
+        original.revision_types_de_champ_private.each do |r_tdc|
+          revision.revision_types_de_champ_private << build(:procedure_revision_type_de_champ, from_original: r_tdc)
+        end
+      end
+    end
   end
 end

spec/factories/procedure_revision_type_de_champ.rb

@@ -1,4 +1,16 @@
 FactoryBot.define do
   factory :procedure_revision_type_de_champ do
+    transient do
+      from_original { nil }
+    end
+
+    after(:build) do |revision_type_de_champ, evaluator|
+      if evaluator.from_original
+        original = evaluator.from_original
+
+        revision_type_de_champ.type_de_champ = original.type_de_champ
+        revision_type_de_champ.position      = original.position
+      end
+    end
   end
 end

spec/helpers/dossier_helper_spec.rb

@@ -49,7 +49,7 @@ RSpec.describe DossierHelper, type: :helper do
       let(:procedure) { create(:simple_procedure, :for_individual) }
 
       context "when the individual is not provided" do
-        let(:individual) { nil }
+        let(:individual) { build(:individual, :empty) }
         it { is_expected.to be_blank }
       end
 

spec/mailers/application_mailer_spec.rb

@@ -1,6 +1,6 @@
 RSpec.describe ApplicationMailer, type: :mailer do
   describe 'dealing with invalid emails' do
-    let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
+    let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) }
     subject { DossierMailer.notify_new_draft(dossier) }
 
     describe 'invalid emails are not sent' do

spec/mailers/dossier_mailer_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe DossierMailer, type: :mailer do
   end
 
   describe '.notify_new_draft' do
-    let(:dossier) { create(:dossier, procedure: build(:simple_procedure, :with_auto_archive)) }
+    let(:dossier) { create(:dossier, procedure: create(:simple_procedure, :with_auto_archive)) }
 
     subject { described_class.notify_new_draft(dossier) }
 
@@ -27,7 +27,7 @@ RSpec.describe DossierMailer, type: :mailer do
   end
 
   describe '.notify_new_answer with dossier brouillon' do
-    let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
+    let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) }
     let(:commentaire) { create(:commentaire, dossier: dossier) }
     subject { described_class.with(commentaire: commentaire).notify_new_answer }
 
@@ -39,8 +39,9 @@ RSpec.describe DossierMailer, type: :mailer do
   end
 
   describe '.notify_new_answer with dossier en construction' do
-    let(:dossier) { create(:dossier, state: "en_construction", procedure: build(:simple_procedure)) }
+    let(:dossier) { create(:dossier, :en_construction, procedure: create(:simple_procedure)) }
     let(:commentaire) { create(:commentaire, dossier: dossier) }
+
     subject { described_class.with(commentaire: commentaire).notify_new_answer }
 
     it { expect(subject.subject).to include("Nouveau message") }
@@ -51,7 +52,7 @@ RSpec.describe DossierMailer, type: :mailer do
   end
 
   describe '.notify_new_answer with commentaire discarded' do
-    let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
+    let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) }
     let(:commentaire) { create(:commentaire, dossier: dossier, discarded_at: 2.minutes.ago) }
 
     subject { described_class.with(commentaire: commentaire).notify_new_answer }
@@ -83,7 +84,7 @@ RSpec.describe DossierMailer, type: :mailer do
   end
 
   describe '.notify_revert_to_instruction' do
-    let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
+    let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) }
 
     subject { described_class.notify_revert_to_instruction(dossier) }
 

spec/models/procedure_presentation_spec.rb

@@ -320,8 +320,8 @@ describe ProcedurePresentation do
 
       let(:procedure) { create(:procedure, :for_individual) }
 
-      let!(:first_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Alain', nom: 'Antonelli')) }
+      let!(:first_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'M', prenom: 'Alain', nom: 'Antonelli')) }
-      let!(:last_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'Mme', prenom: 'Zora', nom: 'Zemmour')) }
+      let!(:last_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'Mme', prenom: 'Zora', nom: 'Zemmour')) }
 
       context 'for gender column' do
         let(:column) { 'gender' }
@@ -617,8 +617,8 @@ describe ProcedurePresentation do
 
     context 'for individual table' do
       let(:procedure) { create(:procedure, :for_individual) }
-      let!(:kept_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'Mme', prenom: 'Josephine', nom: 'Baker')) }
+      let!(:kept_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'Mme', prenom: 'Josephine', nom: 'Baker')) }
-      let!(:discarded_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Jean', nom: 'Tremblay')) }
+      let!(:discarded_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'M', prenom: 'Jean', nom: 'Tremblay')) }
 
       context 'for gender column' do
         let(:filter) { [{ 'table' => 'individual', 'column' => 'gender', 'value' => 'Mme' }] }
@@ -646,7 +646,7 @@ describe ProcedurePresentation do
           ]
         end
 
-        let!(:other_kept_dossier) { create(:dossier, procedure: procedure, individual: create(:individual, gender: 'M', prenom: 'Romuald', nom: 'Pistis')) }
+        let!(:other_kept_dossier) { create(:dossier, procedure: procedure, individual: build(:individual, gender: 'M', prenom: 'Romuald', nom: 'Pistis')) }
 
         it 'returns every dossier that matches any of the search criteria for a given column' do
           is_expected.to contain_exactly(kept_dossier.id, other_kept_dossier.id)

spec/services/dossier_projection_service_spec.rb

@@ -92,7 +92,7 @@ describe DossierProjectionService do
       context 'for individual table' do
         let(:table) { 'individual' }
         let(:procedure) { create(:procedure, :for_individual, :with_type_de_champ, :with_type_de_champ_private) }
-        let(:dossier) { create(:dossier, procedure: procedure, individual: create(:individual, nom: 'Martin', prenom: 'Jacques', gender: 'M.')) }
+        let(:dossier) { create(:dossier, procedure: procedure, individual: build(:individual, nom: 'Martin', prenom: 'Jacques', gender: 'M.')) }
 
         context 'for prenom column' do
           let(:column) { 'prenom' }

spec/system/accessibilite/wcag_usager_spec.rb

@@ -1,5 +1,5 @@
 describe 'wcag rules for usager', js: true do
-  let(:procedure) { create(:procedure, :with_type_de_champ, :with_all_champs, :with_service, :for_individual, :published) }
+  let(:procedure) { create(:procedure, :published, :with_all_champs, :with_service, :for_individual) }
   let(:password) { 'a very complicated password' }
   let(:litteraire_user) { create(:user, password: password) }
 

spec/views/shared/dossiers/_demande.html.haml_spec.rb

@@ -32,7 +32,7 @@ describe 'shared/dossiers/demande.html.haml', type: :view do
   end
 
   context 'when dossier was created by an individual' do
-    let(:individual) { create(:individual) }
+    let(:individual) { build(:individual) }
 
     it 'renders the individual identity infos' do
       expect(subject).to include(individual.gender)