From 50f679746faf090f20a2109489e1477b11633580 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 4 May 2021 15:37:29 +0200 Subject: [PATCH 01/10] do not extract twice the same dossier_id if the dossier is followed twice --- app/models/procedure_presentation.rb | 1 + spec/models/procedure_presentation_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index d64dff58f..647fcda88 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -121,6 +121,7 @@ class ProcedurePresentation < ApplicationRecord .joins('LEFT OUTER JOIN users instructeurs_users ON instructeurs_users.instructeur_id = instructeurs.id') .order("instructeurs_users.email #{order}") .pluck(:id) + .uniq when 'self', 'user', 'individual', 'etablissement', 'groupe_instructeur' (table == 'self' ? dossiers : dossiers.includes(table)) .order("#{self.class.sanitized_column(table, column)} #{order}") diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index 815096316..3be47afce 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -273,6 +273,7 @@ describe ProcedurePresentation do before do 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 From 9fd1c604a129f4b5e6a3d98ec13595a13738ef86 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 4 May 2021 15:41:38 +0200 Subject: [PATCH 02/10] display follower instructeur emails in alphabetic order --- app/services/dossier_projection_service.rb | 2 +- spec/services/dossier_projection_service_spec.rb | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/services/dossier_projection_service.rb b/app/services/dossier_projection_service.rb index 2a3c06730..eec073616 100644 --- a/app/services/dossier_projection_service.rb +++ b/app/services/dossier_projection_service.rb @@ -81,7 +81,7 @@ class DossierProjectionService .where(dossier_id: dossiers_ids) .pluck('dossier_id, users.email') .group_by { |dossier_id, _| dossier_id } - .to_h { |dossier_id, dossier_id_emails| [dossier_id, dossier_id_emails.map { |_, email| email }&.join(', ')] } + .to_h { |dossier_id, dossier_id_emails| [dossier_id, dossier_id_emails.sort.map { |_, email| email }&.join(', ')] } end end diff --git a/spec/services/dossier_projection_service_spec.rb b/spec/services/dossier_projection_service_spec.rb index 7cd750a71..ec14ac9e1 100644 --- a/spec/services/dossier_projection_service_spec.rb +++ b/spec/services/dossier_projection_service_spec.rb @@ -136,10 +136,11 @@ describe DossierProjectionService do let(:column) { 'email' } let(:dossier) { create(:dossier) } - let!(:follow1) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user1@host')) } - let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'user2@host')) } + let!(:follow1) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'b@host')) } + let!(:follow2) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'a@host')) } + let!(:follow3) { create(:follow, dossier: dossier, instructeur: create(:instructeur, email: 'c@host')) } - it { is_expected.to eq "user1@host, user2@host" } + it { is_expected.to eq "a@host, b@host, c@host" } end context 'for type_de_champ table' do From 4dc314d24b8384a17b5d94c4c60455783924527a Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 12 Apr 2021 15:05:05 +0200 Subject: [PATCH 03/10] [fix #6084] add db constraints to france_connect_informations table --- app/models/france_connect_information.rb | 6 +++--- ...445_add_constraints_to_france_connect_informations.rb | 8 ++++++++ db/schema.rb | 9 +++++---- 3 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 db/migrate/20210504115445_add_constraints_to_france_connect_informations.rb diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index 76bf06043..b8f1928aa 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -10,10 +10,10 @@ # family_name :string # gender :string # given_name :string -# created_at :datetime -# updated_at :datetime +# created_at :datetime not null +# updated_at :datetime not null # france_connect_particulier_id :string -# user_id :integer +# user_id :integer not null # class FranceConnectInformation < ApplicationRecord belongs_to :user, optional: true diff --git a/db/migrate/20210504115445_add_constraints_to_france_connect_informations.rb b/db/migrate/20210504115445_add_constraints_to_france_connect_informations.rb new file mode 100644 index 000000000..f12703779 --- /dev/null +++ b/db/migrate/20210504115445_add_constraints_to_france_connect_informations.rb @@ -0,0 +1,8 @@ +class AddConstraintsToFranceConnectInformations < ActiveRecord::Migration[6.1] + def change + change_column_null :france_connect_informations, :user_id, false + change_column_null :france_connect_informations, :created_at, false + change_column_null :france_connect_informations, :updated_at, false + add_foreign_key :france_connect_informations, :users + end +end diff --git a/db/schema.rb b/db/schema.rb index c33baa8f5..ccf0e6e28 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_28_114228) do +ActiveRecord::Schema.define(version: 2021_05_04_115445) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -429,10 +429,10 @@ ActiveRecord::Schema.define(version: 2021_04_28_114228) do t.date "birthdate" t.string "birthplace" t.string "france_connect_particulier_id" - t.integer "user_id" + t.integer "user_id", null: false t.string "email_france_connect" - t.datetime "created_at" - t.datetime "updated_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.jsonb "data" t.index ["user_id"], name: "index_france_connect_informations_on_user_id" end @@ -773,6 +773,7 @@ ActiveRecord::Schema.define(version: 2021_04_28_114228) do add_foreign_key "experts_procedures", "experts" add_foreign_key "experts_procedures", "procedures" add_foreign_key "feedbacks", "users" + add_foreign_key "france_connect_informations", "users" add_foreign_key "geo_areas", "champs" add_foreign_key "groupe_instructeurs", "procedures" add_foreign_key "initiated_mails", "procedures" From ed404f129b97258dce0af8502b5955c3c8c842ab Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Tue, 13 Apr 2021 16:02:58 +0200 Subject: [PATCH 04/10] remove now falling spec of obsolete task --- ...0412093054_fill_missing_date_of_fc_spec.rb | 77 ------------------- 1 file changed, 77 deletions(-) delete mode 100644 spec/lib/tasks/deployment/20210412093054_fill_missing_date_of_fc_spec.rb diff --git a/spec/lib/tasks/deployment/20210412093054_fill_missing_date_of_fc_spec.rb b/spec/lib/tasks/deployment/20210412093054_fill_missing_date_of_fc_spec.rb deleted file mode 100644 index 79657ced7..000000000 --- a/spec/lib/tasks/deployment/20210412093054_fill_missing_date_of_fc_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -describe '20210412093054_fill_missing_date_of_fc' do - let(:rake_task) { Rake::Task['after_party:fill_missing_date_of_fc'] } - - let!(:user) { create(:user, created_at: Time.zone.parse('2000/01/01')) } - - let!(:valid_fci) do - FranceConnectInformation.create!( - user: user, - france_connect_particulier_id: '123', - created_at: Time.zone.parse('2010/01/01'), - updated_at: Time.zone.parse('2012/01/01') - ) - end - - let!(:missing_created_fci) do - fci = FranceConnectInformation.create!( - user: user, - france_connect_particulier_id: '123', - updated_at: Time.zone.parse('2013/01/01') - ) - - fci.update_column('created_at', nil) - fci - end - - let!(:missing_created_updated_fci) do - fci = FranceConnectInformation.create!( - user: user, - france_connect_particulier_id: '123' - ) - - fci.update_column('created_at', nil) - fci.update_column('updated_at', nil) - fci - end - - let!(:missing_created_updated_without_user_fci) do - fci = FranceConnectInformation.create!( - france_connect_particulier_id: '123' - ) - - fci.update_column('created_at', nil) - fci.update_column('updated_at', nil) - fci - end - - before do - rake_task.invoke - end - - after { rake_task.reenable } - - it "does not change valid fci" do - valid_fci.reload - expect(valid_fci.created_at).to eq(Time.zone.parse('2010/01/01')) - expect(valid_fci.updated_at).to eq(Time.zone.parse('2012/01/01')) - expect(valid_fci.data).to be_nil - end - - it "fills missing created from updated" do - missing_created_fci.reload - expect(missing_created_fci.created_at).to eq(Time.zone.parse('2013/01/01')) - expect(missing_created_fci.data['note']).to eq("missing created_at has been copied from updated_at") - end - - it "fills missing created, updated from users created" do - missing_created_updated_fci.reload - expect(missing_created_updated_fci.created_at).to eq(Time.zone.parse('2000/01/01')) - expect(missing_created_updated_fci.updated_at).to eq(Time.zone.parse('2000/01/01')) - expect(missing_created_updated_fci.data['note']).to eq("missing created_at, updated_at have been copied from users.created_at") - end - - it "destroys fci when there is no user" do - expect { missing_created_updated_without_user_fci.reload } - .to raise_error(ActiveRecord::RecordNotFound) - end -end From 1cf59f722e7c3284c4779bcdc13d18680ae65f02 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 16 Apr 2021 14:32:36 +0200 Subject: [PATCH 05/10] add :with_user trait to fci as user_id is mandatory --- spec/factories/france_connect_information.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/factories/france_connect_information.rb b/spec/factories/france_connect_information.rb index 60aaf7d69..68555cb65 100644 --- a/spec/factories/france_connect_information.rb +++ b/spec/factories/france_connect_information.rb @@ -6,5 +6,9 @@ FactoryBot.define do birthdate { '1976-02-24' } france_connect_particulier_id { '1234567' } email_france_connect { 'plip@octo.com' } + + trait :with_user do + user { build(:user, email: email_france_connect) } + end end end From 9dc9e92c16a4a41d66f2b7fd02f3bda695daa6a8 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 26 Apr 2021 16:25:33 +0200 Subject: [PATCH 06/10] use the with_user trait in feature --- .../france_connect_particulier_spec.rb | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/spec/features/france_connect/france_connect_particulier_spec.rb b/spec/features/france_connect/france_connect_particulier_spec.rb index 37a5171c3..08901e21c 100644 --- a/spec/features/france_connect/france_connect_particulier_spec.rb +++ b/spec/features/france_connect/france_connect_particulier_spec.rb @@ -33,23 +33,23 @@ feature 'France Connect Particulier Connexion' do let(:code) { 'plop' } context 'when authentification is ok' do - let(:france_connect_information) do - create(:france_connect_information, - france_connect_particulier_id: france_connect_particulier_id, - given_name: given_name, - family_name: family_name, - birthdate: birthdate, - birthplace: birthplace, - gender: gender, - email_france_connect: email) - end - before do allow_any_instance_of(FranceConnectParticulierClient).to receive(:authorization_uri).and_return(france_connect_particulier_callback_path(code: code)) - allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(FranceConnectInformation.new(user_info)) + allow(FranceConnectService).to receive(:retrieve_user_informations_particulier).and_return(france_connect_information) end context 'when is the first connexion' do + let(:france_connect_information) do + build(:france_connect_information, + france_connect_particulier_id: france_connect_particulier_id, + given_name: given_name, + family_name: family_name, + birthdate: birthdate, + birthplace: birthplace, + gender: gender, + email_france_connect: email) + end + before do page.find('.france-connect-login-button').click end @@ -60,8 +60,21 @@ feature 'France Connect Particulier Connexion' do end context 'when is not the first connexion' do + let!(:france_connect_information) do + create(:france_connect_information, + :with_user, + france_connect_particulier_id: france_connect_particulier_id, + given_name: given_name, + family_name: family_name, + birthdate: birthdate, + birthplace: birthplace, + gender: gender, + email_france_connect: email, + created_at: Time.zone.parse('12/12/2012'), + updated_at: Time.zone.parse('12/12/2012')) + end + before do - create(:user, france_connect_information: france_connect_information) page.find('.france-connect-login-button').click end From 8d0082419c9f325691a2760156a85321aa148722 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 26 Apr 2021 16:26:09 +0200 Subject: [PATCH 07/10] do not know how it worked before --- app/models/france_connect_information.rb | 1 + spec/features/france_connect/france_connect_particulier_spec.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/france_connect_information.rb b/app/models/france_connect_information.rb index b8f1928aa..d76ee60d8 100644 --- a/app/models/france_connect_information.rb +++ b/app/models/france_connect_information.rb @@ -39,5 +39,6 @@ class FranceConnectInformation < ApplicationRecord end update_attribute('user_id', user.id) + touch # needed to update updated_at column end end diff --git a/spec/features/france_connect/france_connect_particulier_spec.rb b/spec/features/france_connect/france_connect_particulier_spec.rb index 08901e21c..7dbbbef1f 100644 --- a/spec/features/france_connect/france_connect_particulier_spec.rb +++ b/spec/features/france_connect/france_connect_particulier_spec.rb @@ -83,7 +83,7 @@ feature 'France Connect Particulier Connexion' do end scenario 'the updated_at date is well updated' do - expect(france_connect_information.updated_at).not_to eq(france_connect_information.created_at) + expect(france_connect_information.reload.updated_at).not_to eq(france_connect_information.created_at) end end end From e18fb116c6938aa2b21c40d67bc9feed84e6057c Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 26 Apr 2021 16:26:42 +0200 Subject: [PATCH 08/10] cannot create fci without user --- spec/models/france_connect_information_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/france_connect_information_spec.rb b/spec/models/france_connect_information_spec.rb index 3115b9bee..6e8a0822b 100644 --- a/spec/models/france_connect_information_spec.rb +++ b/spec/models/france_connect_information_spec.rb @@ -9,7 +9,7 @@ describe FranceConnectInformation, type: :model do describe 'associate_user!' do context 'when there is no user with same email' do - let(:fci) { create(:france_connect_information) } + let(:fci) { build(:france_connect_information) } let(:subject) { fci.associate_user! } it { expect { subject }.to change(User, :count).by(1) } From ccce7249763bab4ccf69405908b8a703fdc58be7 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Mon, 26 Apr 2021 16:27:28 +0200 Subject: [PATCH 09/10] refactor as created fci must have an user_id --- .../particulier_controller_spec.rb | 64 +++++-------------- 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/spec/controllers/france_connect/particulier_controller_spec.rb b/spec/controllers/france_connect/particulier_controller_spec.rb index 939c6b559..9a11c282e 100644 --- a/spec/controllers/france_connect/particulier_controller_spec.rb +++ b/spec/controllers/france_connect/particulier_controller_spec.rb @@ -50,68 +50,36 @@ describe FranceConnect::ParticulierController, type: :controller do end context 'when france_connect_particulier_id exist in database' do - let!(:france_connect_information) { create(:france_connect_information, user_info) } + let!(:france_connect_information) { create(:france_connect_information, :with_user, user_info) } + let(:user) { france_connect_information.user } it { expect { subject }.not_to change { FranceConnectInformation.count } } - context 'when france_connect_particulier_id have an associate user' do - let!(:user) { create(:user, email: email, france_connect_information: france_connect_information) } - - it do - subject - expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) - end - - context 'and the user has a stored location' do - let(:stored_location) { '/plip/plop' } - before { controller.store_location_for(:user, stored_location) } - - it { is_expected.to redirect_to(stored_location) } - end + it do + subject + expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) end - context 'when france_connect_particulier_id does not have an associate user' do - context 'when the email address is not used yet' do - it { expect { subject }.to change(User, :count).by(1) } - it { is_expected.to redirect_to(root_path) } - end + context 'and the user has a stored location' do + let(:stored_location) { '/plip/plop' } + before { controller.store_location_for(:user, stored_location) } - context 'when the email address is already used' do - let!(:user) { create(:user, email: email, france_connect_information: nil) } + it { is_expected.to redirect_to(stored_location) } + end - it 'associates the france_connect infos with the existing user' do - expect { subject }.not_to change(User, :count) - expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) - expect(subject).to redirect_to(root_path) - end + context 'and the user is also instructeur' do + let!(:instructeur) { create(:instructeur, email: email) } + before { subject } - context 'and the user is also instructeur' do - let(:instructeur) { create(:instructeur) } - let(:email) { instructeur.email } - let(:user) { instructeur.user } - before { subject } + it { expect(response).to redirect_to(new_user_session_path) } - it { expect(response).to redirect_to(new_user_session_path) } - - it { expect(flash[:alert]).to be_present } - end - end - - context 'when a differently cased email address is already used' do - let(:email) { 'TEST@test.com' } - let!(:user) { create(:user, email: email.downcase, france_connect_information: nil) } - - it 'associates the france_connect infos with the existing user' do - expect { subject }.not_to change(User, :count) - expect(user.reload.loged_in_with_france_connect).to eq(User.loged_in_with_france_connects.fetch(:particulier)) - expect(subject).to redirect_to(root_path) - end - end + it { expect(flash[:alert]).to be_present } end end context 'when france_connect_particulier_id does not exist in database' do it { expect { subject }.to change { FranceConnectInformation.count }.by(1) } + it { expect { subject }.to change { FranceConnectInformation.count }.by(1) } describe 'FranceConnectInformation attributs' do let(:stored_fci) { FranceConnectInformation.last } From 2516abc2770d6e5f7788def56b16c53200852ecc Mon Sep 17 00:00:00 2001 From: Christophe Robillard Date: Tue, 4 May 2021 11:51:35 +0200 Subject: [PATCH 10/10] activate rack_mini_profiler in dev and display query count --- config/initializers/rack_mini_profiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/rack_mini_profiler.rb b/config/initializers/rack_mini_profiler.rb index fc9414e45..a389dff41 100644 --- a/config/initializers/rack_mini_profiler.rb +++ b/config/initializers/rack_mini_profiler.rb @@ -1,3 +1,3 @@ if Rails.env.development? - Rack::MiniProfiler.config.authorization_mode = :whitelist + Rack::MiniProfiler.config.show_total_sql_count = true end