diff --git a/app/controllers/champs/repetition_controller.rb b/app/controllers/champs/repetition_controller.rb index 1fd2db8dd42..625d98602e3 100644 --- a/app/controllers/champs/repetition_controller.rb +++ b/app/controllers/champs/repetition_controller.rb @@ -2,14 +2,13 @@ class Champs::RepetitionController < Champs::ChampController def add - row = @champ.add_row(@champ.dossier.revision) - @first_champ_id = row.map(&:focusable_input_id).compact.first - @row_id = row.first&.row_id + @row_id = @champ.add_row(updated_by: current_user.email) + @first_champ_id = @champ.focusable_input_id @row_number = @row_id.nil? ? 0 : @champ.row_ids.find_index(@row_id) + 1 end def remove - @champ.remove_row(params[:row_id]) + @champ.remove_row(params[:row_id], updated_by: current_user.email) @to_remove = "safe-row-selector-#{params[:row_id]}" @to_focus = @champ.focusable_input_id || helpers.dom_id(@champ, :create_repetition) end diff --git a/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb b/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb index bf12432f62c..cf65b97b1f6 100644 --- a/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb +++ b/app/graphql/mutations/dossier_modifier_annotation_ajouter_ligne.rb @@ -16,7 +16,7 @@ def resolve(dossier:, annotation_id:, instructeur:) return { errors: ["L’annotation \"#{annotation_id}\" n’existe pas"] } end - annotation.add_row(dossier.revision) + annotation.add_row(updated_by: instructeur.email) { annotation:, errors: nil } end diff --git a/app/graphql/types/dossier_type.rb b/app/graphql/types/dossier_type.rb index b30adf2045f..3236b3a4a81 100644 --- a/app/graphql/types/dossier_type.rb +++ b/app/graphql/types/dossier_type.rb @@ -170,7 +170,7 @@ def champs(id: nil) .for(object, private: false) .load(ApplicationRecord.id_from_typed_id(id)) else - object.champs_for_revision(scope: :public, root: true).filter(&:visible?) + object.project_champs_public.filter(&:visible?) end end @@ -180,7 +180,7 @@ def annotations(id: nil) .for(object, private: true) .load(ApplicationRecord.id_from_typed_id(id)) else - object.champs_for_revision(scope: :private, root: true).filter(&:visible?) + object.project_champs_private.filter(&:visible?) end end diff --git a/app/models/champs/repetition_champ.rb b/app/models/champs/repetition_champ.rb index 532a16832e9..1e699c35843 100644 --- a/app/models/champs/repetition_champ.rb +++ b/app/models/champs/repetition_champ.rb @@ -1,35 +1,26 @@ # frozen_string_literal: true class Champs::RepetitionChamp < Champ - accepts_nested_attributes_for :champs + delegate :libelle_for_export, to: :type_de_champ def rows - dossier - .champs_for_revision(scope: type_de_champ) - .group_by(&:row_id) - .sort - .map(&:second) + dossier.project_rows_for(type_de_champ) end def row_ids - rows.map { _1.first.row_id } + dossier.repetition_row_ids(type_de_champ) end - def add_row(revision) - added_champs = [] - transaction do - row_id = ULID.generate - revision.children_of(type_de_champ).each do |type_de_champ| - added_champs << type_de_champ.build_champ(row_id:) - end - self.champs << added_champs - end - added_champs + def add_row(updated_by:) + # TODO: clean this up when parent_id is deprecated + row_id, added_champs = dossier.repetition_add_row(type_de_champ, updated_by:) + self.champs << added_champs + dossier.champs.reload if dossier.persisted? + row_id end - def remove_row(row_id) - dossier.champs.where(row_id:).destroy_all - dossier.champs.reload + def remove_row(row_id, updated_by:) + dossier.repetition_remove_row(type_de_champ, row_id, updated_by:) end def focusable_input_id diff --git a/app/models/concerns/dossier_champs_concern.rb b/app/models/concerns/dossier_champs_concern.rb index 53a6bb96ec3..ae8784815d3 100644 --- a/app/models/concerns/dossier_champs_concern.rb +++ b/app/models/concerns/dossier_champs_concern.rb @@ -3,21 +3,10 @@ module DossierChampsConcern extend ActiveSupport::Concern - def champs_for_revision(scope: nil, root: false) + def champs_for_revision(scope: nil) champs_index = champs.group_by(&:stable_id) - # Due to some bad data we can have multiple copies of the same champ. Ignore extra copy. - .transform_values { _1.sort_by(&:id).uniq(&:row_id) } - - if scope.is_a?(TypeDeChamp) - revision - .children_of(scope) - .flat_map { champs_index[_1.stable_id] || [] } - .filter(&:child?) # TODO: remove once bad data (child champ without a row id) is cleaned - else - revision - .types_de_champ_for(scope:, root:) - .flat_map { champs_index[_1.stable_id] || [] } - end + revision.types_de_champ_for(scope:) + .flat_map { champs_index[_1.stable_id] || [] } end # Get all the champs values for the types de champ in the final list. @@ -74,6 +63,50 @@ def update_champs_attributes(attributes, scope, updated_by:) assign_attributes(champs_attributes:) end + def project_champs_public + revision.types_de_champ_public.map { project_champ(_1, nil) } + end + + def project_champs_private + revision.types_de_champ_private.map { project_champ(_1, nil) } + end + + def repetition_row_ids(type_de_champ) + stable_ids = revision.children_of(type_de_champ).map(&:stable_id) + champs.filter { _1.stable_id.in?(stable_ids) && _1.row_id.present? } + .map(&:row_id) + .uniq + .sort + end + + def repetition_add_row(type_de_champ, updated_by:) + row_id = ULID.generate + types_de_champ = revision.children_of(type_de_champ) + # TODO: clean this up when parent_id is deprecated + added_champs = types_de_champ.map { _1.build_champ(row_id:, updated_by:) } + @champs_by_public_id = nil + [row_id, added_champs] + end + + def repetition_remove_row(type_de_champ, row_id, updated_by:) + champs.where(row_id:).destroy_all + champs.reload if persisted? + @champs_by_public_id = nil + end + + def project_rows_for(type_de_champ) + types_de_champ = revision.children_of(type_de_champ) + repetition_row_ids(type_de_champ).map do |row_id| + types_de_champ.map { project_champ(_1, row_id) } + end + end + + def reload + super.tap do + @champs_by_public_id = nil + end + end + private def champs_by_public_id diff --git a/app/models/concerns/dossier_searchable_concern.rb b/app/models/concerns/dossier_searchable_concern.rb index f7433337c67..13f65a602ac 100644 --- a/app/models/concerns/dossier_searchable_concern.rb +++ b/app/models/concerns/dossier_searchable_concern.rb @@ -15,7 +15,7 @@ def index_search_terms search_terms = [ user&.email, - *champs_public.flat_map(&:search_terms), + *project_champs_public.flat_map(&:search_terms), *etablissement&.search_terms, individual&.nom, individual&.prenom, @@ -23,7 +23,7 @@ def index_search_terms mandataire_last_name ].compact_blank.join(' ') - private_search_terms = champs_private.flat_map(&:search_terms).compact_blank.join(' ') + private_search_terms = project_champs_private.flat_map(&:search_terms).compact_blank.join(' ') sql = "UPDATE dossiers SET search_terms = :search_terms, private_search_terms = :private_search_terms WHERE id = :id" sanitized_sql = self.class.sanitize_sql_array([sql, search_terms:, private_search_terms:, id:]) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 468868ea2ed..e4292030298 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -479,10 +479,10 @@ def build_default_champs_for_new_dossier champs_private << champ end champs_public.filter { _1.repetition? && _1.mandatory? }.each do |champ| - champ.add_row(revision) + champ.add_row(updated_by: user.email) end champs_private.filter(&:repetition?).each do |champ| - champ.add_row(revision) + champ.add_row(updated_by: user.email) end end @@ -942,14 +942,21 @@ def remove_piece_justificative_file_not_visible! end def check_mandatory_and_visible_champs - champs_for_revision(scope: :public) - .filter { _1.child? ? _1.parent.visible? : true } - .filter(&:visible?) - .filter(&:mandatory_blank?) - .map do |champ| - champ.errors.add(:value, :missing) + project_champs_public.filter(&:visible?).each do |champ| + if champ.mandatory_blank? + error = champ.errors.add(:value, :missing) + errors.import(error) end - .each { errors.import(_1) } + if champ.repetition? + champ.rows.each do |champs| + champs.filter(&:visible?).filter(&:mandatory_blank?).each do |champ| + error = champ.errors.add(:value, :missing) + errors.import(error) + end + end + end + end + errors end def demander_un_avis!(avis) diff --git a/app/models/dossier_preloader.rb b/app/models/dossier_preloader.rb index 4eea0553405..7b32e65f4c9 100644 --- a/app/models/dossier_preloader.rb +++ b/app/models/dossier_preloader.rb @@ -80,8 +80,8 @@ def load_dossier(dossier, champs, pj_template: false) dossier.association(:revision).target = revision end dossier.association(:champs).target = champs - dossier.association(:champs_public).target = dossier.champs_for_revision(scope: :public, root: true) - dossier.association(:champs_private).target = dossier.champs_for_revision(scope: :private, root: true) + dossier.association(:champs_public).target = dossier.project_champs_public + dossier.association(:champs_private).target = dossier.project_champs_private # remove once parent_id is deprecated champs_by_parent_id = champs.group_by(&:parent_id) diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index c1181ad02e3..a672f6aa733 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -179,19 +179,14 @@ def dossier_for_preview(user) dossier end - def types_de_champ_for(scope: nil, root: false) - # We return an unordered collection - return types_de_champ if !root && scope.nil? - return types_de_champ.filter { scope == :public ? _1.public? : _1.private? } if !root - - # We return an ordered collection + def types_de_champ_for(scope: nil) case scope when :public - types_de_champ_public + types_de_champ.filter(&:public?) when :private - types_de_champ_private + types_de_champ.filter(&:private?) else - types_de_champ_public + types_de_champ_private + types_de_champ end end diff --git a/app/models/types_de_champ/prefill_repetition_type_de_champ.rb b/app/models/types_de_champ/prefill_repetition_type_de_champ.rb index 1696c6b7c47..5af53c178a9 100644 --- a/app/models/types_de_champ/prefill_repetition_type_de_champ.rb +++ b/app/models/types_de_champ/prefill_repetition_type_de_champ.rb @@ -56,8 +56,7 @@ def initialize(champ, repetition, index, revision) def to_assignable_attributes return unless repetition.is_a?(Hash) - row = champ.rows[index] || champ.add_row(revision) - row_id = row.first.row_id + row_id = champ.row_ids[index] || champ.add_row(updated_by: nil) repetition.map do |key, value| next unless key.is_a?(String) && key.starts_with?("champ_") diff --git a/app/serializers/champ_serializer.rb b/app/serializers/champ_serializer.rb index bd8c4327163..5a555cd27ae 100644 --- a/app/serializers/champ_serializer.rb +++ b/app/serializers/champ_serializer.rb @@ -48,11 +48,7 @@ def read_attribute_for_serialization(attribute) end def rows - object.dossier - .champs_for_revision(scope: object.type_de_champ) - .group_by(&:row_id) - .values - .map.with_index(1) { |champs, index| Row.new(index:, champs:) } + object.rows.map.with_index(1) { |champs, index| Row.new(index:, champs:) } end def include_etablissement? diff --git a/app/serializers/dossier_serializer.rb b/app/serializers/dossier_serializer.rb index 804a10c896c..474a79131bf 100644 --- a/app/serializers/dossier_serializer.rb +++ b/app/serializers/dossier_serializer.rb @@ -32,7 +32,7 @@ class DossierSerializer < ActiveModel::Serializer has_many :champs, serializer: ChampSerializer def champs - champs = object.champs_public.reject { |c| c.type_de_champ.old_pj.present? } + champs = object.project_champs_public.reject { |c| c.type_de_champ.old_pj.present? } if object.expose_legacy_carto_api? champ_carte = champs.find do |champ| @@ -52,12 +52,16 @@ def champs champs end + def champs_private + object.project_champs_private + end + def cerfa [] end def pieces_justificatives - object.champs_public.filter { |champ| champ.type_de_champ.old_pj }.map do |champ| + object.project_champs_public.filter { |champ| champ.type_de_champ.old_pj }.map do |champ| { created_at: champ.created_at&.in_time_zone('UTC'), type_de_piece_justificative_id: champ.type_de_champ.old_pj[:stable_id], diff --git a/spec/graphql/annotation_spec.rb b/spec/graphql/annotation_spec.rb index cbec007d75d..cd3d731f1cf 100644 --- a/spec/graphql/annotation_spec.rb +++ b/spec/graphql/annotation_spec.rb @@ -5,7 +5,7 @@ let(:procedure) { create(:procedure, :published, :for_individual, types_de_champ_private: [{ type: :repetition, children: [{ libelle: 'Nom' }, { type: :integer_number, libelle: 'Age' }] }, {}], administrateurs: [admin]) } let(:dossiers) { [] } let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) } - let(:champs_private) { dossier.champs_for_revision(scope: :private, root: true) } + let(:champs_private) { dossier.project_champs_private } let(:query) { '' } let(:context) { { administrateur_id: admin.id, procedure_ids: admin.procedure_ids, write_access: true } } diff --git a/spec/lib/recovery/dossier_life_cycle_spec.rb b/spec/lib/recovery/dossier_life_cycle_spec.rb index 79c9bc311dd..863e3e8fe40 100644 --- a/spec/lib/recovery/dossier_life_cycle_spec.rb +++ b/spec/lib/recovery/dossier_life_cycle_spec.rb @@ -17,7 +17,7 @@ let(:dossier) do d = create(:dossier, procedure:) - repetition(d).add_row(d.revision) + repetition(d).add_row(updated_by: 'test') pj_champ(d).piece_justificative_file.attach(some_file) carte(d).update(geo_areas: [geo_area]) d.etablissement = create(:etablissement, :with_exercices) diff --git a/spec/models/concerns/dossier_rebase_concern_spec.rb b/spec/models/concerns/dossier_rebase_concern_spec.rb index 3a1ff0e1bfe..50df35b9d83 100644 --- a/spec/models/concerns/dossier_rebase_concern_spec.rb +++ b/spec/models/concerns/dossier_rebase_concern_spec.rb @@ -346,8 +346,8 @@ datetime_champ.update(value: Time.zone.now.to_s) text_champ.update(value: 'bonjour') # Add two rows then remove previous to last row in order to create a "hole" in the sequence - repetition_champ.add_row(repetition_champ.dossier.revision) - repetition_champ.add_row(repetition_champ.dossier.revision) + repetition_champ.add_row(updated_by: 'test') + repetition_champ.add_row(updated_by: 'test') repetition_champ.champs.where(row_id: repetition_champ.rows[-2].first.row_id).destroy_all repetition_champ.reload end diff --git a/spec/models/concerns/tags_substitution_concern_spec.rb b/spec/models/concerns/tags_substitution_concern_spec.rb index 5b5db6a0ee1..7caf9626333 100644 --- a/spec/models/concerns/tags_substitution_concern_spec.rb +++ b/spec/models/concerns/tags_substitution_concern_spec.rb @@ -233,9 +233,8 @@ def procedure let(:dossier) { create(:dossier, procedure:) } before do - repetition = dossier.champs_public - .find { |champ| champ.libelle == 'Répétition' } - repetition.add_row(dossier.revision) + repetition = dossier.project_champs_public.find(&:repetition?) + repetition.add_row(updated_by: 'test') paul_champs, pierre_champs = repetition.rows paul_champs.first.update(value: 'Paul') diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 03509f89572..dffa4f6b1a3 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -515,7 +515,7 @@ context 'when piece_justificative' do let(:types_de_champ_public) { [{ type: :piece_justificative }] } - let(:champ) { dossier.champs_for_revision(scope: :public).find(&:piece_justificative?) } + let(:champ) { dossier.project_champs_public.find(&:piece_justificative?) } context 'when not visible' do let(:visible) { false } @@ -530,7 +530,7 @@ context 'when titre identite' do let(:types_de_champ_public) { [{ type: :titre_identite }] } - let(:champ) { dossier.champs_for_revision(scope: :public).find(&:piece_justificative?) } + let(:champ) { dossier.project_champs_public.find(&:piece_justificative?) } context 'when not visible' do let(:visible) { false } @@ -728,10 +728,10 @@ end describe "#unspecified_attestation_champs" do - let(:procedure) { create(:procedure, attestation_template: attestation_template, types_de_champ_public: types_de_champ, types_de_champ_private: types_de_champ_private) } - let(:dossier) { create(:dossier, :en_instruction, procedure: procedure) } + let(:procedure) { create(:procedure, attestation_template:, types_de_champ_public:, types_de_champ_private:) } + let(:dossier) { create(:dossier, :en_instruction, procedure:) } - let(:types_de_champ) { [tdc_1, tdc_2, tdc_3, tdc_4] } + let(:types_de_champ_public) { [tdc_1, tdc_2, tdc_3, tdc_4] } let(:types_de_champ_private) { [tdc_5, tdc_6, tdc_7, tdc_8] } let(:tdc_1) { { libelle: "specified champ-in-title" } } @@ -744,7 +744,7 @@ let(:tdc_8) { { libelle: "unspecified annotation privée-in-body" } } before do - (dossier.champs_public + dossier.champs_private) + (dossier.project_champs_public + dossier.project_champs_private) .filter { |c| c.libelle.match?(/^specified/) } .each { |c| c.update_attribute(:value, "specified") } end @@ -799,7 +799,7 @@ let(:attestation_template) { build(:attestation_template, :v2) } before do - tdc_content = (types_de_champ + types_de_champ_private).filter_map do |tdc_config| + tdc_content = (types_de_champ_public + types_de_champ_private).filter_map do |tdc_config| next if tdc_config[:libelle].include?("in-title") { @@ -1608,7 +1608,7 @@ context "with mandatory champs" do let(:type_de_champ) { { mandatory: true } } - let(:champ_with_error) { dossier.champs_public.first } + let(:champ_with_error) { dossier.champs.first } before do champ_with_error.value = nil @@ -1617,7 +1617,7 @@ it 'should have errors' do expect(errors).not_to be_empty - expect(errors.first.full_message).to eq("doit être rempli") + expect(errors.first.full_message).to eq("Le champ « Value » doit être rempli") end context "conditionaly visible" do @@ -1650,7 +1650,7 @@ it 'should have errors' do expect(errors).not_to be_empty - expect(errors.first.full_message).to eq("doit être rempli") + expect(errors.first.full_message).to eq("Le champ « Value » doit être rempli") end end end @@ -1661,39 +1661,37 @@ let(:type_de_champ_repetition) { revision.types_de_champ.first } context "when no champs" do - let(:champ_with_error) { dossier.champs_public.first } - it 'should have errors' do - dossier.champs_public.first.champs.destroy_all - expect(dossier.champs_public.first.rows).to be_empty + dossier.champs.first.row_ids.each do |row_id| + dossier.repetition_remove_row(type_de_champ_repetition, row_id, updated_by: 'test') + end + expect(dossier.champs.first.rows).to be_empty expect(errors).not_to be_empty - expect(errors.first.full_message).to eq("doit être rempli") + expect(errors.first.full_message).to eq("Le champ « Value » doit être rempli") end end context "when mandatory champ inside repetition" do - let(:champ_with_error) { dossier.champs_public.first.champs.first } - it 'should have errors' do - expect(dossier.champs_public.first.rows).not_to be_empty - expect(errors.first.full_message).to eq("doit être rempli") + expect(dossier.champs.first.rows).not_to be_empty + expect(errors).not_to be_empty + expect(errors.first.full_message).to eq("Le champ « Value » doit être rempli") end context "conditionaly visible" do - let(:champ_with_error) { dossier.champs_public.second.champs.first } let(:types_de_champ) { [{ type: :yes_no, stable_id: 99, mandatory: false }, type_de_champ] } let(:type_de_champ) { { type: :repetition, mandatory: true, children: [{ mandatory: true }], condition: ds_eq(champ_value(99), constant(true)) } } it 'should not have errors' do - expect(dossier.champs_public.second.rows).not_to be_empty + expect(dossier.champs.second.rows).not_to be_empty expect(errors).to be_empty end it 'should have errors' do - dossier.champs_public.first.update(value: 'true') - expect(dossier.champs_public.second.rows).not_to be_empty + dossier.champs.first.update(value: 'true') + expect(dossier.champs.second.rows).not_to be_empty expect(errors).not_to be_empty - expect(errors.first.full_message).to eq("doit être rempli") + expect(errors.first.full_message).to eq("Le champ « Value » doit être rempli") end end end @@ -2027,7 +2025,7 @@ procedure.publish! dossier procedure.draft_revision.remove_type_de_champ(text_type_de_champ.stable_id) - coordinate = procedure.draft_revision.add_type_de_champ(type_champ: TypeDeChamp.type_champs.fetch(:text), libelle: 'New text field', after_stable_id: repetition_champ.stable_id) + coordinate = procedure.draft_revision.add_type_de_champ(type_champ: TypeDeChamp.type_champs.fetch(:text), libelle: 'New text field', after_stable_id: repetition_type_de_champ.stable_id) procedure.draft_revision.find_and_ensure_exclusive_use(yes_no_type_de_champ.stable_id).update(libelle: 'Updated yes/no') procedure.draft_revision.find_and_ensure_exclusive_use(commune_type_de_champ.stable_id).update(libelle: 'Commune de naissance') procedure.draft_revision.find_and_ensure_exclusive_use(repetition_type_de_champ.stable_id).update(libelle: 'Repetition') diff --git a/spec/services/pieces_justificatives_service_spec.rb b/spec/services/pieces_justificatives_service_spec.rb index 0e58f6c3f7e..61661ece29b 100644 --- a/spec/services/pieces_justificatives_service_spec.rb +++ b/spec/services/pieces_justificatives_service_spec.rb @@ -56,11 +56,11 @@ def attachments(champ) = champ.piece_justificative_file.attachments let(:second_champ) { repetition(dossier).champs.second } before do - repetition(dossier).add_row(dossier.revision) + repetition(dossier).add_row(updated_by: 'test') attach_file_to_champ(first_champ) attach_file_to_champ(first_champ) - repetition(dossier).add_row(dossier.revision) + repetition(dossier).add_row(updated_by: 'test') attach_file_to_champ(second_champ) end @@ -527,11 +527,11 @@ def repetition(d, index:) = d.champs_public.filter(&:repetition?)[index] repet_0 = repetition(dossier_1, index: 0) repet_1 = repetition(dossier_1, index: 1) - repet_0.add_row(dossier_1.revision) - repet_0.add_row(dossier_1.revision) + repet_0.add_row(updated_by: 'test') + repet_0.add_row(updated_by: 'test') - repet_1.add_row(dossier_1.revision) - repet_1.add_row(dossier_1.revision) + repet_1.add_row(updated_by: 'test') + repet_1.add_row(updated_by: 'test') end it do diff --git a/spec/services/procedure_export_service_zip_spec.rb b/spec/services/procedure_export_service_zip_spec.rb index c872eb67904..696a6206757 100644 --- a/spec/services/procedure_export_service_zip_spec.rb +++ b/spec/services/procedure_export_service_zip_spec.rb @@ -15,11 +15,11 @@ def attachments(champ) = champ.piece_justificative_file.attachments dossiers.each do |dossier| attach_file_to_champ(pj_champ(dossier)) - repetition(dossier).add_row(dossier.revision) + repetition(dossier).add_row(updated_by: 'test') attach_file_to_champ(repetition(dossier).champs.first) attach_file_to_champ(repetition(dossier).champs.first) - repetition(dossier).add_row(dossier.revision) + repetition(dossier).add_row(updated_by: 'test') attach_file_to_champ(repetition(dossier).champs.second) end