Merge pull request #2317 from betagouv/frederic/fix_2179-dossier_search

Frederic/fix #2179 dossier search

app/controllers/new_gestionnaire/recherche_controller.rb

@@ -2,42 +2,7 @@ module NewGestionnaire
   class RechercheController < GestionnaireController
     def index
       @search_terms = params[:q]
-
+      @dossiers = DossierSearchService.matching_dossiers_for_gestionnaire(@search_terms, current_gestionnaire)
-      # exact id match?
-      id = @search_terms.to_i
-      if id != 0 && id_compatible?(id) # Sometimes gestionnaire is searching dossiers with a big number (ex: SIRET), ActiveRecord can't deal with them and throws ActiveModel::RangeError. id_compatible? prevents this.
-        @dossiers = dossiers_by_id(id)
-      end
-
-      if @dossiers.nil?
-        @dossiers = Dossier.none
-      end
-
-      # full text search
-      if @dossiers.empty?
-        @dossiers = Search.new(
-          gestionnaire: current_gestionnaire,
-          query: @search_terms,
-          page: params[:page]
-        ).results
-      end
-    end
-
-    private
-
-    def dossiers_by_id(id)
-      dossiers = current_gestionnaire.dossiers.where(id: id) +
-        current_gestionnaire.dossiers_from_avis.where(id: id)
-      dossiers.uniq
-    end
-
-    def id_compatible?(number)
-      begin
-        ActiveRecord::Type::Integer.new.serialize(number)
-        true
-      rescue ActiveModel::RangeError
-        false
-      end
     end
   end
 end

app/models/search.rb

@@ -1,82 +0,0 @@
-# See:
-# - https://robots.thoughtbot.com/implementing-multi-table-full-text-search-with-postgres
-# - http://calebthompson.io/talks/search.html
-class Search < ApplicationRecord
-  # :nodoc:
-  #
-  # Englobs a search result (actually a collection of Search objects) so it acts
-  # like a collection of regular Dossier objects, which can be decorated,
-  # paginated, ...
-  class Results
-    include Enumerable
-
-    def initialize(results)
-      @results = results
-    end
-
-    def each
-      @results.each do |search|
-        yield search.dossier
-      end
-    end
-
-    def method_missing(name, *args, &block)
-      @results.__send__(name, *args, &block)
-    end
-
-    def decorate!
-      @results.each do |search|
-        search.dossier = search.dossier.decorate
-      end
-    end
-  end
-
-  attr_accessor :gestionnaire
-  attr_accessor :query
-  attr_accessor :page
-
-  belongs_to :dossier
-
-  def results
-    if @query.blank?
-      return Search.none
-    end
-
-    search_term = Search.connection.quote(to_tsquery)
-
-    dossier_ids = @gestionnaire.dossiers
-      .select(:id)
-      .not_archived
-      .state_not_brouillon
-
-    q = Search
-      .select("DISTINCT(searches.dossier_id)")
-      .select("COALESCE(ts_rank(to_tsvector('french', searches.term::text), to_tsquery('french', #{search_term})), 0) AS rank")
-      .joins(:dossier)
-      .where(dossier_id: dossier_ids)
-      .where("to_tsvector('french', searches.term::text) @@ to_tsquery('french', #{search_term})")
-      .order("rank DESC")
-      .preload(:dossier)
-
-    if @page.present?
-      q = q.paginate(page: @page)
-    end
-
-    Results.new(q)
-  end
-
-  # def self.refresh
-  #   # TODO: could be executed concurrently
-  #   # See https://github.com/thoughtbot/scenic#what-about-materialized-views
-  #   Scenic.database.refresh_materialized_view(table_name, concurrently: false)
-  # end
-
-  private
-
-  def to_tsquery
-    @query.gsub(/['?\\:&|!]/, "") # drop disallowed characters
-      .split(/\s+/)               # split words
-      .map { |x| "#{x}:*" }       # enable prefix matching
-      .join(" & ")
-  end
-end

app/services/dossier_search_service.rb

@@ -0,0 +1,48 @@
+class DossierSearchService
+  def self.matching_dossiers_for_gestionnaire(search_terms, gestionnaire)
+    dossier_by_exact_id_for_gestionnaire(search_terms, gestionnaire)
+      .presence || dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire)
+  end
+
+  private
+
+  def self.dossier_by_exact_id_for_gestionnaire(search_terms, gestionnaire)
+    id = search_terms.to_i
+    if id != 0 && id_compatible?(id) # Sometimes gestionnaire is searching dossiers with a big number (ex: SIRET), ActiveRecord can't deal with them and throws ActiveModel::RangeError. id_compatible? prevents this.
+      dossiers_by_id(id, gestionnaire)
+    else
+      Dossier.none
+    end
+  end
+
+  def self.dossiers_by_id(id, gestionnaire)
+    (gestionnaire.dossiers.where(id: id) + gestionnaire.dossiers_from_avis.where(id: id)).uniq
+  end
+
+  def self.id_compatible?(number)
+    ActiveRecord::Type::Integer.new.serialize(number)
+    true
+  rescue ActiveModel::RangeError
+    false
+  end
+
+  def self.dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire)
+    ts_vector = "to_tsvector('french', search_terms || private_search_terms)"
+    ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})"
+
+    gestionnaire
+      .dossiers
+      .not_archived
+      .state_not_brouillon
+      .where("#{ts_vector} @@ #{ts_query}")
+      .order("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC")
+  end
+
+  def self.to_tsquery(search_terms)
+    search_terms.strip
+      .gsub(/['?\\:&|!]/, "") # drop disallowed characters
+      .split(/\s+/)           # split words
+      .map { |x| "#{x}:*" }   # enable prefix matching
+      .join(" & ")
+  end
+end

spec/services/dossier_search_service_spec.rb

@@ -1,11 +1,11 @@
 require 'spec_helper'
 
-describe Search do
+describe DossierSearchService do
-  describe '#results' do
+  describe '#matching_dossiers_for_gestionnaire' do
     subject { liste_dossiers }
 
     let(:liste_dossiers) do
-      described_class.new(gestionnaire: gestionnaire_1, query: terms).results
+      described_class.matching_dossiers_for_gestionnaire(terms, gestionnaire_1)
     end
 
     let(:administrateur_1) { create(:administrateur) }
@@ -23,14 +23,17 @@ describe Search do
     let(:procedure_2) { create(:procedure, :published, administrateur: administrateur_2) }
 
     let!(:dossier_0) { create(:dossier, state: 'brouillon', procedure: procedure_1, user: create(:user, email: 'brouillon@clap.fr')) }
-    let!(:dossier_1) { create(:dossier, state: 'en_construction', procedure: procedure_1, user: create(:user, email: 'contact@test.com')) }
-    let!(:dossier_2) { create(:dossier, state: 'en_construction', procedure: procedure_1, user: create(:user, email: 'plop@gmail.com')) }
-    let!(:dossier_3) { create(:dossier, state: 'en_construction', procedure: procedure_2, user: create(:user, email: 'peace@clap.fr')) }
-    let!(:dossier_archived) { create(:dossier, state: 'en_construction', procedure: procedure_1, archived: true, user: create(:user, email: 'brouillonArchived@clap.fr')) }
 
-    let!(:etablissement_1) { create(:etablissement, entreprise_raison_sociale: 'OCTO Academy', dossier: dossier_1, siret: '41636169600051') }
+    let!(:etablissement_1) { create(:etablissement, entreprise_raison_sociale: 'OCTO Academy', siret: '41636169600051') }
-    let!(:etablissement_2) { create(:etablissement, entreprise_raison_sociale: 'Plop octo', dossier: dossier_2, siret: '41816602300012') }
+    let!(:dossier_1) { create(:dossier, state: 'en_construction', procedure: procedure_1, user: create(:user, email: 'contact@test.com'), etablissement: etablissement_1) }
-    let!(:etablissement_3) { create(:etablissement, entreprise_raison_sociale: 'OCTO Technology', dossier: dossier_3, siret: '41816609600051') }
+
+    let!(:etablissement_2) { create(:etablissement, entreprise_raison_sociale: 'Plop octo', siret: '41816602300012') }
+    let!(:dossier_2) { create(:dossier, state: 'en_construction', procedure: procedure_1, user: create(:user, email: 'plop@gmail.com'), etablissement: etablissement_2) }
+
+    let!(:etablissement_3) { create(:etablissement, entreprise_raison_sociale: 'OCTO Technology', siret: '41816609600051') }
+    let!(:dossier_3) { create(:dossier, state: 'en_construction', procedure: procedure_2, user: create(:user, email: 'peace@clap.fr'), etablissement: etablissement_3) }
+
+    let!(:dossier_archived) { create(:dossier, state: 'en_construction', procedure: procedure_1, archived: true, user: create(:user, email: 'brouillonArchived@clap.fr')) }
 
     describe 'search is empty' do
       let(:terms) { '' }
@@ -70,6 +73,12 @@ describe Search do
       it { expect(subject.size).to eq(2) }
     end
 
+    describe 'search terms surrounded with spurious spaces' do
+      let(:terms) { ' OCTO ' }
+
+      it { expect(subject.size).to eq(2) }
+    end
+
     describe 'search on multiple fields' do
       let(:terms) { 'octo plop' }