DGNum/demarches-normaliennes
13
1
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity

fix(dossier): always remove previous champ version

Browse source 
If champ type_de_champ gets out of sync with its type, the persisted champ will not be part of the filled champs collection. In the merge code, we need to remove the previous champ, disregarding its type. The problem should have been caught earlier, but our unique index is not actually unique because our version of PG misses `NULLS NOT DISTINCT`. The unique index only works for champs in repetitions.
This commit is contained in:
Paul Chavard 2024-12-16 13:08:56 +00:00
parent a6f39b3ad1
commit 9dd1973e18
No known key found for this signature in database
2 changed files with 16 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/models/concerns/dossier_clone_concern.rb
View file

@ -167,7 +167,7 @@ module DossierCloneConcern
added_champs.each { _1.update_column(:dossier_id, id) } added_champs.each { _1.update_column(:dossier_id, id) }
if updated_champs.present? if updated_champs.present?
champs_index = filled_champs_public.index_by(&:public_id) champs_index = champs.index_by(&:public_id)
updated_champs.each do |champ| updated_champs.each do |champ|
champs_index[champ.public_id]&.destroy! champs_index[champ.public_id]&.destroy!
champ.update_column(:dossier_id, id) champ.update_column(:dossier_id, id)

25
spec/models/concerns/dossier_clone_concern_spec.rb
View file

@ -378,11 +378,15 @@ RSpec.describe DossierCloneConcern do
} }
let(:removed_champ) { dossier.champs.find { _1.stable_id == 99 } } let(:removed_champ) { dossier.champs.find { _1.stable_id == 99 } }
let(:updated_champ) { dossier.champs.find { _1.stable_id == 991 } } let(:updated_champ) { dossier.champs.find { _1.stable_id == 991 } }
let(:repetition_updated_champ) { champ_for_update(dossier.champs.find { _1.stable_id == 994 }) }
let(:forked_updated_champ) { champ_for_update(forked_dossier.champs.find { _1.stable_id == 991 }) }
let(:forked_repetition_updated_champ) { champ_for_update(forked_dossier.champs.find { _1.stable_id == 994 }) }
before do before do
dossier.champs.each do |champ| dossier.champs.each do |champ|
champ.update(value: 'old value') champ.update(value: 'old value')
end end
dossier.reload
procedure.draft_revision.add_type_de_champ({ procedure.draft_revision.add_type_de_champ({
type_champ: TypeDeChamp.type_champs.fetch(:text), type_champ: TypeDeChamp.type_champs.fetch(:text),
libelle: "Un nouveau champ text" libelle: "Un nouveau champ text"
@ -395,18 +399,19 @@ RSpec.describe DossierCloneConcern do
procedure.draft_revision.remove_type_de_champ(removed_champ.stable_id) procedure.draft_revision.remove_type_de_champ(removed_champ.stable_id)
procedure.draft_revision.find_and_ensure_exclusive_use(updated_champ.stable_id).update(libelle: "Un nouveau libelle") procedure.draft_revision.find_and_ensure_exclusive_use(updated_champ.stable_id).update(libelle: "Un nouveau libelle")
procedure.publish_revision! procedure.publish_revision!
added_champ.update(value: 'new value for added champ')
added_repetition_champ.update(value: "new value in repetition champ")
forked_updated_champ.update(value: 'new value for updated champ')
forked_repetition_updated_champ.update(value: 'new value for updated champ in repetition')
updated_champ.update(type: 'Champs::TextareaChamp')
repetition_updated_champ.update(type: 'Champs::TextareaChamp')
dossier.reload
forked_dossier.reload
end end
subject { it { expect { subject }.to change { dossier.filled_champs.size }.by(3) }
added_champ.update(value: 'new value for added champ') it { expect { subject }.to change { dossier.filled_champs.sort_by(&:created_at).map(&:to_s) }.from(['old value', 'Non', 'old value']).to(['new value for updated champ', 'Non', 'new value for updated champ in repetition', 'old value', 'new value for added champ', 'new value in repetition champ']) }
updated_champ.update(value: 'new value for updated champ')
added_repetition_champ.update(value: "new value in repetition champ")
dossier.reload
super()
}
it { expect { subject }.to change { dossier.filled_champs.size }.by(1) }
it { expect { subject }.to change { dossier.filled_champs.sort_by(&:created_at).map(&:to_s) }.from(['old value', 'old value', 'Non', 'old value', 'old value']).to(['new value for updated champ', 'Non', 'old value', 'old value', 'new value for added champ', 'new value in repetition champ']) }
it "dossier after merge should be on last published revision" do it "dossier after merge should be on last published revision" do
expect(dossier.revision_id).to eq(procedure.revisions.first.id) expect(dossier.revision_id).to eq(procedure.revisions.first.id)