From 9dcd92a67820fbdda40a92b9976f41bb2f726ec3 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 22 Mar 2022 12:31:34 +0000 Subject: [PATCH 1/3] db: add not-null constraints to AdministrateursInstructeur --- app/models/administrateurs_instructeur.rb | 4 ++-- ..._null_constraints_to_administrateurs_instructeur.rb | 10 ++++++++++ db/schema.rb | 6 +++--- 3 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20220322110900_add_not_null_constraints_to_administrateurs_instructeur.rb diff --git a/app/models/administrateurs_instructeur.rb b/app/models/administrateurs_instructeur.rb index 96a9320f7..f885c14ee 100644 --- a/app/models/administrateurs_instructeur.rb +++ b/app/models/administrateurs_instructeur.rb @@ -4,8 +4,8 @@ # # created_at :datetime # updated_at :datetime -# administrateur_id :integer -# instructeur_id :integer +# administrateur_id :integer not null +# instructeur_id :integer not null # class AdministrateursInstructeur < ApplicationRecord belongs_to :administrateur diff --git a/db/migrate/20220322110900_add_not_null_constraints_to_administrateurs_instructeur.rb b/db/migrate/20220322110900_add_not_null_constraints_to_administrateurs_instructeur.rb new file mode 100644 index 000000000..eb6047fda --- /dev/null +++ b/db/migrate/20220322110900_add_not_null_constraints_to_administrateurs_instructeur.rb @@ -0,0 +1,10 @@ +class AddNotNullConstraintsToAdministrateursInstructeur < ActiveRecord::Migration[6.1] + def change + # We ignore strong_migrations safety warnings, because those tables are relatively small, and the null check + # will be very fast. + safety_assured do + change_column_null :administrateurs_instructeurs, :administrateur_id, false + change_column_null :administrateurs_instructeurs, :instructeur_id, false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c5dac3a04..23114c7c9 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: 2022_03_15_113510) do +ActiveRecord::Schema.define(version: 2022_03_22_110900) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -64,9 +64,9 @@ ActiveRecord::Schema.define(version: 2022_03_15_113510) do end create_table "administrateurs_instructeurs", id: false, force: :cascade do |t| - t.integer "administrateur_id" + t.integer "administrateur_id", null: false t.datetime "created_at" - t.integer "instructeur_id" + t.integer "instructeur_id", null: false t.datetime "updated_at" t.index ["administrateur_id"], name: "index_administrateurs_instructeurs_on_administrateur_id" t.index ["instructeur_id", "administrateur_id"], name: "unique_couple_administrateur_instructeur", unique: true From 04c3739a3d157235971aa8450cb149bdbd1ab8c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Vantomme?= Date: Fri, 11 Mar 2022 17:21:02 +0100 Subject: [PATCH 2/3] fix(after_party): rescue from PG::UndefinedColumn ActiveRecord::StatementInvalid: PG::UndefinedColumn: ERROR: column types_de_champ.procedure_id does not exist fixes: #7036 --- .../20200728150458_fix_cloned_revisions.rake | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/tasks/deployment/20200728150458_fix_cloned_revisions.rake b/lib/tasks/deployment/20200728150458_fix_cloned_revisions.rake index abfb8b23a..715affec7 100644 --- a/lib/tasks/deployment/20200728150458_fix_cloned_revisions.rake +++ b/lib/tasks/deployment/20200728150458_fix_cloned_revisions.rake @@ -5,21 +5,26 @@ namespace :after_party do Procedure.with_discarded.where(aasm_state: :brouillon).where.not(published_revision_id: nil).update_all(published_revision_id: nil) - types_de_champ = TypeDeChamp.joins(:revision).where('types_de_champ.procedure_id != procedure_revisions.procedure_id') - progress = ProgressReport.new(types_de_champ.count) + begin + types_de_champ = TypeDeChamp.joins(:revision).where('types_de_champ.procedure_id != procedure_revisions.procedure_id') + progress = ProgressReport.new(types_de_champ.count) - types_de_champ.find_each do |type_de_champ| - procedure = type_de_champ.procedure ? type_de_champ.procedure : Procedure.with_discarded.find(type_de_champ.procedure_id) - revision_id = procedure.published_revision_id || procedure.draft_revision_id - type_de_champ.update_column(:revision_id, revision_id) - progress.inc + types_de_champ.find_each do |type_de_champ| + procedure = type_de_champ.procedure ? type_de_champ.procedure : Procedure.with_discarded.find(type_de_champ.procedure_id) + revision_id = procedure.published_revision_id || procedure.draft_revision_id + type_de_champ.update_column(:revision_id, revision_id) + progress.inc + end + + progress.finish + rescue ActiveRecord::StatementInvalid, PG::UndefinedColumn => e + warn e.message + puts "Skip deploy task." + ensure + # 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 - - 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 7ecf20ce75efa588d6ee4069a53130d530e37438 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Thu, 24 Mar 2022 12:38:32 +0100 Subject: [PATCH 3/3] harden email validation --- config/initializers/devise.rb | 2 +- .../groupe_instructeurs_controller_spec.rb | 20 +++++++++---------- .../users/sessions_controller_spec.rb | 9 +++++++-- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 9ed537df5..f23da8124 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -148,7 +148,7 @@ Devise.setup do |config| # Email regex used to validate email formats. It simply asserts that # one (and only one) @ exists in the given string. This is mainly # to give user feedback and not to assert the e-mail validity. - # config.email_regexp = /\A[^@]+@[^@]+\z/ + config.email_regexp = URI::MailTo::EMAIL_REGEXP # ==> Configuration for :timeoutable # The time you want to timeout the user session without activity. After this diff --git a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb index b2009de94..a19bb76f7 100644 --- a/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb +++ b/spec/controllers/administrateurs/groupe_instructeurs_controller_spec.rb @@ -34,17 +34,17 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do context 'when the routage is not activated on the procedure' do let(:procedure) { create :procedure, administrateur: admin, instructeurs: [instructeur_assigned_1, instructeur_assigned_2] } - let!(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere_a.gouv.fr', administrateurs: [admin] } - let!(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere_b.gouv.fr', administrateurs: [admin] } - let!(:instructeur_not_assigned_1) { create :instructeur, email: 'instructeur_3@ministere_a.gouv.fr', administrateurs: [admin] } - let!(:instructeur_not_assigned_2) { create :instructeur, email: 'instructeur_4@ministere_b.gouv.fr', administrateurs: [admin] } + let!(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere-a.gouv.fr', administrateurs: [admin] } + let!(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere-b.gouv.fr', administrateurs: [admin] } + let!(:instructeur_not_assigned_1) { create :instructeur, email: 'instructeur_3@ministere-a.gouv.fr', administrateurs: [admin] } + let!(:instructeur_not_assigned_2) { create :instructeur, email: 'instructeur_4@ministere-b.gouv.fr', administrateurs: [admin] } subject! { get :show, params: { procedure_id: procedure.id, id: gi_1_1.id } } it { expect(response.status).to eq(200) } it 'sets the assigned and not assigned instructeurs' do expect(assigns(:instructeurs)).to match_array([instructeur_assigned_1, instructeur_assigned_2]) - expect(assigns(:available_instructeur_emails)).to match_array(['instructeur_3@ministere_a.gouv.fr', 'instructeur_4@ministere_b.gouv.fr']) + expect(assigns(:available_instructeur_emails)).to match_array(['instructeur_3@ministere-a.gouv.fr', 'instructeur_4@ministere-b.gouv.fr']) end end end @@ -324,9 +324,9 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do describe '#remove_instructeur_procedure_non_routee' do let(:procedure) { create :procedure, administrateur: admin, instructeurs: [instructeur_assigned_1, instructeur_assigned_2] } - let!(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere_a.gouv.fr', administrateurs: [admin] } - let!(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere_b.gouv.fr', administrateurs: [admin] } - let!(:instructeur_assigned_3) { create :instructeur, email: 'instructeur_3@ministere_a.gouv.fr', administrateurs: [admin] } + let!(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere-a.gouv.fr', administrateurs: [admin] } + let!(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere-b.gouv.fr', administrateurs: [admin] } + let!(:instructeur_assigned_3) { create :instructeur, email: 'instructeur_3@ministere-a.gouv.fr', administrateurs: [admin] } subject! { get :show, params: { procedure_id: procedure.id, id: gi_1_1.id } } it 'sets the assigned instructeurs' do expect(assigns(:instructeurs)).to match_array([instructeur_assigned_1, instructeur_assigned_2]) @@ -418,8 +418,8 @@ describe Administrateurs::GroupeInstructeursController, type: :controller do describe '#export_groupe_instructeurs' do let(:procedure) { create(:procedure, :published) } let(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 1 2') } - let(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere_a.gouv.fr', administrateurs: [admin] } - let(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere_b.gouv.fr', administrateurs: [admin] } + let(:instructeur_assigned_1) { create :instructeur, email: 'instructeur_1@ministere-a.gouv.fr', administrateurs: [admin] } + let(:instructeur_assigned_2) { create :instructeur, email: 'instructeur_2@ministere-b.gouv.fr', administrateurs: [admin] } subject do get :export_groupe_instructeurs, params: { procedure_id: procedure.id, format: :csv } diff --git a/spec/controllers/users/sessions_controller_spec.rb b/spec/controllers/users/sessions_controller_spec.rb index f1ddc88d5..99b156f6a 100644 --- a/spec/controllers/users/sessions_controller_spec.rb +++ b/spec/controllers/users/sessions_controller_spec.rb @@ -234,9 +234,14 @@ describe Users::SessionsController, type: :controller do end context 'when the email is evil' do - let(:link_email) { 'Hello, I am an evil email' } + [ + 'Hello, I am an evil email', + 'a@a%C2%A0evil%C2%A0text%C2%A0with%C2%A0spaces' + ].each do |evil_attempt| + let(:link_email) { evil_attempt } - it { expect(response).to redirect_to(root_path) } + it { expect(response).to redirect_to(root_path) } + end end end end