From f66d8451b10035212f68eab318a42f9154e79dab Mon Sep 17 00:00:00 2001
From: gregoirenovel <log@kinko.fr>
Date: Tue, 4 Sep 2018 16:46:48 +0200
Subject: [PATCH 1/2] Fix FactoryBot warnings

---
 spec/factories/administrateur.rb              |  2 +-
 spec/factories/administration.rb              |  2 +-
 spec/factories/attestation_template.rb        |  8 +--
 spec/factories/avis.rb                        |  2 +-
 spec/factories/cadastre.rb                    |  8 +--
 spec/factories/commentaire.rb                 |  2 +-
 spec/factories/deleted_dossier.rb             |  6 +-
 spec/factories/dossier.rb                     |  8 +--
 spec/factories/drop_down_list.rb              |  2 +-
 spec/factories/etablissement.rb               | 56 ++++++++--------
 spec/factories/exercice.rb                    |  6 +-
 spec/factories/feedback.rb                    |  2 +-
 spec/factories/france_connect_information.rb  | 10 +--
 spec/factories/gestionnaire.rb                |  2 +-
 spec/factories/individual.rb                  |  8 +--
 spec/factories/invite.rb                      |  2 +-
 spec/factories/invite_user.rb                 |  2 +-
 spec/factories/mail_templates.rb              |  8 +--
 spec/factories/module_api_carto.rb            | 22 +++----
 spec/factories/piece_justificative.rb         |  4 +-
 spec/factories/procedure.rb                   | 22 +++----
 spec/factories/procedure_path.rb              |  2 +-
 spec/factories/quartier_prioritaire.rb        |  8 +--
 spec/factories/service.rb                     | 14 ++--
 spec/factories/type_de_champ.rb               | 66 ++++++++++---------
 spec/factories/type_de_piece_justificative.rb | 16 ++---
 spec/factories/user.rb                        |  4 +-
 27 files changed, 149 insertions(+), 145 deletions(-)

diff --git a/spec/factories/administrateur.rb b/spec/factories/administrateur.rb
index be7cac11a..b936e069b 100644
--- a/spec/factories/administrateur.rb
+++ b/spec/factories/administrateur.rb
@@ -2,6 +2,6 @@ FactoryBot.define do
   sequence(:administrateur_email) { |n| "admin#{n}@admin.com" }
   factory :administrateur do
     email { generate(:administrateur_email) }
-    password 'password'
+    password { 'password' }
   end
 end
diff --git a/spec/factories/administration.rb b/spec/factories/administration.rb
index 5f55b5971..d7c7ed996 100644
--- a/spec/factories/administration.rb
+++ b/spec/factories/administration.rb
@@ -2,6 +2,6 @@ FactoryBot.define do
   sequence(:administration_email) { |n| "plop#{n}@plop.com" }
   factory :administration do
     email { generate(:administration_email) }
-    password 'password'
+    password { 'password' }
   end
 end
diff --git a/spec/factories/attestation_template.rb b/spec/factories/attestation_template.rb
index faefd5e5c..b39d6f5a5 100644
--- a/spec/factories/attestation_template.rb
+++ b/spec/factories/attestation_template.rb
@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :attestation_template do
-    title 'title'
-    body 'body'
-    footer 'footer'
-    activated true
+    title { 'title' }
+    body { 'body' }
+    footer { 'footer' }
+    activated { true }
   end
 end
diff --git a/spec/factories/avis.rb b/spec/factories/avis.rb
index 679b16605..97abb989b 100644
--- a/spec/factories/avis.rb
+++ b/spec/factories/avis.rb
@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :avis do
-    introduction 'Bonjour, merci de me donner votre avis sur ce dossier'
+    introduction { 'Bonjour, merci de me donner votre avis sur ce dossier' }
 
     before(:create) do |avis, _evaluator|
       if !avis.gestionnaire
diff --git a/spec/factories/cadastre.rb b/spec/factories/cadastre.rb
index fe6833b4e..49f21e5c8 100644
--- a/spec/factories/cadastre.rb
+++ b/spec/factories/cadastre.rb
@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :cadastre do
-    numero '001'
-    feuille 1
-    section 'OM'
-    geometry '{"type": "MultiPolygon", "coordinates": [[[[2.37112834276229, 48.8773116214902], [2.37163254350824, 48.8775780792784],  [2.37112834276229, 48.8773116214902]]]]}'
+    numero { '001' }
+    feuille { 1 }
+    section { 'OM' }
+    geometry { '{"type": "MultiPolygon", "coordinates": [[[[2.37112834276229, 48.8773116214902], [2.37163254350824, 48.8775780792784],  [2.37112834276229, 48.8773116214902]]]]}' }
   end
 end
diff --git a/spec/factories/commentaire.rb b/spec/factories/commentaire.rb
index c03091049..db2824699 100644
--- a/spec/factories/commentaire.rb
+++ b/spec/factories/commentaire.rb
@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :commentaire do
-    body 'plop'
+    body { 'plop' }
 
     before(:create) do |commentaire, _evaluator|
       if !commentaire.dossier
diff --git a/spec/factories/deleted_dossier.rb b/spec/factories/deleted_dossier.rb
index e476c64b2..9a972ae79 100644
--- a/spec/factories/deleted_dossier.rb
+++ b/spec/factories/deleted_dossier.rb
@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :deleted_dossier do
-    dossier_id  1111
-    state       Dossier.states.fetch(:en_construction)
-    deleted_at  DateTime.now
+    dossier_id  { 1111 }
+    state       { Dossier.states.fetch(:en_construction) }
+    deleted_at  { DateTime.now }
 
     association :procedure, :published
   end
diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb
index d2962ea4b..9cb453142 100644
--- a/spec/factories/dossier.rb
+++ b/spec/factories/dossier.rb
@@ -1,7 +1,7 @@
 FactoryBot.define do
   factory :dossier do
-    autorisation_donnees true
-    state Dossier.states.fetch(:brouillon)
+    autorisation_donnees { true }
+    state { Dossier.states.fetch(:brouillon) }
     association :user, factory: [:user]
 
     before(:create) do |dossier, _evaluator|
@@ -46,11 +46,11 @@ FactoryBot.define do
     end
 
     trait :archived do
-      archived true
+      archived { true }
     end
 
     trait :not_archived do
-      archived false
+      archived { false }
     end
 
     trait :with_dossier_link do
diff --git a/spec/factories/drop_down_list.rb b/spec/factories/drop_down_list.rb
index 9b7de1e7e..5cc47a7fa 100644
--- a/spec/factories/drop_down_list.rb
+++ b/spec/factories/drop_down_list.rb
@@ -1,5 +1,5 @@
 FactoryBot.define do
   factory :drop_down_list do
-    value "val1\r\nval2\r\n--separateur--\r\nval3"
+    value { "val1\r\nval2\r\n--separateur--\r\nval3" }
   end
 end
diff --git a/spec/factories/etablissement.rb b/spec/factories/etablissement.rb
index c39f86cf5..ef3f74fc2 100644
--- a/spec/factories/etablissement.rb
+++ b/spec/factories/etablissement.rb
@@ -1,36 +1,36 @@
 FactoryBot.define do
   factory :etablissement do
-    siret '44011762001530'
-    siege_social true
-    naf '4950Z'
-    libelle_naf 'Transports par conduites'
-    adresse "GRTGAZ\r IMMEUBLE BORA\r 6 RUE RAOUL NORDLING\r 92270 BOIS COLOMBES\r"
-    numero_voie '6'
-    type_voie 'RUE'
-    nom_voie 'RAOUL NORDLING'
-    complement_adresse 'IMMEUBLE BORA'
-    code_postal '92270'
-    localite 'BOIS COLOMBES'
-    code_insee_localite '92009'
+    siret { '44011762001530' }
+    siege_social { true }
+    naf { '4950Z' }
+    libelle_naf { 'Transports par conduites' }
+    adresse { "GRTGAZ\r IMMEUBLE BORA\r 6 RUE RAOUL NORDLING\r 92270 BOIS COLOMBES\r" }
+    numero_voie { '6' }
+    type_voie { 'RUE' }
+    nom_voie { 'RAOUL NORDLING' }
+    complement_adresse { 'IMMEUBLE BORA' }
+    code_postal { '92270' }
+    localite { 'BOIS COLOMBES' }
+    code_insee_localite { '92009' }
 
-    entreprise_siren '440117620'
-    entreprise_capital_social 537_100_000
-    entreprise_numero_tva_intracommunautaire 'FR27440117620'
-    entreprise_forme_juridique 'SA à conseil d\'administration (s.a.i.)'
-    entreprise_forme_juridique_code '5599'
-    entreprise_nom_commercial 'GRTGAZ'
-    entreprise_raison_sociale 'GRTGAZ'
-    entreprise_siret_siege_social '44011762001530'
-    entreprise_code_effectif_entreprise '51'
-    entreprise_date_creation "1990-04-24"
+    entreprise_siren { '440117620' }
+    entreprise_capital_social { 537_100_000 }
+    entreprise_numero_tva_intracommunautaire { 'FR27440117620' }
+    entreprise_forme_juridique { 'SA à conseil d\'administration (s.a.i.)' }
+    entreprise_forme_juridique_code { '5599' }
+    entreprise_nom_commercial { 'GRTGAZ' }
+    entreprise_raison_sociale { 'GRTGAZ' }
+    entreprise_siret_siege_social { '44011762001530' }
+    entreprise_code_effectif_entreprise { '51' }
+    entreprise_date_creation { "1990-04-24" }
   end
 
   trait :is_association do
-    association_rna "W072000535"
-    association_titre "ASSOCIATION POUR LA PROMOTION DE SPECTACLES AU CHATEAU DE ROCHEMAURE"
-    association_objet "mise en oeuvre et réalisation de spectacles au chateau de rochemaure"
-    association_date_creation "1990-04-24"
-    association_date_declaration "2014-11-28"
-    association_date_publication "1990-05-16"
+    association_rna { "W072000535" }
+    association_titre { "ASSOCIATION POUR LA PROMOTION DE SPECTACLES AU CHATEAU DE ROCHEMAURE" }
+    association_objet { "mise en oeuvre et réalisation de spectacles au chateau de rochemaure" }
+    association_date_creation { "1990-04-24" }
+    association_date_declaration { "2014-11-28" }
+    association_date_publication { "1990-05-16" }
   end
 end
diff --git a/spec/factories/exercice.rb b/spec/factories/exercice.rb
index fedc9ce47..ee0bb7fa3 100644
--- a/spec/factories/exercice.rb
+++ b/spec/factories/exercice.rb
@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :exercice do
-    ca '12345678'
-    date_fin_exercice "2014-12-30 23:00:00"
-    date_fin_exercice_timestamp 1419980400
+    ca { '12345678' }
+    date_fin_exercice { "2014-12-30 23:00:00" }
+    date_fin_exercice_timestamp { 1419980400 }
     association :etablissement, factory: [:etablissement]
   end
 end
diff --git a/spec/factories/feedback.rb b/spec/factories/feedback.rb
index d332d2eae..6c12442bc 100644
--- a/spec/factories/feedback.rb
+++ b/spec/factories/feedback.rb
@@ -1,5 +1,5 @@
 FactoryBot.define do
   factory :feedback do
-    rating Feedback.ratings.fetch(:happy)
+    rating { Feedback.ratings.fetch(:happy) }
   end
 end
diff --git a/spec/factories/france_connect_information.rb b/spec/factories/france_connect_information.rb
index 64f5ac533..2a398c09e 100644
--- a/spec/factories/france_connect_information.rb
+++ b/spec/factories/france_connect_information.rb
@@ -1,9 +1,9 @@
 FactoryBot.define do
   factory :france_connect_information do
-    given_name 'plop'
-    family_name 'plip'
-    birthdate '1976-02-24'
-    france_connect_particulier_id '1234567'
-    email_france_connect 'plip@octo.com'
+    given_name { 'plop' }
+    family_name { 'plip' }
+    birthdate { '1976-02-24' }
+    france_connect_particulier_id { '1234567' }
+    email_france_connect { 'plip@octo.com' }
   end
 end
diff --git a/spec/factories/gestionnaire.rb b/spec/factories/gestionnaire.rb
index 1595ce313..dc07d61e9 100644
--- a/spec/factories/gestionnaire.rb
+++ b/spec/factories/gestionnaire.rb
@@ -2,6 +2,6 @@ FactoryBot.define do
   sequence(:gestionnaire_email) { |n| "gest#{n}@gest.com" }
   factory :gestionnaire do
     email { generate(:gestionnaire_email) }
-    password 'password'
+    password { 'password' }
   end
 end
diff --git a/spec/factories/individual.rb b/spec/factories/individual.rb
index 75e0c4b19..dc7835e2e 100644
--- a/spec/factories/individual.rb
+++ b/spec/factories/individual.rb
@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :individual do
-    gender 'M.'
-    nom 'Julien'
-    prenom 'Xavier'
-    birthdate Date.new(1991, 11, 01)
+    gender { 'M.' }
+    nom { 'Julien' }
+    prenom { 'Xavier' }
+    birthdate { Date.new(1991, 11, 01) }
   end
 end
diff --git a/spec/factories/invite.rb b/spec/factories/invite.rb
index ec5142cc2..8d0ec0bac 100644
--- a/spec/factories/invite.rb
+++ b/spec/factories/invite.rb
@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :invite do
-    email 'plop@octo.com'
+    email { 'plop@octo.com' }
 
     after(:build) do |invite, _evaluator|
       if invite.dossier.nil?
diff --git a/spec/factories/invite_user.rb b/spec/factories/invite_user.rb
index e1cc5a6f6..eb9363240 100644
--- a/spec/factories/invite_user.rb
+++ b/spec/factories/invite_user.rb
@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :invite_user do
-    email 'plop@octo.com'
+    email { 'plop@octo.com' }
 
     after(:build) do |invite, _evaluator|
       if invite.dossier.nil?
diff --git a/spec/factories/mail_templates.rb b/spec/factories/mail_templates.rb
index 468b011f5..6e5c2190d 100644
--- a/spec/factories/mail_templates.rb
+++ b/spec/factories/mail_templates.rb
@@ -1,7 +1,7 @@
 FactoryBot.define do
   factory :closed_mail, class: Mails::ClosedMail do
-    subject "Subject, voila voila"
-    body "Blabla ceci est mon body"
+    subject { "Subject, voila voila" }
+    body { "Blabla ceci est mon body" }
 
     factory :received_mail, class: Mails::ReceivedMail
 
@@ -10,8 +10,8 @@ FactoryBot.define do
     factory :without_continuation_mail, class: Mails::WithoutContinuationMail
 
     factory :initiated_mail, class: Mails::InitiatedMail do
-      subject "[demarches-simplifiees.fr] Accusé de réception pour votre dossier nº --numéro du dossier--"
-      body "Votre administration vous confirme la bonne réception de votre dossier nº --numéro du dossier--"
+      subject { "[demarches-simplifiees.fr] Accusé de réception pour votre dossier nº --numéro du dossier--" }
+      body { "Votre administration vous confirme la bonne réception de votre dossier nº --numéro du dossier--" }
     end
   end
 end
diff --git a/spec/factories/module_api_carto.rb b/spec/factories/module_api_carto.rb
index d76698ee5..6a366c776 100644
--- a/spec/factories/module_api_carto.rb
+++ b/spec/factories/module_api_carto.rb
@@ -1,27 +1,27 @@
 FactoryBot.define do
   factory :module_api_carto do
-    use_api_carto false
-    quartiers_prioritaires false
-    cadastre false
+    use_api_carto { false }
+    quartiers_prioritaires { false }
+    cadastre { false }
 
     trait :with_api_carto do
-      use_api_carto true
+      use_api_carto { true }
     end
 
     trait :with_quartiers_prioritaires do
-      use_api_carto true
-      quartiers_prioritaires true
+      use_api_carto { true }
+      quartiers_prioritaires { true }
     end
 
     trait :with_cadastre do
-      use_api_carto true
-      cadastre true
+      use_api_carto { true }
+      cadastre { true }
     end
 
     trait :with_qp_and_cadastre do
-      use_api_carto true
-      quartiers_prioritaires true
-      cadastre true
+      use_api_carto { true }
+      quartiers_prioritaires { true }
+      cadastre { true }
     end
   end
 end
diff --git a/spec/factories/piece_justificative.rb b/spec/factories/piece_justificative.rb
index c15d09d18..e204bdba4 100644
--- a/spec/factories/piece_justificative.rb
+++ b/spec/factories/piece_justificative.rb
@@ -1,11 +1,11 @@
 FactoryBot.define do
   factory :piece_justificative do
     trait :rib do
-      content Rack::Test::UploadedFile.new("./spec/support/files/RIB.pdf", 'application/pdf')
+      content { Rack::Test::UploadedFile.new("./spec/support/files/RIB.pdf", 'application/pdf') }
     end
 
     trait :contrat do
-      content Rack::Test::UploadedFile.new("./spec/support/files/Contrat.pdf", 'application/pdf')
+      content { Rack::Test::UploadedFile.new("./spec/support/files/Contrat.pdf", 'application/pdf') }
     end
   end
 end
diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb
index b5944c502..182103e72 100644
--- a/spec/factories/procedure.rb
+++ b/spec/factories/procedure.rb
@@ -1,20 +1,20 @@
 FactoryBot.define do
   sequence(:published_path) { |n| "fake_path#{n}" }
   factory :procedure do
-    lien_demarche 'http://localhost'
+    lien_demarche { 'http://localhost' }
     sequence(:libelle) { |n| "Procedure #{n}" }
-    description "Demande de subvention à l'intention des associations"
-    organisation "Orga DINSIC"
-    direction "direction DINSIC"
-    cadre_juridique "un cadre juridique important"
-    published_at nil
+    description { "Demande de subvention à l'intention des associations" }
+    organisation { "Orga DINSIC" }
+    direction { "direction DINSIC" }
+    cadre_juridique { "un cadre juridique important" }
+    published_at { nil }
     administrateur { create(:administrateur) }
-    duree_conservation_dossiers_dans_ds 3
-    duree_conservation_dossiers_hors_ds 6
+    duree_conservation_dossiers_dans_ds { 3 }
+    duree_conservation_dossiers_hors_ds { 6 }
 
     factory :procedure_with_dossiers do
       transient do
-        dossiers_count 1
+        dossiers_count { 1 }
       end
 
       after(:build) do |procedure, _evaluator|
@@ -58,7 +58,7 @@ FactoryBot.define do
 
     trait :with_type_de_champ do
       transient do
-        types_de_champ_count 1
+        types_de_champ_count { 1 }
       end
 
       after(:build) do |procedure, evaluator|
@@ -72,7 +72,7 @@ FactoryBot.define do
 
     trait :with_type_de_champ_private do
       transient do
-        types_de_champ_private_count 1
+        types_de_champ_private_count { 1 }
       end
 
       after(:build) do |procedure, evaluator|
diff --git a/spec/factories/procedure_path.rb b/spec/factories/procedure_path.rb
index 651696899..bd05581d1 100644
--- a/spec/factories/procedure_path.rb
+++ b/spec/factories/procedure_path.rb
@@ -1,5 +1,5 @@
 FactoryBot.define do
   factory :procedure_path do
-    path 'fake_path'
+    path { 'fake_path' }
   end
 end
diff --git a/spec/factories/quartier_prioritaire.rb b/spec/factories/quartier_prioritaire.rb
index 3d1de0e59..617ff2f6a 100644
--- a/spec/factories/quartier_prioritaire.rb
+++ b/spec/factories/quartier_prioritaire.rb
@@ -1,8 +1,8 @@
 FactoryBot.define do
   factory :quartier_prioritaire do
-    code 'QPcode'
-    commune 'Paris'
-    nom 'Test des QP'
-    geometry '{"type": "MultiPolygon", "coordinates": [[[[2.37112834276229, 48.8773116214902], [2.37163254350824, 48.8775780792784],  [2.37112834276229, 48.8773116214902]]]]}'
+    code { 'QPcode' }
+    commune { 'Paris' }
+    nom { 'Test des QP' }
+    geometry { '{"type": "MultiPolygon", "coordinates": [[[[2.37112834276229, 48.8773116214902], [2.37163254350824, 48.8775780792784],  [2.37112834276229, 48.8773116214902]]]]}' }
   end
 end
diff --git a/spec/factories/service.rb b/spec/factories/service.rb
index 78867d00c..710721e7c 100644
--- a/spec/factories/service.rb
+++ b/spec/factories/service.rb
@@ -1,12 +1,12 @@
 FactoryBot.define do
   factory :service do
-    nom 'service'
-    organisme 'organisme'
-    type_organisme Service.type_organismes.fetch(:commune)
+    nom { 'service' }
+    organisme { 'organisme' }
+    type_organisme { Service.type_organismes.fetch(:commune) }
     administrateur { create(:administrateur) }
-    email 'email@toto.com'
-    telephone '1234'
-    horaires 'de 9 h à 18 h'
-    adresse 'adresse'
+    email { 'email@toto.com' }
+    telephone { '1234' }
+    horaires { 'de 9 h à 18 h' }
+    adresse { 'adresse' }
   end
 end
diff --git a/spec/factories/type_de_champ.rb b/spec/factories/type_de_champ.rb
index b0de3bc04..2c66117be 100644
--- a/spec/factories/type_de_champ.rb
+++ b/spec/factories/type_de_champ.rb
@@ -1,90 +1,94 @@
 FactoryBot.define do
   factory :type_de_champ, class: 'TypesDeChamp::TextTypeDeChamp' do
-    private false
+    private { false }
+
+    # Previous line is kept blank so that rubocop does not complain
     sequence(:libelle) { |n| "Libelle du champ #{n}" }
     sequence(:description) { |n| "description du champ #{n}" }
-    type_champ TypeDeChamp.type_champs.fetch(:text)
-    order_place 1
-    mandatory false
+    type_champ { TypeDeChamp.type_champs.fetch(:text) }
+    order_place { 1 }
+    mandatory { false }
 
     factory :type_de_champ_text, class: 'TypesDeChamp::TextTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:text)
+      type_champ { TypeDeChamp.type_champs.fetch(:text) }
     end
     factory :type_de_champ_textarea, class: 'TypesDeChamp::TextareaTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:textarea)
+      type_champ { TypeDeChamp.type_champs.fetch(:textarea) }
     end
     factory :type_de_champ_number, class: 'TypesDeChamp::NumberTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:number)
+      type_champ { TypeDeChamp.type_champs.fetch(:number) }
     end
     factory :type_de_champ_checkbox, class: 'TypesDeChamp::CheckboxTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:checkbox)
+      type_champ { TypeDeChamp.type_champs.fetch(:checkbox) }
     end
     factory :type_de_champ_civilite, class: 'TypesDeChamp::CiviliteTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:civilite)
+      type_champ { TypeDeChamp.type_champs.fetch(:civilite) }
     end
     factory :type_de_champ_email, class: 'TypesDeChamp::EmailTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:email)
+      type_champ { TypeDeChamp.type_champs.fetch(:email) }
     end
     factory :type_de_champ_phone, class: 'TypesDeChamp::PhoneTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:phone)
+      type_champ { TypeDeChamp.type_champs.fetch(:phone) }
     end
     factory :type_de_champ_address, class: 'TypesDeChamp::AddressTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:address)
+      type_champ { TypeDeChamp.type_champs.fetch(:address) }
     end
     factory :type_de_champ_yes_no, class: 'TypesDeChamp::YesNoTypeDeChamp' do
-      libelle 'Yes/no'
-      type_champ TypeDeChamp.type_champs.fetch(:yes_no)
+      libelle { 'Yes/no' }
+      type_champ { TypeDeChamp.type_champs.fetch(:yes_no) }
     end
     factory :type_de_champ_date, class: 'TypesDeChamp::DateTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:date)
+      type_champ { TypeDeChamp.type_champs.fetch(:date) }
     end
     factory :type_de_champ_datetime, class: 'TypesDeChamp::DatetimeTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:datetime)
+      type_champ { TypeDeChamp.type_champs.fetch(:datetime) }
     end
     factory :type_de_champ_drop_down_list, class: 'TypesDeChamp::DropDownListTypeDeChamp' do
-      libelle 'Menu déroulant'
-      type_champ TypeDeChamp.type_champs.fetch(:drop_down_list)
+      libelle { 'Menu déroulant' }
+      type_champ { TypeDeChamp.type_champs.fetch(:drop_down_list) }
       drop_down_list { create(:drop_down_list) }
     end
     factory :type_de_champ_multiple_drop_down_list, class: 'TypesDeChamp::MultipleDropDownListTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:multiple_drop_down_list)
+      type_champ { TypeDeChamp.type_champs.fetch(:multiple_drop_down_list) }
       drop_down_list { create(:drop_down_list) }
     end
     factory :type_de_champ_linked_drop_down_list, class: 'TypesDeChamp::LinkedDropDownListTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:linked_drop_down_list)
+      type_champ { TypeDeChamp.type_champs.fetch(:linked_drop_down_list) }
       drop_down_list { create(:drop_down_list) }
     end
     factory :type_de_champ_pays, class: 'TypesDeChamp::PaysTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:pays)
+      type_champ { TypeDeChamp.type_champs.fetch(:pays) }
     end
     factory :type_de_champ_regions, class: 'TypesDeChamp::RegionTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:regions)
+      type_champ { TypeDeChamp.type_champs.fetch(:regions) }
     end
     factory :type_de_champ_departements, class: 'TypesDeChamp::DepartementTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:departements)
+      type_champ { TypeDeChamp.type_champs.fetch(:departements) }
     end
     factory :type_de_champ_engagement, class: 'TypesDeChamp::EngagementTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:engagement)
+      type_champ { TypeDeChamp.type_champs.fetch(:engagement) }
     end
     factory :type_de_champ_header_section, class: 'TypesDeChamp::HeaderSectionTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:header_section)
+      type_champ { TypeDeChamp.type_champs.fetch(:header_section) }
     end
     factory :type_de_champ_explication, class: 'TypesDeChamp::ExplicationTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:explication)
+      type_champ { TypeDeChamp.type_champs.fetch(:explication) }
     end
     factory :type_de_champ_dossier_link, class: 'TypesDeChamp::DossierLinkTypeDeChamp' do
-      libelle 'Référence autre dossier'
-      type_champ TypeDeChamp.type_champs.fetch(:dossier_link)
+      libelle { 'Référence autre dossier' }
+      type_champ { TypeDeChamp.type_champs.fetch(:dossier_link) }
     end
     factory :type_de_champ_piece_justificative, class: 'TypesDeChamp::PieceJustificativeTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:piece_justificative)
+      type_champ { TypeDeChamp.type_champs.fetch(:piece_justificative) }
     end
     factory :type_de_champ_siret, class: 'TypesDeChamp::SiretTypeDeChamp' do
-      type_champ TypeDeChamp.type_champs.fetch(:siret)
+      type_champ { TypeDeChamp.type_champs.fetch(:siret) }
     end
 
     trait :private do
-      private true
+      private { true }
+
+      # Previous line is kept blank so that rubocop does not complain
       sequence(:libelle) { |n| "Libelle champ privé #{n}" }
       sequence(:description) { |n| "description du champ privé #{n}" }
     end
diff --git a/spec/factories/type_de_piece_justificative.rb b/spec/factories/type_de_piece_justificative.rb
index b461f700e..e9f700a95 100644
--- a/spec/factories/type_de_piece_justificative.rb
+++ b/spec/factories/type_de_piece_justificative.rb
@@ -1,18 +1,18 @@
 FactoryBot.define do
   factory :type_de_piece_justificative do
-    libelle 'RIB'
-    description 'Releve identité bancaire'
+    libelle { 'RIB' }
+    description { 'Releve identité bancaire' }
 
     trait :rib do
-      libelle 'RIB'
-      description 'Releve identité bancaire'
-      api_entreprise false
+      libelle { 'RIB' }
+      description { 'Releve identité bancaire' }
+      api_entreprise { false }
     end
 
     trait :msa do
-      libelle 'Attestation MSA'
-      description 'recuperation automatique'
-      api_entreprise true
+      libelle { 'Attestation MSA' }
+      description { 'recuperation automatique' }
+      api_entreprise { true }
     end
   end
 end
diff --git a/spec/factories/user.rb b/spec/factories/user.rb
index 28b072108..f39fc3e43 100644
--- a/spec/factories/user.rb
+++ b/spec/factories/user.rb
@@ -2,7 +2,7 @@ FactoryBot.define do
   sequence(:user_email) { |n| "user#{n}@user.com" }
   factory :user do
     email { generate(:user_email) }
-    password 'password'
-    confirmed_at DateTime.now
+    password { 'password' }
+    confirmed_at { DateTime.now }
   end
 end

From 750e1e0c833d1cadac6f5b368f6bb26b0ac9ef68 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Tue, 4 Sep 2018 16:19:29 +0000
Subject: [PATCH 2/2] gestionnaire: move commentaire creation into a service

---
 .../new_gestionnaire/dossiers_controller.rb   | 10 +---
 app/services/commentaire_service.rb           | 17 +++++++
 .../dossiers_controller_spec.rb               | 38 +++++----------
 spec/services/commentaire_service_spec.rb     | 48 +++++++++++++++++++
 4 files changed, 79 insertions(+), 34 deletions(-)
 create mode 100644 app/services/commentaire_service.rb
 create mode 100644 spec/services/commentaire_service_spec.rb

diff --git a/app/controllers/new_gestionnaire/dossiers_controller.rb b/app/controllers/new_gestionnaire/dossiers_controller.rb
index c7e11d740..b23359872 100644
--- a/app/controllers/new_gestionnaire/dossiers_controller.rb
+++ b/app/controllers/new_gestionnaire/dossiers_controller.rb
@@ -113,15 +113,7 @@ module NewGestionnaire
     end
 
     def create_commentaire
-      commentaire_hash = commentaire_params.merge(email: current_gestionnaire.email, dossier: dossier)
-
-      # avoid simple_format replacing '' by '<p></p>'
-      # and thus skipping the not empty constraint on commentaire's body
-      if commentaire_hash[:body].present?
-        commentaire_hash[:body] = simple_format(commentaire_hash[:body])
-      end
-
-      @commentaire = Commentaire.new(commentaire_hash)
+      @commentaire = CommentaireService.create(current_gestionnaire, dossier, commentaire_params)
 
       if @commentaire.save
         current_gestionnaire.follow(dossier)
diff --git a/app/services/commentaire_service.rb b/app/services/commentaire_service.rb
new file mode 100644
index 000000000..c70ddf815
--- /dev/null
+++ b/app/services/commentaire_service.rb
@@ -0,0 +1,17 @@
+class CommentaireService
+  class << self
+    def create(sender, dossier, params)
+      attributes = params.merge(email: sender.email, dossier: dossier)
+
+      # If the user submits a empty message, simple_format will replace '' by '<p></p>',
+      # and thus bypass the not-empty constraint on commentaire's body.
+      #
+      # To avoid this, format the message only if a body is present in the first place.
+      if attributes[:body].present?
+        attributes[:body] = ActionController::Base.helpers.simple_format(attributes[:body])
+      end
+
+      Commentaire.new(attributes)
+    end
+  end
+end
diff --git a/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb b/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb
index 87a5eb02a..570a63cb9 100644
--- a/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb
+++ b/spec/controllers/new_gestionnaire/dossiers_controller_spec.rb
@@ -269,7 +269,7 @@ describe NewGestionnaire::DossiersController, type: :controller do
   describe "#create_commentaire" do
     let(:saved_commentaire) { dossier.commentaires.first }
     let(:body) { "avant\napres" }
-    let(:file) { nil }
+    let(:file) { Rack::Test::UploadedFile.new("./spec/support/files/piece_justificative_0.pdf", 'application/pdf') }
     let(:scan_result) { true }
 
     subject {
@@ -287,35 +287,23 @@ describe NewGestionnaire::DossiersController, type: :controller do
       allow(ClamavService).to receive(:safe_file?).and_return(scan_result)
     end
 
-    it do
-      subject
-
-      expect(saved_commentaire.body).to eq("<p>avant\n<br />apres</p>")
-      expect(saved_commentaire.email).to eq(gestionnaire.email)
-      expect(saved_commentaire.dossier).to eq(dossier)
-      expect(response).to redirect_to(messagerie_gestionnaire_dossier_path(dossier.procedure, dossier))
+    it "creates a commentaire" do
+      expect { subject }.to change(Commentaire, :count).by(1)
       expect(gestionnaire.followed_dossiers).to include(dossier)
-      expect(saved_commentaire.file.present?).to eq(false)
+
+      expect(response).to redirect_to(messagerie_gestionnaire_dossier_path(dossier.procedure, dossier))
+      expect(flash.notice).to be_present
     end
 
-    it { expect { subject }.to change(Commentaire, :count).by(1) }
+    context "when the commentaire creation fails" do
+      let(:scan_result) { false }
 
-    context "without a body" do
-      let(:body) { nil }
+      it "renders the messagerie page with the invalid commentaire" do
+        expect { subject }.not_to change(Commentaire, :count)
 
-      it { expect { subject }.not_to change(Commentaire, :count) }
-    end
-
-    context "with a file" do
-      let(:file) { Rack::Test::UploadedFile.new("./spec/support/files/piece_justificative_0.pdf", 'application/pdf') }
-
-      it { subject; expect(saved_commentaire.file.present?).to eq(true) }
-      it { expect { subject }.to change(Commentaire, :count).by(1) }
-
-      context "and a virus" do
-        let(:scan_result) { false }
-
-        it { expect { subject }.not_to change(Commentaire, :count) }
+        expect(response).to render_template :messagerie
+        expect(flash.alert).to be_present
+        expect(assigns(:commentaire).body).to eq("<p>avant\n<br />apres</p>")
       end
     end
   end
diff --git a/spec/services/commentaire_service_spec.rb b/spec/services/commentaire_service_spec.rb
new file mode 100644
index 000000000..7029d0c47
--- /dev/null
+++ b/spec/services/commentaire_service_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe CommentaireService do
+  describe '.create' do
+    let(:dossier) { create :dossier }
+    let(:sender) { dossier.user }
+    let(:body) { 'Contenu du message.' }
+    let(:file) { nil }
+    let(:scan_result) { true }
+
+    subject(:commentaire) { CommentaireService.create(sender, dossier, { body: body, file: file }) }
+
+    before do
+      allow(ClamavService).to receive(:safe_file?).and_return(scan_result)
+    end
+
+    it 'creates a new valid commentaire' do
+      expect(commentaire.email).to eq sender.email
+      expect(commentaire.dossier).to eq dossier
+      expect(commentaire.body).to eq '<p>Contenu du message.</p>'
+      expect(commentaire.file).to be_blank
+      expect(commentaire).to be_valid
+    end
+
+    context 'when the body is empty' do
+      let(:body) { nil }
+
+      it 'creates an invalid comment' do
+        expect(commentaire.body).to be nil
+        expect(commentaire.valid?).to be false
+      end
+    end
+
+    context 'when it has a file' do
+      let(:file) { Rack::Test::UploadedFile.new("./spec/support/files/piece_justificative_0.pdf", 'application/pdf') }
+
+      it 'saves the attached file' do
+        expect(commentaire.file).to be_present
+        expect(commentaire).to be_valid
+      end
+
+      context 'and a virus' do
+        let(:scan_result) { false }
+        it { expect(commentaire).not_to be_valid }
+      end
+    end
+  end
+end