From 54132df77d3497fe68c548788452f52dd00f838c Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Mon, 16 Sep 2024 12:26:37 +0200 Subject: [PATCH] dossier(champ): validate presence of row_ids --- app/models/concerns/dossier_champs_concern.rb | 13 +++++++++ app/models/procedure_revision.rb | 2 +- .../concerns/dossier_champs_concern_spec.rb | 27 +++++++++++++------ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index 53a6bb96e..7ab2dc65d 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -33,6 +33,7 @@ module DossierChampsConcern end def project_champ(type_de_champ, row_id) + check_valid_row_id?(type_de_champ, row_id) champ = champs_by_public_id[type_de_champ.public_id(row_id)] if champ.nil? type_de_champ.build_champ(dossier: self, row_id:) @@ -81,6 +82,7 @@ module DossierChampsConcern end def champ_for_export(type_de_champ, row_id) + check_valid_row_id?(type_de_champ, row_id) champ = champs_by_public_id[type_de_champ.public_id(row_id)] if champ.blank? || !champ.visible? nil @@ -96,6 +98,7 @@ module DossierChampsConcern end def champ_with_attributes_for_update(type_de_champ, row_id, updated_by:) + check_valid_row_id?(type_de_champ, row_id) attributes = type_de_champ.params_for_champ # TODO: Once we have the right index in place, we should change this to use `create_or_find_by` instead of `find_or_create_by` champ = champs @@ -124,4 +127,14 @@ module DossierChampsConcern [champ, attributes] end + + def check_valid_row_id?(type_de_champ, row_id) + if revision.child?(type_de_champ) + if row_id.blank? + raise "type_de_champ #{type_de_champ.stable_id} must have a row_id because it is part of a repetition" + end + elsif row_id.present? + raise "type_de_champ #{type_de_champ.stable_id} can not have a row_id because it is not part of a repetition" + end + end end diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index c1181ad02..d1073e5e5 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -221,7 +221,7 @@ class ProcedureRevision < ApplicationRecord def child?(tdc) revision_types_de_champ - .find { _1.type_de_champ_id == tdc.id }.child? + .find { _1.type_de_champ_id == tdc.id }&.child? end def remove_children_of(tdc) diff --git a/spec/models/concerns/dossier_champs_concern_spec.rb b/spec/models/concerns/dossier_champs_concern_spec.rb index ce6581fbb..5e68d5e99 100644 --- a/spec/models/concerns/dossier_champs_concern_spec.rb +++ b/spec/models/concerns/dossier_champs_concern_spec.rb @@ -35,7 +35,6 @@ RSpec.describe DossierChampsConcern do let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) } let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) } let(:type_de_champ_private) { dossier.find_type_de_champ_by_stable_id(995) } - let(:row_ids) { dossier.project_champ(type_de_champ_repetition, nil).row_ids } context "public champ" do let(:row_id) { nil } @@ -45,13 +44,20 @@ RSpec.describe DossierChampsConcern do context "in repetition" do let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) } - let(:row_id) { row_ids.first } + let(:row_id) { dossier.project_champ(type_de_champ_repetition, nil).row_ids.first } it { expect(subject.persisted?).to be_truthy expect(subject.row_id).to eq(row_id) expect(subject.parent_id).not_to be_nil } + + context "invalid row_id" do + let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) } + it { + expect { subject }.to raise_error("type_de_champ #{type_de_champ_public.stable_id} can not have a row_id because it is not part of a repetition") + } + end end context "missing champ" do @@ -64,13 +70,20 @@ RSpec.describe DossierChampsConcern do context "in repetition" do let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) } - let(:row_id) { row_ids.first } + let(:row_id) { ULID.generate } it { expect(subject.new_record?).to be_truthy expect(subject.is_a?(Champs::TextChamp)).to be_truthy expect(subject.row_id).to eq(row_id) } + + context "invalid row_id" do + let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) } + it { + expect { subject }.to raise_error("type_de_champ #{type_de_champ_public.stable_id} can not have a row_id because it is not part of a repetition") + } + end end end end @@ -122,7 +135,6 @@ RSpec.describe DossierChampsConcern do let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) } let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(99) } let(:type_de_champ_private) { dossier.find_type_de_champ_by_stable_id(995) } - let(:row_ids) { dossier.project_champ(type_de_champ_repetition, nil).row_ids } let(:row_id) { nil } context "public champ" do @@ -135,7 +147,7 @@ RSpec.describe DossierChampsConcern do context "in repetition" do let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) } - let(:row_id) { row_ids.first } + let(:row_id) { ULID.generate } it { expect(subject.persisted?).to be_truthy @@ -154,7 +166,7 @@ RSpec.describe DossierChampsConcern do context "in repetition" do let(:type_de_champ_public) { dossier.find_type_de_champ_by_stable_id(994) } - let(:row_id) { row_ids.first } + let(:row_id) { ULID.generate } it { expect(subject.persisted?).to be_truthy @@ -178,8 +190,7 @@ RSpec.describe DossierChampsConcern do describe "#update_champs_attributes(public)" do let(:type_de_champ_repetition) { dossier.find_type_de_champ_by_stable_id(993) } - let(:row_ids) { dossier.project_champ(type_de_champ_repetition, nil).row_ids } - let(:row_id) { row_ids.first } + let(:row_id) { ULID.generate } let(:attributes) do {