diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index f77eff7ab..f03d28f19 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -360,12 +360,12 @@ module Users if champs_params[:dossier] @dossier.assign_attributes(champs_params[:dossier]) - # FIXME in some cases a removed repetition bloc row is submitted. - # In this case it will be trated as a new records and action will fail. + # FIXME: in some cases a removed repetition bloc row is submitted. + # In this case it will be treated as a new record, and the action will fail. @dossier.champs.filter(&:repetition?).each do |champ| champ.champs = champ.champs.filter(&:persisted?) end - if @dossier.champs.any?(&:changed?) + if @dossier.champs.any?(&:changed_for_autosave?) @dossier.last_champ_updated_at = Time.zone.now end if !@dossier.save diff --git a/app/models/champ.rb b/app/models/champ.rb index 864ced9fb..8cb1d8804 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -18,7 +18,7 @@ # type_de_champ_id :integer # class Champ < ApplicationRecord - belongs_to :dossier, -> { with_discarded }, inverse_of: :champs, touch: true, optional: false + belongs_to :dossier, -> { with_discarded }, inverse_of: false, touch: true, optional: false belongs_to :type_de_champ, inverse_of: :champ, optional: false belongs_to :parent, class_name: 'Champ', optional: true has_many :commentaires diff --git a/app/models/dossier.rb b/app/models/dossier.rb index aaf93e14f..2441035f0 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -65,8 +65,8 @@ class Dossier < ApplicationRecord has_one_attached :justificatif_motivation has_one_attached :pdf_export_for_instructeur - has_many :champs, -> { root.public_ordered }, inverse_of: :dossier, dependent: :destroy - has_many :champs_private, -> { root.private_ordered }, class_name: 'Champ', inverse_of: :dossier, dependent: :destroy + has_many :champs, -> { root.public_ordered }, inverse_of: false, dependent: :destroy + has_many :champs_private, -> { root.private_ordered }, class_name: 'Champ', inverse_of: false, dependent: :destroy has_many :commentaires, inverse_of: :dossier, dependent: :destroy has_many :invites, dependent: :destroy has_many :follows, -> { active }, inverse_of: :dossier diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 1bd747264..d69c83670 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -619,13 +619,14 @@ describe Users::DossiersController, type: :controller do it 'updates the champs' do subject expect(first_champ.reload.value).to eq('beautiful value') - expect(first_champ.dossier.reload.last_champ_updated_at).to eq(now) expect(piece_justificative_champ.reload.piece_justificative_file).to be_attached end - it 'updates the dossier modification date' do + it 'updates the dossier timestamps' do subject - expect(dossier.reload.updated_at.year).to eq(2100) + dossier.reload + expect(dossier.updated_at).to eq(now) + expect(dossier.last_champ_updated_at).to eq(now) end it 'updates the dossier state' do @@ -635,22 +636,34 @@ describe Users::DossiersController, type: :controller do it { is_expected.to redirect_to(demande_dossier_path(dossier)) } - context 'when only files champs are modified' do + context 'when only a single file champ are modified' do + # A bug in ActiveRecord causes records changed through grand-parent <-> parent <-> child + # relationships do not touch the grand-parent record on change. + # This situation is hit when updating just the attachment of a champ (and not the + # champ itself). + # + # This test ensures that, whatever workaround we wrote for this, it still works properly. + # + # See https://github.com/rails/rails/issues/26726 let(:submit_payload) do { id: dossier.id, dossier: { - champs_attributes: { - id: piece_justificative_champ.id, - piece_justificative_file: file - } + champs_attributes: [ + { + id: piece_justificative_champ.id, + piece_justificative_file: file + } + ] } } end - it 'updates the dossier modification date' do + it 'updates the dossier timestamps' do subject - expect(dossier.reload.updated_at.year).to eq(2100) + dossier.reload + expect(dossier.updated_at).to eq(now) + expect(dossier.last_champ_updated_at).to eq(now) end end end @@ -667,6 +680,12 @@ describe Users::DossiersController, type: :controller do it { expect(response).to render_template(:modifier) } it { expect(flash.alert).to eq(['nop']) } + it 'does not update the dossier timestamps' do + dossier.reload + expect(dossier.updated_at).not_to eq(now) + expect(dossier.last_champ_updated_at).not_to eq(now) + end + it 'does not send an email' do expect(NotificationMailer).not_to receive(:send_initiated_notification)