Merge pull request #10515 from colinux/contact-form-async

ETQ opérateur, le form de contact devient asynchrone et l'antivirus passe sur les PJ

app/controllers/support_controller.rb

@@ -14,24 +14,16 @@ class SupportController < ApplicationController
       flash.notice = "Votre message a été envoyé sur la messagerie de votre dossier."
 
       redirect_to messagerie_dossier_path(dossier)
-    elsif create_conversation
-      flash.notice = "Votre message a été envoyé."
+      return
+    end
 
-      if params[:admin]
-        redirect_to root_path(formulaire_contact_admin_submitted: true)
-      else
-        redirect_to root_path(formulaire_contact_general_submitted: true)
-      end
+    create_conversation_later
+    flash.notice = "Votre message a été envoyé."
+
+    if params[:admin]
+      redirect_to root_path(formulaire_contact_admin_submitted: true)
     else
-      flash.now.alert = "Une erreur est survenue. Vous pouvez nous contacter à #{helpers.mail_to(Current.contact_email)}."
-
-      if params[:admin]
-        setup_context_admin
-        render :admin
-      else
-        setup_context
-        render :index
-      end
+      redirect_to root_path(formulaire_contact_general_submitted: true)
     end
   end
 
@@ -48,17 +40,26 @@ class SupportController < ApplicationController
     @options = Helpscout::FormAdapter.admin_options
   end
 
-  def create_conversation
-    Helpscout::FormAdapter.new(
+  def create_conversation_later
+    if params[:piece_jointe]
+      blob = ActiveStorage::Blob.create_and_upload!(
+        io: params[:piece_jointe].tempfile,
+        filename: params[:piece_jointe].original_filename,
+        content_type: params[:piece_jointe].content_type,
+        identify: false
+      ).tap(&:scan_for_virus_later)
+    end
+
+    HelpscoutCreateConversationJob.perform_later(
+      blob_id: blob&.id,
       subject: params[:subject],
       email: email,
       phone: params[:phone],
       text: params[:text],
-      file: params[:piece_jointe],
       dossier_id: dossier&.id,
       browser: browser_name,
       tags: tags
-    ).send_form
+    )
   end
 
   def create_commentaire

app/jobs/helpscout_create_conversation_job.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+class HelpscoutCreateConversationJob < ApplicationJob
+  queue_as :default
+
+  class FileNotScannedYetError < StandardError
+  end
+
+  retry_on FileNotScannedYetError, wait: :exponentially_longer, attempts: 10
+
+  def perform(blob_id: nil, **args)
+    if blob_id.present?
+      blob = ActiveStorage::Blob.find(blob_id)
+      raise FileNotScannedYetError if blob.virus_scanner.pending?
+
+      blob = nil unless blob.virus_scanner.safe?
+    end
+
+    Helpscout::FormAdapter.new(**args, blob:).send_form
+  end
+end

app/lib/helpscout/api.rb

@@ -22,7 +22,7 @@ class Helpscout::API
     })
   end
 
-  def create_conversation(email, subject, text, file)
+  def create_conversation(email, subject, text, blob)
     body = {
       subject: subject,
       customer: customer(email),
@@ -34,7 +34,7 @@ class Helpscout::API
           type: 'customer',
           customer: customer(email),
           text: text,
-          attachments: attachments(file)
+          attachments: attachments(blob)
         }
       ]
     }.compact
@@ -76,13 +76,13 @@ class Helpscout::API
 
   private
 
-  def attachments(file)
-    if file.present?
+  def attachments(blob)
+    if blob.present?
       [
         {
-          fileName: file.original_filename,
-          mimeType: file.content_type,
-          data: Base64.strict_encode64(file.read)
+          fileName: blob.filename,
+          mimeType: blob.content_type,
+          data: Base64.strict_encode64(blob.download)
         }
       ]
     else

app/lib/helpscout/form_adapter.rb

@@ -66,7 +66,7 @@ class Helpscout::FormAdapter
       params[:email],
       params[:subject],
       params[:text],
-      params[:file]
+      params[:blob]
     )
 
     if response.success?
@@ -74,6 +74,8 @@ class Helpscout::FormAdapter
         @api.add_phone_number(params[:email], params[:phone])
       end
       response.headers['Resource-ID']
+    else
+      raise StandardError, "Error while creating conversation: #{response.response_code} '#{response.body}'"
     end
   end
 end

config/initializers/transition_to_sidekiq.rb

@@ -1,99 +1,33 @@
 if Rails.env.production? && SIDEKIQ_ENABLED
   ActiveSupport.on_load(:after_initialize) do
-    class ActiveStorage::PurgeJob < ActiveStorage::BaseJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ActiveStorage::AnalyzeJob < ActiveStorage::BaseJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class VirusScannerJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class DossierRebaseJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ProcedureExternalURLCheckJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class MaintenanceTasks::TaskJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class PriorizedMailDeliveryJob < ActionMailer::MailDeliveryJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ProcedureSVASVRProcessDossierJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class WebHookJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class DestroyRecordLaterJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ChampFetchExternalDataJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class DossierIndexSearchTermsJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class Migrations::BackfillStableIdJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class Cron::CronJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class APIEntreprise::Job < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class DossierOperationLogMoveToColdStorageBatchJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class BatchOperationEnqueueAllJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class BatchOperationProcessOneJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class TitreIdentiteWatermarkJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class AdminUpdateDefaultZonesJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ProcessStalledDeclarativeDossierJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ResetExpiringDossiersJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class SendClosingNotificationJob < ApplicationJob
-      self.queue_adapter = :sidekiq
-    end
-
-    class ImageProcessorJob < ApplicationJob
-      self.queue_adapter = :sidekiq
+    [
+      ActiveStorage::AnalyzeJob,
+      ActiveStorage::PurgeJob,
+      AdminUpdateDefaultZonesJob,
+      APIEntreprise::Job,
+      AdminUpdateDefaultZonesJob,
+      BatchOperationEnqueueAllJob,
+      BatchOperationProcessOneJob,
+      ChampFetchExternalDataJob,
+      Cron::CronJob,
+      DestroyRecordLaterJob,
+      DossierIndexSearchTermsJob,
+      DossierOperationLogMoveToColdStorageBatchJob,
+      DossierRebaseJob,
+      HelpscoutCreateConversationJob,
+      ImageProcessorJob,
+      MaintenanceTasks::TaskJob,
+      Migrations::BackfillStableIdJob,
+      PriorizedMailDeliveryJob,
+      ProcedureExternalURLCheckJob,
+      ProcedureSVASVRProcessDossierJob,
+      ProcessStalledDeclarativeDossierJob,
+      ResetExpiringDossiersJob,
+      SendClosingNotificationJob,
+      VirusScannerJob,
+      WebHookJob
+    ].each do |job_class|
+      job_class.queue_adapter = :sidekiq
     end
   end
 end

spec/controllers/support_controller_spec.rb

@@ -58,9 +58,9 @@ describe SupportController, type: :controller do
         let(:params) { { subject: 'bonjour', text: 'un message' } }
 
         it 'creates a conversation on HelpScout' do
-          expect_any_instance_of(Helpscout::FormAdapter).to receive(:send_form).and_return(true)
-
-          expect { subject }.to change(Commentaire, :count).by(0)
+          expect { subject }.to \
+            change(Commentaire, :count).by(0).and \
+            have_enqueued_job(HelpscoutCreateConversationJob).with(hash_including(params))
 
           expect(flash[:notice]).to match('Votre message a été envoyé.')
           expect(response).to redirect_to root_path(formulaire_contact_general_submitted: true)
@@ -80,9 +80,9 @@ describe SupportController, type: :controller do
           end
 
           it 'creates a conversation on HelpScout' do
-            expect_any_instance_of(Helpscout::FormAdapter).to receive(:send_form).and_return(true)
-
-            expect { subject }.to change(Commentaire, :count).by(0)
+            expect { subject }.to \
+              change(Commentaire, :count).by(0).and \
+              have_enqueued_job(HelpscoutCreateConversationJob).with(hash_including(subject: 'bonjour', dossier_id: dossier.id))
 
             expect(flash[:notice]).to match('Votre message a été envoyé.')
             expect(response).to redirect_to root_path(formulaire_contact_general_submitted: true)
@@ -103,9 +103,8 @@ describe SupportController, type: :controller do
           end
 
           it 'posts the message to the dossier messagerie' do
-            expect_any_instance_of(Helpscout::FormAdapter).not_to receive(:send_form)
-
             expect { subject }.to change(Commentaire, :count).by(1)
+            assert_no_enqueued_jobs(only: HelpscoutCreateConversationJob)
 
             expect(Commentaire.last.email).to eq(user.email)
             expect(Commentaire.last.dossier).to eq(dossier)
@@ -159,10 +158,21 @@ describe SupportController, type: :controller do
 
     describe "when form is filled" do
       it "creates a conversation on HelpScout" do
-        expect_any_instance_of(Helpscout::FormAdapter).to receive(:send_form).and_return(true)
-        subject
+        expect { subject }.to have_enqueued_job(HelpscoutCreateConversationJob).with(hash_including(params.except(:admin)))
         expect(flash[:notice]).to match('Votre message a été envoyé.')
       end
+
+      context "with a piece justificative" do
+        let(:logo) { fixture_file_upload('spec/fixtures/files/white.png', 'image/png') }
+        let(:params) { super().merge(piece_jointe: logo) }
+
+        it "create blob and pass it to conversation job" do
+          expect { subject }.to \
+            change(ActiveStorage::Blob, :count).by(1).and \
+              have_enqueued_job(HelpscoutCreateConversationJob).with(hash_including(blob_id: Integer)).and \
+              have_enqueued_job(VirusScannerJob)
+        end
+      end
     end
 
     describe "when invisible captcha is filled" do

spec/jobs/helpscout_create_conversation_job_spec.rb

@@ -0,0 +1,64 @@
+require 'rails_helper'
+
+RSpec.describe HelpscoutCreateConversationJob, type: :job do
+  let(:args) { { email: 'sender@email.com' } }
+
+  describe '#perform' do
+    context 'when blob_id is not present' do
+      it 'sends the form without a file' do
+        form_adapter = double('Helpscout::FormAdapter')
+        allow(Helpscout::FormAdapter).to receive(:new).with(hash_including(args.merge(blob: nil))).and_return(form_adapter)
+        expect(form_adapter).to receive(:send_form)
+
+        described_class.perform_now(**args)
+      end
+    end
+
+    context 'when blob_id is present' do
+      let(:blob) {
+        ActiveStorage::Blob.create_and_upload!(io: StringIO.new("toto"), filename: "toto.png")
+      }
+
+      before do
+        allow(blob).to receive(:virus_scanner).and_return(double('VirusScanner', pending?: pending, safe?: safe))
+      end
+
+      context 'when the file has not been scanned yet' do
+        let(:pending) { true }
+        let(:safe) { false }
+
+        it 'reenqueue job' do
+          expect {
+            described_class.perform_now(blob_id: blob.id, **args)
+          }.to have_enqueued_job(described_class).with(blob_id: blob.id, **args)
+        end
+      end
+
+      context 'when the file is safe' do
+        let(:pending) { false }
+        let(:safe) { true }
+
+        it 'downloads the file and sends the form' do
+          form_adapter = double('Helpscout::FormAdapter')
+          allow(Helpscout::FormAdapter).to receive(:new).with(hash_including(args.merge(blob:))).and_return(form_adapter)
+          allow(form_adapter).to receive(:send_form)
+
+          described_class.perform_now(blob_id: blob.id, **args)
+        end
+      end
+
+      context 'when the file is not safe' do
+        let(:pending) { false }
+        let(:safe) { false }
+
+        it 'downloads the file and sends the form' do
+          form_adapter = double('Helpscout::FormAdapter')
+          allow(Helpscout::FormAdapter).to receive(:new).with(hash_including(args.merge(blob: nil))).and_return(form_adapter)
+          allow(form_adapter).to receive(:send_form)
+
+          described_class.perform_now(blob_id: blob.id, **args)
+        end
+      end
+    end
+  end
+end

spec/lib/helpscout/form_adapter_spec.rb

@@ -5,7 +5,7 @@ describe Helpscout::FormAdapter do
     context 'create_conversation' do
       before do
         allow(api).to receive(:create_conversation)
-          .and_return(double(success?: false))
+          .and_return(double(success?: true, headers: {}))
         described_class.new(params, api).send_form
       end