From d31b4b68485e034146e5e1c05478b5e3b76d527a Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 20 Jul 2020 17:20:12 +0200 Subject: [PATCH 1/2] ajout d'un test sur le telechargement de zips --- app/lib/active_storage/downloadable_file.rb | 7 ++- .../features/instructeurs/instruction_spec.rb | 49 +++++++++++++++++++ spec/spec_helper.rb | 13 ++++- spec/support/download_helpers.rb | 36 ++++++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 spec/support/download_helpers.rb diff --git a/app/lib/active_storage/downloadable_file.rb b/app/lib/active_storage/downloadable_file.rb index 8af799df4..914ca478d 100644 --- a/app/lib/active_storage/downloadable_file.rb +++ b/app/lib/active_storage/downloadable_file.rb @@ -24,11 +24,14 @@ class ActiveStorage::DownloadableFile private def self.timestamped_filename(piece_justificative) + # we pad the original file name with a timestamp + # and a short id in order to help identify multiple versions and avoid name collisions extension = File.extname(piece_justificative.filename.to_s) basename = File.basename(piece_justificative.filename.to_s, extension) - timestamp = piece_justificative.created_at.strftime("%d-%m-%Y-%H-%S") + timestamp = piece_justificative.created_at.strftime("%d-%m-%Y-%H-%M") + id = piece_justificative.id % 10000 - "#{basename}-#{timestamp}#{extension}" + "#{basename}-#{timestamp}-#{id}#{extension}" end def using_local_backend? diff --git a/spec/features/instructeurs/instruction_spec.rb b/spec/features/instructeurs/instruction_spec.rb index 2b03cfa8c..7a7c8fdd5 100644 --- a/spec/features/instructeurs/instruction_spec.rb +++ b/spec/features/instructeurs/instruction_spec.rb @@ -138,6 +138,55 @@ feature 'Instructing a dossier:' do expect(page).to have_text("Dossier envoyé") end + context 'A instructeur can download a zip of the attachments of a dossier', js: true do + let(:procedure) { create(:procedure, :published, :with_piece_justificative, instructeurs: [instructeur]) } + let(:dossier) { create(:dossier, :en_construction, procedure: procedure) } + let(:champ) { dossier.champs.first } + let(:path) { 'spec/fixtures/files/piece_justificative_0.pdf' } + let(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier) } + + before { + champ.piece_justificative_file.attach(io: File.open(path), filename: "piece_justificative_0.pdf", content_type: "application/pdf") + + log_in(instructeur.email, password) + visit instructeur_dossier_path(procedure, dossier) + } + + scenario 'it possible to open the zip file' do + find(:css, '.attached').click + click_on 'Télécharger toutes les pièces jointes' + # For some reason, clicking the download link does not trigger the download in the headless browser ; + # So we need to go to the download link directly + visit telecharger_pjs_instructeur_dossier_path(procedure, dossier) + + DownloadHelpers.wait_for_download + files = ZipTricks::FileReader.read_zip_structure(io: File.open(DownloadHelpers.download)) + + expect(DownloadHelpers.download).to include "dossier-#{dossier.id}.zip" + expect(files.size).to be 1 + expect(files[0].filename.include?('piece_justificative_0')).to be_truthy + expect(files[0].uncompressed_size).to be File.size(path) + end + + scenario 'it possible to open the zip file with multiple attachments' do + commentaire.piece_jointe.attach(io: File.open(path), filename: "piece_justificative_0.pdf", content_type: "application/pdf") + + visit telecharger_pjs_instructeur_dossier_path(procedure, dossier) + DownloadHelpers.wait_for_download + files = ZipTricks::FileReader.read_zip_structure(io: File.open(DownloadHelpers.download)) + + expect(DownloadHelpers.download).to include "dossier-#{dossier.id}.zip" + expect(files.size).to be 2 + expect(files[0].filename.include?('piece_justificative_0')).to be_truthy + expect(files[1].filename.include?('piece_justificative_0')).to be_truthy + expect(files[0].filename).not_to eq files[1].filename + expect(files[0].uncompressed_size).to be File.size(path) + expect(files[1].uncompressed_size).to be File.size(path) + end + + after { DownloadHelpers.clear_downloads } + end + def log_in(email, password, check_email: true) visit '/' click_on 'Connexion' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8d9099c30..80723180c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -45,10 +45,19 @@ Capybara.register_driver :headless_chrome do |app| chromeOptions: { args: ['disable-dev-shm-usage', 'disable-software-rasterizer', 'mute-audio', 'window-size=1440,900'] } ) - Capybara::Selenium::Driver.new app, + download_path = Capybara.save_path + # Chromedriver 77 requires setting this for headless mode on linux + # Different versions of Chrome/selenium-webdriver require setting differently - jus set them all + options.add_preference('download.default_directory', download_path) + options.add_preference(:download, default_directory: download_path) + + Capybara::Selenium::Driver.new(app, browser: :chrome, desired_capabilities: capabilities, - options: options + options: options).tap do |driver| + # Set download dir for Chrome < 77 + driver.browser.download_path = download_path + end end # FIXME: remove this line when https://github.com/rspec/rspec-rails/issues/1897 has been fixed diff --git a/spec/support/download_helpers.rb b/spec/support/download_helpers.rb new file mode 100644 index 000000000..1515adaec --- /dev/null +++ b/spec/support/download_helpers.rb @@ -0,0 +1,36 @@ +module DownloadHelpers + TIMEOUT = 2 + + extend self + + def downloads + Dir[Capybara.save_path.join("*.zip")] + end + + def download + downloads.first + end + + def download_content + wait_for_download + File.read(download) + end + + def wait_for_download + Timeout.timeout(TIMEOUT) do + sleep 0.1 until downloaded? + end + end + + def downloaded? + !downloading? && downloads.any? + end + + def downloading? + downloads.grep(/\.part$/).any? + end + + def clear_downloads + FileUtils.rm_f(downloads) + end +end From 1a53bfed6236ee28cb7b2d7a5a41194be31bfb81 Mon Sep 17 00:00:00 2001 From: Keirua Date: Tue, 21 Jul 2020 17:11:40 +0200 Subject: [PATCH 2/2] renommage des tests Co-authored-by: Pierre de La Morinerie --- spec/features/instructeurs/instruction_spec.rb | 10 +++++----- spec/spec_helper.rb | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/features/instructeurs/instruction_spec.rb b/spec/features/instructeurs/instruction_spec.rb index 7a7c8fdd5..8d2862cb9 100644 --- a/spec/features/instructeurs/instruction_spec.rb +++ b/spec/features/instructeurs/instruction_spec.rb @@ -138,21 +138,21 @@ feature 'Instructing a dossier:' do expect(page).to have_text("Dossier envoyé") end - context 'A instructeur can download a zip of the attachments of a dossier', js: true do + context 'with dossiers having attached files', js: true do let(:procedure) { create(:procedure, :published, :with_piece_justificative, instructeurs: [instructeur]) } let(:dossier) { create(:dossier, :en_construction, procedure: procedure) } let(:champ) { dossier.champs.first } let(:path) { 'spec/fixtures/files/piece_justificative_0.pdf' } let(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier) } - before { + before do champ.piece_justificative_file.attach(io: File.open(path), filename: "piece_justificative_0.pdf", content_type: "application/pdf") log_in(instructeur.email, password) visit instructeur_dossier_path(procedure, dossier) - } + end - scenario 'it possible to open the zip file' do + scenario 'A instructeur can download an archive containing a single attachment' do find(:css, '.attached').click click_on 'Télécharger toutes les pièces jointes' # For some reason, clicking the download link does not trigger the download in the headless browser ; @@ -168,7 +168,7 @@ feature 'Instructing a dossier:' do expect(files[0].uncompressed_size).to be File.size(path) end - scenario 'it possible to open the zip file with multiple attachments' do + scenario 'A instructeur can download an archive containing several identical attachments' do commentaire.piece_jointe.attach(io: File.open(path), filename: "piece_justificative_0.pdf", content_type: "application/pdf") visit telecharger_pjs_instructeur_dossier_path(procedure, dossier) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 80723180c..869896403 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -47,7 +47,7 @@ Capybara.register_driver :headless_chrome do |app| download_path = Capybara.save_path # Chromedriver 77 requires setting this for headless mode on linux - # Different versions of Chrome/selenium-webdriver require setting differently - jus set them all + # Different versions of Chrome/selenium-webdriver require setting differently - just set them all options.add_preference('download.default_directory', download_path) options.add_preference(:download, default_directory: download_path)