Merge pull request #6173 from betagouv/main

2021-05-05-01

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
@@ -39,5 +39,6 @@ class FranceConnectInformation < ApplicationRecord
     end
 
     update_attribute('user_id', user.id)
+    touch # needed to update updated_at column
   end
 end

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}")

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
 

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

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

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"

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 }

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

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
 
@@ -70,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

spec/lib/tasks/deployment/20210412093054_fill_missing_date_of_fc_spec.rb

@@ -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

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) }

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
 

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