From b5ed74c09d7309a44c2a86122b00d32f22f48b44 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 14:30:05 +0200 Subject: [PATCH 01/21] dossiers: always allow to create a new dossier Turns out this is confusing for users (some UI elements are present for some procedures, but not for others). Better enable it for everyone. --- app/views/users/dossiers/_dossier_actions.html.haml | 2 +- .../users/dossiers/_dossier_actions.html.haml_spec.rb | 11 +++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/app/views/users/dossiers/_dossier_actions.html.haml b/app/views/users/dossiers/_dossier_actions.html.haml index 57d39ceb8..cc9cc58ed 100644 --- a/app/views/users/dossiers/_dossier_actions.html.haml +++ b/app/views/users/dossiers/_dossier_actions.html.haml @@ -1,5 +1,5 @@ - has_delete_action = dossier.can_be_deleted_by_user? -- has_new_dossier_action = dossier.procedure.expects_multiple_submissions? && dossier.procedure.accepts_new_dossiers? +- has_new_dossier_action = dossier.procedure.accepts_new_dossiers? - has_actions = has_delete_action || has_new_dossier_action diff --git a/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb b/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb index 61c2ed107..1e3f19431 100644 --- a/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb +++ b/spec/views/users/dossiers/_dossier_actions.html.haml_spec.rb @@ -1,5 +1,5 @@ describe 'users/dossiers/dossier_actions.html.haml', type: :view do - let(:procedure) { create(:procedure, :published, expects_multiple_submissions: true) } + let(:procedure) { create(:procedure, :published) } let(:dossier) { create(:dossier, :en_construction, procedure: procedure) } subject { render 'users/dossiers/dossier_actions.html.haml', dossier: dossier } @@ -12,18 +12,13 @@ describe 'users/dossiers/dossier_actions.html.haml', type: :view do it { is_expected.not_to have_link('Supprimer le dossier') } end - context 'when the procedure doesn’t expect multiple submissions' do - let(:procedure) { create(:procedure, :published, expects_multiple_submissions: false) } - it { is_expected.not_to have_link('Commencer un autre dossier') } - end - context 'when the procedure is closed' do - let(:procedure) { create(:procedure, :archived, expects_multiple_submissions: true) } + let(:procedure) { create(:procedure, :archived) } it { is_expected.not_to have_link('Commencer un autre dossier') } end context 'when there are no actions to display' do - let(:procedure) { create(:procedure, :published, expects_multiple_submissions: false) } + let(:procedure) { create(:procedure, :archived) } let(:dossier) { create(:dossier, :accepte, procedure: procedure) } it 'doesn’t render the menu at all' do From d77a5552b7f7418e9454552b1575254a9c2c3641 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 14:39:47 +0200 Subject: [PATCH 02/21] dossiers: allow to start a new dossier after submitting one --- app/views/users/dossiers/merci.html.haml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/users/dossiers/merci.html.haml b/app/views/users/dossiers/merci.html.haml index 5d411136b..23ec4da75 100644 --- a/app/views/users/dossiers/merci.html.haml +++ b/app/views/users/dossiers/merci.html.haml @@ -20,4 +20,6 @@ et %b échanger avec un instructeur. - = link_to 'Accéder à votre dossier', dossier_path(@dossier), class: 'button large primary' + .flex.column.align-center + = link_to 'Accéder à votre dossier', dossier_path(@dossier), class: 'button large primary' + = link_to 'Déposer un autre dossier', procedure_lien(@dossier.procedure) From ecfccae6f0e79a83ed7ee070415c1200cbde491a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 12:44:39 +0000 Subject: [PATCH 03/21] dossiers: remove support for `Dossier#expects_multiple_submissions` --- app/controllers/admin/procedures_controller.rb | 2 +- app/models/dossier.rb | 2 ++ app/views/admin/procedures/_informations.html.haml | 9 --------- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 19c0758aa..ae22689a5 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -265,7 +265,7 @@ class Admin::ProceduresController < AdminController end def procedure_params - editable_params = [:libelle, :description, :organisation, :direction, :lien_site_web, :cadre_juridique, :deliberation, :notice, :expects_multiple_submissions, :web_hook_url, :euro_flag, :logo, :auto_archive_on] + editable_params = [:libelle, :description, :organisation, :direction, :lien_site_web, :cadre_juridique, :deliberation, :notice, :web_hook_url, :euro_flag, :logo, :auto_archive_on] if @procedure&.locked? params.require(:procedure).permit(*editable_params) else diff --git a/app/models/dossier.rb b/app/models/dossier.rb index df3133982..025f145f1 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1,6 +1,8 @@ class Dossier < ApplicationRecord include DossierFilteringConcern + self.ignored_columns = [:expects_multiple_submissions] + enum state: { brouillon: 'brouillon', en_construction: 'en_construction', diff --git a/app/views/admin/procedures/_informations.html.haml b/app/views/admin/procedures/_informations.html.haml index 746305403..33ba974e9 100644 --- a/app/views/admin/procedures/_informations.html.haml +++ b/app/views/admin/procedures/_informations.html.haml @@ -127,15 +127,6 @@ .col-md-6 %h4 Options avancées - = f.label :expects_multiple_submissions do - = f.check_box :expects_multiple_submissions - Ajuster pour le dépôt récurrent de dossiers - %p.help-block - %i.fa.fa-info-circle - Si cette démarche est conçue pour qu’une même personne y dépose régulièrement plusieurs - dossiers, l’interface est ajustée pour rendre plus facile la création de plusieurs dossiers - à la suite. - - if Flipflop.web_hook? %label{ for: :web_hook_url } Lien de rappel HTTP (webhook) = f.text_field :web_hook_url, class: 'form-control', placeholder: 'https://callback.exemple.fr/' From d05bab3df3081ab9daefa04435bb04baaa2b1c74 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 8 Jul 2019 15:19:43 +0000 Subject: [PATCH 04/21] france_connect: refactor specs --- .../particulier_controller_spec.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 7e4a00f15..18c4b56cf 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -71,11 +71,19 @@ describe FranceConnect::ParticulierController, type: :controller do end context 'when france_connect_particulier_id does not have an associate user' do - it { is_expected.to redirect_to(root_path) } + context 'when the email address is not used yet' do + it { expect { subject }.to change(User, :count).by(1) } + it { is_expected.to redirect_to(root_path) } + end - it do - subject - expect(User.find_by(email: email)).not_to be_nil + context 'when the email address is already used' do + let!(:user) { create(:user, email: email, france_connect_information: nil) } + + it 'associates the france_connect infos with the existing user' do + expect { subject }.not_to change(User, :count) + expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) + expect(subject).to redirect_to(root_path) + end end end end From 6b27ac85141e4ca7bfe03aa4f17e91948fa34113 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 8 Jul 2019 17:28:35 +0200 Subject: [PATCH 05/21] france_connect: make existing user lookup case-insensitive Fix #4053 --- .../france_connect/particulier_controller.rb | 2 +- .../france_connect/particulier_controller_spec.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index a1f95b292..745d46be1 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -13,7 +13,7 @@ class FranceConnect::ParticulierController < ApplicationController fetched_fci.tap(&:save) if fci.user.nil? - user = User.find_or_create_by(email: fci.email_france_connect) do |new_user| + user = User.find_or_create_by(email: fci.email_france_connect.downcase) do |new_user| new_user.password = Devise.friendly_token[0, 20] new_user.confirmed_at = Time.zone.now end diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 18c4b56cf..4e1f1f66b 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -85,6 +85,17 @@ describe FranceConnect::ParticulierController, type: :controller do expect(subject).to redirect_to(root_path) end end + + context 'when a differently cased email address is already used' do + let(:email) { 'TEST@test.com' } + let!(:user) { create(:user, email: email.downcase, france_connect_information: nil) } + + it 'associates the france_connect infos with the existing user' do + expect { subject }.not_to change(User, :count) + expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) + expect(subject).to redirect_to(root_path) + end + end end end From 06afc3890fc9d385c8ee367debdec2fca7966d9a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 8 Jul 2019 17:30:07 +0200 Subject: [PATCH 06/21] france_connect: raise an error if some validation error occurs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a validation error occurs in `find_or_create`, an object without `id` will be returned–and the code will crash soon after. Ensure that we crash immediately, so that we can report the root cause (the validation error) instead of a seemingly-unrelated crash later. --- app/controllers/france_connect/particulier_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/france_connect/particulier_controller.rb b/app/controllers/france_connect/particulier_controller.rb index 745d46be1..49f97f180 100644 --- a/app/controllers/france_connect/particulier_controller.rb +++ b/app/controllers/france_connect/particulier_controller.rb @@ -13,7 +13,7 @@ class FranceConnect::ParticulierController < ApplicationController fetched_fci.tap(&:save) if fci.user.nil? - user = User.find_or_create_by(email: fci.email_france_connect.downcase) do |new_user| + user = User.find_or_create_by!(email: fci.email_france_connect.downcase) do |new_user| new_user.password = Devise.friendly_token[0, 20] new_user.confirmed_at = Time.zone.now end From 1f3015e24fe63d8ca00369cf516a6c20c2b95135 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Tue, 9 Jul 2019 15:57:20 +0200 Subject: [PATCH 07/21] suppression de camille de la liste pipedrive --- app/lib/biz_dev.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/lib/biz_dev.rb b/app/lib/biz_dev.rb index d4a50b4ff..a54b081d1 100644 --- a/app/lib/biz_dev.rb +++ b/app/lib/biz_dev.rb @@ -1,10 +1,5 @@ module BizDev BIZ_DEV_MAPPING = { - 8 => - { - full_name: "Camille Garrigue", - pipedrive_id: 3189424 - }, 9 => { full_name: "Philippe Vrignaud", From 4c9846dcd1904a47212a628c5f598a4a98a3e617 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Tue, 9 Jul 2019 16:01:54 +0200 Subject: [PATCH 08/21] =?UTF-8?q?Mise=20=C3=A0=20jour=20du=20mail=20d'acti?= =?UTF-8?q?vation=20admin?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/views/administrateur_mailer/activate_before_expiration.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/administrateur_mailer/activate_before_expiration.haml b/app/views/administrateur_mailer/activate_before_expiration.haml index a5e0bea12..a0331b59a 100644 --- a/app/views/administrateur_mailer/activate_before_expiration.haml +++ b/app/views/administrateur_mailer/activate_before_expiration.haml @@ -15,4 +15,4 @@ Nous restons à votre disposition si vous avez besoin d’accompagnement. %p - = render partial: "layouts/mailers/bizdev_signature", locals: { author_name: "Camille Garrigue" } + = render partial: "layouts/mailers/bizdev_signature", locals: { author_name: "Benjamin Doberset" } From d29be1fe4174443dc96f02cb8c7d2dcf2fbf5142 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Tue, 9 Jul 2019 16:17:41 +0200 Subject: [PATCH 09/21] suppression de redouane de Pipedrive --- app/lib/biz_dev.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/lib/biz_dev.rb b/app/lib/biz_dev.rb index a54b081d1..52285e23d 100644 --- a/app/lib/biz_dev.rb +++ b/app/lib/biz_dev.rb @@ -9,11 +9,6 @@ module BizDev { full_name: "Benjamin Doberset", pipedrive_id: 4223834 - }, - 11 => - { - full_name: "Rédouane Bouchane", - pipedrive_id: 4438645 } } From f6c402deecee57661a4776977dd25934508cce41 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 9 Jul 2019 15:58:20 +0200 Subject: [PATCH 10/21] better default value for linked_drop_down_list --- .../components/TypesDeChampEditor/index.js | 1 - .../TypesDeChampEditor/typeDeChampsReducer.js | 12 ++++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/javascript/components/TypesDeChampEditor/index.js b/app/javascript/components/TypesDeChampEditor/index.js index e9da90fcb..dff850ac6 100644 --- a/app/javascript/components/TypesDeChampEditor/index.js +++ b/app/javascript/components/TypesDeChampEditor/index.js @@ -22,7 +22,6 @@ class TypesDeChampEditor extends Component { type_champ: 'text', types_de_champ: [], private: props.isAnnotation, - drop_down_list_value: '--Premier élément du menu--\n', libelle: `${ props.isAnnotation ? 'Nouvelle annotation' : 'Nouveau champ' } ${props.typeDeChampsTypes[0][0]}` diff --git a/app/javascript/components/TypesDeChampEditor/typeDeChampsReducer.js b/app/javascript/components/TypesDeChampEditor/typeDeChampsReducer.js index c3e52c6b7..5d27c8399 100644 --- a/app/javascript/components/TypesDeChampEditor/typeDeChampsReducer.js +++ b/app/javascript/components/TypesDeChampEditor/typeDeChampsReducer.js @@ -115,6 +115,18 @@ function updateTypeDeChamp( { typeDeChamp, field, value }, done ) { + if (field == 'type_champ' && !typeDeChamp.drop_down_list_value) { + switch (value) { + case 'linked_drop_down_list': + typeDeChamp.drop_down_list_value = + '--Fromage--\nbleu de sassenage\npicodon\n--Dessert--\néclair\ntarte aux pommes\n'; + break; + case 'drop_down_list': + case 'multiple_drop_down_list': + typeDeChamp.drop_down_list_value = '--Premier élément du menu--\n'; + } + } + typeDeChamp[field] = value; getUpdateHandler(typeDeChamp, state)(done); From c36b884adf4ad1bc8423d395eb2dc9c5733b235e Mon Sep 17 00:00:00 2001 From: benjaminhenkel Date: Wed, 10 Jul 2019 09:48:53 +0200 Subject: [PATCH 11/21] Update _state_button.html.haml changement faute d'orthographe --- app/views/gestionnaires/dossiers/_state_button.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/gestionnaires/dossiers/_state_button.html.haml b/app/views/gestionnaires/dossiers/_state_button.html.haml index e186ff2ee..6fa054804 100644 --- a/app/views/gestionnaires/dossiers/_state_button.html.haml +++ b/app/views/gestionnaires/dossiers/_state_button.html.haml @@ -11,7 +11,7 @@ %h4 En construction Vous permettez à l'usager de modifier ses réponses au formulaire %li - = link_to passer_en_instruction_gestionnaire_dossier_path(dossier.procedure, dossier), method: :post, data: { remote: true, confirm: "Confirmer vous le passage en instruction de ce dossier ?" } do + = link_to passer_en_instruction_gestionnaire_dossier_path(dossier.procedure, dossier), method: :post, data: { remote: true, confirm: "Confirmez-vous le passage en instruction de ce dossier ?" } do %span.icon.in-progress .dropdown-description %h4 Passer en instruction @@ -19,7 +19,7 @@ - if dossier.en_instruction? %li - = link_to repasser_en_construction_gestionnaire_dossier_path(dossier.procedure, dossier), method: :post, data: { remote:true, confirm: "Confirmer vous le passage en construction de ce dossier ?" } do + = link_to repasser_en_construction_gestionnaire_dossier_path(dossier.procedure, dossier), method: :post, data: { remote:true, confirm: "Confirmez-vous le passage en construction de ce dossier ?" } do %span.icon.edit .dropdown-description %h4 Repasser en construction From 56bc06cfbffaabaaf968b40db5cda73f40676637 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 10 Jul 2019 08:40:45 +0000 Subject: [PATCH 12/21] dossier: fix flaky spec for `nearing_end_of_retention` If the spec takes longer than 1s to run, the test would fail. --- spec/models/dossier_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index b6a96f225..5e087f3c3 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -31,7 +31,7 @@ describe Dossier do let(:procedure) { create(:procedure, duree_conservation_dossiers_dans_ds: 6) } let!(:young_dossier) { create(:dossier, procedure: procedure) } let!(:expiring_dossier) { create(:dossier, :en_instruction, en_instruction_at: 170.days.ago, procedure: procedure) } - let!(:just_expired_dossier) { create(:dossier, :en_instruction, en_instruction_at: (6.months + 1.hour + 1.second).ago, procedure: procedure) } + let!(:just_expired_dossier) { create(:dossier, :en_instruction, en_instruction_at: (6.months + 1.hour + 10.seconds).ago, procedure: procedure) } let!(:long_expired_dossier) { create(:dossier, :en_instruction, en_instruction_at: 1.year.ago, procedure: procedure) } context 'with default delay to end of retention' do From 58a8d017f2558f2cfac60e5648496f25251c4471 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 17:08:41 +0200 Subject: [PATCH 13/21] spec: cleanup dossier spec --- spec/models/dossier_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 5e087f3c3..5de1da3db 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require 'rails_helper' describe Dossier do include ActiveJob::TestHelper @@ -420,7 +420,6 @@ describe Dossier do it "send an email when the dossier is created for the very first time" do dossier = nil - ActiveJob::Base.queue_adapter = :test expect do perform_enqueued_jobs do dossier = Dossier.create(procedure: procedure, state: Dossier.states.fetch(:brouillon), user: user) From 2b2493b0e6838c4afea15e90f7014bf1cd64c180 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 16:53:50 +0200 Subject: [PATCH 14/21] profile: improve spec --- spec/features/users/change_email_spec.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/features/users/change_email_spec.rb b/spec/features/users/change_email_spec.rb index a2267733c..00933a698 100644 --- a/spec/features/users/change_email_spec.rb +++ b/spec/features/users/change_email_spec.rb @@ -19,14 +19,15 @@ feature 'Changing an email' do click_button 'Changer mon adresse' end - user.reload - expect(user.email).to eq(old_email) - expect(user.unconfirmed_email).to eq(new_email) + expect(page).to have_content(I18n.t('devise.registrations.update_needs_confirmation')) + expect(page).to have_content(old_email) + expect(page).to have_content(new_email) click_confirmation_link_for(new_email) - user.reload - expect(user.email).to eq(new_email) - expect(user.unconfirmed_email).to be_nil + expect(page).to have_content(I18n.t('devise.confirmations.confirmed')) + expect(page).not_to have_content(old_email) + expect(page).to have_content(new_email) + expect(user.reload.email).to eq(new_email) end end From 055fc63c45644c78d38d4cb62d218ceb4586f3ad Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 16:53:28 +0200 Subject: [PATCH 15/21] profile: improve email success message wording --- config/locales/devise.fr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/devise.fr.yml b/config/locales/devise.fr.yml index 30f3ca558..73ede86e3 100755 --- a/config/locales/devise.fr.yml +++ b/config/locales/devise.fr.yml @@ -42,7 +42,7 @@ fr: signed_up_but_inactive: "Vous êtes bien enregistré. Vous ne pouvez cependant pas vous connecter car votre compte n'est pas encore activé." signed_up_but_locked: "Vous êtes bien enregistré. Vous ne pouvez cependant pas vous connecter car votre compte est verrouillé." signed_up_but_unconfirmed: "Nous vous avons envoyé un email contenant un lien d'activation. Ouvrez ce lien pour activer votre compte." - update_needs_confirmation: "Votre compte a bien été mis à jour mais nous devons vérifier votre nouvelle adresse email. Merci de vérifier vos emails et de cliquer sur le lien d’activation pour finaliser la validation de votre nouvelle adresse." + update_needs_confirmation: "Vous devez confirmer votre nouvelle adresse email. Vérifiez vos emails, et cliquez sur le lien de confirmation pour confirmer votre changement d’adresse." updated: "Votre compte a été modifié avec succès." sessions: signed_in: "Connecté." From f8a1911625896cd446a0784430f1adf7e5ff94af Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 13:49:38 +0000 Subject: [PATCH 16/21] profile: display profile page even for single-role users --- app/views/layouts/_account_dropdown.haml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/layouts/_account_dropdown.haml b/app/views/layouts/_account_dropdown.haml index 03bd65201..181e23687 100644 --- a/app/views/layouts/_account_dropdown.haml +++ b/app/views/layouts/_account_dropdown.haml @@ -26,11 +26,11 @@ = link_to admin_procedures_path, class: "menu-item menu-link" do = image_tag "icons/switch-profile.svg" Passer en administrateur - %li - = link_to profil_path, class: "menu-item menu-link" do - = image_tag "icons/switch-profile.svg" - Voir mon profil + %li + = link_to profil_path, class: "menu-item menu-link" do + = image_tag "icons/switch-profile.svg" + Voir mon profil %li = link_to destroy_user_session_path, method: :delete, class: "menu-item menu-link" do = image_tag "icons/sign-out.svg" From 80074d6d82341b419e21ab5dd6bdec4864ebc179 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 14:21:45 +0000 Subject: [PATCH 17/21] profile: improve layout and wording of the confirmation message --- app/assets/stylesheets/new_design/card.scss | 8 ++++++++ app/assets/stylesheets/new_design/profil.scss | 4 ++++ app/views/root/patron.html.haml | 5 +++++ app/views/users/profil/show.html.haml | 14 +++++++++----- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/new_design/card.scss b/app/assets/stylesheets/new_design/card.scss index c745f88a0..122fb7aa8 100644 --- a/app/assets/stylesheets/new_design/card.scss +++ b/app/assets/stylesheets/new_design/card.scss @@ -21,6 +21,14 @@ } } + &.warning { + border-top: 8px solid $orange; + + .card-title { + color: $orange; + } + } + &.feedback { max-width: 600px; margin: 30px auto; diff --git a/app/assets/stylesheets/new_design/profil.scss b/app/assets/stylesheets/new_design/profil.scss index d98c451c3..1a3abdeb9 100644 --- a/app/assets/stylesheets/new_design/profil.scss +++ b/app/assets/stylesheets/new_design/profil.scss @@ -12,4 +12,8 @@ p { margin: 16px auto; } + + .email-address { + font-weight: bold; + } } diff --git a/app/views/root/patron.html.haml b/app/views/root/patron.html.haml index 5d6967001..fa905ce27 100644 --- a/app/views/root/patron.html.haml +++ b/app/views/root/patron.html.haml @@ -157,6 +157,11 @@ Titre de la carte mise en avant %p Et voici le contenu de la carte + .card.warning + .card-title + Titre de la carte d’avertissement + %p Et voici le contenu de la carte + .card.feedback .card-title Titre de la carte pour demander un avis diff --git a/app/views/users/profil/show.html.haml b/app/views/users/profil/show.html.haml index 674992d1b..58187a6ed 100644 --- a/app/views/users/profil/show.html.haml +++ b/app/views/users/profil/show.html.haml @@ -7,12 +7,16 @@ .card .card-title Coordonnées - %p Votre email est actuellement #{current_user.email} + %p + Votre email est actuellement + %span.email-address= current_user.email - if current_user.unconfirmed_email.present? - %p - Un email a été envoyé à #{current_user.unconfirmed_email}. - %br - Merci de vérifier vos emails et de cliquer sur le lien d’activation pour finaliser la validation de votre nouvelle adresse. + .card.warning + .card-title + Changement en attente : + %span.email-address= current_user.unconfirmed_email + %p + Pour finaliser votre changement d’adresse, vérifiez vos emails et cliquez sur le lien de confirmation. = form_for @current_user, url: update_email_path, method: :patch, html: { class: 'form' } do |f| = f.email_field :email, value: nil, placeholder: 'Nouvelle adresse email', required: true From 03fc555edf566a2e383281164411bcc4200a4c8a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 9 Jul 2019 17:08:27 +0200 Subject: [PATCH 18/21] profile: send an email when the account is already taken --- app/controllers/users/profil_controller.rb | 7 ++++++- app/mailers/user_mailer.rb | 8 +++++++ .../user_mailer/account_already_taken.haml | 20 ++++++++++++++++++ .../users/profil_controller_spec.rb | 10 +++++++-- spec/mailers/previews/user_mailer_preview.rb | 4 ++++ spec/mailers/user_mailer_spec.rb | 21 +++++++++++++++++++ 6 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 app/views/user_mailer/account_already_taken.haml create mode 100644 spec/mailers/user_mailer_spec.rb diff --git a/app/controllers/users/profil_controller.rb b/app/controllers/users/profil_controller.rb index f976a8775..13c9c7f20 100644 --- a/app/controllers/users/profil_controller.rb +++ b/app/controllers/users/profil_controller.rb @@ -12,8 +12,9 @@ module Users def update_email if @current_user.update(update_email_params) flash.notice = t('devise.registrations.update_needs_confirmation') - # to avoid leaking who has signed in elsif @current_user.errors&.details&.dig(:email)&.any? { |e| e[:error] == :taken } + UserMailer.account_already_taken(@current_user, requested_email).deliver_later + # avoid leaking information about whether an account with this email exists or not flash.notice = t('devise.registrations.update_needs_confirmation') else flash.alert = @current_user.errors.full_messages @@ -27,5 +28,9 @@ module Users def update_email_params params.require(:user).permit(:email) end + + def requested_email + update_email_params[:email] + end end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 2304d41fd..d0f7b91f6 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -8,4 +8,12 @@ class UserMailer < ApplicationMailer mail(to: user.email, subject: @subject) end + + def account_already_taken(user, requested_email) + @user = user + @requested_email = requested_email + @subject = "Changement d’adresse email" + + mail(to: requested_email, subject: @subject) + end end diff --git a/app/views/user_mailer/account_already_taken.haml b/app/views/user_mailer/account_already_taken.haml new file mode 100644 index 000000000..85cbf69ac --- /dev/null +++ b/app/views/user_mailer/account_already_taken.haml @@ -0,0 +1,20 @@ +- content_for(:title, @subject) + +%p + Bonjour, + +%p + L’utilisateur « #{@user.email} » a demandé le changement de son adresse vers « #{@requested_email} ». + +%p + Malheureusement, votre compte « #{@requested_email} » existe déjà. Nous ne pouvons pas fusionner automatiquement vos comptes. + +%p + %strong Nous ne pouvons donc pas effectuer le changement d’adresse email. + +%p + Si vous n'êtes pas à l’origine de cette demande, vous pouvez ignorer ce message. Et si vous avez besoin d’assistance, n’hésitez pas à nous contacter à + = succeed '.' do + = mail_to CONTACT_EMAIL + += render partial: "layouts/mailers/signature" diff --git a/spec/controllers/users/profil_controller_spec.rb b/spec/controllers/users/profil_controller_spec.rb index e151e2f82..38f46f2dd 100644 --- a/spec/controllers/users/profil_controller_spec.rb +++ b/spec/controllers/users/profil_controller_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Users::ProfilController, type: :controller do + include ActiveJob::TestHelper + let(:user) { create(:user) } before { sign_in(user) } @@ -34,13 +36,17 @@ describe Users::ProfilController, type: :controller do end context 'when the mail is already taken' do - let!(:user2) { create(:user) } + let(:existing_user) { create(:user) } before do - patch :update_email, params: { user: { email: user2.email } } + perform_enqueued_jobs do + patch :update_email, params: { user: { email: existing_user.email } } + end user.reload end + it { expect(user.unconfirmed_email).to be_nil } + it { expect(ActionMailer::Base.deliveries.last.to).to eq([existing_user.email]) } it { expect(response).to redirect_to(profil_path) } it { expect(flash.notice).to eq(I18n.t('devise.registrations.update_needs_confirmation')) } end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 4204d63be..e99994fa4 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -3,6 +3,10 @@ class UserMailerPreview < ActionMailer::Preview UserMailer.new_account_warning(user) end + def account_already_taken + UserMailer.account_already_taken(user, 'dircab@territoires.gouv.fr') + end + private def user diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb new file mode 100644 index 000000000..dd48194a5 --- /dev/null +++ b/spec/mailers/user_mailer_spec.rb @@ -0,0 +1,21 @@ +require "rails_helper" + +RSpec.describe UserMailer, type: :mailer do + let(:user) { build(:user) } + + describe '.new_account_warning' do + subject { described_class.new_account_warning(user) } + + it { expect(subject.to).to eq([user.email]) } + it { expect(subject.body).to include(user.email) } + end + + describe '.account_already_taken' do + let(:requested_email) { 'new@exemple.fr' } + + subject { described_class.account_already_taken(user, requested_email) } + + it { expect(subject.to).to eq([requested_email]) } + it { expect(subject.body).to include(requested_email) } + end +end From 3cb39c28405e678213e4ac6e68770fe2411f0b72 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 Jun 2019 17:12:44 +0200 Subject: [PATCH 19/21] Refactor message attachements to use active_storage --- .../gestionnaires/avis_controller.rb | 2 +- .../gestionnaires/dossiers_controller.rb | 2 +- app/controllers/support_controller.rb | 4 ++-- app/controllers/users/dossiers_controller.rb | 2 +- app/models/commentaire.rb | 22 ++++++++--------- app/models/dossier.rb | 2 +- app/services/commentaire_service.rb | 5 ++++ app/uploaders/local_downloader.rb | 2 +- .../gestionnaires/dossiers/print.html.haml | 2 +- .../shared/dossiers/_messagerie.html.haml | 2 +- .../shared/dossiers/messages/_form.html.haml | 4 ++-- .../dossiers/messages/_message.html.haml | 5 +++- app/views/support/index.html.haml | 2 +- config/application.rb | 2 -- config/initializers/urls.rb | 3 +++ .../gestionnaires/avis_controller_spec.rb | 10 ++------ .../gestionnaires/dossiers_controller_spec.rb | 12 +++++----- .../users/dossiers_controller_spec.rb | 14 +---------- spec/factories/commentaire.rb | 4 ++++ spec/services/commentaire_service_spec.rb | 24 +++++++++---------- 20 files changed, 59 insertions(+), 66 deletions(-) diff --git a/app/controllers/gestionnaires/avis_controller.rb b/app/controllers/gestionnaires/avis_controller.rb index e6b1e3e87..167ea9fb2 100644 --- a/app/controllers/gestionnaires/avis_controller.rb +++ b/app/controllers/gestionnaires/avis_controller.rb @@ -138,7 +138,7 @@ module Gestionnaires end def commentaire_params - params.require(:commentaire).permit(:body, :file) + params.require(:commentaire).permit(:body, :piece_jointe) end end end diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index 685db3f81..764f24b6c 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -166,7 +166,7 @@ module Gestionnaires end def commentaire_params - params.require(:commentaire).permit(:body, :file) + params.require(:commentaire).permit(:body, :piece_jointe) end def champs_private_params diff --git a/app/controllers/support_controller.rb b/app/controllers/support_controller.rb index 307407a79..73fd56348 100644 --- a/app/controllers/support_controller.rb +++ b/app/controllers/support_controller.rb @@ -52,7 +52,7 @@ class SupportController < ApplicationController email: email, phone: params[:phone], text: params[:text], - file: params[:file], + file: params[:piece_jointe], dossier_id: dossier&.id, browser: browser_name, tags: tags @@ -61,7 +61,7 @@ class SupportController < ApplicationController def create_commentaire attributes = { - file: params[:file], + piece_jointe: params[:piece_jointe], body: "[#{params[:subject]}]

#{params[:text]}" } commentaire = CommentaireService.build(current_user, dossier, attributes) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 9e599a194..3e13bbc46 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -354,7 +354,7 @@ module Users end def commentaire_params - params.require(:commentaire).permit(:body, :file) + params.require(:commentaire).permit(:body, :piece_jointe) end def passage_en_construction? diff --git a/app/models/commentaire.rb b/app/models/commentaire.rb index 0bf5356df..61e45d5f9 100644 --- a/app/models/commentaire.rb +++ b/app/models/commentaire.rb @@ -5,9 +5,10 @@ class Commentaire < ApplicationRecord belongs_to :gestionnaire mount_uploader :file, CommentaireFileUploader - validates :file, file_size: { maximum: 20.megabytes, message: "La taille du fichier doit être inférieure à 20 Mo" } - validate :is_virus_free? validate :messagerie_available?, on: :create + + has_one_attached :piece_jointe + validates :body, presence: { message: "Votre message ne peut être vide" } default_scope { order(created_at: :asc) } @@ -47,10 +48,15 @@ class Commentaire < ApplicationRecord end def file_url - if Flipflop.remote_storage? + if piece_jointe.attached? + if piece_jointe.virus_scanner.safe? + Rails.application.routes.url_helpers.url_for(piece_jointe) + end + elsif Flipflop.remote_storage? RemoteDownloader.new(file.path).url - else - file.url + elsif file&.url + # FIXME: this is horrible but used only in dev and will be removed after migration + File.join(LOCAL_DOWNLOAD_URL, file.url) end end @@ -74,12 +80,6 @@ class Commentaire < ApplicationRecord DossierMailer.notify_new_answer(dossier).deliver_later end - def is_virus_free? - if file.present? && file_changed? && !ClamavService.safe_file?(file.path) - errors.add(:file, "Virus détecté dans le fichier joint, merci de changer de fichier") - end - end - def messagerie_available? return if sent_by_system? if dossier.present? && !dossier.messagerie_available? diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 025f145f1..250da547c 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -117,7 +117,7 @@ class Dossier < ApplicationRecord scope :nearing_end_of_retention, -> (duration = '1 month') { joins(:procedure).where("en_instruction_at + (duree_conservation_dossiers_dans_ds * interval '1 month') - now() < interval ?", duration) } scope :since, -> (since) { where('dossiers.en_construction_at >= ?', since) } scope :for_api, -> { - includes(commentaires: [], + includes(commentaires: { piece_jointe_attachment: :blob }, champs: [ :geo_areas, :etablissement, diff --git a/app/services/commentaire_service.rb b/app/services/commentaire_service.rb index fa3b9a576..8708401a6 100644 --- a/app/services/commentaire_service.rb +++ b/app/services/commentaire_service.rb @@ -13,6 +13,11 @@ class CommentaireService def build_with_email(email, dossier, params) attributes = params.merge(email: email, dossier: dossier) + # For some reason ActiveStorage trows an error in tests if we passe an empty string here. + # I suspect it could be resolved in rails 6 by using explicit `attach()` + if attributes[:piece_jointe].blank? + attributes.delete(:piece_jointe) + end Commentaire.new(attributes) end end diff --git a/app/uploaders/local_downloader.rb b/app/uploaders/local_downloader.rb index 82f33d12f..011a29b52 100644 --- a/app/uploaders/local_downloader.rb +++ b/app/uploaders/local_downloader.rb @@ -14,7 +14,7 @@ class LocalDownloader end def url - @url ||= File.join(TPS::Application::URL, 'downloads', random_folder_name, "#{@filename_suffix}.#{@extension}") + @url ||= File.join(LOCAL_DOWNLOAD_URL, 'downloads', random_folder_name, "#{@filename_suffix}.#{@extension}") end protected diff --git a/app/views/gestionnaires/dossiers/print.html.haml b/app/views/gestionnaires/dossiers/print.html.haml index eb9f4f6ff..e41192349 100644 --- a/app/views/gestionnaires/dossiers/print.html.haml +++ b/app/views/gestionnaires/dossiers/print.html.haml @@ -61,7 +61,7 @@ .messagerie %ul.messages-list - - @dossier.commentaires.each do |commentaire| + - @dossier.commentaires.with_attached_piece_jointe.each do |commentaire| %li = render partial: "shared/dossiers/messages/message", locals: { commentaire: commentaire, connected_user: current_gestionnaire, messagerie_seen_at: nil } diff --git a/app/views/shared/dossiers/_messagerie.html.haml b/app/views/shared/dossiers/_messagerie.html.haml index de7c34713..123b5f510 100644 --- a/app/views/shared/dossiers/_messagerie.html.haml +++ b/app/views/shared/dossiers/_messagerie.html.haml @@ -1,6 +1,6 @@ .messagerie.container %ul.messages-list - - dossier.commentaires.each do |commentaire| + - dossier.commentaires.with_attached_piece_jointe.each do |commentaire| %li.message{ class: commentaire_is_from_me_class(commentaire, connected_user) } = render partial: "shared/dossiers/messages/message", locals: { commentaire: commentaire, connected_user: connected_user, messagerie_seen_at: messagerie_seen_at } diff --git a/app/views/shared/dossiers/messages/_form.html.haml b/app/views/shared/dossiers/messages/_form.html.haml index e6dbb38ee..594bec357 100644 --- a/app/views/shared/dossiers/messages/_form.html.haml +++ b/app/views/shared/dossiers/messages/_form.html.haml @@ -2,8 +2,8 @@ = f.text_area :body, rows: 5, placeholder: 'Répondre ici', required: true, class: 'message-textarea' .flex.justify-between.wrap %div - = f.file_field :file, id: :file, accept: commentaire.file.accept_extension_list - %label{ for: :file } + = f.file_field :piece_jointe, id: 'piece_jointe', direct_upload: true + %label{ for: :piece_jointe } .notice (taille max : 20 Mo) diff --git a/app/views/shared/dossiers/messages/_message.html.haml b/app/views/shared/dossiers/messages/_message.html.haml index 74f314ad8..4ccfa7707 100644 --- a/app/views/shared/dossiers/messages/_message.html.haml +++ b/app/views/shared/dossiers/messages/_message.html.haml @@ -10,7 +10,10 @@ = commentaire_date(commentaire) .rich-text= sanitize(simple_format(commentaire.body)) - - if commentaire.file.present? + - if commentaire.piece_jointe.attached? + .attachment-link + = render partial: "shared/attachment/show", locals: { attachment: commentaire.piece_jointe.attachment } + - elsif commentaire.file.present? .attachment-link = link_to commentaire.file_url, class: "button", target: "_blank", rel: "noopener", title: "Télécharger" do %span.icon.attachment diff --git a/app/views/support/index.html.haml b/app/views/support/index.html.haml index 212767bb9..1ce5483d2 100644 --- a/app/views/support/index.html.haml +++ b/app/views/support/index.html.haml @@ -76,7 +76,7 @@ Une capture d’écran peut nous aider à identifier plus facilement l’endroit à améliorer. .notice.hidden{ data: { 'contact-type-only': Helpscout::FormAdapter::TYPE_AUTRE } } Une capture d’écran peut nous aider à identifier plus facilement le problème. - = file_field_tag :file + = file_field_tag :piece_jointe = hidden_field_tag :tags, @tags&.join(',') diff --git a/config/application.rb b/config/application.rb index 663bcda18..97e20adce 100644 --- a/config/application.rb +++ b/config/application.rb @@ -31,8 +31,6 @@ module TPS config.assets.paths << Rails.root.join('app', 'assets', 'fonts') config.assets.precompile += ['.woff'] - URL = ENV['APP_HOST'] || "http://localhost:3000/" - config.active_job.queue_adapter = :delayed_job config.action_view.sanitized_allowed_tags = ActionView::Base.sanitized_allowed_tags + ['u'] diff --git a/config/initializers/urls.rb b/config/initializers/urls.rb index 11b34497b..7d279eec6 100644 --- a/config/initializers/urls.rb +++ b/config/initializers/urls.rb @@ -28,3 +28,6 @@ FAQ_ADMIN_URL = "https://faq.demarches-simplifiees.fr/collection/1-administrateu COMMENT_TROUVER_MA_DEMARCHE_URL = [FAQ_URL, "article", "59-comment-trouver-ma-demarche"].join("/") STATUS_PAGE_URL = "https://status.demarches-simplifiees.fr" MATOMO_IFRAME_URL = "https://stats.data.gouv.fr/index.php?module=CoreAdminHome&action=optOut&language=fr&&fontColor=333333&fontSize=16px&fontFamily=Muli" + +# FIXME: This is only used in dev in couple of places and should be removed after PJ migration +LOCAL_DOWNLOAD_URL = "http://#{ENV.fetch('APP_HOST', 'localhost:3000')}" diff --git a/spec/controllers/gestionnaires/avis_controller_spec.rb b/spec/controllers/gestionnaires/avis_controller_spec.rb index 074396ad1..14ae22034 100644 --- a/spec/controllers/gestionnaires/avis_controller_spec.rb +++ b/spec/controllers/gestionnaires/avis_controller_spec.rb @@ -92,7 +92,7 @@ describe Gestionnaires::AvisController, type: :controller do let(:file) { nil } let(:scan_result) { true } - subject { post :create_commentaire, params: { id: avis_without_answer.id, commentaire: { body: 'commentaire body', file: file } } } + subject { post :create_commentaire, params: { id: avis_without_answer.id, commentaire: { body: 'commentaire body', piece_jointe: file } } } before do allow(ClamavService).to receive(:safe_file?).and_return(scan_result) @@ -110,16 +110,10 @@ describe Gestionnaires::AvisController, type: :controller do it do subject - expect(Commentaire.last.file.path).to include("piece_justificative_0.pdf") + expect(Commentaire.last.piece_jointe.filename).to eq("piece_justificative_0.pdf") end it { expect { subject }.to change(Commentaire, :count).by(1) } - - context "and a virus" do - let(:scan_result) { false } - - it { expect { subject }.not_to change(Commentaire, :count) } - end end end diff --git a/spec/controllers/gestionnaires/dossiers_controller_spec.rb b/spec/controllers/gestionnaires/dossiers_controller_spec.rb index ca53d1210..532913960 100644 --- a/spec/controllers/gestionnaires/dossiers_controller_spec.rb +++ b/spec/controllers/gestionnaires/dossiers_controller_spec.rb @@ -350,15 +350,15 @@ describe Gestionnaires::DossiersController, type: :controller do expect(flash.notice).to be_present end - context "when the commentaire creation fails" do + context "when the commentaire created with virus file" do let(:scan_result) { false } - it "renders the messagerie page with the invalid commentaire" do - expect { subject }.not_to change(Commentaire, :count) + it "creates a commentaire (shows message that file have a virus)" do + expect { subject }.to change(Commentaire, :count).by(1) + expect(gestionnaire.followed_dossiers).to include(dossier) - expect(response).to render_template :messagerie - expect(flash.alert).to be_present - expect(assigns(:commentaire).body).to eq("avant\napres") + expect(response).to redirect_to(messagerie_gestionnaire_dossier_path(dossier.procedure, dossier)) + expect(flash.notice).to be_present end end end diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 1da3997af..129cf6075 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -806,7 +806,7 @@ describe Users::DossiersController, type: :controller do id: dossier.id, commentaire: { body: body, - file: file + piece_jointe: file } } } @@ -822,18 +822,6 @@ describe Users::DossiersController, type: :controller do expect(response).to redirect_to(messagerie_dossier_path(dossier)) expect(flash.notice).to be_present end - - context "when the commentaire creation fails" do - let(:scan_result) { false } - - it "renders the messagerie page with the invalid commentaire" do - expect { subject }.not_to change(Commentaire, :count) - - expect(response).to render_template :messagerie - expect(flash.alert).to be_present - expect(assigns(:commentaire).body).to eq("avant\napres") - end - end end describe '#ask_deletion' do diff --git a/spec/factories/commentaire.rb b/spec/factories/commentaire.rb index 811cf0501..dc092a50d 100644 --- a/spec/factories/commentaire.rb +++ b/spec/factories/commentaire.rb @@ -7,5 +7,9 @@ FactoryBot.define do commentaire.dossier = create :dossier, :en_construction end end + + trait :with_file do + file { Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'application/pdf') } + end end end diff --git a/spec/services/commentaire_service_spec.rb b/spec/services/commentaire_service_spec.rb index 1e80c92d3..5ab383243 100644 --- a/spec/services/commentaire_service_spec.rb +++ b/spec/services/commentaire_service_spec.rb @@ -1,24 +1,21 @@ require 'spec_helper' describe CommentaireService do + include ActiveJob::TestHelper + describe '.create' do let(:dossier) { create :dossier, :en_construction } let(:sender) { dossier.user } let(:body) { 'Contenu du message.' } let(:file) { nil } - let(:scan_result) { true } - subject(:commentaire) { CommentaireService.build(sender, dossier, { body: body, file: file }) } - - before do - allow(ClamavService).to receive(:safe_file?).and_return(scan_result) - end + subject(:commentaire) { CommentaireService.build(sender, dossier, { body: body, piece_jointe: file }) } it 'creates a new valid commentaire' do expect(commentaire.email).to eq sender.email expect(commentaire.dossier).to eq dossier expect(commentaire.body).to eq 'Contenu du message.' - expect(commentaire.file).to be_blank + expect(commentaire.piece_jointe.attached?).to be_falsey expect(commentaire).to be_valid end @@ -34,14 +31,15 @@ describe CommentaireService do context 'when it has a file' do let(:file) { Rack::Test::UploadedFile.new("./spec/fixtures/files/piece_justificative_0.pdf", 'application/pdf') } - it 'saves the attached file' do - expect(commentaire.file).to be_present - expect(commentaire).to be_valid + before do + expect(ClamavService).to receive(:safe_file?).and_return(true) end - context 'and a virus' do - let(:scan_result) { false } - it { expect(commentaire).not_to be_valid } + it 'saves the attached file' do + perform_enqueued_jobs do + commentaire.save + expect(commentaire.piece_jointe.attached?).to be_truthy + end end end end From be3283a9a8c49dba2b1c1b2f9e33c508b5b24e8d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 Jun 2019 17:16:45 +0200 Subject: [PATCH 20/21] Add commentaire pj migration task --- .../2019_05_29_migrate_commentaire_pj.rake | 37 ++++++++++++++++ .../2019_05_29_migrate_commentaire_pj_spec.rb | 42 +++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 lib/tasks/2019_05_29_migrate_commentaire_pj.rake create mode 100644 spec/lib/tasks/2019_05_29_migrate_commentaire_pj_spec.rb diff --git a/lib/tasks/2019_05_29_migrate_commentaire_pj.rake b/lib/tasks/2019_05_29_migrate_commentaire_pj.rake new file mode 100644 index 000000000..1725d40c0 --- /dev/null +++ b/lib/tasks/2019_05_29_migrate_commentaire_pj.rake @@ -0,0 +1,37 @@ +namespace :'2019_05_29_migrate_commentaire_pj' do + task run: :environment do + commentaires = Commentaire.where + .not(file: nil) + .left_joins(:piece_jointe_attachment) + .where('active_storage_attachments.id IS NULL') + .order(:created_at) + + limit = ENV['LIMIT'] + if limit + commentaires.limit!(limit.to_i) + end + + progress = ProgressReport.new(commentaires.count) + commentaires.find_each do |commentaire| + if commentaire.file.present? + uri = URI.parse(URI.escape(commentaire.file_url)) + response = Typhoeus.get(uri) + if response.success? + filename = commentaire.file.filename || commentaire.file_identifier + updated_at = commentaire.updated_at + dossier_updated_at = commentaire.dossier.updated_at + commentaire.piece_jointe.attach( + io: StringIO.new(response.body), + filename: filename, + content_type: commentaire.file.content_type, + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } + ) + commentaire.update_column(:updated_at, updated_at) + commentaire.dossier.update_column(:updated_at, dossier_updated_at) + end + end + progress.inc + end + progress.finish + end +end diff --git a/spec/lib/tasks/2019_05_29_migrate_commentaire_pj_spec.rb b/spec/lib/tasks/2019_05_29_migrate_commentaire_pj_spec.rb new file mode 100644 index 000000000..7df801b37 --- /dev/null +++ b/spec/lib/tasks/2019_05_29_migrate_commentaire_pj_spec.rb @@ -0,0 +1,42 @@ +describe '2019_05_29_migrate_commentaire_pj.rake' do + let(:rake_task) { Rake::Task['2019_05_29_migrate_commentaire_pj:run'] } + + let!(:commentaires) do + create(:commentaire) + create(:commentaire, :with_file) + create(:commentaire, :with_file) + end + + before do + Commentaire.all.each do |commentaire| + if commentaire.file.present? + stub_request(:get, commentaire.file_url) + .to_return(status: 200, body: File.read(commentaire.file.path)) + end + end + end + + after do + ENV['LIMIT'] = nil + rake_task.reenable + end + + it 'should migrate pj' do + comment_updated_at = Commentaire.last.updated_at + dossier_updated_at = Commentaire.last.dossier.updated_at + expect(Commentaire.all.map(&:piece_jointe).map(&:attached?)).to eq([false, false, false]) + rake_task.invoke + expect(Commentaire.where(file: nil).count).to eq(1) + expect(Commentaire.all.map(&:piece_jointe).map(&:attached?)).to eq([false, true, true]) + expect(Commentaire.last.updated_at).to eq(comment_updated_at) + expect(Commentaire.last.dossier.updated_at).to eq(dossier_updated_at) + end + + it 'should migrate pj within limit' do + expect(Commentaire.all.map(&:piece_jointe).map(&:attached?)).to eq([false, false, false]) + ENV['LIMIT'] = '1' + rake_task.invoke + expect(Commentaire.where(file: nil).count).to eq(1) + expect(Commentaire.all.map(&:piece_jointe).map(&:attached?)).to eq([false, true, false]) + end +end From 7b63ddfabb4012a21542100cedc7473da79ec56b Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 9 Jul 2019 15:57:48 +0200 Subject: [PATCH 21/21] Better champs factories --- spec/factories/champ.rb | 26 +++++++++++++------ spec/models/champ_spec.rb | 2 +- spec/models/dossier_spec.rb | 8 ++++-- spec/serializers/dossier_serializer_spec.rb | 2 +- .../procedure_export_v2_service_spec.rb | 13 ++-------- 5 files changed, 28 insertions(+), 23 deletions(-) diff --git a/spec/factories/champ.rb b/spec/factories/champ.rb index 0af61cbcb..114b25bdd 100644 --- a/spec/factories/champ.rb +++ b/spec/factories/champ.rb @@ -41,12 +41,12 @@ FactoryBot.define do factory :champ_date, class: 'Champs::DateChamp' do type_de_champ { create(:type_de_champ_date) } - value { 1.day.ago.iso8601 } + value { '2019-07-10' } end factory :champ_datetime, class: 'Champs::DatetimeChamp' do type_de_champ { create(:type_de_champ_datetime) } - value { 1.day.ago.iso8601 } + value { '15/09/1962 15:35' } end factory :champ_number, class: 'Champs::NumberChamp' do @@ -96,17 +96,17 @@ FactoryBot.define do factory :champ_drop_down_list, class: 'Champs::DropDownListChamp' do type_de_champ { create(:type_de_champ_drop_down_list) } - value { '' } + value { 'choix 1' } end factory :champ_multiple_drop_down_list, class: 'Champs::MultipleDropDownListChamp' do type_de_champ { create(:type_de_champ_multiple_drop_down_list) } - value { '' } + value { '["choix 1", "choix 2"]' } end factory :champ_linked_drop_down_list, class: 'Champs::LinkedDropDownListChamp' do type_de_champ { create(:type_de_champ_linked_drop_down_list) } - value { '{}' } + value { '["categorie 1", "choix 1"]' } end factory :champ_pays, class: 'Champs::PaysChamp' do @@ -116,12 +116,12 @@ FactoryBot.define do factory :champ_regions, class: 'Champs::RegionChamp' do type_de_champ { create(:type_de_champ_regions) } - value { '' } + value { 'Guadeloupe' } end factory :champ_departements, class: 'Champs::DepartementChamp' do type_de_champ { create(:type_de_champ_departements) } - value { '' } + value { '971 - Guadeloupe' } end factory :champ_engagement, class: 'Champs::EngagementChamp' do @@ -136,7 +136,7 @@ FactoryBot.define do factory :champ_explication, class: 'Champs::ExplicationChamp' do type_de_champ { create(:type_de_champ_explication) } - value { 'une explication' } + value { '' } end factory :champ_dossier_link, class: 'Champs::DossierLinkChamp' do @@ -164,5 +164,15 @@ FactoryBot.define do factory :champ_repetition, class: 'Champs::RepetitionChamp' do type_de_champ { create(:type_de_champ_repetition) } + + after(:build) do |champ_repetition, _evaluator| + type_de_champ_text = create(:type_de_champ_text, order_place: 0, parent: champ_repetition.type_de_champ, libelle: 'Nom') + type_de_champ_number = create(:type_de_champ_number, order_place: 1, parent: champ_repetition.type_de_champ, libelle: 'Age') + + create(:champ_text, row: 0, type_de_champ: type_de_champ_text, parent: champ_repetition) + create(:champ_number, row: 0, type_de_champ: type_de_champ_number, parent: champ_repetition) + create(:champ_text, row: 1, type_de_champ: type_de_champ_text, parent: champ_repetition) + create(:champ_number, row: 1, type_de_champ: type_de_champ_number, parent: champ_repetition) + end end end diff --git a/spec/models/champ_spec.rb b/spec/models/champ_spec.rb index 9aff08c62..e387c5a6d 100644 --- a/spec/models/champ_spec.rb +++ b/spec/models/champ_spec.rb @@ -395,7 +395,7 @@ describe Champ do describe "repetition" do let(:dossier) { create(:dossier) } - let(:champ) { create(:champ_repetition, dossier: dossier) } + let(:champ) { Champs::RepetitionChamp.create(dossier: dossier) } let(:champ_text) { create(:champ_text, row: 0) } let(:champ_integer_number) { create(:champ_integer_number, row: 0) } let(:champ_text_attrs) { attributes_for(:champ_text, row: 1) } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 5de1da3db..a363e8bb3 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -886,7 +886,7 @@ describe Dossier do describe "#check_mandatory_champs" do let(:procedure) { create(:procedure, :with_type_de_champ) } - let(:dossier) { create(:dossier, :with_all_champs, procedure: procedure) } + let(:dossier) { create(:dossier, procedure: procedure) } it 'no mandatory champs' do expect(dossier.check_mandatory_champs).to be_empty @@ -945,7 +945,11 @@ describe Dossier do end context "when no champs" do - let(:champ_with_error) { dossier.champs.first } + let(:champ_with_error) do + repetition_champ = dossier.champs.first + text_champ = repetition_champ.rows.first.first + text_champ + end it 'should have errors' do errors = dossier.check_mandatory_champs diff --git a/spec/serializers/dossier_serializer_spec.rb b/spec/serializers/dossier_serializer_spec.rb index 86963686c..5b8f3c78b 100644 --- a/spec/serializers/dossier_serializer_spec.rb +++ b/spec/serializers/dossier_serializer_spec.rb @@ -41,7 +41,7 @@ describe DossierSerializer do expect(subject[3][:value]).to eq(42) expect(subject[4][:value]).to eq(42.1) - expect(subject[5][:value]).to eq({ primary: nil, secondary: nil }) + expect(subject[5][:value]).to eq({ primary: 'categorie 1', secondary: 'choix 1' }) } end end diff --git a/spec/services/procedure_export_v2_service_spec.rb b/spec/services/procedure_export_v2_service_spec.rb index 6c88afc94..5790307bb 100644 --- a/spec/services/procedure_export_v2_service_spec.rb +++ b/spec/services/procedure_export_v2_service_spec.rb @@ -157,15 +157,6 @@ describe ProcedureExportV2Service do context 'with repetitions' do let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :for_individual, procedure: procedure) } let(:champ_repetition) { dossier.champs.find { |champ| champ.type_champ == 'repetition' } } - let(:type_de_champ_text) { create(:type_de_champ_text, order_place: 0, parent: champ_repetition.type_de_champ) } - let(:type_de_champ_number) { create(:type_de_champ_number, order_place: 1, parent: champ_repetition.type_de_champ) } - - before do - create(:champ_text, row: 0, type_de_champ: type_de_champ_text, parent: champ_repetition) - create(:champ_number, row: 0, type_de_champ: type_de_champ_number, parent: champ_repetition) - create(:champ_text, row: 1, type_de_champ: type_de_champ_text, parent: champ_repetition) - create(:champ_number, row: 1, type_de_champ: type_de_champ_number, parent: champ_repetition) - end it 'should have sheets' do expect(subject.sheets.map(&:name)).to eq(['Dossiers', 'Etablissements', 'Avis', champ_repetition.libelle]) @@ -175,8 +166,8 @@ describe ProcedureExportV2Service do expect(repetition_sheet.headers).to eq([ "Dossier ID", "Ligne", - type_de_champ_text.libelle, - type_de_champ_number.libelle + "Nom", + "Age" ]) end