From 650a7406425e25cd8687e8387286c5238a2f0b1a Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 11:47:13 +0200 Subject: [PATCH 01/13] [#2179] Remove dead code --- app/controllers/new_gestionnaire/recherche_controller.rb | 3 +-- app/models/search.rb | 5 ----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/app/controllers/new_gestionnaire/recherche_controller.rb b/app/controllers/new_gestionnaire/recherche_controller.rb index 58658a08b..74d00107e 100644 --- a/app/controllers/new_gestionnaire/recherche_controller.rb +++ b/app/controllers/new_gestionnaire/recherche_controller.rb @@ -17,8 +17,7 @@ module NewGestionnaire if @dossiers.empty? @dossiers = Search.new( gestionnaire: current_gestionnaire, - query: @search_terms, - page: params[:page] + query: @search_terms ).results end end diff --git a/app/models/search.rb b/app/models/search.rb index 3ab9adbd0..8f84ce027 100644 --- a/app/models/search.rb +++ b/app/models/search.rb @@ -33,7 +33,6 @@ class Search < ApplicationRecord attr_accessor :gestionnaire attr_accessor :query - attr_accessor :page belongs_to :dossier @@ -58,10 +57,6 @@ class Search < ApplicationRecord .order("rank DESC") .preload(:dossier) - if @page.present? - q = q.paginate(page: @page) - end - Results.new(q) end From 57fd59b8d5146d070c380838b55aa34f832bbfef Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 12:03:01 +0200 Subject: [PATCH 02/13] [#2179] Move code to DossierSearchService --- .../new_gestionnaire/recherche_controller.rb | 36 +---------------- app/services/dossier_search_service.rb | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 35 deletions(-) create mode 100644 app/services/dossier_search_service.rb diff --git a/app/controllers/new_gestionnaire/recherche_controller.rb b/app/controllers/new_gestionnaire/recherche_controller.rb index 74d00107e..ec58b9083 100644 --- a/app/controllers/new_gestionnaire/recherche_controller.rb +++ b/app/controllers/new_gestionnaire/recherche_controller.rb @@ -2,41 +2,7 @@ module NewGestionnaire class RechercheController < GestionnaireController def index @search_terms = params[:q] - - # 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 - ).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 + @dossiers = DossierSearchService.matching_dossiers_for_gestionnaire(@search_terms) end end end diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb new file mode 100644 index 000000000..1cf851b29 --- /dev/null +++ b/app/services/dossier_search_service.rb @@ -0,0 +1,40 @@ +class DossierSearchService + def self.matching_dossiers_for_gestionnaire(search_terms) + # 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 + ).results + end + + dossiers + end + + private + + def self.dossiers_by_id(id) + dossiers = current_gestionnaire.dossiers.where(id: id) + + current_gestionnaire.dossiers_from_avis.where(id: id) + dossiers.uniq + end + + def self.id_compatible?(number) + begin + ActiveRecord::Type::Integer.new.serialize(number) + true + rescue ActiveModel::RangeError + false + end + end +end From a72388bb3762de7c68783e0b3d702258ff15b0d2 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 12:04:49 +0200 Subject: [PATCH 03/13] [#2179] Don't depend on current_gestionnaire in DossierSearchService --- .../new_gestionnaire/recherche_controller.rb | 2 +- app/services/dossier_search_service.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/new_gestionnaire/recherche_controller.rb b/app/controllers/new_gestionnaire/recherche_controller.rb index ec58b9083..c50d321ad 100644 --- a/app/controllers/new_gestionnaire/recherche_controller.rb +++ b/app/controllers/new_gestionnaire/recherche_controller.rb @@ -2,7 +2,7 @@ module NewGestionnaire class RechercheController < GestionnaireController def index @search_terms = params[:q] - @dossiers = DossierSearchService.matching_dossiers_for_gestionnaire(@search_terms) + @dossiers = DossierSearchService.matching_dossiers_for_gestionnaire(@search_terms, current_gestionnaire) end end end diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 1cf851b29..c00c2964f 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -1,9 +1,9 @@ class DossierSearchService - def self.matching_dossiers_for_gestionnaire(search_terms) + def self.matching_dossiers_for_gestionnaire(search_terms, 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) + dossiers = dossiers_by_id(id, gestionnaire) end if dossiers.nil? @@ -13,7 +13,7 @@ class DossierSearchService # full text search if dossiers.empty? dossiers = Search.new( - gestionnaire: current_gestionnaire, + gestionnaire: gestionnaire, query: search_terms ).results end @@ -23,9 +23,9 @@ class DossierSearchService private - def self.dossiers_by_id(id) - dossiers = current_gestionnaire.dossiers.where(id: id) + - current_gestionnaire.dossiers_from_avis.where(id: id) + def self.dossiers_by_id(id, gestionnaire) + dossiers = gestionnaire.dossiers.where(id: id) + + gestionnaire.dossiers_from_avis.where(id: id) dossiers.uniq end From d734f978a32a0b2cf7a68513d710a80edee82ec0 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 14:29:37 +0200 Subject: [PATCH 04/13] [#2179] Extract dossiers_by_exact_id_for_gestionnaire method --- app/services/dossier_search_service.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index c00c2964f..e95af3b8d 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -1,14 +1,7 @@ class DossierSearchService def self.matching_dossiers_for_gestionnaire(search_terms, 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, gestionnaire) - end - - if dossiers.nil? - dossiers = Dossier.none - end + dossiers = dossier_by_exact_id_for_gestionnaire(search_terms, gestionnaire) # full text search if dossiers.empty? @@ -23,6 +16,17 @@ class DossierSearchService 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 = dossiers_by_id(id, gestionnaire) + end + + if dossiers.nil? + dossiers = Dossier.none + end + end + def self.dossiers_by_id(id, gestionnaire) dossiers = gestionnaire.dossiers.where(id: id) + gestionnaire.dossiers_from_avis.where(id: id) From b3cb06b7e590d31adba6938a91d9b27ccb038f02 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 14:32:17 +0200 Subject: [PATCH 05/13] [#2179] Extract dossier_by_full_text_for_gestionnaire method --- app/services/dossier_search_service.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index e95af3b8d..899ab383b 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -5,10 +5,7 @@ class DossierSearchService # full text search if dossiers.empty? - dossiers = Search.new( - gestionnaire: gestionnaire, - query: search_terms - ).results + dossier = dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire) end dossiers @@ -41,4 +38,11 @@ class DossierSearchService false end end + + def self.dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire) + Search.new( + gestionnaire: gestionnaire, + query: search_terms + ).results + end end From a42c4ade4cc2b95ac37c720472320cfb32edb089 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 14:45:50 +0200 Subject: [PATCH 06/13] [#2179] Simplify id_compatible? --- app/services/dossier_search_service.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 899ab383b..13cee573a 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -31,12 +31,10 @@ class DossierSearchService end def self.id_compatible?(number) - begin - ActiveRecord::Type::Integer.new.serialize(number) - true - rescue ActiveModel::RangeError - false - end + ActiveRecord::Type::Integer.new.serialize(number) + true + rescue ActiveModel::RangeError + false end def self.dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire) From 20b886fb85fa5e1a84b9a2b34294dd8dac31bca9 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 14:53:22 +0200 Subject: [PATCH 07/13] [#2179] Simplify dossiers_by_id --- app/services/dossier_search_service.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 13cee573a..38ce56610 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -25,9 +25,7 @@ class DossierSearchService end def self.dossiers_by_id(id, gestionnaire) - dossiers = gestionnaire.dossiers.where(id: id) + - gestionnaire.dossiers_from_avis.where(id: id) - dossiers.uniq + (gestionnaire.dossiers.where(id: id) + gestionnaire.dossiers_from_avis.where(id: id)).uniq end def self.id_compatible?(number) From 8744e9b83ddf05d3bf634a5336f0c7d815e1d990 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 14:54:55 +0200 Subject: [PATCH 08/13] [#2179] Simplify dossier_by_exact_id_for_gestionnaire --- app/services/dossier_search_service.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 38ce56610..5c735bb52 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -16,11 +16,9 @@ class DossierSearchService 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 = dossiers_by_id(id, gestionnaire) - end - - if dossiers.nil? - dossiers = Dossier.none + dossiers_by_id(id, gestionnaire) + else + Dossier.none end end From 8fc359c54de185c8225fd394181bee91db378e86 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 14:56:21 +0200 Subject: [PATCH 09/13] [#2179] Simplify matching_dossiers_for_gestionnaire --- app/services/dossier_search_service.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 5c735bb52..e5ce40c04 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -1,14 +1,7 @@ class DossierSearchService def self.matching_dossiers_for_gestionnaire(search_terms, gestionnaire) - # exact id match? - dossiers = dossier_by_exact_id_for_gestionnaire(search_terms, gestionnaire) - - # full text search - if dossiers.empty? - dossier = dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire) - end - - dossiers + dossier_by_exact_id_for_gestionnaire(search_terms, gestionnaire) + .presence || dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire) end private From d681b1116f1a553d1beb84e6ca29306a78e5fc03 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 15:08:10 +0200 Subject: [PATCH 10/13] [Fix #2179] Use new full text search --- app/services/dossier_search_service.rb | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index e5ce40c04..7bfb521e9 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -27,9 +27,21 @@ class DossierSearchService end def self.dossier_by_full_text_for_gestionnaire(search_terms, gestionnaire) - Search.new( - gestionnaire: gestionnaire, - query: search_terms - ).results + 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.gsub(/['?\\:&|!]/, "") # drop disallowed characters + .split(/\s+/) # split words + .map { |x| "#{x}:*" } # enable prefix matching + .join(" & ") end end From 7643459a558efc8371f45dba6a22a28e3681b0b5 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 15:47:49 +0200 Subject: [PATCH 11/13] [#2179] Reuse search spec for search service --- .../dossier_search_service_spec.rb} | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) rename spec/{models/search_spec.rb => services/dossier_search_service_spec.rb} (84%) diff --git a/spec/models/search_spec.rb b/spec/services/dossier_search_service_spec.rb similarity index 84% rename from spec/models/search_spec.rb rename to spec/services/dossier_search_service_spec.rb index 93365ae3e..697de3aa5 100644 --- a/spec/models/search_spec.rb +++ b/spec/services/dossier_search_service_spec.rb @@ -1,11 +1,11 @@ require 'spec_helper' -describe Search do - describe '#results' do +describe DossierSearchService 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_2) { create(:etablissement, entreprise_raison_sociale: 'Plop octo', dossier: dossier_2, siret: '41816602300012') } - let!(:etablissement_3) { create(:etablissement, entreprise_raison_sociale: 'OCTO Technology', dossier: dossier_3, siret: '41816609600051') } + let!(:etablissement_1) { create(:etablissement, entreprise_raison_sociale: 'OCTO Academy', siret: '41636169600051') } + let!(:dossier_1) { create(:dossier, state: 'en_construction', procedure: procedure_1, user: create(:user, email: 'contact@test.com'), etablissement: etablissement_1) } + + 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) { '' } From 1134877d5912de54bb5f583f4480e73112dca348 Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Tue, 31 Jul 2018 15:09:33 +0200 Subject: [PATCH 12/13] [#2179] Remove newly dead code --- app/models/search.rb | 77 -------------------------------------------- 1 file changed, 77 deletions(-) delete mode 100644 app/models/search.rb diff --git a/app/models/search.rb b/app/models/search.rb deleted file mode 100644 index 8f84ce027..000000000 --- a/app/models/search.rb +++ /dev/null @@ -1,77 +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 - - 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) - - 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 From cef0eafb1a589ef90f06b9f6e27aea0dbac486ba Mon Sep 17 00:00:00 2001 From: Frederic Merizen Date: Wed, 22 Aug 2018 18:36:24 +0200 Subject: [PATCH 13/13] [#2179] Tolerate spurious spaces around search terms --- app/services/dossier_search_service.rb | 7 ++++--- spec/services/dossier_search_service_spec.rb | 6 ++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 7bfb521e9..d7c61d5f7 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -39,9 +39,10 @@ class DossierSearchService end def self.to_tsquery(search_terms) - search_terms.gsub(/['?\\:&|!]/, "") # drop disallowed characters - .split(/\s+/) # split words - .map { |x| "#{x}:*" } # enable prefix matching + search_terms.strip + .gsub(/['?\\:&|!]/, "") # drop disallowed characters + .split(/\s+/) # split words + .map { |x| "#{x}:*" } # enable prefix matching .join(" & ") end end diff --git a/spec/services/dossier_search_service_spec.rb b/spec/services/dossier_search_service_spec.rb index 697de3aa5..9099d7aa4 100644 --- a/spec/services/dossier_search_service_spec.rb +++ b/spec/services/dossier_search_service_spec.rb @@ -73,6 +73,12 @@ describe DossierSearchService 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' }