From e1740798508e65e711949e2b70ccd8732cbbb3f8 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Tue, 29 Sep 2020 10:52:13 +0200 Subject: [PATCH 1/5] fix send a copy link --- .../new_administrateur/procedures_controller.rb | 7 +++++-- .../new_administrateur/procedures/publication.html.haml | 2 -- app/views/new_administrateur/procedures/show.html.haml | 2 +- .../new_administrateur/procedures/transfert.html.haml | 6 ++++++ config/routes.rb | 1 + .../new_administrateur/procedures_controller_spec.rb | 2 +- 6 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 app/views/new_administrateur/procedures/transfert.html.haml diff --git a/app/controllers/new_administrateur/procedures_controller.rb b/app/controllers/new_administrateur/procedures_controller.rb index ac9c15f99..136dffb12 100644 --- a/app/controllers/new_administrateur/procedures_controller.rb +++ b/app/controllers/new_administrateur/procedures_controller.rb @@ -1,6 +1,6 @@ module NewAdministrateur class ProceduresController < AdministrateurController - before_action :retrieve_procedure, only: [:champs, :annotations, :edit, :monavis, :update_monavis, :jeton, :update_jeton, :publication, :publish] + before_action :retrieve_procedure, only: [:champs, :annotations, :edit, :monavis, :update_monavis, :jeton, :update_jeton, :publication, :publish, :transfert] before_action :procedure_locked?, only: [:champs, :annotations] ITEMS_PER_PAGE = 25 @@ -155,10 +155,13 @@ module NewAdministrateur end end + def transfert + end + def transfer admin = Administrateur.by_email(params[:email_admin].downcase) if admin.nil? - redirect_to admin_procedure_publication_path(params[:procedure_id]) + redirect_to admin_procedure_transfert_path(params[:procedure_id]) flash.alert = "Envoi vers #{params[:email_admin]} impossible : cet administrateur n'existe pas" else procedure = current_administrateur.procedures.find(params[:procedure_id]) diff --git a/app/views/new_administrateur/procedures/publication.html.haml b/app/views/new_administrateur/procedures/publication.html.haml index 26d4ed03b..efd17f78e 100644 --- a/app/views/new_administrateur/procedures/publication.html.haml +++ b/app/views/new_administrateur/procedures/publication.html.haml @@ -20,8 +20,6 @@ = link_to @procedure_lien, sanitize_url(@procedure_lien), target: :blank, rel: :noopener, class: "mb-4" %p.mb-4 Attention, diffusez toujours le lien complet affiché ci-dessus, et non pas un lien générique vers #{APPLICATION_NAME}. Ne dites pas non plus aux usagers de se rendre sur le site générique #{APPLICATION_NAME}, donnez-leur toujours le lien complet. - - = render partial: 'procedure_transfert' - elsif @procedure.brouillon? - if @procedure.missing_steps.empty? %p diff --git a/app/views/new_administrateur/procedures/show.html.haml b/app/views/new_administrateur/procedures/show.html.haml index af87662ef..802dda5e3 100644 --- a/app/views/new_administrateur/procedures/show.html.haml +++ b/app/views/new_administrateur/procedures/show.html.haml @@ -13,7 +13,7 @@ Tester - if @procedure.publiee? || @procedure.brouillon? - = link_to admin_procedure_publication_path(@procedure), class: 'button' do + = link_to admin_procedure_transfert_path(@procedure), class: 'button' do %span.icon.reply Envoyer une copie diff --git a/app/views/new_administrateur/procedures/transfert.html.haml b/app/views/new_administrateur/procedures/transfert.html.haml new file mode 100644 index 000000000..1f7666686 --- /dev/null +++ b/app/views/new_administrateur/procedures/transfert.html.haml @@ -0,0 +1,6 @@ += render partial: 'new_administrateur/breadcrumbs', + locals: { steps: [link_to('Démarches', admin_procedures_path), + link_to(@procedure.libelle, admin_procedure_path(@procedure)), + 'Transfert'] } +.container + = render partial: 'procedure_transfert' diff --git a/config/routes.rb b/config/routes.rb index fc8eec97d..408c3fcb3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -369,6 +369,7 @@ Rails.application.routes.draw do get 'publication' => 'procedures#publication', as: :publication put 'publish' => 'procedures#publish', as: :publish + get 'transfert' => 'procedures#transfert', as: :transfert post 'transfer' => 'procedures#transfer', as: :transfer resources :mail_templates, only: [:edit, :update] diff --git a/spec/controllers/new_administrateur/procedures_controller_spec.rb b/spec/controllers/new_administrateur/procedures_controller_spec.rb index be3a6845b..289016808 100644 --- a/spec/controllers/new_administrateur/procedures_controller_spec.rb +++ b/spec/controllers/new_administrateur/procedures_controller_spec.rb @@ -450,7 +450,7 @@ describe NewAdministrateur::ProceduresController, type: :controller do let(:email_admin) { 'plop' } it { expect(subject.status).to eq 302 } - it { expect(response.body).to include(admin_procedure_publication_path(procedure.id)) } + it { expect(response.body).to include(admin_procedure_transfert_path(procedure.id)) } it { expect(flash[:alert]).to be_present } it { expect(flash[:alert]).to eq("Envoi vers #{email_admin} impossible : cet administrateur n'existe pas") } end From 1bd59c72e5a1e636664f88d1434629995b70c6bd Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 8 Sep 2020 15:53:07 +0200 Subject: [PATCH 2/5] Include in exports types_de_champ from all revisions --- app/models/dossier.rb | 44 ++++++++++++++++-------- app/models/procedure.rb | 39 ++++++++++++++++++--- app/models/procedure_revision.rb | 11 +++++- app/models/type_de_champ.rb | 12 +++---- app/services/procedure_export_service.rb | 11 +++++- spec/models/dossier_spec.rb | 27 +++++++++++++++ 6 files changed, 117 insertions(+), 27 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 5001e0c5f..1c1b1b559 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -649,19 +649,19 @@ class Dossier < ApplicationRecord log_dossier_operation(avis.claimant, :demander_un_avis, avis) end - def spreadsheet_columns_csv - spreadsheet_columns(with_etablissement: true) + def spreadsheet_columns_csv(types_de_champ:, types_de_champ_private:) + spreadsheet_columns(with_etablissement: true, types_de_champ: types_de_champ, types_de_champ_private: types_de_champ_private) end - def spreadsheet_columns_xlsx - spreadsheet_columns + def spreadsheet_columns_xlsx(types_de_champ:, types_de_champ_private:) + spreadsheet_columns(types_de_champ: types_de_champ, types_de_champ_private: types_de_champ_private) end - def spreadsheet_columns_ods - spreadsheet_columns + def spreadsheet_columns_ods(types_de_champ:, types_de_champ_private:) + spreadsheet_columns(types_de_champ: types_de_champ, types_de_champ_private: types_de_champ_private) end - def spreadsheet_columns(with_etablissement: false) + def spreadsheet_columns(with_etablissement: false, types_de_champ:, types_de_champ_private:) columns = [ ['ID', id.to_s], ['Email', user.email] @@ -728,18 +728,34 @@ class Dossier < ApplicationRecord columns << ['Groupe instructeur', groupe_instructeur.label] end - columns + champs_for_export + annotations_for_export + columns + champs_for_export(types_de_champ) + champs_private_for_export(types_de_champ_private) end - def champs_for_export - champs.reject(&:exclude_from_export?).map do |champ| - [champ.libelle, champ.for_export] + def champs_for_export(types_de_champ) + # Index values by stable_id + values = champs.reject(&:exclude_from_export?).reduce({}) do |champs, champ| + champs[champ.stable_id] = champ.for_export + champs + end + + # Get all the champs values for the types de champ in the final list. + # Dossier might not have corresponding champ – display nil. + types_de_champ.map do |type_de_champ| + [type_de_champ.libelle, values[type_de_champ.stable_id]] end end - def annotations_for_export - champs_private.reject(&:exclude_from_export?).map do |champ| - [champ.libelle, champ.for_export] + def champs_private_for_export(types_de_champ) + # Index values by stable_id + values = champs_private.reject(&:exclude_from_export?).reduce({}) do |champs, champ| + champs[champ.stable_id] = champ.for_export + champs + end + + # Get all the champs values for the types de champ in the final list. + # Dossier might not have corresponding champ – display nil. + types_de_champ.map do |type_de_champ| + [type_de_champ.libelle, values[type_de_champ.stable_id]] end end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index af7bc9a9d..7493f8cb6 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -61,10 +61,13 @@ class Procedure < ApplicationRecord belongs_to :published_revision, class_name: 'ProcedureRevision', optional: true has_many :deleted_dossiers, dependent: :destroy - has_many :published_types_de_champ, through: :published_revision, source: :types_de_champ - has_many :published_types_de_champ_private, through: :published_revision, source: :types_de_champ_private - has_many :draft_types_de_champ, through: :draft_revision, source: :types_de_champ - has_many :draft_types_de_champ_private, through: :draft_revision, source: :types_de_champ_private + has_many :published_types_de_champ, -> { ordered }, through: :published_revision, source: :types_de_champ + has_many :published_types_de_champ_private, -> { ordered }, through: :published_revision, source: :types_de_champ_private + has_many :draft_types_de_champ, -> { ordered }, through: :draft_revision, source: :types_de_champ + has_many :draft_types_de_champ_private, -> { ordered }, through: :draft_revision, source: :types_de_champ_private + + has_many :all_types_de_champ, -> { joins(:procedure).where('types_de_champ.revision_id != procedures.draft_revision_id').ordered }, through: :revisions, source: :types_de_champ + has_many :all_types_de_champ_private, -> { joins(:procedure).where('types_de_champ.revision_id != procedures.draft_revision_id').ordered }, through: :revisions, source: :types_de_champ_private has_one :module_api_carto, dependent: :destroy has_one :attestation_template, dependent: :destroy @@ -85,6 +88,34 @@ class Procedure < ApplicationRecord brouillon? ? draft_types_de_champ_private : published_types_de_champ_private end + def types_de_champ_for_export + if brouillon? + draft_types_de_champ + else + all_types_de_champ + .uniq + .reject(&:exclude_from_export?) + .filter(&:active_revision?) + .group_by(&:stable_id).values.map do |types_de_champ| + types_de_champ.sort_by(&:created_at).last + end + end + end + + def types_de_champ_private_for_export + if brouillon? + draft_types_de_champ_private + else + all_types_de_champ_private + .uniq + .reject(&:exclude_from_export?) + .filter(&:active_revision?) + .group_by(&:stable_id).values.map do |types_de_champ| + types_de_champ.sort_by(&:created_at).last + end + end + end + has_many :administrateurs_procedures has_many :administrateurs, through: :administrateurs_procedures, after_remove: -> (procedure, _admin) { procedure.validate! } has_many :groupe_instructeurs, dependent: :destroy diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index 54fe8b729..3c1a05ba3 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -50,7 +50,7 @@ class ProcedureRevision < ApplicationRecord elsif type_de_champ.parent.present? find_or_clone_type_de_champ(type_de_champ.parent.stable_id).types_de_champ.find_by!(stable_id: id) else - type_de_champ.revise! + revise_type_de_champ(type_de_champ) end end @@ -98,6 +98,15 @@ class ProcedureRevision < ApplicationRecord private + def revise_type_de_champ(type_de_champ) + types_de_champ_association = type_de_champ.private? ? :revision_types_de_champ_private : :revision_types_de_champ + association = send(types_de_champ_association).find_by!(type_de_champ: type_de_champ) + cloned_type_de_champ = type_de_champ.deep_clone(include: [:types_de_champ], &type_de_champ.method(:clone_attachments)) + cloned_type_de_champ.revision = self + association.update!(type_de_champ: cloned_type_de_champ) + cloned_type_de_champ + end + def find_type_de_champ_by_id(id) types_de_champ.find_by(stable_id: id) || types_de_champ_private.find_by(stable_id: id) || diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index 2f391cd11..50f4e576a 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -58,6 +58,7 @@ class TypeDeChamp < ApplicationRecord store_accessor :options, :cadastres, :quartiers_prioritaires, :parcelles_agricoles, :mnhn, :old_pj, :drop_down_options, :skip_pj_validation has_many :revision_types_de_champ, class_name: 'ProcedureRevisionTypeDeChamp', dependent: :destroy, inverse_of: :type_de_champ + has_many :revisions, through: :revision_types_de_champ delegate :tags_for_template, to: :dynamic_type @@ -200,6 +201,10 @@ class TypeDeChamp < ApplicationRecord !private? end + def active_revision? + revisions.include?(procedure.active_revision) + end + def self.type_champ_to_class_name(type_champ) "TypesDeChamp::#{type_champ.classify}TypeDeChamp" end @@ -248,13 +253,6 @@ class TypeDeChamp < ApplicationRecord GraphQL::Schema::UniqueWithinType.encode('Champ', stable_id) end - def revise! - types_de_champ_association = private? ? :revision_types_de_champ_private : :revision_types_de_champ - association = revision.send(types_de_champ_association).find_by!(type_de_champ: self) - association.update!(type_de_champ: deep_clone(include: [:types_de_champ], &method(:clone_attachments))) - association.type_de_champ - end - FEATURE_FLAGS = {} def self.type_de_champ_types_for(procedure, user) diff --git a/app/services/procedure_export_service.rb b/app/services/procedure_export_service.rb index ff4ecb179..437d4b13d 100644 --- a/app/services/procedure_export_service.rb +++ b/app/services/procedure_export_service.rb @@ -64,7 +64,7 @@ class ProcedureExportService def options_for(table, format) options = case table when :dossiers - { instances: dossiers.to_a, sheet_name: 'Dossiers', spreadsheet_columns: :"spreadsheet_columns_#{format}" } + { instances: dossiers.to_a, sheet_name: 'Dossiers', spreadsheet_columns: spreadsheet_columns(format) } when :etablissements { instances: etablissements.to_a, sheet_name: 'Etablissements' } when :avis @@ -82,4 +82,13 @@ class ProcedureExportService options end + + def spreadsheet_columns(format) + types_de_champ = @procedure.types_de_champ_for_export + types_de_champ_private = @procedure.types_de_champ_private_for_export + + Proc.new do |instance| + instance.send(:"spreadsheet_columns_#{format}", types_de_champ: types_de_champ, types_de_champ_private: types_de_champ_private) + end + end end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 8a6780d37..b3f8966a8 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -1343,4 +1343,31 @@ describe Dossier do end end end + + describe "champs_for_export" do + let(:procedure) { create(:procedure, :with_type_de_champ, :with_datetime, :with_yes_no) } + let(:text_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:text) } } + let(:yes_no_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:yes_no) } } + let(:datetime_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:dossier_second_revision) { create(:dossier, procedure: procedure) } + + before do + procedure.publish! + dossier + procedure.draft_revision.remove_type_de_champ(text_type_de_champ.stable_id) + procedure.draft_revision.add_type_de_champ(type_champ: TypeDeChamp.type_champs.fetch(:text), libelle: 'New text field') + procedure.draft_revision.find_or_clone_type_de_champ(yes_no_type_de_champ.stable_id).update(libelle: 'Updated yes/no') + procedure.update(published_revision: procedure.draft_revision, draft_revision: procedure.create_new_revision) + dossier.reload + procedure.reload + end + + it "should have champs from all revisions" do + expect(dossier.types_de_champ.map(&:libelle)).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Yes/no"]) + expect(dossier_second_revision.types_de_champ.map(&:libelle)).to eq([datetime_type_de_champ.libelle, "Updated yes/no", "New text field"]) + expect(dossier.champs_for_export(dossier.procedure.types_de_champ_for_export).map { |(libelle)| libelle }).to eq([datetime_type_de_champ.libelle, "Updated yes/no", "New text field"]) + expect(dossier.champs_for_export(dossier.procedure.types_de_champ_for_export)).to eq(dossier_second_revision.champs_for_export(dossier_second_revision.procedure.types_de_champ_for_export)) + end + end end From 67b8bf47543c95880ec8eeaa145dcb0c5867cb08 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Fri, 25 Sep 2020 15:19:59 +0200 Subject: [PATCH 3/5] smaller font-size for large footers in attestation --- .../attestation_templates/show.pdf.prawn | 7 ++++++- .../attestation_templates_controller_spec.rb | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/views/new_administrateur/attestation_templates/show.pdf.prawn b/app/views/new_administrateur/attestation_templates/show.pdf.prawn index 120f35383..cb3e64eae 100644 --- a/app/views/new_administrateur/attestation_templates/show.pdf.prawn +++ b/app/views/new_administrateur/attestation_templates/show.pdf.prawn @@ -74,6 +74,11 @@ prawn_document(margin: [top_margin, right_margin, bottom_margin, left_margin], p pdf.repeat(:all) do pdf.move_cursor_to footer_height - 10 pdf.fill_color grey - pdf.text footer, align: :center, size: 8 + if footer.present? + # We reduce the size of large footer so they can be drawn in the corresponding area. + # This is due to a font change, the replacing font is slightly bigger than the previous one + footer_font_size = footer.length > 170 ? 7 : 8 + pdf.text footer, align: :center, size: footer_font_size + end end end diff --git a/spec/controllers/new_administrateur/attestation_templates_controller_spec.rb b/spec/controllers/new_administrateur/attestation_templates_controller_spec.rb index 4e50739e9..2b7927ba7 100644 --- a/spec/controllers/new_administrateur/attestation_templates_controller_spec.rb +++ b/spec/controllers/new_administrateur/attestation_templates_controller_spec.rb @@ -66,6 +66,22 @@ describe NewAdministrateur::AttestationTemplatesController, type: :controller do it { expect(assigns(:attestation)[:signature]).to eq(nil) } it_behaves_like 'rendering a PDF successfully' end + + context 'with empty footer' do + let!(:attestation_template) do + create(:attestation_template, { title: 't', body: 'b', footer: nil }) + end + + it_behaves_like 'rendering a PDF successfully' + end + + context 'with large footer' do + let!(:attestation_params) do + create(:attestation_template, { title: 't', body: 'b', footer: ' ' * 190 }) + end + + it_behaves_like 'rendering a PDF successfully' + end end end From 775a677465c6ae56fccc2d8647257c3efb92c5eb Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 30 Sep 2020 11:59:49 +0200 Subject: [PATCH 4/5] GraphQL handle parse errors --- app/controllers/api/v2/graphql_controller.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/api/v2/graphql_controller.rb b/app/controllers/api/v2/graphql_controller.rb index 2e2f783bd..f25bf041d 100644 --- a/app/controllers/api/v2/graphql_controller.rb +++ b/app/controllers/api/v2/graphql_controller.rb @@ -18,6 +18,12 @@ class API::V2::GraphqlController < API::V2::BaseController private + def process_action(*args) + super + rescue ActionDispatch::Http::Parameters::ParseError => exception + render status: 400, json: { errors: [{ message: exception.cause.message }] } + end + # Handle form data, JSON body, or a blank value def ensure_hash(ambiguous_param) case ambiguous_param From 9019a8b3805d93a1a8eb4cc1f6f293871a245880 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Wed, 30 Sep 2020 15:56:06 +0200 Subject: [PATCH 5/5] ensure a dossier_id is set if possible --- app/models/champ.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/champ.rb b/app/models/champ.rb index 47bb90ba1..19e2d3987 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -61,6 +61,7 @@ class Champ < ApplicationRecord scope :root, -> { where(parent_id: nil) } + before_create :set_dossier_id, if: :needs_dossier_id? before_validation :set_dossier_id, if: :needs_dossier_id? validates :type_de_champ_id, uniqueness: { scope: [:dossier_id, :row] }