From 5f419954b408d3bb43a098af20ed74aa9d37afe9 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 12 May 2021 19:04:31 +0200 Subject: [PATCH 01/37] Fix dossier deleted user display --- app/models/dossier.rb | 4 ++-- app/views/instructeurs/dossiers/print.html.haml | 2 +- app/views/shared/dossiers/_demande.html.haml | 2 +- app/views/shared/dossiers/_user_infos.html.haml | 2 +- spec/models/user_spec.rb | 1 + 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index acb3b8db5..cd288cae0 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -63,7 +63,6 @@ class Dossier < ApplicationRecord has_one :etablissement, dependent: :destroy has_one :individual, validate: false, dependent: :destroy has_one :attestation, dependent: :destroy - has_one :france_connect_information, through: :user # FIXME: some dossiers have more than one attestation has_many :attestations, dependent: :destroy @@ -88,6 +87,7 @@ class Dossier < ApplicationRecord belongs_to :groupe_instructeur, optional: true belongs_to :revision, class_name: 'ProcedureRevision', optional: false belongs_to :user, optional: true + has_one :france_connect_information, through: :user has_one :procedure, through: :revision has_many :types_de_champ, through: :revision @@ -340,7 +340,7 @@ class Dossier < ApplicationRecord accepts_nested_attributes_for :individual 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 :update_search_terms diff --git a/app/views/instructeurs/dossiers/print.html.haml b/app/views/instructeurs/dossiers/print.html.haml index 25bfa2aec..656085d58 100644 --- a/app/views/instructeurs/dossiers/print.html.haml +++ b/app/views/instructeurs/dossiers/print.html.haml @@ -3,7 +3,7 @@ %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? = render partial: "shared/dossiers/identite_entreprise", locals: { etablissement: @dossier.etablissement, profile: 'instructeur' } diff --git a/app/views/shared/dossiers/_demande.html.haml b/app/views/shared/dossiers/_demande.html.haml index a4c45ec87..585a49319 100644 --- a/app/views/shared/dossiers/_demande.html.haml +++ b/app/views/shared/dossiers/_demande.html.haml @@ -7,7 +7,7 @@ .card - if dossier.france_connect_information.present? = 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? = render partial: "shared/dossiers/identite_entreprise", locals: { etablissement: dossier.etablissement, profile: profile } diff --git a/app/views/shared/dossiers/_user_infos.html.haml b/app/views/shared/dossiers/_user_infos.html.haml index bf003b719..5aed997d0 100644 --- a/app/views/shared/dossiers/_user_infos.html.haml +++ b/app/views/shared/dossiers/_user_infos.html.haml @@ -2,4 +2,4 @@ %tbody %tr %th.libelle Email : - %td= user.email + %td= user_deleted ? "#{email} (l‘usager a supprimé son compte)" : email diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 002f0b2d2..15401ff14 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -348,6 +348,7 @@ describe User, type: :model do expect(dossier_termine.user).to be_nil expect(dossier_termine.user_email_for(:display)).to eq(user.email) 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(User.find_by(id: user.id)).to be_nil From b3caa2e5f4e5c9ed04c453e13c59be6b7f525257 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Thu, 29 Apr 2021 09:32:25 +0200 Subject: [PATCH 02/37] add route --- config/routes.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index acdac88ea..c6bb99c2a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -147,6 +147,7 @@ Rails.application.routes.draw do end resources :attachments, only: [:show, :destroy] + resources :recherche, only: [:index] get "patron" => "root#patron" get "suivi" => "root#suivi" @@ -294,7 +295,6 @@ Rails.application.routes.draw do # scope module: 'experts', as: 'expert' do get 'avis', to: 'avis#index', as: 'all_avis' - # this redirections are ephemeral, to ensure that emails sent to experts before are still valid # TODO : they will be removed in September, 2020 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' end end - get "recherche" => "recherche#index" end # From 7d9cf63056308c935d8b5a9447c42039a367bb29 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Thu, 29 Apr 2021 09:32:47 +0200 Subject: [PATCH 03/37] add and modify controllers --- .../instructeurs/recherche_controller.rb | 12 ----- app/controllers/recherche_controller.rb | 53 +++++++++++++++++++ 2 files changed, 53 insertions(+), 12 deletions(-) delete mode 100644 app/controllers/instructeurs/recherche_controller.rb create mode 100644 app/controllers/recherche_controller.rb diff --git a/app/controllers/instructeurs/recherche_controller.rb b/app/controllers/instructeurs/recherche_controller.rb deleted file mode 100644 index 298abb3ea..000000000 --- a/app/controllers/instructeurs/recherche_controller.rb +++ /dev/null @@ -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 diff --git a/app/controllers/recherche_controller.rb b/app/controllers/recherche_controller.rb new file mode 100644 index 000000000..681ddb166 --- /dev/null +++ b/app/controllers/recherche_controller.rb @@ -0,0 +1,53 @@ +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 + matching_dossiers_ids = [] + instructeur_dossiers_ids = [] + expert_dossiers_ids = [] + + if instructeur_signed_in? + instructeur_dossiers_ids.concat(current_instructeur.dossiers.ids) + @followed_dossiers_id = current_instructeur.followed_dossiers.where(id: instructeur_dossiers_ids).ids + + if instructeur_dossiers_ids.present? + matching_dossiers_ids.concat(DossierSearchService.matching_dossiers(instructeur_dossiers_ids, @search_terms, true)) + end + end + + if expert_signed_in? + @dossier_avis_ids_h = current_expert.avis.pluck(:dossier_id, :id).to_h + expert_dossiers_ids.concat(@dossier_avis_ids_h.keys) + + if expert_dossiers_ids.present? + matching_dossiers_ids.concat(DossierSearchService.matching_dossiers(expert_dossiers_ids, @search_terms)) + end + end + + @paginated_ids = Kaminari + .paginate_array(matching_dossiers_ids.uniq) + .page(page) + .per(ITEMS_PER_PAGE) + + @dossiers_count = matching_dossiers_ids.count + + @projected_dossiers = DossierProjectionService.project(@paginated_ids, PROJECTIONS) + end + + private + + def page + params[:page].presence || 1 + end + + def search_terms + params[:q] + end +end From fdde55f6753abcbb0f4c62b32153d0e599860ca1 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Thu, 29 Apr 2021 09:33:04 +0200 Subject: [PATCH 04/37] modify service --- app/services/dossier_projection_service.rb | 6 ++++++ app/services/dossier_search_service.rb | 25 ++++++++++++++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index eec073616..a8672bba0 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -74,6 +74,12 @@ class DossierProjectionService .where(id: dossiers_ids) .pluck('dossiers.id, groupe_instructeurs.label') .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' fields[0][:id_value_h] = Follow .active diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index 583ab512d..dabf56e7a 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -1,7 +1,7 @@ class DossierSearchService - def self.matching_dossiers_for_instructeur(search_terms, instructeur) - dossier_by_exact_id_for_instructeur(search_terms, instructeur) - .presence || dossier_by_full_text_for_instructeur(search_terms, instructeur) + def self.matching_dossiers(ids, search_terms, with_annotations = false) + dossier_by_exact_id(ids, search_terms) + .presence || dossier_by_full_text(ids, search_terms, with_annotations) end def self.matching_dossiers_for_user(search_terms, user) @@ -11,17 +11,24 @@ class DossierSearchService 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 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 Dossier.none end end - def self.dossiers_by_id(id, instructeur) - instructeur.dossiers.where(id: id).uniq + def self.dossier_by_full_text(ids, search_terms, with_annotations) + ts_vector = "to_tsvector('french', #{with_annotations ? 'dossiers.search_terms || dossiers.private_search_terms' : 'dossiers.search_terms'})" + ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" + + Dossier.where(id: ids) + .where("#{ts_vector} @@ #{ts_query}") + .order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC")) + .pluck('id') + .uniq end def self.id_compatible?(number) @@ -49,11 +56,11 @@ class DossierSearchService end end - def self.dossier_by_full_text_for_instructeur(search_terms, instructeur) + def self.dossier_by_full_text_for_current_user(search_terms, user) ts_vector = "to_tsvector('french', search_terms || private_search_terms)" ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" - instructeur + user .dossiers .state_not_brouillon .where("#{ts_vector} @@ #{ts_query}") From 6a1ed2e02d22e68111c063de859ae0752f462c27 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Thu, 29 Apr 2021 09:33:23 +0200 Subject: [PATCH 05/37] layout --- .../procedures/_dossier_actions.html.haml | 8 +- .../instructeurs/recherche/index.html.haml | 42 ------- app/views/layouts/_header.haml | 8 +- app/views/recherche/index.html.haml | 105 ++++++++++++++++++ 4 files changed, 116 insertions(+), 47 deletions(-) delete mode 100644 app/views/instructeurs/recherche/index.html.haml create mode 100644 app/views/recherche/index.html.haml diff --git a/app/views/instructeurs/procedures/_dossier_actions.html.haml b/app/views/instructeurs/procedures/_dossier_actions.html.haml index 14983ffa8..6aba070b5 100644 --- a/app/views/instructeurs/procedures/_dossier_actions.html.haml +++ b/app/views/instructeurs/procedures/_dossier_actions.html.haml @@ -1,19 +1,19 @@ - if Dossier::EN_CONSTRUCTION_OU_INSTRUCTION.include?(state) - if dossier_is_followed - = link_to unfollow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do + = link_to unfollow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do %span.icon.unfollow> Ne plus suivre - else - = link_to follow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do + = link_to follow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do %span.icon.follow> Suivre le dossier - elsif Dossier::TERMINE.include?(state) - if archived - = link_to unarchive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do + = link_to unarchive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do %span.icon.unarchive> Désarchiver le dossier - else - = link_to archive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do + = link_to archive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do %span.icon.archive> Archiver le dossier diff --git a/app/views/instructeurs/recherche/index.html.haml b/app/views/instructeurs/recherche/index.html.haml deleted file mode 100644 index b08a849b5..000000000 --- a/app/views/instructeurs/recherche/index.html.haml +++ /dev/null @@ -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é diff --git a/app/views/layouts/_header.haml b/app/views/layouts/_header.haml index 88e2c7266..7d3c684e7 100644 --- a/app/views/layouts/_header.haml +++ b/app/views/layouts/_header.haml @@ -50,9 +50,15 @@ = active_link_to "Dossiers", dossiers_path, active: :inclusive, class: 'tab-link' %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? %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 %li diff --git a/app/views/recherche/index.html.haml b/app/views/recherche/index.html.haml new file mode 100644 index 000000000..f83bbafe5 --- /dev/null +++ b/app/views/recherche/index.html.haml @@ -0,0 +1,105 @@ +- content_for(:title, "Recherche : #{@search_terms}") +- pagination = paginate @paginated_ids +- procedure_libelle_index = 0 +- user_email_index = 1 + +.container + .page-title + Résultat de la recherche : + = t('pluralize.dossier_trouve', count: @dossiers_count) + + - 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_id = p.columns.last + - if current_expert.present? + - avis_id = @dossier_avis_ids_h[p.dossier_id] + + - if (current_expert.present? && current_instructeur.blank?) || (current_instructeur.present? && current_expert.blank?) + + - path = current_instructeur.present? ? instructeur_dossier_path(procedure_id, p.dossier_id) : expert_avis_path(procedure_id, avis_id) + + %tr + %td.folder-col + = link_to(path, class: 'cell-link') do + %span.icon.folder + %td.number-col + = link_to(path, class: 'cell-link') do + = p.dossier_id + + - p.columns.values_at(procedure_libelle_index, user_email_index).each do |column| + %td + %a.cell-link{ href: path }= column + + %td.status-col + = link_to(path, class: 'cell-link') do + = status_badge(p.state) + + - if current_instructeur.present? + %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, + button_class: false, + dossier_is_followed: @followed_dossiers_id.include?(p.dossier_id) } + + - else + %tr + %td.folder-col + %span.icon.folder + %td.number-col + .cell-link + = p.dossier_id + + - p.columns.values_at(procedure_libelle_index, user_email_index).each do |column| + %td + .cell-link + = column + + %td.status-col + .cell-link + = status_badge(p.state) + %td.action-col.follow-col + .dropdown + .button.dropdown-button + Actions + .dropdown-content.fade-in-down + %ul.dropdown-items.pl-0 + - if current_instructeur.present? + %li + .dropdown-description + %h4 + = render partial: "instructeurs/procedures/dossier_actions", + locals: { procedure_id: procedure_id, + dossier_id: p.dossier_id, + state: p.state, + archived: p.archived, + button_class: false, + dossier_is_followed: @followed_dossiers_id.include?(p.dossier_id) } + + %li + = link_to instructeur_dossier_path(procedure_id, p.dossier_id) do + %span.icon.in-progress + .dropdown-description + %h4 Instruire le dossier + - if avis_id.present? + %li + = link_to(expert_avis_path(procedure_id, avis_id)) do + %span.icon.in-progress + .dropdown-description + %h4 Donner mon avis + = pagination + + + - else + %h2 Aucun dossier correspondant à votre recherche n'a été trouvé From 468e9e849a2cfc9e7069b1dbfb85f2f1c5011fb5 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Thu, 29 Apr 2021 09:33:32 +0200 Subject: [PATCH 06/37] tests --- .../recherche_controller_spec.rb | 15 +++++++++++---- spec/services/dossier_search_service_spec.rb | 4 ++-- 2 files changed, 13 insertions(+), 6 deletions(-) rename spec/controllers/{instructeurs => }/recherche_controller_spec.rb (85%) diff --git a/spec/controllers/instructeurs/recherche_controller_spec.rb b/spec/controllers/recherche_controller_spec.rb similarity index 85% rename from spec/controllers/instructeurs/recherche_controller_spec.rb rename to spec/controllers/recherche_controller_spec.rb index 8698c6102..73ae9e0c1 100644 --- a/spec/controllers/instructeurs/recherche_controller_spec.rb +++ b/spec/controllers/recherche_controller_spec.rb @@ -1,23 +1,30 @@ -describe Instructeurs::RechercheController, type: :controller do +describe 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 } + before do + #instructeur.groupe_instructeurs << dossier2.procedure.defaut_groupe_instructeur + instructeur.groupe_instructeurs << dossier.procedure.defaut_groupe_instructeur + end 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 + + before do + subject + end + 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 diff --git a/spec/services/dossier_search_service_spec.rb b/spec/services/dossier_search_service_spec.rb index c004f1d5a..cf8b02456 100644 --- a/spec/services/dossier_search_service_spec.rb +++ b/spec/services/dossier_search_service_spec.rb @@ -1,9 +1,9 @@ describe DossierSearchService do - describe '#matching_dossiers_for_instructeur' do + describe '#matching_dossiers_for_current_user' do subject { liste_dossiers } let(:liste_dossiers) do - described_class.matching_dossiers_for_instructeur(terms, instructeur_1) + described_class.matching_dossiers_for_current_user(terms, instructeur_1.user) end let(:administrateur_1) { create(:administrateur) } From e043645a885a1ae9d548566893aa76fb7316f4e8 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 01:14:57 +0200 Subject: [PATCH 07/37] cleanup tests --- app/services/dossier_search_service.rb | 21 ++---- spec/controllers/recherche_controller_spec.rb | 73 +++++++++++++++---- spec/services/dossier_search_service_spec.rb | 4 +- 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index dabf56e7a..dc5eab1a1 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -31,13 +31,6 @@ class DossierSearchService .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_user(search_terms, dossiers) ts_vector = "to_tsvector('french', search_terms)" ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" @@ -56,15 +49,11 @@ class DossierSearchService end end - def self.dossier_by_full_text_for_current_user(search_terms, user) - ts_vector = "to_tsvector('french', search_terms || private_search_terms)" - ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" - - user - .dossiers - .state_not_brouillon - .where("#{ts_vector} @@ #{ts_query}") - .order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC")) + def self.id_compatible?(number) + ActiveRecord::Type::Integer.new.serialize(number) + true + rescue ActiveModel::RangeError + false end def self.to_tsquery(search_terms) diff --git a/spec/controllers/recherche_controller_spec.rb b/spec/controllers/recherche_controller_spec.rb index 73ae9e0c1..2e643bd84 100644 --- a/spec/controllers/recherche_controller_spec.rb +++ b/spec/controllers/recherche_controller_spec.rb @@ -1,32 +1,47 @@ describe RechercheController, type: :controller do - let(:dossier) { create(:dossier, :en_construction) } + 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.groupe_instructeurs << dossier2.procedure.defaut_groupe_instructeur - instructeur.groupe_instructeurs << dossier.procedure.defaut_groupe_instructeur + instructeur.assign_to_procedure(dossier.procedure) end describe 'GET #index' do - before { sign_in(instructeur.user) } - + before { sign_in(user) } + subject { get :index, params: { q: query } } describe 'by id' do context 'when instructeur own the dossier' do - - before do - subject - end - let(:query) { dossier.id } + before { subject } + it { is_expected.to have_http_status(200) } it 'returns the expected dossier' do - expect(assigns(:dossiers).count).to eq(1) - expect(assigns(:dossiers).first.id).to eq(dossier.id) + 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 @@ -38,7 +53,7 @@ describe RechercheController, type: :controller do it 'does not return the dossier' do subject - expect(assigns(:dossiers).count).to eq(0) + expect(assigns(:projected_dossiers).count).to eq(0) end end @@ -49,7 +64,35 @@ describe RechercheController, type: :controller do it 'does not return the dossier' do subject - expect(assigns(:dossiers).count).to eq(0) + 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 @@ -61,7 +104,7 @@ describe RechercheController, type: :controller do it 'returns 0 dossier' do subject - expect(assigns(:dossiers).count).to eq(0) + expect(assigns(:projected_dossiers).count).to eq(0) end end end diff --git a/spec/services/dossier_search_service_spec.rb b/spec/services/dossier_search_service_spec.rb index cf8b02456..6528847fe 100644 --- a/spec/services/dossier_search_service_spec.rb +++ b/spec/services/dossier_search_service_spec.rb @@ -1,9 +1,9 @@ describe DossierSearchService do - describe '#matching_dossiers_for_current_user' do + describe '#matching_dossiers' do subject { liste_dossiers } let(:liste_dossiers) do - described_class.matching_dossiers_for_current_user(terms, instructeur_1.user) + described_class.matching_dossiers(instructeur_1.dossiers, terms) end let(:administrateur_1) { create(:administrateur) } From 02e2128fb7244b4c1e9d374ab4e03812c31f202e Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 17 May 2021 15:42:21 +0200 Subject: [PATCH 08/37] proposition de simplification --- app/controllers/recherche_controller.rb | 24 +---- app/views/recherche/index.html.haml | 117 ++++++++++-------------- 2 files changed, 53 insertions(+), 88 deletions(-) diff --git a/app/controllers/recherche_controller.rb b/app/controllers/recherche_controller.rb index 681ddb166..a5eadbf59 100644 --- a/app/controllers/recherche_controller.rb +++ b/app/controllers/recherche_controller.rb @@ -9,27 +9,13 @@ class RechercheController < ApplicationController def index @search_terms = search_terms - matching_dossiers_ids = [] - instructeur_dossiers_ids = [] - expert_dossiers_ids = [] - if instructeur_signed_in? - instructeur_dossiers_ids.concat(current_instructeur.dossiers.ids) - @followed_dossiers_id = current_instructeur.followed_dossiers.where(id: instructeur_dossiers_ids).ids + @instructeur_dossiers_ids = current_instructeur&.dossiers&.ids || [] + matching_dossiers_ids = DossierSearchService.matching_dossiers(@instructeur_dossiers_ids, @search_terms, true) - if instructeur_dossiers_ids.present? - matching_dossiers_ids.concat(DossierSearchService.matching_dossiers(instructeur_dossiers_ids, @search_terms, true)) - end - end - - if expert_signed_in? - @dossier_avis_ids_h = current_expert.avis.pluck(:dossier_id, :id).to_h - expert_dossiers_ids.concat(@dossier_avis_ids_h.keys) - - if expert_dossiers_ids.present? - matching_dossiers_ids.concat(DossierSearchService.matching_dossiers(expert_dossiers_ids, @search_terms)) - end - end + @dossier_avis_ids_h = current_expert&.avis&.pluck(:dossier_id, :id).to_h || {} + expert_dossiers_ids = @dossier_avis_ids_h.keys + matching_dossiers_ids.concat(DossierSearchService.matching_dossiers(expert_dossiers_ids, @search_terms)) @paginated_ids = Kaminari .paginate_array(matching_dossiers_ids.uniq) diff --git a/app/views/recherche/index.html.haml b/app/views/recherche/index.html.haml index f83bbafe5..d1806d853 100644 --- a/app/views/recherche/index.html.haml +++ b/app/views/recherche/index.html.haml @@ -1,13 +1,13 @@ - content_for(:title, "Recherche : #{@search_terms}") - pagination = paginate @paginated_ids -- procedure_libelle_index = 0 -- user_email_index = 1 .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 @@ -20,84 +20,63 @@ %th.action-col.follow-col %tbody - @projected_dossiers.each do |p| - - procedure_id = p.columns.last - - if current_expert.present? - - avis_id = @dossier_avis_ids_h[p.dossier_id] + - 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]) - - if (current_expert.present? && current_instructeur.blank?) || (current_instructeur.present? && current_expert.blank?) - - - path = current_instructeur.present? ? instructeur_dossier_path(procedure_id, p.dossier_id) : expert_avis_path(procedure_id, avis_id) - - %tr - %td.folder-col - = link_to(path, class: 'cell-link') do - %span.icon.folder - %td.number-col - = link_to(path, class: 'cell-link') do - = p.dossier_id - - - p.columns.values_at(procedure_libelle_index, user_email_index).each do |column| - %td - %a.cell-link{ href: path }= column - - %td.status-col - = link_to(path, class: 'cell-link') do - = status_badge(p.state) - - - if current_instructeur.present? - %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, - button_class: false, - dossier_is_followed: @followed_dossiers_id.include?(p.dossier_id) } - - - else - %tr - %td.folder-col + %tr + - if instructeur_and_expert_dossier + %td.folder-col.cell-link %span.icon.folder %td.number-col - .cell-link - = p.dossier_id + .cell-link= p.dossier_id + %td + .cell-link= procedure_libelle + %td + .cell-link= user_email + %td + .cell-link= status_badge(p.state) - - p.columns.values_at(procedure_libelle_index, user_email_index).each do |column| - %td - .cell-link - = column + - 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 - .cell-link - = status_badge(p.state) + %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.pl-0 - - if current_instructeur.present? - %li - .dropdown-description - %h4 - = render partial: "instructeurs/procedures/dossier_actions", - locals: { procedure_id: procedure_id, - dossier_id: p.dossier_id, - state: p.state, - archived: p.archived, - button_class: false, - dossier_is_followed: @followed_dossiers_id.include?(p.dossier_id) } + .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 + - else + %td - %li - = link_to instructeur_dossier_path(procedure_id, p.dossier_id) do - %span.icon.in-progress - .dropdown-description - %h4 Instruire le dossier - - if avis_id.present? - %li - = link_to(expert_avis_path(procedure_id, avis_id)) do - %span.icon.in-progress - .dropdown-description - %h4 Donner mon avis = pagination From 663d2879620c0630f01acf039ef8b9990b5de8b4 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 17 May 2021 15:42:21 +0200 Subject: [PATCH 09/37] proposition de simplification --- app/controllers/recherche_controller.rb | 6 ++++-- .../procedures/_dossier_actions.html.haml | 8 ++++---- app/views/recherche/index.html.haml | 12 ++++++++++-- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/app/controllers/recherche_controller.rb b/app/controllers/recherche_controller.rb index a5eadbf59..d6bb47dc2 100644 --- a/app/controllers/recherche_controller.rb +++ b/app/controllers/recherche_controller.rb @@ -17,14 +17,16 @@ class RechercheController < ApplicationController expert_dossiers_ids = @dossier_avis_ids_h.keys matching_dossiers_ids.concat(DossierSearchService.matching_dossiers(expert_dossiers_ids, @search_terms)) + @dossiers_count = matching_dossiers_ids.count + @paginated_ids = Kaminari .paginate_array(matching_dossiers_ids.uniq) .page(page) .per(ITEMS_PER_PAGE) - @dossiers_count = matching_dossiers_ids.count - @projected_dossiers = DossierProjectionService.project(@paginated_ids, PROJECTIONS) + + @followed_dossiers_id = current_instructeur&.followed_dossiers&.where(id: @paginated_ids)&.ids || [] end private diff --git a/app/views/instructeurs/procedures/_dossier_actions.html.haml b/app/views/instructeurs/procedures/_dossier_actions.html.haml index 6aba070b5..14983ffa8 100644 --- a/app/views/instructeurs/procedures/_dossier_actions.html.haml +++ b/app/views/instructeurs/procedures/_dossier_actions.html.haml @@ -1,19 +1,19 @@ - if Dossier::EN_CONSTRUCTION_OU_INSTRUCTION.include?(state) - if dossier_is_followed - = link_to unfollow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do + = link_to unfollow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do %span.icon.unfollow> Ne plus suivre - else - = link_to follow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do + = link_to follow_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do %span.icon.follow> Suivre le dossier - elsif Dossier::TERMINE.include?(state) - if archived - = link_to unarchive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do + = link_to unarchive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do %span.icon.unarchive> Désarchiver le dossier - else - = link_to archive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: defined?(button_class) && button_class.blank? ? '' : 'button' do + = link_to archive_instructeur_dossier_path(procedure_id, dossier_id), method: :patch, class: 'button' do %span.icon.archive> Archiver le dossier diff --git a/app/views/recherche/index.html.haml b/app/views/recherche/index.html.haml index d1806d853..ba1c84573 100644 --- a/app/views/recherche/index.html.haml +++ b/app/views/recherche/index.html.haml @@ -36,7 +36,7 @@ .cell-link= procedure_libelle %td .cell-link= user_email - %td + %td.status-col .cell-link= status_badge(p.state) - else @@ -74,11 +74,19 @@ %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é From 3c01488db2c813acf3440ff4e40da3e267ff2f8e Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 17 May 2021 17:10:50 +0200 Subject: [PATCH 10/37] use set to avoid duplicate in @dossier_count --- app/controllers/recherche_controller.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/recherche_controller.rb b/app/controllers/recherche_controller.rb index d6bb47dc2..6d02bcbea 100644 --- a/app/controllers/recherche_controller.rb +++ b/app/controllers/recherche_controller.rb @@ -11,16 +11,18 @@ class RechercheController < ApplicationController @search_terms = search_terms @instructeur_dossiers_ids = current_instructeur&.dossiers&.ids || [] - matching_dossiers_ids = DossierSearchService.matching_dossiers(@instructeur_dossiers_ids, @search_terms, true) + 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.concat(DossierSearchService.matching_dossiers(expert_dossiers_ids, @search_terms)) + 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.uniq) + .paginate_array(matching_dossiers_ids.to_a) .page(page) .per(ITEMS_PER_PAGE) From a46000dc1f617c44cc9f32a5f97bb4f716e762aa Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 18 May 2021 11:34:05 +0200 Subject: [PATCH 11/37] ensure to_s is used on specialized champ --- app/services/dossier_projection_service.rb | 2 +- spec/services/dossier_projection_service_spec.rb | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index a8672bba0..c46525147 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -33,7 +33,7 @@ class DossierProjectionService types_de_champ: { stable_id: fields.map { |f| f[COLUMN] } }, 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 .map do |stable_id, champs| field = fields.find { |f| f[COLUMN] == stable_id.to_s } diff --git a/spec/services/dossier_projection_service_spec.rb b/spec/services/dossier_projection_service_spec.rb index ec14ac9e1..0589d7baa 100644 --- a/spec/services/dossier_projection_service_spec.rb +++ b/spec/services/dossier_projection_service_spec.rb @@ -162,6 +162,17 @@ describe DossierProjectionService do it { is_expected.to eq('quinoa') } 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 From 17617fba4360a7dff2519d6b78cb33599f589251 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 19 May 2021 15:59:18 +0200 Subject: [PATCH 12/37] address champ needs the data column --- app/services/dossier_projection_service.rb | 2 +- spec/factories/procedure.rb | 6 ++++++ spec/services/dossier_projection_service_spec.rb | 11 +++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index c46525147..e25552aa0 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -33,7 +33,7 @@ class DossierProjectionService types_de_champ: { stable_id: fields.map { |f| f[COLUMN] } }, dossier_id: dossiers_ids ) - .select(:dossier_id, :value, :type_de_champ_id, :stable_id, :type) # we cannot pluck :value, as we need the champ.to_s method + .select(:dossier_id, :value, :type_de_champ_id, :stable_id, :type, :data) # we cannot pluck :value, as we need the champ.to_s method .group_by(&:stable_id) # the champs are redispatched to their respective fields .map do |stable_id, champs| field = fields.find { |f| f[COLUMN] == stable_id.to_s } diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index a966e222a..646f39c93 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -193,6 +193,12 @@ FactoryBot.define do end end + trait :with_address do + after(:build) do |procedure, _evaluator| + build(:type_de_champ_address, procedure: procedure) + end + end + trait :with_explication do after(:build) do |procedure, _evaluator| build(:type_de_champ_explication, procedure: procedure) diff --git a/spec/services/dossier_projection_service_spec.rb b/spec/services/dossier_projection_service_spec.rb index 0589d7baa..243f81084 100644 --- a/spec/services/dossier_projection_service_spec.rb +++ b/spec/services/dossier_projection_service_spec.rb @@ -173,6 +173,17 @@ describe DossierProjectionService do it { is_expected.to eq('Oui') } end + + context 'for type_de_champ table and value to.s which needs data field' do + let(:table) { 'type_de_champ' } + let(:procedure) { create(:procedure, :with_address) } + let(:dossier) { create(:dossier, procedure: procedure) } + let(:column) { dossier.procedure.types_de_champ.first.stable_id.to_s } + + before { dossier.champs.first.update(data: { 'label' => '18 a la bonne rue', 'departement' => 'd' }) } + + it { is_expected.to eq('18 a la bonne rue') } + end end end end From 20933579b010f928212327dde4f6b914622bb7ed Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Wed, 19 May 2021 15:41:12 +0200 Subject: [PATCH 13/37] do not display confidential avis to other experts --- app/models/dossier.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index cd288cae0..0ed94a2bc 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -535,14 +535,10 @@ class Dossier < ApplicationRecord end def avis_for_expert(expert) - if expert.dossiers.include?(self) - avis.order(created_at: :asc) - else - avis - .where(confidentiel: false) - .or(avis.where(claimant: expert)) - .order(created_at: :asc) - end + Avis + .where(dossier_id: id, confidentiel: false) + .or(Avis.where(id: expert.avis)) + .order(created_at: :asc) end def owner_name From aaf943569a19b24bda33b16e1ff5587451436981 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Thu, 20 May 2021 10:36:01 +0200 Subject: [PATCH 14/37] rename avis confidential test --- spec/models/dossier_spec.rb | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 7d87596c1..8ae36a3d7 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -337,20 +337,22 @@ describe Dossier do it { expect(dossier.avis_for_expert(expert_2)).not_to match([avis]) } end - context 'when there is a public advice asked from one expert to another' do - let!(:avis) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure_2, confidentiel: false) } + context 'when there is a public advice asked from one instructeur to an expert' do + let!(:avis_1) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure, confidentiel: false) } + let!(:avis_2) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure_2, confidentiel: false) } - it { expect(dossier.avis_for_instructeur(instructeur)).to match([avis]) } - it { expect(dossier.avis_for_expert(expert_1)).to match([avis]) } - it { expect(dossier.avis_for_expert(expert_2)).to match([avis]) } + it { expect(dossier.avis_for_instructeur(instructeur)).to match([avis_1, avis_2]) } + it { expect(dossier.avis_for_expert(expert_1)).to match([avis_1, avis_2]) } + it { expect(dossier.avis_for_expert(expert_2)).to match([avis_1, avis_2]) } end - context 'when there is a private advice asked from one expert to another' do - let!(:avis) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure_2, confidentiel: true) } + context 'when there is a private advice asked from one instructeur to an expert' do + let!(:avis_1) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure, confidentiel: true) } + let!(:avis_2) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure_2, confidentiel: true) } - it { expect(dossier.avis_for_instructeur(instructeur)).to match([avis]) } - it { expect(dossier.avis_for_expert(expert_1)).not_to match([avis]) } - it { expect(dossier.avis_for_expert(expert_2)).to match([avis]) } + it { expect(dossier.avis_for_instructeur(instructeur)).to match([avis_1, avis_2]) } + it { expect(dossier.avis_for_expert(expert_1)).to match([avis_1]) } + it { expect(dossier.avis_for_expert(expert_2)).to match([avis_2]) } end context 'when they are a lot of advice' do From 7045d8457f9d07600c6a82307572a530e795da11 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 18 May 2021 11:48:49 +0000 Subject: [PATCH 15/37] Bump nokogiri from 1.11.3 to 1.11.4 Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.11.3 to 1.11.4. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.11.3...v1.11.4) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3c58933a0..21d840a70 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -423,7 +423,7 @@ GEM ruby2_keywords (~> 0.0.1) netrc (0.11.0) nio4r (2.5.7) - nokogiri (1.11.3) + nokogiri (1.11.4) mini_portile2 (~> 2.5.0) racc (~> 1.4) open4 (1.3.4) From fb045f972997e7b469a8a8a75932f48f3fed01ee Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 18 May 2021 01:46:08 +0000 Subject: [PATCH 16/37] Bump puma from 5.2.1 to 5.3.1 Bumps [puma](https://github.com/puma/puma) from 5.2.1 to 5.3.1. - [Release notes](https://github.com/puma/puma/releases) - [Changelog](https://github.com/puma/puma/blob/master/History.md) - [Commits](https://github.com/puma/puma/compare/v5.2.1...v5.3.1) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 21d840a70..52442360c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -471,7 +471,7 @@ GEM byebug (~> 11.0) pry (~> 0.13.0) public_suffix (4.0.6) - puma (5.2.1) + puma (5.3.1) nio4r (~> 2.0) pundit (2.1.0) activesupport (>= 3.0.0) From bf9f29cc7141e4183553c3af50d80327e2e4a338 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 May 2021 11:20:28 +0200 Subject: [PATCH 17/37] Fix messagerie scroll to last message --- app/javascript/packs/application.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 940713e7b..d191c84c9 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -19,6 +19,7 @@ import '../new_design/procedure-context'; import '../new_design/procedure-form'; import '../new_design/spinner'; import '../new_design/support'; +import '../new_design/messagerie'; import '../new_design/dossiers/auto-save'; import '../new_design/dossiers/auto-upload'; From 63311eceb07651a9aa9b4d1737500d1b19048236 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Wed, 19 May 2021 14:52:54 +0200 Subject: [PATCH 18/37] avoid serializing long id list in case of huge procedure, search on it before --- app/controllers/recherche_controller.rb | 17 ++++++++--------- app/services/dossier_search_service.rb | 20 ++++++++++++-------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/app/controllers/recherche_controller.rb b/app/controllers/recherche_controller.rb index 6d02bcbea..f2d96ffb6 100644 --- a/app/controllers/recherche_controller.rb +++ b/app/controllers/recherche_controller.rb @@ -10,25 +10,24 @@ class RechercheController < ApplicationController 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 + @instructeur_dossiers_ids = DossierSearchService + .matching_dossiers(current_instructeur&.dossiers, @search_terms, with_annotation: true) - @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)) + expert_dossier_ids = DossierSearchService + .matching_dossiers(current_expert&.dossiers, @search_terms) - @dossiers_count = matching_dossiers_ids.count + matching_dossiers_ids = (@instructeur_dossiers_ids + expert_dossier_ids).uniq @paginated_ids = Kaminari - .paginate_array(matching_dossiers_ids.to_a) + .paginate_array(matching_dossiers_ids) .page(page) .per(ITEMS_PER_PAGE) @projected_dossiers = DossierProjectionService.project(@paginated_ids, PROJECTIONS) + @dossiers_count = matching_dossiers_ids.count @followed_dossiers_id = current_instructeur&.followed_dossiers&.where(id: @paginated_ids)&.ids || [] + @dossier_avis_ids_h = current_expert&.avis&.where(dossier_id: @paginated_ids)&.pluck(:dossier_id, :id).to_h || {} end private diff --git a/app/services/dossier_search_service.rb b/app/services/dossier_search_service.rb index dc5eab1a1..a3f95a3f5 100644 --- a/app/services/dossier_search_service.rb +++ b/app/services/dossier_search_service.rb @@ -1,7 +1,11 @@ class DossierSearchService - def self.matching_dossiers(ids, search_terms, with_annotations = false) - dossier_by_exact_id(ids, search_terms) - .presence || dossier_by_full_text(ids, search_terms, with_annotations) + def self.matching_dossiers(dossiers, search_terms, with_annotations = false) + if dossiers.nil? + [] + else + dossier_by_exact_id(dossiers, search_terms) + .presence || dossier_by_full_text(dossiers, search_terms, with_annotations) + end end def self.matching_dossiers_for_user(search_terms, user) @@ -11,20 +15,20 @@ class DossierSearchService private - def self.dossier_by_exact_id(ids, search_terms) + def self.dossier_by_exact_id(dossiers, search_terms) 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. - ids.filter { |dossier_id| dossier_id == id }.uniq + dossiers.where(id: id).ids else - Dossier.none + [] end end - def self.dossier_by_full_text(ids, search_terms, with_annotations) + def self.dossier_by_full_text(dossiers, search_terms, with_annotations) ts_vector = "to_tsvector('french', #{with_annotations ? 'dossiers.search_terms || dossiers.private_search_terms' : 'dossiers.search_terms'})" ts_query = "to_tsquery('french', #{Dossier.connection.quote(to_tsquery(search_terms))})" - Dossier.where(id: ids) + dossiers .where("#{ts_vector} @@ #{ts_query}") .order(Arel.sql("COALESCE(ts_rank(#{ts_vector}, #{ts_query}), 0) DESC")) .pluck('id') From b62088859774a0960d75717a0755e8aef90466d2 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 20 May 2021 16:15:59 +0200 Subject: [PATCH 19/37] show only dossier avis --- app/models/dossier.rb | 2 +- spec/models/dossier_spec.rb | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 0ed94a2bc..d457f2cf7 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -537,7 +537,7 @@ class Dossier < ApplicationRecord def avis_for_expert(expert) Avis .where(dossier_id: id, confidentiel: false) - .or(Avis.where(id: expert.avis)) + .or(Avis.where(id: expert.avis, dossier_id: id)) .order(created_at: :asc) end diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 8ae36a3d7..90f76235f 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -363,6 +363,12 @@ describe Dossier do it { expect(dossier.avis_for_instructeur(instructeur)).to match([avis_2, avis_1, avis_3]) } it { expect(dossier.avis_for_expert(expert_1)).to match([avis_2, avis_1, avis_3]) } end + + context 'when they are a advice published on another dossier' do + let!(:avis) { create(:avis, dossier: create(:dossier, procedure: procedure), claimant: instructeur, experts_procedure: experts_procedure, confidentiel: false, created_at: Time.zone.parse('9/01/2010')) } + + it { expect(dossier.avis_for_expert(expert_1)).to match([]) } + end end describe '#update_state_dates' do From 868ec214cebc8fdeac1deb437676a08e8957e3d7 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 May 2021 11:51:39 +0200 Subject: [PATCH 20/37] MultiSelect: enable adding multiple emails with , or space --- app/javascript/components/ComboMultipleDropdownList.jsx | 8 +++++++- package.json | 1 + yarn.lock | 5 +++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/javascript/components/ComboMultipleDropdownList.jsx b/app/javascript/components/ComboMultipleDropdownList.jsx index 1e25987ec..c88763070 100644 --- a/app/javascript/components/ComboMultipleDropdownList.jsx +++ b/app/javascript/components/ComboMultipleDropdownList.jsx @@ -19,6 +19,7 @@ import '@reach/combobox/styles.css'; import { matchSorter } from 'match-sorter'; import { fire } from '@utils'; import { XIcon } from '@heroicons/react/outline'; +import isHotkey from 'is-hotkey'; const Context = createContext(); @@ -83,7 +84,12 @@ function ComboMultipleDropdownList({ }; const onKeyDown = (event) => { - if (event.key === 'Enter') { + if ( + isHotkey('enter', event) || + isHotkey(' ', event) || + isHotkey(',', event) || + isHotkey(';', event) + ) { if ( term && [...extraOptions, ...options].map(([label]) => label).includes(term) diff --git a/package.json b/package.json index 102d3b750..c9aa65e27 100644 --- a/package.json +++ b/package.json @@ -19,6 +19,7 @@ "email-butler": "^1.0.13", "highcharts": "^8.1.1", "intersection-observer": "^0.12.0", + "is-hotkey": "^0.2.0", "mapbox-gl": "^1.3.0", "match-sorter": "^6.2.0", "prop-types": "^15.7.2", diff --git a/yarn.lock b/yarn.lock index 6b8bf6958..9259fad90 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6982,6 +6982,11 @@ is-glob@^4.0.0, is-glob@^4.0.1, is-glob@~4.0.1: dependencies: is-extglob "^2.1.1" +is-hotkey@^0.2.0: + version "0.2.0" + resolved "https://registry.yarnpkg.com/is-hotkey/-/is-hotkey-0.2.0.tgz#1835a68171a91e5c9460869d96336947c8340cef" + integrity sha512-UknnZK4RakDmTgz4PI1wIph5yxSs/mvChWs9ifnlXsKuXgWmOkY/hAE0H/k2MIqH0RlRye0i1oC07MCRSD28Mw== + is-installed-globally@^0.3.1: version "0.3.2" resolved "https://registry.yarnpkg.com/is-installed-globally/-/is-installed-globally-0.3.2.tgz#fd3efa79ee670d1187233182d5b0a1dd00313141" From e0d8d096f2f8aa1c1eaee99729d4953778b5acac Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 May 2021 13:05:36 +0200 Subject: [PATCH 21/37] MultiSelect: improuve setters --- .../components/ComboMultipleDropdownList.jsx | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/app/javascript/components/ComboMultipleDropdownList.jsx b/app/javascript/components/ComboMultipleDropdownList.jsx index c88763070..213e9fe07 100644 --- a/app/javascript/components/ComboMultipleDropdownList.jsx +++ b/app/javascript/components/ComboMultipleDropdownList.jsx @@ -100,12 +100,15 @@ function ComboMultipleDropdownList({ } }; - const saveSelection = (selections) => { - setSelections(selections); - if (hiddenField) { - hiddenField.setAttribute('value', JSON.stringify(selections)); - fire(hiddenField, 'autosave:trigger'); - } + const saveSelection = (fn) => { + setSelections((selections) => { + selections = fn(selections); + if (hiddenField) { + hiddenField.setAttribute('value', JSON.stringify(selections)); + fire(hiddenField, 'autosave:trigger'); + } + return selections; + }); }; const onSelect = (value) => { @@ -113,15 +116,15 @@ function ComboMultipleDropdownList({ ([val]) => val == value ); const selectedValue = maybeValue && maybeValue[1]; - if (value) { + if (selectedValue) { if ( acceptNewValues && extraOptions[0] && extraOptions[0][0] == selectedValue ) { - setNewValues([...newValues, selectedValue]); + setNewValues((newValues) => [...newValues, selectedValue]); } - saveSelection([...selections, selectedValue]); + saveSelection((selections) => [...selections, selectedValue]); } setTerm(''); }; @@ -129,8 +132,12 @@ function ComboMultipleDropdownList({ const onRemove = (label) => { const optionValue = optionValueByLabel(label); if (optionValue) { - saveSelection(selections.filter((value) => value != optionValue)); - setNewValues(newValues.filter((value) => value != optionValue)); + saveSelection((selections) => + selections.filter((value) => value != optionValue) + ); + setNewValues((newValues) => + newValues.filter((value) => value != optionValue) + ); } inputRef.current.focus(); }; From 3313ea5885d418629103e44b1b9667c4fbeea711 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 May 2021 13:11:49 +0200 Subject: [PATCH 22/37] MultiSelect: do not show empty results on free form inputs --- app/javascript/components/ComboMultipleDropdownList.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/components/ComboMultipleDropdownList.jsx b/app/javascript/components/ComboMultipleDropdownList.jsx index 213e9fe07..0e6d146ec 100644 --- a/app/javascript/components/ComboMultipleDropdownList.jsx +++ b/app/javascript/components/ComboMultipleDropdownList.jsx @@ -165,7 +165,7 @@ function ComboMultipleDropdownList({ autocomplete={false} /> - {results && ( + {results && (results.length > 0 || !acceptNewValues) && ( {results.length === 0 && (

From bf462380e6aca2c8d83f9db54883a574ac35641a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 May 2021 13:15:27 +0200 Subject: [PATCH 23/37] MultiSelect: select values on blur --- .../components/ComboMultipleDropdownList.jsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/javascript/components/ComboMultipleDropdownList.jsx b/app/javascript/components/ComboMultipleDropdownList.jsx index 0e6d146ec..1d6ebf01b 100644 --- a/app/javascript/components/ComboMultipleDropdownList.jsx +++ b/app/javascript/components/ComboMultipleDropdownList.jsx @@ -100,6 +100,17 @@ function ComboMultipleDropdownList({ } }; + const onBlur = (event) => { + if ( + acceptNewValues && + term && + [...extraOptions, ...options].map(([label]) => label).includes(term) + ) { + event.preventDefault(); + return onSelect(term); + } + }; + const saveSelection = (fn) => { setSelections((selections) => { selections = fn(selections); @@ -162,6 +173,7 @@ function ComboMultipleDropdownList({ value={term} onChange={handleChange} onKeyDown={onKeyDown} + onBlur={onBlur} autocomplete={false} /> From 01c558953b6dd352e7caf1a958074005db327fdf Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 18:47:58 +0200 Subject: [PATCH 24/37] Remove API GEO legacy adapter --- app/lib/api_carto/api.rb | 23 -------- app/lib/api_carto/cadastre_adapter.rb | 29 ---------- app/services/api_carto_service.rb | 25 -------- config/initializers/urls.rb | 1 - spec/lib/api_carto/api_spec.rb | 39 ------------- spec/lib/api_carto/cadastre_adapter_spec.rb | 63 --------------------- 6 files changed, 180 deletions(-) delete mode 100644 app/lib/api_carto/api.rb delete mode 100644 app/lib/api_carto/cadastre_adapter.rb delete mode 100644 app/services/api_carto_service.rb delete mode 100644 spec/lib/api_carto/api_spec.rb delete mode 100644 spec/lib/api_carto/cadastre_adapter_spec.rb diff --git a/app/lib/api_carto/api.rb b/app/lib/api_carto/api.rb deleted file mode 100644 index 7cf076dd6..000000000 --- a/app/lib/api_carto/api.rb +++ /dev/null @@ -1,23 +0,0 @@ -class APICarto::API - class ResourceNotFound < StandardError - end - - def self.search_cadastre(geojson) - url = [API_CARTO_URL, "cadastre", "geometrie"].join("/") - call(url, geojson) - end - - private - - def self.call(url, geojson) - response = Typhoeus.post(url, body: geojson.to_s, headers: { 'content-type' => 'application/json' }) - - if response.success? - response.body - else - message = response.code == 0 ? response.return_message : response.code.to_s - Rails.logger.error "[APICarto] Error on #{url}: #{message}" - raise ResourceNotFound - end - end -end diff --git a/app/lib/api_carto/cadastre_adapter.rb b/app/lib/api_carto/cadastre_adapter.rb deleted file mode 100644 index 65c939def..000000000 --- a/app/lib/api_carto/cadastre_adapter.rb +++ /dev/null @@ -1,29 +0,0 @@ -class APICarto::CadastreAdapter - def initialize(coordinates) - @coordinates = GeojsonService.to_json_polygon_for_cadastre(coordinates) - end - - def data_source - @data_source ||= JSON.parse(APICarto::API.search_cadastre(@coordinates), symbolize_names: true) - end - - def results - data_source[:features].map do |feature| - filter_properties(feature[:properties]).merge({ geometry: feature[:geometry] }) - end - end - - def filter_properties(properties) - properties.slice( - :surface_intersection, - :surface_parcelle, - :numero, - :feuille, - :section, - :code_dep, - :nom_com, - :code_com, - :code_arr - ) - end -end diff --git a/app/services/api_carto_service.rb b/app/services/api_carto_service.rb deleted file mode 100644 index 9c9379beb..000000000 --- a/app/services/api_carto_service.rb +++ /dev/null @@ -1,25 +0,0 @@ -class APICartoService - def self.generate_qp(coordinates) - coordinates.flat_map do |coordinate| - APICarto::QuartiersPrioritairesAdapter.new( - coordinate.map { |element| [element['lng'], element['lat']] } - ).results - end - end - - def self.generate_cadastre(coordinates) - coordinates.flat_map do |coordinate| - APICarto::CadastreAdapter.new( - coordinate.map { |element| [element['lng'], element['lat']] } - ).results - end - end - - def self.generate_rpg(coordinates) - coordinates.flat_map do |coordinate| - ApiGeo::RPGAdapter.new( - coordinate.map { |element| [element['lng'], element['lat']] } - ).results - end - end -end diff --git a/config/initializers/urls.rb b/config/initializers/urls.rb index 36c058067..c36c51aa2 100644 --- a/config/initializers/urls.rb +++ b/config/initializers/urls.rb @@ -1,6 +1,5 @@ # rubocop:disable DS/ApplicationName # API URLs -API_CARTO_URL = ENV.fetch("API_CARTO_URL", "https://sandbox.geo.api.gouv.fr/apicarto") API_ENTREPRISE_URL = ENV.fetch("API_ENTREPRISE_URL", "https://entreprise.api.gouv.fr/v2") API_EDUCATION_URL = ENV.fetch("API_EDUCATION_URL", "https://data.education.gouv.fr/api/records/1.0") HELPSCOUT_API_URL = ENV.fetch("HELPSCOUT_API_URL", "https://api.helpscout.net/v2") diff --git a/spec/lib/api_carto/api_spec.rb b/spec/lib/api_carto/api_spec.rb deleted file mode 100644 index 246cd5ce4..000000000 --- a/spec/lib/api_carto/api_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -describe APICarto::API do - describe '.search_cadastre' do - subject { described_class.search_cadastre(geojson) } - - before do - stub_request(:post, "https://sandbox.geo.api.gouv.fr/apicarto/cadastre/geometrie") - .with(:body => /.*/, - :headers => { 'Content-Type' => 'application/json' }) - .to_return(status: status, body: body) - end - context 'when geojson is empty' do - let(:geojson) { '' } - let(:status) { 404 } - let(:body) { '' } - - it 'raises APICarto::API::ResourceNotFound' do - expect { subject }.to raise_error(APICarto::API::ResourceNotFound) - end - end - - context 'when geojson exist' do - let(:geojson) { File.read('spec/fixtures/files/api_carto/request_cadastre.json') } - let(:status) { 200 } - let(:body) { 'toto' } - - it 'returns response body' do - expect(subject).to eq(body) - end - - context 'when geojson is at format JSON' do - let(:geojson) { JSON.parse(File.read('spec/fixtures/files/api_carto/request_cadastre.json')) } - - it 'returns response body' do - expect(subject).to eq(body) - end - end - end - end -end diff --git a/spec/lib/api_carto/cadastre_adapter_spec.rb b/spec/lib/api_carto/cadastre_adapter_spec.rb deleted file mode 100644 index fcbb64fa4..000000000 --- a/spec/lib/api_carto/cadastre_adapter_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -describe APICarto::CadastreAdapter do - subject { described_class.new(coordinates).results } - - before do - stub_request(:post, "https://sandbox.geo.api.gouv.fr/apicarto/cadastre/geometrie") - .with(:body => /.*/, - :headers => { 'Content-Type' => 'application/json' }) - .to_return(status: status, body: body) - end - - context 'coordinates are filled' do - let(:coordinates) { '[[2.252728, 43.27151][2.323223, 32.835332]]' } - let(:status) { 200 } - let(:body) { File.read('spec/fixtures/files/api_carto/response_cadastre.json') } - - it { expect(subject).to be_a_instance_of(Array) } - it { expect(subject.size).to eq(16) } - - describe 'Attribut filter' do - let(:adapter) { described_class.new(coordinates) } - subject { adapter.filter_properties(adapter.data_source[:features].first[:properties]) } - - it { expect(subject.size).to eq(9) } - it do - expect(subject.keys).to eq([ - :surface_intersection, - :surface_parcelle, - :numero, - :feuille, - :section, - :code_dep, - :nom_com, - :code_com, - :code_arr - ]) - end - end - - describe 'Attributes' do - subject { super().first } - - it { expect(subject[:surface_intersection]).to eq('0.0202') } - it { expect(subject[:surface_parcelle]).to eq(220.0664659755941) } - it { expect(subject[:numero]).to eq('0082') } - it { expect(subject[:feuille]).to eq(1) } - it { expect(subject[:section]).to eq('0J') } - it { expect(subject[:code_dep]).to eq('94') } - it { expect(subject[:nom_com]).to eq('Maisons-Alfort') } - it { expect(subject[:code_com]).to eq('046') } - it { expect(subject[:code_arr]).to eq('000') } - - it { expect(subject[:geometry]).to eq({ type: "MultiPolygon", coordinates: [[[[2.4362443, 48.8092078], [2.436384, 48.8092043], [2.4363802, 48.8091414]]]] }) } - end - end - - context 'coordinates are empty' do - let(:coordinates) { '' } - let(:status) { 404 } - let(:body) { '' } - - it { expect { subject }.to raise_error(APICarto::API::ResourceNotFound) } - end -end From e74dcb0056ab7a8d785dce2ae292a94896219407 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 18:48:24 +0200 Subject: [PATCH 25/37] Remove ign feature flag --- app/models/champs/carte_champ.rb | 5 +---- config/initializers/flipper.rb | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/app/models/champs/carte_champ.rb b/app/models/champs/carte_champ.rb index 0c97eed26..e56e98f67 100644 --- a/app/models/champs/carte_champ.rb +++ b/app/models/champs/carte_champ.rb @@ -62,10 +62,7 @@ class Champs::CarteChamp < Champ end def render_options - { - ign: Flipper.enabled?(:carte_ign, procedure), - layers: optional_layers - } + { layers: optional_layers } end def position diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index e6ded11d0..cec532155 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -27,7 +27,6 @@ end features = [ :administrateur_routage, :administrateur_web_hook, - :carte_ign, :dossier_pdf_vide, :expert_not_allowed_to_invite, :hide_instructeur_email, From 3b85ade440794d64f76dd79169e377091ce3808c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 18:50:06 +0200 Subject: [PATCH 26/37] Add compatibility cadsatre layer with old API GEO --- app/controllers/champs/carte_controller.rb | 89 ++++------ app/graphql/schema.graphql | 18 +- .../geo_areas/parcelle_cadastrale_type.rb | 18 +- .../geo_areas/selection_utilisateur_type.rb | 2 + app/helpers/champ_helper.rb | 2 +- app/models/geo_area.rb | 163 ++++++++++++++---- app/views/champs/carte/index.js.erb | 4 +- .../shared/champs/carte/_geo_areas.html.haml | 2 +- .../champs/carte_controller_spec.rb | 51 ++---- spec/factories/geo_area.rb | 10 +- spec/serializers/champ_serializer_spec.rb | 2 +- 11 files changed, 206 insertions(+), 155 deletions(-) diff --git a/app/controllers/champs/carte_controller.rb b/app/controllers/champs/carte_controller.rb index f0e51754a..11989e116 100644 --- a/app/controllers/champs/carte_controller.rb +++ b/app/controllers/champs/carte_controller.rb @@ -4,29 +4,26 @@ class Champs::CarteController < ApplicationController def index @selector = ".carte-#{params[:champ_id]}" @champ = policy_scope(Champ).find(params[:champ_id]) - @update_cadastres = params[:cadastres] - - if @champ.cadastres? && @update_cadastres - @champ.geo_areas.cadastres.destroy_all - @champ.geo_areas += GeoArea.from_feature_collection(cadastres_features_collection(@champ.to_feature_collection)) - @champ.save! - end - rescue APICarto::API::ResourceNotFound - flash.alert = 'Les données cartographiques sont temporairement indisponibles. Réessayez dans un instant.' - response.status = 503 + @focus = params[:focus].present? end def create champ = policy_scope(Champ).find(params[:champ_id]) - geo_area = champ.geo_areas.selections_utilisateur.new - save_feature!(geo_area, params_feature) + geo_area = if params_source == GeoArea.sources.fetch(:cadastre) + champ.geo_areas.find_by("properties->>'id' = :id", id: params_feature[:properties][:id]) + end + + if geo_area.nil? + geo_area = champ.geo_areas.build(source: params_source, properties: {}) + save_feature!(geo_area, params_feature) + end render json: { feature: geo_area.to_feature }, status: :created end def update champ = policy_scope(Champ).find(params[:champ_id]) - geo_area = champ.geo_areas.selections_utilisateur.find(params[:id]) + geo_area = champ.geo_areas.find(params[:id]) save_feature!(geo_area, params_feature) head :no_content @@ -34,66 +31,42 @@ class Champs::CarteController < ApplicationController def destroy champ = policy_scope(Champ).find(params[:champ_id]) - champ.geo_areas.selections_utilisateur.find(params[:id]).destroy! + champ.geo_areas.find(params[:id]).destroy! head :no_content end - def import - champ = policy_scope(Champ).find(params[:champ_id]) - params_features.each do |feature| - geo_area = champ.geo_areas.selections_utilisateur.new - save_feature!(geo_area, feature) - end - - render json: champ.to_feature_collection, status: :created - end - private - def params_feature - params[:feature] + def params_source + params[:source] end - def params_features - params[:features] + def params_feature + params.require(:feature).permit(properties: [ + :filename, + :description, + :arpente, + :commune, + :contenance, + :created, + :id, + :numero, + :prefixe, + :section, + :updated + ]).tap do |feature| + feature[:geometry] = params[:feature][:geometry] + end end def save_feature!(geo_area, feature) if feature[:geometry] geo_area.geometry = feature[:geometry] end - if feature[:properties] && feature[:properties][:description] - geo_area.description = feature[:properties][:description] + if feature[:properties] + geo_area.properties.merge!(feature[:properties]) end geo_area.save! end - - def cadastres_features_collection(feature_collection) - coordinates = feature_collection[:features].filter do |feature| - feature[:properties][:source] == GeoArea.sources.fetch(:selection_utilisateur) && feature[:geometry] && feature[:geometry]['type'] == 'Polygon' - end.map do |feature| - feature[:geometry]['coordinates'][0].map { |(lng, lat)| { 'lng' => lng, 'lat' => lat } } - end - - if coordinates.present? - cadastres = APICartoService.generate_cadastre(coordinates) - - { - type: 'FeatureCollection', - features: cadastres.map do |cadastre| - { - type: 'Feature', - geometry: cadastre.delete(:geometry), - properties: cadastre.merge(source: GeoArea.sources.fetch(:cadastre)) - } - end - } - else - { - type: 'FeatureCollection', - features: [] - } - end - end end diff --git a/app/graphql/schema.graphql b/app/graphql/schema.graphql index 6e0df05e6..d2c597811 100644 --- a/app/graphql/schema.graphql +++ b/app/graphql/schema.graphql @@ -1467,18 +1467,21 @@ type PageInfo { } type ParcelleCadastrale implements GeoArea { - codeArr: String! - codeCom: String! - codeDep: String! - feuille: Int! + codeArr: String! @deprecated(reason: "Utilisez le champ `prefixe` à la place.") + codeCom: String! @deprecated(reason: "Utilisez le champ `commune` à la place.") + codeDep: String! @deprecated(reason: "Utilisez le champ `commune` à la place.") + commune: String! + feuille: Int! @deprecated(reason: "L‘information n‘est plus disponible.") geometry: GeoJSON! id: ID! - nomCom: String! + nomCom: String! @deprecated(reason: "Utilisez le champ `commune` à la place.") numero: String! + prefixe: String! section: String! source: GeoAreaSource! - surfaceIntersection: Float! - surfaceParcelle: Float! + surface: String! + surfaceIntersection: Float! @deprecated(reason: "L‘information n‘est plus disponible.") + surfaceParcelle: Float! @deprecated(reason: "Utilisez le champ `surface` à la place.") } type PersonneMorale implements Demandeur { @@ -1587,6 +1590,7 @@ type Revision { } type SelectionUtilisateur implements GeoArea { + description: String! geometry: GeoJSON! id: ID! source: GeoAreaSource! diff --git a/app/graphql/types/geo_areas/parcelle_cadastrale_type.rb b/app/graphql/types/geo_areas/parcelle_cadastrale_type.rb index 22b40c5d9..fdbecf76d 100644 --- a/app/graphql/types/geo_areas/parcelle_cadastrale_type.rb +++ b/app/graphql/types/geo_areas/parcelle_cadastrale_type.rb @@ -2,14 +2,18 @@ module Types::GeoAreas class ParcelleCadastraleType < Types::BaseObject implements Types::GeoAreaType - field :surface_intersection, Float, null: false - field :surface_parcelle, Float, null: false field :numero, String, null: false - field :feuille, Int, null: false field :section, String, null: false - field :code_dep, String, null: false - field :nom_com, String, null: false - field :code_com, String, null: false - field :code_arr, String, null: false + field :surface, String, null: false + field :prefixe, String, null: false + field :commune, String, null: false + + field :code_dep, String, null: false, deprecation_reason: 'Utilisez le champ `commune` à la place.' + field :nom_com, String, null: false, deprecation_reason: 'Utilisez le champ `commune` à la place.' + field :code_com, String, null: false, deprecation_reason: 'Utilisez le champ `commune` à la place.' + field :code_arr, String, null: false, deprecation_reason: 'Utilisez le champ `prefixe` à la place.' + field :feuille, Int, null: false, deprecation_reason: 'L‘information n‘est plus disponible.' + field :surface_intersection, Float, null: false, deprecation_reason: 'L‘information n‘est plus disponible.' + field :surface_parcelle, Float, null: false, deprecation_reason: 'Utilisez le champ `surface` à la place.' end end diff --git a/app/graphql/types/geo_areas/selection_utilisateur_type.rb b/app/graphql/types/geo_areas/selection_utilisateur_type.rb index 004f07583..6de50ab73 100644 --- a/app/graphql/types/geo_areas/selection_utilisateur_type.rb +++ b/app/graphql/types/geo_areas/selection_utilisateur_type.rb @@ -1,5 +1,7 @@ module Types::GeoAreas class SelectionUtilisateurType < Types::BaseObject implements Types::GeoAreaType + + field :description, String, null: false end end diff --git a/app/helpers/champ_helper.rb b/app/helpers/champ_helper.rb index 42b8d9310..41881adf7 100644 --- a/app/helpers/champ_helper.rb +++ b/app/helpers/champ_helper.rb @@ -36,7 +36,7 @@ module ChampHelper case geo_area.source when GeoArea.sources.fetch(:cadastre) capture do - concat "Parcelle n° #{geo_area.numero} - Feuille #{geo_area.code_arr} #{geo_area.section} #{geo_area.feuille} - #{geo_area.surface_parcelle.round} m" + concat "Parcelle n° #{geo_area.numero} - Feuille #{geo_area.prefixe} #{geo_area.section} - #{geo_area.surface.round} m" concat tag.sup("2") end when GeoArea.sources.fetch(:selection_utilisateur) diff --git a/app/models/geo_area.rb b/app/models/geo_area.rb index 46df9af73..b8dcd31fe 100644 --- a/app/models/geo_area.rb +++ b/app/models/geo_area.rb @@ -14,25 +14,33 @@ class GeoArea < ApplicationRecord belongs_to :champ, optional: false - store :properties, accessors: [ - :description, - :surface_intersection, - :surface_parcelle, - :numero, - :feuille, - :section, - :code_dep, - :nom_com, - :code_com, - :code_arr, - :code, - :nom, - :commune, - :culture, - :code_culture, - :surface, - :bio - ] + # FIXME: once geo_areas are migrated to not use YAML serialization we can enable store_accessor + # store_accessor :properties, :description, :numero, :section + + def properties + value = read_attribute(:properties) + if value.is_a? String + ActiveRecord::Coders::YAMLColumn.new(:properties).load(value) + else + value + end + end + + def description + properties['description'] + end + + def numero + properties['numero'] + end + + def section + properties['section'] + end + + def filename + properties['filename'] + end enum source: { cadastre: 'cadastre', @@ -48,10 +56,12 @@ class GeoArea < ApplicationRecord { type: 'Feature', geometry: safe_geometry, - properties: properties.symbolize_keys.merge( + properties: cadastre_properties.merge( source: source, area: area, length: length, + description: description, + filename: filename, id: id, champ_id: champ.stable_id, dossier_id: champ.dossier_id @@ -69,16 +79,6 @@ class GeoArea < ApplicationRecord nil end - def self.from_feature_collection(feature_collection) - feature_collection[:features].map do |feature| - GeoArea.new( - source: feature[:properties].delete(:source), - properties: feature[:properties], - geometry: feature[:geometry] - ) - end - end - def area if polygon? GeojsonService.area(geometry.deep_symbolize_keys).round(1) @@ -108,4 +108,107 @@ class GeoArea < ApplicationRecord def point? geometry['type'] == 'Point' end + + def legacy_cadastre? + cadastre? && properties['surface_intersection'].present? + end + + def cadastre? + source == GeoArea.sources.fetch(:cadastre) + end + + def cadastre_properties + if cadastre? + { + cid: cid, + numero: numero, + section: section, + prefixe: prefixe, + commune: commune, + surface: surface + } + else + {} + end + end + + def code_dep + if legacy_cadastre? + properties['code_dep'] + else + properties['commune'][0..1] + end + end + + def code_com + if legacy_cadastre? + properties['code_com'] + else + properties['commune'][2...commune.size] + end + end + + def nom_com + if legacy_cadastre? + properties['nom_com'] + else + '' + end + end + + def surface_intersection + if legacy_cadastre? + properties['surface_intersection'] + else + '' + end + end + + def feuille + if legacy_cadastre? + properties['feuille'] + else + 1 + end + end + + def code_arr + prefixe + end + + def surface_parcelle + surface + end + + def surface + if legacy_cadastre? + properties['surface_parcelle'] + else + properties['contenance'] + end + end + + def prefixe + if legacy_cadastre? + properties['code_arr'] + else + properties['prefixe'] + end + end + + def commune + if legacy_cadastre? + "#{properties['code_dep']}#{properties['code_com']}" + else + properties['commune'] + end + end + + def cid + if legacy_cadastre? + "#{code_dep}#{code_com}#{code_arr}#{section}#{numero}" + else + properties['id'] + end + end end diff --git a/app/views/champs/carte/index.js.erb b/app/views/champs/carte/index.js.erb index c77c1eb8f..37ef6c3a7 100644 --- a/app/views/champs/carte/index.js.erb +++ b/app/views/champs/carte/index.js.erb @@ -4,6 +4,6 @@ partial: 'shared/champs/carte/geo_areas', locals: { champ: @champ, editing: true }) %> -<% if @update_cadastres %> - <%= fire_event('cadastres:update', { featureCollection: @champ.to_feature_collection }.to_json) %> +<% if @focus %> + <%= fire_event('map:feature:focus', { bbox: @champ.bounding_box }.to_json) %> <% end %> diff --git a/app/views/shared/champs/carte/_geo_areas.html.haml b/app/views/shared/champs/carte/_geo_areas.html.haml index f5a7348f5..6d594b3da 100644 --- a/app/views/shared/champs/carte/_geo_areas.html.haml +++ b/app/views/shared/champs/carte/_geo_areas.html.haml @@ -7,7 +7,7 @@ - if editing = link_to '#', data: { geo_area: geo_area.id } do = geo_area_label(geo_area) - = text_field_tag :description, geo_area.description, data: { geo_area: geo_area.id }, placeholder: 'Description de la sélection' + = text_field_tag :description, geo_area.description, data: { geo_area: geo_area.id }, placeholder: 'Description de la sélection', class: 'no-margin' - else = link_to '#', data: { geo_area: geo_area.id } do = geo_area_label(geo_area) diff --git a/spec/controllers/champs/carte_controller_spec.rb b/spec/controllers/champs/carte_controller_spec.rb index da7f2a7af..d618a1dad 100644 --- a/spec/controllers/champs/carte_controller_spec.rb +++ b/spec/controllers/champs/carte_controller_spec.rb @@ -25,7 +25,8 @@ describe Champs::CarteController, type: :controller do let(:params) do { champ_id: champ.id, - feature: feature + feature: feature, + source: GeoArea.sources.fetch(:selection_utilisateur) } end @@ -98,67 +99,37 @@ describe Champs::CarteController, type: :controller do it { expect(response.status).to eq 204 } end - describe 'POST #import' do + describe 'GET #index' do render_views let(:params) do - { - champ_id: champ.id, - features: [feature] - - } + { champ_id: champ.id } end - before do - post :import, params: params - end - - it { - expect(response.status).to eq 201 - expect(response.body).to include("bbox") - } - end - - describe 'GET #index' do - render_views - before do request.accept = "application/javascript" request.content_type = "application/javascript" + get :index, params: params end - context 'with cadastres update' do - let(:params) do - { - champ_id: champ.id, - cadastres: 'update' - } - end - - before do - get :index, params: params - end - + context "update list" do it { + expect(response.body).not_to include("DS.fire('map:feature:focus'") expect(response.status).to eq 200 - expect(response.body).to include("DS.fire('cadastres:update'") } end - context 'without cadastres update' do + context "update list and focus" do let(:params) do { - champ_id: champ.id + champ_id: champ.id, + focus: true } end - before do - get :index, params: params - end - it { + expect(response.body).to include("DS.fire('map:feature:focus'") expect(response.status).to eq 200 - expect(response.body).not_to include("DS.fire('cadastres:update'") } end end diff --git a/spec/factories/geo_area.rb b/spec/factories/geo_area.rb index 4e3a20275..5a5909be7 100644 --- a/spec/factories/geo_area.rb +++ b/spec/factories/geo_area.rb @@ -1,17 +1,11 @@ FactoryBot.define do factory :geo_area do association :champ + properties { {} } trait :cadastre do source { GeoArea.sources.fetch(:cadastre) } - numero { '42' } - feuille { 'A11' } - end - - trait :quartier_prioritaire do - source { GeoArea.sources.fetch(:quartier_prioritaire) } - nom { 'XYZ' } - commune { 'Paris' } + properties { { numero: '42', section: 'A11', commune: '75127' } } end trait :selection_utilisateur do diff --git a/spec/serializers/champ_serializer_spec.rb b/spec/serializers/champ_serializer_spec.rb index a80922f5e..27868f291 100644 --- a/spec/serializers/champ_serializer_spec.rb +++ b/spec/serializers/champ_serializer_spec.rb @@ -85,7 +85,7 @@ describe ChampSerializer do source: GeoArea.sources.fetch(:cadastre), geometry: geo_json, numero: '42', - feuille: 'A11' + section: 'A11' ) expect(subject[:geo_areas].first.key?(:nom)).to be_falsey } From 19440afebfa51d895734e580c81cf9494a7c13d3 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 18:51:19 +0200 Subject: [PATCH 27/37] Improuve mapbox utilis and shared components --- app/assets/stylesheets/carte.scss | 37 +++++++++ app/assets/stylesheets/forms.scss | 8 +- .../components/ComboAdresseSearch.jsx | 5 +- app/javascript/components/ComboSearch.jsx | 7 +- .../shared/mapbox/MapStyleControl.jsx | 69 ++++++++++++++++ .../components/shared/mapbox/Mapbox.js | 3 - .../shared/mapbox/SwitchMapStyle.jsx | 81 ------------------- .../components/shared/mapbox/styles/base.js | 22 ++--- .../components/shared/mapbox/styles/index.js | 32 ++++---- .../cadastre.js} | 2 +- .../shared/mapbox/styles/layers/ign.js | 8 ++ .../{ortho-style.js => layers/ortho.js} | 0 .../{vector-style.js => layers/vector.js} | 0 .../components/shared/mapbox/utils.js | 30 +++---- 14 files changed, 168 insertions(+), 136 deletions(-) create mode 100644 app/javascript/components/shared/mapbox/MapStyleControl.jsx delete mode 100644 app/javascript/components/shared/mapbox/Mapbox.js delete mode 100644 app/javascript/components/shared/mapbox/SwitchMapStyle.jsx rename app/javascript/components/shared/mapbox/styles/{cadastre-layers.js => layers/cadastre.js} (98%) create mode 100644 app/javascript/components/shared/mapbox/styles/layers/ign.js rename app/javascript/components/shared/mapbox/styles/{ortho-style.js => layers/ortho.js} (100%) rename app/javascript/components/shared/mapbox/styles/{vector-style.js => layers/vector.js} (100%) diff --git a/app/assets/stylesheets/carte.scss b/app/assets/stylesheets/carte.scss index 8728a4915..4e4a6cf70 100644 --- a/app/assets/stylesheets/carte.scss +++ b/app/assets/stylesheets/carte.scss @@ -1,3 +1,5 @@ +@import "colors"; + .areas-title { font-weight: bold; margin-top: 5px; @@ -15,3 +17,38 @@ .form [data-react-class='MapEditor'] [data-reach-combobox-input] { margin-bottom: 0; } + +.map-style-control { + position: absolute; + bottom: 4px; + left: 10px; + + img { + width: 100%; + } + + button { + padding: 0; + border: none; + cursor: pointer; + + > div { + position: absolute; + bottom: 5px; + left: 5px; + } + } +} + +.cadastres-selection-control { + position: absolute; + top: 135px; + left: 10px; + + button { + &.on, + &:hover { + background-color: rgba(0, 0, 0, 0.05); + } + } +} diff --git a/app/assets/stylesheets/forms.scss b/app/assets/stylesheets/forms.scss index cfe13af7d..a73a7d8e2 100644 --- a/app/assets/stylesheets/forms.scss +++ b/app/assets/stylesheets/forms.scss @@ -163,7 +163,7 @@ } } - input[type=text]:not([data-address='true']), + input[type=text], input[type=email], input[type=password], input[type=date], @@ -178,6 +178,10 @@ &.small-margin { margin-bottom: $default-spacer; } + + &.no-margin { + margin-bottom: 0; + } } .add-row { @@ -475,7 +479,7 @@ } [data-react-class]:not([data-react-class="ComboMultipleDropdownList"]) { - [data-reach-combobox-input] { + [data-reach-combobox-input]:not(.no-margin) { margin-bottom: $default-fields-spacer; } } diff --git a/app/javascript/components/ComboAdresseSearch.jsx b/app/javascript/components/ComboAdresseSearch.jsx index 150be5f37..78177dbe8 100644 --- a/app/javascript/components/ComboAdresseSearch.jsx +++ b/app/javascript/components/ComboAdresseSearch.jsx @@ -11,13 +11,15 @@ function ComboAdresseSearch({ hiddenFieldId, onChange, transformResult = ({ properties: { label } }) => [label, label], - allowInputValues = true + allowInputValues = true, + className }) { const transformResults = useCallback((_, { features }) => features); return ( getMapStyle(styleId, optionalLayers), [ + styleId, + optionalLayers + ]); + + useEffect(() => onStyleChange(), [styleId, cadastreEnabled]); + + return [style, setStyle]; +} + +function MapStyleControl({ style, setStyle }) { + const nextStyle = getNextStyle(style); + const { title, preview, color } = STYLES[nextStyle]; + + return ( +

+ +
+ ); +} + +MapStyleControl.propTypes = { + style: PropTypes.string, + setStyle: PropTypes.func +}; + +export default MapStyleControl; diff --git a/app/javascript/components/shared/mapbox/Mapbox.js b/app/javascript/components/shared/mapbox/Mapbox.js deleted file mode 100644 index 3acc17e94..000000000 --- a/app/javascript/components/shared/mapbox/Mapbox.js +++ /dev/null @@ -1,3 +0,0 @@ -import ReactMapboxGl from 'react-mapbox-gl'; - -export default ReactMapboxGl({}); diff --git a/app/javascript/components/shared/mapbox/SwitchMapStyle.jsx b/app/javascript/components/shared/mapbox/SwitchMapStyle.jsx deleted file mode 100644 index d971219f9..000000000 --- a/app/javascript/components/shared/mapbox/SwitchMapStyle.jsx +++ /dev/null @@ -1,81 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; - -import ortho from './styles/images/preview-ortho.png'; -import vector from './styles/images/preview-vector.png'; - -const STYLES = { - ortho: { - title: 'Satellite', - preview: ortho, - color: '#fff' - }, - vector: { - title: 'Vectoriel', - preview: vector, - color: '#000' - } -}; - -const IGN_STYLES = { - ...STYLES, - ign: { - title: 'Carte IGN', - preview: vector, - color: '#000' - } -}; - -function getNextStyle(style, ign) { - const styles = Object.keys(ign ? IGN_STYLES : STYLES); - let index = styles.indexOf(style) + 1; - if (index === styles.length) { - return styles[0]; - } - return styles[index]; -} - -function SwitchMapStyle({ style, setStyle, ign }) { - const nextStyle = getNextStyle(style, ign); - const { title, preview, color } = (ign ? IGN_STYLES : STYLES)[nextStyle]; - - const imgStyle = { - width: '100%', - height: '100%', - cursor: 'pointer' - }; - - const textStyle = { - position: 'relative', - bottom: '26px', - left: '4px', - color - }; - - return ( -
setStyle(nextStyle)} - > -
- {title} -
- {title} -
-
-
- ); -} - -SwitchMapStyle.propTypes = { - style: PropTypes.string, - setStyle: PropTypes.func, - ign: PropTypes.bool -}; - -export default SwitchMapStyle; diff --git a/app/javascript/components/shared/mapbox/styles/base.js b/app/javascript/components/shared/mapbox/styles/base.js index f0e9c2242..e233e607f 100644 --- a/app/javascript/components/shared/mapbox/styles/base.js +++ b/app/javascript/components/shared/mapbox/styles/base.js @@ -1,4 +1,4 @@ -import cadastreLayers from './cadastre-layers'; +import cadastreLayers from './layers/cadastre'; const IGN_TOKEN = 'rc1egnbeoss72hxvd143tbyk'; @@ -138,7 +138,16 @@ function rasterSource(tiles, attribution) { }; } -export function buildLayers(ids) { +function rasterLayer(source) { + return { + id: source, + source, + type: 'raster', + paint: { 'raster-resampling': 'linear' } + }; +} + +export function buildOptionalLayers(ids) { return OPTIONAL_LAYERS.filter(({ id }) => ids.includes(id)) .flatMap(({ layers }) => layers) .flatMap(([, code]) => @@ -148,15 +157,6 @@ export function buildLayers(ids) { ); } -export function rasterLayer(source) { - return { - id: source, - source, - type: 'raster', - paint: { 'raster-resampling': 'linear' } - }; -} - export default { version: 8, metadat: { diff --git a/app/javascript/components/shared/mapbox/styles/index.js b/app/javascript/components/shared/mapbox/styles/index.js index 97257b33f..218cd5ddf 100644 --- a/app/javascript/components/shared/mapbox/styles/index.js +++ b/app/javascript/components/shared/mapbox/styles/index.js @@ -1,29 +1,27 @@ -import baseStyle, { rasterLayer, buildLayers } from './base'; -import orthoStyle from './ortho-style'; -import vectorStyle from './vector-style'; +import baseStyle, { buildOptionalLayers } from './base'; +import orthoStyle from './layers/ortho'; +import vectorStyle from './layers/vector'; +import ignLayers from './layers/ign'; -export function getMapStyle(style, optionalLayers) { - const mapStyle = { ...baseStyle }; +export function getMapStyle(id, optionalLayers) { + const style = { ...baseStyle, id }; - switch (style) { + switch (id) { case 'ortho': - mapStyle.layers = orthoStyle; - mapStyle.id = 'ortho'; - mapStyle.name = 'Photographies aériennes'; + style.layers = orthoStyle; + style.name = 'Photographies aériennes'; break; case 'vector': - mapStyle.layers = vectorStyle; - mapStyle.id = 'vector'; - mapStyle.name = 'Carte OSM'; + style.layers = vectorStyle; + style.name = 'Carte OSM'; break; case 'ign': - mapStyle.layers = [rasterLayer('plan-ign')]; - mapStyle.id = 'ign'; - mapStyle.name = 'Carte IGN'; + style.layers = ignLayers; + style.name = 'Carte IGN'; break; } - mapStyle.layers = mapStyle.layers.concat(buildLayers(optionalLayers)); + style.layers = style.layers.concat(buildOptionalLayers(optionalLayers)); - return mapStyle; + return style; } diff --git a/app/javascript/components/shared/mapbox/styles/cadastre-layers.js b/app/javascript/components/shared/mapbox/styles/layers/cadastre.js similarity index 98% rename from app/javascript/components/shared/mapbox/styles/cadastre-layers.js rename to app/javascript/components/shared/mapbox/styles/layers/cadastre.js index 46e02c9fe..0aed8996c 100644 --- a/app/javascript/components/shared/mapbox/styles/cadastre-layers.js +++ b/app/javascript/components/shared/mapbox/styles/layers/cadastre.js @@ -82,7 +82,7 @@ export default [ type: 'fill', source: 'cadastre', 'source-layer': 'parcelles', - filter: ['==', 'id', ''], + filter: ['in', 'id', ''], paint: { 'fill-color': 'rgba(1, 129, 0, 1)', 'fill-opacity': 0.7 diff --git a/app/javascript/components/shared/mapbox/styles/layers/ign.js b/app/javascript/components/shared/mapbox/styles/layers/ign.js new file mode 100644 index 000000000..e7e7614a7 --- /dev/null +++ b/app/javascript/components/shared/mapbox/styles/layers/ign.js @@ -0,0 +1,8 @@ +export default [ + { + id: 'ign', + source: 'plan-ign', + type: 'raster', + paint: { 'raster-resampling': 'linear' } + } +]; diff --git a/app/javascript/components/shared/mapbox/styles/ortho-style.js b/app/javascript/components/shared/mapbox/styles/layers/ortho.js similarity index 100% rename from app/javascript/components/shared/mapbox/styles/ortho-style.js rename to app/javascript/components/shared/mapbox/styles/layers/ortho.js diff --git a/app/javascript/components/shared/mapbox/styles/vector-style.js b/app/javascript/components/shared/mapbox/styles/layers/vector.js similarity index 100% rename from app/javascript/components/shared/mapbox/styles/vector-style.js rename to app/javascript/components/shared/mapbox/styles/layers/vector.js diff --git a/app/javascript/components/shared/mapbox/utils.js b/app/javascript/components/shared/mapbox/utils.js index c54ae3fb9..721a0bfb5 100644 --- a/app/javascript/components/shared/mapbox/utils.js +++ b/app/javascript/components/shared/mapbox/utils.js @@ -1,5 +1,4 @@ import { LngLatBounds } from 'mapbox-gl'; -import { useEffect } from 'react'; export function getBounds(geometry) { const bbox = new LngLatBounds(); @@ -18,15 +17,9 @@ export function getBounds(geometry) { return bbox; } -export function fitBounds(map, feature) { - if (map) { - map.fitBounds(getBounds(feature.geometry), { padding: 100 }); - } -} - -export function findFeature(featureCollection, id) { +export function findFeature(featureCollection, value, property = 'id') { return featureCollection.features.find( - (feature) => feature.properties.id === id + (feature) => feature.properties[property] === value ); } @@ -48,19 +41,10 @@ export function filterFeatureCollectionByGeometryType(featureCollection, type) { }; } -export function noop() {} - export function generateId() { return Math.random().toString(20).substr(2, 6); } -export function useEvent(eventName, callback) { - return useEffect(() => { - addEventListener(eventName, callback); - return () => removeEventListener(eventName, callback); - }, [eventName, callback]); -} - export function getCenter(geometry, lngLat) { const bbox = new LngLatBounds(); @@ -76,3 +60,13 @@ export function getCenter(geometry, lngLat) { return bbox.getCenter(); } } + +export function defer() { + const deferred = {}; + const promise = new Promise(function (resolve, reject) { + deferred.resolve = resolve; + deferred.reject = reject; + }); + deferred.promise = promise; + return deferred; +} From 2244263b49b5cacd6679dc987b9833d22b58a881 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 18:51:53 +0200 Subject: [PATCH 28/37] Add cadastres to MapEditor --- app/javascript/components/MapEditor/index.jsx | 352 +++-------- .../MapEditor/{utils.js => readGeoFile.js} | 60 +- .../components/MapEditor/useMapboxEditor.js | 553 ++++++++++++++++++ 3 files changed, 653 insertions(+), 312 deletions(-) rename app/javascript/components/MapEditor/{utils.js => readGeoFile.js} (82%) create mode 100644 app/javascript/components/MapEditor/useMapboxEditor.js diff --git a/app/javascript/components/MapEditor/index.jsx b/app/javascript/components/MapEditor/index.jsx index 96e5ceea0..fcedfba4f 100644 --- a/app/javascript/components/MapEditor/index.jsx +++ b/app/javascript/components/MapEditor/index.jsx @@ -1,257 +1,47 @@ -import React, { - useState, - useCallback, - useRef, - useMemo, - useEffect -} from 'react'; +import React, { useState } from 'react'; import PropTypes from 'prop-types'; -import mapboxgl from 'mapbox-gl'; -import { GeoJSONLayer, ZoomControl } from 'react-mapbox-gl'; +import ReactMapboxGl, { ZoomControl } from 'react-mapbox-gl'; import DrawControl from 'react-mapbox-gl-draw'; +import { MapIcon } from '@heroicons/react/outline'; import 'mapbox-gl/dist/mapbox-gl.css'; import '@mapbox/mapbox-gl-draw/dist/mapbox-gl-draw.css'; -import { getJSON, ajax, fire } from '@utils'; - -import Mapbox from '../shared/mapbox/Mapbox'; -import { getMapStyle } from '../shared/mapbox/styles'; -import SwitchMapStyle from '../shared/mapbox/SwitchMapStyle'; +import MapStyleControl, { useMapStyle } from '../shared/mapbox/MapStyleControl'; import { FlashMessage } from '../shared/FlashMessage'; import ComboAdresseSearch from '../ComboAdresseSearch'; -import { - polygonCadastresFill, - polygonCadastresLine, - readGeoFile -} from './utils'; -import { - noop, - filterFeatureCollection, - fitBounds, - generateId, - useEvent, - findFeature -} from '../shared/mapbox/utils'; +import { useMapboxEditor } from './useMapboxEditor'; -function MapEditor({ featureCollection, url, preview, options }) { - const drawControl = useRef(null); - const [currentMap, setCurrentMap] = useState(null); +const Mapbox = ReactMapboxGl({}); - const [errorMessage, setErrorMessage] = useState(); - const [style, setStyle] = useState('ortho'); +function MapEditor({ featureCollection, url, options, preview }) { + const [cadastreEnabled, setCadastreEnabled] = useState(false); const [coords, setCoords] = useState([1.7, 46.9]); const [zoom, setZoom] = useState([5]); - const [bbox, setBbox] = useState(featureCollection.bbox); - const [importInputs, setImportInputs] = useState([]); - const [cadastresFeatureCollection, setCadastresFeatureCollection] = useState( - filterFeatureCollection(featureCollection, 'cadastre') - ); - const mapStyle = useMemo(() => getMapStyle(style, options.layers), [ - style, - options - ]); - const hasCadastres = useMemo(() => options.layers.includes('cadastres')); + const { + isSupported, + error, + inputs, + onLoad, + onStyleChange, + onFileChange, + drawRef, + createFeatures, + updateFeatures, + deleteFeatures, + addInputFile, + removeInputFile + } = useMapboxEditor(featureCollection, { + url, + enabled: !preview, + cadastreEnabled + }); + const [style, setStyle] = useMapStyle(options.layers, { + onStyleChange, + cadastreEnabled + }); - useEffect(() => { - const timer = setTimeout(() => setErrorMessage(null), 5000); - return () => clearTimeout(timer); - }, [errorMessage]); - - const translations = [ - ['.mapbox-gl-draw_line', 'Tracer une ligne'], - ['.mapbox-gl-draw_polygon', 'Dessiner un polygone'], - ['.mapbox-gl-draw_point', 'Ajouter un point'], - ['.mapbox-gl-draw_trash', 'Supprimer'] - ]; - for (const [selector, translation] of translations) { - const element = document.querySelector(selector); - if (element) { - element.setAttribute('title', translation); - } - } - - const onFeatureFocus = useCallback( - ({ detail }) => { - const { id } = detail; - const featureCollection = drawControl.current.draw.getAll(); - const feature = findFeature(featureCollection, id); - if (feature) { - fitBounds(currentMap, feature); - } - }, - [currentMap, drawControl.current] - ); - - const onFeatureUpdate = useCallback( - async ({ detail }) => { - const { id, properties } = detail; - const featureCollection = drawControl.current.draw.getAll(); - const feature = findFeature(featureCollection, id); - - if (feature) { - getJSON(`${url}/${id}`, { feature: { properties } }, 'patch'); - } - }, - [url, drawControl.current] - ); - - const updateFeaturesList = useCallback( - async (features) => { - const cadastres = features.find( - ({ geometry }) => geometry.type === 'Polygon' - ); - await ajax({ - url, - type: 'get', - data: cadastres ? 'cadastres=update' : '' - }); - fire(document, 'ds:page:update'); - }, - [url] - ); - - const onCadastresUpdate = useCallback(({ detail }) => { - setCadastresFeatureCollection( - filterFeatureCollection(detail.featureCollection, 'cadastre') - ); - }, []); - - useEvent('map:feature:focus', onFeatureFocus); - useEvent('map:feature:update', onFeatureUpdate); - useEvent('cadastres:update', onCadastresUpdate); - - function setFeatureId(lid, feature) { - const draw = drawControl.current.draw; - draw.setFeatureProperty(lid, 'id', feature.properties.id); - } - - function updateImportInputs(inputs, inputId) { - const updatedInputs = inputs.filter((input) => input.id !== inputId); - setImportInputs(updatedInputs); - } - - async function onDrawCreate({ features }) { - try { - for (const feature of features) { - const data = await getJSON(url, { feature }, 'post'); - setFeatureId(feature.id, data.feature); - } - - updateFeaturesList(features); - } catch { - setErrorMessage('Le polygone dessiné n’est pas valide.'); - } - } - - async function onDrawUpdate({ features }) { - try { - for (const feature of features) { - const { id } = feature.properties; - if (id) { - await getJSON(`${url}/${id}`, { feature }, 'patch'); - } else { - const data = await getJSON(url, { feature }, 'post'); - setFeatureId(feature.id, data.feature); - } - } - - updateFeaturesList(features); - } catch { - setErrorMessage('Le polygone dessiné n’est pas valide.'); - } - } - - async function onDrawDelete({ features }) { - for (const feature of features) { - const { id } = feature.properties; - await getJSON(`${url}/${id}`, null, 'delete'); - } - - updateFeaturesList(features); - } - - function onMapLoad(map) { - setCurrentMap(map); - - drawControl.current.draw.set( - filterFeatureCollection(featureCollection, 'selection_utilisateur') - ); - } - - const onFileImport = async (e, inputId) => { - try { - const featureCollection = await readGeoFile(e.target.files[0]); - const resultFeatureCollection = await getJSON( - `${url}/import`, - featureCollection, - 'post' - ); - let inputs = [...importInputs]; - const setInputs = inputs.map((input) => { - if (input.id === inputId) { - input.disabled = true; - input.hasValue = true; - resultFeatureCollection.features.forEach((resultFeature) => { - featureCollection.features.forEach((feature) => { - if ( - JSON.stringify(resultFeature.geometry) === - JSON.stringify(feature.geometry) - ) { - input.featureIds.push(resultFeature.properties.id); - } - }); - }); - } - return input; - }); - - drawControl.current.draw.set( - filterFeatureCollection( - resultFeatureCollection, - 'selection_utilisateur' - ) - ); - - updateFeaturesList(resultFeatureCollection.features); - setImportInputs(setInputs); - setBbox(resultFeatureCollection.bbox); - } catch { - setErrorMessage('Le fichier importé contient des polygones invalides.'); - } - }; - - const addInputFile = (e) => { - e.preventDefault(); - let inputs = [...importInputs]; - inputs.push({ - id: generateId(), - disabled: false, - featureIds: [], - hasValue: false - }); - setImportInputs(inputs); - }; - - const removeInputFile = async (e, inputId) => { - e.preventDefault(); - const draw = drawControl.current.draw; - const featureCollection = draw.getAll(); - let inputs = [...importInputs]; - const inputToRemove = inputs.find((input) => input.id === inputId); - - for (const feature of featureCollection.features) { - if (inputToRemove.featureIds.includes(feature.properties.id)) { - const featureToRemove = draw.get(feature.id); - await getJSON(`${url}/${feature.properties.id}`, null, 'delete'); - draw.delete(feature.id).getAll(); - updateFeaturesList([featureToRemove]); - } - } - updateImportInputs(inputs, inputId); - }; - - if (!mapboxgl.supported()) { + if (!isSupported) { return (

Nous ne pouvons pas afficher notre éditeur de carte car il est @@ -263,9 +53,7 @@ function MapEditor({ featureCollection, url, preview, options }) { return ( <> - {errorMessage && ( - - )} + {error && }

Besoin d'aide ?  @@ -278,12 +66,12 @@ function MapEditor({ featureCollection, url, preview, options }) {

-
+
- {importInputs.map((input) => ( + {inputs.map((input) => (
onFileImport(e, input.id)} + onChange={(e) => onFileChange(e, input.id)} /> {input.hasValue && (
{ @@ -323,38 +112,43 @@ function MapEditor({ featureCollection, url, preview, options }) { />
onMapLoad(map)} - fitBounds={bbox} - fitBoundsOptions={{ padding: 100 }} + onStyleLoad={(map) => onLoad(map)} center={coords} zoom={zoom} - style={mapStyle} - containerStyle={{ - height: '500px' - }} + style={style} + containerStyle={{ height: '500px' }} > - {hasCadastres ? ( - - ) : null} - - + )} + + {options.layers.includes('cadastres') && ( +
+ +
+ )} ); @@ -363,15 +157,11 @@ function MapEditor({ featureCollection, url, preview, options }) { MapEditor.propTypes = { featureCollection: PropTypes.shape({ bbox: PropTypes.array, - features: PropTypes.array, - id: PropTypes.number + features: PropTypes.array }), url: PropTypes.string, preview: PropTypes.bool, - options: PropTypes.shape({ - layers: PropTypes.array, - ign: PropTypes.bool - }) + options: PropTypes.shape({ layers: PropTypes.array }) }; export default MapEditor; diff --git a/app/javascript/components/MapEditor/utils.js b/app/javascript/components/MapEditor/readGeoFile.js similarity index 82% rename from app/javascript/components/MapEditor/utils.js rename to app/javascript/components/MapEditor/readGeoFile.js index 955c92859..59f972f02 100644 --- a/app/javascript/components/MapEditor/utils.js +++ b/app/javascript/components/MapEditor/readGeoFile.js @@ -1,17 +1,28 @@ import { gpx, kml } from '@tmcw/togeojson/dist/togeojson.es.js'; +import { generateId } from '../shared/mapbox/utils'; -export const polygonCadastresFill = { - 'fill-color': '#EC3323', - 'fill-opacity': 0.3 -}; +export function readGeoFile(file) { + const isGpxFile = file.name.includes('.gpx'); + const reader = new FileReader(); -export const polygonCadastresLine = { - 'line-color': 'rgba(255, 0, 0, 1)', - 'line-width': 4, - 'line-dasharray': [1, 1] -}; + return new Promise((resolve) => { + reader.onload = (event) => { + const xml = new DOMParser().parseFromString( + event.target.result, + 'text/xml' + ); + const featureCollection = normalizeFeatureCollection( + isGpxFile ? gpx(xml) : kml(xml), + file.name + ); -export function normalizeFeatureCollection(featureCollection) { + resolve(featureCollection); + }; + reader.readAsText(file, 'UTF-8'); + }); +} + +function normalizeFeatureCollection(featureCollection, filename) { const features = []; for (const feature of featureCollection.features) { switch (feature.geometry.type) { @@ -65,26 +76,13 @@ export function normalizeFeatureCollection(featureCollection) { } } - featureCollection.features = features; + featureCollection.filename = `${generateId()}-${filename}`; + featureCollection.features = features.map((feature) => ({ + ...feature, + properties: { + ...feature.properties, + filename: featureCollection.filename + } + })); return featureCollection; } - -export function readGeoFile(file) { - const isGpxFile = file.name.includes('.gpx'); - const reader = new FileReader(); - - return new Promise((resolve) => { - reader.onload = (event) => { - const xml = new DOMParser().parseFromString( - event.target.result, - 'text/xml' - ); - const featureCollection = normalizeFeatureCollection( - isGpxFile ? gpx(xml) : kml(xml) - ); - - resolve(featureCollection); - }; - reader.readAsText(file, 'UTF-8'); - }); -} diff --git a/app/javascript/components/MapEditor/useMapboxEditor.js b/app/javascript/components/MapEditor/useMapboxEditor.js new file mode 100644 index 000000000..aa2bd7f0b --- /dev/null +++ b/app/javascript/components/MapEditor/useMapboxEditor.js @@ -0,0 +1,553 @@ +import { useState, useCallback, useRef, useEffect, useMemo } from 'react'; +import mapboxgl from 'mapbox-gl'; +import { getJSON, ajax, fire } from '@utils'; + +import { readGeoFile } from './readGeoFile'; +import { + filterFeatureCollection, + generateId, + findFeature, + getBounds, + defer +} from '../shared/mapbox/utils'; + +const SOURCE_SELECTION_UTILISATEUR = 'selection_utilisateur'; +const SOURCE_CADASTRE = 'cadastre'; + +export function useMapboxEditor( + featureCollection, + { url, enabled = true, cadastreEnabled = true } +) { + const [isLoaded, setLoaded] = useState(false); + const mapRef = useRef(); + const drawRef = useRef(); + const loadedRef = useRef(defer()); + const selectedCadastresRef = useRef(() => new Set()); + const isSupported = useMemo(() => mapboxgl.supported()); + + useEffect(() => { + const translations = [ + ['.mapbox-gl-draw_line', 'Tracer une ligne'], + ['.mapbox-gl-draw_polygon', 'Dessiner un polygone'], + ['.mapbox-gl-draw_point', 'Ajouter un point'], + ['.mapbox-gl-draw_trash', 'Supprimer'] + ]; + for (const [selector, translation] of translations) { + const element = document.querySelector(selector); + if (element) { + element.setAttribute('title', translation); + } + } + }, [isLoaded]); + + const addEventListener = useCallback((eventName, target, callback) => { + loadedRef.current.promise.then(() => { + mapRef.current.on(eventName, target, callback); + }); + return () => { + if (mapRef.current) { + mapRef.current.off(eventName, target, callback); + } + }; + }, []); + + const highlightFeature = useCallback((cid, highlight) => { + if (highlight) { + selectedCadastresRef.current.add(cid); + } else { + selectedCadastresRef.current.delete(cid); + } + if (selectedCadastresRef.current.size == 0) { + mapRef.current.setFilter('parcelle-highlighted', ['in', 'id', '']); + } else { + mapRef.current.setFilter('parcelle-highlighted', [ + 'in', + 'id', + ...selectedCadastresRef.current + ]); + } + }, []); + + const fitBounds = useCallback((bbox) => { + mapRef.current.fitBounds(bbox, { padding: 100 }); + }, []); + + const hoverFeature = useCallback((feature, hover) => { + if (!selectedCadastresRef.current.has(feature.properties.id)) { + mapRef.current.setFeatureState( + { + source: 'cadastre', + sourceLayer: 'parcelles', + id: feature.id + }, + { hover } + ); + } + }, []); + + const addFeatures = useCallback((features, external) => { + for (const feature of features) { + if (feature.lid) { + drawRef.current?.draw.setFeatureProperty( + feature.lid, + 'id', + feature.properties.id + ); + delete feature.lid; + } + if (external) { + if (feature.properties.source == SOURCE_SELECTION_UTILISATEUR) { + drawRef.current?.draw.add({ id: feature.properties.id, ...feature }); + } else { + highlightFeature(feature.properties.cid, true); + } + } + } + }, []); + + const removeFeatures = useCallback((features, external) => { + if (external) { + for (const feature of features) { + if (feature.properties.source == SOURCE_SELECTION_UTILISATEUR) { + drawRef.current?.draw.delete(feature.id); + } else { + highlightFeature(feature.properties.cid, false); + } + } + } + }, []); + + const onLoad = useCallback( + (map) => { + if (!mapRef.current) { + mapRef.current = map; + mapRef.current.fitBounds(props.featureCollection.bbox, { + padding: 100 + }); + onStyleChange(); + setLoaded(true); + loadedRef.current.resolve(); + } + }, + [featureCollection] + ); + + const addEventListeners = useCallback((events) => { + const unsubscribe = Object.entries( + events + ).map(([eventName, [target, callback]]) => + addEventListener(eventName, target, callback) + ); + return () => unsubscribe.map((unsubscribe) => unsubscribe()); + }, []); + + const { + createFeatures, + updateFeatures, + deleteFeatures, + ...props + } = useFeatureCollection(featureCollection, { + url, + enabled: isSupported && enabled, + addFeatures, + removeFeatures + }); + + const onStyleChange = useCallback(() => { + if (mapRef.current) { + const featureCollection = props.featureCollection; + if (!cadastreEnabled) { + drawRef.current?.draw.set( + filterFeatureCollection( + featureCollection, + SOURCE_SELECTION_UTILISATEUR + ) + ); + } + selectedCadastresRef.current = new Set( + filterFeatureCollection( + featureCollection, + SOURCE_CADASTRE + ).features.map(({ properties }) => properties.cid) + ); + if (selectedCadastresRef.current.size > 0) { + mapRef.current.setFilter('parcelle-highlighted', [ + 'in', + 'id', + ...selectedCadastresRef.current + ]); + } + } + }, [props.featureCollection, cadastreEnabled]); + + useExternalEvents(props.featureCollection, { + fitBounds, + createFeatures, + updateFeatures, + deleteFeatures + }); + useCadastres(props.featureCollection, { + addEventListeners, + hoverFeature, + createFeatures, + deleteFeatures, + enabled: cadastreEnabled + }); + + return { + isSupported, + onLoad, + onStyleChange, + drawRef, + createFeatures, + updateFeatures, + deleteFeatures, + ...props, + ...useImportFiles(props.featureCollection, { + createFeatures, + deleteFeatures + }) + }; +} + +function useFeatureCollection( + initialFeatureCollection, + { url, addFeatures, removeFeatures, enabled = true } +) { + const [error, onError] = useError(); + const [featureCollection, setFeatureCollection] = useState( + initialFeatureCollection + ); + const updateFeatureCollection = useCallback( + (callback) => { + setFeatureCollection(({ features }) => ({ + type: 'FeatureCollection', + features: callback(features) + })); + ajax({ url, type: 'GET' }) + .then(() => fire(document, 'ds:page:update')) + .catch(() => {}); + }, + [setFeatureCollection] + ); + + const createFeatures = useCallback( + async ({ features, source = SOURCE_SELECTION_UTILISATEUR, external }) => { + if (!enabled) { + return; + } + try { + const newFeatures = []; + for (const feature of features) { + const data = await getJSON(url, { feature, source }, 'post'); + if (data) { + if (source == SOURCE_SELECTION_UTILISATEUR) { + data.feature.lid = feature.id; + } + newFeatures.push(data.feature); + } + } + addFeatures(newFeatures, external); + updateFeatureCollection( + (features) => [...features, ...newFeatures], + external + ); + } catch (error) { + console.error(error); + onError('Le polygone dessiné n’est pas valide.'); + } + }, + [enabled, url, updateFeatureCollection, addFeatures] + ); + + const updateFeatures = useCallback( + async ({ features, source = SOURCE_SELECTION_UTILISATEUR, external }) => { + if (!enabled) { + return; + } + try { + const newFeatures = []; + for (const feature of features) { + const { id } = feature.properties; + if (id) { + await getJSON(`${url}/${id}`, { feature }, 'patch'); + } else { + const data = await getJSON(url, { feature, source }, 'post'); + if (data) { + if (source == SOURCE_SELECTION_UTILISATEUR) { + data.feature.lid = feature.id; + } + newFeatures.push(data.feature); + } + } + } + if (newFeatures.length > 0) { + addFeatures(newFeatures, external); + updateFeatureCollection((features) => [...features, ...newFeatures]); + } + } catch (error) { + console.error(error); + onError('Le polygone dessiné n’est pas valide.'); + } + }, + [enabled, url, updateFeatureCollection, addFeatures] + ); + + const deleteFeatures = useCallback( + async ({ features, external }) => { + if (!enabled) { + return; + } + try { + const deletedFeatures = []; + for (const feature of features) { + const { id } = feature.properties; + await getJSON(`${url}/${id}`, null, 'delete'); + deletedFeatures.push(feature); + } + removeFeatures(deletedFeatures, external); + const deletedFeatureIds = deletedFeatures.map( + ({ properties }) => properties.id + ); + updateFeatureCollection( + (features) => + features.filter( + ({ properties }) => !deletedFeatureIds.includes(properties.id) + ), + external + ); + } catch (error) { + console.error(error); + onError('Le polygone n’a pas pu être supprimé.'); + } + }, + [enabled, url, updateFeatureCollection, removeFeatures] + ); + + return { + featureCollection, + createFeatures, + updateFeatures, + deleteFeatures, + error + }; +} + +function useImportFiles(featureCollection, { createFeatures, deleteFeatures }) { + const [inputs, setInputs] = useState([]); + const addInput = useCallback( + (input) => { + setInputs((inputs) => [...inputs, input]); + }, + [setInputs] + ); + const removeInput = useCallback( + (inputId) => { + setInputs((inputs) => inputs.filter((input) => input.id !== inputId)); + }, + [setInputs] + ); + + const onFileChange = useCallback( + async (event, inputId) => { + const { features, filename } = await readGeoFile(event.target.files[0]); + createFeatures({ features, external: true }); + setInputs((inputs) => { + return inputs.map((input) => { + if (input.id === inputId) { + return { ...input, disabled: true, hasValue: true, filename }; + } + return input; + }); + }); + }, + [setInputs, createFeatures, featureCollection] + ); + + const addInputFile = useCallback( + (event) => { + event.preventDefault(); + addInput({ + id: generateId(), + disabled: false, + hasValue: false, + filename: '' + }); + }, + [addInput] + ); + + const removeInputFile = useCallback( + (event, inputId) => { + event.preventDefault(); + const { filename } = inputs.find((input) => input.id === inputId); + const features = featureCollection.features.filter( + (feature) => feature.properties.filename == filename + ); + deleteFeatures({ features, external: true }); + removeInput(inputId); + }, + [removeInput, deleteFeatures, featureCollection] + ); + + return { + inputs, + onFileChange, + addInputFile, + removeInputFile + }; +} + +function useExternalEvents( + featureCollection, + { fitBounds, createFeatures, updateFeatures, deleteFeatures } +) { + const onFeatureFocus = useCallback( + ({ detail }) => { + const { id, bbox } = detail; + if (id) { + const feature = findFeature(featureCollection, id); + if (feature) { + fitBounds(getBounds(feature.geometry)); + } + } else if (bbox) { + fitBounds(bbox); + } + }, + [featureCollection, fitBounds] + ); + + const onFeatureCreate = useCallback( + ({ detail }) => { + const { geometry, properties } = detail; + + if (geometry) { + createFeatures({ + features: [{ geometry, properties }], + external: true + }); + } + }, + [createFeatures] + ); + + const onFeatureUpdate = useCallback( + ({ detail }) => { + const { id, properties } = detail; + const feature = findFeature(featureCollection, id); + + if (feature) { + feature.properties = { ...feature.properties, ...properties }; + updateFeatures({ features: [feature], external: true }); + } + }, + [featureCollection, updateFeatures] + ); + + const onFeatureDelete = useCallback( + ({ detail }) => { + const { id } = detail; + const feature = findFeature(featureCollection, id); + + if (feature) { + deleteFeatures({ features: [feature], external: true }); + } + }, + [featureCollection, deleteFeatures] + ); + + useEvent('map:feature:focus', onFeatureFocus); + useEvent('map:feature:create', onFeatureCreate); + useEvent('map:feature:update', onFeatureUpdate); + useEvent('map:feature:delete', onFeatureDelete); +} + +function useCadastres( + featureCollection, + { + addEventListeners, + hoverFeature, + createFeatures, + deleteFeatures, + enabled = true + } +) { + const hoveredFeature = useRef(); + + const onMouseMove = useCallback( + (event) => { + if (event.features.length > 0) { + const feature = event.features[0]; + if (hoveredFeature.current?.id != feature.id) { + if (hoveredFeature.current) { + hoverFeature(hoveredFeature.current, false); + } + hoveredFeature.current = feature; + hoverFeature(feature, true); + } + } + }, + [hoverFeature] + ); + + const onMouseLeave = useCallback(() => { + if (hoveredFeature.current) { + hoverFeature(hoveredFeature.current, false); + } + hoveredFeature.current = null; + }, [hoverFeature]); + + const onClick = useCallback( + async (event) => { + if (event.features.length > 0) { + const currentId = event.features[0].properties.id; + const feature = findFeature( + filterFeatureCollection(featureCollection, SOURCE_CADASTRE), + currentId, + 'cid' + ); + if (feature) { + deleteFeatures({ + features: [feature], + source: SOURCE_CADASTRE, + external: true + }); + } else { + createFeatures({ + features: event.features, + source: SOURCE_CADASTRE, + external: true + }); + } + } + }, + [featureCollection, createFeatures, deleteFeatures] + ); + + useEffect(() => { + if (enabled) { + return addEventListeners({ + click: ['parcelles-fill', onClick], + mousemove: ['parcelles-fill', onMouseMove], + mouseleave: ['parcelles-fill', onMouseLeave] + }); + } + }, [onClick, onMouseMove, onMouseLeave, enabled]); +} + +function useError() { + const [error, onError] = useState(); + useEffect(() => { + const timer = setTimeout(() => onError(null), 5000); + return () => clearTimeout(timer); + }, [error]); + + return [error, onError]; +} + +export function useEvent(eventName, callback) { + return useEffect(() => { + addEventListener(eventName, callback); + return () => removeEventListener(eventName, callback); + }, [eventName, callback]); +} From 1b0cc62fc243ef324599e936bdb98fc8b1341c6c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 6 May 2021 18:52:02 +0200 Subject: [PATCH 29/37] Add cadastres to MapReader --- app/javascript/components/MapReader/index.jsx | 318 +++++++++--------- .../components/MapReader/useMapbox.js | 104 ++++++ 2 files changed, 255 insertions(+), 167 deletions(-) create mode 100644 app/javascript/components/MapReader/useMapbox.js diff --git a/app/javascript/components/MapReader/index.jsx b/app/javascript/components/MapReader/index.jsx index 9a14202cb..c976464f8 100644 --- a/app/javascript/components/MapReader/index.jsx +++ b/app/javascript/components/MapReader/index.jsx @@ -1,141 +1,28 @@ -import React, { useState, useCallback, useMemo } from 'react'; -import { ZoomControl, GeoJSONLayer } from 'react-mapbox-gl'; -import mapboxgl, { Popup } from 'mapbox-gl'; +import React, { useMemo } from 'react'; +import ReactMapboxGl, { ZoomControl, GeoJSONLayer } from 'react-mapbox-gl'; import PropTypes from 'prop-types'; +import 'mapbox-gl/dist/mapbox-gl.css'; -import Mapbox from '../shared/mapbox/Mapbox'; -import { getMapStyle } from '../shared/mapbox/styles'; -import SwitchMapStyle from '../shared/mapbox/SwitchMapStyle'; +import MapStyleControl, { useMapStyle } from '../shared/mapbox/MapStyleControl'; import { filterFeatureCollection, - filterFeatureCollectionByGeometryType, - useEvent, - findFeature, - fitBounds, - getCenter + filterFeatureCollectionByGeometryType } from '../shared/mapbox/utils'; +import { useMapbox } from './useMapbox'; + +const Mapbox = ReactMapboxGl({}); const MapReader = ({ featureCollection, options }) => { - const [currentMap, setCurrentMap] = useState(null); - const [style, setStyle] = useState('ortho'); - const cadastresFeatureCollection = useMemo( - () => filterFeatureCollection(featureCollection, 'cadastre'), - [featureCollection] - ); - const selectionsUtilisateurFeatureCollection = useMemo( - () => filterFeatureCollection(featureCollection, 'selection_utilisateur'), - [featureCollection] - ); - const selectionsLineFeatureCollection = useMemo( - () => - filterFeatureCollectionByGeometryType( - selectionsUtilisateurFeatureCollection, - 'LineString' - ), - [selectionsUtilisateurFeatureCollection] - ); - const selectionsPolygonFeatureCollection = useMemo( - () => - filterFeatureCollectionByGeometryType( - selectionsUtilisateurFeatureCollection, - 'Polygon' - ), - [selectionsUtilisateurFeatureCollection] - ); - const selectionsPointFeatureCollection = useMemo( - () => - filterFeatureCollectionByGeometryType( - selectionsUtilisateurFeatureCollection, - 'Point' - ), - [selectionsUtilisateurFeatureCollection] - ); - const hasCadastres = useMemo(() => options.layers.includes('cadastres')); - const mapStyle = useMemo(() => getMapStyle(style, options.layers), [ - style, - options - ]); - const popup = useMemo( - () => - new Popup({ - closeButton: false, - closeOnClick: false - }) - ); + const { + isSupported, + onLoad, + onStyleChange, + onMouseEnter, + onMouseLeave + } = useMapbox(featureCollection); + const [style, setStyle] = useMapStyle(options.layers, { onStyleChange }); - const onMouseEnter = useCallback( - (event) => { - const feature = event.features[0]; - if (feature.properties && feature.properties.description) { - const coordinates = getCenter(feature.geometry, event.lngLat); - const description = feature.properties.description; - currentMap.getCanvas().style.cursor = 'pointer'; - popup.setLngLat(coordinates).setHTML(description).addTo(currentMap); - } else { - popup.remove(); - } - }, - [currentMap, popup] - ); - - const onMouseLeave = useCallback(() => { - currentMap.getCanvas().style.cursor = ''; - popup.remove(); - }, [currentMap, popup]); - - const onFeatureFocus = useCallback( - ({ detail }) => { - const feature = findFeature(featureCollection, detail.id); - if (feature) { - fitBounds(currentMap, feature); - } - }, - [currentMap, featureCollection] - ); - - useEvent('map:feature:focus', onFeatureFocus); - - const [a1, a2, b1, b2] = featureCollection.bbox; - const boundData = [ - [a1, a2], - [b1, b2] - ]; - - const polygonSelectionFill = { - 'fill-color': '#EC3323', - 'fill-opacity': 0.5 - }; - - const polygonSelectionLine = { - 'line-color': 'rgba(255, 0, 0, 1)', - 'line-width': 4 - }; - - const lineStringSelectionLine = { - 'line-color': 'rgba(55, 42, 127, 1.00)', - 'line-width': 3 - }; - - const pointSelectionFill = { - 'circle-color': '#EC3323' - }; - - const polygonCadastresFill = { - 'fill-color': '#FAD859', - 'fill-opacity': 0.5 - }; - - const polygonCadastresLine = { - 'line-color': 'rgba(156, 160, 144, 255)', - 'line-width': 2, - 'line-dasharray': [1, 1] - }; - - function onMapLoad(map) { - setCurrentMap(map); - } - - if (!mapboxgl.supported()) { + if (!isSupported) { return (

Nous ne pouvons pas afficher la carte car elle est imcompatible avec @@ -147,58 +34,155 @@ const MapReader = ({ featureCollection, options }) => { return ( onMapLoad(map)} - fitBounds={boundData} - fitBoundsOptions={{ padding: 100 }} - style={mapStyle} - containerStyle={{ - height: '400px', - width: '100%' - }} + onStyleLoad={(map) => onLoad(map)} + style={style} + containerStyle={{ height: '400px' }} > - - - - {hasCadastres ? ( - - ) : null} - + ); }; -MapReader.propTypes = { +const polygonSelectionFill = { + 'fill-color': '#EC3323', + 'fill-opacity': 0.5 +}; +const polygonSelectionLine = { + 'line-color': 'rgba(255, 0, 0, 1)', + 'line-width': 4 +}; +const lineStringSelectionLine = { + 'line-color': 'rgba(55, 42, 127, 1.00)', + 'line-width': 3 +}; +const pointSelectionFill = { + 'circle-color': '#EC3323' +}; + +function SelectionUtilisateurPolygonLayer({ + featureCollection, + onMouseEnter, + onMouseLeave +}) { + const data = useMemo( + () => + filterFeatureCollectionByGeometryType( + filterFeatureCollection(featureCollection, 'selection_utilisateur'), + 'Polygon' + ), + [featureCollection] + ); + + return ( + + ); +} + +function SelectionUtilisateurLineLayer({ + featureCollection, + onMouseEnter, + onMouseLeave +}) { + const data = useMemo( + () => + filterFeatureCollectionByGeometryType( + filterFeatureCollection(featureCollection, 'selection_utilisateur'), + 'LineString' + ), + [featureCollection] + ); + return ( + + ); +} + +function SelectionUtilisateurPointLayer({ + featureCollection, + onMouseEnter, + onMouseLeave +}) { + const data = useMemo( + () => + filterFeatureCollectionByGeometryType( + filterFeatureCollection(featureCollection, 'selection_utilisateur'), + 'Point' + ), + [featureCollection] + ); + return ( + + ); +} + +SelectionUtilisateurPolygonLayer.propTypes = { featureCollection: PropTypes.shape({ type: PropTypes.string, bbox: PropTypes.array, features: PropTypes.array }), - options: PropTypes.shape({ - layers: PropTypes.array, - ign: PropTypes.bool - }) + onMouseEnter: PropTypes.func, + onMouseLeave: PropTypes.func +}; + +SelectionUtilisateurLineLayer.propTypes = { + featureCollection: PropTypes.shape({ + type: PropTypes.string, + bbox: PropTypes.array, + features: PropTypes.array + }), + onMouseEnter: PropTypes.func, + onMouseLeave: PropTypes.func +}; + +SelectionUtilisateurPointLayer.propTypes = { + featureCollection: PropTypes.shape({ + type: PropTypes.string, + bbox: PropTypes.array, + features: PropTypes.array + }), + onMouseEnter: PropTypes.func, + onMouseLeave: PropTypes.func +}; + +MapReader.propTypes = { + featureCollection: PropTypes.shape({ + bbox: PropTypes.array, + features: PropTypes.array + }), + options: PropTypes.shape({ layers: PropTypes.array }) }; export default MapReader; diff --git a/app/javascript/components/MapReader/useMapbox.js b/app/javascript/components/MapReader/useMapbox.js new file mode 100644 index 000000000..d58b4e463 --- /dev/null +++ b/app/javascript/components/MapReader/useMapbox.js @@ -0,0 +1,104 @@ +import { useCallback, useRef, useEffect, useMemo } from 'react'; +import mapboxgl, { Popup } from 'mapbox-gl'; + +import { + filterFeatureCollection, + findFeature, + getBounds, + getCenter +} from '../shared/mapbox/utils'; + +const SOURCE_CADASTRE = 'cadastre'; + +export function useMapbox(featureCollection) { + const mapRef = useRef(); + const selectedCadastresRef = useRef(() => new Set()); + const isSupported = useMemo(() => mapboxgl.supported()); + + const fitBounds = useCallback((bbox) => { + mapRef.current.fitBounds(bbox, { padding: 100 }); + }, []); + + const onLoad = useCallback( + (map) => { + if (!mapRef.current) { + mapRef.current = map; + mapRef.current.fitBounds(featureCollection.bbox, { padding: 100 }); + onStyleChange(); + } + }, + [featureCollection] + ); + + const onStyleChange = useCallback(() => { + if (mapRef.current) { + selectedCadastresRef.current = new Set( + filterFeatureCollection( + featureCollection, + SOURCE_CADASTRE + ).features.map(({ properties }) => properties.cid) + ); + if (selectedCadastresRef.current.size > 0) { + mapRef.current.setFilter('parcelle-highlighted', [ + 'in', + 'id', + ...selectedCadastresRef.current + ]); + } + } + }, [featureCollection]); + + const popup = useMemo( + () => + new Popup({ + closeButton: false, + closeOnClick: false + }) + ); + + const onMouseEnter = useCallback( + (event) => { + const feature = event.features[0]; + if (feature.properties && feature.properties.description) { + const coordinates = getCenter(feature.geometry, event.lngLat); + const description = feature.properties.description; + mapRef.current.getCanvas().style.cursor = 'pointer'; + popup.setLngLat(coordinates).setHTML(description).addTo(mapRef.current); + } else { + popup.remove(); + } + }, + [popup] + ); + + const onMouseLeave = useCallback(() => { + mapRef.current.getCanvas().style.cursor = ''; + popup.remove(); + }, [popup]); + + useExternalEvents(featureCollection, { fitBounds }); + + return { isSupported, onLoad, onStyleChange, onMouseEnter, onMouseLeave }; +} + +function useExternalEvents(featureCollection, { fitBounds }) { + const onFeatureFocus = useCallback( + ({ detail }) => { + const { id } = detail; + const feature = findFeature(featureCollection, id); + if (feature) { + fitBounds(getBounds(feature.geometry)); + } + }, + [featureCollection, fitBounds] + ); + + useEvent('map:feature:focus', onFeatureFocus); +} + +export function useEvent(eventName, callback) { + return useEffect(() => { + addEventListener(eventName, callback); + return () => removeEventListener(eventName, callback); + }, [eventName, callback]); +} From 55080706ce5005e06805fbb662ce6edc5c48742d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 13 May 2021 11:48:41 +0200 Subject: [PATCH 30/37] Convert geo_areas properties to jsonb --- ...316_use_jsonb_in_geo_areas_properties.rake | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 lib/tasks/deployment/20210513093316_use_jsonb_in_geo_areas_properties.rake diff --git a/lib/tasks/deployment/20210513093316_use_jsonb_in_geo_areas_properties.rake b/lib/tasks/deployment/20210513093316_use_jsonb_in_geo_areas_properties.rake new file mode 100644 index 000000000..0f075cb6d --- /dev/null +++ b/lib/tasks/deployment/20210513093316_use_jsonb_in_geo_areas_properties.rake @@ -0,0 +1,22 @@ +namespace :after_party do + desc 'Deployment task: use_jsonb_in_geo_areas_properties' + task use_jsonb_in_geo_areas_properties: :environment do + puts "Running deploy task 'use_jsonb_in_geo_areas_properties'" + + geo_areas = GeoArea.where("properties::text LIKE ?", "%--- !ruby%") + progress = ProgressReport.new(geo_areas.count) + geo_areas.find_each do |geo_area| + geo_area.properties = geo_area.properties + if !geo_area.save + geo_area.destroy + end + progress.inc + end + progress.finish + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end From c5f2faa3d2e45882a4fccd8a1a6f7c5683d42f0a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 13 May 2021 12:46:42 +0200 Subject: [PATCH 31/37] add tests for backward compatibility of geo_areas --- spec/factories/geo_area.rb | 7 ++++++- spec/models/geo_area_spec.rb | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/spec/factories/geo_area.rb b/spec/factories/geo_area.rb index 5a5909be7..3c4e428e5 100644 --- a/spec/factories/geo_area.rb +++ b/spec/factories/geo_area.rb @@ -5,7 +5,12 @@ FactoryBot.define do trait :cadastre do source { GeoArea.sources.fetch(:cadastre) } - properties { { numero: '42', section: 'A11', commune: '75127' } } + properties { { numero: '42', section: 'A11', prefixe: '000', commune: '75127', contenance: '1234', id: '75127000A1142' } } + end + + trait :legacy_cadastre do + source { GeoArea.sources.fetch(:cadastre) } + properties { { numero: '42', section: 'A11', code_com: '127', code_dep: '75', code_arr: '000', surface_parcelle: '1234', surface_intersection: 1234 } } end trait :selection_utilisateur do diff --git a/spec/models/geo_area_spec.rb b/spec/models/geo_area_spec.rb index 0279ce746..ac3e1aab2 100644 --- a/spec/models/geo_area_spec.rb +++ b/spec/models/geo_area_spec.rb @@ -80,4 +80,24 @@ RSpec.describe GeoArea, type: :model do it { expect(geo_area.valid?).to be_falsey } end end + + describe "cadastre properties" do + let(:geo_area) { build(:geo_area, :cadastre) } + let(:legacy_geo_area) { build(:geo_area, :legacy_cadastre) } + + it "should be backward compatible" do + expect("#{geo_area.code_dep}#{geo_area.code_com}").to eq(geo_area.commune) + expect(geo_area.code_arr).to eq(geo_area.prefixe) + expect(geo_area.surface_parcelle).to eq(geo_area.surface) + end + + context "(legacy)" do + it "should be forward compatible" do + expect("#{legacy_geo_area.code_dep}#{legacy_geo_area.code_com}").to eq(legacy_geo_area.commune) + expect(legacy_geo_area.code_arr).to eq(legacy_geo_area.prefixe) + expect(legacy_geo_area.surface_parcelle).to eq(legacy_geo_area.surface) + expect(legacy_geo_area.cid).to eq(geo_area.cid) + end + end + end end From 84d5c95a0daa6a9586ea075b515f5c3d7cd90085 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 25 May 2021 07:58:22 +0000 Subject: [PATCH 32/37] Bump browserslist from 4.12.0 to 4.16.6 Bumps [browserslist](https://github.com/browserslist/browserslist) from 4.12.0 to 4.16.6. - [Release notes](https://github.com/browserslist/browserslist/releases) - [Changelog](https://github.com/browserslist/browserslist/blob/main/CHANGELOG.md) - [Commits](https://github.com/browserslist/browserslist/compare/4.12.0...4.16.6) Signed-off-by: dependabot[bot] --- yarn.lock | 90 +++++++++++++++++++------------------------------------ 1 file changed, 30 insertions(+), 60 deletions(-) diff --git a/yarn.lock b/yarn.lock index 9259fad90..aed914713 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2925,14 +2925,15 @@ browserify-zlib@^0.2.0: pako "~1.0.5" browserslist@^4.0.0, browserslist@^4.11.1, browserslist@^4.6.4, browserslist@^4.8.5: - version "4.12.0" - resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.12.0.tgz#06c6d5715a1ede6c51fc39ff67fd647f740b656d" - integrity sha512-UH2GkcEDSI0k/lRkuDSzFl9ZZ87skSy9w2XAn1MsZnL+4c4rqbBd3e82UWHbYDpztABrPBhZsTEeuxVfHppqDg== + version "4.16.6" + resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.16.6.tgz#d7901277a5a88e554ed305b183ec9b0c08f66fa2" + integrity sha512-Wspk/PqO+4W9qp5iUTJsa1B/QrYn1keNCcEP5OvP7WBwT4KaDly0uONYmC6Xa3Z5IqnUgS0KcgLYu1l74x0ZXQ== dependencies: - caniuse-lite "^1.0.30001043" - electron-to-chromium "^1.3.413" - node-releases "^1.1.53" - pkg-up "^2.0.0" + caniuse-lite "^1.0.30001219" + colorette "^1.2.2" + electron-to-chromium "^1.3.723" + escalade "^3.1.1" + node-releases "^1.1.71" btoa-lite@^1.0.0: version "1.0.0" @@ -3175,10 +3176,10 @@ caniuse-api@^3.0.0: lodash.memoize "^4.1.2" lodash.uniq "^4.5.0" -caniuse-lite@^1.0.0, caniuse-lite@^1.0.30000981, caniuse-lite@^1.0.30001039, caniuse-lite@^1.0.30001043: - version "1.0.30001055" - resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001055.tgz#7b52c3537f7a8c0408aca867e83d2b04268b54cd" - integrity sha512-MbwsBmKrBSKIWldfdIagO5OJWZclpJtS4h0Jrk/4HFrXJxTdVdH23Fd+xCiHriVGvYcWyW8mR/CPsYajlH8Iuw== +caniuse-lite@^1.0.0, caniuse-lite@^1.0.30000981, caniuse-lite@^1.0.30001039, caniuse-lite@^1.0.30001219: + version "1.0.30001228" + resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001228.tgz#bfdc5942cd3326fa51ee0b42fbef4da9d492a7fa" + integrity sha512-QQmLOGJ3DEgokHbMSA8cj2a+geXqmnpyOFT0lhQV6P3/YOJvGDEwoedcwxEQ30gJIwIIunHIicunJ2rzK5gB2A== cardinal@^2.1.1: version "2.1.1" @@ -3582,6 +3583,11 @@ color@^3.0.0: color-convert "^1.9.1" color-string "^1.5.2" +colorette@^1.2.2: + version "1.2.2" + resolved "https://registry.yarnpkg.com/colorette/-/colorette-1.2.2.tgz#cbcc79d5e99caea2dbf10eb3a26fd8b3e6acfa94" + integrity sha512-MKGMzyfeuutC/ZJ1cba9NqcNpfeqMUcYmyF1ZFY6/Cn7CNSAKx6a+s48sqLqyAiZuaP2TcqMhoo+dlwFnVxT9w== + colors@^1.1.2, colors@^1.2.1: version "1.4.0" resolved "https://registry.yarnpkg.com/colors/-/colors-1.4.0.tgz#c50491479d4c1bdaed2c9ced32cf7c7dc2360f78" @@ -4720,10 +4726,10 @@ ejs@^2.6.1: resolved "https://registry.yarnpkg.com/ejs/-/ejs-2.7.4.tgz#48661287573dcc53e366c7a1ae52c3a120eec9ba" integrity sha512-7vmuyh5+kuUyJKePhQfRQBhXV5Ce+RnaeeQArKu1EAMpL3WbgMt5WG6uQZpEVvYSSsxMXRKOewtDk9RaTKXRlA== -electron-to-chromium@^1.3.413: - version "1.3.435" - resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.435.tgz#22a7008e8f5a317a6d2d80802bddacebb19ae025" - integrity sha512-BVXnq+NCefidU7GOFPx4CPBfPcccLCRBKZYSbvBJMSn2kwGD7ML+eUA9tqfHAumRqy3oX5zaeTI1Bpt7qVat0Q== +electron-to-chromium@^1.3.723: + version "1.3.737" + resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.737.tgz#196f2e9656f4f3c31930750e1899c091b72d36b5" + integrity sha512-P/B84AgUSQXaum7a8m11HUsYL8tj9h/Pt5f7Hg7Ty6bm5DxlFq+e5+ouHUoNQMsKDJ7u4yGfI8mOErCmSH9wyg== elf-tools@^1.1.1: version "1.1.2" @@ -4915,6 +4921,11 @@ es-to-primitive@^1.2.1: is-date-object "^1.0.1" is-symbol "^1.0.2" +escalade@^3.1.1: + version "3.1.1" + resolved "https://registry.yarnpkg.com/escalade/-/escalade-3.1.1.tgz#d8cfdc7000965c5a0174b4a82eaa5c0552742e40" + integrity sha512-k0er2gUkLf8O0zKJiAhmkTnJlTvINGv7ygDNPbeIsX/TJjGJZHuh9B2UxbsaEkmlEo9MfhrSzmhIlhRlI2GXnw== + escape-goat@^2.0.0: version "2.1.1" resolved "https://registry.yarnpkg.com/escape-goat/-/escape-goat-2.1.1.tgz#1b2dc77003676c457ec760b2dc68edb648188675" @@ -5547,13 +5558,6 @@ find-up@^1.0.0: path-exists "^2.0.0" pinkie-promise "^2.0.0" -find-up@^2.1.0: - version "2.1.0" - resolved "https://registry.yarnpkg.com/find-up/-/find-up-2.1.0.tgz#45d1b7e506c717ddd482775a2b77920a3c0c57a7" - integrity sha1-RdG35QbHF93UgndaK3eSCjwMV6c= - dependencies: - locate-path "^2.0.0" - find-up@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/find-up/-/find-up-3.0.0.tgz#49169f1d7993430646da61ecc5ae355c21c97b73" @@ -7593,14 +7597,6 @@ loader-utils@^2.0.0: emojis-list "^3.0.0" json5 "^2.1.2" -locate-path@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-2.0.0.tgz#2b568b265eec944c6d9c0de9c3dbbbca0354cd8e" - integrity sha1-K1aLJl7slExtnA3pw9u7ygNUzY4= - dependencies: - p-locate "^2.0.0" - path-exists "^3.0.0" - locate-path@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-3.0.0.tgz#dbec3b3ab759758071b58fe59fc41871af21400e" @@ -8577,10 +8573,10 @@ node-libs-browser@^2.2.1: util "^0.11.0" vm-browserify "^1.0.1" -node-releases@^1.1.53: - version "1.1.55" - resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.55.tgz#8af23b7c561d8e2e6e36a46637bab84633b07cee" - integrity sha512-H3R3YR/8TjT5WPin/wOoHOUPHgvj8leuU/Keta/rwelEQN9pA/S2Dx8/se4pZ2LBxSd0nAGzsNzhqwa77v7F1w== +node-releases@^1.1.71: + version "1.1.72" + resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.72.tgz#14802ab6b1039a79a0c7d662b610a5bbd76eacbe" + integrity sha512-LLUo+PpH3dU6XizX3iVoubUNheF/owjXCZZ5yACDxNnPtgFuludV1ZL3ayK1kVep42Rmm0+R9/Y60NQbZ2bifw== node-sass@^4.13.1: version "4.14.1" @@ -9093,13 +9089,6 @@ p-is-promise@^2.0.0: resolved "https://registry.yarnpkg.com/p-is-promise/-/p-is-promise-2.1.0.tgz#918cebaea248a62cf7ffab8e3bca8c5f882fc42e" integrity sha512-Y3W0wlRPK8ZMRbNq97l4M5otioeA5lm1z7bkNkxCka8HSPjR0xRWmpCmc9utiaLP9Jb1eD8BgeIxTW4AIF45Pg== -p-limit@^1.1.0: - version "1.3.0" - resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-1.3.0.tgz#b86bd5f0c25690911c7590fcbfc2010d54b3ccb8" - integrity sha512-vvcXsLAJ9Dr5rQOPk7toZQZJApBl2K4J6dANSsEuh6QI41JYcsS/qhTGa9ErIUUgK3WNQoJYvylxvjqmiqEA9Q== - dependencies: - p-try "^1.0.0" - p-limit@^2.0.0, p-limit@^2.2.0, p-limit@^2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-2.3.0.tgz#3dd33c647a214fdfffd835933eb086da0dc21db1" @@ -9107,13 +9096,6 @@ p-limit@^2.0.0, p-limit@^2.2.0, p-limit@^2.3.0: dependencies: p-try "^2.0.0" -p-locate@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-2.0.0.tgz#20a0103b222a70c8fd39cc2e580680f3dde5ec43" - integrity sha1-IKAQOyIqcMj9OcwuWAaA893l7EM= - dependencies: - p-limit "^1.1.0" - p-locate@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-3.0.0.tgz#322d69a05c0264b25997d9f40cd8a891ab0064a4" @@ -9166,11 +9148,6 @@ p-timeout@^3.0.0, p-timeout@^3.1.0: dependencies: p-finally "^1.0.0" -p-try@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/p-try/-/p-try-1.0.0.tgz#cbc79cdbaf8fd4228e13f621f2b1a237c1b207b3" - integrity sha1-y8ec26+P1CKOE/Yh8rGiN8GyB7M= - p-try@^2.0.0: version "2.2.0" resolved "https://registry.yarnpkg.com/p-try/-/p-try-2.2.0.tgz#cb2868540e313d61de58fafbe35ce9004d5540e6" @@ -9447,13 +9424,6 @@ pkg-dir@^4.1.0, pkg-dir@^4.2.0: dependencies: find-up "^4.0.0" -pkg-up@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/pkg-up/-/pkg-up-2.0.0.tgz#c819ac728059a461cab1c3889a2be3c49a004d7f" - integrity sha1-yBmscoBZpGHKscOImivjxJoATX8= - dependencies: - find-up "^2.1.0" - pnp-webpack-plugin@^1.6.4: version "1.6.4" resolved "https://registry.yarnpkg.com/pnp-webpack-plugin/-/pnp-webpack-plugin-1.6.4.tgz#c9711ac4dc48a685dabafc86f8b6dd9f8df84149" From d93342e1d7122492b39378f16f2a36146f906aba Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 11 May 2021 14:44:18 +0200 Subject: [PATCH 33/37] config: cleanup allowed tags after Rails 6.1 migration --- config/application.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/config/application.rb b/config/application.rb index 62cc0206e..f5ab729ae 100644 --- a/config/application.rb +++ b/config/application.rb @@ -38,12 +38,7 @@ module TPS config.assets.paths << Rails.root.join('app', 'assets', 'fonts') config.assets.precompile += ['.woff'] - # The default list used to be accessible through `ActionView::Base.sanitized_allowed_tags`, - # but a regression in Rails 6.0 makes it unavailable. - # It should be fixed in Rails 6.1. - # See https://github.com/rails/rails/issues/39586 - # default_allowed_tags = ActionView::Base.sanitized_allowed_tags - default_allowed_tags = ['strong', 'em', 'b', 'i', 'p', 'code', 'pre', 'tt', 'samp', 'kbd', 'var', 'sub', 'sup', 'dfn', 'cite', 'big', 'small', 'address', 'hr', 'br', 'div', 'span', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ul', 'ol', 'li', 'dl', 'dt', 'dd', 'abbr', 'acronym', 'a', 'img', 'blockquote', 'del', 'ins'] + default_allowed_tags = ActionView::Base.sanitized_allowed_tags config.action_view.sanitized_allowed_tags = default_allowed_tags + ['u'] # Some mobile browsers have a behaviour where, although they will delete the session From 4b6196e6f62f68fb5599865120d9c2325aa34a51 Mon Sep 17 00:00:00 2001 From: kara Diaby Date: Tue, 25 May 2021 10:54:04 +0200 Subject: [PATCH 34/37] verify avis privacy --- spec/features/experts/expert_spec.rb | 46 +++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/spec/features/experts/expert_spec.rb b/spec/features/experts/expert_spec.rb index df81c2bd5..6c2f45c05 100644 --- a/spec/features/experts/expert_spec.rb +++ b/spec/features/experts/expert_spec.rb @@ -67,11 +67,49 @@ feature 'Inviting an expert:' do expect(page).to have_text('1 avis donné') end - # TODO - # scenario 'I can read other experts advices' do - # end - # scenario 'I can invite other experts' do # end end + + context 'when there are two experts' do + let(:expert_1) { create(:expert) } + let(:expert_2) { create(:expert) } + let(:instructeur) { create(:instructeur) } + let(:procedure) { create(:procedure, :published, instructeurs: [instructeur]) } + let(:experts_procedure_1) { create(:experts_procedure, expert: expert_1, procedure: procedure) } + let(:experts_procedure_2) { create(:experts_procedure, expert: expert_2, procedure: procedure) } + let(:dossier) { create(:dossier, :en_construction, :with_dossier_link, procedure: procedure) } + let!(:avis_1) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure_1, confidentiel: true) } + let!(:avis_2) { create(:avis, dossier: dossier, claimant: instructeur, experts_procedure: experts_procedure_2, confidentiel: false) } + + scenario 'As a expert_1, I can read expert_2 advice because it is not confidential' do + login_as expert_1.user, scope: :user + + visit expert_all_avis_path + expect(page).to have_text('1 avis à donner') + expect(page).to have_text('0 avis donnés') + + click_on '1 avis à donner' + click_on avis_1.dossier.user.email + within('.tabs') { click_on 'Avis' } + expect(page).to have_text("Demandeur : #{avis_1.claimant.email}") + expect(page).to have_text("Vous") + expect(page).to have_text(avis_2.expert.email.to_s) + end + + scenario 'As a expert_2, I cannot read expert_1 advice because it is confidential' do + login_as expert_2.user, scope: :user + + visit expert_all_avis_path + expect(page).to have_text('1 avis à donner') + expect(page).to have_text('0 avis donnés') + + click_on '1 avis à donner' + click_on avis_2.dossier.user.email + within('.tabs') { click_on 'Avis' } + expect(page).to have_text("Demandeur : #{avis_2.claimant.email}") + expect(page).to have_text("Vous") + expect(page).not_to have_text(avis_1.expert.email.to_s) + end + end end From acb277e6501fab22a13bd1173be723ce51a5199f Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 21 Jan 2021 14:11:17 +0100 Subject: [PATCH 35/37] Use stable_id when replacing tags --- .../concerns/tags_substitution_concern.rb | 33 +++++++++++++++---- app/models/procedure.rb | 24 ++++++++++++++ .../linked_drop_down_list_type_de_champ.rb | 12 +++---- .../types_de_champ/type_de_champ_base.rb | 5 +-- .../concern/tags_substitution_concern_spec.rb | 28 ++++++++++++++++ 5 files changed, 86 insertions(+), 16 deletions(-) diff --git a/app/models/concerns/tags_substitution_concern.rb b/app/models/concerns/tags_substitution_concern.rb index cb2464c93..e562cfa8c 100644 --- a/app/models/concerns/tags_substitution_concern.rb +++ b/app/models/concerns/tags_substitution_concern.rb @@ -196,12 +196,14 @@ module TagsSubstitutionConcern tags.filter { |tag| tag[:available_for_states].include?(self.class::DOSSIER_STATE) } end - def champ_public_tags - types_de_champ_tags(procedure.types_de_champ, Dossier::SOUMIS) + def champ_public_tags(dossier: nil) + types_de_champ = (dossier ? dossier : procedure.active_revision).types_de_champ + types_de_champ_tags(types_de_champ, Dossier::SOUMIS) end - def champ_private_tags - types_de_champ_tags(procedure.types_de_champ_private, Dossier::INSTRUCTION_COMMENCEE) + def champ_private_tags(dossier: nil) + types_de_champ = (dossier ? dossier : procedure.active_revision).types_de_champ_private + types_de_champ_tags(types_de_champ, Dossier::INSTRUCTION_COMMENCEE) end def types_de_champ_tags(types_de_champ, available_for_states) @@ -217,9 +219,11 @@ module TagsSubstitutionConcern return '' end + text = normalize_tags(text) + tags_and_datas = [ - [champ_public_tags, dossier.champs], - [champ_private_tags, dossier.champs_private], + [champ_public_tags(dossier: dossier), dossier.champs], + [champ_private_tags(dossier: dossier), dossier.champs_private], [dossier_tags, dossier], [ROUTAGE_TAGS, dossier], [INDIVIDUAL_TAGS, dossier.individual], @@ -242,7 +246,7 @@ module TagsSubstitutionConcern end def replace_tag(text, tag, data) - libelle = Regexp.quote(tag[:libelle]) + libelle = Regexp.quote(tag[:id] ? tag[:id] : tag[:libelle]) # allow any kind of space (non-breaking or other) in the tag’s libellé to match any kind of space in the template # (the '\\ |' is there because plain ASCII spaces were escaped by preceding Regexp.quote) @@ -256,4 +260,19 @@ module TagsSubstitutionConcern text.gsub(/--#{libelle}--/, value.to_s) end + + def normalize_tags(text) + tags = types_de_champ_tags(procedure.types_de_champ_for_tags, Dossier::SOUMIS) + types_de_champ_tags(procedure.types_de_champ_private_for_tags, Dossier::INSTRUCTION_COMMENCEE) + filter_tags(tags).reduce(text) { |text, tag| normalize_tag(text, tag) } + end + + def normalize_tag(text, tag) + libelle = Regexp.quote(tag[:libelle]) + + # allow any kind of space (non-breaking or other) in the tag’s libellé to match any kind of space in the template + # (the '\\ |' is there because plain ASCII spaces were escaped by preceding Regexp.quote) + libelle.gsub!(/\\ |[[:blank:]]/, "[[:blank:]]") + + text.gsub(/--#{libelle}--/, "--#{tag[:id]}--") + end end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index 0e6a11783..27ceb25c5 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -87,6 +87,30 @@ class Procedure < ApplicationRecord brouillon? ? draft_types_de_champ_private : published_types_de_champ_private end + def types_de_champ_for_tags + if brouillon? + draft_types_de_champ + else + TypeDeChamp.root + .public_only + .where(revision: revisions - [draft_revision]) + .order(:created_at) + .uniq + end + end + + def types_de_champ_private_for_tags + if brouillon? + draft_types_de_champ_private + else + TypeDeChamp.root + .private_only + .where(revision: revisions - [draft_revision]) + .order(:created_at) + .uniq + end + end + def types_de_champ_for_export types_de_champ.reject(&:exclude_from_export?) end diff --git a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb index ca6817c56..5b903e666 100644 --- a/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb +++ b/app/models/types_de_champ/linked_drop_down_list_type_de_champ.rb @@ -6,26 +6,24 @@ class TypesDeChamp::LinkedDropDownListTypeDeChamp < TypesDeChamp::TypeDeChampBas def tags_for_template tags = super - tdc = @type_de_champ + stable_id = @type_de_champ.stable_id tags.push( { libelle: "#{libelle}/primaire", + id: "tdc#{stable_id}/primaire", description: "#{description} (menu primaire)", lambda: -> (champs) { - champs - .find { |champ| champ.type_de_champ == tdc } - &.primary_value + champs.find { |champ| champ.stable_id == stable_id }&.primary_value } } ) tags.push( { libelle: "#{libelle}/secondaire", + id: "tdc#{stable_id}/secondaire", description: "#{description} (menu secondaire)", lambda: -> (champs) { - champs - .find { |champ| champ.type_de_champ == tdc } - &.secondary_value + champs.find { |champ| champ.stable_id == stable_id }&.secondary_value } } ) diff --git a/app/models/types_de_champ/type_de_champ_base.rb b/app/models/types_de_champ/type_de_champ_base.rb index b9d2f3f56..10dbc74fb 100644 --- a/app/models/types_de_champ/type_de_champ_base.rb +++ b/app/models/types_de_champ/type_de_champ_base.rb @@ -8,13 +8,14 @@ class TypesDeChamp::TypeDeChampBase end def tags_for_template - tdc = @type_de_champ + stable_id = @type_de_champ.stable_id [ { libelle: libelle, + id: "tdc#{stable_id}", description: description, lambda: -> (champs) { - champs.find { |champ| champ.type_de_champ == tdc }&.for_tag + champs.find { |champ| champ.stable_id == stable_id }&.for_tag } } ] diff --git a/spec/models/concern/tags_substitution_concern_spec.rb b/spec/models/concern/tags_substitution_concern_spec.rb index cbc914d98..3b5404541 100644 --- a/spec/models/concern/tags_substitution_concern_spec.rb +++ b/spec/models/concern/tags_substitution_concern_spec.rb @@ -8,6 +8,7 @@ describe TagsSubstitutionConcern, type: :model do let(:procedure) do create(:procedure, + :published, libelle: 'Une magnifique démarche', types_de_champ: types_de_champ, types_de_champ_private: types_de_champ_private, @@ -389,6 +390,33 @@ describe TagsSubstitutionConcern, type: :model do is_expected.to eq('--motivation-- --date de décision--') end end + + context 'when procedure has revisions' do + let(:types_de_champ) { [build(:type_de_champ, libelle: 'mon tag')] } + let(:draft_type_de_champ) { procedure.draft_revision.find_or_clone_type_de_champ(types_de_champ[0].stable_id) } + + before do + draft_type_de_champ.update(libelle: 'ton tag') + dossier.champs.first.update(value: 'valeur') + procedure.update!(draft_revision: procedure.create_new_revision, published_revision: procedure.draft_revision) + end + + context "replace by old label" do + let(:template) { '--mon tag--' } + + it "should replace tag" do + is_expected.to eq('valeur') + end + end + + context "replace by new label" do + let(:template) { '--ton tag--' } + + it "should replace tag" do + is_expected.to eq('valeur') + end + end + end end describe 'tags' do From 179bb5a9fee60900dabde43a7b73dfe404680858 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 May 2021 11:14:02 +0200 Subject: [PATCH 36/37] Apply suggestions from code review Co-authored-by: Pierre de La Morinerie --- app/models/concerns/tags_substitution_concern.rb | 6 +++--- spec/models/concern/tags_substitution_concern_spec.rb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/concerns/tags_substitution_concern.rb b/app/models/concerns/tags_substitution_concern.rb index e562cfa8c..e9ab51137 100644 --- a/app/models/concerns/tags_substitution_concern.rb +++ b/app/models/concerns/tags_substitution_concern.rb @@ -197,12 +197,12 @@ module TagsSubstitutionConcern end def champ_public_tags(dossier: nil) - types_de_champ = (dossier ? dossier : procedure.active_revision).types_de_champ + types_de_champ = (dossier || procedure.active_revision).types_de_champ types_de_champ_tags(types_de_champ, Dossier::SOUMIS) end def champ_private_tags(dossier: nil) - types_de_champ = (dossier ? dossier : procedure.active_revision).types_de_champ_private + types_de_champ = (dossier || procedure.active_revision).types_de_champ_private types_de_champ_tags(types_de_champ, Dossier::INSTRUCTION_COMMENCEE) end @@ -246,7 +246,7 @@ module TagsSubstitutionConcern end def replace_tag(text, tag, data) - libelle = Regexp.quote(tag[:id] ? tag[:id] : tag[:libelle]) + libelle = Regexp.quote(tag[:id].presence || tag[:libelle]) # allow any kind of space (non-breaking or other) in the tag’s libellé to match any kind of space in the template # (the '\\ |' is there because plain ASCII spaces were escaped by preceding Regexp.quote) diff --git a/spec/models/concern/tags_substitution_concern_spec.rb b/spec/models/concern/tags_substitution_concern_spec.rb index 3b5404541..c9c95ea9c 100644 --- a/spec/models/concern/tags_substitution_concern_spec.rb +++ b/spec/models/concern/tags_substitution_concern_spec.rb @@ -401,18 +401,18 @@ describe TagsSubstitutionConcern, type: :model do procedure.update!(draft_revision: procedure.create_new_revision, published_revision: procedure.draft_revision) end - context "replace by old label" do + context "when using the champ's original label" do let(:template) { '--mon tag--' } - it "should replace tag" do + it "replaces the tag" do is_expected.to eq('valeur') end end - context "replace by new label" do + context "when using the champ's revised label" do let(:template) { '--ton tag--' } - it "should replace tag" do + it "replaces the tag" do is_expected.to eq('valeur') end end From 21ee79669deabbb43d8affdd7a2a250c1f52ec93 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 May 2021 11:29:06 +0200 Subject: [PATCH 37/37] Better labels in test --- spec/models/concern/tags_substitution_concern_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/concern/tags_substitution_concern_spec.rb b/spec/models/concern/tags_substitution_concern_spec.rb index c9c95ea9c..a8854c45c 100644 --- a/spec/models/concern/tags_substitution_concern_spec.rb +++ b/spec/models/concern/tags_substitution_concern_spec.rb @@ -392,17 +392,17 @@ describe TagsSubstitutionConcern, type: :model do end context 'when procedure has revisions' do - let(:types_de_champ) { [build(:type_de_champ, libelle: 'mon tag')] } + let(:types_de_champ) { [build(:type_de_champ, libelle: 'mon ancien libellé')] } let(:draft_type_de_champ) { procedure.draft_revision.find_or_clone_type_de_champ(types_de_champ[0].stable_id) } before do - draft_type_de_champ.update(libelle: 'ton tag') + draft_type_de_champ.update(libelle: 'mon nouveau libellé') dossier.champs.first.update(value: 'valeur') procedure.update!(draft_revision: procedure.create_new_revision, published_revision: procedure.draft_revision) end context "when using the champ's original label" do - let(:template) { '--mon tag--' } + let(:template) { '--mon ancien libellé--' } it "replaces the tag" do is_expected.to eq('valeur') @@ -410,7 +410,7 @@ describe TagsSubstitutionConcern, type: :model do end context "when using the champ's revised label" do - let(:template) { '--ton tag--' } + let(:template) { '--mon nouveau libellé--' } it "replaces the tag" do is_expected.to eq('valeur')