From afa02373491887663476fa39f56f0bf1104d9cb6 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 16 Sep 2019 14:04:18 +0200 Subject: [PATCH 1/7] remove logo n+1 --- app/controllers/instructeurs/procedures_controller.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 6b04eb4ed..aa97b2de8 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -6,7 +6,10 @@ module Instructeurs ITEMS_PER_PAGE = 25 def index - @procedures = current_instructeur.visible_procedures.order(archived_at: :desc, published_at: :desc, created_at: :desc) + @procedures = current_instructeur + .visible_procedures + .includes(:logo_attachment, :logo_active_storage_attachment) + .order(archived_at: :desc, published_at: :desc, created_at: :desc) groupe_instructeurs = current_instructeur.groupe_instructeurs.where(procedure: @procedures) From acb99609ba313141f4cb6735666556ee489c2c6d Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 16 Sep 2019 15:37:12 +0200 Subject: [PATCH 2/7] Move defaut_groupe_instructeur as has_one and eager_load --- app/controllers/instructeurs/procedures_controller.rb | 2 +- app/models/procedure.rb | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index aa97b2de8..85353c29b 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -8,7 +8,7 @@ module Instructeurs def index @procedures = current_instructeur .visible_procedures - .includes(:logo_attachment, :logo_active_storage_attachment) + .includes(:logo_attachment, :logo_active_storage_attachment, :defaut_groupe_instructeur) .order(archived_at: :desc, published_at: :desc, created_at: :desc) groupe_instructeurs = current_instructeur.groupe_instructeurs.where(procedure: @procedures) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index a714fe691..c6b9d5f94 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -27,6 +27,8 @@ class Procedure < ApplicationRecord has_one :refused_mail, class_name: "Mails::RefusedMail", dependent: :destroy has_one :without_continuation_mail, class_name: "Mails::WithoutContinuationMail", dependent: :destroy + has_one :defaut_groupe_instructeur, -> { where(label: GroupeInstructeur::DEFAULT_LABEL) }, class_name: 'GroupeInstructeur', inverse_of: :procedure + has_one_attached :logo has_one_attached :logo_active_storage has_one_attached :notice @@ -477,10 +479,6 @@ class Procedure < ApplicationRecord end end - def defaut_groupe_instructeur - groupe_instructeurs.find_by(label: GroupeInstructeur::DEFAULT_LABEL) - end - def missing_instructeurs? !AssignTo.exists?(groupe_instructeur: groupe_instructeurs) end From 3eeebd62eba40decb807f9f147d6e35073d93e88 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 11:11:08 +0200 Subject: [PATCH 3/7] Use built in active_storage helper --- app/controllers/instructeurs/procedures_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index 85353c29b..83cc2d03d 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -8,7 +8,8 @@ module Instructeurs def index @procedures = current_instructeur .visible_procedures - .includes(:logo_attachment, :logo_active_storage_attachment, :defaut_groupe_instructeur) + .with_attached_logo + .includes(:defaut_groupe_instructeur) .order(archived_at: :desc, published_at: :desc, created_at: :desc) groupe_instructeurs = current_instructeur.groupe_instructeurs.where(procedure: @procedures) From 88842918f969089760e60c5a80bc18d8e164287f Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 10:16:52 +0200 Subject: [PATCH 4/7] Prepare to drop columns --- app/models/administrateur.rb | 3 +-- app/models/attestation.rb | 2 +- app/models/attestation_template.rb | 2 +- app/models/commentaire.rb | 2 +- app/models/dossier.rb | 1 + app/models/instructeur.rb | 2 +- app/models/procedure.rb | 2 +- app/models/service.rb | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index ed29cc034..c0a32352e 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -1,6 +1,5 @@ class Administrateur < ApplicationRecord - self.ignored_columns = ['features'] - + self.ignored_columns = ['features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at'] include EmailSanitizableConcern include ActiveRecord::SecureToken diff --git a/app/models/attestation.rb b/app/models/attestation.rb index 33239b4a8..bd599de4c 100644 --- a/app/models/attestation.rb +++ b/app/models/attestation.rb @@ -1,5 +1,5 @@ class Attestation < ApplicationRecord - self.ignored_columns = ['pdf'] + self.ignored_columns = ['pdf', 'content_secure_token'] belongs_to :dossier diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb index c53da0f6c..0f31cc36f 100644 --- a/app/models/attestation_template.rb +++ b/app/models/attestation_template.rb @@ -1,5 +1,5 @@ class AttestationTemplate < ApplicationRecord - self.ignored_columns = ['logo', 'signature'] + self.ignored_columns = ['logo', 'signature', 'logo_secure_token', 'signature_secure_token'] include ActionView::Helpers::NumberHelper include TagsSubstitutionConcern diff --git a/app/models/commentaire.rb b/app/models/commentaire.rb index 73f10ff9a..5f7c4f11c 100644 --- a/app/models/commentaire.rb +++ b/app/models/commentaire.rb @@ -1,5 +1,5 @@ class Commentaire < ApplicationRecord - self.ignored_columns = ['file'] + self.ignored_columns = ['file', 'piece_justificative_id'] belongs_to :dossier, inverse_of: :commentaires, touch: true diff --git a/app/models/dossier.rb b/app/models/dossier.rb index c1e11a3a3..d7a807ca3 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -1,4 +1,5 @@ class Dossier < ApplicationRecord + self.ignored_columns = ['json_latlngs'] include DossierFilteringConcern enum state: { diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index baa45513d..0657032be 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -1,5 +1,5 @@ class Instructeur < ApplicationRecord - self.ignored_columns = ['features'] + self.ignored_columns = ['features', 'encrypted_password', 'reset_password_token', 'reset_password_sent_at', 'remember_created_at', 'sign_in_count', 'current_sign_in_at', 'last_sign_in_at', 'current_sign_in_ip', 'last_sign_in_ip', 'failed_attempts', 'unlock_token', 'locked_at'] include EmailSanitizableConcern has_and_belongs_to_many :administrateurs diff --git a/app/models/procedure.rb b/app/models/procedure.rb index c6b9d5f94..a93d73a5f 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -1,7 +1,7 @@ require Rails.root.join('lib', 'percentile') class Procedure < ApplicationRecord - self.ignored_columns = ['logo'] + self.ignored_columns = ['logo', 'logo_secure_token'] MAX_DUREE_CONSERVATION = 36 diff --git a/app/models/service.rb b/app/models/service.rb index 4f8cab66c..167863ed4 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -1,4 +1,5 @@ class Service < ApplicationRecord + self.ignored_columns = ['siret'] has_many :procedures belongs_to :administrateur @@ -17,7 +18,6 @@ class Service < ApplicationRecord validates :nom, presence: { message: 'doit être renseigné' }, allow_nil: false validates :nom, uniqueness: { scope: :administrateur, message: 'existe déjà' } validates :organisme, presence: { message: 'doit être renseigné' }, allow_nil: false - validates :siret, length: { is: 14, message: 'doit être une suite de 14 chiffres' }, allow_nil: true validates :type_organisme, presence: { message: 'doit être renseigné' }, allow_nil: false validates :email, presence: { message: 'doit être renseigné' }, allow_nil: false validates :telephone, presence: { message: 'doit être renseigné' }, allow_nil: false From adfa80142ae109d68afc22d94984618c2bcaa6e6 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 11:01:20 +0200 Subject: [PATCH 5/7] Cleanup application_controller and current_user --- app/controllers/application_controller.rb | 55 +++++++------------ app/controllers/champs/siret_controller.rb | 2 +- app/controllers/support_controller.rb | 2 +- app/views/support/admin.html.haml | 2 +- app/views/support/index.html.haml | 2 +- .../application_controller_spec.rb | 2 +- 6 files changed, 26 insertions(+), 39 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e70dadfda..9927b5b81 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -18,7 +18,7 @@ class ApplicationController < ActionController::Base before_action :set_active_storage_host before_action :setup_tracking - helper_method :logged_in?, :multiple_devise_profile_connect?, :instructeur_signed_in?, :current_instructeur, + helper_method :multiple_devise_profile_connect?, :instructeur_signed_in?, :current_instructeur, :administrateur_signed_in?, :current_administrateur def staging_authenticate @@ -41,10 +41,6 @@ class ApplicationController < ActionController::Base @left_pannel_url = service.left_panel end - def logged_in? - logged_user.present? - end - def multiple_devise_profile_connect? user_signed_in? && instructeur_signed_in? || instructeur_signed_in? && administrateur_signed_in? || @@ -128,22 +124,17 @@ class ApplicationController < ActionController::Base end end - def logged_users - @logged_users ||= [ - current_user, - current_instructeur, - current_administrateur, - current_administration - ].compact - end + def current_user_roles + @current_user_roles ||= begin + roles = [ + current_user, + current_instructeur, + current_administrateur, + current_administration + ].compact.map { |role| role.class.name } - def logged_user - logged_users.first - end - - def logged_user_roles - roles = logged_users.map { |logged_user| logged_user.class.name } - roles.any? ? roles.join(', ') : 'Guest' + roles.any? ? roles.join(', ') : 'Guest' + end end def set_raven_context @@ -152,13 +143,12 @@ class ApplicationController < ActionController::Base def append_info_to_payload(payload) super - user = logged_user payload.merge!({ user_agent: request.user_agent, - user_id: user&.id, - user_email: user&.email, - user_roles: logged_user_roles + user_id: current_user&.id, + user_email: current_user&.email, + user_roles: current_user_roles }.compact) if browser.known? @@ -225,8 +215,7 @@ class ApplicationController < ActionController::Base end def sentry_user - user = logged_user - { id: user ? "#{user.class.name}##{user.id}" : 'Guest' } + { id: user_signed_in? ? "User##{current_user.id}" : 'Guest' } end def sentry_config @@ -257,14 +246,14 @@ class ApplicationController < ActionController::Base key: sendinblue[:client_key], enabled: sendinblue[:enabled], administrateur: { - email: current_administrateur&.email, + email: current_user&.email, payload: { - DS_SIGN_IN_COUNT: current_administrateur&.sign_in_count, + DS_SIGN_IN_COUNT: current_user&.sign_in_count, DS_CREATED_AT: current_administrateur&.created_at, DS_ACTIVE: current_administrateur&.active, DS_ID: current_administrateur&.id, DS_GESTIONNAIRE_ID: current_instructeur&.id, - DS_ROLES: logged_user_roles + DS_ROLES: current_user_roles } } } @@ -277,8 +266,8 @@ class ApplicationController < ActionController::Base key: crisp[:client_key], enabled: crisp[:enabled], administrateur: { - email: current_administrateur&.email, - DS_SIGN_IN_COUNT: current_administrateur&.sign_in_count, + email: current_user&.email, + DS_SIGN_IN_COUNT: current_user&.sign_in_count, DS_CREATED_AT: current_administrateur&.created_at, DS_ID: current_administrateur&.id, DS_NB_DEMARCHES_BROUILLONS: current_administrateur&.procedures&.brouillons&.count, @@ -290,8 +279,6 @@ class ApplicationController < ActionController::Base end def current_email - current_user&.email || - current_instructeur&.email || - current_administrateur&.email + current_user&.email end end diff --git a/app/controllers/champs/siret_controller.rb b/app/controllers/champs/siret_controller.rb index 02175e455..f78a35fb6 100644 --- a/app/controllers/champs/siret_controller.rb +++ b/app/controllers/champs/siret_controller.rb @@ -43,7 +43,7 @@ class Champs::SiretController < ApplicationController def find_etablisement if params[:champ_id].present? - @champ = Champ.find_by(dossier_id: logged_user.dossiers, id: params[:champ_id]) + @champ = policy_scope(Champ).find(params[:champ_id]) @etablissement = @champ&.etablissement end @procedure_id = @champ&.dossier&.procedure_id || 'aperçu' diff --git a/app/controllers/support_controller.rb b/app/controllers/support_controller.rb index 73fd56348..4ab449b72 100644 --- a/app/controllers/support_controller.rb +++ b/app/controllers/support_controller.rb @@ -90,6 +90,6 @@ class SupportController < ApplicationController end def email - logged_user ? logged_user.email : params[:email] + current_user&.email || params[:email] end end diff --git a/app/views/support/admin.html.haml b/app/views/support/admin.html.haml index b1fb05145..88177c590 100644 --- a/app/views/support/admin.html.haml +++ b/app/views/support/admin.html.haml @@ -16,7 +16,7 @@ \. = form_tag contact_path, method: :post, class: 'form' do |f| - - if !logged_in? + - if !user_signed_in? .contact-champ = label_tag :email do Adresse email professionnelle diff --git a/app/views/support/index.html.haml b/app/views/support/index.html.haml index 1ce5483d2..60374d8e6 100644 --- a/app/views/support/index.html.haml +++ b/app/views/support/index.html.haml @@ -9,7 +9,7 @@ Pensez bien à nous donner le plus d'informations possible pour que nous puissions vous aider au mieux = form_tag contact_path, method: :post, multipart: true, class: 'form' do |f| - - if !logged_in? + - if !user_signed_in? .contact-champ = label_tag :email do Email diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 52105bbab..dffb9d849 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -21,7 +21,7 @@ describe ApplicationController, type: :controller do let(:payload) { {} } before do - expect(@controller).to receive(:current_user).and_return(current_user) + allow(@controller).to receive(:current_user).and_return(current_user) expect(@controller).to receive(:current_instructeur).and_return(current_instructeur) expect(@controller).to receive(:current_administrateur).and_return(current_administrateur) expect(@controller).to receive(:current_administration).and_return(current_administration) From 8a691d534a3f5c0f45b1c6af40cc6c70e175259b Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 11:26:38 +0200 Subject: [PATCH 6/7] Fix AdministrateurUsageStatisticsService --- .../administrateur_usage_statistics_service.rb | 12 ++++++------ .../administrateur_usage_statistics_service_spec.rb | 4 +--- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/app/services/administrateur_usage_statistics_service.rb b/app/services/administrateur_usage_statistics_service.rb index 47a76c249..c197f1b7d 100644 --- a/app/services/administrateur_usage_statistics_service.rb +++ b/app/services/administrateur_usage_statistics_service.rb @@ -8,7 +8,7 @@ # - For all other dossiers, the synthetic_state and the state are the same class AdministrateurUsageStatisticsService def update_administrateurs - Administrateur.find_each do |administrateur| + Administrateur.includes(:user).find_each do |administrateur| stats = administrateur_stats(administrateur) api.identify(administrateur.email, stats) end @@ -27,7 +27,7 @@ class AdministrateurUsageStatisticsService nb_dossiers_roi = nb_dossiers_by_procedure_id.reject { |procedure_id, _count| is_brouillon(procedure_id) }.map { |_procedure_id, count| count }.sum result = { - ds_sign_in_count: administrateur.sign_in_count, + ds_sign_in_count: administrateur.user.sign_in_count, ds_created_at: administrateur.created_at, ds_active: administrateur.active, ds_id: administrateur.id, @@ -62,12 +62,12 @@ class AdministrateurUsageStatisticsService admin_roi_high: nb_dossiers_roi * 17 } - if administrateur.current_sign_in_at.present? - result[:ds_current_sign_in_at] = administrateur.current_sign_in_at + if administrateur.user.current_sign_in_at.present? + result[:ds_current_sign_in_at] = administrateur.user.current_sign_in_at end - if administrateur.last_sign_in_at.present? - result[:ds_last_sign_in_at] = administrateur.last_sign_in_at + if administrateur.user.last_sign_in_at.present? + result[:ds_last_sign_in_at] = administrateur.user.last_sign_in_at end result diff --git a/spec/services/administrateur_usage_statistics_service_spec.rb b/spec/services/administrateur_usage_statistics_service_spec.rb index 3133e4a28..1ff7ee567 100644 --- a/spec/services/administrateur_usage_statistics_service_spec.rb +++ b/spec/services/administrateur_usage_statistics_service_spec.rb @@ -38,9 +38,7 @@ describe AdministrateurUsageStatisticsService do context 'for an administrateur that has plenty of things' do let(:administrateur) do create(:administrateur, - sign_in_count: 17, - current_sign_in_at: Time.zone.local(2019, 3, 7), - last_sign_in_at: Time.zone.local(2019, 2, 27), + user: create(:user, sign_in_count: 17, current_sign_in_at: Time.zone.local(2019, 3, 7), last_sign_in_at: Time.zone.local(2019, 2, 27)), active: true, services: [create(:service)], instructeurs: [create(:instructeur)]) From ff2b6ca3ea8b57636cd4068956e9d42c6038b78d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 17 Sep 2019 11:38:48 +0200 Subject: [PATCH 7/7] Fix service controller --- app/controllers/new_administrateur/services_controller.rb | 2 +- spec/controllers/new_administrateur/services_controller_spec.rb | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/new_administrateur/services_controller.rb b/app/controllers/new_administrateur/services_controller.rb index 681b07ab4..236c05238 100644 --- a/app/controllers/new_administrateur/services_controller.rb +++ b/app/controllers/new_administrateur/services_controller.rb @@ -73,7 +73,7 @@ module NewAdministrateur private def service_params - params.require(:service).permit(:nom, :organisme, :siret, :type_organisme, :email, :telephone, :horaires, :adresse) + params.require(:service).permit(:nom, :organisme, :type_organisme, :email, :telephone, :horaires, :adresse) end def service diff --git a/spec/controllers/new_administrateur/services_controller_spec.rb b/spec/controllers/new_administrateur/services_controller_spec.rb index 71984efba..a0bb5e0ad 100644 --- a/spec/controllers/new_administrateur/services_controller_spec.rb +++ b/spec/controllers/new_administrateur/services_controller_spec.rb @@ -14,7 +14,6 @@ describe NewAdministrateur::ServicesController, type: :controller do service: { nom: 'super service', organisme: 'organisme', - siret: '01234567891234', type_organisme: 'association', email: 'email@toto.com', telephone: '1234', @@ -29,7 +28,6 @@ describe NewAdministrateur::ServicesController, type: :controller do it { expect(flash.notice).to eq('super service créé') } it { expect(Service.last.nom).to eq('super service') } it { expect(Service.last.organisme).to eq('organisme') } - it { expect(Service.last.siret).to eq('01234567891234') } it { expect(Service.last.type_organisme).to eq(Service.type_organismes.fetch(:association)) } it { expect(Service.last.email).to eq('email@toto.com') } it { expect(Service.last.telephone).to eq('1234') }