Merge pull request #7916 from betagouv/another_try_at_log_out_spec

chore(capybara): nouvelle essai pour stabiliser les deconnexions sur les tests bout en bout

Gemfile

@@ -100,6 +100,7 @@ group :test do
   gem 'launchy'
   gem 'rails-controller-testing'
   gem 'rspec_junit_formatter'
+  gem 'selenium-devtools'
   gem 'selenium-webdriver'
   gem 'shoulda-matchers', require: false
   gem 'timecop'

Gemfile.lock

@@ -660,6 +660,8 @@ GEM
     scss_lint (0.59.0)
       sass (~> 3.5, >= 3.5.5)
     selectize-rails (0.12.6)
+    selenium-devtools (0.106.0)
+      selenium-webdriver (~> 4.2)
     selenium-webdriver (4.5.0)
       childprocess (>= 0.5, < 5.0)
       rexml (~> 3.2, >= 3.2.5)
@@ -905,6 +907,7 @@ DEPENDENCIES
   sanitize-url
   sassc-rails
   scss_lint
+  selenium-devtools
   selenium-webdriver
   sentry-delayed_job
   sentry-rails

README.md

@@ -118,6 +118,10 @@ Pour exécuter les tests de l'application, plusieurs possibilités :
 
         NO_HEADLESS=1 bin/rspec spec/system
 
+- Afficher les logs js en error issus de la console du navigateur `console.error('coucou')`
+
+        JS_LOG=error bin/rspec spec/system
+
 ### Ajout de taches à exécuter au déploiement
 
         rails generate after_party:task task_name

app/controllers/administrateurs/services_controller.rb

@@ -15,6 +15,8 @@ module Administrateurs
       @service.administrateur = current_administrateur
 
       if @service.save
+        @service.enqueue_api_entreprise
+
         redirect_to admin_services_path(procedure_id: params[:procedure_id]),
           notice: "#{@service.nom} créé"
       else
@@ -33,6 +35,10 @@ module Administrateurs
       @service = service
 
       if @service.update(service_params)
+        if @service.siret_previously_changed?
+          @service.enqueue_api_entreprise
+        end
+
         redirect_to admin_services_path(procedure_id: params[:procedure_id]),
           notice: "#{@service.nom} modifié"
       else

app/javascript/controllers/application_controller.ts

@@ -7,9 +7,20 @@ export class ApplicationController extends Controller {
   #debounced = new Map<() => void, () => void>();
 
   protected debounce(fn: () => void, interval: number): void {
+    this.globalDispatch('debounced:added');
+
     let debounced = this.#debounced.get(fn);
     if (!debounced) {
-      debounced = debounce(fn.bind(this), interval);
+      const wrapper = () => {
+        fn.bind(this)();
+        this.#debounced.delete(fn);
+        if (this.#debounced.size == 0) {
+          this.globalDispatch('debounced:empty');
+        }
+      };
+
+      debounced = debounce(wrapper.bind(this), interval);
+
       this.#debounced.set(fn, debounced);
     }
     debounced();

app/javascript/controllers/autosave_controller.ts

@@ -37,6 +37,7 @@ export class AutosaveController extends ApplicationController {
   #abortController?: AbortController;
   #latestPromise = Promise.resolve();
   #needsRetry = false;
+  #pendingPromiseCount = 0;
 
   connect() {
     this.#latestPromise = Promise.resolve();
@@ -119,11 +120,15 @@ export class AutosaveController extends ApplicationController {
   }
 
   private didSucceed() {
+    this.#pendingPromiseCount -= 1;
+    if (this.#pendingPromiseCount == 0) {
       this.globalDispatch('autosave:end');
     }
+  }
 
   private didFail(error: ResponseError) {
     this.#needsRetry = true;
+    this.#pendingPromiseCount -= 1;
     this.globalDispatch('autosave:error', { error });
   }
 
@@ -179,6 +184,8 @@ export class AutosaveController extends ApplicationController {
       }
     }
 
+    this.#pendingPromiseCount++;
+
     return httpRequest(form.action, {
       method: 'patch',
       body: formData,

app/javascript/controllers/autosave_status_controller.ts

@@ -27,11 +27,24 @@ export class AutosaveStatusController extends ApplicationController {
   connect(): void {
     this.onGlobal('autosave:enqueue', () => this.didEnqueue());
     this.onGlobal('autosave:end', () => this.didSucceed());
-    // This event is used in tests to reset the state of the controller
-    this.onGlobal('autosave:reset', () => this.didReset());
     this.onGlobal<CustomEvent>('autosave:error', (event) =>
       this.didFail(event)
     );
+
+    this.onGlobal('debounced:added', () => this.debouncedAdded());
+    this.onGlobal('debounced:empty', () => this.debouncedEmpty());
+  }
+
+  private debouncedAdded() {
+    const autosave = this.element as HTMLDivElement;
+    removeClass(autosave, 'debounced-empty');
+    addClass(autosave, 'debounced-added');
+  }
+
+  private debouncedEmpty() {
+    const autosave = this.element as HTMLDivElement;
+    addClass(autosave, 'debounced-empty');
+    removeClass(autosave, 'debounced-added');
   }
 
   onClickRetryButton() {
@@ -48,10 +61,6 @@ export class AutosaveStatusController extends ApplicationController {
     this.debounce(this.hideSucceededStatus, AUTOSAVE_STATUS_VISIBLE_DURATION);
   }
 
-  private didReset() {
-    this.setState('idle');
-  }
-
   private didFail(event: CustomEvent<{ error: ResponseError }>) {
     const error = event.detail.error;
 

app/models/service.rb

@@ -45,8 +45,6 @@ class Service < ApplicationRecord
   validates :adresse, presence: { message: 'doit être renseignée' }, allow_nil: false
   validates :administrateur, presence: { message: 'doit être renseigné' }, allow_nil: false
 
-  after_commit :enqueue_api_entreprise, if: -> { siret_previously_changed? }
-
   def clone_and_assign_to_administrateur(administrateur)
     service_cloned = self.dup
     service_cloned.administrateur = administrateur
@@ -67,8 +65,6 @@ class Service < ApplicationRecord
     [etablissement_lat, etablissement_lng]
   end
 
-  private
-
   def enqueue_api_entreprise
     APIEntreprise::ServiceJob.perform_later(self.id)
   end

config/environments/test.rb

@@ -76,7 +76,7 @@ Rails.application.configure do
   config.active_storage.service = :test
 
   config.ds_autosave = {
-    debounce_delay: 500,
+    debounce_delay: 0,
     status_visible_duration: 500
   }
 

spec/controllers/administrateurs/services_controller_spec.rb

@@ -25,17 +25,21 @@ describe Administrateurs::ServicesController, type: :controller do
         }
       end
 
-      it { expect(flash.alert).to be_nil }
+      it do
-      it { expect(flash.notice).to eq('super service créé') }
+        expect(flash.alert).to be_nil
-      it { expect(Service.last.nom).to eq('super service') }
+        expect(flash.notice).to eq('super service créé')
-      it { expect(Service.last.organisme).to eq('organisme') }
+        expect(Service.last.nom).to eq('super service')
-      it { expect(Service.last.type_organisme).to eq(Service.type_organismes.fetch(:association)) }
+        expect(Service.last.organisme).to eq('organisme')
-      it { expect(Service.last.email).to eq('email@toto.com') }
+        expect(Service.last.type_organisme).to eq(Service.type_organismes.fetch(:association))
-      it { expect(Service.last.telephone).to eq('1234') }
+        expect(Service.last.email).to eq('email@toto.com')
-      it { expect(Service.last.horaires).to eq('horaires') }
+        expect(Service.last.telephone).to eq('1234')
-      it { expect(Service.last.adresse).to eq('adresse') }
+        expect(Service.last.horaires).to eq('horaires')
-      it { expect(Service.last.siret).to eq('35600082800018') }
+        expect(Service.last.adresse).to eq('adresse')
-      it { expect(response).to redirect_to(admin_services_path(procedure_id: procedure.id)) }
+        expect(Service.last.siret).to eq('35600082800018')
+        expect(APIEntreprise::ServiceJob).to have_been_enqueued.with(Service.last.id)
+
+        expect(response).to redirect_to(admin_services_path(procedure_id: procedure.id))
+      end
     end
 
     context 'when submitting an invalid service' do
@@ -49,7 +53,7 @@ describe Administrateurs::ServicesController, type: :controller do
 
   describe '#update' do
     let!(:service) { create(:service, administrateur: admin) }
-    let(:service_params) { { nom: 'nom', type_organisme: Service.type_organismes.fetch(:association) } }
+    let(:service_params) { { nom: 'nom', type_organisme: Service.type_organismes.fetch(:association), siret: "13002526500013" } }
 
     before do
       sign_in(admin.user)
@@ -67,6 +71,7 @@ describe Administrateurs::ServicesController, type: :controller do
       it { expect(Service.last.nom).to eq('nom') }
       it { expect(Service.last.type_organisme).to eq(Service.type_organismes.fetch(:association)) }
       it { expect(response).to redirect_to(admin_services_path(procedure_id: procedure.id)) }
+      it { expect(APIEntreprise::ServiceJob).to have_been_enqueued.with(service.id) }
     end
 
     context 'when updating a service with invalid data' do

spec/models/service_spec.rb

@@ -87,25 +87,6 @@ describe Service, type: :model do
     end
   end
 
-  describe "API Entreprise job" do
-    subject { create(:service) }
-    it "should enqueue a job when created" do
-      expect(APIEntreprise::ServiceJob).to have_been_enqueued.with(subject.id)
-    end
-
-    it "should enqueue a job when siret changed" do
-      subject.update(siret: "35600082800018")
-      expect(APIEntreprise::ServiceJob).to have_been_enqueued.with(subject.id)
-    end
-
-    it "should not enqueue a job when siret is unchanged" do
-      subject
-      clear_enqueued_jobs
-      subject.update(telephone: "09879789")
-      expect(APIEntreprise::ServiceJob).not_to have_been_enqueued
-    end
-  end
-
   describe "etablissement adresse & geo coordinates" do
     subject { create(:service, etablissement_lat: latitude, etablissement_lng: longitude, etablissement_infos: etablissement_infos) }
 

spec/support/capybara.rb

@@ -7,6 +7,7 @@ Capybara.register_driver :chrome do |app|
   options = Selenium::WebDriver::Chrome::Options.new
   options.add_argument('--no-sandbox') unless ENV['SANDBOX']
   options.add_argument('--mute-audio')
+  options.add_argument('--window-size=1440,900')
 
   download_path = Capybara.save_path
   # Chromedriver 77 requires setting this for headless mode on linux
@@ -47,6 +48,8 @@ Capybara.ignore_hidden_elements = false
 
 Capybara.enable_aria_label = true
 
+Capybara.disable_animation = true
+
 # Save a snapshot of the HTML page when an integration test fails
 Capybara::Screenshot.autosave_on_failure = true
 # Keep only the screenshots generated from the last failing test suite
@@ -63,6 +66,12 @@ RSpec.configure do |config|
 
   config.before(:each, type: :system, js: true) do
     driven_by ENV['NO_HEADLESS'] ? :chrome : :headless_chrome
+
+    if ENV['JS_LOG'].present?
+      page.driver.browser.on_log_event(:console) do |event|
+        puts event.args if event.type == ENV['JS_LOG'].downcase.to_sym
+      end
+    end
   end
 
   # Set the user preferred language before Javascript system specs.

spec/support/system_helpers.rb

@@ -174,8 +174,8 @@ module SystemHelpers
 
   def wait_for_autosave(brouillon = true)
     blur
-    expect(page).to have_css('span', text: "#{brouillon ? 'Brouillon' : 'Dossier'} enregistré", visible: true)
+    expect(page).to have_css('.debounced-empty') # no more debounce
-    page.execute_script("document.documentElement.dispatchEvent(new CustomEvent('autosave:reset'));")
+    expect(page).to have_css('.autosave-state-idle') # no more in flight promise
   end
 end
 

spec/system/api_particulier/api_particulier_spec.rb

@@ -279,19 +279,17 @@ describe 'fetch API Particulier Data', js: true do
         dossier = Dossier.last
         cnaf_champ = dossier.champs.find(&:cnaf?)
 
-        expect(cnaf_champ.code_postal).to eq('wrong_code')
+        wait_until { cnaf_champ.reload.code_postal == 'wrong_code' }
 
         click_on 'Déposer le dossier'
         expect(page).to have_content(/code postal doit posséder 5 caractères/)
 
         VCR.use_cassette('api_particulier/success/composition_familiale') do
-          perform_enqueued_jobs do
           fill_in 'Le code postal', with: code_postal
           wait_for_autosave
-          end
-        end
-
           click_on 'Déposer le dossier'
+          perform_enqueued_jobs
+        end
 
         visit demande_dossier_path(dossier)
         expect(page).to have_content(/Des données.*ont été reçues depuis la CAF/)
@@ -335,18 +333,17 @@ describe 'fetch API Particulier Data', js: true do
         dossier = Dossier.last
         pole_emploi_champ = dossier.champs.find(&:pole_emploi?)
 
-        expect(pole_emploi_champ.identifiant).to eq('wrong code')
+        wait_until { pole_emploi_champ.reload.identifiant == 'wrong code' }
+
         clear_enqueued_jobs
         pole_emploi_champ.update(external_id: nil, identifiant: nil)
 
         VCR.use_cassette('api_particulier/success/situation_pole_emploi') do
-          perform_enqueued_jobs do
           fill_in "Identifiant", with: identifiant
           wait_until { pole_emploi_champ.reload.external_id.present? }
-          end
-        end
-
           click_on 'Déposer le dossier'
+          perform_enqueued_jobs
+        end
 
         visit demande_dossier_path(dossier)
         expect(page).to have_content(/Des données.*ont été reçues depuis Pôle emploi/)
@@ -406,18 +403,16 @@ describe 'fetch API Particulier Data', js: true do
         dossier = Dossier.last
         mesri_champ = dossier.champs.find(&:mesri?)
 
-        expect(mesri_champ.ine).to eq('wrong code')
+        wait_until { mesri_champ.reload.ine == 'wrong code' }
         clear_enqueued_jobs
         mesri_champ.update(external_id: nil, ine: nil)
 
         VCR.use_cassette('api_particulier/success/etudiants') do
-          perform_enqueued_jobs do
           fill_in "INE", with: ine
           wait_until { mesri_champ.reload.external_id.present? }
-          end
-        end
-
           click_on 'Déposer le dossier'
+          perform_enqueued_jobs
+        end
 
         visit demande_dossier_path(dossier)
         expect(page).to have_content(/Des données.*ont été reçues depuis le MESRI/)
@@ -469,19 +464,17 @@ describe 'fetch API Particulier Data', js: true do
         dossier = Dossier.last
         dgfip_champ = dossier.champs.find(&:dgfip?)
 
-        expect(dgfip_champ.reference_avis).to eq('wrong_code')
+        wait_until { dgfip_champ.reload.reference_avis == 'wrong_code' }
 
         click_on 'Déposer le dossier'
         expect(page).to have_content(/reference avis doit posséder 13 ou 14 caractères/)
 
         VCR.use_cassette('api_particulier/success/avis_imposition') do
-          perform_enqueued_jobs do
           fill_in "La référence d'avis d'imposition", with: reference_avis
           wait_for_autosave
-          end
-        end
-
           click_on 'Déposer le dossier'
+          perform_enqueued_jobs
+        end
 
         visit demande_dossier_path(dossier)
         expect(page).to have_content(/Des données.*ont été reçues depuis la DGFiP/)

spec/system/users/brouillon_spec.rb

@@ -114,15 +114,11 @@ describe 'The user' do
       fill_in('sub type de champ', with: 'un autre texte')
     end
 
-    expect(page).to have_content('Supprimer', count: 2)
-    wait_for_autosave
-
     expect(page).to have_content('Supprimer', count: 2)
 
     within '.repetition .row:first-child' do
       click_on 'Supprimer l’élément'
     end
-    wait_for_autosave
 
     expect(page).to have_content('Supprimer', count: 1)
   end