From c0a928ac7a85658de04b3818a9fde1766c2b898f Mon Sep 17 00:00:00 2001 From: pedong Date: Thu, 7 Mar 2019 12:56:12 +0100 Subject: [PATCH 01/19] [fix #3440] use attachment.download for get attachment file --- app/models/procedure.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 88b04ec84..4e2c75f4c 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -454,13 +454,10 @@ class Procedure < ApplicationRecord def clone_attachment(cloned_procedure, attachment_symbol) attachment = send(attachment_symbol) if attachment.attached? - response = Typhoeus.get(attachment.service_url, timeout: 5) - if response.success? - cloned_procedure.send(attachment_symbol).attach( - io: StringIO.new(response.body), - filename: attachment.filename - ) - end + cloned_procedure.send(attachment_symbol).attach( + io: StringIO.new(attachment.download), + filename: attachment.filename + ) end end From fb29d30826bc2fd7d1d0742e436c6279755cda38 Mon Sep 17 00:00:00 2001 From: pedong Date: Thu, 31 Jan 2019 17:11:53 +0100 Subject: [PATCH 02/19] [fix #3342] show date with format letter --- app/helpers/application_helper.rb | 16 +++++ .../shared/dossiers/_champ_row.html.haml | 4 ++ spec/helpers/application_helper_spec.rb | 58 +++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 28dc0af2a..10a3cc258 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -119,4 +119,20 @@ module ApplicationHelper {} end end + + def try_format_date(date) + begin + Date.parse(date).strftime("%d %B %Y") + rescue + date + end + end + + def try_format_datetime(datetime) + begin + Time.zone.parse(datetime).strftime("%d %B %Y %R") + rescue + datetime + end + end end diff --git a/app/views/shared/dossiers/_champ_row.html.haml b/app/views/shared/dossiers/_champ_row.html.haml index 361c6e786..62f6feb87 100644 --- a/app/views/shared/dossiers/_champ_row.html.haml +++ b/app/views/shared/dossiers/_champ_row.html.haml @@ -30,6 +30,10 @@ = render partial: "shared/champs/siret/show", locals: { champ: c, profile: profile } - when TypeDeChamp.type_champs.fetch(:textarea) = render partial: "shared/champs/textarea/show", locals: { champ: c } + - when TypeDeChamp.type_champs.fetch(:date) + = try_format_date(c.to_s) + - when TypeDeChamp.type_champs.fetch(:datetime) + = try_format_datetime(c.to_s) - else = sanitize(c.to_s) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 30a7becef..6998f17d2 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -17,4 +17,62 @@ describe ApplicationHelper do it { is_expected.to be_nil } end end + + describe "#try_format_date" do + subject { try_format_date(date) } + + describe 'try formatting 2019-01-24' do + let(:date) { "2019-01-24" } + it { is_expected.to eq("24 January 2019") } + end + + describe 'try formatting 24/01/2019' do + let(:date) { "24/01/2019" } + it { is_expected.to eq("24 January 2019") } + end + + describe 'try formatting 2019-01-32' do + let(:date) { "2019-01-32" } + it { is_expected.to eq("2019-01-32") } + end + + describe 'try formatting a blank string' do + let(:date) { "" } + it { is_expected.to eq("") } + end + + describe 'try formatting a nil string' do + let(:date) { nil } + it { is_expected.to be_nil } + end + end + + describe "#try_format_datetime" do + subject { try_format_datetime(datetime) } + + describe 'try formatting 31/01/2019 11:25' do + let(:datetime) { "31/01/2019 11:25" } + it { is_expected.to eq("31 January 2019 11:25") } + end + + describe 'try formatting 2019-01-31 11:25' do + let(:datetime) { "2019-01-31 11:25" } + it { is_expected.to eq("31 January 2019 11:25") } + end + + describe 'try formatting 2019-01-32 11:25' do + let(:datetime) { "2019-01-32 11:25" } + it { is_expected.to eq("2019-01-32 11:25") } + end + + describe 'try formatting a blank string' do + let(:datetime) { "" } + it { is_expected.to eq("") } + end + + describe 'try formatting a nil string' do + let(:datetime) { nil } + it { is_expected.to be_nil } + end + end end From f82fdef45f25e371f8a12d8b3554bd82bce578b5 Mon Sep 17 00:00:00 2001 From: pedong Date: Wed, 27 Mar 2019 16:10:54 +0100 Subject: [PATCH 03/19] [fix #3342] format letter for date SIRET --- app/helpers/application_helper.rb | 4 ++-- app/views/shared/dossiers/_identite_entreprise.html.haml | 8 ++++---- .../dossiers/etablissement/_infos_association.html.haml | 6 +++--- .../dossiers/etablissement/_infos_entreprise.html.haml | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 10a3cc258..efadf5712 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -122,7 +122,7 @@ module ApplicationHelper def try_format_date(date) begin - Date.parse(date).strftime("%d %B %Y") + date.is_a?(String) ? Date.parse(date).strftime("%d %B %Y") : date.strftime("%d %B %Y") rescue date end @@ -130,7 +130,7 @@ module ApplicationHelper def try_format_datetime(datetime) begin - Time.zone.parse(datetime).strftime("%d %B %Y %R") + datetime.is_a?(String) ? Time.zone.parse(datetime).strftime("%d %B %Y %R") : datetime.strftime("%d %B %Y %R") rescue datetime end diff --git a/app/views/shared/dossiers/_identite_entreprise.html.haml b/app/views/shared/dossiers/_identite_entreprise.html.haml index 87be8390f..33f0ce548 100644 --- a/app/views/shared/dossiers/_identite_entreprise.html.haml +++ b/app/views/shared/dossiers/_identite_entreprise.html.haml @@ -22,7 +22,7 @@ %td= etablissement.naf %tr %th.libelle Date de création : - %td= etablissement.entreprise.date_creation&.strftime("%d/%m/%Y") + %td= try_format_date(etablissement.entreprise.date_creation) %tr %th.libelle Effectif de l'organisation : %td= effectif(etablissement) @@ -64,10 +64,10 @@ %td= etablissement.association_objet %tr %th.libelle Date de création : - %td= etablissement.association_date_creation&.strftime("%d/%m/%Y") + %td= try_format_date(etablissement.association_date_creation) %tr %th.libelle Date de publication : - %td= etablissement.association_date_publication&.strftime("%d/%m/%Y") + %td= try_format_date(etablissement.association_date_publication) %tr %th.libelle Date de déclaration : - %td= etablissement.association_date_declaration&.strftime("%d/%m/%Y") + %td= try_format_date(etablissement.association_date_declaration) diff --git a/app/views/users/dossiers/etablissement/_infos_association.html.haml b/app/views/users/dossiers/etablissement/_infos_association.html.haml index 4fe9b8f91..d8afcbdb6 100644 --- a/app/views/users/dossiers/etablissement/_infos_association.html.haml +++ b/app/views/users/dossiers/etablissement/_infos_association.html.haml @@ -13,12 +13,12 @@ %li Date de création : - = etablissement.association_date_creation&.strftime('%d/%m/%Y') + = try_format_date(etablissement.association_date_creation) %li Date de déclaration : - = etablissement.association_date_declaration&.strftime('%d/%m/%Y') + = try_format_date(etablissement.association_date_declaration) %li Date de publication : - = etablissement.association_date_publication&.strftime('%d/%m/%Y') + = try_format_date(etablissement.association_date_publication) diff --git a/app/views/users/dossiers/etablissement/_infos_entreprise.html.haml b/app/views/users/dossiers/etablissement/_infos_entreprise.html.haml index 54f03b486..9667a54bd 100644 --- a/app/views/users/dossiers/etablissement/_infos_entreprise.html.haml +++ b/app/views/users/dossiers/etablissement/_infos_entreprise.html.haml @@ -22,7 +22,7 @@ %li Date de création : - = etablissement.entreprise.date_creation&.strftime('%d/%m/%Y') + = try_format_date(etablissement.entreprise.date_creation) %li Effectif organisation : From 21dbe44e0771b0489d2346a91aa2e50c223d73aa Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 26 Mar 2019 11:21:37 +0100 Subject: [PATCH 04/19] [#2180] Fix ordering of piece jointe champs --- app/services/pieces_justificatives_service.rb | 8 ++++- .../pieces_justificatives_service_spec.rb | 32 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/app/services/pieces_justificatives_service.rb b/app/services/pieces_justificatives_service.rb index dd617b28b..7ca08d233 100644 --- a/app/services/pieces_justificatives_service.rb +++ b/app/services/pieces_justificatives_service.rb @@ -33,7 +33,13 @@ class PiecesJustificativesService end def self.types_pj_as_types_de_champ(procedure) - order_place = procedure.types_de_champ.last&.order_place || 0 + last_champ = procedure.types_de_champ.last + if last_champ.present? + order_place = last_champ.order_place + 1 + else + order_place = 0 + end + types_de_champ = [ TypeDeChamp.new( libelle: "Pièces jointes", diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 0e6704515..63754ecdd 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -89,4 +89,36 @@ describe PiecesJustificativesService do it { expect(errors).to match([]) } end end + + describe 'types_pj_as_types_de_champ' do + subject { PiecesJustificativesService.types_pj_as_types_de_champ(procedure) } + + it 'generates one header champ, plus one champ per PJ' do + expect(subject.pluck(:libelle)).to contain_exactly("Pièces jointes", "not mandatory") + end + + it 'remembers the id of the PJ that got converted into a champ' do + expect(subject.map(&:old_pj)).to include({ 'stable_id' => tpj_not_mandatory.id }) + end + + context 'without pre-existing champs' do + it 'generates a sequence of order_places incrementing from zero' do + expect(subject.pluck(:order_place)).to contain_exactly(0, 1) + end + end + + context 'with pre-existing champs' do + let(:procedure) do + create( + :procedure, + types_de_piece_justificative: tpjs, + types_de_champ: [build(:type_de_champ, order_place: 0)] + ) + end + + it 'generates a sequence of incrementing order_places that continues where the last type de champ left off' do + expect(subject.pluck(:order_place)).to contain_exactly(1, 2) + end + end + end end From e24242e4b2c00f3981bc407c4f7ac72d6fe6839b Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 26 Mar 2019 15:41:34 +0100 Subject: [PATCH 05/19] [#2180] Low-level Carrierwave to ActiveStorage migration --- ...erwave_active_storage_migration_service.rb | 143 ++++++++++++++++++ ...e_active_storage_migration_service_spec.rb | 9 ++ 2 files changed, 152 insertions(+) create mode 100644 app/services/carrierwave_active_storage_migration_service.rb create mode 100644 spec/services/carrierwave_active_storage_migration_service_spec.rb diff --git a/app/services/carrierwave_active_storage_migration_service.rb b/app/services/carrierwave_active_storage_migration_service.rb new file mode 100644 index 000000000..c9fd7df86 --- /dev/null +++ b/app/services/carrierwave_active_storage_migration_service.rb @@ -0,0 +1,143 @@ +class CarrierwaveActiveStorageMigrationService + def ensure_openstack_copy_possible!(uploader) + ensure_active_storage_openstack! + ensure_carrierwave_openstack!(uploader) + ensure_active_storage_and_carrierwave_credetials_match(uploader) + end + + def ensure_active_storage_openstack! + # If we manage to get the client, it means that ActiveStorage is on OpenStack + openstack_client! + end + + def openstack_client! + @openstack_client ||= active_storage_openstack_client! + end + + def active_storage_openstack_client! + service = ActiveStorage::Blob.service + + if !defined?(ActiveStorage::Service::OpenStackService) || + !service.is_a?(ActiveStorage::Service::OpenStackService) + raise StandardError, 'ActiveStorage must be backed by OpenStack' + end + + service.client + end + + def ensure_carrierwave_openstack!(uploader) + storage = fog_client!(uploader) + + if !defined?(Fog::OpenStack::Storage::Real) || + !storage.is_a?(Fog::OpenStack::Storage::Real) + raise StandardError, 'Carrierwave must be backed by OpenStack' + end + end + + def fog_client!(uploader) + storage = uploader.new.send(:storage) + + if !defined?(CarrierWave::Storage::Fog) || + !storage.is_a?(CarrierWave::Storage::Fog) + raise StandardError, 'Carrierwave must be backed by a Fog provider' + end + + storage.connection + end + + # OpenStack Swift's COPY object command works across different buckets, but they still need + # to be on the same object store. This method tries to ensure that Carrierwave and ActiveStorage + # are indeed pointing to the same Swift store. + def ensure_active_storage_and_carrierwave_credetials_match(uploader) + auth_keys = [ + :openstack_tenant, + :openstack_api_key, + :openstack_username, + :openstack_region, + :openstack_management_url + ] + + active_storage_creds = openstack_client!.credentials.slice(*auth_keys) + carrierwave_creds = fog_client!(uploader).credentials.slice(*auth_keys) + + if active_storage_creds != carrierwave_creds + raise StandardError, "Active Storage and Carrierwave credentials must match" + end + end + + # If identify is true, force ActiveStorage to examine the beginning of the file + # to determine its MIME type. This identification does not happen immediately, + # but when the first attachment that references this blob is created. + def make_blob(uploader, created_at, filename: nil, identify: false) + content_type = uploader.content_type + + ActiveStorage::Blob.create( + filename: filename || uploader.filename, + content_type: uploader.content_type, + identified: content_type.present? && !identify, + byte_size: uploader.size, + checksum: checksum(uploader), + created_at: created_at + ) + end + + def checksum(uploader) + hex_to_base64(uploader.file.send(:file).etag) + end + + def hex_to_base64(hexdigest) + [[hexdigest].pack("H*")].pack("m0") + end + + def copy_from_carrierwave_to_active_storage!(source_name, blob) + openstack_client!.copy_object( + carrierwave_container_name, + source_name, + active_storage_container_name, + blob.key + ) + + fix_content_type(blob) + end + + def carrierwave_container_name + Rails.application.secrets.fog[:directory] + end + + def active_storage_container_name + ENV['FOG_ACTIVESTORAGE_DIRECTORY'] + end + + def delete_from_active_storage!(blob) + openstack_client!.delete_object( + active_storage_container_name, + blob.key + ) + end + + # Before calling this method, you must make sure the file has been uploaded for the blob. + # Otherwise, this method might fail if it needs to read the beginning of the file to + # update the blob’s MIME type. + def make_attachment(model, attachment_name, blob) + attachment = ActiveStorage::Attachment.create( + name: attachment_name, + record_type: model.class.base_class.name, + record_id: model.id, + blob: blob, + created_at: model.updated_at.iso8601 + ) + + # Making the attachment may have triggerred MIME type auto detection on the blob, + # so we make sure to sync that potentially new MIME type to the object in OpenStack + fix_content_type(blob) + + attachment + end + + def fix_content_type(blob) + # In OpenStack, ActiveStorage cannot inject the MIME type on the fly during direct + # download. Instead, the MIME type needs to be stored statically on the file object + # in OpenStack. This is what this call does. + blob.service.change_content_type(blob.key, blob.content_type) + end +end diff --git a/spec/services/carrierwave_active_storage_migration_service_spec.rb b/spec/services/carrierwave_active_storage_migration_service_spec.rb new file mode 100644 index 000000000..7d3d470b0 --- /dev/null +++ b/spec/services/carrierwave_active_storage_migration_service_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe CarrierwaveActiveStorageMigrationService do + describe '#hex_to_base64' do + let(:service) { CarrierwaveActiveStorageMigrationService.new } + + it { expect(service.hex_to_base64('deadbeef')).to eq('3q2+7w==') } + end +end From 7d316b836975509ca787d1388733ba456a296b05 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 26 Mar 2019 21:41:26 +0100 Subject: [PATCH 06/19] [#2180] High-level PJ to champ PJ migration service --- ...to_champ_piece_jointe_migration_service.rb | 93 ++++++++++ ...amp_piece_jointe_migration_service_spec.rb | 170 ++++++++++++++++++ 2 files changed, 263 insertions(+) create mode 100644 app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb create mode 100644 spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb diff --git a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb new file mode 100644 index 000000000..45910f3f9 --- /dev/null +++ b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb @@ -0,0 +1,93 @@ +class PieceJustificativeToChampPieceJointeMigrationService + def initialize(**params) + params.each do |key, value| + instance_variable_set("@#{key}", value) + end + end + + def ensure_correct_storage_configuration! + storage_service.ensure_openstack_copy_possible!(PieceJustificativeUploader) + end + + def convert_procedure_pjs_to_champ_pjs(procedure) + types_de_champ_pj = PiecesJustificativesService.types_pj_as_types_de_champ(procedure) + populate_champs_pjs!(procedure, types_de_champ_pj) + + # Only destroy the old types PJ once everything has been safely migrated to + # champs PJs. Destroying the types PJ will cascade and destroy the PJs, + # and delete the linked objects from remote storage. This means that no other + # cleanup action is required. + procedure.types_de_piece_justificative.destroy_all + end + + def storage_service + @storage_service ||= CarrierwaveActiveStorageMigrationService.new + end + + def populate_champs_pjs!(procedure, types_de_champ_pj) + procedure.types_de_champ += types_de_champ_pj + + # Unscope to make sure all dossiers are migrated, even the soft-deleted ones + procedure.dossiers.unscope(where: :hidden_at).find_each do |dossier| + champs_pj = types_de_champ_pj.map(&:build_champ) + dossier.champs += champs_pj + + champs_pj.each do |champ| + type_pj_id = champ.type_de_champ.old_pj&.fetch('stable_id', nil) + pj = dossier.retrieve_last_piece_justificative_by_type(type_pj_id) + if pj.present? + convert_pj_to_champ!(pj, champ) + end + end + end + rescue + # If anything goes wrong, we roll back the migration by destroying the newly created + # types de champ, champs blobs and attachments. + types_de_champ_pj.each do |type_champ| + type_champ.champ.each { |c| c.piece_justificative_file.purge } + type_champ.destroy + end + + # Reraise the exception to abort the migration. + raise + end + + def convert_pj_to_champ!(pj, champ) + blob = make_blob(pj) + + # Upload the file before creating the attachment to make sure MIME type + # identification doesn’t fail. + storage_service.copy_from_carrierwave_to_active_storage!(pj.content.path, blob) + attachment = storage_service.make_attachment(champ, 'piece_justificative_file', blob) + + # By reloading, we force ActiveStorage to look at the attachment again, and see + # that one exists now. We do this so that, if we need to roll back and destroy the champ, + # the blob, the attachment and the actual file on OpenStack also get deleted. + champ.reload + rescue + # Destroy partially attached object that the more general rescue in `populate_champs_pjs!` + # might not be able to handle. + + if blob&.key.present? + begin + storage_service.delete_from_active_storage!(blob) + rescue => e + # The cleanup attempt failed, perhaps because the object had not been + # successfully copied to the Active Storage bucket yet. + # Continue trying to clean up the rest anyway. + pp e + end + end + + blob&.destroy + attachment&.destroy + champ.reload + + # Reraise the exception to abort the migration. + raise + end + + def make_blob(pj) + storage_service.make_blob(pj.content, pj.updated_at.iso8601, filename: pj.original_filename) + end +end diff --git a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb new file mode 100644 index 000000000..8ebfd4647 --- /dev/null +++ b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb @@ -0,0 +1,170 @@ +require 'spec_helper' + +describe PieceJustificativeToChampPieceJointeMigrationService do + let(:service) { PieceJustificativeToChampPieceJointeMigrationService.new(storage_service: storage_service) } + let(:storage_service) { CarrierwaveActiveStorageMigrationService.new } + let(:pj_uploader) { class_double(PieceJustificativeUploader) } + let(:pj_service) { class_double(PiecesJustificativesService) } + + let(:procedure) { create(:procedure, types_de_piece_justificative: types_pj) } + let(:types_pj) { [create(:type_de_piece_justificative)] } + + let!(:dossier) do + create( + :dossier, + procedure: procedure, + pieces_justificatives: pjs + ) + end + + let(:pjs) { [] } + + def make_pjs + types_pj.map do |tpj| + create(:piece_justificative, :contrat, type_de_piece_justificative: tpj) + end + end + + def expect_storage_service_to_convert_object + expect(storage_service).to receive(:make_blob) + expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!) + expect(storage_service).to receive(:make_attachment) + end + + context 'when conversion succeeds' do + context 'for the procedure' do + it 'types de champ are created for the "pièces jointes" header and for each PJ' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to change { procedure.types_de_champ.count } + .by(types_pj.count + 1) + end + + it 'the old types de pj are removed' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to change { procedure.types_de_piece_justificative.count } + .to(0) + end + end + + context 'for the dossier' do + let(:pjs) { make_pjs } + + before { expect_storage_service_to_convert_object } + + it 'champs are created for the "pièces jointes" header and for each PJ' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to change { dossier.champs.count } + .by(types_pj.count + 1) + end + + it 'the old pjs are removed' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to change { dossier.pieces_justificatives.count } + .to(0) + end + end + + context 'when the dossier is soft-deleted it still gets converted' do + let(:pjs) { make_pjs } + + let!(:dossier) do + create( + :dossier, + procedure: procedure, + pieces_justificatives: pjs, + hidden_at: Time.zone.now + ) + end + + before { expect_storage_service_to_convert_object } + + it 'champs are created for the "pièces jointes" header and for each PJ' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to change { dossier.champs.count } + .by(types_pj.count + 1) + end + + it 'the old pjs are removed' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to change { dossier.pieces_justificatives.count } + .to(0) + end + end + + context 'when there are several pjs for one type' do + let(:pjs) { make_pjs + make_pjs } + + it 'only converts the most recent PJ for each type PJ' do + expect(storage_service).to receive(:make_blob).exactly(types_pj.count) + expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!).exactly(types_pj.count) + expect(storage_service).to receive(:make_attachment).exactly(types_pj.count) + + service.convert_procedure_pjs_to_champ_pjs(procedure) + end + end + end + + context 'cleanup when conversion fails' do + let(:pjs) { make_pjs } + + let!(:failing_dossier) do + create( + :dossier, + procedure: procedure, + pieces_justificatives: make_pjs + ) + end + + before do + allow(storage_service).to receive(:checksum).and_return('cafe') + allow(storage_service).to receive(:fix_content_type) + + expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!) + expect(storage_service).to receive(:copy_from_carrierwave_to_active_storage!) + .and_raise('LOL no!') + + expect(storage_service).to receive(:delete_from_active_storage!) + end + + def try_convert(procedure) + service.convert_procedure_pjs_to_champ_pjs(procedure) + rescue => e + e + end + + it 'passes on the exception' do + expect { service.convert_procedure_pjs_to_champ_pjs(procedure) } + .to raise_error('LOL no!') + end + + it 'does not create champs' do + expect { try_convert(procedure) } + .not_to change { dossier.champs.count } + end + + it 'does not remove any old pjs' do + expect { try_convert(procedure) } + .not_to change { dossier.pieces_justificatives.count } + end + + it 'does not creates types de champ' do + expect { try_convert(procedure) } + .not_to change { procedure.types_de_champ.count } + end + + it 'does not remove old types de pj' do + expect { try_convert(procedure) } + .not_to change { procedure.types_de_piece_justificative.count } + end + + it 'does not leave stale blobs behind' do + expect { try_convert(procedure) } + .not_to change { ActiveStorage::Blob.count } + end + + it 'does not leave stale attachments behind' do + expect { try_convert(procedure) } + .not_to change { ActiveStorage::Attachment.count } + end + end +end From 498fe3b6ef6dcbe438cb6e278e45c579d970c8b6 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 26 Mar 2019 21:41:54 +0100 Subject: [PATCH 07/19] [#2180] Task to migrate PJs to champs PJ for one procedure --- lib/tasks/2019_03_13_migrate_pjs_to_champs.rake | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 lib/tasks/2019_03_13_migrate_pjs_to_champs.rake diff --git a/lib/tasks/2019_03_13_migrate_pjs_to_champs.rake b/lib/tasks/2019_03_13_migrate_pjs_to_champs.rake new file mode 100644 index 000000000..ad8194116 --- /dev/null +++ b/lib/tasks/2019_03_13_migrate_pjs_to_champs.rake @@ -0,0 +1,8 @@ +namespace :'2019_03_13_migrate_pjs_to_champs' do + task run: :environment do + procedure_id = ENV['PROCEDURE_ID'] + service = PieceJustificativeToChampPieceJointeMigrationService.new + service.ensure_correct_storage_configuration! + service.convert_procedure_pjs_to_champ_pjs(Procedure.find(procedure_id)) + end +end From 9aebb76e77ed3c04291a0fbc6ef03cc51e4764e7 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Wed, 27 Mar 2019 17:32:29 +0100 Subject: [PATCH 08/19] [#2180] Avoid sending spurious notifications to instructeurs --- ...to_champ_piece_jointe_migration_service.rb | 11 ++++ ...amp_piece_jointe_migration_service_spec.rb | 52 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb index 45910f3f9..5d2ace62e 100644 --- a/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb +++ b/app/services/piece_justificative_to_champ_piece_jointe_migration_service.rb @@ -35,8 +35,19 @@ class PieceJustificativeToChampPieceJointeMigrationService champs_pj.each do |champ| type_pj_id = champ.type_de_champ.old_pj&.fetch('stable_id', nil) pj = dossier.retrieve_last_piece_justificative_by_type(type_pj_id) + if pj.present? + champ.update( + updated_at: pj.updated_at, + created_at: pj.created_at + ) + convert_pj_to_champ!(pj, champ) + else + champ.update( + updated_at: dossier.updated_at, + created_at: dossier.created_at + ) end end end diff --git a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb index 8ebfd4647..9f7f83e34 100644 --- a/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb +++ b/spec/services/piece_justificative_to_champ_piece_jointe_migration_service_spec.rb @@ -46,6 +46,58 @@ describe PieceJustificativeToChampPieceJointeMigrationService do end end + context 'no notifications are sent to instructeurs' do + context 'when there is a PJ' do + let(:pjs) { make_pjs } + + before do + # Reload PJ because the resolution of in-database timestamps is + # different from the resolution of in-memory timestamps, causing the + # tests to fail on fractional time differences. + pjs.last.reload + + expect_storage_service_to_convert_object + Timecop.travel(1.hour) { service.convert_procedure_pjs_to_champ_pjs(procedure) } + + # Reload the dossier to see the newly created champs + dossier.reload + end + + it 'the champ has the same created_at as the PJ' do + expect(dossier.champs.last.created_at).to eq(pjs.last.created_at) + end + + it 'the champ has the same updated_at as the PJ' do + expect(dossier.champs.last.updated_at).to eq(pjs.last.updated_at) + end + end + + context 'when there is no PJ' do + let!(:expected_updated_at) do + # Reload dossier because the resolution of in-database timestamps is + # different from the resolution of in-memory timestamps, causing the + # tests to fail on fractional time differences. + dossier.reload + dossier.updated_at + end + + before do + Timecop.travel(1.hour) { service.convert_procedure_pjs_to_champ_pjs(procedure) } + + # Reload the dossier to see the newly created champs + dossier.reload + end + + it 'the champ has the same created_at as the dossier' do + expect(dossier.champs.last.created_at).to eq(dossier.created_at) + end + + it 'the champ has the same updated_at as the dossier' do + expect(dossier.champs.last.updated_at).to eq(expected_updated_at) + end + end + end + context 'for the dossier' do let(:pjs) { make_pjs } From bc408e337466fa76bccb6c8ecf1e0207a2e4cec1 Mon Sep 17 00:00:00 2001 From: philemon95 <40801449+philemon95@users.noreply.github.com> Date: Thu, 14 Mar 2019 13:02:26 +0000 Subject: [PATCH 09/19] support: allow toggling any element depending on the question type --- app/javascript/new_design/support.js | 16 ++++------------ app/views/support/index.html.haml | 25 +++++++++++++++---------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/javascript/new_design/support.js b/app/javascript/new_design/support.js index 100b71240..7743f3ffd 100644 --- a/app/javascript/new_design/support.js +++ b/app/javascript/new_design/support.js @@ -2,17 +2,9 @@ import { show, hide, delegate } from '@utils'; delegate('change', '#contact-form #type', event => { const type = event.target.value; - const answer = document.querySelector(`[data-answer="${type}"]`); - const card = document.querySelector('.support.card'); + const visibleElements = `[data-contact-type-only="${type}"]`; + const hiddenElements = `[data-contact-type-only]:not([data-contact-type-only="${type}"])`; - for (let element of document.querySelectorAll('.card-content')) { - hide(element); - } - - if (answer) { - show(card); - show(answer); - } else { - hide(card); - } + document.querySelectorAll(visibleElements).forEach(show); + document.querySelectorAll(hiddenElements).forEach(hide); }); diff --git a/app/views/support/index.html.haml b/app/views/support/index.html.haml index fb52b1d04..55306c186 100644 --- a/app/views/support/index.html.haml +++ b/app/views/support/index.html.haml @@ -22,17 +22,20 @@ %span.mandatory * = select_tag :type, options_for_select(@options, params[:type]), include_blank: "Choisir un problème", required: true - .support.card.featured.hidden + .support.card.featured.hidden{ data: { 'contact-type-only': 'info demarche' } } .card-title 👉 Notre réponse - .card-content.hidden{ data: { answer: "info demarche" } } + .card-content %p Avez-vous bien vérifié que tous les champs obligatoires (*) sont bien remplis ? %p Si vous avez des questions sur les informations à saisir, contactez les services en charge de la démarche. %p %a{ href: 'https://faq.demarches-simplifiees.fr/article/12-contacter-le-service-en-charge-de-ma-demarche' } En savoir plus - .card-content.hidden{ data: { answer: "usager perdu" } } + .support.card.featured.hidden{ data: { 'contact-type-only': 'usager perdu' } } + .card-title + 👉 Notre réponse + .card-content %p Nous vous invitons à contacter l’administration en charge de votre démarche pour qu’elle vous indique le lien à suivre. Celui-ci devrait ressembler à cela : https://www.demarches-simplifiees.fr/commencer/NOM_DE_LA_DEMARCHE . %br %p Vous pouvez aussi consulter ici la liste de nos démarches les plus frequentes (permis, detr etc) : @@ -40,13 +43,15 @@ %a{ href: 'https://doc.demarches-simplifiees.fr/listes-des-demarches' } https://doc.demarches-simplifiees.fr/listes-des-demarches - .card-content.hidden{ data: { answer: "info instruction" } } - %p Si vous avez des questions sur l’instruction de votre dossier (par exemple sur les délais), nous vous invitons à contacter directement les services qui instruisent votre dossier par votre messagerie - %p - %a{ href: 'https://faq.demarches-simplifiees.fr/article/11-je-veux-savoir-ou-en-est-linstruction-de-ma-demarche' } - En savoir plus - %br - %p Si vous souhaitez poser une question pour un problème technique sur le site, utilisez le formulaire ci-dessous. Nous ne pourrons pas vous renseigner sur l'instruction de votre dossier. + .support.card.featured.hidden{ data: { 'contact-type-only': 'info instruction' } } + .card-title + 👉 Notre réponse + %p Si vous avez des questions sur l’instruction de votre dossier (par exemple sur les délais), nous vous invitons à contacter directement les services qui instruisent votre dossier par votre messagerie. + %p + %a{ href: 'https://faq.demarches-simplifiees.fr/article/11-je-veux-savoir-ou-en-est-linstruction-de-ma-demarche' } + En savoir plus + %br + %p Si vous souhaitez poser une question pour un problème technique sur le site, utilisez le formulaire ci-dessous. Nous ne pourrons pas vous renseigner sur l'instruction de votre dossier. .contact-champ = label_tag :dossier_id, 'Numéro du dossier concerné' From 4e2e8e9a57b7ffb7c65f0122c660046fe609b73f Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 14 Mar 2019 11:41:54 +0100 Subject: [PATCH 10/19] support: toggle correct elements on page load --- app/javascript/new_design/support.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/app/javascript/new_design/support.js b/app/javascript/new_design/support.js index 7743f3ffd..1e6485c30 100644 --- a/app/javascript/new_design/support.js +++ b/app/javascript/new_design/support.js @@ -1,10 +1,16 @@ import { show, hide, delegate } from '@utils'; -delegate('change', '#contact-form #type', event => { - const type = event.target.value; - const visibleElements = `[data-contact-type-only="${type}"]`; - const hiddenElements = `[data-contact-type-only]:not([data-contact-type-only="${type}"])`; +function updateContactElementsVisibility() { + const contactSelect = document.querySelector('#contact-form #type'); + if (contactSelect) { + const type = contactSelect.value; + const visibleElements = `[data-contact-type-only="${type}"]`; + const hiddenElements = `[data-contact-type-only]:not([data-contact-type-only="${type}"])`; - document.querySelectorAll(visibleElements).forEach(show); - document.querySelectorAll(hiddenElements).forEach(hide); -}); + document.querySelectorAll(visibleElements).forEach(show); + document.querySelectorAll(hiddenElements).forEach(hide); + } +} + +addEventListener('turbolinks:load', updateContactElementsVisibility); +delegate('change', '#contact-form #type', updateContactElementsVisibility); From 6c350befd514edbac078b4e7d57ded54bde46ff0 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 14 Mar 2019 13:52:41 +0100 Subject: [PATCH 11/19] support: use constants rather than strings --- app/views/support/index.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/support/index.html.haml b/app/views/support/index.html.haml index 55306c186..fdffeb78c 100644 --- a/app/views/support/index.html.haml +++ b/app/views/support/index.html.haml @@ -22,7 +22,7 @@ %span.mandatory * = select_tag :type, options_for_select(@options, params[:type]), include_blank: "Choisir un problème", required: true - .support.card.featured.hidden{ data: { 'contact-type-only': 'info demarche' } } + .support.card.featured.hidden{ data: { 'contact-type-only': Helpscout::FormAdapter::TYPE_INFO } } .card-title 👉 Notre réponse .card-content @@ -32,7 +32,7 @@ %a{ href: 'https://faq.demarches-simplifiees.fr/article/12-contacter-le-service-en-charge-de-ma-demarche' } En savoir plus - .support.card.featured.hidden{ data: { 'contact-type-only': 'usager perdu' } } + .support.card.featured.hidden{ data: { 'contact-type-only': Helpscout::FormAdapter::TYPE_PERDU } } .card-title 👉 Notre réponse .card-content @@ -43,7 +43,7 @@ %a{ href: 'https://doc.demarches-simplifiees.fr/listes-des-demarches' } https://doc.demarches-simplifiees.fr/listes-des-demarches - .support.card.featured.hidden{ data: { 'contact-type-only': 'info instruction' } } + .support.card.featured.hidden{ data: { 'contact-type-only': Helpscout::FormAdapter::TYPE_INSTRUCTION } } .card-title 👉 Notre réponse %p Si vous avez des questions sur l’instruction de votre dossier (par exemple sur les délais), nous vous invitons à contacter directement les services qui instruisent votre dossier par votre messagerie. From 27c5d01fa483d4eabee8648ddcb7810c3bc1462a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 14 Mar 2019 10:47:44 +0000 Subject: [PATCH 12/19] support: add nudge to send a screenshot --- app/assets/stylesheets/new_design/contact.scss | 4 ++++ app/views/support/index.html.haml | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/new_design/contact.scss b/app/assets/stylesheets/new_design/contact.scss index b85f64123..476b3c3b3 100644 --- a/app/assets/stylesheets/new_design/contact.scss +++ b/app/assets/stylesheets/new_design/contact.scss @@ -8,4 +8,8 @@ $contact-padding: $default-space * 2; .description { padding-bottom: $contact-padding; } + + .hidden { + display: none; + } } diff --git a/app/views/support/index.html.haml b/app/views/support/index.html.haml index fdffeb78c..212767bb9 100644 --- a/app/views/support/index.html.haml +++ b/app/views/support/index.html.haml @@ -70,7 +70,12 @@ = text_area_tag :text, params[:text], rows: 6, required: true .contact-champ - = label_tag :text, 'Pièce jointe' + = label_tag :text do + Pièce jointe + .notice.hidden{ data: { 'contact-type-only': Helpscout::FormAdapter::TYPE_AMELIORATION } } + 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 = hidden_field_tag :tags, @tags&.join(',') From e6351b5b1c7d7b34dd6f3b14a77e332026e80be0 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 25 Mar 2019 15:04:30 +0000 Subject: [PATCH 13/19] help: move all partials to `shared/help` --- .../gestionnaires/_help_dropdown.html.haml | 20 --------- app/views/layouts/_new_header.haml | 10 +++-- app/views/shared/help/_help_button.html.haml | 1 + .../help/_help_dropdown_dossier.html.haml | 14 +++++++ .../_help_dropdown_gestionnaire.html.haml | 6 +++ .../help/dropdown_items/_email_item.html.haml | 6 +++ .../help/dropdown_items/_faq_item.html.haml | 6 +++ .../dropdown_items/_messagerie_item.html.haml | 6 +++ .../dropdown_items/_service_item.html.haml | 15 +++++++ .../users/_dossier_help_dropdown.html.haml | 42 ------------------- 10 files changed, 60 insertions(+), 66 deletions(-) delete mode 100644 app/views/gestionnaires/_help_dropdown.html.haml create mode 100644 app/views/shared/help/_help_button.html.haml create mode 100644 app/views/shared/help/_help_dropdown_dossier.html.haml create mode 100644 app/views/shared/help/_help_dropdown_gestionnaire.html.haml create mode 100644 app/views/shared/help/dropdown_items/_email_item.html.haml create mode 100644 app/views/shared/help/dropdown_items/_faq_item.html.haml create mode 100644 app/views/shared/help/dropdown_items/_messagerie_item.html.haml create mode 100644 app/views/shared/help/dropdown_items/_service_item.html.haml delete mode 100644 app/views/users/_dossier_help_dropdown.html.haml diff --git a/app/views/gestionnaires/_help_dropdown.html.haml b/app/views/gestionnaires/_help_dropdown.html.haml deleted file mode 100644 index 1e4d4f7e1..000000000 --- a/app/views/gestionnaires/_help_dropdown.html.haml +++ /dev/null @@ -1,20 +0,0 @@ -.dropdown.help-dropdown - .button.primary.dropdown-button Aide - .dropdown-content.fade-in-down - %ul.dropdown-items - - -# Use the help website - %li - = link_to FAQ_URL, target: "_blank", rel: "noopener" do - %span.icon.help - .dropdown-description - %h4.help-dropdown-title Un problème avec le site ? - %p Trouvez votre réponse dans l’aide en ligne. - - -# Technical contact - %li - = mail_to CONTACT_EMAIL do - %span.icon.mail - .dropdown-description - %h4.help-dropdown-title Contact technique - %p Envoyez nous un message à #{CONTACT_EMAIL}. diff --git a/app/views/layouts/_new_header.haml b/app/views/layouts/_new_header.haml index 738eb173b..c4102cf53 100644 --- a/app/views/layouts/_new_header.haml +++ b/app/views/layouts/_new_header.haml @@ -88,9 +88,11 @@ %li .header-help - - if nav_bar_profile == :user && dossier.present? - = render partial: 'users/dossier_help_dropdown', locals: { dossier: dossier } + - if dossier.present? && nav_bar_profile == :user + = render partial: 'shared/help/help_dropdown_dossier', locals: { dossier: dossier } + - elsif nav_bar_profile == :gestionnaire - = render partial: 'gestionnaires/help_dropdown' + = render partial: 'shared/help/help_dropdown_gestionnaire' + - else - = link_to 'Aide', FAQ_URL, class: "button primary" + = render partial: 'shared/help/help_button' diff --git a/app/views/shared/help/_help_button.html.haml b/app/views/shared/help/_help_button.html.haml new file mode 100644 index 000000000..a73a69381 --- /dev/null +++ b/app/views/shared/help/_help_button.html.haml @@ -0,0 +1 @@ += link_to 'Aide', FAQ_URL, class: 'button primary' diff --git a/app/views/shared/help/_help_dropdown_dossier.html.haml b/app/views/shared/help/_help_dropdown_dossier.html.haml new file mode 100644 index 000000000..38b9af8bc --- /dev/null +++ b/app/views/shared/help/_help_dropdown_dossier.html.haml @@ -0,0 +1,14 @@ +.dropdown.help-dropdown + .button.primary.dropdown-button Aide + .dropdown-content.fade-in-down + %ul.dropdown-items + - title = dossier.brouillon? ? "Besoin d’aide pour remplir votre dossier ?" : "Une question sur votre dossier ?" + + - if dossier.messagerie_available? + = render partial: 'shared/help/dropdown_items/messagerie_item', + locals: { dossier: dossier, title: title } + - elsif dossier.procedure.service.present? + = render partial: 'shared/help/dropdown_items/service_item', + locals: { service: dossier.procedure.service, title: title } + + = render partial: 'shared/help/dropdown_items/faq_item' diff --git a/app/views/shared/help/_help_dropdown_gestionnaire.html.haml b/app/views/shared/help/_help_dropdown_gestionnaire.html.haml new file mode 100644 index 000000000..d5b2091b8 --- /dev/null +++ b/app/views/shared/help/_help_dropdown_gestionnaire.html.haml @@ -0,0 +1,6 @@ +.dropdown.help-dropdown + .button.primary.dropdown-button Aide + .dropdown-content.fade-in-down + %ul.dropdown-items + = render partial: 'shared/help/dropdown_items/faq_item' + = render partial: 'shared/help/dropdown_items/email_item' diff --git a/app/views/shared/help/dropdown_items/_email_item.html.haml b/app/views/shared/help/dropdown_items/_email_item.html.haml new file mode 100644 index 000000000..e921003e5 --- /dev/null +++ b/app/views/shared/help/dropdown_items/_email_item.html.haml @@ -0,0 +1,6 @@ +%li + = mail_to CONTACT_EMAIL do + %span.icon.mail + .dropdown-description + %h4.help-dropdown-title Contact technique + %p Envoyez nous un message à #{CONTACT_EMAIL}. diff --git a/app/views/shared/help/dropdown_items/_faq_item.html.haml b/app/views/shared/help/dropdown_items/_faq_item.html.haml new file mode 100644 index 000000000..86f82348b --- /dev/null +++ b/app/views/shared/help/dropdown_items/_faq_item.html.haml @@ -0,0 +1,6 @@ +%li + = link_to FAQ_URL, target: "_blank", rel: "noopener" do + %span.icon.help + .dropdown-description + %h4.help-dropdown-title Un problème avec le site ? + %p Trouvez votre réponse dans l’aide en ligne. diff --git a/app/views/shared/help/dropdown_items/_messagerie_item.html.haml b/app/views/shared/help/dropdown_items/_messagerie_item.html.haml new file mode 100644 index 000000000..2e25a78c8 --- /dev/null +++ b/app/views/shared/help/dropdown_items/_messagerie_item.html.haml @@ -0,0 +1,6 @@ +%li + = link_to messagerie_dossier_path(dossier) do + %span.icon.mail + .dropdown-description + %h4.help-dropdown-title= title + %p Envoyez directement un message à l’instructeur. diff --git a/app/views/shared/help/dropdown_items/_service_item.html.haml b/app/views/shared/help/dropdown_items/_service_item.html.haml new file mode 100644 index 000000000..af8624549 --- /dev/null +++ b/app/views/shared/help/dropdown_items/_service_item.html.haml @@ -0,0 +1,15 @@ +%li.help-dropdown-service + %span.icon.person + .dropdown-description + %h4.help-dropdown-title= title + .help-dropdown-service-action + %p Contactez directement l’administration : + %p.help-dropdown-service-item + %span.icon.small.mail + = link_to service.email, "mailto:#{service.email}" + %p.help-dropdown-service-item + %span.icon.small.phone + = link_to service.telephone, "tel:#{service.telephone}" + %p.help-dropdown-service-item + %span.icon.small.clock + = service.horaires diff --git a/app/views/users/_dossier_help_dropdown.html.haml b/app/views/users/_dossier_help_dropdown.html.haml deleted file mode 100644 index 275b7e1ac..000000000 --- a/app/views/users/_dossier_help_dropdown.html.haml +++ /dev/null @@ -1,42 +0,0 @@ -.dropdown.help-dropdown - .button.primary.dropdown-button Aide - .dropdown-content.fade-in-down - %ul.dropdown-items - - - title = dossier.brouillon? ? "Besoin d’aide pour remplir votre dossier ?" : "Une question sur votre dossier ?" - - - if dossier.messagerie_available? - -# Contact the administration using the messagerie - %li - = link_to messagerie_dossier_path(dossier) do - %span.icon.mail - .dropdown-description - %h4.help-dropdown-title= title - %p Envoyez directement un message à l’instructeur. - - - elsif dossier.procedure.service.present? - - service = dossier.procedure.service - -# Contact the administration using email or phone - %li.help-dropdown-service - %span.icon.person - .dropdown-description - %h4.help-dropdown-title= title - .help-dropdown-service-action - %p Contactez directement l’administration : - %p.help-dropdown-service-item - %span.icon.small.mail - = link_to service.email, "mailto:#{service.email}" - %p.help-dropdown-service-item - %span.icon.small.phone - = link_to service.telephone, "tel:#{service.telephone}" - %p.help-dropdown-service-item - %span.icon.small.clock - = service.horaires - - -# Use the help website - %li - = link_to FAQ_URL, target: "_blank", rel: "noopener" do - %span.icon.help - .dropdown-description - %h4.help-dropdown-title Un problème avec le site ? - %p Trouvez votre réponse dans l’aide en ligne. From d267d782b8ea2ad22fdbc3d4e1aa10bd97196c28 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 28 Mar 2019 10:14:01 +0000 Subject: [PATCH 14/19] help: add administration contact on /commencer pages --- app/controllers/users/commencer_controller.rb | 4 ++ app/views/layouts/_new_header.haml | 8 ++- .../help/_help_dropdown_procedure.html.haml | 9 +++ spec/features/help_spec.rb | 18 ++++++ spec/views/layouts/_new_header_spec.rb | 64 +++++++++++++------ 5 files changed, 80 insertions(+), 23 deletions(-) create mode 100644 app/views/shared/help/_help_dropdown_procedure.html.haml diff --git a/app/controllers/users/commencer_controller.rb b/app/controllers/users/commencer_controller.rb index 95970cc1b..f74a56d7d 100644 --- a/app/controllers/users/commencer_controller.rb +++ b/app/controllers/users/commencer_controller.rb @@ -34,6 +34,10 @@ module Users redirect_to new_user_registration_path end + def procedure_for_help + Procedure.publiees.find_by(path: params[:path]) || Procedure.brouillons.find_by(path: params[:path]) + end + private def store_user_location! diff --git a/app/views/layouts/_new_header.haml b/app/views/layouts/_new_header.haml index c4102cf53..2038dbbae 100644 --- a/app/views/layouts/_new_header.haml +++ b/app/views/layouts/_new_header.haml @@ -1,6 +1,7 @@ -# We can't use &. because the controller may not implement #nav_bar_profile -- nav_bar_profile = controller.try(:nav_bar_profile) +- nav_bar_profile = controller.try(:nav_bar_profile) || :guest - dossier = controller.try(:dossier_for_help) +- procedure = controller.try(:procedure_for_help) .new-header{ class: current_page?(root_path) ? nil : "new-header-with-border" } .header-inner-content @@ -88,9 +89,12 @@ %li .header-help - - if dossier.present? && nav_bar_profile == :user + - if dossier.present? && nav_bar_profile == :user = render partial: 'shared/help/help_dropdown_dossier', locals: { dossier: dossier } + - elsif procedure.present? && (nav_bar_profile == :user || nav_bar_profile == :guest) + = render partial: 'shared/help/help_dropdown_procedure', locals: { procedure: procedure } + - elsif nav_bar_profile == :gestionnaire = render partial: 'shared/help/help_dropdown_gestionnaire' diff --git a/app/views/shared/help/_help_dropdown_procedure.html.haml b/app/views/shared/help/_help_dropdown_procedure.html.haml new file mode 100644 index 000000000..d8defbfbd --- /dev/null +++ b/app/views/shared/help/_help_dropdown_procedure.html.haml @@ -0,0 +1,9 @@ +.dropdown.help-dropdown + .button.primary.dropdown-button Aide + .dropdown-content.fade-in-down + %ul.dropdown-items + - if procedure.service.present? + = render partial: 'shared/help/dropdown_items/service_item', + locals: { service: procedure.service, title: "Une question sur cette démarche ?" } + + = render partial: 'shared/help/dropdown_items/faq_item' diff --git a/spec/features/help_spec.rb b/spec/features/help_spec.rb index 6293bddad..511090c5f 100644 --- a/spec/features/help_spec.rb +++ b/spec/features/help_spec.rb @@ -8,6 +8,24 @@ feature 'Getting help:' do end end + context 'on pages related to a procedure' do + let(:procedure) { create(:procedure, :published, :with_service) } + + scenario 'a Help menu provides administration contacts and a link to the FAQ' do + visit commencer_path(path: procedure.path) + + within('.new-header') do + expect(page).to have_help_menu + end + + within('.help-dropdown') do + expect(page).to have_content(procedure.service.email) + expect(page).to have_content(procedure.service.telephone) + expect(page).to have_link(nil, href: FAQ_URL) + end + end + end + context 'as a signed-in user' do let(:user) { create(:user) } let(:procedure) { create(:procedure, :with_service) } diff --git a/spec/views/layouts/_new_header_spec.rb b/spec/views/layouts/_new_header_spec.rb index c47bd3ae2..48a2b5b5a 100644 --- a/spec/views/layouts/_new_header_spec.rb +++ b/spec/views/layouts/_new_header_spec.rb @@ -1,36 +1,58 @@ require 'spec_helper' describe 'layouts/_new_header.html.haml', type: :view do - describe 'logo link' do - before do + before do + if user sign_in user allow(controller).to receive(:nav_bar_profile).and_return(profile) - render + end + end + + subject { render } + + context 'when rendering without context' do + let(:user) { nil } + let(:profile) { nil } + + it { is_expected.to have_css("a.header-logo[href=\"#{root_path}\"]") } + + it 'displays the Help link' do + expect(subject).to have_link('Aide', href: FAQ_URL) end - subject { rendered } + context 'when on a procedure page' do + let(:procedure) { create(:procedure, :with_service) } - context 'when rendering for user' do - let(:user) { create(:user) } - let(:profile) { :user } - - it { is_expected.to have_css("a.header-logo[href=\"#{dossiers_path}\"]") } - it { is_expected.to have_link("Dossiers", href: dossiers_path) } - - it 'displays the Help button' do - expect(subject).to have_link("Aide", href: FAQ_URL) + before do + allow(controller).to receive(:procedure_for_help).and_return(procedure) end - end - - context 'when rendering for gestionnaire' do - let(:user) { create(:gestionnaire) } - let(:profile) { :gestionnaire } - - it { is_expected.to have_css("a.header-logo[href=\"#{gestionnaire_procedures_path}\"]") } it 'displays the Help dropdown menu' do - expect(rendered).to have_css(".help-dropdown") + expect(subject).to have_css(".help-dropdown") end end end + + context 'when rendering for user' do + let(:user) { create(:user) } + let(:profile) { :user } + + it { is_expected.to have_css("a.header-logo[href=\"#{dossiers_path}\"]") } + it { is_expected.to have_link("Dossiers", href: dossiers_path) } + + it 'displays the Help button' do + expect(subject).to have_link("Aide", href: FAQ_URL) + end + end + + context 'when rendering for gestionnaire' do + let(:user) { create(:gestionnaire) } + let(:profile) { :gestionnaire } + + it { is_expected.to have_css("a.header-logo[href=\"#{gestionnaire_procedures_path}\"]") } + + it 'displays the Help dropdown menu' do + expect(subject).to have_css(".help-dropdown") + end + end end From 1daf5236956bc2e4640fcd11670e26a0d1656768 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 28 Mar 2019 11:33:45 +0100 Subject: [PATCH 15/19] layout: extract part of the header to a partial --- app/views/layouts/_account_dropdown.haml | 33 ++++++++++++++++++++++++ app/views/layouts/_new_header.haml | 33 +----------------------- 2 files changed, 34 insertions(+), 32 deletions(-) create mode 100644 app/views/layouts/_account_dropdown.haml diff --git a/app/views/layouts/_account_dropdown.haml b/app/views/layouts/_account_dropdown.haml new file mode 100644 index 000000000..4044885c4 --- /dev/null +++ b/app/views/layouts/_account_dropdown.haml @@ -0,0 +1,33 @@ +%span.dropdown.header-menu-opener + %button.button.dropdown-button.header-menu-button + = image_tag "icons/account-circle.svg", title: "Mon compte" + %ul.header-menu.dropdown-content + %li + .menu-item{ title: current_email } + = current_email + - if administration_signed_in? + %li + = link_to manager_root_path, class: "menu-item menu-link" do + = image_tag "icons/super-admin.svg" + Passer en super-admin + - if SwitchDeviseProfileService.new(warden).multiple_devise_profile_connect? + - if user_signed_in? && nav_bar_profile != :user + %li + = link_to dossiers_path, class: "menu-item menu-link" do + = image_tag "icons/switch-profile.svg" + Passer en usager + - if gestionnaire_signed_in? && nav_bar_profile != :gestionnaire + %li + = link_to gestionnaire_procedures_path, class: "menu-item menu-link" do + = image_tag "icons/switch-profile.svg" + Passer en instructeur + - if administrateur_signed_in? && nav_bar_profile != :administrateur + %li + = link_to admin_procedures_path, class: "menu-item menu-link" do + = image_tag "icons/switch-profile.svg" + Passer en administrateur + + %li + = link_to destroy_user_session_path, method: :delete, class: "menu-item menu-link" do + = image_tag "icons/sign-out.svg" + Se déconnecter diff --git a/app/views/layouts/_new_header.haml b/app/views/layouts/_new_header.haml index 2038dbbae..8bb1ca264 100644 --- a/app/views/layouts/_new_header.haml +++ b/app/views/layouts/_new_header.haml @@ -47,39 +47,8 @@ - if gestionnaire_signed_in? || user_signed_in? %li - %span.dropdown.header-menu-opener - %button.button.dropdown-button.header-menu-button - = image_tag "icons/account-circle.svg", title: "Mon compte" - %ul.header-menu.dropdown-content - %li - .menu-item{ title: current_email } - = current_email - - if administration_signed_in? - %li - = link_to manager_root_path, class: "menu-item menu-link" do - = image_tag "icons/super-admin.svg" - Passer en super-admin - - if SwitchDeviseProfileService.new(warden).multiple_devise_profile_connect? - - if user_signed_in? && nav_bar_profile != :user - %li - = link_to dossiers_path, class: "menu-item menu-link" do - = image_tag "icons/switch-profile.svg" - Passer en usager - - if gestionnaire_signed_in? && nav_bar_profile != :gestionnaire - %li - = link_to gestionnaire_procedures_path, class: "menu-item menu-link" do - = image_tag "icons/switch-profile.svg" - Passer en instructeur - - if administrateur_signed_in? && nav_bar_profile != :administrateur - %li - = link_to admin_procedures_path, class: "menu-item menu-link" do - = image_tag "icons/switch-profile.svg" - Passer en administrateur + = render partial: 'layouts/account_dropdown', locals: { nav_bar_profile: nav_bar_profile } - %li - = link_to destroy_user_session_path, method: :delete, class: "menu-item menu-link" do - = image_tag "icons/sign-out.svg" - Se déconnecter - elsif request.path != new_user_session_path - if request.path == new_user_registration_path %li From e72638f3bab90992fc33980f8beef025a37ed95b Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 28 Mar 2019 11:04:00 +0100 Subject: [PATCH 16/19] bin: run after_party tasks on bin/update --- bin/update | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bin/update b/bin/update index 34bd0bdcd..c1968201d 100755 --- a/bin/update +++ b/bin/update @@ -21,6 +21,9 @@ chdir APP_ROOT do puts "\n== Updating database ==" system! 'bin/rails db:migrate' + puts "\n== Running after_party tasks ==" + system! 'bin/rails after_party:run' + puts "\n== Removing old logs ==" system! 'bin/rails log:clear' From 5e21ebd93f3865d7d7de81f48efe8e20ebaedcf2 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Wed, 27 Mar 2019 18:31:28 +0100 Subject: [PATCH 17/19] Move procedure library to new design --- .../new_procedure_from_existing.scss | 12 ++++ .../admin/procedures_controller.rb | 1 + .../procedures/new_from_existing.html.haml | 58 ++++++++++--------- 3 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 app/assets/stylesheets/new_design/new_procedure_from_existing.scss diff --git a/app/assets/stylesheets/new_design/new_procedure_from_existing.scss b/app/assets/stylesheets/new_design/new_procedure_from_existing.scss new file mode 100644 index 000000000..350c11bf3 --- /dev/null +++ b/app/assets/stylesheets/new_design/new_procedure_from_existing.scss @@ -0,0 +1,12 @@ +.table.vertical.procedure-library-list th { + padding-top: 32px; +} + +.table.vertical.procedure-library-list td { + padding-top: 0; + padding-bottom: 4px; +} + +.procedure-library-list .button { + margin: 0 2px; +} diff --git a/app/controllers/admin/procedures_controller.rb b/app/controllers/admin/procedures_controller.rb index 2a8b8dc6b..ed180491d 100644 --- a/app/controllers/admin/procedures_controller.rb +++ b/app/controllers/admin/procedures_controller.rb @@ -199,6 +199,7 @@ class Admin::ProceduresController < AdminController .where(id: significant_procedure_ids) .group_by(&:organisation_name) .sort_by { |_, procedures| procedures.first.created_at } + render layout: 'application' end def active_class diff --git a/app/views/admin/procedures/new_from_existing.html.haml b/app/views/admin/procedures/new_from_existing.html.haml index a89acf5e6..c13262c8a 100644 --- a/app/views/admin/procedures/new_from_existing.html.haml +++ b/app/views/admin/procedures/new_from_existing.html.haml @@ -1,30 +1,34 @@ -- if current_administrateur.procedures.brouillons.count == 0 - %h4{ style: 'padding: 20px; margin: 20px !important;' } - Bienvenue, vous allez pouvoir créer une première démarche de test. Celle-ci sera visible uniquement par vous et ne sera publiée nulle part, alors pas de crainte à avoir. +.container + - if current_administrateur.procedures.brouillons.count == 0 + .card.feedback + .card-title + Bienvenue, + vous allez pouvoir créer une première démarche de test. + Celle-ci sera visible uniquement par vous et ne sera publiée nulle part, alors pas de crainte à avoir. -.row{ style: 'padding: 20px; margin: 20px !important;' } - %a#from-scratch{ href: new_admin_procedure_path, class: 'btn-lg btn-primary' } - Créer une nouvelle démarche de zéro + .form + .send-wrapper + %a#from-scratch.button.primary{ href: new_admin_procedure_path } + Créer une nouvelle démarche de zéro -.row.white-back - %h3 - Créer une nouvelle démarche à partir d'une démarche existante + .card + %h2.header-section + Créer une nouvelle démarche à partir d'une démarche existante - .section.section-label - Pour rechercher dans cette liste, utilisez la fonction "Recherche" de votre navigateur (CTRL+F ou command+F) - %br - %br - - @grouped_procedures.each do |_, procedures| - %b - = procedures.first.organisation_name - %table{ style: 'margin-bottom: 40px;' } - - procedures.sort_by(&:id).each do |procedure| - %tr{ style: 'height: 36px;' } - %td{ style: 'width: 750px;' } - = procedure.libelle - %td{ style: 'padding-right: 10px; padding-left: 10px; width: 60px;' } - = link_to('Consulter', apercu_procedure_path(id: procedure.id), target: "_blank", rel: "noopener") - %td - = link_to('Cloner', admin_procedure_clone_path(procedure.id, from_new_from_existing: true), 'data-method' => :put, class: 'btn-sm btn-primary clone-btn') - %td{ style: 'padding-left: 10px;' } - = link_to('Contacter', "mailto:#{procedure.administrateurs.pluck(:email) * ","}") + %label + .notice + Pour rechercher dans cette liste, utilisez la fonction "Recherche" de votre navigateur (CTRL+F ou command+F) + + %table.table.vertical.procedure-library-list + - @grouped_procedures.each do |_, procedures| + %tr + %th + = procedures.first.organisation_name + - procedures.sort_by(&:id).each do |procedure| + %tr + %td + = procedure.libelle + %td.flex + = link_to('Consulter', apercu_procedure_path(id: procedure.id), target: "_blank", rel: "noopener", class: 'button small') + = link_to('Cloner', admin_procedure_clone_path(procedure.id, from_new_from_existing: true), 'data-method' => :put, class: 'button small primary') + = link_to('Contacter', "mailto:#{procedure.administrateurs.pluck(:email) * ","}", class: 'button small') From cbec49aa0f9596a77578e3fb7ede31c07ae37dbe Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Wed, 27 Mar 2019 18:46:25 +0100 Subject: [PATCH 18/19] Revert "Revert "Redesign admin roles menu"" --- app/assets/stylesheets/switch_menu.scss | 14 ++++++- .../_switch_devise_profile_module.html.haml | 37 +++++++------------ app/views/layouts/application_old.html.haml | 2 - ...admin_procedurescontroller_index.html.haml | 1 + 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/app/assets/stylesheets/switch_menu.scss b/app/assets/stylesheets/switch_menu.scss index e9c18733c..bd8e913ea 100644 --- a/app/assets/stylesheets/switch_menu.scss +++ b/app/assets/stylesheets/switch_menu.scss @@ -1,6 +1,16 @@ #switch-menu { position: fixed; - left: 10px; bottom: 10px; - z-index: 300; + color: #FFFFFF; + list-style: none; + text-decoration: none; + padding-left: 20px; +} + +#switch-menu a, +#switch-menu a:link, +#switch-menu a:visited, +#switch-menu a:hover { + text-decoration: none; + color: #FFFFFF; } diff --git a/app/views/layouts/_switch_devise_profile_module.html.haml b/app/views/layouts/_switch_devise_profile_module.html.haml index 2b13779b5..02f2678a2 100644 --- a/app/views/layouts/_switch_devise_profile_module.html.haml +++ b/app/views/layouts/_switch_devise_profile_module.html.haml @@ -1,25 +1,14 @@ - if SwitchDeviseProfileService.new(warden).multiple_devise_profile_connect? - #switch-menu.dropdown.dropup - %button.btn.btn-default.dropdown-toggle{ type: :button, 'data-toggle' => 'dropdown', 'aria-haspopup' => true, 'aria-expanded' => false } - %i.fa.fa-toggle-on - %span.caret - %ul.dropdown-menu.dropdown-menu-left - - if user_signed_in? - %li - = link_to(dossiers_path, id: :menu_item_procedure) do - %i.fa.fa-user -   - Usager - - if gestionnaire_signed_in? - %li - = link_to(gestionnaire_procedures_path) do - %i.fa.fa-user -   - Instructeur - - - if administrateur_signed_in? - %li - = link_to(admin_procedures_path) do - %i.fa.fa-user -   - Administrateur + %ul#switch-menu + %li + Changer de rôle + - if user_signed_in? + %li + = link_to(dossiers_path, id: :menu_item_procedure, title: 'Aller dans votre espace usager. Vous pourrez revenir ici ensuite') do + %i.fa.fa-users +   Usager + - if gestionnaire_signed_in? + %li + = link_to(gestionnaire_procedures_path, title: 'Aller dans votre espace instructeur. Vous pourrez revenir ici ensuite.') do + %i.fa.fa-user +   Instructeur diff --git a/app/views/layouts/application_old.html.haml b/app/views/layouts/application_old.html.haml index a5a6a67b8..633e368dc 100644 --- a/app/views/layouts/application_old.html.haml +++ b/app/views/layouts/application_old.html.haml @@ -48,6 +48,4 @@ %h1 %i.fa.fa-times{ style: 'position: fixed; top: 10; right: 30; color: white;' } - = render partial: 'layouts/switch_devise_profile_module' - = render partial: 'layouts/footer', locals: { main_container_size: main_container_size } diff --git a/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_index.html.haml b/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_index.html.haml index 508fafc9d..3a8c3aec8 100644 --- a/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_index.html.haml +++ b/app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_index.html.haml @@ -28,6 +28,7 @@ = current_administrateur.procedures.archivees.count .split-hr-left + = render partial: 'layouts/switch_devise_profile_module' #infos-block From 06e7355d5b9886cdd00a597f4c7bef1448a75191 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 27 Mar 2019 16:46:35 +0000 Subject: [PATCH 19/19] commencer: fix invalid URL on test procedures Fix #3693 --- app/controllers/users/commencer_controller.rb | 2 +- .../users/commencer_controller_spec.rb | 48 ++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/app/controllers/users/commencer_controller.rb b/app/controllers/users/commencer_controller.rb index f74a56d7d..f42c5c31c 100644 --- a/app/controllers/users/commencer_controller.rb +++ b/app/controllers/users/commencer_controller.rb @@ -42,7 +42,7 @@ module Users def store_user_location! procedure = Procedure.find_by(path: params[:path]) - store_location_for(:user, commencer_path(path: procedure.path)) + store_location_for(:user, helpers.procedure_lien(procedure)) end end end diff --git a/spec/controllers/users/commencer_controller_spec.rb b/spec/controllers/users/commencer_controller_spec.rb index 2ef2a9ab5..cb6e8b069 100644 --- a/spec/controllers/users/commencer_controller_spec.rb +++ b/spec/controllers/users/commencer_controller_spec.rb @@ -18,7 +18,7 @@ describe Users::CommencerController, type: :controller do end end - context 'when the path is for a non-published procedure' do + context 'when the path is for a draft procedure' do let(:path) { draft_procedure.path } it 'redirects with an error message' do @@ -66,24 +66,50 @@ describe Users::CommencerController, type: :controller do end describe '#sign_in' do - subject { get :sign_in, params: { path: published_procedure.path } } + context 'for a published procedure' do + subject { get :sign_in, params: { path: published_procedure.path } } - it 'set the path to return after sign-in to the dossier creation path' do - subject - expect(controller.stored_location_for(:user)).to eq(commencer_path(path: published_procedure.path)) + it 'set the path to return after sign-in to the procedure start page' do + subject + expect(controller.stored_location_for(:user)).to eq(commencer_path(path: published_procedure.path)) + end + + it { expect(subject).to redirect_to(new_user_session_path) } end - it { expect(subject).to redirect_to(new_user_session_path) } + context 'for a draft procedure' do + subject { get :sign_in, params: { path: draft_procedure.path } } + + it 'set the path to return after sign-in to the draft procedure start page' do + subject + expect(controller.stored_location_for(:user)).to eq(commencer_test_path(path: draft_procedure.path)) + end + + it { expect(subject).to redirect_to(new_user_session_path) } + end end describe '#sign_up' do - subject { get :sign_up, params: { path: published_procedure.path } } + context 'for a published procedure' do + subject { get :sign_up, params: { path: published_procedure.path } } - it 'set the path to return after sign-up to the dossier creation path' do - subject - expect(controller.stored_location_for(:user)).to eq(commencer_path(path: published_procedure.path)) + it 'set the path to return after sign-up to the procedure start page' do + subject + expect(controller.stored_location_for(:user)).to eq(commencer_path(path: published_procedure.path)) + end + + it { expect(subject).to redirect_to(new_user_registration_path) } end - it { expect(subject).to redirect_to(new_user_registration_path) } + context 'for a draft procedure' do + subject { get :sign_up, params: { path: draft_procedure.path } } + + it 'set the path to return after sign-up to the draft procedure start page' do + subject + expect(controller.stored_location_for(:user)).to eq(commencer_test_path(path: draft_procedure.path)) + end + + it { expect(subject).to redirect_to(new_user_registration_path) } + end end end