From ff44b7a6002805ac221b7b1f447c27bf87fa6e1b Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 29 May 2019 12:05:28 +0200 Subject: [PATCH 01/15] Refactor purge pj to be more generic --- app/controllers/attachments_controller.rb | 7 +++ .../gestionnaires/dossiers_controller.rb | 7 --- app/controllers/users/dossiers_controller.rb | 12 +---- app/views/attachments/destroy.js.erb | 3 ++ .../gestionnaires/avis/instruction.html.haml | 2 +- .../purge_champ_piece_justificative.js.erb | 3 -- app/views/shared/attachment/_update.html.haml | 22 +++++--- .../_piece_justificative.html.haml | 25 +-------- .../purge_champ_piece_justificative.js.erb | 3 -- config/routes.rb | 9 +--- .../attachments_controller_spec.rb | 51 +++++++++++++++++++ .../gestionnaires/dossiers_controller_spec.rb | 40 --------------- .../users/dossiers_controller_spec.rb | 41 --------------- 13 files changed, 82 insertions(+), 143 deletions(-) create mode 100644 app/views/attachments/destroy.js.erb delete mode 100644 app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb delete mode 100644 app/views/users/dossiers/purge_champ_piece_justificative.js.erb create mode 100644 spec/controllers/attachments_controller_spec.rb diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index c2edd378c..93f86f718 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -6,4 +6,11 @@ class AttachmentsController < ApplicationController @attachment = @blob.attachments.find(params[:id]) @user_can_upload = params[:user_can_upload] end + + def destroy + attachment = @blob.attachments.find(params[:id]) + @attachment_id = attachment.id + attachment.purge_later + flash.now.notice = 'La pièce jointe a bien été supprimée.' + end end diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index 02512b173..7a36bd6d0 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -141,13 +141,6 @@ module Gestionnaires redirect_to annotations_privees_gestionnaire_dossier_path(procedure, dossier) end - def purge_champ_piece_justificative - @champ = dossier.champs_private.find(params[:champ_id]) - @champ.piece_justificative_file.purge_later - - flash.notice = 'La pièce jointe a bien été supprimée.' - end - def print @dossier = dossier render layout: "print" diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 420a81891..9e599a194 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -6,11 +6,11 @@ module Users layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret] ACTIONS_ALLOWED_TO_ANY_USER = [:index, :recherche, :new] - ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire, :purge_champ_piece_justificative] + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire] 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, :purge_champ_piece_justificative] + 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 :show_demarche_en_test_banner @@ -236,14 +236,6 @@ module Users redirect_to url_for dossiers_path end - def purge_champ_piece_justificative - @champ = dossier.champs.find(params[:champ_id]) - - @champ.piece_justificative_file.purge_later - - flash.notice = 'La pièce jointe a bien été supprimée.' - end - def dossier_for_help dossier_id = params[:id] || params[:dossier_id] @dossier || (dossier_id.present? && Dossier.find_by(id: dossier_id.to_i)) diff --git a/app/views/attachments/destroy.js.erb b/app/views/attachments/destroy.js.erb new file mode 100644 index 000000000..ca79c22a1 --- /dev/null +++ b/app/views/attachments/destroy.js.erb @@ -0,0 +1,3 @@ +<%= render_flash(timeout: 5000, sticky: true) %> +<%= remove_element("#piece_justificative_#{@attachment_id}") %> +<%= show_element("#piece_justificative_file_#{@attachment_id}") %> diff --git a/app/views/gestionnaires/avis/instruction.html.haml b/app/views/gestionnaires/avis/instruction.html.haml index 25f03d299..dadee5f12 100644 --- a/app/views/gestionnaires/avis/instruction.html.haml +++ b/app/views/gestionnaires/avis/instruction.html.haml @@ -13,7 +13,7 @@ = form_for @avis, url: gestionnaire_avis_path(@avis), html: { class: 'form' } do |f| = f.text_area :answer, rows: 3, placeholder: 'Votre avis', required: true - = render partial: "shared/attachment/update", locals: { pj: @avis.piece_justificative_file, object: @avis, form: f } + = render partial: "shared/attachment/update", locals: { attachment: @avis.piece_justificative_file.attachment, user_can_destroy: true, form: f } .flex.justify-between.align-baseline %p.confidentiel.flex diff --git a/app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb b/app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb deleted file mode 100644 index 9d8807116..000000000 --- a/app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb +++ /dev/null @@ -1,3 +0,0 @@ -<%= render_flash(timeout: 5000, sticky: true) %> -<%= remove_element("#piece_justificative_#{@champ.id}") %> -<%= show_element("#champs_#{@champ.id}") %> diff --git a/app/views/shared/attachment/_update.html.haml b/app/views/shared/attachment/_update.html.haml index eb5c9dc83..ee86ddd7b 100644 --- a/app/views/shared/attachment/_update.html.haml +++ b/app/views/shared/attachment/_update.html.haml @@ -1,12 +1,22 @@ .piece-justificative - - if pj.attached? - .piece-justificative-actions{ id: "piece_justificative_#{object.id}" } + - if defined?(template) && template.attached? + %p.edit-pj-template.mb-1 + Veuillez télécharger, remplir et joindre + = link_to('le modèle suivant', url_for(template), target: '_blank', rel: 'noopener') + + - attachment_id = attachment ? attachment.id : SecureRandom.uuid + - user_can_destroy = defined?(user_can_destroy) ? user_can_destroy : false + - if attachment + .piece-justificative-actions{ id: "piece_justificative_#{attachment_id}" } .piece-justificative-action - = render partial: "shared/attachment/show", locals: { attachment: pj.attachment, user_can_upload: true } + = render partial: "shared/attachment/show", locals: { attachment: attachment, user_can_upload: true } + - if user_can_destroy + .piece-justificative-action + = link_to 'Supprimer', attachment_url(attachment.id, { signed_id: attachment.blob.signed_id }), remote: true, method: :delete, class: 'button small danger' .piece-justificative-action - = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#champs_#{object.id}" } + = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#piece_justificative_file_#{attachment_id}" } = form.file_field :piece_justificative_file, - id: "champs_#{object.id}", - class: "piece-justificative-input #{'hidden' if pj.attached?}", + id: "piece_justificative_file_#{attachment_id}", + class: "piece-justificative-input #{'hidden' if attachment}", direct_upload: true diff --git a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml index 346317669..3fefd0238 100644 --- a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml +++ b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml @@ -1,24 +1 @@ -- pj = champ.piece_justificative_file - -.piece-justificative - - if champ.type_de_champ.piece_justificative_template.attached? - %p.edit-pj-template.mb-1 - Veuillez télécharger, remplir et joindre - = link_to('le modèle suivant', url_for(champ.type_de_champ.piece_justificative_template), target: '_blank', rel: 'noopener') - - - if pj.attached? - .piece-justificative-actions{ id: "piece_justificative_#{champ.id}" } - .piece-justificative-action - = render partial: "shared/attachment/show", locals: { attachment: pj.attachment, user_can_upload: true } - .piece-justificative-action - - if champ.private? - = link_to 'Supprimer', gestionnaire_champ_purge_champ_piece_justificative_path(procedure_id: champ.dossier.procedure_id, dossier_id: champ.dossier_id, champ_id: champ.id), remote: true, method: :delete, class: 'button small danger' - - else - = link_to 'Supprimer', champ_purge_champ_piece_justificative_path(id: champ.dossier_id, champ_id: champ.id), remote: true, method: :delete, class: 'button small danger' - .piece-justificative-action - = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#champs_#{champ.id}" } - - = form.file_field :piece_justificative_file, - id: "champs_#{champ.id}", - class: "piece-justificative-input #{'hidden' if pj.attached?}", - direct_upload: true += render partial: "shared/attachment/update", locals: { attachment: champ.piece_justificative_file.attachment, template: champ.type_de_champ.piece_justificative_template, user_can_destroy: true, form: form } diff --git a/app/views/users/dossiers/purge_champ_piece_justificative.js.erb b/app/views/users/dossiers/purge_champ_piece_justificative.js.erb deleted file mode 100644 index 9d8807116..000000000 --- a/app/views/users/dossiers/purge_champ_piece_justificative.js.erb +++ /dev/null @@ -1,3 +0,0 @@ -<%= render_flash(timeout: 5000, sticky: true) %> -<%= remove_element("#piece_justificative_#{@champ.id}") %> -<%= show_element("#champs_#{@champ.id}") %> diff --git a/config/routes.rb b/config/routes.rb index 16c0d618a..da1c66ecc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -134,6 +134,7 @@ Rails.application.routes.draw do end get 'attachments/:id', to: 'attachments#show', as: :attachment + delete 'attachments/:id', to: 'attachments#destroy' get 'tour-de-france' => 'root#tour_de_france' get "patron" => "root#patron" @@ -281,10 +282,6 @@ Rails.application.routes.draw do post 'commentaire' => 'dossiers#create_commentaire' post 'ask_deletion' get 'attestation' - - resources :champs, only: [] do - delete 'purge_champ_piece_justificative' => 'dossiers#purge_champ_piece_justificative' - end end collection do @@ -330,10 +327,6 @@ Rails.application.routes.draw do post 'send-to-instructeurs' => 'dossiers#send_to_instructeurs' post 'avis' => 'dossiers#create_avis' get 'print' => 'dossiers#print' - - resources :champs, only: [] do - delete 'purge_champ_piece_justificative' => 'dossiers#purge_champ_piece_justificative' - end end end end diff --git a/spec/controllers/attachments_controller_spec.rb b/spec/controllers/attachments_controller_spec.rb new file mode 100644 index 000000000..c9b732ca5 --- /dev/null +++ b/spec/controllers/attachments_controller_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe AttachmentsController, type: :controller do + let(:user) { create(:user) } + + describe '#destroy' do + render_views + + let(:attachment) { champ.piece_justificative_file.attachment } + let(:dossier) { create(:dossier, user: user) } + let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } + let(:signed_id) { attachment.blob.signed_id } + + subject do + delete :destroy, params: { id: attachment.id, signed_id: signed_id }, format: :js + end + + context "when authenticated" do + before { sign_in(user) } + + context 'and dossier is owned by user' do + it { is_expected.to have_http_status(200) } + + it do + subject + expect(champ.reload.piece_justificative_file.attached?).to be(false) + end + end + + context 'and signed_id is invalid' do + let(:signed_id) { 'yolo' } + + it { is_expected.to have_http_status(404) } + + it do + subject + expect(champ.reload.piece_justificative_file.attached?).to be(true) + end + end + end + + context 'when not authenticated' do + it { is_expected.to have_http_status(401) } + + it do + subject + expect(champ.reload.piece_justificative_file.attached?).to be(true) + end + end + end +end diff --git a/spec/controllers/gestionnaires/dossiers_controller_spec.rb b/spec/controllers/gestionnaires/dossiers_controller_spec.rb index ad7415874..570f6b3c4 100644 --- a/spec/controllers/gestionnaires/dossiers_controller_spec.rb +++ b/spec/controllers/gestionnaires/dossiers_controller_spec.rb @@ -469,44 +469,4 @@ describe Gestionnaires::DossiersController, type: :controller do it { expect(champ_repetition.champs.first.value).to eq('text') } it { expect(response).to redirect_to(annotations_privees_gestionnaire_dossier_path(dossier.procedure, dossier)) } end - - describe '#purge_champ_piece_justificative' do - before { sign_in(gestionnaire) } - - subject { delete :purge_champ_piece_justificative, params: { procedure_id: champ.dossier.procedure.id, dossier_id: champ.dossier.id, champ_id: champ.id }, format: :js } - - context 'when gestionnaire can process dossier' do - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id, private: true) } - - it { is_expected.to have_http_status(200) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(false) - end - - context 'but champ is not linked to this dossier' do - let(:champ) { create(:champ_piece_justificative, dossier: create(:dossier), private: true) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end - - context 'when gestionnaire cannot process dossier' do - let(:dossier) { create(:dossier, procedure: create(:procedure)) } - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id, private: true) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end end diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 351e2fae7..9d68d2003 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -919,47 +919,6 @@ describe Users::DossiersController, type: :controller do end end - describe '#purge_champ_piece_justificative' do - before { sign_in(user) } - - subject { delete :purge_champ_piece_justificative, params: { id: champ.dossier.id, champ_id: champ.id }, format: :js } - - context 'when dossier is owned by user' do - let(:dossier) { create(:dossier, user: user) } - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } - - it { is_expected.to have_http_status(200) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(false) - end - - context 'but champ is not linked to this dossier' do - let(:champ) { create(:champ_piece_justificative, dossier: create(:dossier)) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end - - context 'when dossier is not owned by user' do - let(:dossier) { create(:dossier, user: create(:user)) } - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end - describe "#dossier_for_help" do before do sign_in(user) From 6393401f3701bd89a22265050062d711be34d53f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Fri, 31 May 2019 10:15:16 +0000 Subject: [PATCH 02/15] Bump fstream from 1.0.11 to 1.0.12 Bumps [fstream](https://github.com/npm/fstream) from 1.0.11 to 1.0.12. - [Release notes](https://github.com/npm/fstream/releases) - [Commits](https://github.com/npm/fstream/compare/v1.0.11...v1.0.12) --- yarn.lock | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/yarn.lock b/yarn.lock index 3f7c3b20f..cc2d47945 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3541,9 +3541,9 @@ fsevents@^1.2.7: node-pre-gyp "^0.12.0" fstream@^1.0.0, fstream@^1.0.2: - version "1.0.11" - resolved "https://registry.yarnpkg.com/fstream/-/fstream-1.0.11.tgz#5c1fb1f117477114f0632a0eb4b71b3cb0fd3171" - integrity sha1-XB+x8RdHcRTwYyoOtLcbPLD9MXE= + version "1.0.12" + resolved "https://registry.yarnpkg.com/fstream/-/fstream-1.0.12.tgz#4e8ba8ee2d48be4f7d0de505455548eae5932045" + integrity sha512-WvJ193OHa0GHPEL+AycEJgxvBEwyfRkN1vhjca23OaPVMCaLCXTd5qAu82AjTcgP1UJmytkOKb63Ypde7raDIg== dependencies: graceful-fs "^4.1.2" inherits "~2.0.0" @@ -3644,7 +3644,7 @@ glob-stream@^6.1.0: to-absolute-glob "^2.0.0" unique-stream "^2.0.2" -glob@^7.0.0, glob@^7.0.3, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@~7.1.1: +glob@^7.0.0, glob@^7.0.3, glob@^7.1.1, glob@^7.1.2, glob@~7.1.1: version "7.1.3" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.3.tgz#3960832d3f1574108342dafd3a67b332c0969df1" integrity sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ== @@ -3656,6 +3656,18 @@ glob@^7.0.0, glob@^7.0.3, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@~7.1.1: once "^1.3.0" path-is-absolute "^1.0.0" +glob@^7.1.3: + version "7.1.4" + resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.4.tgz#aa608a2f6c577ad357e1ae5a5c26d9a8d1969255" + integrity sha512-hkLPepehmnKk41pUGm3sYxoFs/umurYfYJCerbXEyFIWcAzvpipAgVkBqqT9RBKMGjnq6kMuyYwha6csxbiM1A== + dependencies: + fs.realpath "^1.0.0" + inflight "^1.0.4" + inherits "2" + minimatch "^3.0.4" + once "^1.3.0" + path-is-absolute "^1.0.0" + global-modules@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/global-modules/-/global-modules-1.0.0.tgz#6d770f0eb523ac78164d72b5e71a8877265cc3ea" From c05dc38b952a92d899398f564d50d0c3d99f5d8d Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 12:46:12 +0200 Subject: [PATCH 03/15] specs: remove 'disable-gpu' from chromedriver options This option as only ever needed on Windows [1] (which we don't use), and it shouldn't be required anymore, as the underlying bug has been fixed [2]. - [1] https://developers.google.com/web/updates/2017/04/headless-chrome - [2] https://bugs.chromium.org/p/chromium/issues/detail?id=737678 --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6072acede..cf66f61e8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -40,7 +40,7 @@ end Capybara.register_driver :headless_chrome do |app| capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( - chromeOptions: { args: ['headless', 'disable-gpu', 'disable-dev-shm-usage', 'disable-software-rasterizer', 'mute-audio', 'window-size=1440,900'] } + chromeOptions: { args: ['headless', 'disable-dev-shm-usage', 'disable-software-rasterizer', 'mute-audio', 'window-size=1440,900'] } ) Capybara::Selenium::Driver.new app, From 2bd749a325a74d3c100266eb200ea109ad1848ff Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 12:47:03 +0200 Subject: [PATCH 04/15] specs: clear the React champ before adding a new value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prevents the value from being `Nouveau champ TexteLibellé de champ carte`. --- spec/features/new_administrateur/types_de_champ_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/new_administrateur/types_de_champ_spec.rb b/spec/features/new_administrateur/types_de_champ_spec.rb index dd68532bb..3c68e923a 100644 --- a/spec/features/new_administrateur/types_de_champ_spec.rb +++ b/spec/features/new_administrateur/types_de_champ_spec.rb @@ -121,14 +121,14 @@ feature 'As an administrateur I can edit types de champ', js: true do it "Add carte champ" do select('Carte', from: 'champ-0-type_champ') - fill_in 'champ-0-libelle', with: 'libellé de champ carte' + fill_in 'champ-0-libelle', with: 'Libellé de champ carte', fill_options: { clear: :backspace } blur check 'Quartiers prioritaires' expect(page).to have_content('Formulaire enregistré') preview_window = window_opened_by { click_on 'Prévisualiser le formulaire' } within_window(preview_window) do - expect(page).to have_content('libellé de champ carte') + expect(page).to have_content('Libellé de champ carte') expect(page).to have_content('Quartiers prioritaires') expect(page).not_to have_content('Cadastres') end From a555b24675e1ef2841de2053668a7c2643f21d8b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 12:50:59 +0200 Subject: [PATCH 05/15] specs: precompile Webpack assets before running the specs suite This avoids the first feature spec stalling for a few dozens of seconds because Webpack assets are compiling (and thus reduce the risk of the spec timing out). Pre-compilation takes ~ 10s. --- .circleci/config.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index eb8386573..d08c361ae 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -69,6 +69,11 @@ jobs: DATABASE_URL: "postgres://tps_test@localhost:5432/tps_test" name: Create DB command: bundle exec rake db:create db:schema:load db:migrate RAILS_ENV=test + - run: + environment: + RAILS_ENV: test + name: Precompile Webpack assets + command: bin/webpack - run: environment: DATABASE_URL: "postgres://tps_test@localhost:5432/tps_test" From f2f16e2580d5b62ccfd061e4f3ae0346599ffc20 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 15:19:07 +0000 Subject: [PATCH 06/15] carte: disable tiles layer during tests During integration tests, we don't want to load the tiles from OSM: - It hits OSM servers during every test run; - It it slow (Capybara waits for the tiles to be loaded to proceed); - It makes test time out when tiles cannot be loaded for some reason. Fix #3913 --- app/javascript/shared/carte.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/javascript/shared/carte.js b/app/javascript/shared/carte.js index 2ba1f7445..b6f76a27a 100644 --- a/app/javascript/shared/carte.js +++ b/app/javascript/shared/carte.js @@ -13,10 +13,13 @@ export function initMap(element, position, editable = false) { scrollWheelZoom: false }).setView([position.lat, position.lon], editable ? 18 : position.zoom); - L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', { - attribution: - '© OpenStreetMap contributors' - }).addTo(map); + const loadTilesLayer = process.env.RAILS_ENV != 'test'; + if (loadTilesLayer) { + L.tileLayer('https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', { + attribution: + '© OpenStreetMap contributors' + }).addTo(map); + } if (editable) { const freeDraw = new FreeDraw({ From 5419c130f4d43e7882a1a8d2cc0d91cd47a3c292 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 15:45:08 +0200 Subject: [PATCH 07/15] brouillon_spec: make the login sequence faster Log the user with `login_as` (rather than the browser). --- spec/features/users/brouillon_spec.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/spec/features/users/brouillon_spec.rb b/spec/features/users/brouillon_spec.rb index 4652c2366..34bb559ea 100644 --- a/spec/features/users/brouillon_spec.rb +++ b/spec/features/users/brouillon_spec.rb @@ -14,7 +14,7 @@ feature 'The user' do allow(Champs::RegionChamp).to receive(:regions).and_return(['region1', 'region2']).at_least(:once) allow(Champs::DepartementChamp).to receive(:departements).and_return(['dep1', 'dep2']).at_least(:once) - log_in(user.email, password, procedure) + log_in(user, procedure) fill_individual @@ -93,7 +93,7 @@ feature 'The user' do end scenario 'fill a dossier with repetition', js: true do - log_in(user.email, password, procedure_with_repetition) + log_in(user, procedure_with_repetition) fill_individual @@ -127,7 +127,7 @@ feature 'The user' do end scenario 'save an incomplete dossier as draft but cannot not submit it', js: true do - log_in(user.email, password, simple_procedure) + log_in(user, simple_procedure) fill_individual # Check an incomplete dossier can be saved as a draft, even when mandatory fields are missing @@ -156,7 +156,7 @@ feature 'The user' do end scenario 'adding, replacing and removing attachments', js: true do - log_in(user.email, password, procedure_with_pj) + log_in(user, procedure_with_pj) fill_individual # Add an attachment @@ -194,14 +194,10 @@ feature 'The user' do private - def log_in(email, password, procedure) + def log_in(user, procedure) + login_as user, scope: :user + visit "/commencer/#{procedure.path}" - click_on 'J’ai déjà un compte' - - expect(page).to have_current_path(new_user_session_path) - sign_in_with(email, password) - - expect(page).to have_current_path("/commencer/#{procedure.path}") click_on 'Commencer la démarche' expect(page).to have_content("Données d'identité") From b829d105d91550fc680bc9214fad61eb7a957971 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 15:47:56 +0200 Subject: [PATCH 08/15] =?UTF-8?q?brouillon=5Fspec:=20fix=20the=20slow=20us?= =?UTF-8?q?e=20of=20`have=5Fselect(locator,=20selected:=20=E2=80=A6)`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capybara's `have_select` can be very slow for elemtns with many options (see https://github.com/teamcapybara/capybara/issues/1527) This is because Capybara asserts that no other elements than the required ones are selected. This faster version is not as complete, but helps when checking the countries list or the years in a date picker. --- spec/features/users/brouillon_spec.rb | 20 ++++++++++---------- spec/spec_helper.rb | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/spec/features/users/brouillon_spec.rb b/spec/features/users/brouillon_spec.rb index 34bb559ea..a0eef004f 100644 --- a/spec/features/users/brouillon_spec.rb +++ b/spec/features/users/brouillon_spec.rb @@ -75,11 +75,11 @@ feature 'The user' do expect(page).to have_field('email', with: 'loulou@yopmail.com') expect(page).to have_field('phone', with: '1234567890') expect(page).to have_checked_field('Non') - expect(page).to have_select('simple_drop_down_list', selected: 'val2') - expect(page).to have_select('multiple_drop_down_list', selected: ['val1', 'val3']) - expect(page).to have_select('pays', selected: 'AUSTRALIE') - expect(page).to have_select('regions', selected: 'region2') - expect(page).to have_select('departement', selected: 'dep2') + expect(page).to have_selected_value('simple_drop_down_list', selected: 'val2') + expect(page).to have_selected_value('multiple_drop_down_list', selected: ['val1', 'val3']) + expect(page).to have_selected_value('pays', selected: 'AUSTRALIE') + expect(page).to have_selected_value('regions', selected: 'region2') + expect(page).to have_selected_value('departement', selected: 'dep2') expect(page).to have_checked_field('engagement') expect(page).to have_field('dossier_link', with: '123') expect(page).to have_text('file.pdf') @@ -229,10 +229,10 @@ feature 'The user' do end def check_date_and_time(date, field) - expect(page).to have_select("#{field}_1i", selected: date.strftime('%Y')) - expect(page).to have_select("#{field}_2i", selected: I18n.l(date, format: '%B')) - expect(page).to have_select("#{field}_3i", selected: date.strftime('%-d')) - expect(page).to have_select("#{field}_4i", selected: date.strftime('%H')) - expect(page).to have_select("#{field}_5i", selected: date.strftime('%M')) + expect(page).to have_selected_value("#{field}_1i", selected: date.strftime('%Y')) + expect(page).to have_selected_value("#{field}_2i", selected: I18n.l(date, format: '%B')) + expect(page).to have_selected_value("#{field}_3i", selected: date.strftime('%-d')) + expect(page).to have_selected_value("#{field}_4i", selected: date.strftime('%H')) + expect(page).to have_selected_value("#{field}_5i", selected: date.strftime('%M')) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cf66f61e8..291611d37 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -167,4 +167,22 @@ RSpec.configure do |config| actual.attributes.with_indifferent_access.except(*ignored) == expected.attributes.with_indifferent_access.except(*ignored) end end + + # Asserts that a given select element exists in the page, + # and that the option(s) with the given value(s) are selected. + # + # Usage: expect(page).to have_selected_value('Country', selected: 'Australia') + # + # For large lists, this is much faster than `have_select(location, selected: value)`, + # as it doesn’t check that every other options are not selected. + RSpec::Matchers.define(:have_selected_value) do |select_locator, options| + match do |page| + values = options[:selected].is_a?(String) ? [options[:selected]] : options[:selected] + + select_element = page.first(:select, select_locator) + select_element && values.all? do |value| + select_element.first(:option, value).selected? + end + end + end end From 1f69c6c6ebf8ab54e1c36614697e552d67139c21 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 3 Jun 2019 18:08:43 +0200 Subject: [PATCH 09/15] stylesheets: fix font of dropdown items on Firefox Styling `