Merge pull request #10711 from tchak/refactor-champ-related-cleanups

Champ related cleanups and refactoring
This commit is contained in:
Paul Chavard 2024-08-26 13:10:50 +00:00 committed by GitHub
commit 7b4d1e4c03
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
30 changed files with 158 additions and 173 deletions

View file

@ -18,6 +18,7 @@ class API::V1::DossiersController < APIController
def show
dossier = @dossiers.for_api.find(params[:id])
DossierPreloader.load_one(dossier)
render json: { dossier: DossierSerializer.new(dossier).as_json }
rescue ActiveRecord::RecordNotFound

View file

@ -9,8 +9,12 @@ class Champs::ChampController < ApplicationController
def find_champ
dossier = policy_scope(Dossier).includes(:champs, revision: [:types_de_champ]).find(params[:dossier_id])
type_de_champ = dossier.find_type_de_champ_by_stable_id(params[:stable_id])
if type_de_champ.repetition?
dossier.project_champ(type_de_champ, nil)
else
dossier.champ_for_update(type_de_champ, params_row_id, updated_by: current_user.email)
end
end
def params_row_id
params[:row_id]

View file

@ -87,13 +87,13 @@ class AttestationTemplate < ApplicationRecord
end
def unspecified_champs_for_dossier(dossier)
champs_by_stable_id = dossier.champs_for_revision(root: true).index_by { "tdc#{_1.stable_id}" }
types_de_champ_by_tag_id = dossier.revision.types_de_champ.index_by { "tdc#{_1.stable_id}" }
used_tags.filter_map do |used_tag|
corresponding_champ = champs_by_stable_id[used_tag]
corresponding_type_de_champ = types_de_champ_by_tag_id[used_tag]
if corresponding_champ && corresponding_champ.blank?
corresponding_champ
if corresponding_type_de_champ && dossier.project_champ(corresponding_type_de_champ, nil).blank?
corresponding_type_de_champ
end
end
end

View file

@ -94,7 +94,7 @@ class Champ < ApplicationRecord
end
def child?
parent_id.present?
row_id.present?
end
# used for the `required` html attribute

View file

@ -37,7 +37,7 @@ class Champs::RepetitionChamp < Champ
end
def blank?
champs.empty?
row_ids.empty?
end
def search_terms

View file

@ -16,9 +16,9 @@ module ChampsValidateConcern
def validate_champ_value?
case validation_context
when :champs_public_value
public? && visible?
public? && in_dossier_revision? && visible?
when :champs_private_value
private? && visible?
private? && in_dossier_revision? && visible?
else
false
end
@ -27,5 +27,9 @@ module ChampsValidateConcern
def validate_champ_value_or_prefill?
validate_champ_value? || validation_context == :prefill
end
def in_dossier_revision?
dossier.revision.types_de_champ.any? { _1.stable_id = stable_id }
end
end
end

View file

@ -57,7 +57,7 @@ module DossierChampsConcern
.types_de_champ
.filter { _1.stable_id.in?(stable_ids) }
.filter { !revision.child?(_1) }
.map { champ_for_update(_1, nil, updated_by: nil) }
.map { _1.repetition? ? project_champ(_1, nil) : champ_for_update(_1, nil, updated_by: nil) }
end
def champ_for_update(type_de_champ, row_id, updated_by:)

View file

@ -51,7 +51,6 @@ class Dossier < ApplicationRecord
has_many :champs_to_destroy, -> { order(:parent_id) }, class_name: 'Champ', inverse_of: false, dependent: :destroy
has_many :champs_public, -> { root.public_only }, class_name: 'Champ', inverse_of: false
has_many :champs_private, -> { root.private_only }, class_name: 'Champ', inverse_of: false
has_many :prefilled_champs_public, -> { root.public_only.prefilled }, class_name: 'Champ', inverse_of: false
has_many :commentaires, inverse_of: :dossier, dependent: :destroy
has_many :preloaded_commentaires, -> { includes(:dossier_correction, piece_jointe_attachments: :blob) }, class_name: 'Commentaire', inverse_of: :dossier
@ -270,26 +269,9 @@ class Dossier < ApplicationRecord
scope :en_cours, -> { not_archived.state_en_construction_ou_instruction }
scope :without_followers, -> { where.missing(:follows) }
scope :with_followers, -> { left_outer_joins(:follows).where.not(follows: { id: nil }) }
scope :with_champs, -> {
includes(champs_public: [
:geo_areas,
piece_justificative_file_attachments: :blob,
champs: [piece_justificative_file_attachments: :blob]
])
}
scope :brouillons_recently_updated, -> { updated_since(2.days.ago).state_brouillon.order_by_updated_at }
scope :with_annotations, -> {
includes(champs_private: [
:geo_areas,
piece_justificative_file_attachments: :blob,
champs: [piece_justificative_file_attachments: :blob]
])
}
scope :for_api, -> {
with_champs
.with_annotations
.includes(commentaires: { piece_jointe_attachments: :blob },
includes(commentaires: { piece_jointe_attachments: :blob },
justificatif_motivation_attachment: :blob,
attestation: [],
avis: { piece_justificative_file_attachment: :blob },
@ -441,8 +423,6 @@ class Dossier < ApplicationRecord
validates :mandataire_last_name, presence: true, if: :for_tiers?
validates :for_tiers, inclusion: { in: [true, false] }, if: -> { revision&.procedure&.for_individual? }
validates_associated :prefilled_champs_public, on: :champs_public_value
def types_de_champ_public
types_de_champ
end
@ -949,7 +929,7 @@ class Dossier < ApplicationRecord
end
def remove_titres_identite!
champs_public.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later)
champs.filter(&:titre_identite?).map(&:piece_justificative_file).each(&:purge_later)
end
def remove_piece_justificative_file_not_visible!
@ -1164,7 +1144,7 @@ class Dossier < ApplicationRecord
end
def has_annotations?
revision.revision_types_de_champ_private.present?
revision.types_de_champ_private.present?
end
def hide_info_with_accuse_lecture?

View file

@ -37,18 +37,12 @@ class DossierPreloader
private
def revisions
def revisions(pj_template: false)
@revisions ||= ProcedureRevision.where(id: @dossiers.pluck(:revision_id).uniq)
.includes(types_de_champ: { piece_justificative_template_attachment: :blob })
.includes(types_de_champ: pj_template ? { piece_justificative_template_attachment: :blob } : [])
.index_by(&:id)
end
# returns: { revision_id : { stable_id : position } }
def positions
@positions ||= revisions
.transform_values { |revision| revision.revision_types_de_champ.map { [_1.stable_id, _1.position] }.to_h }
end
def load_dossiers(dossiers, pj_template: false)
to_include = @includes_for_champ.dup
to_include << [piece_justificative_file_attachments: :blob]
@ -58,16 +52,10 @@ class DossierPreloader
.where(dossier_id: dossiers)
.to_a
children_champs, root_champs = all_champs.partition(&:child?)
champs_by_dossier = root_champs.group_by(&:dossier_id)
champs_by_dossier_by_parent = children_champs
.group_by(&:dossier_id)
.transform_values do |champs|
champs.group_by(&:parent_id)
end
champs_by_dossier = all_champs.group_by(&:dossier_id)
dossiers.each do |dossier|
load_dossier(dossier, champs_by_dossier[dossier.id] || [], champs_by_dossier_by_parent[dossier.id] || {})
load_dossier(dossier, champs_by_dossier[dossier.id] || [], pj_template:)
end
load_etablissements(all_champs)
@ -86,17 +74,30 @@ class DossierPreloader
end
end
def load_dossier(dossier, champs, children_by_parent = {})
revision = revisions[dossier.revision_id]
def load_dossier(dossier, champs, pj_template: false)
revision = revisions(pj_template:)[dossier.revision_id]
if revision.present?
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)
champs_public, champs_private = champs.partition(&:public?)
# remove once parent_id is deprecated
champs_by_parent_id = champs.group_by(&:parent_id)
dossier.association(:champs).target = []
load_champs(dossier, :champs_public, champs_public, dossier, children_by_parent)
load_champs(dossier, :champs_private, champs_private, dossier, children_by_parent)
champs.each do |champ|
champ.association(:dossier).target = dossier
# remove once parent_id is deprecated
if champ.repetition?
children = champs_by_parent_id.fetch(champ.id, [])
children.each do |child|
child.association(:parent).target = champ
end
champ.association(:champs).target = children
end
end
# We need to do this because of the check on `Etablissement#champ` in
# `Etablissement#libelle_for_export`. By assigning `nil` to `target` we mark association
@ -105,33 +106,4 @@ class DossierPreloader
dossier.etablissement.association(:champ).target = nil
end
end
def load_champs(parent, name, champs, dossier, children_by_parent)
if champs.empty?
parent.association(name).target = [] # tells to Rails association has been loaded
return
end
champs.each do |champ|
champ.association(:dossier).target = dossier
if parent.is_a?(Champ)
champ.association(:parent).target = parent
end
end
dossier.association(:champs).target += champs
parent.association(name).target = champs
.filter { positions[dossier.revision_id][_1.stable_id].present? }
.sort_by { [_1.row_id, positions[dossier.revision_id][_1.stable_id]] }
# Load children champs
champs.filter(&:block?).each do |parent_champ|
champs = children_by_parent[parent_champ.id] || []
parent_champ.association(:dossier).target = dossier
load_champs(parent_champ, :champs, champs, dossier, children_by_parent)
end
end
end

View file

@ -239,20 +239,6 @@ class Procedure < ApplicationRecord
includes(:draft_revision, :published_revision, administrateurs: :user)
}
scope :for_download, -> {
includes(
:groupe_instructeurs,
dossiers: {
champs_public: [
piece_justificative_file_attachments: :blob,
champs: [
piece_justificative_file_attachments: :blob
]
]
}
)
}
validates :libelle, presence: true, allow_blank: false, allow_nil: false
validates :description, presence: true, allow_blank: false, allow_nil: false
validates :administrateurs, presence: true

View file

@ -32,8 +32,8 @@ class ProcedureRevisionTypeDeChamp < ApplicationRecord
end
def siblings
if parent_id.present?
revision.revision_types_de_champ.where(parent_id: parent_id).ordered
if child?
revision.revision_types_de_champ.where(parent_id:).ordered
elsif private?
revision.revision_types_de_champ_private
else

View file

@ -12,7 +12,7 @@
- if unspecified_attestation_champs.present?
.warning
Attention, les valeurs suivantes nont pas été renseignées mais sont nécessaires pour pouvoir envoyer une attestation valide :
- unspecified_annotations_privees, unspecified_champs = unspecified_attestation_champs.partition(&:private)
- unspecified_annotations_privees, unspecified_champs = unspecified_attestation_champs.partition(&:private?)
- if unspecified_champs.present?
%h4 Champs de la demande

View file

@ -9,7 +9,7 @@ describe Champs::RepetitionController, type: :controller do
it 'removes repetition' do
rows, repetitions = dossier.champs.partition(&:child?)
repetition = repetitions.first
expect { delete :remove, params: { dossier_id: repetition.dossier, stable_id: repetition.stable_id, row_id: rows.first.row_id }, format: :turbo_stream }
expect { delete :remove, params: { dossier_id: dossier, stable_id: repetition.stable_id, row_id: rows.first.row_id }, format: :turbo_stream }
.to change { dossier.reload.champs.size }.from(3).to(1)
end
end

View file

@ -8,6 +8,8 @@ describe Champ do
let(:champ) { Champ.new(value: value) }
let(:value) { '' }
let(:mandatory) { true }
context 'with champ' do
before { allow(champ).to receive(:type_de_champ).and_return(type_de_champ) }
context 'when mandatory and blank' do
@ -30,19 +32,11 @@ describe Champ do
context 'when repetition blank' do
let(:type_de_champ) { build(:type_de_champ_repetition) }
let(:champ) { Champs::RepetitionChamp.new }
let(:champ) { Champs::RepetitionChamp.new(dossier: Dossier.new(revision: ProcedureRevision.new)) }
it { expect(champ.blank?).to be(true) }
end
context 'when repetition not blank' do
let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :repetition, children: [{ type: :text }] }]) }
let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
let(:champ) { dossier.champs.find(&:repetition?) }
it { expect(champ.blank?).to be(false) }
end
context 'when not blank' do
let(:value) { 'yop' }
it { expect(champ.mandatory_blank?).to be(false) }
@ -60,6 +54,15 @@ describe Champ do
end
end
context 'when repetition not blank' do
let(:procedure) { create(:procedure, types_de_champ_public: [{ type: :repetition, children: [{ type: :text }] }]) }
let(:dossier) { create(:dossier, :with_populated_champs, procedure:) }
let(:champ) { dossier.champs.find(&:repetition?) }
it { expect(champ.blank?).to be(false) }
end
end
describe "associations" do
it { is_expected.to belong_to(:dossier) }
@ -132,7 +135,7 @@ describe Champ do
let(:standalone_champ) { build(:champ, type_de_champ: build(:type_de_champ), dossier: build(:dossier)) }
let(:public_sections) { dossier.champs_public.filter(&:header_section?) }
let(:private_sections) { dossier.champs_private.filter(&:header_section?) }
let(:sections_in_repetition) { champ_in_repetition.parent.champs.filter(&:header_section?) }
let(:sections_in_repetition) { dossier.champs.filter(&:child?).filter(&:header_section?) }
it 'returns the sibling sections of a champ' do
expect(public_sections).not_to be_empty

View file

@ -2,7 +2,10 @@
describe Champs::CnafChamp, type: :model do
let(:champ) { described_class.new(dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_cnaf)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_cnaf))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
describe 'numero_allocataire and code_postal' do
before do
champ.numero_allocataire = '1234567'

View file

@ -6,6 +6,7 @@ describe Champs::DecimalNumberChamp do
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_decimal_number))
allow(champ).to receive(:visible?).and_return(true)
allow(champ).to receive(:in_dossier_revision?).and_return(true)
champ.run_callbacks(:validation)
end
subject { champ.validate(:champs_public_value) }

View file

@ -4,7 +4,10 @@ describe Champs::DepartementChamp, type: :model do
describe 'validations' do
describe 'external link' do
let(:champ) { described_class.new(external_id: external_id, dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_departements)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_departements))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
subject { champ.validate(:champs_public_value) }
context 'when nil' do

View file

@ -40,7 +40,10 @@ describe Champs::DgfipChamp, type: :model do
let(:numero_fiscal) { '1122299999092' }
let(:reference_avis) { 'FC22299999092' }
let(:champ) { described_class.new(dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dgfip)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_dgfip))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
let(:validation_context) { :champs_public_value }
subject { champ.valid?(validation_context) }

View file

@ -4,7 +4,10 @@ describe Champs::DropDownListChamp do
describe 'validations' do
describe 'inclusion' do
let(:champ) { described_class.new(other:, value:, dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_drop_down_list, drop_down_other: other)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_drop_down_list, drop_down_other: other))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
subject { champ.validate(:champs_public_value) }
context 'when the other value is accepted' do

View file

@ -3,7 +3,10 @@
describe Champs::EmailChamp do
describe 'validation' do
let(:champ) { described_class.new(value:, dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_email)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_email))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
subject { champ.validate(:champs_public_value) }
context 'when nil' do

View file

@ -7,7 +7,10 @@ describe Champs::EngagementJuridiqueChamp do
.new(dossier: build(:dossier))
.tap { _1.value = value }
end
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_engagement_juridique)) }
before {
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_engagement_juridique))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
}
subject { champ.validate(:champs_public_value) }
context 'with [A-Z]' do

View file

@ -6,7 +6,10 @@ describe Champs::EpciChamp, type: :model do
describe 'code_departement' do
let(:champ) { Champs::EpciChamp.new(code_departement: code_departement, dossier: build(:dossier)) }
before { allow(champ).to receive(:visible?).and_return(true) }
before do
allow(champ).to receive(:visible?).and_return(true)
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
context 'when nil' do
let(:code_departement) { nil }

View file

@ -3,7 +3,10 @@
describe Champs::IbanChamp do
describe '#valid?' do
let(:champ) { Champs::IbanChamp.new(dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_iban)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_iban))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
def with_value(value)
champ.tap { _1.value = value }
end

View file

@ -2,7 +2,10 @@
describe Champs::IntegerNumberChamp do
let(:champ) { Champs::IntegerNumberChamp.new(value:, dossier: build(:dossier)) }
before { allow(champ).to receive(:visible?).and_return(true) }
before do
allow(champ).to receive(:visible?).and_return(true)
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
subject { champ.validate(:champs_public_value) }
describe '#valid?' do

View file

@ -4,7 +4,10 @@ describe Champs::MultipleDropDownListChamp do
let(:type_de_champ) { build(:type_de_champ_multiple_drop_down_list, drop_down_list_value: "val1\r\nval2\r\nval3\r\n[brackets] val4") }
let(:value) { nil }
let(:champ) { Champs::MultipleDropDownListChamp.new(value:, dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(type_de_champ) }
before do
allow(champ).to receive(:type_de_champ).and_return(type_de_champ)
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
describe 'validations' do
subject { champ.validate(:champs_public_value) }

View file

@ -2,7 +2,10 @@
describe Champs::PhoneChamp do
let(:champ) { Champs::PhoneChamp.new(dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_phone)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_phone))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
describe '#validate' do
it do
expect(champ_with_value(nil).validate(:champs_public_value)).to be_truthy

View file

@ -2,7 +2,10 @@
describe Champs::RNAChamp do
let(:champ) { Champs::RNAChamp.new(value: "W182736273", dossier: build(:dossier)) }
before { allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_rna)) }
before do
allow(champ).to receive(:type_de_champ).and_return(build(:type_de_champ_rna))
allow(champ).to receive(:in_dossier_revision?).and_return(true)
end
def with_value(value)
champ.tap { _1.value = value }
end

View file

@ -38,10 +38,8 @@ describe DossierSearchableConcern do
end
it "update columns en construction" do
dossier.update(
champs_public_attributes: [{ id: champ_public.id, value: 'nouvelle valeur publique' }],
champs_private_attributes: [{ id: champ_private.id, value: 'nouvelle valeur privee' }]
)
dossier.update_champs_attributes({ champ_public.public_id => { value: 'nouvelle valeur publique' } }, :public, updated_by: 'test')
dossier.update_champs_attributes({ champ_private.public_id => { value: 'nouvelle valeur privee' } }, :private, updated_by: 'test')
assert_enqueued_jobs(1, only: DossierIndexSearchTermsJob) do
dossier.passer_en_construction

View file

@ -50,12 +50,12 @@ describe DossierSectionsConcern do
describe '#index_for_section_header' do
include Logic
let(:number_stable_id) { 99 }
let(:types_de_champ) {
let(:types_de_champ) do
[
{ type: :header_section, libelle: "Infos" }, { type: :integer_number, stable_id: number_stable_id },
{ type: :header_section, libelle: "Details", condition: ds_eq(champ_value(99), constant(5)) }, { type: :header_section, libelle: "Conclusion" }
]
}
end
let(:procedure) { create(:procedure, :for_individual, types_de_champ_public: types_de_champ) }
let(:dossier) { create(:dossier, procedure: procedure) }

View file

@ -390,11 +390,11 @@ describe ProcedureExportService do
context 'with long libelle composed of utf8 characteres' do
before do
procedure.active_revision.types_de_champ_public.each do |c|
c.update!(libelle: "#{c.id} - ?/[] ééé ééé ééééééé ééééééé éééééééé. ééé éé éééééééé éé ééé. ééééé éééééééé ééé ééé.")
procedure.active_revision.types_de_champ_public.each do |type_de_champ|
type_de_champ.update!(libelle: "#{type_de_champ.id} - ?/[] ééé ééé ééééééé ééééééé éééééééé. ééé éé éééééééé éé ééé. ééééé éééééééé ééé ééé.")
end
champ_repetition.champs.each do |c|
c.type_de_champ.update!(libelle: "#{c.id} - Quam rem nam maiores numquam dolorem nesciunt. Cum et possimus et aut. Fugit voluptas qui qui.")
procedure.active_revision.children_of(champ_repetition.type_de_champ).each do |type_de_champ|
type_de_champ.update!(libelle: "#{type_de_champ.id} - Quam rem nam maiores numquam dolorem nesciunt. Cum et possimus et aut. Fugit voluptas qui qui.")
end
end
@ -417,7 +417,7 @@ describe ProcedureExportService do
context 'with empty repetition' do
before do
dossiers.flat_map { |dossier| dossier.champs_public.filter(&:repetition?) }.each do |champ|
champ.champs.destroy_all
Champ.where(row_id: champ.row_ids).destroy_all
end
end