From d409b6f4eff697ffbe3bf8345dca662384a6a10b Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Tue, 11 Oct 2022 12:21:06 +0200
Subject: [PATCH] feat(dossier): do not save dossier on submit

---
 app/controllers/users/dossiers_controller.rb  | 104 ++++---
 config/routes.rb                              |   1 +
 .../users/dossiers_controller_spec.rb         | 286 +++++++-----------
 3 files changed, 162 insertions(+), 229 deletions(-)

diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb
index ea01bba6f..6c36aa7be 100644
--- a/app/controllers/users/dossiers_controller.rb
+++ b/app/controllers/users/dossiers_controller.rb
@@ -5,13 +5,13 @@ module Users
     layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret]
 
     ACTIONS_ALLOWED_TO_ANY_USER = [:index, :recherche, :new, :transferer_all]
-    ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire, :papertrail, :restore]
+    ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :submit_brouillon, :modifier, :update, :create_commentaire, :papertrail, :restore]
 
     before_action :ensure_ownership!, except: ACTIONS_ALLOWED_TO_ANY_USER + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE
     before_action :ensure_ownership_or_invitation!, only: ACTIONS_ALLOWED_TO_OWNER_OR_INVITE
-    before_action :ensure_dossier_can_be_updated, only: [:update_identite, :update_brouillon, :modifier, :update]
-    before_action :forbid_invite_submission!, only: [:update_brouillon]
-    before_action :forbid_closed_submission!, only: [:update_brouillon]
+    before_action :ensure_dossier_can_be_updated, only: [:update_identite, :update_brouillon, :submit_brouillon, :modifier, :update]
+    before_action :forbid_invite_submission!, only: [:submit_brouillon]
+    before_action :forbid_closed_submission!, only: [:submit_brouillon]
     before_action :show_demarche_en_test_banner
     before_action :store_user_location!, only: :new
 
@@ -159,35 +159,24 @@ module Users
       end
     end
 
-    # FIXME:
-    # - delegate draft save logic to champ ?
-    def update_brouillon
+    def submit_brouillon
       @dossier = dossier_with_champs
+      errors = submit_dossier_and_compute_errors
 
-      errors = update_dossier_and_compute_errors
-
-      if passage_en_construction? && errors.blank?
+      if errors.blank?
         @dossier.passer_en_construction!
         NotificationMailer.send_en_construction_notification(@dossier).deliver_later
         @dossier.groupe_instructeur.instructeurs.with_instant_email_dossier_notifications.each do |instructeur|
           DossierMailer.notify_new_dossier_depose_to_instructeur(@dossier, instructeur.email).deliver_later
         end
-        return redirect_to(merci_dossier_path(@dossier))
-      elsif errors.present?
-        flash.now.alert = errors
+
+        redirect_to merci_dossier_path(@dossier)
       else
-        flash.now.notice = t('.draft_saved')
-      end
+        flash.now.alert = errors
 
-      respond_to do |format|
-        format.html { render :brouillon }
-        format.turbo_stream do
-          @to_shows, @to_hides = @dossier.champs
-            .filter(&:conditional?)
-            .partition(&:visible?)
-            .map { |champs| champs_to_one_selector(champs) }
-
-          render(:update, layout: false)
+        respond_to do |format|
+          format.html { render :brouillon }
+          format.turbo_stream
         end
       end
     end
@@ -202,6 +191,23 @@ module Users
       @dossier = dossier_with_champs
     end
 
+    def update_brouillon
+      @dossier = dossier_with_champs
+      update_dossier_and_compute_errors
+
+      respond_to do |format|
+        format.html { render :brouillon }
+        format.turbo_stream do
+          @to_shows, @to_hides = @dossier.champs
+            .filter(&:conditional?)
+            .partition(&:visible?)
+            .map { |champs| champs_to_one_selector(champs) }
+
+          render(:update, layout: false)
+        end
+      end
+    end
+
     def update
       @dossier = dossier_with_champs
       errors = update_dossier_and_compute_errors
@@ -217,8 +223,6 @@ module Users
             .filter(&:conditional?)
             .partition(&:visible?)
             .map { |champs| champs_to_one_selector(champs) }
-
-          render layout: false
         end
       end
     end
@@ -450,21 +454,33 @@ module Users
         end
         if !@dossier.save(**validation_options)
           errors += @dossier.errors.full_messages
-        elsif should_change_groupe_instructeur?
+        end
+
+        if should_change_groupe_instructeur?
           @dossier.assign_to_groupe_instructeur(groupe_instructeur_from_params)
         end
       end
 
+      if dossier.en_construction?
+        errors += @dossier.check_mandatory_champs
+      end
+
+      errors
+    end
+
+    def submit_dossier_and_compute_errors
+      errors = []
+
+      @dossier.valid?(**submit_validation_options)
+      errors += @dossier.errors.full_messages
+      errors += @dossier.check_mandatory_champs
+
       if should_fill_groupe_instructeur?
         @dossier.assign_to_groupe_instructeur(defaut_groupe_instructeur)
       end
 
-      if !save_draft?
-        errors += @dossier.check_mandatory_champs
-
-        if @dossier.groupe_instructeur.nil?
-          errors << "Le champ « #{@dossier.procedure.routing_criteria_name} » doit être rempli"
-        end
+      if @dossier.groupe_instructeur.nil?
+        errors << "Le champ « #{@dossier.procedure.routing_criteria_name} » doit être rempli"
       end
 
       errors
@@ -483,13 +499,13 @@ module Users
     end
 
     def forbid_invite_submission!
-      if passage_en_construction? && !current_user.owns?(dossier)
+      if !current_user.owns?(dossier)
         forbidden!
       end
     end
 
     def forbid_closed_submission!
-      if passage_en_construction? && !dossier.can_transition_to_en_construction?
+      if !dossier.can_transition_to_en_construction?
         forbidden!
       end
     end
@@ -516,22 +532,18 @@ module Users
       params.require(:commentaire).permit(:body, :piece_jointe)
     end
 
-    def passage_en_construction?
-      dossier.brouillon? && !save_draft?
-    end
-
-    def save_draft?
-      dossier.brouillon? && !params[:submit_draft]
+    def submit_validation_options
+      # rubocop:disable Lint/BooleanSymbol
+      # Force ActiveRecord to re-validate associated records.
+      { context: :false }
+      # rubocop:enable Lint/BooleanSymbol
     end
 
     def validation_options
-      if save_draft?
+      if dossier.brouillon?
         { context: :brouillon }
       else
-        # rubocop:disable Lint/BooleanSymbol
-        # Force ActiveRecord to re-validate associated records.
-        { context: :false }
-        # rubocop:enable Lint/BooleanSymbol
+        submit_validation_options
       end
     end
 
diff --git a/config/routes.rb b/config/routes.rb
index 545db36a7..ed05b5cb7 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -276,6 +276,7 @@ Rails.application.routes.draw do
         get 'etablissement'
         get 'brouillon'
         patch 'brouillon', to: 'dossiers#update_brouillon'
+        post 'brouillon', to: 'dossiers#submit_brouillon'
         get 'modifier', to: 'dossiers#modifier'
         patch 'modifier', to: 'dossiers#update'
         get 'merci'
diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb
index ab2ffac25..a2604b4fb 100644
--- a/spec/controllers/users/dossiers_controller_spec.rb
+++ b/spec/controllers/users/dossiers_controller_spec.rb
@@ -98,38 +98,21 @@ describe Users::DossiersController, type: :controller do
     let(:user) { create(:user) }
     let(:asked_dossier) { create(:dossier) }
     let(:ensure_authorized) { :forbid_invite_submission! }
-    let(:submit) { true }
 
     before do
-      @controller.params = @controller.params.merge(dossier_id: asked_dossier.id, submit_draft: submit)
+      @controller.params = @controller.params.merge(dossier_id: asked_dossier.id)
       allow(@controller).to receive(:current_user).and_return(user)
       allow(@controller).to receive(:redirect_to)
     end
 
-    context 'when a user save their own draft' do
-      let(:asked_dossier) { create(:dossier, user: user) }
-      let(:submit) { false }
-
-      it_behaves_like 'does not redirect nor flash'
-    end
-
     context 'when a user submit their own dossier' do
       let(:asked_dossier) { create(:dossier, user: user) }
-      let(:submit) { true }
-
-      it_behaves_like 'does not redirect nor flash'
-    end
-
-    context 'when an invite save the draft for a dossier where they where invited' do
-      before { create(:invite, dossier: asked_dossier, user: user) }
-      let(:submit) { false }
 
       it_behaves_like 'does not redirect nor flash'
     end
 
     context 'when an invite submit a dossier where they where invited' do
       before { create(:invite, dossier: asked_dossier, user: user) }
-      let(:submit) { true }
 
       it_behaves_like 'redirects and flashes'
     end
@@ -355,30 +338,18 @@ describe Users::DossiersController, type: :controller do
     end
   end
 
-  describe '#update_brouillon' do
+  describe '#submit_brouillon' do
     before { sign_in(user) }
 
     let!(:dossier) { create(:dossier, user: user) }
     let(:first_champ) { dossier.champs.first }
     let(:value) { 'beautiful value' }
     let(:now) { Time.zone.parse('01/01/2100') }
-    let(:submit_payload) do
-      {
-        id: dossier.id,
-        dossier: {
-          groupe_instructeur_id: dossier.groupe_instructeur_id,
-          champs_attributes: {
-            id: first_champ.id,
-            value: value
-          }
-        }
-      }
-    end
-    let(:payload) { submit_payload.merge(submit_draft: true) }
+    let(:payload) { { id: dossier.id } }
 
     subject do
       Timecop.freeze(now) do
-        patch :update_brouillon, params: payload
+        post :submit_brouillon, params: payload
       end
     end
 
@@ -393,120 +364,6 @@ describe Users::DossiersController, type: :controller do
       end
     end
 
-    context 'when dossier can be updated by the owner' do
-      it 'updates the champs' do
-        subject
-
-        expect(response).to redirect_to(merci_dossier_path(dossier))
-        expect(first_champ.reload.value).to eq('beautiful value')
-        expect(dossier.reload.updated_at.year).to eq(2100)
-        expect(dossier.reload.state).to eq(Dossier.states.fetch(:en_construction))
-      end
-
-      context 'without new values for champs' do
-        let(:submit_payload) do
-          {
-            id: dossier.id,
-            dossier: {
-              champs_attributes: {}
-            }
-          }
-        end
-
-        it "doesn't set last_champ_updated_at" do
-          subject
-          expect(dossier.reload.last_champ_updated_at).to eq(nil)
-        end
-      end
-
-      context 'with instructeurs ok to be notified instantly' do
-        let!(:instructeur_with_instant_email_dossier) { create(:instructeur) }
-        let!(:instructeur_without_instant_email_dossier) { create(:instructeur) }
-
-        before do
-          allow(DossierMailer).to receive(:notify_new_dossier_depose_to_instructeur).and_return(double(deliver_later: nil))
-          create(:assign_to, instructeur: instructeur_with_instant_email_dossier, procedure: dossier.procedure, instant_email_dossier_notifications_enabled: true)
-          create(:assign_to, instructeur: instructeur_without_instant_email_dossier, procedure: dossier.procedure, instant_email_dossier_notifications_enabled: false)
-        end
-
-        it "sends notification mail to instructeurs" do
-          subject
-
-          expect(DossierMailer).to have_received(:notify_new_dossier_depose_to_instructeur).once.with(dossier, instructeur_with_instant_email_dossier.email)
-          expect(DossierMailer).not_to have_received(:notify_new_dossier_depose_to_instructeur).with(dossier, instructeur_without_instant_email_dossier.email)
-        end
-      end
-
-      context 'with procedure routee' do
-        let(:procedure) { create(:procedure, :routee, :with_type_de_champ) }
-        let(:dossier_group) { create(:groupe_instructeur, procedure: procedure) }
-        let(:another_group) { create(:groupe_instructeur, procedure: procedure) }
-        let(:instructeur_of_dossier) { create(:instructeur) }
-        let(:instructeur_in_another_group) { create(:instructeur) }
-
-        context "and grope instructeur is set" do
-          let!(:dossier) { create(:dossier, groupe_instructeur: dossier_group, user: user) }
-
-          before do
-            allow(DossierMailer).to receive(:notify_new_dossier_depose_to_instructeur).and_return(double(deliver_later: nil))
-            create(:assign_to, instructeur: instructeur_of_dossier, procedure: dossier.procedure, instant_email_dossier_notifications_enabled: true, groupe_instructeur: dossier_group)
-            create(:assign_to, instructeur: instructeur_in_another_group, procedure: dossier.procedure, instant_email_dossier_notifications_enabled: true, groupe_instructeur: another_group)
-          end
-
-          it "sends notification mail to instructeurs in the correct group instructeur" do
-            subject
-
-            expect(DossierMailer).to have_received(:notify_new_dossier_depose_to_instructeur).once.with(dossier, instructeur_of_dossier.email)
-            expect(DossierMailer).not_to have_received(:notify_new_dossier_depose_to_instructeur).with(dossier, instructeur_in_another_group.email)
-          end
-        end
-
-        context "and groupe instructeur is not set" do
-          let(:dossier) { create(:dossier, procedure: procedure, user: user) }
-          let(:submit_payload) do
-            {
-              id: dossier.id,
-              dossier: {
-                champs_attributes: {
-                  id: first_champ.id,
-                  value: value
-                }
-              },
-              submit_draft: false
-            }
-          end
-
-          it "can not submit" do
-            subject
-
-            expect(flash.alert).to eq(['Le champ « Votre ville » doit être rempli'])
-          end
-        end
-      end
-
-      context "when the dossier was created on a routee procedure, but routage was later disabled" do
-        let(:dossier) { create(:dossier, groupe_instructeur: nil, user: user) }
-
-        it "sets a default groupe_instructeur" do
-          subject
-
-          expect(response).to redirect_to(merci_dossier_path(dossier))
-          expect(dossier.reload.groupe_instructeur).to eq(dossier.procedure.defaut_groupe_instructeur)
-        end
-      end
-
-      context "on an closed procedure" do
-        before { dossier.procedure.close! }
-
-        it "it does not change state" do
-          subject
-
-          expect(response).not_to redirect_to(merci_dossier_path(dossier))
-          expect(dossier.reload.state).to eq(Dossier.states.fetch(:brouillon))
-        end
-      end
-    end
-
     it 'sends an email only on the first #update_brouillon' do
       delivery = double
       expect(delivery).to receive(:deliver_later).with(no_args)
@@ -523,9 +380,9 @@ describe Users::DossiersController, type: :controller do
 
     context 'when the update fails' do
       before do
-        expect_any_instance_of(Dossier).to receive(:save).and_return(false)
+        expect_any_instance_of(Dossier).to receive(:valid?).and_return(false)
         expect_any_instance_of(Dossier).to receive(:errors)
-          .and_return(double(full_messages: ['nop']))
+          .and_return(double('errors', full_messages: ['nop']))
 
         subject
       end
@@ -550,21 +407,6 @@ describe Users::DossiersController, type: :controller do
 
       it { expect(response).to render_template(:brouillon) }
       it { expect(flash.alert).to eq(['Le champ l doit être rempli.']) }
-
-      context 'and the user saves a draft' do
-        let(:payload) { submit_payload.except(:submit_draft) }
-
-        it { expect(response).to render_template(:brouillon) }
-        it { expect(flash.notice).to eq('Votre brouillon a bien été sauvegardé.') }
-        it { expect(dossier.reload.state).to eq(Dossier.states.fetch(:brouillon)) }
-
-        context 'and the dossier is in construction' do
-          let!(:dossier) { create(:dossier, :en_construction, user: user) }
-
-          it { expect(response).to render_template(:brouillon) }
-          it { expect(flash.alert).to eq(['Le champ l doit être rempli.']) }
-        end
-      end
     end
 
     context 'when dossier has no champ' do
@@ -581,19 +423,6 @@ describe Users::DossiersController, type: :controller do
       let(:dossier) { create(:dossier) }
       let!(:invite) { create(:invite, dossier: dossier, user: user) }
 
-      context 'and the invite saves a draft' do
-        let(:payload) { submit_payload.except(:submit_draft) }
-
-        before do
-          first_champ.type_de_champ.update(mandatory: true, libelle: 'l')
-          subject
-        end
-
-        it { expect(response).to render_template(:brouillon) }
-        it { expect(flash.notice).to eq('Votre brouillon a bien été sauvegardé.') }
-        it { expect(dossier.reload.state).to eq(Dossier.states.fetch(:brouillon)) }
-      end
-
       context 'and the invite tries to submit the dossier' do
         before { subject }
 
@@ -603,6 +432,101 @@ describe Users::DossiersController, type: :controller do
     end
   end
 
+  describe '#update_brouillon' do
+    before { sign_in(user) }
+
+    let(:procedure) { create(:procedure, :published, :with_type_de_champ, :with_piece_justificative) }
+    let!(:dossier) { create(:dossier, user: user, procedure: procedure) }
+    let(:first_champ) { dossier.champs.first }
+    let(:piece_justificative_champ) { dossier.champs.last }
+    let(:value) { 'beautiful value' }
+    let(:file) { fixture_file_upload('spec/fixtures/files/piece_justificative_0.pdf', 'application/pdf') }
+    let(:now) { Time.zone.parse('01/01/2100') }
+
+    let(:submit_payload) do
+      {
+        id: dossier.id,
+        dossier: {
+          groupe_instructeur_id: dossier.groupe_instructeur_id,
+          champs_attributes: [
+            {
+              id: first_champ.id,
+              value: value
+            },
+            {
+              id: piece_justificative_champ.id,
+              piece_justificative_file: file
+            }
+          ]
+        }
+      }
+    end
+    let(:payload) { submit_payload }
+
+    subject do
+      Timecop.freeze(now) do
+        patch :update_brouillon, params: payload
+      end
+    end
+
+    context 'when the dossier cannot be updated by the user' do
+      let!(:dossier) { create(:dossier, :en_instruction, user: user) }
+
+      it 'redirects to the dossiers list' do
+        subject
+
+        expect(response).to redirect_to(dossiers_path)
+        expect(flash.alert).to eq('Votre dossier ne peut plus être modifié')
+      end
+    end
+
+    context 'when dossier can be updated by the owner' do
+      it 'updates the champs' do
+        subject
+
+        expect(response).to have_http_status(:ok)
+        expect(dossier.reload.updated_at.year).to eq(2100)
+        expect(dossier.reload.state).to eq(Dossier.states.fetch(:brouillon))
+      end
+
+      context 'without new values for champs' do
+        let(:submit_payload) do
+          {
+            id: dossier.id,
+            dossier: {
+              champs_attributes: {}
+            }
+          }
+        end
+
+        it "doesn't set last_champ_updated_at" do
+          subject
+          expect(dossier.reload.last_champ_updated_at).to eq(nil)
+        end
+      end
+    end
+
+    context 'when dossier has no champ' do
+      let(:submit_payload) { { id: dossier.id } }
+
+      it 'does not raise any errors' do
+        subject
+
+        expect(response).to have_http_status(:ok)
+      end
+    end
+
+    context 'when the user has an invitation but is not the owner' do
+      let(:dossier) { create(:dossier) }
+      let!(:invite) { create(:invite, dossier: dossier, user: user) }
+
+      before { subject }
+
+      it { expect(first_champ.reload.value).to eq('beautiful value') }
+      it { expect(response).to have_http_status(:ok) }
+    end
+  end
+
   describe '#update' do
     before { sign_in(user) }
 
@@ -751,16 +675,12 @@ describe Users::DossiersController, type: :controller do
     end
 
     context 'when the user has an invitation but is not the owner' do
-      let(:dossier) { create(:dossier) }
+      let(:dossier) { create(:dossier, :en_construction) }
       let!(:invite) { create(:invite, dossier: dossier, user: user) }
 
-      before do
-        dossier.passer_en_construction!
-        subject
-      end
+      before { subject }
 
       it { expect(first_champ.reload.value).to eq('beautiful value') }
-      it { expect(dossier.reload.state).to eq(Dossier.states.fetch(:en_construction)) }
       it { expect(response).to have_http_status(:ok) }
     end