From 8711e8f9113b4397e341ff0fa899fddf0ac4fed1 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 8 Jul 2020 14:46:31 +0200 Subject: [PATCH 01/26] reenable validator on piece_justificative --- .../champs/piece_justificative_champ.rb | 31 +++++++++++++++---- spec/features/users/brouillon_spec.rb | 7 +---- .../champs/piece_justificative_champ_spec.rb | 6 +--- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index 52530355c..44730b9b1 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -33,12 +33,9 @@ class Champs::PieceJustificativeChamp < Champ "image/jpeg" ] - # TODO: once we're running on Rails 6, re-enable this validation. - # See https://github.com/betagouv/demarches-simplifiees.fr/issues/4926 - # - # validates :piece_justificative_file, - # content_type: ACCEPTED_FORMATS, - # size: { less_than: MAX_SIZE } + validates :piece_justificative_file, + content_type: ACCEPTED_FORMATS, + size: { less_than: MAX_SIZE } before_save :update_skip_pj_validation @@ -58,6 +55,28 @@ class Champs::PieceJustificativeChamp < Champ piece_justificative_file.filename.to_s if piece_justificative_file.attached? end + def piece_justificative_file_errors + errors = [] + + if piece_justificative_file.attached? && piece_justificative_file.previous_changes.present? + if piece_justificative_file.blob.byte_size > MAX_SIZE + errors << "Le fichier #{piece_justificative_file.filename} est trop lourd, il doit faire au plus #{MAX_SIZE.to_s(:human_size, precision: 2)}" + end + + if !piece_justificative_file.blob.content_type.in?(ACCEPTED_FORMATS) + errors << "Le fichier #{piece_justificative_file.filename} est dans un format que nous n'acceptons pas" + end + + # FIXME: add Clamav check + end + + if errors.present? + piece_justificative_file.purge_later + end + + errors + end + def for_api if piece_justificative_file.attached? && (piece_justificative_file.virus_scanner.safe? || piece_justificative_file.virus_scanner.pending?) piece_justificative_file.service_url diff --git a/spec/features/users/brouillon_spec.rb b/spec/features/users/brouillon_spec.rb index 5e2c64fc6..5818f99e8 100644 --- a/spec/features/users/brouillon_spec.rb +++ b/spec/features/users/brouillon_spec.rb @@ -200,12 +200,7 @@ feature 'The user' do expect(page).to have_text('RIB.pdf') end - # TODO: once we're running on Rails 6, re-enable the validator on PieceJustificativeChamp, - # and unmark this spec as pending. - # - # See piece_justificative_champ.rb - # See https://github.com/betagouv/demarches-simplifiees.fr/issues/4926 - scenario 'add an invalid attachment', js: true, pending: true do + scenario 'add an invalid attachment', js: true do log_in(user, procedure_with_pjs) fill_individual diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index a51552b3b..7f5a563e8 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -16,11 +16,7 @@ describe Champs::PieceJustificativeChamp do end end - # TODO: once we're running on Rails 6, re-enable the PieceJustificativeChamp validator, - # and re-enable this spec. - # - # See https://github.com/betagouv/demarches-simplifiees.fr/issues/4926 - describe "validations", pending: true do + describe "validations" do subject(:champ_pj) { build(:champ_piece_justificative) } it { is_expected.to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } From 0d88674cea1f1e832c5caa7b4dd5f17f1ac7309b Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 9 Jul 2020 16:53:35 +0200 Subject: [PATCH 02/26] better translations --- .../champs/piece_justificative_champ.rb | 22 ------------------- .../locales/active_storage_validations.fr.yml | 1 + 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index 44730b9b1..b7aa201f6 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -55,28 +55,6 @@ class Champs::PieceJustificativeChamp < Champ piece_justificative_file.filename.to_s if piece_justificative_file.attached? end - def piece_justificative_file_errors - errors = [] - - if piece_justificative_file.attached? && piece_justificative_file.previous_changes.present? - if piece_justificative_file.blob.byte_size > MAX_SIZE - errors << "Le fichier #{piece_justificative_file.filename} est trop lourd, il doit faire au plus #{MAX_SIZE.to_s(:human_size, precision: 2)}" - end - - if !piece_justificative_file.blob.content_type.in?(ACCEPTED_FORMATS) - errors << "Le fichier #{piece_justificative_file.filename} est dans un format que nous n'acceptons pas" - end - - # FIXME: add Clamav check - end - - if errors.present? - piece_justificative_file.purge_later - end - - errors - end - def for_api if piece_justificative_file.attached? && (piece_justificative_file.virus_scanner.safe? || piece_justificative_file.virus_scanner.pending?) piece_justificative_file.service_url diff --git a/config/locales/active_storage_validations.fr.yml b/config/locales/active_storage_validations.fr.yml index 1494483b0..827795209 100644 --- a/config/locales/active_storage_validations.fr.yml +++ b/config/locales/active_storage_validations.fr.yml @@ -2,3 +2,4 @@ fr: errors: messages: content_type_invalid: n’est pas d’un type accepté + file_size_out_of_range: "est trop lourde, elle doit faire au plus 200 Mo." From 31d48c287974eaec56ef08940df140c2110a365b Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 15 Jul 2020 16:06:24 +0200 Subject: [PATCH 03/26] validation is enableed only for 'new' procedures --- .../champs/piece_justificative_champ.rb | 9 ++------ spec/features/users/brouillon_spec.rb | 17 +++++++++++++++ .../champs/piece_justificative_champ_spec.rb | 21 ++++++++++++++----- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index b7aa201f6..bfa5c2473 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -35,9 +35,8 @@ class Champs::PieceJustificativeChamp < Champ validates :piece_justificative_file, content_type: ACCEPTED_FORMATS, - size: { less_than: MAX_SIZE } - - before_save :update_skip_pj_validation + size: { less_than: MAX_SIZE }, + if: -> { !type_de_champ.skip_pj_validation } def main_value_name :piece_justificative_file @@ -60,8 +59,4 @@ class Champs::PieceJustificativeChamp < Champ piece_justificative_file.service_url end end - - def update_skip_pj_validation - type_de_champ.update(skip_pj_validation: true) - end end diff --git a/spec/features/users/brouillon_spec.rb b/spec/features/users/brouillon_spec.rb index 5818f99e8..898af7cb0 100644 --- a/spec/features/users/brouillon_spec.rb +++ b/spec/features/users/brouillon_spec.rb @@ -176,6 +176,13 @@ feature 'The user' do create(:procedure, :published, :for_individual, types_de_champ: tdcs) end + let(:old_procedure_with_disabled_pj_validation) do + tdcs = [ + create(:type_de_champ_piece_justificative, mandatory: true, libelle: 'Pièce justificative 1', order_place: 1, skip_pj_validation: true) + ] + create(:procedure, :published, :for_individual, types_de_champ: tdcs) + end + scenario 'add an attachment', js: true do log_in(user, procedure_with_pjs) fill_individual @@ -216,6 +223,16 @@ feature 'The user' do expect(page).to have_text('piece_justificative_0.pdf') end + scenario 'add an invalid attachment on an old procedure where pj validation is disabled', js: true do + log_in(user, old_procedure_with_disabled_pj_validation) + fill_individual + + # Test invalid file type + attach_file('Pièce justificative 1', Rails.root + 'spec/fixtures/files/invalid_file_format.json') + expect(page).to have_no_text('La pièce justificative n’est pas d’un type accepté') + expect(page).to have_text('analyse antivirus en cours', count: 1) + end + scenario 'retry on transcient upload error', js: true do log_in(user, procedure_with_pjs) fill_individual diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index 7f5a563e8..b8cacf149 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -3,7 +3,7 @@ require 'active_storage_validations/matchers' describe Champs::PieceJustificativeChamp do include ActiveStorageValidations::Matchers - describe "update_skip_validation" do + describe "skip_validation is not set anymore" do subject { champ_pj.type_de_champ.skip_pj_validation } context 'before_save' do @@ -12,15 +12,26 @@ describe Champs::PieceJustificativeChamp do end context 'after_save' do let(:champ_pj) { create (:champ_piece_justificative) } - it { is_expected.to be_truthy } + it { is_expected.to be_falsy } end end describe "validations" do - subject(:champ_pj) { build(:champ_piece_justificative) } + let(:champ_pj) { create(:champ_piece_justificative) } + subject { champ_pj } - it { is_expected.to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } - it { is_expected.to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_FORMATS) } + context "by default" do + it { is_expected.to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } + it { is_expected.to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_FORMATS) } + it { expect(champ_pj.type_de_champ.skip_pj_validation).to be_falsy } + end + + context "when validation is disabled" do + before { champ_pj.type_de_champ.update(skip_pj_validation: true) } + + it { is_expected.not_to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } + it { is_expected.not_to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_FORMATS) } + end end describe "#for_export" do From a465bad405ab8981d7f16c4005e2b83d020efdb9 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 15 Jul 2020 17:03:48 +0200 Subject: [PATCH 04/26] fix broken test in champ_serializer_spec --- spec/serializers/champ_serializer_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index 1543bac2b..a80922f5e 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -4,16 +4,8 @@ describe ChampSerializer do let(:serializable_object) { champ } context 'when type champ is piece justificative' do - include Rails.application.routes.url_helpers - let(:champ) { create(:champ_piece_justificative) } - before do - champ.piece_justificative_file.attach({ filename: __FILE__, io: File.open(__FILE__) }) - champ.piece_justificative_file.blob.send(:enqueue_virus_scan) - end - after { champ.piece_justificative_file.purge } - it { expect(subject[:value]).to match('/rails/active_storage/disk/') } end From c155de1d7ed4a166760348eb8f4f557508fc5ebc Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 16 Jul 2020 09:58:07 +0200 Subject: [PATCH 05/26] ajout de la liste des extensions --- app/helpers/content_type_helper.rb | 47 +++++++++++++++++++ .../champs/piece_justificative_champ.rb | 4 +- app/views/shared/attachment/_edit.html.haml | 5 ++ .../_piece_justificative.html.haml | 3 +- spec/helpers/content_type_helper_spec.rb | 44 +++++++++++++++++ .../champs/piece_justificative_champ_spec.rb | 4 +- 6 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 app/helpers/content_type_helper.rb create mode 100644 spec/helpers/content_type_helper_spec.rb diff --git a/app/helpers/content_type_helper.rb b/app/helpers/content_type_helper.rb new file mode 100644 index 000000000..27fefe569 --- /dev/null +++ b/app/helpers/content_type_helper.rb @@ -0,0 +1,47 @@ +module ContentTypeHelper + def file_descriptions_from_content_types(content_types) + content_types.map { |ct| CONTENT_TYPE_DESCRIPTIONS[ct] }.uniq.join(', ') + end + + def file_extensions_from_content_types(content_types) + content_types.map do |ct| + if CONTENT_TYPE_EXTENSIONS.include?(ct) + ".#{CONTENT_TYPE_EXTENSIONS[ct]}" + end + end.uniq.sort.join(', ') + end + + CONTENT_TYPE_DESCRIPTIONS = { + "text/plain" => 'Word', + "application/pdf" => "PDF", + "application/msword" => "Word", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document" => "Excel", + "application/vnd.ms-excel" => "Excel", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" => "Excel", + "application/vnd.ms-powerpoint" => "PowerPoint", + "application/vnd.openxmlformats-officedocument.presentationml.presentation" => "PowerPoint", + "application/vnd.oasis.opendocument.text" => "LibreOffice", + "application/vnd.oasis.opendocument.presentation" => "LibreOffice", + "application/vnd.oasis.opendocument.spreadsheet" => "LibreOffice", + "image/png" => "PNG", + "image/jpeg" => "JPG" + } + + #  Mapping between format and extension documented on MDN: + # https://developer.mozilla.org/fr/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types + CONTENT_TYPE_EXTENSIONS = { + "text/plain" => "txt", + "application/pdf" => "pdf", + "application/msword" => "doc", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document" => "docx", + "application/vnd.ms-excel" => "xls", + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" => "xlsx", + "application/vnd.ms-powerpoint" => "ppt", + "application/vnd.openxmlformats-officedocument.presentationml.presentation" => "pptx", + "application/vnd.oasis.opendocument.text" => "odt", + "application/vnd.oasis.opendocument.presentation" => "odp", + "application/vnd.oasis.opendocument.spreadsheet" => "ods", + "image/png" => "png", + "image/jpeg" => "jpg" + } +end diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index bfa5c2473..33de24975 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -17,7 +17,7 @@ class Champs::PieceJustificativeChamp < Champ MAX_SIZE = 200.megabytes - ACCEPTED_FORMATS = [ + ACCEPTED_CONTENT_TYPES = [ "text/plain", "application/pdf", "application/msword", @@ -34,7 +34,7 @@ class Champs::PieceJustificativeChamp < Champ ] validates :piece_justificative_file, - content_type: ACCEPTED_FORMATS, + content_type: ACCEPTED_CONTENT_TYPES, size: { less_than: MAX_SIZE }, if: -> { !type_de_champ.skip_pj_validation } diff --git a/app/views/shared/attachment/_edit.html.haml b/app/views/shared/attachment/_edit.html.haml index 79910db79..be1c48ffe 100644 --- a/app/views/shared/attachment/_edit.html.haml +++ b/app/views/shared/attachment/_edit.html.haml @@ -31,6 +31,11 @@ = button_tag type: 'button', class: 'button attachment-error-retry', data: { 'input-target': ".attachment-input-#{attachment_id}" } do %span.icon.retry Ré-essayer + - if defined?(allowed_content_types) && allowed_content_types.present? + %p.mb-1 + Formats acceptés : #{file_descriptions_from_content_types(allowed_content_types)} + %sup + %span.icon.info{ :title => "Cela correspond aux fichiers ayant comme extension #{file_extensions_from_content_types(allowed_content_types)}" } = form.file_field attached_file.name, class: "attachment-input attachment-input-#{attachment_id} #{'hidden' if persisted}", 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 0fc1f99c8..048aa5c33 100644 --- a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml +++ b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml @@ -1,4 +1,5 @@ = render 'shared/attachment/edit', { form: form, attached_file: champ.piece_justificative_file, - template: champ.type_de_champ.piece_justificative_template, user_can_destroy: true } + template: champ.type_de_champ.piece_justificative_template, user_can_destroy: true, + allowed_content_types: Champs::PieceJustificativeChamp::ACCEPTED_CONTENT_TYPES } diff --git a/spec/helpers/content_type_helper_spec.rb b/spec/helpers/content_type_helper_spec.rb new file mode 100644 index 000000000..2c66426ca --- /dev/null +++ b/spec/helpers/content_type_helper_spec.rb @@ -0,0 +1,44 @@ +RSpec.describe ContentTypeHelper, type: :helper do + describe ".file_descriptions_from_content_types" do + subject { file_descriptions_from_content_types(content_types) } + + context "when a content-type does not exist" do + let(:content_types) { ['invalid content type'] } + it { is_expected.to eq("") } + end + + context "when a content type is valid" do + let(:content_types) { ["application/msword"] } + it { is_expected.to eq 'Word' } + end + + context "when two content types have the same description" do + let(:content_types) { ["application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/vnd.ms-excel"] } + it { is_expected.to eq 'Excel' } + end + + context "when two content types have different descriptions" do + let(:content_types) { ["application/msword", "application/vnd.ms-excel"] } + it { is_expected.to eq 'Word, Excel' } + end + end + + describe ".file_extensions_from_content_types" do + subject { file_extensions_from_content_types(content_types) } + + context "when a content-type does not exist" do + let(:content_types) { ['invalid content type'] } + it { is_expected.to eq("") } + end + + context "when a content type is valid" do + let(:content_types) { ["application/msword"] } + it { is_expected.to eq '.doc' } + end + + context "when two content types have different extensions" do + let(:content_types) { ["application/vnd.ms-excel", "application/msword"] } + it { is_expected.to eq '.doc, .xls' } + end + end +end diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index b8cacf149..1adebc18b 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -22,7 +22,7 @@ describe Champs::PieceJustificativeChamp do context "by default" do it { is_expected.to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } - it { is_expected.to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_FORMATS) } + it { is_expected.to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_CONTENT_TYPES) } it { expect(champ_pj.type_de_champ.skip_pj_validation).to be_falsy } end @@ -30,7 +30,7 @@ describe Champs::PieceJustificativeChamp do before { champ_pj.type_de_champ.update(skip_pj_validation: true) } it { is_expected.not_to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } - it { is_expected.not_to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_FORMATS) } + it { is_expected.not_to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_CONTENT_TYPES) } end end From eb46cc6425bd57ea2b04b46bb4ae5188c78e3e73 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 9 Sep 2020 10:27:49 +0200 Subject: [PATCH 06/26] piecejusticative file validation focus on size --- app/helpers/content_type_helper.rb | 47 ------------------- .../champs/piece_justificative_champ.rb | 17 ------- app/views/shared/attachment/_edit.html.haml | 5 -- .../_piece_justificative.html.haml | 3 +- spec/features/users/brouillon_spec.rb | 16 ------- spec/helpers/content_type_helper_spec.rb | 44 ----------------- .../champs/piece_justificative_champ_spec.rb | 2 - 7 files changed, 1 insertion(+), 133 deletions(-) delete mode 100644 app/helpers/content_type_helper.rb delete mode 100644 spec/helpers/content_type_helper_spec.rb diff --git a/app/helpers/content_type_helper.rb b/app/helpers/content_type_helper.rb deleted file mode 100644 index 27fefe569..000000000 --- a/app/helpers/content_type_helper.rb +++ /dev/null @@ -1,47 +0,0 @@ -module ContentTypeHelper - def file_descriptions_from_content_types(content_types) - content_types.map { |ct| CONTENT_TYPE_DESCRIPTIONS[ct] }.uniq.join(', ') - end - - def file_extensions_from_content_types(content_types) - content_types.map do |ct| - if CONTENT_TYPE_EXTENSIONS.include?(ct) - ".#{CONTENT_TYPE_EXTENSIONS[ct]}" - end - end.uniq.sort.join(', ') - end - - CONTENT_TYPE_DESCRIPTIONS = { - "text/plain" => 'Word', - "application/pdf" => "PDF", - "application/msword" => "Word", - "application/vnd.openxmlformats-officedocument.wordprocessingml.document" => "Excel", - "application/vnd.ms-excel" => "Excel", - "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" => "Excel", - "application/vnd.ms-powerpoint" => "PowerPoint", - "application/vnd.openxmlformats-officedocument.presentationml.presentation" => "PowerPoint", - "application/vnd.oasis.opendocument.text" => "LibreOffice", - "application/vnd.oasis.opendocument.presentation" => "LibreOffice", - "application/vnd.oasis.opendocument.spreadsheet" => "LibreOffice", - "image/png" => "PNG", - "image/jpeg" => "JPG" - } - - #  Mapping between format and extension documented on MDN: - # https://developer.mozilla.org/fr/docs/Web/HTTP/Basics_of_HTTP/MIME_types/Common_types - CONTENT_TYPE_EXTENSIONS = { - "text/plain" => "txt", - "application/pdf" => "pdf", - "application/msword" => "doc", - "application/vnd.openxmlformats-officedocument.wordprocessingml.document" => "docx", - "application/vnd.ms-excel" => "xls", - "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" => "xlsx", - "application/vnd.ms-powerpoint" => "ppt", - "application/vnd.openxmlformats-officedocument.presentationml.presentation" => "pptx", - "application/vnd.oasis.opendocument.text" => "odt", - "application/vnd.oasis.opendocument.presentation" => "odp", - "application/vnd.oasis.opendocument.spreadsheet" => "ods", - "image/png" => "png", - "image/jpeg" => "jpg" - } -end diff --git a/app/models/champs/piece_justificative_champ.rb b/app/models/champs/piece_justificative_champ.rb index 33de24975..59886ef43 100644 --- a/app/models/champs/piece_justificative_champ.rb +++ b/app/models/champs/piece_justificative_champ.rb @@ -17,24 +17,7 @@ class Champs::PieceJustificativeChamp < Champ MAX_SIZE = 200.megabytes - ACCEPTED_CONTENT_TYPES = [ - "text/plain", - "application/pdf", - "application/msword", - "application/vnd.openxmlformats-officedocument.wordprocessingml.document", - "application/vnd.ms-excel", - "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", - "application/vnd.ms-powerpoint", - "application/vnd.openxmlformats-officedocument.presentationml.presentation", - "application/vnd.oasis.opendocument.text", - "application/vnd.oasis.opendocument.presentation", - "application/vnd.oasis.opendocument.spreadsheet", - "image/png", - "image/jpeg" - ] - validates :piece_justificative_file, - content_type: ACCEPTED_CONTENT_TYPES, size: { less_than: MAX_SIZE }, if: -> { !type_de_champ.skip_pj_validation } diff --git a/app/views/shared/attachment/_edit.html.haml b/app/views/shared/attachment/_edit.html.haml index be1c48ffe..79910db79 100644 --- a/app/views/shared/attachment/_edit.html.haml +++ b/app/views/shared/attachment/_edit.html.haml @@ -31,11 +31,6 @@ = button_tag type: 'button', class: 'button attachment-error-retry', data: { 'input-target': ".attachment-input-#{attachment_id}" } do %span.icon.retry Ré-essayer - - if defined?(allowed_content_types) && allowed_content_types.present? - %p.mb-1 - Formats acceptés : #{file_descriptions_from_content_types(allowed_content_types)} - %sup - %span.icon.info{ :title => "Cela correspond aux fichiers ayant comme extension #{file_extensions_from_content_types(allowed_content_types)}" } = form.file_field attached_file.name, class: "attachment-input attachment-input-#{attachment_id} #{'hidden' if persisted}", 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 048aa5c33..0fc1f99c8 100644 --- a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml +++ b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml @@ -1,5 +1,4 @@ = render 'shared/attachment/edit', { form: form, attached_file: champ.piece_justificative_file, - template: champ.type_de_champ.piece_justificative_template, user_can_destroy: true, - allowed_content_types: Champs::PieceJustificativeChamp::ACCEPTED_CONTENT_TYPES } + template: champ.type_de_champ.piece_justificative_template, user_can_destroy: true } diff --git a/spec/features/users/brouillon_spec.rb b/spec/features/users/brouillon_spec.rb index 898af7cb0..7a3cfd055 100644 --- a/spec/features/users/brouillon_spec.rb +++ b/spec/features/users/brouillon_spec.rb @@ -207,22 +207,6 @@ feature 'The user' do expect(page).to have_text('RIB.pdf') end - scenario 'add an invalid attachment', js: true do - log_in(user, procedure_with_pjs) - fill_individual - - # Test invalid file type - attach_file('Pièce justificative 1', Rails.root + 'spec/fixtures/files/invalid_file_format.json') - expect(page).to have_text('La pièce justificative n’est pas d’un type accepté') - expect(page).to have_no_button('Ré-essayer', visible: true) - - # Replace the file by another with a valid type - attach_file('Pièce justificative 1', Rails.root + 'spec/fixtures/files/piece_justificative_0.pdf') - expect(page).to have_no_text('La pièce justificative n’est pas d’un type accepté') - expect(page).to have_text('analyse antivirus en cours') - expect(page).to have_text('piece_justificative_0.pdf') - end - scenario 'add an invalid attachment on an old procedure where pj validation is disabled', js: true do log_in(user, old_procedure_with_disabled_pj_validation) fill_individual diff --git a/spec/helpers/content_type_helper_spec.rb b/spec/helpers/content_type_helper_spec.rb deleted file mode 100644 index 2c66426ca..000000000 --- a/spec/helpers/content_type_helper_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -RSpec.describe ContentTypeHelper, type: :helper do - describe ".file_descriptions_from_content_types" do - subject { file_descriptions_from_content_types(content_types) } - - context "when a content-type does not exist" do - let(:content_types) { ['invalid content type'] } - it { is_expected.to eq("") } - end - - context "when a content type is valid" do - let(:content_types) { ["application/msword"] } - it { is_expected.to eq 'Word' } - end - - context "when two content types have the same description" do - let(:content_types) { ["application/vnd.openxmlformats-officedocument.wordprocessingml.document", "application/vnd.ms-excel"] } - it { is_expected.to eq 'Excel' } - end - - context "when two content types have different descriptions" do - let(:content_types) { ["application/msword", "application/vnd.ms-excel"] } - it { is_expected.to eq 'Word, Excel' } - end - end - - describe ".file_extensions_from_content_types" do - subject { file_extensions_from_content_types(content_types) } - - context "when a content-type does not exist" do - let(:content_types) { ['invalid content type'] } - it { is_expected.to eq("") } - end - - context "when a content type is valid" do - let(:content_types) { ["application/msword"] } - it { is_expected.to eq '.doc' } - end - - context "when two content types have different extensions" do - let(:content_types) { ["application/vnd.ms-excel", "application/msword"] } - it { is_expected.to eq '.doc, .xls' } - end - end -end diff --git a/spec/models/champs/piece_justificative_champ_spec.rb b/spec/models/champs/piece_justificative_champ_spec.rb index 1adebc18b..56d716ba3 100644 --- a/spec/models/champs/piece_justificative_champ_spec.rb +++ b/spec/models/champs/piece_justificative_champ_spec.rb @@ -22,7 +22,6 @@ describe Champs::PieceJustificativeChamp do context "by default" do it { is_expected.to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } - it { is_expected.to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_CONTENT_TYPES) } it { expect(champ_pj.type_de_champ.skip_pj_validation).to be_falsy } end @@ -30,7 +29,6 @@ describe Champs::PieceJustificativeChamp do before { champ_pj.type_de_champ.update(skip_pj_validation: true) } it { is_expected.not_to validate_size_of(:piece_justificative_file).less_than(Champs::PieceJustificativeChamp::MAX_SIZE) } - it { is_expected.not_to validate_content_type_of(:piece_justificative_file).allowing(Champs::PieceJustificativeChamp::ACCEPTED_CONTENT_TYPES) } end end From a10e692d35a549c7eb3a452c0c16fcadf9e839ad Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:01:53 +0200 Subject: [PATCH 07/26] bump rubocop & dependencies Error: The `Layout/Tab` cop has been renamed to `Layout/IndentationStyle`. (obsolete configuration found in .rubocop.yml, please update it) The `Style/MethodMissingSuper` cop has been removed since it has been superseded by `Lint/MissingSuper`. Please use `Lint/MissingSuper` instead. (obsolete configuration found in .rubocop.yml, please update it) The `Lint/UselessComparison` cop has been removed since it has been superseded by `Lint/BinaryOperatorWithIdenticalOperands`. Please use `Lint/BinaryOperatorWithIdenticalOperands` instead. (obsolete configuration found in .rubocop.yml, please update it) --- Gemfile.lock | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 115805861..a58a0188e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -344,7 +344,6 @@ GEM iban-tools (1.1.0) ice_nine (0.11.2) ipaddress (0.8.3) - jaro_winkler (1.5.4) jquery-rails (4.4.0) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) @@ -554,7 +553,7 @@ GEM execjs railties (>= 3.2) tilt - regexp_parser (1.7.1) + regexp_parser (1.8.0) request_store (1.5.0) rack (>= 1.4) responders (3.0.1) @@ -597,23 +596,27 @@ GEM rspec-support (3.9.3) rspec_junit_formatter (0.4.1) rspec-core (>= 2, < 4, != 2.12.0) - rubocop (0.81.0) - jaro_winkler (~> 1.5.1) + rubocop (0.91.1) parallel (~> 1.10) - parser (>= 2.7.0.1) + parser (>= 2.7.1.1) rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 1.7) rexml + rubocop-ast (>= 0.4.0, < 1.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 2.0) - rubocop-performance (1.5.2) - rubocop (>= 0.71.0) - rubocop-rails (2.5.2) - activesupport + rubocop-ast (0.4.2) + parser (>= 2.7.1.4) + rubocop-performance (1.8.1) + rubocop (>= 0.87.0) + rubocop-ast (>= 0.4.0) + rubocop-rails (2.8.1) + activesupport (>= 4.2.0) rack (>= 1.1) - rubocop (>= 0.72.0) - rubocop-rails_config (0.10.0) + rubocop (>= 0.87.0) + rubocop-rails_config (0.12.6) railties (>= 5.0) - rubocop (~> 0.80) + rubocop (~> 0.82) rubocop-performance (~> 1.3) rubocop-rails (~> 2.0) rubocop-rspec-focused (1.0.0) From 19294a93e4f80c480678ffd0ebc1f562504f74ab Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:06:39 +0200 Subject: [PATCH 08/26] replace deprecate rubocop rules as suggested --- .rubocop.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6010a7d4c..cce3f354a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -289,7 +289,7 @@ Layout/SpaceInsideReferenceBrackets: Layout/SpaceInsideStringInterpolation: Enabled: true -Layout/Tab: +Layout/IndentationStyle: Enabled: true Layout/TrailingEmptyLines: @@ -506,7 +506,7 @@ Lint/UselessAssignment: Exclude: - "spec/**/*" -Lint/UselessComparison: +Lint/BinaryOperatorWithIdenticalOperands: Enabled: true Lint/UselessElseWithoutRescue: @@ -1045,7 +1045,7 @@ Style/MethodCalledOnDoEndBlock: Style/MethodDefParentheses: Enabled: true -Style/MethodMissingSuper: +Lint/MissingSuper: Enabled: false Style/MinMax: From dfd2c1ee79ff928d8fe4edbfe4f28aadfea9fcb6 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:08:03 +0200 Subject: [PATCH 09/26] reorder gems --- Gemfile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index afed52c8e..761caed6a 100644 --- a/Gemfile +++ b/Gemfile @@ -3,8 +3,8 @@ source 'https://rubygems.org' gem 'aasm' gem 'active_link_to' # Automatically set a class on active links gem 'active_model_serializers' -gem 'active_storage_validations' gem 'activestorage-openstack' +gem 'active_storage_validations' gem 'administrate' gem 'after_party' gem 'anchored' @@ -31,13 +31,13 @@ gem 'flipper-active_record' gem 'flipper-ui' gem 'font-awesome-rails' gem 'fugit' -gem 'geo_coord', require: "geo/coord" gem 'geocoder' +gem 'geo_coord', require: "geo/coord" gem 'gon' gem 'graphql' gem 'graphql-batch' -gem 'graphql-rails_logger' gem 'graphql_playground-rails' +gem 'graphql-rails_logger' gem 'groupdate' gem 'haml-rails' gem 'hashie' @@ -118,8 +118,8 @@ group :development, :test do gem 'graphql-schema_comparator' gem 'mina', git: 'https://github.com/mina-deploy/mina.git', require: false # Deploy gem 'pry-byebug' - gem 'rspec-rails' gem 'rspec_junit_formatter', require: false + gem 'rspec-rails' gem 'ruby-debug-ide', require: false gem 'simple_xlsx_reader' gem 'spring' # Spring speeds up development by keeping your application running in the background From f3e487bd9eeb060435de1c698fa9e1798269c0b2 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:09:41 +0200 Subject: [PATCH 10/26] replace filter+first with find --- spec/features/routing/full_scenario_spec.rb | 3 +-- spec/models/attestation_template_spec.rb | 6 ++---- .../concern/tags_substitution_concern_spec.rb | 15 +++++---------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/spec/features/routing/full_scenario_spec.rb b/spec/features/routing/full_scenario_spec.rb index dcd4ad238..0609a22c0 100644 --- a/spec/features/routing/full_scenario_spec.rb +++ b/spec/features/routing/full_scenario_spec.rb @@ -213,8 +213,7 @@ feature 'The routing', js: true do def register_instructeur_and_log_in(email) confirmation_email = emails_sent_to(email) - .filter { |m| m.subject == 'Activez votre compte instructeur' } - .first + .find { |m| m.subject == 'Activez votre compte instructeur' } token_params = confirmation_email.body.match(/token=[^"]+/) visit "users/activate?#{token_params}" diff --git a/spec/models/attestation_template_spec.rb b/spec/models/attestation_template_spec.rb index a9561e26c..c6778934f 100644 --- a/spec/models/attestation_template_spec.rb +++ b/spec/models/attestation_template_spec.rb @@ -151,13 +151,11 @@ describe AttestationTemplate, type: :model do context 'and their value in the dossier are not nil' do before do dossier.champs - .filter { |champ| champ.libelle == 'libelleA' } - .first + .find { |champ| champ.libelle == 'libelleA' } .update(value: 'libelle1') dossier.champs - .filter { |champ| champ.libelle == 'libelleB' } - .first + .find { |champ| champ.libelle == 'libelleB' } .update(value: 'libelle2') end diff --git a/spec/models/concern/tags_substitution_concern_spec.rb b/spec/models/concern/tags_substitution_concern_spec.rb index e0aab4b00..cbc914d98 100644 --- a/spec/models/concern/tags_substitution_concern_spec.rb +++ b/spec/models/concern/tags_substitution_concern_spec.rb @@ -123,13 +123,11 @@ describe TagsSubstitutionConcern, type: :model do context 'and their value in the dossier are not nil' do before do dossier.champs - .filter { |champ| champ.libelle == 'libelleA' } - .first + .find { |champ| champ.libelle == 'libelleA' } .update(value: 'libelle1') dossier.champs - .filter { |champ| champ.libelle == 'libelleB' } - .first + .find { |champ| champ.libelle == 'libelleB' } .update(value: 'libelle2') end @@ -151,8 +149,7 @@ describe TagsSubstitutionConcern, type: :model do context 'and their value in the dossier are not nil' do before do dossier.champs - .filter { |champ| champ.libelle == "Intitulé de l'‘«\"évènement\"»’" } - .first + .find { |champ| champ.libelle == "Intitulé de l'‘«\"évènement\"»’" } .update(value: 'ceci est mon évènement') end @@ -302,13 +299,11 @@ describe TagsSubstitutionConcern, type: :model do context 'and its value in the dossier are not nil' do before do dossier.champs - .filter { |champ| champ.type_champ == TypeDeChamp.type_champs.fetch(:date) } - .first + .find { |champ| champ.type_champ == TypeDeChamp.type_champs.fetch(:date) } .update(value: '2017-04-15') dossier.champs - .filter { |champ| champ.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } - .first + .find { |champ| champ.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } .update(value: '2017-09-13 09:00') end From 9b324614b8621ed7f09b3d127df8804503feaa63 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:11:46 +0200 Subject: [PATCH 11/26] transform_values instead of map+to_h --- app/models/procedure_presentation.rb | 3 +-- .../administrateur_usage_statistics_service.rb | 11 +++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index 64515f196..82d8ca2dc 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -295,8 +295,7 @@ class ProcedurePresentation < ApplicationRecord def valid_columns_for_table(table) @column_whitelist ||= fields .group_by { |field| field['table'] } - .map { |table, fields| [table, Set.new(fields.pluck('column'))] } - .to_h + .transform_values { |fields| Set.new(fields.pluck('column')) } @column_whitelist[table] || [] end diff --git a/app/services/administrateur_usage_statistics_service.rb b/app/services/administrateur_usage_statistics_service.rb index 16d220efb..6377ce4e7 100644 --- a/app/services/administrateur_usage_statistics_service.rb +++ b/app/services/administrateur_usage_statistics_service.rb @@ -81,16 +81,12 @@ class AdministrateurUsageStatisticsService with_default( 0, nb_dossiers_by_administrateur_id_and_procedure_id_and_synthetic_state[administrateur_id] - .map do |procedure_id, nb_dossiers_by_synthetic_state| - [ - procedure_id, - nb_dossiers_by_synthetic_state + .transform_values do |nb_dossiers_by_synthetic_state| + nb_dossiers_by_synthetic_state .reject { |synthetic_state, _count| ['brouillon', 'archive'].include?(synthetic_state) } .map { |_synthetic_state, count| count } .sum - ] end - .to_h ) end @@ -105,8 +101,7 @@ class AdministrateurUsageStatisticsService .reject { |procedure_id, _nb_dossiers_by_synthetic_state| is_brouillon(procedure_id) } .flat_map { |_procedure_id, nb_dossiers_by_synthetic_state| nb_dossiers_by_synthetic_state.to_a } .group_by { |synthetic_state, _count| synthetic_state } - .map { |synthetic_state, synthetic_states_and_counts| [synthetic_state, synthetic_states_and_counts.map { |_synthetic_state, count| count }.sum] } - .to_h + .transform_values { |synthetic_states_and_counts| synthetic_states_and_counts.map { |_synthetic_state, count| count }.sum } ) end From f7ebbdc7c2eb65aa1c02722a4870012565f7a550 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:12:21 +0200 Subject: [PATCH 12/26] use match if possible --- app/mailers/application_mailer.rb | 2 +- app/mailers/devise_user_mailer.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index bc613b853..8efa47136 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -9,7 +9,7 @@ class ApplicationMailer < ActionMailer::Base end rescue_from Net::SMTPServerBusy do |error| - if error.message =~ /unexpected recipients/ + if /unexpected recipients/.match?(error.message) message.perform_deliveries = false end end diff --git a/app/mailers/devise_user_mailer.rb b/app/mailers/devise_user_mailer.rb index 06a9ead28..9377e9f10 100644 --- a/app/mailers/devise_user_mailer.rb +++ b/app/mailers/devise_user_mailer.rb @@ -10,7 +10,7 @@ class DeviseUserMailer < Devise::Mailer end rescue_from Net::SMTPServerBusy do |error| - if error.message =~ /unexpected recipients/ + if /unexpected recipients/.match?(error.message) message.perform_deliveries = false end end From 2cb3b1c68e1514ed749980a8c60dde97ef8c8d3a Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:14:23 +0200 Subject: [PATCH 13/26] more idiomatic count --- app/controllers/stats_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/stats_controller.rb b/app/controllers/stats_controller.rb index 5051f4153..a39604a1e 100644 --- a/app/controllers/stats_controller.rb +++ b/app/controllers/stats_controller.rb @@ -360,7 +360,7 @@ class StatsController < ApplicationController if weekly_dossiers_count == 0 result = 0 else - weekly_dossier_with_avis_count = weekly_dossiers.filter { |dossier| dossier.avis.present? }.count + weekly_dossier_with_avis_count = weekly_dossiers.count { |dossier| dossier.avis.present? } result = percentage(weekly_dossier_with_avis_count, weekly_dossiers_count) end From 560e1fc4fe68489270691199f1f80d5b9a381145 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:14:48 +0200 Subject: [PATCH 14/26] tag.span instead of content_tag --- app/helpers/champ_helper.rb | 4 ++-- app/helpers/dossier_helper.rb | 4 ++-- app/helpers/procedure_helper.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/helpers/champ_helper.rb b/app/helpers/champ_helper.rb index a9c25965b..41dcf4162 100644 --- a/app/helpers/champ_helper.rb +++ b/app/helpers/champ_helper.rb @@ -37,7 +37,7 @@ module ChampHelper when GeoArea.sources.fetch(:cadastre) capture do concat "Parcelle n° #{geo_area.numero} - Feuille #{geo_area.code_arr} #{geo_area.section} #{geo_area.feuille} - #{geo_area.surface_parcelle.round} m" - concat content_tag(:sup, "2") + concat tag.sup("2") end when GeoArea.sources.fetch(:quartier_prioritaire) "#{geo_area.commune} : #{geo_area.nom}" @@ -48,7 +48,7 @@ module ChampHelper if geo_area.area.present? capture do concat "Une aire de surface #{geo_area.area} m" - concat content_tag(:sup, "2") + concat tag.sup("2") end else "Une aire de surface inconnue" diff --git a/app/helpers/dossier_helper.rb b/app/helpers/dossier_helper.rb index c0c2d6435..a67f65713 100644 --- a/app/helpers/dossier_helper.rb +++ b/app/helpers/dossier_helper.rb @@ -75,7 +75,7 @@ module DossierHelper def status_badge(state) status_text = dossier_display_state(state, lower: true) status_class = state.tr('_', '-') - content_tag(:span, status_text, class: "label #{status_class} ") + tag.span(status_text, class: "label #{status_class} ") end def deletion_reason_badge(reason) @@ -87,7 +87,7 @@ module DossierHelper status_class = 'unknown' end - content_tag(:span, status_text, class: "label #{status_class} ") + tag.span(status_text, class: "label #{status_class} ") end def demandeur_dossier(dossier) diff --git a/app/helpers/procedure_helper.rb b/app/helpers/procedure_helper.rb index 4124d3c1d..6188838e5 100644 --- a/app/helpers/procedure_helper.rb +++ b/app/helpers/procedure_helper.rb @@ -8,7 +8,7 @@ module ProcedureHelper end def procedure_libelle(procedure) - parts = procedure.brouillon? ? [content_tag(:span, 'démarche en test', class: 'badge')] : [] + parts = procedure.brouillon? ? [tag.span('démarche en test', class: 'badge')] : [] parts << procedure.libelle safe_join(parts, ' ') end From bdfc25c9283ac1651d5f2848f9d2b7602b4d8c08 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:15:14 +0200 Subject: [PATCH 15/26] argument expand path --- config.ru | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.ru b/config.ru index 25551f99a..3d607b486 100644 --- a/config.ru +++ b/config.ru @@ -1,5 +1,5 @@ # This file is used by Rack-based servers to start the application. -require ::File.expand_path('../config/environment', __FILE__) +require ::File.expand_path('config/environment', __dir__) run Rails.application From 4053198e786d4e2222949c427a8937fc030a7c1d Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:19:44 +0200 Subject: [PATCH 16/26] add disabled new rules to rubocop.yml --- .rubocop.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index cce3f354a..bc2ced334 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1315,3 +1315,34 @@ Style/YodaCondition: Style/ZeroLengthPredicate: Enabled: true + +Rails/ActiveRecordCallbacksOrder: + Enabled: false +Rails/AfterCommitOverride: + Enabled: false +Rails/FindById: + Enabled: false +Rails/Inquiry: + Enabled: false +Rails/MailerName: + Enabled: false +Rails/MatchRoute: + Enabled: false +Rails/NegateInclude: + Enabled: false +Rails/Pluck: + Enabled: false +Rails/PluckInWhere: + Enabled: false +Rails/RenderInline: + Enabled: false +Rails/RenderPlainText: + Enabled: false +Rails/ShortI18n: + Enabled: false +Rails/SquishedSQLHeredocs: + Enabled: false +Rails/WhereExists: + Enabled: false +Rails/WhereNot: + Enabled: false From 5db826bc8d9a7a9e64b567aaa90e9dca2d69ad25 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 24 Sep 2020 15:28:46 +0200 Subject: [PATCH 17/26] enable the rules that are already passing --- .rubocop.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index bc2ced334..0d27b6c97 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1319,30 +1319,30 @@ Style/ZeroLengthPredicate: Rails/ActiveRecordCallbacksOrder: Enabled: false Rails/AfterCommitOverride: - Enabled: false + Enabled: true Rails/FindById: - Enabled: false + Enabled: true Rails/Inquiry: - Enabled: false + Enabled: true Rails/MailerName: - Enabled: false + Enabled: true Rails/MatchRoute: - Enabled: false + Enabled: true Rails/NegateInclude: Enabled: false Rails/Pluck: Enabled: false Rails/PluckInWhere: - Enabled: false + Enabled: true Rails/RenderInline: - Enabled: false + Enabled: true Rails/RenderPlainText: - Enabled: false + Enabled: true Rails/ShortI18n: - Enabled: false + Enabled: true Rails/SquishedSQLHeredocs: - Enabled: false + Enabled: true Rails/WhereExists: - Enabled: false + Enabled: true Rails/WhereNot: - Enabled: false + Enabled: true From 36da8455014e99ca492cc3200e227fd5c367befe Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 25 Sep 2020 13:59:07 +0000 Subject: [PATCH 18/26] Bump browser from 4.2.0 to 5.0.0 Bumps [browser](https://github.com/fnando/browser) from 4.2.0 to 5.0.0. - [Release notes](https://github.com/fnando/browser/releases) - [Changelog](https://github.com/fnando/browser/blob/main/CHANGELOG.md) - [Commits](https://github.com/fnando/browser/compare/v4.2.0...v5.0.0) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index a58a0188e..619975e7e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -127,7 +127,7 @@ GEM bootstrap-wysihtml5-rails (0.3.3.8) railties (>= 3.0) brakeman (4.9.1) - browser (4.2.0) + browser (5.0.0) builder (3.2.4) byebug (11.1.3) capybara (3.33.0) From 12ad186c256a103d363d6f369f66a4ffa01addcf Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 25 Sep 2020 14:17:35 +0000 Subject: [PATCH 19/26] Bump geocoder from 1.6.2 to 1.6.3 Bumps [geocoder](https://github.com/alexreisner/geocoder) from 1.6.2 to 1.6.3. - [Release notes](https://github.com/alexreisner/geocoder/releases) - [Changelog](https://github.com/alexreisner/geocoder/blob/master/CHANGELOG.md) - [Commits](https://github.com/alexreisner/geocoder/compare/v1.6.2...v1.6.3) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 619975e7e..61cdec41e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -265,7 +265,7 @@ GEM et-orbi (~> 1.1, >= 1.1.8) raabro (~> 1.1) geo_coord (0.1.0) - geocoder (1.6.2) + geocoder (1.6.3) globalid (0.4.2) activesupport (>= 4.2.0) gon (6.3.2) From d8d32a7fc8412a596d1dd8c188f3da3e44822215 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 25 Sep 2020 14:53:37 +0000 Subject: [PATCH 20/26] Bump jwt from 2.2.1 to 2.2.2 Bumps [jwt](https://github.com/jwt/ruby-jwt) from 2.2.1 to 2.2.2. - [Release notes](https://github.com/jwt/ruby-jwt/releases) - [Changelog](https://github.com/jwt/ruby-jwt/blob/master/CHANGELOG.md) - [Commits](https://github.com/jwt/ruby-jwt/compare/v2.2.1...v2.2.2) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 61cdec41e..61d09f8b8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -353,7 +353,7 @@ GEM aes_key_wrap bindata jsonapi-renderer (0.2.2) - jwt (2.2.1) + jwt (2.2.2) kaminari (1.2.1) activesupport (>= 4.1.0) kaminari-actionview (= 1.2.1) From 9cba957c605f7a3539ff9e3f5bcd913733097023 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2020 05:03:00 +0000 Subject: [PATCH 21/26] Bump rubocop from 0.91.1 to 0.92.0 Bumps [rubocop](https://github.com/rubocop-hq/rubocop) from 0.91.1 to 0.92.0. - [Release notes](https://github.com/rubocop-hq/rubocop/releases) - [Changelog](https://github.com/rubocop-hq/rubocop/blob/master/CHANGELOG.md) - [Commits](https://github.com/rubocop-hq/rubocop/compare/v0.91.1...v0.92.0) Signed-off-by: dependabot[bot] --- Gemfile.lock | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 61d09f8b8..1c5ffbb5c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -449,7 +449,7 @@ GEM webfinger (>= 1.0.1) orm_adapter (0.5.0) parallel (1.19.2) - parser (2.7.1.4) + parser (2.7.1.5) ast (~> 2.4.1) pdf-core (0.7.0) pg (1.2.3) @@ -596,17 +596,18 @@ GEM rspec-support (3.9.3) rspec_junit_formatter (0.4.1) rspec-core (>= 2, < 4, != 2.12.0) - rubocop (0.91.1) + rubocop (0.92.0) parallel (~> 1.10) - parser (>= 2.7.1.1) + parser (>= 2.7.1.5) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.7) rexml - rubocop-ast (>= 0.4.0, < 1.0) + rubocop-ast (>= 0.5.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 2.0) - rubocop-ast (0.4.2) - parser (>= 2.7.1.4) + rubocop-ast (0.7.0) + parser (>= 2.7.1.5) + strscan (>= 1.0.0) rubocop-performance (1.8.1) rubocop (>= 0.87.0) rubocop-ast (>= 0.4.0) @@ -688,6 +689,7 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) + strscan (1.0.3) swd (1.1.2) activesupport (>= 3) attr_required (>= 0.0.5) From bf02238662f9ea0bfa5d0b2c22aeef5c2e212d06 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 28 Sep 2020 15:55:18 +0200 Subject: [PATCH 22/26] fix for missing user_sign_in_count --- app/services/administrateur_usage_statistics_service.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/administrateur_usage_statistics_service.rb b/app/services/administrateur_usage_statistics_service.rb index 6377ce4e7..1623b34c5 100644 --- a/app/services/administrateur_usage_statistics_service.rb +++ b/app/services/administrateur_usage_statistics_service.rb @@ -27,7 +27,7 @@ class AdministrateurUsageStatisticsService nb_dossiers_roi = nb_dossiers_by_procedure_id.reject { |procedure_id, _count| is_brouillon(procedure_id) }.map { |_procedure_id, count| count }.sum result = { - ds_sign_in_count: administrateur.user.sign_in_count, + ds_sign_in_count: administrateur&.user&.sign_in_count, ds_created_at: administrateur.created_at, ds_active: administrateur.user.active?, ds_id: administrateur.id, @@ -62,11 +62,11 @@ class AdministrateurUsageStatisticsService admin_roi_high: nb_dossiers_roi * 17 } - if administrateur.user.current_sign_in_at.present? + if administrateur&.user&.current_sign_in_at.present? result[:ds_current_sign_in_at] = administrateur.user.current_sign_in_at end - if administrateur.user.last_sign_in_at.present? + if administrateur&.user&.last_sign_in_at.present? result[:ds_last_sign_in_at] = administrateur.user.last_sign_in_at end From 245e9e59c7a84d4ec39a034043c4c99c7330463d Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 28 Sep 2020 14:37:34 +0200 Subject: [PATCH 23/26] do not run ApiEntreprise jobs on missing etablissements --- app/jobs/api_entreprise/job.rb | 4 ++++ spec/jobs/api_entreprise/association_job_spec.rb | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/app/jobs/api_entreprise/job.rb b/app/jobs/api_entreprise/job.rb index 371d769e7..25e299b6f 100644 --- a/app/jobs/api_entreprise/job.rb +++ b/app/jobs/api_entreprise/job.rb @@ -3,6 +3,10 @@ class ApiEntreprise::Job < ApplicationJob DEFAULT_MAX_ATTEMPTS_API_ENTREPRISE_JOBS = 5 + # If by the time the job runs the Etablissement has been deleted + # (it can happen through EtablissementUpdateJob for instance), ignore the job + discard_on ActiveRecord::RecordNotFound + rescue_from(ApiEntreprise::API::ResourceNotFound) do |exception| error(self, exception) end diff --git a/spec/jobs/api_entreprise/association_job_spec.rb b/spec/jobs/api_entreprise/association_job_spec.rb index 8d8b3cb94..e77e81edf 100644 --- a/spec/jobs/api_entreprise/association_job_spec.rb +++ b/spec/jobs/api_entreprise/association_job_spec.rb @@ -18,4 +18,14 @@ RSpec.describe ApiEntreprise::AssociationJob, type: :job do subject expect(Etablissement.find(etablissement.id).association_rna).to eq('W595001988') end + + context "when the etablissement has been deleted" do + before do + allow_any_instance_of(Etablissement).to receive(:find) { raise ActiveRecord::RecordNotFound } + end + + it "ignores the error" do + expect { subject }.not_to raise_error + end + end end From 5ed491b958836af0d20369b532baeab7175f583f Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 29 Sep 2020 13:59:57 +0200 Subject: [PATCH 24/26] export are kept for 1 hour --- app/models/export.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/export.rb b/app/models/export.rb index 508c63792..c4a36ca7a 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -8,7 +8,7 @@ # updated_at :datetime not null # class Export < ApplicationRecord - MAX_DUREE_CONSERVATION_EXPORT = 15.minutes + MAX_DUREE_CONSERVATION_EXPORT = 1.hour enum format: { csv: 'csv', From 29ef5e67855418552179bea8ff37846a44ef476d Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 29 Sep 2020 14:00:31 +0200 Subject: [PATCH 25/26] compute async only after create --- app/models/export.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/export.rb b/app/models/export.rb index c4a36ca7a..b64f35cfe 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -24,7 +24,7 @@ class Export < ApplicationRecord scope :stale, -> { where('updated_at < ?', (Time.zone.now - MAX_DUREE_CONSERVATION_EXPORT)) } - after_save_commit :compute_async + after_create_commit :compute_async def compute_async ExportJob.perform_later(self) From 1eb780b0c9f2185cce485d4061b44d5b99abb217 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 29 Sep 2020 14:01:27 +0200 Subject: [PATCH 26/26] convert sheet_name to ascii before tuncate to 30 to ensure length < 31 bytes --- app/services/procedure_export_service.rb | 6 +++++- spec/services/procedure_export_service_spec.rb | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/services/procedure_export_service.rb b/app/services/procedure_export_service.rb index 66a325a66..5aa7a67de 100644 --- a/app/services/procedure_export_service.rb +++ b/app/services/procedure_export_service.rb @@ -73,7 +73,11 @@ class ProcedureExportService { instances: table.last, sheet_name: table.first } end.merge(DEFAULT_STYLES) - options[:sheet_name] = options[:sheet_name].truncate(30) + # transliterate: convert to ASCII characteres + # to ensure truncate respects 30 bytes + options[:sheet_name] = I18n.transliterate(options[:sheet_name], '') + .truncate(30, omission: '') + options end end diff --git a/spec/services/procedure_export_service_spec.rb b/spec/services/procedure_export_service_spec.rb index 741f9bdd1..930021394 100644 --- a/spec/services/procedure_export_service_spec.rb +++ b/spec/services/procedure_export_service_spec.rb @@ -356,10 +356,10 @@ describe ProcedureExportService do end end - context 'with long libelle' do + context 'with long libelle composed of utf8 characteres' do before do procedure.types_de_champ.each do |c| - c.update!(libelle: "#{c.id} - Quam rem nam maiores numquam dolorem nesciunt. Cum et possimus et aut. Fugit voluptas qui qui.") + c.update!(libelle: "#{c.id} - éééé ééé ééé ééééééé ééééééé ééééééé éééééééé. ééé éé éééééééé éé ééé. ééééé éééééééé ééé ééé.") end champ_repetition.champs.each do |c| c.type_de_champ.update!(libelle: "#{c.id} - Quam rem nam maiores numquam dolorem nesciunt. Cum et possimus et aut. Fugit voluptas qui qui.")