feat(stats#index): update Stat model to also query DossierDeleted in stats computation

tech(question): discard_and_keep_track! ; are we really keeping track with default_scope { kept } ?

feat(stats): add DeletedDossier in Stat computations

Revert "tech(question): discard_and_keep_track! ; are we really keeping track with default_scope { kept } ?"

This reverts commit d1155b7eeaaf1a9f80189e59667e109541fcb089.

feat(stats): support deleted_dossiers for last_four_months_hash and cumulative_hash. extract sanitize query & merge hashes in methdos

clean(rubocop): lint with rubocop

Update db/migrate/20211126080118_add_index_to_deleted_at_to_deleted_dossiers.rb

Co-authored-by: LeSim <mail@simon.lehericey.net>

fix(rubocop): avoid uneeded allocation

fix(migration): add concurrent index with expected synthax

fix(brakeman): add ignore message since group date_trunc evaluation is used by only ourself

app/models/stat.rb

@@ -19,7 +19,7 @@
 class Stat < ApplicationRecord
   class << self
     def update_stats
-      states = dossiers_states
+      states = sum_hashes(dossiers_states, deleted_dossiers_states)
       stat = Stat.first || Stat.new
 
       stat.update(
@@ -30,8 +30,14 @@ class Stat < ApplicationRecord
         dossiers_deposes_entre_60_et_30_jours: states['dossiers_deposes_entre_60_et_30_jours'],
         dossiers_not_brouillon: states['not_brouillon'],
         dossiers_termines: states['termines'],
-        dossiers_cumulative: cumulative_hash(Dossier.state_not_brouillon, :en_construction_at),
+        dossiers_cumulative: cumulative_hash([
-        dossiers_in_the_last_4_months: last_four_months_hash(Dossier.state_not_brouillon, :en_construction_at),
+          [Dossier.state_not_brouillon, :en_construction_at],
+          [DeletedDossier.where.not(state: :brouillon), :deleted_at]
+        ]),
+        dossiers_in_the_last_4_months: last_four_months_hash([
+          [Dossier.state_not_brouillon, :en_construction_at],
+          [DeletedDossier.where.not(state: :brouillon), :deleted_at]
+        ]),
         administrations_partenaires: AdministrateursProcedure.joins(:procedure).merge(Procedure.publiees_ou_closes).select('distinct administrateur_id').count
       )
     end
@@ -39,7 +45,7 @@ class Stat < ApplicationRecord
     private
 
     def dossiers_states
-      query = <<-EOF
+      sanitize_and_exec(Dossier, <<-EOF
         SELECT
           COUNT(*) FILTER ( WHERE state != 'brouillon' ) AS "not_brouillon",
           COUNT(*) FILTER ( WHERE state != 'brouillon' and en_construction_at BETWEEN :one_month_ago AND :now ) AS "dossiers_depose_avant_30_jours",
@@ -51,43 +57,71 @@ class Stat < ApplicationRecord
         FROM dossiers
         WHERE hidden_at IS NULL
       EOF
-
+      )
-      sanitized_query = ActiveRecord::Base.sanitize_sql([
-        query,
-        now: Time.zone.now,
-        one_month_ago: 1.month.ago,
-        two_months_ago: 2.months.ago
-      ])
-
-      Dossier.connection.select_all(sanitized_query).first
     end
 
-    def last_four_months_hash(association, date_attribute)
+    def deleted_dossiers_states
-      min_date = 3.months.ago.beginning_of_month.to_date
+      sanitize_and_exec(DeletedDossier, <<-EOF
+        SELECT
+          COUNT(*) FILTER ( WHERE state != 'brouillon' ) AS "not_brouillon",
+          COUNT(*) FILTER ( WHERE state != 'brouillon' and deleted_at BETWEEN :one_month_ago AND :now ) AS "dossiers_depose_avant_30_jours",
+          COUNT(*) FILTER ( WHERE state != 'brouillon' and deleted_at BETWEEN :two_months_ago AND :one_month_ago ) AS "dossiers_deposes_entre_60_et_30_jours",
+          COUNT(*) FILTER ( WHERE state = 'brouillon' ) AS "brouillon",
+          COUNT(*) FILTER ( WHERE state = 'en_construction' ) AS "en_construction",
+          COUNT(*) FILTER ( WHERE state = 'en_instruction' ) AS "en_instruction",
+          COUNT(*) FILTER ( WHERE state in ('accepte', 'refuse', 'sans_suite') ) AS "termines"
+        FROM deleted_dossiers
+      EOF
+      )
+    end
 
+    def last_four_months_hash(associations_with_date_attribute)
+      min_date = 3.months.ago.beginning_of_month.to_date
+      timeseries = associations_with_date_attribute.map do |association, date_attribute|
         association
           .where(date_attribute => min_date..max_date)
           .group("DATE_TRUNC('month', #{date_attribute})")
           .count
+      end
+
+      sum_hashes(*timeseries)
         .to_a
         .sort_by { |a| a[0] }
         .map { |e| [I18n.l(e.first, format: "%B %Y"), e.last] }
     end
 
-    def cumulative_hash(association, date_attribute)
+    def cumulative_hash(associations_with_date_attribute)
-      sum = 0
+      timeseries = associations_with_date_attribute.map do |association, date_attribute|
         association
           .where("#{date_attribute} < ?", max_date)
           .group("DATE_TRUNC('month', #{date_attribute})")
           .count
+      end
+
+      sum = 0
+      sum_hashes(*timeseries)
         .to_a
         .sort_by { |a| a[0] }
         .map { |x, y| { x => (sum += y) } }
         .reduce({}, :merge)
     end
 
+    def sum_hashes(*hashes)
+      {}.merge(*hashes) { |_k, hash_one_value, hash_two_value| hash_one_value + hash_two_value }
+    end
+
     def max_date
       Time.zone.now.beginning_of_month - 1.second
     end
+
+    def sanitize_and_exec(model, query)
+      sanitized_query = ActiveRecord::Base.sanitize_sql([
+        query,
+        now: Time.zone.now,
+        one_month_ago: 1.month.ago,
+        two_months_ago: 2.months.ago
+      ])
+      model.connection.select_all(sanitized_query).first
+    end
   end
 end

config/brakeman.ignore

@@ -46,7 +46,7 @@
           "type": "controller",
           "class": "Users::DossiersController",
           "method": "merci",
-          "line": 193,
+          "line": 195,
           "file": "app/controllers/users/dossiers_controller.rb",
           "rendered": {
             "name": "users/dossiers/merci",
@@ -62,6 +62,26 @@
       "confidence": "Weak",
       "note": ""
     },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "6c98e520dd368104bb0c81334875010711cd523afc28057ef86a10930f95c4b7",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/stat.rb",
+      "line": 83,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "association.where(date_attribute => ((3.months.ago.beginning_of_month.to_date..max_date))).group(\"DATE_TRUNC('month', #{date_attribute})\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Stat",
+        "method": "last_four_months_hash"
+      },
+      "user_input": "date_attribute",
+      "confidence": "Weak",
+      "note": "no user input, fixed value"
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
@@ -101,8 +121,28 @@
       "user_input": "Export.find_or_create_export(params[:export_format], (params[:time_span_type] or \"everything\"), current_instructeur.groupe_instructeurs.where(:procedure => procedure)).file.service_url",
       "confidence": "High",
       "note": ""
+    },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "dc6d873aff3dc5e51e3349b17e1f35039b23d0bddbf04224b0f1bca3e4608c1e",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/stat.rb",
+      "line": 97,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "association.where(\"#{date_attribute} < ?\", max_date).group(\"DATE_TRUNC('month', #{date_attribute})\")",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Stat",
+        "method": "cumulative_hash"
+      },
+      "user_input": "date_attribute",
+      "confidence": "Weak",
+      "note": "no user input, fixed value"
     }
   ],
-  "updated": "2021-11-23 14:09:21 +0100",
+  "updated": "2021-11-26 13:22:41 +0100",
   "brakeman_version": "5.1.1"
 }

db/migrate/20211126080118_add_index_to_deleted_at_to_deleted_dossiers.rb

@@ -0,0 +1,11 @@
+class AddIndexToDeletedAtToDeletedDossiers < ActiveRecord::Migration[6.1]
+  include Database::MigrationHelpers
+  disable_ddl_transaction!
+  def up
+    add_concurrent_index :deleted_dossiers, :deleted_at
+  end
+
+  def down
+    remove_index :deleted_dossiers, [:deleted_at]
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_11_24_111429) do
+ActiveRecord::Schema.define(version: 2021_11_26_080118) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -253,6 +253,7 @@ ActiveRecord::Schema.define(version: 2021_11_24_111429) do
     t.bigint "user_id"
     t.bigint "groupe_instructeur_id"
     t.bigint "revision_id"
+    t.index ["deleted_at"], name: "index_deleted_dossiers_on_deleted_at"
     t.index ["dossier_id"], name: "index_deleted_dossiers_on_dossier_id", unique: true
     t.index ["procedure_id"], name: "index_deleted_dossiers_on_procedure_id"
   end

spec/models/stat_spec.rb

@@ -0,0 +1,102 @@
+describe Stat do
+  describe '.deleted_dossiers_states' do
+    subject { Stat.send(:deleted_dossiers_states) }
+    it 'find counts for columns' do
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :termine)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :en_construction, deleted_at: 1.month.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :en_construction, deleted_at: 2.months.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :brouillon, deleted_at: 3.months.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :en_construction, deleted_at: 3.months.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :en_instruction, deleted_at: 3.months.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :accepte, deleted_at: 3.months.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :refuse, deleted_at: 3.months.ago)
+      create(:deleted_dossier, dossier_id: create(:dossier).id, state: :sans_suite, deleted_at: 3.months.ago)
+
+      expect(subject["not_brouillon"]).to eq(8)
+      expect(subject["dossiers_depose_avant_30_jours"]).to eq(1)
+      expect(subject["dossiers_deposes_entre_60_et_30_jours"]).to eq(1)
+      expect(subject["brouillon"]).to eq(1)
+      expect(subject["en_construction"]).to eq(3)
+      expect(subject["en_instruction"]).to eq(1)
+      expect(subject["termines"]).to eq(3)
+    end
+  end
+
+  describe '.update_stats' do
+    it 'merges dossiers_states and deleted_dossiers_states' do
+      stats = {
+        "not_brouillon" => 1,
+        "dossiers_depose_avant_30_jours" => 2,
+        "dossiers_deposes_entre_60_et_30_jours" => 3,
+        "brouillon" => 4,
+        "en_construction" => 5,
+        "en_instruction" => 6,
+        "termines" => 7
+      }
+      allow(Stat).to receive(:dossiers_states).and_return(stats)
+      allow(Stat).to receive(:deleted_dossiers_states).and_return(stats)
+
+      Stat.update_stats
+      computed_stats = Stat.first
+
+      expect(computed_stats.dossiers_not_brouillon).to eq(stats["not_brouillon"] * 2)
+      expect(computed_stats.dossiers_depose_avant_30_jours).to eq(stats["dossiers_depose_avant_30_jours"] * 2)
+      expect(computed_stats.dossiers_deposes_entre_60_et_30_jours).to eq(stats["dossiers_deposes_entre_60_et_30_jours"] * 2)
+      expect(computed_stats.dossiers_brouillon).to eq(stats["brouillon"] * 2)
+      expect(computed_stats.dossiers_en_construction).to eq(stats["en_construction"] * 2)
+      expect(computed_stats.dossiers_en_instruction).to eq(stats["en_instruction"] * 2)
+      expect(computed_stats.dossiers_termines).to eq(stats["termines"] * 2)
+    end
+  end
+
+  describe '.cumulative_hash' do
+    it 'works count and cumulate counters by month for both dossier and deleted dossiers' do
+      12.downto(1).map do |i|
+        create(:dossier, state: :en_construction, en_construction_at: i.months.ago)
+        create(:deleted_dossier, dossier_id: i + 100, state: :en_construction, deleted_at: i.month.ago)
+      end
+      rs = Stat.send(:cumulative_hash, [
+        [Dossier.state_not_brouillon, :en_construction_at],
+        [DeletedDossier.where.not(state: :brouillon), :deleted_at]
+      ])
+      expect(rs).to eq({
+        12.months.ago.utc.beginning_of_month => 2,
+       11.months.ago.utc.beginning_of_month => 4,
+       10.months.ago.utc.beginning_of_month => 6,
+       9.months.ago.utc.beginning_of_month => 8,
+       8.months.ago.utc.beginning_of_month => 10,
+       7.months.ago.utc.beginning_of_month => 12,
+       6.months.ago.utc.beginning_of_month => 14,
+       5.months.ago.utc.beginning_of_month => 16,
+       4.months.ago.utc.beginning_of_month => 18,
+       3.months.ago.utc.beginning_of_month => 20,
+       2.months.ago.utc.beginning_of_month => 22,
+       1.month.ago.utc.beginning_of_month => 24
+      })
+    end
+  end
+
+  describe '.last_four_months_hash' do
+    it 'works count and cumulate counters by month for both dossier and deleted dossiers' do
+      4.downto(1).map do |i|
+        create(:dossier, state: :en_construction, en_construction_at: i.months.ago)
+        create(:deleted_dossier, dossier_id: i + 100, state: :en_construction, deleted_at: i.month.ago)
+      end
+      rs = Stat.send(:last_four_months_hash, [
+        [Dossier.state_not_brouillon, :en_construction_at],
+        [DeletedDossier.where.not(state: :brouillon), :deleted_at]
+      ])
+      expect(rs).to eq([
+        ["août 2021", 2],
+        ["septembre 2021", 2],
+        ["octobre 2021", 2]
+      ])
+    end
+  end
+
+  describe '.sum_hashes' do
+    it 'sum up hashes keys' do
+      expect(Stat.send(:sum_hashes, *[{ a: 1, b: 2, d: 5 }, { a: 2, b: 3, c: 5 }])).to eq({ a: 3, b: 5, c: 5, d: 5 })
+    end
+  end
+end