Merge pull request #5653 from betagouv/dev

2020-09-30-02

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

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])

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] }

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
 

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

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) ||

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)

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

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

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 <strong>lien complet</strong> 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

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
 

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'

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]

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
 

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

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