feat(Column::JSONPathColumn): allow search by json path column

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

app/components/instructeurs/column_filter_component.rb

@@ -20,7 +20,7 @@ class Instructeurs::ColumnFilterComponent < ApplicationComponent
         end
       end
     else
-      find_type_de_champ(column.column).options_for_select
+      find_type_de_champ(column.column).options_for_select(column)
     end
   end
 

app/models/column.rb

@@ -17,6 +17,10 @@ class Column
     "#{table}/#{column}"
   end
 
+  def self.make_id(table, column)
+    "#{table}/#{column}"
+  end
+
   def ==(other)
     other.to_json == to_json
   end

app/models/columns/json_path_column.rb

@@ -0,0 +1,52 @@
+class Columns::JSONPathColumn < Column
+  def column
+    "#{@column}->#{value_column}" # override column otherwise json path facets will have same id as other
+  end
+
+  def filtered_ids(dossiers, search_occurences)
+    queries = Array.new(search_occurences.count, "(#{json_path_query_part} ILIKE ?)").join(' OR ')
+    dossiers.with_type_de_champ(stable_id)
+      .where(queries, *(search_occurences.map { |value| "%#{value}%" }))
+      .ids
+  end
+
+  def options_for_select
+    case value_column.last
+    when 'departement_code'
+      APIGeoService.departements.map { ["#{_1[:code]} – #{_1[:name]}", _1[:code]] }
+    when 'region_name'
+      APIGeoService.regions.map { [_1[:name], _1[:name]] }
+    else
+      []
+    end
+  end
+
+  private
+
+  def stable_id
+    @column
+  end
+
+  # given a value_column as ['value_json', 'address', 'postal_code']
+  # build SQL query as 'champs'.'value_json'->'address'->>'postal_code'
+  # see: https://www.postgresql.org/docs/9.5/functions-json.html
+  def json_path_query_part
+    *json_segments, key = value_column
+
+    if json_segments.blank? # not nested, only access using ->> Get JSON array element as text
+      "#{quote_table_column('champs')}.#{quote_table_column('value_json')}->>#{quote_json_segment(key)}"
+    else # nested, have to dig in json using -> Get JSON object field by key
+      field_accessor = json_segments.map(&method(:quote_json_segment)).join('->')
+
+      "#{quote_table_column('champs')}.#{quote_table_column('value_json')}->#{field_accessor}->>#{quote_json_segment(key)}"
+    end
+  end
+
+  def quote_table_column(table_or_column)
+    ActiveRecord::Base.connection.quote_column_name(table_or_column)
+  end
+
+  def quote_json_segment(path)
+    "'#{path}'"
+  end
+end

app/models/concerns/addressable_column_concern.rb

@@ -0,0 +1,42 @@
+module AddressableColumnConcern
+  extend ActiveSupport::Concern
+
+  included do
+    def columns(table:)
+      super.concat([
+        Columns::JSONPathColumn.new(
+          table:,
+          virtual: true,
+          column: stable_id,
+          label: "#{libelle} – code postal (5 chiffres)",
+          type: :text,
+          value_column: ['postal_code']
+        ),
+        Columns::JSONPathColumn.new(
+          table:,
+          virtual: true,
+          column: stable_id,
+          label: "#{libelle} – commune",
+          type: :text,
+          value_column: ['city_name']
+        ),
+        Columns::JSONPathColumn.new(
+          table:,
+          virtual: true,
+          column: stable_id,
+          label: "#{libelle} – département",
+          type: :enum,
+          value_column: ['departement_code']
+        ),
+        Columns::JSONPathColumn.new(
+          table:,
+          virtual: true,
+          column: stable_id,
+          label: "#{libelle} – region",
+          type: :enum,
+          value_column: ['region_name']
+        )
+      ])
+    end
+  end
+end

app/models/procedure_presentation.rb

@@ -196,59 +196,63 @@ class ProcedurePresentation < ApplicationRecord
       .map do |(table, column), filters|
       values = filters.pluck('value')
       value_column = filters.pluck('value_column').compact.first || :value
-      case table
+      dossier_column = procedure.find_column(id: Column.make_id(table, column)) # hack to find json path columns
-      when 'self'
+      if dossier_column.is_a?(Columns::JSONPathColumn)
-        field = procedure.dossier_columns.find { |h| h.column == column }
+        dossier_column.filtered_ids(dossiers, values)
-        if field.type == :date
+      else
-          dates = values
+        case table
-            .filter_map { |v| Time.zone.parse(v).beginning_of_day rescue nil }
+        when 'self'
+          if dossier_column.type == :date
+            dates = values
+              .filter_map { |v| Time.zone.parse(v).beginning_of_day rescue nil }
 
-          dossiers.filter_by_datetimes(column, dates)
+            dossiers.filter_by_datetimes(column, dates)
-        elsif field.column == "state" && values.include?("pending_correction")
+          elsif dossier_column.column == "state" && values.include?("pending_correction")
-          dossiers.joins(:corrections).where(corrections: DossierCorrection.pending)
+            dossiers.joins(:corrections).where(corrections: DossierCorrection.pending)
-        elsif field.column == "state" && values.include?("en_construction")
+          elsif dossier_column.column == "state" && values.include?("en_construction")
-          dossiers.where("dossiers.#{column} IN (?)", values).includes(:corrections).where.not(corrections: DossierCorrection.pending)
+            dossiers.where("dossiers.#{column} IN (?)", values).includes(:corrections).where.not(corrections: DossierCorrection.pending)
-        else
+          else
-          dossiers.where("dossiers.#{column} IN (?)", values)
+            dossiers.where("dossiers.#{column} IN (?)", values)
-        end
+          end
-      when TYPE_DE_CHAMP
+        when TYPE_DE_CHAMP
-        dossiers.with_type_de_champ(column)
+          dossiers.with_type_de_champ(column)
-          .filter_ilike(:champs, value_column, values)
+            .filter_ilike(:champs, value_column, values)
-      when 'etablissement'
+        when 'etablissement'
-        if column == 'entreprise_date_creation'
+          if column == 'entreprise_date_creation'
-          dates = values
+            dates = values
-            .filter_map { |v| v.to_date rescue nil }
+              .filter_map { |v| v.to_date rescue nil }
 
+            dossiers
+              .includes(table)
+              .where(table.pluralize => { column => dates })
+          else
+            dossiers
+              .includes(table)
+              .filter_ilike(table, column, values)
+          end
+        when 'followers_instructeurs'
+          assert_supported_column(table, column)
           dossiers
-            .includes(table)
+            .includes(:followers_instructeurs)
-            .where(table.pluralize => { column => dates })
+            .joins('INNER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id')
-        else
+            .filter_ilike('instructeurs_users', :email, values)
+        when 'user', 'individual', 'avis'
           dossiers
             .includes(table)
             .filter_ilike(table, column, values)
-        end
+        when 'groupe_instructeur'
-      when 'followers_instructeurs'
+          assert_supported_column(table, column)
-        assert_supported_column(table, column)
+          if column == 'label'
-        dossiers
+            dossiers
-          .includes(:followers_instructeurs)
+              .joins(:groupe_instructeur)
-          .joins('INNER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id')
+              .filter_ilike(table, column, values)
-          .filter_ilike('instructeurs_users', :email, values)
+          else
-      when 'user', 'individual', 'avis'
+            dossiers
-        dossiers
+              .joins(:groupe_instructeur)
-          .includes(table)
+              .where(groupe_instructeur_id: values)
-          .filter_ilike(table, column, values)
+          end
-      when 'groupe_instructeur'
+        end.pluck(:id)
-        assert_supported_column(table, column)
+      end
-        if column == 'label'
-          dossiers
-            .joins(:groupe_instructeur)
-            .filter_ilike(table, column, values)
-        else
-          dossiers
-            .joins(:groupe_instructeur)
-            .where(groupe_instructeur_id: values)
-        end
-      end.pluck(:id)
     end.reduce(:&)
   end
 

app/models/type_de_champ.rb

@@ -539,7 +539,7 @@ class TypeDeChamp < ApplicationRecord
     end
   end
 
-  def options_for_select
+  def options_for_select(column)
     if departement?
       APIGeoService.departements.map { ["#{_1[:code]} – #{_1[:name]}", _1[:code]] }
     elsif region?
@@ -552,6 +552,8 @@ class TypeDeChamp < ApplicationRecord
       elsif checkbox?
         Champs::CheckboxChamp.options
       end
+    elsif siret? || rna? || rnf?
+      column.options_for_select
     end
   end
 

app/models/types_de_champ/rna_type_de_champ.rb

@@ -1,4 +1,6 @@
 class TypesDeChamp::RNATypeDeChamp < TypesDeChamp::TypeDeChampBase
+  include AddressableColumnConcern
+
   def estimated_fill_duration(revision)
     FILL_DURATION_MEDIUM
   end

app/models/types_de_champ/rnf_type_de_champ.rb

@@ -1,4 +1,6 @@
 class TypesDeChamp::RNFTypeDeChamp < TypesDeChamp::TextTypeDeChamp
+  include AddressableColumnConcern
+
   class << self
     def champ_value_for_export(champ, path = :value)
       case path

app/models/types_de_champ/siret_type_de_champ.rb

@@ -1,4 +1,6 @@
 class TypesDeChamp::SiretTypeDeChamp < TypesDeChamp::TypeDeChampBase
+  include AddressableColumnConcern
+
   def estimated_fill_duration(revision)
     FILL_DURATION_MEDIUM
   end

config/brakeman.ignore

@@ -7,7 +7,7 @@
       "check_name": "CrossSiteScripting",
       "message": "Unescaped model attribute",
       "file": "app/views/users/dossiers/_merci.html.haml",
-      "line": 30,
+      "line": 34,
       "link": "https://brakemanscanner.org/docs/warning_types/cross_site_scripting",
       "code": "current_user.dossiers.includes(:procedure).find(params[:id]).procedure.monavis_embed_html_source(\"site\")",
       "render_path": [
@@ -15,7 +15,7 @@
           "type": "controller",
           "class": "Users::DossiersController",
           "method": "merci",
-          "line": 320,
+          "line": 329,
           "file": "app/controllers/users/dossiers_controller.rb",
           "rendered": {
             "name": "users/dossiers/merci",
@@ -44,6 +44,29 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "SQL Injection",
+      "warning_code": 0,
+      "fingerprint": "5ba3f5d525b15c710215829e0db49f58e8cca06d68eff5931ebfd7d0ca0e35de",
+      "check_name": "SQL",
+      "message": "Possible SQL injection",
+      "file": "app/models/columns/json_path_column.rb",
+      "line": 10,
+      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
+      "code": "dossiers.with_type_de_champ(stable_id).where(\"#{search_occurences.count} OR #{\"(#{json_path_query_part} ILIKE ?)\"}\", *search_occurences.map do\n \"%#{value}%\"\n end)",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Columns::JSONPathColumn",
+        "method": "filtered_ids"
+      },
+      "user_input": "search_occurences.count",
+      "confidence": "Weak",
+      "cwe_id": [
+        89
+      ],
+      "note": "already sanitized"
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
@@ -203,6 +226,6 @@
       "note": "Current is not a model"
     }
   ],
-  "updated": "2024-06-10 11:21:19 +0200",
+  "updated": "2024-08-20 14:34:27 +0200",
   "brakeman_version": "6.1.2"
 }

spec/models/concerns/columns_concern_spec.rb

@@ -3,11 +3,7 @@ describe ColumnsConcern do
     subject { procedure.columns }
 
     context 'when the procedure can have a SIRET number' do
-      let(:procedure) do
+      let(:procedure) { create(:procedure, types_de_champ_public:, types_de_champ_private:) }
-        create(:procedure,
-               types_de_champ_public: Array.new(4) { { type: :text } },
-               types_de_champ_private: Array.new(4) { { type: :text } })
-      end
       let(:tdc_1) { procedure.active_revision.types_de_champ_public[0] }
       let(:tdc_2) { procedure.active_revision.types_de_champ_public[1] }
       let(:tdc_private_1) { procedure.active_revision.types_de_champ_private[0] }
@@ -61,10 +57,10 @@ describe ColumnsConcern do
         it { expect(subject).to eq(expected) }
       end
 
-      xcontext 'with rna' do
+      context 'with rna' do
         let(:types_de_champ_public) { [{ type: :rna, libelle: 'rna' }] }
         let(:types_de_champ_private) { [] }
-        xit { expect(subject.map(&:label)).to include('rna – commune') }
+        it { expect(subject.map(&:label)).to include('rna – commune') }
       end
     end
 

spec/models/procedure_presentation_spec.rb

@@ -1,7 +1,8 @@
 describe ProcedurePresentation do
   include ActiveSupport::Testing::TimeHelpers
 
-  let(:procedure) { create(:procedure, :published, types_de_champ_public: [{}], types_de_champ_private: [{}]) }
+  let(:procedure) { create(:procedure, :published, types_de_champ_public:, types_de_champ_private: [{}]) }
+  let(:types_de_champ_public) { [{}] }
   let(:instructeur) { create(:instructeur) }
   let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: instructeur) }
   let(:first_type_de_champ) { assign_to.procedure.active_revision.types_de_champ_public.first }
@@ -558,6 +559,60 @@ describe ProcedurePresentation do
       end
     end
 
+    context 'for type_de_champ using AddressableColumnConcern' do
+      let(:types_de_champ_public) { [{ type: :rna, stable_id: 1 }] }
+      let(:type_de_champ) { procedure.active_revision.types_de_champ.first }
+      let(:available_columns) { type_de_champ.dynamic_type.columns(table: 'type_de_champ') }
+      let(:column) { available_columns.find { _1.value_column == value_column_searched } }
+      let(:filter) { [column.to_json.merge({ "value" => value })] }
+      let(:kept_dossier) { create(:dossier, procedure: procedure) }
+
+      context "when searching by postal_code (text)" do
+        let(:value) { "60580" }
+        let(:value_column_searched) { ['postal_code'] }
+
+        before do
+          kept_dossier.champs_public.find_by(stable_id: 1).update(value_json: { "postal_code" => value })
+          create(:dossier, procedure: procedure).champs_public.find_by(stable_id: 1).update(value_json: { "postal_code" => "unknown" })
+        end
+        it { is_expected.to contain_exactly(kept_dossier.id) }
+        it 'describes column' do
+          expect(column.type).to eq(:text)
+          expect(column.options_for_select).to eq([])
+        end
+      end
+
+      context "when searching by departement_code (enum)" do
+        let(:value) { "99" }
+        let(:value_column_searched) { ['departement_code'] }
+
+        before do
+          kept_dossier.champs_public.find_by(stable_id: 1).update(value_json: { "departement_code" => value })
+          create(:dossier, procedure: procedure).champs_public.find_by(stable_id: 1).update(value_json: { "departement_code" => "unknown" })
+        end
+        it { is_expected.to contain_exactly(kept_dossier.id) }
+        it 'describes column' do
+          expect(column.type).to eq(:enum)
+          expect(column.options_for_select.first).to eq(["99 – Etranger", "99"])
+        end
+      end
+
+      context "when searching by region_name" do
+        let(:value) { "60" }
+        let(:value_column_searched) { ['region_name'] }
+
+        before do
+          kept_dossier.champs_public.find_by(stable_id: 1).update(value_json: { "region_name" => value })
+          create(:dossier, procedure: procedure).champs_public.find_by(stable_id: 1).update(value_json: { "region_name" => "unknown" })
+        end
+        it { is_expected.to contain_exactly(kept_dossier.id) }
+        it 'describes column' do
+          expect(column.type).to eq(:enum)
+          expect(column.options_for_select.first).to eq(["Auvergne-Rhône-Alpes", "Auvergne-Rhône-Alpes"])
+        end
+      end
+    end
+
     context 'for etablissement table' do
       context 'for entreprise_date_creation column' do
         let(:filter) { [{ 'table' => 'etablissement', 'column' => 'entreprise_date_creation', 'value' => '21/6/2018' }] }