From 1021a31f7bd635bbcbc722b8bdae1429a7672cae Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 27 Mar 2024 11:39:15 +0100 Subject: [PATCH] refactor(dossier): use update_champs_private and update_champs_public methods --- .../concerns/turbo_champs_concern.rb | 9 +- .../instructeurs/dossiers_controller.rb | 49 ++++++++--- app/controllers/users/dossiers_controller.rb | 31 +++++-- app/models/champ.rb | 3 + .../instructeurs/dossiers_controller_spec.rb | 87 +++++++++++++++---- .../users/dossiers_controller_spec.rb | 28 +++--- 6 files changed, 155 insertions(+), 52 deletions(-) diff --git a/app/controllers/concerns/turbo_champs_concern.rb b/app/controllers/concerns/turbo_champs_concern.rb index af5be0438..1a8fc819f 100644 --- a/app/controllers/concerns/turbo_champs_concern.rb +++ b/app/controllers/concerns/turbo_champs_concern.rb @@ -4,9 +4,14 @@ module TurboChampsConcern private def champs_to_turbo_update(params, champs) - champ_ids = params.keys.map(&:to_i) + to_update = if params.values.filter { _1.key?(:with_public_id) }.empty? + champ_ids = params.keys.map(&:to_i) + champs.filter { _1.id.in?(champ_ids) } + else + champ_public_ids = params.keys + champs.filter { _1.public_id.in?(champ_public_ids) } + end.filter { _1.refresh_after_update? || _1.forked_with_changes? } - to_update = champs.filter { _1.id.in?(champ_ids) && (_1.refresh_after_update? || _1.forked_with_changes?) } to_show, to_hide = champs.filter(&:conditional?) .partition(&:visible?) .map { champs_to_one_selector(_1 - to_update) } diff --git a/app/controllers/instructeurs/dossiers_controller.rb b/app/controllers/instructeurs/dossiers_controller.rb index 51c45ebe6..212a6c8b0 100644 --- a/app/controllers/instructeurs/dossiers_controller.rb +++ b/app/controllers/instructeurs/dossiers_controller.rb @@ -273,8 +273,8 @@ module Instructeurs end def update_annotations - dossier_with_champs.assign_attributes(champs_private_params) - if dossier.champs_private_all.any?(&:changed?) + dossier_with_champs.update_champs_attributes(champs_private_attributes_params, :private) + if dossier.champs.any?(&:changed_for_autosave?) || dossier.champs_private_all.any?(&:changed_for_autosave?) # TODO remove second condition after one deploy dossier.last_champ_private_updated_at = Time.zone.now end @@ -282,7 +282,7 @@ module Instructeurs respond_to do |format| format.turbo_stream do - @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_private_params.fetch(:champs_private_all_attributes), dossier.champs_private_all) + @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_private_attributes_params, dossier.champs.filter(&:private?)) end end end @@ -294,7 +294,12 @@ module Instructeurs def annotation @dossier = dossier_with_champs(pj_template: false) - annotation = @dossier.champs_private_all.find(params[:annotation_id]) + annotation = if params[:with_public_id].present? + type_de_champ = @dossier.find_type_de_champ_by_stable_id(params[:annotation_id], :private) + @dossier.project_champ(type_de_champ, params[:row_id]) + else + @dossier.champs_private_all.find(params[:annotation_id]) + end respond_to do |format| format.turbo_stream do @@ -392,12 +397,36 @@ module Instructeurs end def champs_private_params - champs_params = params.require(:dossier).permit(champs_private_attributes: [ - :id, :value, :primary_value, :secondary_value, :piece_justificative_file, :value_other, :external_id, :numero_allocataire, :code_postal, :code_departement, value: [], - champs_attributes: [:id, :_destroy, :value, :primary_value, :secondary_value, :piece_justificative_file, :value_other, :external_id, :numero_allocataire, :code_postal, :code_departement, :feature, value: []] - ]) - champs_params[:champs_private_all_attributes] = champs_params.delete(:champs_private_attributes) || {} - champs_params + champ_attributes = [ + :id, + :value, + :value_other, + :external_id, + :primary_value, + :secondary_value, + :numero_allocataire, + :code_postal, + :identifiant, + :numero_fiscal, + :reference_avis, + :ine, + :piece_justificative_file, + :code_departement, + :accreditation_number, + :accreditation_birthdate, + :feature, + :with_public_id, + value: [] + ] + # Strong attributes do not support records (indexed hash); they only support hashes with + # static keys. We create a static hash based on the available keys. + public_ids = params.dig(:dossier, :champs_private_attributes)&.keys || [] + champs_private_attributes = public_ids.map { [_1, champ_attributes] }.to_h + params.require(:dossier).permit(champs_private_attributes:) + end + + def champs_private_attributes_params + champs_private_params.fetch(:champs_private_attributes) end def mark_demande_as_read diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 00234d794..06dd6d742 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -284,7 +284,7 @@ module Users end format.turbo_stream do - @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_public_params.fetch(:champs_public_all_attributes), dossier.champs_public_all) + @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_public_attributes_params, dossier.champs.filter(&:public?)) render :update, layout: false end end @@ -298,7 +298,7 @@ module Users respond_to do |format| format.turbo_stream do - @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_public_params.fetch(:champs_public_all_attributes), dossier.champs_public_all) + @to_show, @to_hide, @to_update = champs_to_turbo_update(champs_public_attributes_params, dossier.champs.filter(&:public?)) render :update, layout: false end end @@ -310,7 +310,12 @@ module Users def champ @dossier = dossier_with_champs(pj_template: false) - champ = @dossier.champs_public_all.find(params[:champ_id]) + champ = if params[:with_public_id].present? + type_de_champ = @dossier.find_type_de_champ_by_stable_id(params[:champ_id], :public) + @dossier.project_champ(type_de_champ, params[:row_id]) + else + @dossier.champs_public_all.find(params[:champ_id]) + end respond_to do |format| format.turbo_stream do @@ -478,7 +483,7 @@ module Users end def champs_public_params - champs_params = params.require(:dossier).permit(champs_public_attributes: [ + champ_attributes = [ :id, :value, :value_other, @@ -496,10 +501,18 @@ module Users :accreditation_number, :accreditation_birthdate, :feature, + :with_public_id, value: [] - ]) - champs_params[:champs_public_all_attributes] = champs_params.delete(:champs_public_attributes) || {} - champs_params + ] + # Strong attributes do not support records (indexed hash); they only support hashes with + # static keys. We create a static hash based on the available keys. + public_ids = params.dig(:dossier, :champs_public_attributes)&.keys || [] + champs_public_attributes = public_ids.map { [_1, champ_attributes] }.to_h + params.require(:dossier).permit(champs_public_attributes:) + end + + def champs_public_attributes_params + champs_public_params.fetch(:champs_public_attributes) end def dossier_scope @@ -532,8 +545,8 @@ module Users end def update_dossier_and_compute_errors - @dossier.assign_attributes(champs_public_params) - if @dossier.champs_public_all.any?(&:changed_for_autosave?) + @dossier.update_champs_attributes(champs_public_attributes_params, :public) + if @dossier.champs.any?(&:changed_for_autosave?) || @dossier.champs_public_all.any?(&:changed_for_autosave?) # TODO remove second condition after one deploy @dossier.last_champ_updated_at = Time.zone.now end diff --git a/app/models/champ.rb b/app/models/champ.rb index b3672d9f0..fc7ee4aa1 100644 --- a/app/models/champ.rb +++ b/app/models/champ.rb @@ -2,6 +2,9 @@ class Champ < ApplicationRecord include ChampConditionalConcern include ChampsValidateConcern + # TODO: remove after one deploy + attr_writer :with_public_id + belongs_to :dossier, 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 diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb index 108c675ba..f4f065948 100644 --- a/spec/controllers/instructeurs/dossiers_controller_spec.rb +++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb @@ -1005,25 +1005,25 @@ describe Instructeurs::DossiersController, type: :controller do dossier_id: dossier.id, dossier: { champs_private_attributes: { - '0': { - id: champ_multiple_drop_down_list.id, + champ_multiple_drop_down_list.public_id => { + with_public_id: true, value: ['', 'val1', 'val2'] }, - '1': { - id: champ_datetime.id, + champ_datetime.public_id => { + with_public_id: true, value: '2019-12-21T13:17' }, - '2': { - id: champ_linked_drop_down_list.id, + champ_linked_drop_down_list.public_id => { + with_public_id: true, primary_value: 'primary', secondary_value: 'secondary' }, - '3': { - id: champ_repetition.champs.first.id, + champ_repetition.champs.first.public_id => { + with_public_id: true, value: 'text' }, - '4': { - id: champ_drop_down_list.id, + champ_drop_down_list.public_id => { + with_public_id: true, value: '__other__', value_other: 'other value' } @@ -1074,12 +1074,11 @@ describe Instructeurs::DossiersController, type: :controller do end end - context 'with invalid champs_public (DecimalNumberChamp)' do - let(:types_de_champ_public) do - [ - { type: :decimal_number } - ] - end + after do + Timecop.return + end + + context "with new values for champs_private (legacy)" do let(:params) do { procedure_id: procedure.id, @@ -1095,9 +1094,63 @@ describe Instructeurs::DossiersController, type: :controller do } end + it 'update champs_private' do + patch :update_annotations, params: params, format: :turbo_stream + champ_datetime.reload + expect(champ_datetime.value).to eq(Time.zone.parse('2024-03-30T07:03:00').iso8601) + end + end + + context "without new values for champs_private" do + let(:params) do + { + procedure_id: procedure.id, + dossier_id: dossier.id, + dossier: { + champs_private_attributes: {}, + champs_public_attributes: { + champ_multiple_drop_down_list.public_id => { + with_public_id: true, + value: ['', 'val1', 'val2'] + } + } + } + } + end + + it { + expect(dossier.reload.last_champ_private_updated_at).to eq(nil) + expect(response).to have_http_status(200) + } + end + + context "with invalid champs_public (DecimalNumberChamp)" do + let(:types_de_champ_public) do + [ + { type: :decimal_number } + ] + end + + let(:champ_decimal_number) { dossier.champs_public.first } + + let(:params) do + { + procedure_id: procedure.id, + dossier_id: dossier.id, + dossier: { + champs_private_attributes: { + champ_datetime.public_id => { + with_public_id: true, + value: '2024-03-30T07:03' + } + } + } + } + end + it 'update champs_private' do too_long_float = '3.1415' - dossier.champs_public.first.update_column(:value, too_long_float) + champ_decimal_number.update_column(:value, too_long_float) patch :update_annotations, params: params, format: :turbo_stream champ_datetime.reload expect(champ_datetime.value).to eq(Time.zone.parse('2024-03-30T07:03:00').iso8601) diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 5253ba54c..ad99422a4 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -674,12 +674,12 @@ describe Users::DossiersController, type: :controller do dossier: { groupe_instructeur_id: dossier.groupe_instructeur_id, champs_public_attributes: { - first_champ.id => { - id: first_champ.id, + first_champ.public_id => { + with_public_id: true, value: value }, - piece_justificative_champ.id => { - id: piece_justificative_champ.id, + piece_justificative_champ.public_id => { + with_public_id: true, piece_justificative_file: file } } @@ -719,7 +719,7 @@ describe Users::DossiersController, type: :controller do { id: dossier.id, dossier: { - champs_public_attributes: { first_champ.id => { id: first_champ.id } } + champs_public_attributes: { first_champ.public_id => { with_public_id: true } } } } end @@ -747,7 +747,7 @@ describe Users::DossiersController, type: :controller do { id: dossier.id, dossier: { - champs_public_attributes: { first_champ.id => { id: first_champ.id, value: value } } + champs_public_attributes: { first_champ.public_id => { with_public_id: true, value: value } } } } end @@ -790,12 +790,12 @@ describe Users::DossiersController, type: :controller do dossier: { groupe_instructeur_id: dossier.groupe_instructeur_id, champs_public_attributes: { - first_champ.id => { - id: first_champ.id, + first_champ.public_id => { + with_public_id: true, value: value }, - piece_justificative_champ.id => { - id: piece_justificative_champ.id, + piece_justificative_champ.public_id => { + with_public_id: true, piece_justificative_file: file } } @@ -855,8 +855,8 @@ describe Users::DossiersController, type: :controller do id: dossier.id, dossier: { champs_public_attributes: { - piece_justificative_champ.id => { - id: piece_justificative_champ.id, + piece_justificative_champ.public_id => { + with_public_id: true, piece_justificative_file: file } } @@ -951,8 +951,8 @@ describe Users::DossiersController, type: :controller do id: dossier.id, dossier: { champs_public_attributes: { - first_champ.id => { - id: first_champ.id, + first_champ.public_id => { + with_public_id: true, value: value } }