From 56edba37bca8950c7c63d4ddf9c6b33962f81063 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 16 Mar 2022 18:23:08 +0100 Subject: [PATCH 1/5] specs: make the ProcedurePresentation#sorted_ids test more robust Before the Dossiers were already created in the correct order, so the sorting wasn't really tested. --- spec/models/procedure_presentation_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index cde42e401..dbd0c728c 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -347,14 +347,14 @@ describe ProcedurePresentation do let(:table) { 'followers_instructeurs' } let(:order) { 'asc' } # Desc works the same, no extra test required - let!(:dossier_a) { create(:dossier, :en_construction, procedure: procedure) } let!(:dossier_z) { create(:dossier, :en_construction, procedure: procedure) } + let!(:dossier_a) { create(:dossier, :en_construction, procedure: procedure) } let!(:dossier_without_instructeur) { create(:dossier, :en_construction, procedure: procedure) } before do + create(:follow, dossier: dossier_z, instructeur: create(:instructeur, email: 'zythum@exemple.fr')) create(:follow, dossier: dossier_a, instructeur: create(:instructeur, email: 'abaca@exemple.fr')) create(:follow, dossier: dossier_a, instructeur: create(:instructeur, email: 'abaca2@exemple.fr')) - create(:follow, dossier: dossier_z, instructeur: create(:instructeur, email: 'zythum@exemple.fr')) end context 'for email column' do From d4da6502ba7f1773e8ee42c481a0173b638f223e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 10 Mar 2022 17:34:56 +0100 Subject: [PATCH 2/5] models: remove double-write callback on User --- app/models/user.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 9c0467b13..441737c09 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -65,21 +65,6 @@ class User < ApplicationRecord validate :does_not_merge_on_self, if: :requested_merge_into_id_changed? - # Temporary code for double writing the admin, instructeur and expert id to the foreign key - after_save do - if saved_change_to_attribute?(:administrateur_id) && administrateur_id.present? - Administrateur.find(administrateur_id).update!(user_id: id) - end - - if saved_change_to_attribute?(:instructeur_id) && instructeur_id.present? - Instructeur.find(instructeur_id).update!(user_id: id) - end - - if saved_change_to_attribute?(:expert_id) && expert_id.present? - Expert.find(expert_id).update!(user_id: id) - end - end - def validate_password_complexity? administrateur? end From 31bd05f8350a8dea66b130b2a4b4cf33c6465cd1 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 15 Mar 2022 16:28:22 +0000 Subject: [PATCH 3/5] models: inverse the direction of the User role associations --- app/models/administrateur.rb | 2 +- app/models/expert.rb | 2 +- app/models/instructeur.rb | 2 +- app/models/procedure_presentation.rb | 4 ++-- app/models/user.rb | 21 +++++++++------------ spec/models/user_spec.rb | 4 ++-- 6 files changed, 16 insertions(+), 19 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 6fa4f929a..f5eb26d4a 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -16,7 +16,7 @@ class Administrateur < ApplicationRecord has_and_belongs_to_many :procedures has_many :services - has_one :user, dependent: :nullify + belongs_to :user default_scope { eager_load(:user) } diff --git a/app/models/expert.rb b/app/models/expert.rb index 19ccddcc9..07cc31940 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -8,7 +8,7 @@ # user_id :bigint # class Expert < ApplicationRecord - has_one :user + belongs_to :user has_many :experts_procedures has_many :procedures, through: :experts_procedures has_many :avis, through: :experts_procedures diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index 66a6e3fb6..8fb79ddc1 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -31,7 +31,7 @@ class Instructeur < ApplicationRecord has_many :archives has_many :bulk_messages, dependent: :destroy - has_one :user, dependent: :nullify + belongs_to :user scope :with_instant_email_message_notifications, -> { includes(:assign_to).where(assign_tos: { instant_email_message_notifications_enabled: true }) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index ef3ea6528..5a43fde56 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -124,7 +124,7 @@ class ProcedurePresentation < ApplicationRecord # LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet dossiers .includes(:followers_instructeurs) - .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id') + .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id') .order("instructeurs_users.email #{order}") .pluck(:id) .uniq @@ -167,7 +167,7 @@ class ProcedurePresentation < ApplicationRecord assert_supported_column(table, column) dossiers .includes(:followers_instructeurs) - .joins('INNER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id') + .joins('INNER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id') .filter_ilike('instructeurs_users', :email, values) when 'user', 'individual' dossiers diff --git a/app/models/user.rb b/app/models/user.rb index 441737c09..15c8a8514 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,9 +25,6 @@ # unlock_token :string # created_at :datetime # updated_at :datetime -# administrateur_id :bigint -# expert_id :bigint -# instructeur_id :bigint # requested_merge_into_id :bigint # class User < ApplicationRecord @@ -52,9 +49,9 @@ class User < ApplicationRecord has_many :requested_merge_from, class_name: 'User', dependent: :nullify, inverse_of: :requested_merge_into, foreign_key: :requested_merge_into_id has_one :france_connect_information, dependent: :destroy - belongs_to :instructeur, optional: true, dependent: :destroy - belongs_to :administrateur, optional: true, dependent: :destroy - belongs_to :expert, optional: true, dependent: :destroy + has_one :instructeur, dependent: :destroy + has_one :administrateur, dependent: :destroy + has_one :expert, dependent: :destroy belongs_to :requested_merge_into, class_name: 'User', optional: true accepts_nested_attributes_for :france_connect_information @@ -120,7 +117,7 @@ class User < ApplicationRecord .find_or_create_by(email: email) if user.valid? - if user.instructeur_id.nil? + if user.instructeur.nil? user.create_instructeur! user.update(france_connect_information: nil) end @@ -134,7 +131,7 @@ class User < ApplicationRecord def self.create_or_promote_to_administrateur(email, password) user = User.create_or_promote_to_instructeur(email, password) - if user.valid? && user.administrateur_id.nil? + if user.valid? && user.administrateur.nil? user.create_administrateur! user.update(france_connect_information: nil) end @@ -148,7 +145,7 @@ class User < ApplicationRecord .find_or_create_by(email: email) if user.valid? - if user.expert_id.nil? + if user.expert.nil? user.create_expert! end end @@ -165,15 +162,15 @@ class User < ApplicationRecord end def administrateur? - administrateur_id.present? + administrateur.present? end def instructeur? - instructeur_id.present? + instructeur.present? end def expert? - expert_id.present? + expert.present? end def can_france_connect? diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 35bc79042..088f2bcd6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -372,7 +372,7 @@ describe User, type: :model do end context 'for administrateurs' do - let(:user) { build(:user, email: 'admin@exemple.fr', password: password, administrateur: create(:administrateur, user: nil)) } + let(:user) { build(:user, email: 'admin@exemple.fr', password: password, administrateur: build(:administrateur, user: nil)) } context 'when the password is too short' do let(:password) { 's' * (PASSWORD_MIN_LENGTH - 1) } @@ -453,8 +453,8 @@ describe User, type: :model do targeted_user.reload expect(targeted_user.instructeur).to match(instructeur) - expect(targeted_user.expert).to match(expert) expect(targeted_user.administrateur).to match(administrateur) + expect(targeted_user.expert).to match(expert) end context 'and the targeted account owns an instructeur and expert as well' do From 6849a73aec9becb1bbf0340e33828bc02e2ca929 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 16 Mar 2022 07:46:13 +0100 Subject: [PATCH 4/5] models: ignore deprecated User columns --- app/models/user.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/user.rb b/app/models/user.rb index 15c8a8514..082e607e9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -41,6 +41,8 @@ class User < ApplicationRecord devise :database_authenticatable, :registerable, :async, :recoverable, :rememberable, :trackable, :validatable, :confirmable, :lockable + self.ignored_columns = [:administrateur_id, :instructeur_id, :expert_id] + has_many :dossiers, dependent: :destroy has_many :invites, dependent: :destroy has_many :dossiers_invites, through: :invites, source: :dossier From fbe041070292e22de730c99a9028bde06c322ba2 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 16 Mar 2022 18:23:21 +0100 Subject: [PATCH 5/5] models: fix a typo in a ProcedurePresentation's comment --- app/models/procedure_presentation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index 5a43fde56..d85f20340 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -121,7 +121,7 @@ class ProcedurePresentation < ApplicationRecord end when 'followers_instructeurs' assert_supported_column(table, column) - # LEFT OUTER JOIN allows to keep dossiers without assignated instructeurs yet + # LEFT OUTER JOIN allows to keep dossiers without assigned instructeurs yet dossiers .includes(:followers_instructeurs) .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.id = instructeurs.user_id')