From 7054c2d9efdba0670ba03a4a45e40c0148d6ce9e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 11 Apr 2023 16:59:38 +0200 Subject: [PATCH 1/2] fix(demarche): always reset outside of transactions --- app/controllers/administrateurs/types_de_champ_controller.rb | 2 +- app/models/procedure.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/administrateurs/types_de_champ_controller.rb b/app/controllers/administrateurs/types_de_champ_controller.rb index 2cb3e82b4..4933ecece 100644 --- a/app/controllers/administrateurs/types_de_champ_controller.rb +++ b/app/controllers/administrateurs/types_de_champ_controller.rb @@ -1,7 +1,7 @@ module Administrateurs class TypesDeChampController < AdministrateurController before_action :retrieve_procedure - after_action :reset_procedure, only: [:create, :update, :destroy] + after_action :reset_procedure, only: [:create, :update, :destroy, :piece_justificative_template] def create type_de_champ = draft.add_type_de_champ(type_de_champ_create_params) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index e0048b19b..84bc01c70 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -797,6 +797,7 @@ class Procedure < ApplicationRecord end def publish_revision! + reset! transaction do self.published_revision = draft_revision self.draft_revision = create_new_revision @@ -810,8 +811,8 @@ class Procedure < ApplicationRecord def reset_draft_revision! if published_revision.present? && draft_changed? + reset! transaction do - reset! draft_revision.types_de_champ.filter(&:only_present_on_draft?).each(&:destroy) draft_revision.update(dossier_submitted_message: nil) draft_revision.destroy From 21b548d32bdcbc4036dbc67b7cd91bc4d6632a2d Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 11 Apr 2023 17:00:25 +0200 Subject: [PATCH 2/2] fix(dossier): delete all champs starting with children --- app/models/champ.rb | 2 +- app/models/dossier.rb | 7 +++++-- spec/factories/dossier.rb | 14 ++++++++------ spec/models/procedure_presentation_spec.rb | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/app/models/champ.rb b/app/models/champ.rb index e744d0832..9b7a1c727 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -32,7 +32,7 @@ class Champ < ApplicationRecord # here because otherwise we can't easily use includes in our queries. has_many :geo_areas, -> { order(:created_at) }, dependent: :destroy, inverse_of: :champ belongs_to :etablissement, optional: true, dependent: :destroy - has_many :champs, -> { ordered }, foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy + has_many :champs, -> { ordered }, foreign_key: :parent_id, inverse_of: :parent delegate :procedure, to: :dossier diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 5866700c0..5b6ddc16d 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -86,8 +86,11 @@ class Dossier < ApplicationRecord has_one_attached :justificatif_motivation has_many :champs - has_many :champs_public, -> { root.public_ordered }, class_name: 'Champ', inverse_of: false, dependent: :destroy - has_many :champs_private, -> { root.private_ordered }, class_name: 'Champ', inverse_of: false, dependent: :destroy + # We have to remove champs in a particular order - champs with a reference to a parent have to be + # removed first, otherwise we get a foreign key constraint error. + has_many :champs_to_destroy, -> { order(:parent_id) }, class_name: 'Champ', inverse_of: false, dependent: :destroy + has_many :champs_public, -> { root.public_ordered }, class_name: 'Champ', inverse_of: false + has_many :champs_private, -> { root.private_ordered }, class_name: 'Champ', inverse_of: false has_many :champs_public_all, -> { public_only }, class_name: 'Champ', inverse_of: false has_many :champs_private_all, -> { private_only }, class_name: 'Champ', inverse_of: false has_many :prefilled_champs_public, -> { root.public_only.prefilled }, class_name: 'Champ', inverse_of: false diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb index c766505cd..69588d810 100644 --- a/spec/factories/dossier.rb +++ b/spec/factories/dossier.rb @@ -241,19 +241,21 @@ FactoryBot.define do trait :with_populated_champs do after(:create) do |dossier, _evaluator| - dossier.champs_public = dossier.types_de_champ.map do |type_de_champ| - build(:"champ_#{type_de_champ.type_champ}", dossier: dossier, type_de_champ: type_de_champ) + dossier.champs_to_destroy.where(private: false).destroy_all + dossier.types_de_champ.each do |type_de_champ| + create(:"champ_#{type_de_champ.type_champ}", dossier:, type_de_champ:) end - dossier.save! + dossier.reload end end trait :with_populated_annotations do after(:create) do |dossier, _evaluator| - dossier.champs_private = dossier.types_de_champ_private.map do |type_de_champ| - build(:"champ_#{type_de_champ.type_champ}", private: true, dossier: dossier, type_de_champ: type_de_champ) + dossier.champs_to_destroy.where(private: true).destroy_all + dossier.types_de_champ_private.each do |type_de_champ| + create(:"champ_#{type_de_champ.type_champ}", private: true, dossier:, type_de_champ:) end - dossier.save! + dossier.reload end end diff --git a/spec/models/procedure_presentation_spec.rb b/spec/models/procedure_presentation_spec.rb index 63d489532..a4bd21d45 100644 --- a/spec/models/procedure_presentation_spec.rb +++ b/spec/models/procedure_presentation_spec.rb @@ -1,5 +1,5 @@ describe ProcedurePresentation do - let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private) } + let(:procedure) { create(:procedure, :published, types_de_champ_public: [{}], types_de_champ_private: [{}]) } let(:instructeur) { create(:instructeur) } let(:assign_to) { create(:assign_to, procedure: procedure, instructeur: instructeur) } let(:first_type_de_champ) { assign_to.procedure.active_revision.types_de_champ_public.first }