Merge pull request #2958 from tchak/optimize-api

Eliminate n+1 from API calls

app/controllers/api/v1/dossiers_controller.rb

@@ -4,7 +4,7 @@ class API::V1::DossiersController < APIController
   DEFAULT_PAGE_SIZE = 100
 
   def index
-    dossiers = @procedure.dossiers.state_not_brouillon.page(params[:page]).per(per_page)
+    dossiers = @dossiers.page(params[:page]).per(per_page)
 
     render json: { dossiers: dossiers.map{ |dossier| DossiersSerializer.new(dossier) }, pagination: pagination(dossiers) }, status: 200
   rescue ActiveRecord::RecordNotFound
@@ -12,7 +12,7 @@ class API::V1::DossiersController < APIController
   end
 
   def show
-    dossier = @procedure.dossiers.find(params[:id])
+    dossier = @dossiers.for_api.find(params[:id])
 
     respond_to do |format|
       format.json { render json: { dossier: DossierSerializer.new(dossier).as_json }, status: 200 }
@@ -36,12 +36,14 @@ class API::V1::DossiersController < APIController
   end
 
   def fetch_procedure_and_check_token
-    @procedure = Procedure.includes(:administrateur).find(params[:procedure_id])
+    @procedure = Procedure.for_api.find(params[:procedure_id])
 
     if !valid_token_for_administrateur?(@procedure.administrateur)
       render json: {}, status: :unauthorized
     end
 
+    @dossiers = @procedure.dossiers.state_not_brouillon
+
   rescue ActiveRecord::RecordNotFound
     render json: {}, status: :not_found
   end

app/controllers/api/v1/procedures_controller.rb

@@ -8,7 +8,7 @@ class API::V1::ProceduresController < APIController
   private
 
   def fetch_procedure_and_check_token
-    @procedure = Procedure.includes(:administrateur).find(params[:id])
+    @procedure = Procedure.for_api.find(params[:id])
 
     if !valid_token_for_administrateur?(@procedure.administrateur)
       render json: {}, status: :unauthorized

app/models/champ.rb

@@ -5,6 +5,11 @@ class Champ < ApplicationRecord
   has_one_attached :piece_justificative_file
   has_one :virus_scan
 
+  # We declare champ specific relationships (Champs::CarteChamp and Champs::SiretChamp)
+  # here because otherwise we can't easily use includes in our queries.
+  has_many :geo_areas, dependent: :destroy
+  belongs_to :etablissement, dependent: :destroy
+
   delegate :libelle, :type_champ, :order_place, :mandatory?, :description, :drop_down_list, to: :type_de_champ
 
   scope :updated_since?, -> (date) { where('champs.updated_at > ?', date) }

app/models/champs/carte_champ.rb

@@ -1,6 +1,4 @@
 class Champs::CarteChamp < Champ
-  has_many :geo_areas, foreign_key: :champ_id, dependent: :destroy
-
   # We are not using scopes here as we want to access
   # the following collections on unsaved records.
   def cadastres

app/models/champs/siret_champ.rb

@@ -37,7 +37,6 @@ class Champs::SiretChamp < Champ
     ]
   ]
 
-  belongs_to :etablissement, dependent: :destroy
   accepts_nested_attributes_for :etablissement, allow_destroy: true, update_only: true
 
   def search_terms

app/models/dossier.rb

@@ -59,6 +59,26 @@ class Dossier < ApplicationRecord
   scope :with_champs,                 -> { includes(champs: :type_de_champ) }
   scope :nearing_end_of_retention,    -> (duration = '1 month') { joins(:procedure).where("en_instruction_at + (duree_conservation_dossiers_dans_ds * interval '1 month') - now() < interval ?", duration) }
 
+  scope :for_api, -> {
+    includes(commentaires: [],
+      champs: [
+        :geo_areas,
+        :etablissement,
+        piece_justificative_file_attachment: :blob
+      ],
+      champs_private: [
+        :geo_areas,
+        :etablissement,
+        piece_justificative_file_attachment: :blob
+      ],
+      pieces_justificatives: [],
+      quartier_prioritaires: [],
+      cadastres: [],
+      etablissement: [],
+      individual: [],
+      user: [])
+  }
+
   accepts_nested_attributes_for :individual
 
   delegate :siret, :siren, to: :etablissement, allow_nil: true

app/models/procedure.rb

@@ -47,6 +47,16 @@ class Procedure < ApplicationRecord
   scope :cloned_from_library,   -> { where(cloned_from_library: true) }
   scope :avec_lien,             -> { where.not(path: nil) }
 
+  scope :for_api, -> {
+    includes(
+      :administrateur,
+      :types_de_champ_private,
+      :types_de_champ,
+      :types_de_piece_justificative,
+      :module_api_carto
+    )
+  }
+
   validates :libelle, presence: true, allow_blank: false, allow_nil: false
   validates :description, presence: true, allow_blank: false, allow_nil: false
   validate :check_juridique

lib/tasks/2018_07_31_nutriscore.rake

@@ -67,6 +67,9 @@ namespace :'2018_07_31_nutriscore' do
             libelle: 'Numéro SIRET'
           )
         ) do |d, target_tdc|
+          if d.etablissement.present?
+            d.etablissement.signature = d.etablissement.sign
+          end
           target_tdc.champ.create(
             value: d.etablissement&.siret,
             etablissement: d.etablissement,

spec/controllers/api/v1/dossiers_controller_spec.rb

@@ -136,7 +136,7 @@ describe API::V1::DossiersController do
       context 'when dossier exists and belongs to procedure' do
         let(:procedure_id) { procedure.id }
         let(:date_creation) { Time.zone.local(2008, 9, 1, 10, 5, 0) }
-        let!(:dossier) { Timecop.freeze(date_creation) { create(:dossier, :with_entreprise, procedure: procedure, motivation: "Motivation") } }
+        let!(:dossier) { Timecop.freeze(date_creation) { create(:dossier, :with_entreprise, :en_construction, procedure: procedure, motivation: "Motivation") } }
         let(:dossier_id) { dossier.id }
         let(:body) { JSON.parse(retour.body, symbolize_names: true) }
         let(:field_list) { [:id, :created_at, :updated_at, :archived, :individual, :entreprise, :etablissement, :cerfa, :types_de_piece_justificative, :pieces_justificatives, :champs, :champs_private, :commentaires, :state, :simplified_state, :initiated_at, :processed_at, :received_at, :motivation, :email, :instructeurs] }
@@ -147,7 +147,7 @@ describe API::V1::DossiersController do
         end
 
         it { expect(subject[:id]).to eq(dossier.id) }
-        it { expect(subject[:state]).to eq(dossier.state) }
+        it { expect(subject[:state]).to eq('initiated') }
         it { expect(subject[:created_at]).to eq('2008-09-01T08:05:00.000Z') }
         it { expect(subject[:updated_at]).to eq('2008-09-01T08:05:00.000Z') }
         it { expect(subject[:archived]).to eq(dossier.archived) }