From ccddf885f6adabc9d2aaf3d6e7d212738d25ce7e Mon Sep 17 00:00:00 2001 From: mfo Date: Mon, 26 Aug 2024 15:05:22 +0200 Subject: [PATCH 1/2] fix(data): add missing foreign key on production between champs.dossier_id and dossier.id, use validate: false to delay validation --- .../20240826130121_add_missing_index_champs_to_dossier.rb | 7 +++++++ db/schema.rb | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20240826130121_add_missing_index_champs_to_dossier.rb diff --git a/db/migrate/20240826130121_add_missing_index_champs_to_dossier.rb b/db/migrate/20240826130121_add_missing_index_champs_to_dossier.rb new file mode 100644 index 000000000..562cb1dc5 --- /dev/null +++ b/db/migrate/20240826130121_add_missing_index_champs_to_dossier.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddMissingIndexChampsToDossier < ActiveRecord::Migration[7.0] + def change + add_foreign_key :champs, :dossiers, if_not_exists: true, validate: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 90c1c10b2..c8fc6ebfa 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[7.0].define(version: 2024_07_29_160650) do +ActiveRecord::Schema[7.0].define(version: 2024_08_26_130121) do # These are extensions that must be enabled in order to support this database enable_extension "pg_buffercache" enable_extension "pg_stat_statements" From 1d84c0f55b15bff330e94afe51d8f84be52cd190 Mon Sep 17 00:00:00 2001 From: mfo Date: Mon, 26 Aug 2024 15:29:26 +0200 Subject: [PATCH 2/2] fix(champs.orphaned): add maintenance task to clean orphaned champs where dossier is missing --- ...phaned_champs_with_missing_dossier_task.rb | 33 +++++++++++++ ...d_champs_with_missing_dossier_task_spec.rb | 47 +++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 app/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task.rb create mode 100644 spec/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task_spec.rb diff --git a/app/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task.rb b/app/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task.rb new file mode 100644 index 000000000..533eb25df --- /dev/null +++ b/app/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Maintenance + # On our production environment, the fk between `champs.dossier_id` and `dossiers.id` had not been created + # this maintenance task cleans up orphaned champ pointing to deleted dossier + # next step is to validate the recreated fk + # For our own record, use it with PG_STATEMENT_TIMEOUT=300000 .... + class DeleteOrphanedChampsWithMissingDossierTask < MaintenanceTasks::Task + def collection + Champ.select(:id, :type).where.missing(:dossier) + end + + def process(champ) + champ.reload # in case we need more data on this champ + + case champ.type + when 'Champs::CarteChamp' then + champ.geo_areas.delete_all + when 'Champs::RepetitionChamp' then + champ.champs.delete_all + when 'Champs::SiretChamp' then + etablissement = champ.etablissement + champ.update(etablissement_id: nil) + etablissement.delete + end + champ.delete + end + + def count + # no count, it otherwise it will timeout + end + end +end diff --git a/spec/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task_spec.rb b/spec/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task_spec.rb new file mode 100644 index 000000000..60a171e74 --- /dev/null +++ b/spec/tasks/maintenance/delete_orphaned_champs_with_missing_dossier_task_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "rails_helper" + +module Maintenance + RSpec.describe DeleteOrphanedChampsWithMissingDossierTask do + describe "#process" do + let(:procedure) { create(:procedure, types_de_champ_public:) } + let(:dossier) { create(:dossier, :with_populated_champs, procedure:) } + let(:champ) { dossier.champs.first } + + subject(:process) { described_class.process(champ) } + + context 'with other champs' do + let(:types_de_champ_public) { [{ type: :text }] } + it 'delete champ' do + expect { subject }.to change { Champ.exists?(champ.id) }.from(true).to(false) + end + end + + context "with carte" do + let(:types_de_champ_public) { [{ type: :carte }] } + it 'deletes champ.geo_areas' do + geo_area_ids = champ.geo_areas.ids + expect { subject }.to change { GeoArea.where(id: geo_area_ids).count }.from(2).to(0) + expect(Champ.exists?(champ.id)).to be_falsey + end + end + + context "with repetition" do + let(:types_de_champ_public) { [{ type: :repetition, mandatory: true, children: [{ type: :text }] }] } + it 'deletes champ.champs (children)' do + expect { subject }.to change { champ.champs.count }.from(2).to(0) + expect(Champ.exists?(champ.id)).to be_falsey + end + end + + context "with siret" do + let(:types_de_champ_public) { [{ type: :siret }] } + it 'delete champ.etablissement' do + expect { subject }.to change { Etablissement.exists?(champ.etablissement_id) }.from(true).to(false) + expect(Champ.exists?(champ.id)).to be_falsey + end + end + end + end +end