Merge pull request #6212 from betagouv/main

2021-05-19-01
This commit is contained in:
Kara Diaby 2021-05-19 11:40:34 +02:00 committed by GitHub
commit d57e1cc286
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 298 additions and 148 deletions

View file

@ -1,12 +0,0 @@
module Instructeurs
class RechercheController < InstructeurController
def index
@search_terms = params[:q]
@dossiers = DossierSearchService.matching_dossiers_for_instructeur(@search_terms, current_instructeur)
@followed_dossiers_id = current_instructeur
.followed_dossiers
.where(groupe_instructeur_id: @dossiers.pluck(:groupe_instructeur_id))
.pluck(:id)
end
end
end

View file

@ -0,0 +1,43 @@
class RechercheController < ApplicationController
before_action :authenticate_logged_user!
ITEMS_PER_PAGE = 25
PROJECTIONS = [
{ "table" => 'procedure', "column" => 'libelle' },
{ "table" => 'user', "column" => 'email' },
{ "table" => 'procedure', "column" => 'procedure_id' }
]
def index
@search_terms = search_terms
@instructeur_dossiers_ids = current_instructeur&.dossiers&.ids || []
matching_dossiers_ids = DossierSearchService
.matching_dossiers(@instructeur_dossiers_ids, @search_terms, with_annotation: true)
.to_set
@dossier_avis_ids_h = current_expert&.avis&.pluck(:dossier_id, :id).to_h || {}
expert_dossiers_ids = @dossier_avis_ids_h.keys
matching_dossiers_ids.merge(DossierSearchService.matching_dossiers(expert_dossiers_ids, @search_terms))
@dossiers_count = matching_dossiers_ids.count
@paginated_ids = Kaminari
.paginate_array(matching_dossiers_ids.to_a)
.page(page)
.per(ITEMS_PER_PAGE)
@projected_dossiers = DossierProjectionService.project(@paginated_ids, PROJECTIONS)
@followed_dossiers_id = current_instructeur&.followed_dossiers&.where(id: @paginated_ids)&.ids || []
end
private
def page
params[:page].presence || 1
end
def search_terms
params[:q]
end
end

View file

@ -63,7 +63,6 @@ class Dossier < ApplicationRecord
has_one :etablissement, dependent: :destroy has_one :etablissement, dependent: :destroy
has_one :individual, validate: false, dependent: :destroy has_one :individual, validate: false, dependent: :destroy
has_one :attestation, dependent: :destroy has_one :attestation, dependent: :destroy
has_one :france_connect_information, through: :user
# FIXME: some dossiers have more than one attestation # FIXME: some dossiers have more than one attestation
has_many :attestations, dependent: :destroy has_many :attestations, dependent: :destroy
@ -88,6 +87,7 @@ class Dossier < ApplicationRecord
belongs_to :groupe_instructeur, optional: true belongs_to :groupe_instructeur, optional: true
belongs_to :revision, class_name: 'ProcedureRevision', optional: false belongs_to :revision, class_name: 'ProcedureRevision', optional: false
belongs_to :user, optional: true belongs_to :user, optional: true
has_one :france_connect_information, through: :user
has_one :procedure, through: :revision has_one :procedure, through: :revision
has_many :types_de_champ, through: :revision has_many :types_de_champ, through: :revision
@ -340,7 +340,7 @@ class Dossier < ApplicationRecord
accepts_nested_attributes_for :individual accepts_nested_attributes_for :individual
delegate :siret, :siren, to: :etablissement, allow_nil: true delegate :siret, :siren, to: :etablissement, allow_nil: true
delegate :france_connect_information, to: :user delegate :france_connect_information, to: :user, allow_nil: true
before_save :build_default_champs, if: Proc.new { revision_id_was.nil? } before_save :build_default_champs, if: Proc.new { revision_id_was.nil? }
before_save :update_search_terms before_save :update_search_terms

View file

@ -33,7 +33,7 @@ class DossierProjectionService
types_de_champ: { stable_id: fields.map { |f| f[COLUMN] } }, types_de_champ: { stable_id: fields.map { |f| f[COLUMN] } },
dossier_id: dossiers_ids dossier_id: dossiers_ids
) )
.select(:dossier_id, :value, :type_de_champ_id, :stable_id) # we cannot pluck :value, as we need the champ.to_s method .select(:dossier_id, :value, :type_de_champ_id, :stable_id, :type) # we cannot pluck :value, as we need the champ.to_s method
.group_by(&:stable_id) # the champs are redispatched to their respective fields .group_by(&:stable_id) # the champs are redispatched to their respective fields
.map do |stable_id, champs| .map do |stable_id, champs|
field = fields.find { |f| f[COLUMN] == stable_id.to_s } field = fields.find { |f| f[COLUMN] == stable_id.to_s }
@ -74,6 +74,12 @@ class DossierProjectionService
.where(id: dossiers_ids) .where(id: dossiers_ids)
.pluck('dossiers.id, groupe_instructeurs.label') .pluck('dossiers.id, groupe_instructeurs.label')
.to_h .to_h
when 'procedure'
Dossier
.joins(:procedure)
.where(id: dossiers_ids)
.pluck(:id, *fields.map { |f| f[COLUMN].to_sym })
.each { |id, *columns| fields.zip(columns).each { |field, value| field[:id_value_h][id] = value } }
when 'followers_instructeurs' when 'followers_instructeurs'
fields[0][:id_value_h] = Follow fields[0][:id_value_h] = Follow
.active .active

View file

@ -1,7 +1,7 @@
class DossierSearchService class DossierSearchService
def self.matching_dossiers_for_instructeur(search_terms, instructeur) def self.matching_dossiers(ids, search_terms, with_annotations = false)
dossier_by_exact_id_for_instructeur(search_terms, instructeur) dossier_by_exact_id(ids, search_terms)
.presence || dossier_by_full_text_for_instructeur(search_terms, instructeur) .presence || dossier_by_full_text(ids, search_terms, with_annotations)
end end
def self.matching_dossiers_for_user(search_terms, user) def self.matching_dossiers_for_user(search_terms, user)
@ -11,24 +11,24 @@ class DossierSearchService
private private
def self.dossier_by_exact_id_for_instructeur(search_terms, instructeur) def self.dossier_by_exact_id(ids, search_terms)
id = search_terms.to_i id = search_terms.to_i
if id != 0 && id_compatible?(id) # Sometimes instructeur is searching dossiers with a big number (ex: SIRET), ActiveRecord can't deal with them and throws ActiveModel::RangeError. id_compatible? prevents this. if id != 0 && id_compatible?(id) # Sometimes instructeur 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, instructeur) ids.filter { |dossier_id| dossier_id == id }.uniq
else else
Dossier.none Dossier.none
end end
end end
def self.dossiers_by_id(id, instructeur) def self.dossier_by_full_text(ids, search_terms, with_annotations)
instructeur.dossiers.where(id: id).uniq ts_vector = "to_tsvector('french', #{with_annotations ? 'dossiers.search_terms || dossiers.private_search_terms' : 'dossiers.search_terms'})"
end ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})"
def self.id_compatible?(number) Dossier.where(id: ids)
ActiveRecord::Type::Integer.new.serialize(number) .where("#{ts_vector} @@ #{ts_query}")
true .order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC"))
rescue ActiveModel::RangeError .pluck('id')
false .uniq
end end
def self.dossier_by_full_text_for_user(search_terms, dossiers) def self.dossier_by_full_text_for_user(search_terms, dossiers)
@ -49,15 +49,11 @@ class DossierSearchService
end end
end end
def self.dossier_by_full_text_for_instructeur(search_terms, instructeur) def self.id_compatible?(number)
ts_vector = "to_tsvector('french', search_terms || private_search_terms)" ActiveRecord::Type::Integer.new.serialize(number)
ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" true
rescue ActiveModel::RangeError
instructeur false
.dossiers
.state_not_brouillon
.where("#{ts_vector} @@ #{ts_query}")
.order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC"))
end end
def self.to_tsquery(search_terms) def self.to_tsquery(search_terms)

View file

@ -3,7 +3,7 @@
%h2 Identité du demandeur %h2 Identité du demandeur
= render partial: "shared/dossiers/user_infos", locals: { user: @dossier.user } = render partial: "shared/dossiers/user_infos", locals: { user_deleted: @dossier.user_deleted?, email: @dossier.user_email_for(:display) }
- if @dossier.etablissement.present? - if @dossier.etablissement.present?
= render partial: "shared/dossiers/identite_entreprise", locals: { etablissement: @dossier.etablissement, profile: 'instructeur' } = render partial: "shared/dossiers/identite_entreprise", locals: { etablissement: @dossier.etablissement, profile: 'instructeur' }

View file

@ -1,42 +0,0 @@
- content_for(:title, "Recherche : #{@search_terms}")
.container
.page-title
Résultat de la recherche :
= t('pluralize.dossier_trouve', count: @dossiers.count)
- if @dossiers.present?
%table.table.dossiers-table.hoverable
%thead
%tr
%th.notification-col
%th.number-col Nº dossier
%th Démarche
%th Demandeur
%th.status-col Statut
%th.action-col.follow-col
%tbody
- @dossiers.each do |dossier|
/ # FIXME: here we have a n+1, we fire a request
/ (due to dossier_linked_path) per result
%tr
%td.folder-col
= link_to(dossier_linked_path(current_instructeur, dossier), class: 'cell-link') do
%span.icon.folder
%td.number-col
= link_to(dossier_linked_path(current_instructeur, dossier), class: 'cell-link') do
= dossier.id
%td= link_to(dossier.procedure.libelle, dossier_linked_path(current_instructeur, dossier), class: 'cell-link')
%td= link_to(dossier.user_email_for(:display), dossier_linked_path(current_instructeur, dossier), class: 'cell-link')
%td.status-col
= link_to(dossier_linked_path(current_instructeur, dossier), class: 'cell-link') do
= status_badge(dossier.state)
%td.action-col.follow-col= render partial: "instructeurs/procedures/dossier_actions",
locals: { procedure_id: dossier.procedure.id,
dossier_id: dossier.id,
state: dossier.state,
archived: dossier.archived,
dossier_is_followed: @followed_dossiers_id.include?(dossier.id) }
- else
%h2 Aucun dossier correspondant à votre recherche n'a été trouvé

View file

@ -50,9 +50,15 @@
= active_link_to "Dossiers", dossiers_path, active: :inclusive, class: 'tab-link' = active_link_to "Dossiers", dossiers_path, active: :inclusive, class: 'tab-link'
%ul.header-right-content %ul.header-right-content
- if params[:controller] == 'recherche'
= render partial: 'layouts/search_dossiers_form', locals: { search_endpoint: recherche_index_path }
- if nav_bar_profile == :instructeur && instructeur_signed_in? - if nav_bar_profile == :instructeur && instructeur_signed_in?
%li %li
= render partial: 'layouts/search_dossiers_form', locals: { search_endpoint: instructeur_recherche_path } = render partial: 'layouts/search_dossiers_form', locals: { search_endpoint: recherche_index_path }
- if nav_bar_profile == :expert && expert_signed_in?
= render partial: 'layouts/search_dossiers_form', locals: { search_endpoint: recherche_index_path }
- if nav_bar_profile == :user && user_signed_in? && current_user.dossiers.count > 2 - if nav_bar_profile == :user && user_signed_in? && current_user.dossiers.count > 2
%li %li

View file

@ -0,0 +1,92 @@
- content_for(:title, "Recherche : #{@search_terms}")
- pagination = paginate @paginated_ids
.container
.page-title
Résultat de la recherche :
= t('pluralize.dossier_trouve', count: @dossiers_count)
= pagination
- if @projected_dossiers.present?
%table.table.dossiers-table.hoverable
%thead
%tr
%th.notification-col
%th.number-col Nº dossier
%th Démarche
%th Demandeur
%th.status-col Statut
%th.action-col.follow-col
%tbody
- @projected_dossiers.each do |p|
- procedure_libelle, user_email, procedure_id = p.columns
- instructeur_dossier = @instructeur_dossiers_ids.include?(p.dossier_id)
- expert_dossier = @dossier_avis_ids_h[p.dossier_id].present?
- instructeur_and_expert_dossier = instructeur_dossier && expert_dossier
- path = instructeur_dossier ? instructeur_dossier_path(procedure_id, p.dossier_id) : expert_avis_path(procedure_id, @dossier_avis_ids_h[p.dossier_id])
%tr
- if instructeur_and_expert_dossier
%td.folder-col.cell-link
%span.icon.folder
%td.number-col
.cell-link= p.dossier_id
%td
.cell-link= procedure_libelle
%td
.cell-link= user_email
%td.status-col
.cell-link= status_badge(p.state)
- else
%td.folder-col
%a.cell-link{ href: path }
%span.icon.folder
%td.number-col
%a.cell-link{ href: path }= p.dossier_id
%td
%a.cell-link{ href: path }= procedure_libelle
%td
%a.cell-link{ href: path }= user_email
%td.status-col
%a.cell-link{ href: path }= status_badge(p.state)
- if instructeur_dossier && expert_dossier
%td.action-col.follow-col
.dropdown
.button.dropdown-button
Actions
.dropdown-content.fade-in-down
%ul.dropdown-items
%li
= link_to(instructeur_dossier_path(procedure_id, p.dossier_id)) do
%span.icon.in-progress>
.dropdown-description
Voir le dossier
%li
= link_to(expert_avis_path(procedure_id, @dossier_avis_ids_h[p.dossier_id])) do
%span.icon.in-progress>
.dropdown-description
Donner mon avis
- elsif instructeur_dossier
%td.action-col.follow-col= render partial: "instructeurs/procedures/dossier_actions",
locals: { procedure_id: procedure_id,
dossier_id: p.dossier_id,
state: p.state,
archived: p.archived,
dossier_is_followed: @followed_dossiers_id.include?(p.dossier_id) }
- else
%td
= pagination
- else
%h2 Aucun dossier correspondant à votre recherche n'a été trouvé

View file

@ -7,7 +7,7 @@
.card .card
- if dossier.france_connect_information.present? - if dossier.france_connect_information.present?
= render partial: "shared/dossiers/france_connect_informations", locals: { user_information: dossier.france_connect_information } = render partial: "shared/dossiers/france_connect_informations", locals: { user_information: dossier.france_connect_information }
= render partial: "shared/dossiers/user_infos", locals: { user: dossier.user } = render partial: "shared/dossiers/user_infos", locals: { user_deleted: dossier.user_deleted?, email: dossier.user_email_for(:display) }
- if dossier.etablissement.present? - if dossier.etablissement.present?
= render partial: "shared/dossiers/identite_entreprise", locals: { etablissement: dossier.etablissement, profile: profile } = render partial: "shared/dossiers/identite_entreprise", locals: { etablissement: dossier.etablissement, profile: profile }

View file

@ -2,4 +2,4 @@
%tbody %tbody
%tr %tr
%th.libelle Email : %th.libelle Email :
%td= user.email %td= user_deleted ? "#{email} (lusager a supprimé son compte)" : email

View file

@ -147,6 +147,7 @@ Rails.application.routes.draw do
end end
resources :attachments, only: [:show, :destroy] resources :attachments, only: [:show, :destroy]
resources :recherche, only: [:index]
get "patron" => "root#patron" get "patron" => "root#patron"
get "suivi" => "root#suivi" get "suivi" => "root#suivi"
@ -294,7 +295,6 @@ Rails.application.routes.draw do
# #
scope module: 'experts', as: 'expert' do scope module: 'experts', as: 'expert' do
get 'avis', to: 'avis#index', as: 'all_avis' get 'avis', to: 'avis#index', as: 'all_avis'
# this redirections are ephemeral, to ensure that emails sent to experts before are still valid # this redirections are ephemeral, to ensure that emails sent to experts before are still valid
# TODO : they will be removed in September, 2020 # TODO : they will be removed in September, 2020
get 'avis/:id', to: redirect('/procedures/old/avis/%{id}') get 'avis/:id', to: redirect('/procedures/old/avis/%{id}')
@ -387,7 +387,6 @@ Rails.application.routes.draw do
resources :archives, only: [:index, :create, :show], controller: 'archives' resources :archives, only: [:index, :create, :show], controller: 'archives'
end end
end end
get "recherche" => "recherche#index"
end end
# #

View file

@ -1,61 +0,0 @@
describe Instructeurs::RechercheController, type: :controller do
let(:dossier) { create(:dossier, :en_construction) }
let(:dossier2) { create(:dossier, :en_construction, procedure: dossier.procedure) }
let(:instructeur) { create(:instructeur) }
before { instructeur.groupe_instructeurs << dossier2.procedure.defaut_groupe_instructeur }
describe 'GET #index' do
before { sign_in(instructeur.user) }
subject { get :index, params: { q: query } }
describe 'by id' do
context 'when instructeur own the dossier' do
let(:query) { dossier.id }
it { is_expected.to have_http_status(200) }
it 'returns the expected dossier' do
subject
expect(assigns(:dossiers).count).to eq(1)
expect(assigns(:dossiers).first.id).to eq(dossier.id)
end
end
context 'when instructeur do not own the dossier' do
let(:dossier3) { create(:dossier, :en_construction) }
let(:query) { dossier3.id }
it { is_expected.to have_http_status(200) }
it 'does not return the dossier' do
subject
expect(assigns(:dossiers).count).to eq(0)
end
end
context 'with an id out of range' do
let(:query) { 123456789876543234567 }
it { is_expected.to have_http_status(200) }
it 'does not return the dossier' do
subject
expect(assigns(:dossiers).count).to eq(0)
end
end
end
context 'with no query param it does not crash' do
subject { get :index, params: {} }
it { is_expected.to have_http_status(200) }
it 'returns 0 dossier' do
subject
expect(assigns(:dossiers).count).to eq(0)
end
end
end
end

View file

@ -0,0 +1,111 @@
describe RechercheController, type: :controller do
let(:dossier) { create(:dossier, :en_construction, :with_all_annotations) }
let(:dossier2) { create(:dossier, :en_construction, procedure: dossier.procedure) }
let(:instructeur) { create(:instructeur) }
let(:dossier_with_expert) { avis.dossier }
let(:avis) { create(:avis, dossier: create(:dossier, :en_construction, :with_all_annotations)) }
let(:user) { instructeur.user }
before do
instructeur.assign_to_procedure(dossier.procedure)
end
describe 'GET #index' do
before { sign_in(user) }
subject { get :index, params: { q: query } }
describe 'by id' do
context 'when instructeur own the dossier' do
let(:query) { dossier.id }
before { subject }
it { is_expected.to have_http_status(200) }
it 'returns the expected dossier' do
expect(assigns(:projected_dossiers).count).to eq(1)
expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier.id)
end
end
context 'when expert own the dossier' do
let(:user) { avis.experts_procedure.expert.user }
let(:query) { dossier_with_expert.id }
before { subject }
it { is_expected.to have_http_status(200) }
it 'returns the expected dossier' do
expect(assigns(:projected_dossiers).count).to eq(1)
expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier_with_expert.id)
end
end
context 'when instructeur do not own the dossier' do
let(:dossier3) { create(:dossier, :en_construction) }
let(:query) { dossier3.id }
it { is_expected.to have_http_status(200) }
it 'does not return the dossier' do
subject
expect(assigns(:projected_dossiers).count).to eq(0)
end
end
context 'with an id out of range' do
let(:query) { 123456789876543234567 }
it { is_expected.to have_http_status(200) }
it 'does not return the dossier' do
subject
expect(assigns(:projected_dossiers).count).to eq(0)
end
end
end
describe 'by private annotations' do
context 'when instructeur search by private annotations' do
let(:query) { dossier.private_search_terms }
before { subject }
it { is_expected.to have_http_status(200) }
it 'returns the expected dossier' do
expect(assigns(:projected_dossiers).count).to eq(1)
expect(assigns(:projected_dossiers).first.dossier_id).to eq(dossier.id)
end
end
context 'when expert search by private annotations' do
let(:user) { avis.experts_procedure.expert.user }
let(:query) { dossier_with_expert.private_search_terms }
before { subject }
it { is_expected.to have_http_status(200) }
it 'returns 0 dossiers' do
expect(assigns(:projected_dossiers).count).to eq(0)
end
end
end
context 'with no query param it does not crash' do
subject { get :index, params: {} }
it { is_expected.to have_http_status(200) }
it 'returns 0 dossier' do
subject
expect(assigns(:projected_dossiers).count).to eq(0)
end
end
end
end

View file

@ -348,6 +348,7 @@ describe User, type: :model do
expect(dossier_termine.user).to be_nil expect(dossier_termine.user).to be_nil
expect(dossier_termine.user_email_for(:display)).to eq(user.email) expect(dossier_termine.user_email_for(:display)).to eq(user.email)
expect(dossier_termine.valid?).to be_truthy expect(dossier_termine.valid?).to be_truthy
expect(dossier_termine.france_connect_information).to be_nil
expect { dossier_termine.user_email_for(:notification) }.to raise_error(RuntimeError) expect { dossier_termine.user_email_for(:notification) }.to raise_error(RuntimeError)
expect(User.find_by(id: user.id)).to be_nil expect(User.find_by(id: user.id)).to be_nil

View file

@ -162,6 +162,17 @@ describe DossierProjectionService do
it { is_expected.to eq('quinoa') } it { is_expected.to eq('quinoa') }
end end
context 'for type_de_champ table and value to.s' do
let(:table) { 'type_de_champ' }
let(:procedure) { create(:procedure, :with_yes_no) }
let(:dossier) { create(:dossier, procedure: procedure) }
let(:column) { dossier.procedure.types_de_champ.first.stable_id.to_s }
before { dossier.champs.first.update(value: 'true') }
it { is_expected.to eq('Oui') }
end
end end
end end
end end

View file

@ -1,9 +1,9 @@
describe DossierSearchService do describe DossierSearchService do
describe '#matching_dossiers_for_instructeur' do describe '#matching_dossiers' do
subject { liste_dossiers } subject { liste_dossiers }
let(:liste_dossiers) do let(:liste_dossiers) do
described_class.matching_dossiers_for_instructeur(terms, instructeur_1) described_class.matching_dossiers(instructeur_1.dossiers, terms)
end end
let(:administrateur_1) { create(:administrateur) } let(:administrateur_1) { create(:administrateur) }