From 16e93a217b07155455e68982b3ac1a86bdcd2c80 Mon Sep 17 00:00:00 2001 From: simon lehericey Date: Fri, 11 Oct 2024 15:29:52 +0200 Subject: [PATCH] reset procedure_presentation if a pb occurs with a column deserialization --- app/models/assign_to.rb | 21 ++++++++------ app/models/column.rb | 5 ++++ app/models/procedure_presentation.rb | 4 +-- app/models/sorted_column.rb | 5 ++++ spec/models/assign_to_spec.rb | 43 +++++++++++++++++----------- 5 files changed, 51 insertions(+), 27 deletions(-) diff --git a/app/models/assign_to.rb b/app/models/assign_to.rb index bfd7990e0..d9f6071c5 100644 --- a/app/models/assign_to.rb +++ b/app/models/assign_to.rb @@ -10,28 +10,31 @@ class AssignTo < ApplicationRecord def procedure_presentation_or_default_and_errors errors = reset_procedure_presentation_if_invalid + if self.procedure_presentation.nil? - self.procedure_presentation = build_procedure_presentation - self.procedure_presentation.save if procedure_presentation.valid? && !procedure_presentation.persisted? + self.procedure_presentation = create_procedure_presentation! end + [self.procedure_presentation, errors] end private def reset_procedure_presentation_if_invalid - if procedure_presentation&.invalid? - # This is a last defense against invalid `ProcedurePresentation`s persistently - # hindering instructeurs. Whenever this gets triggered, it means that there is - # a bug somewhere else that we need to fix. + errors = begin + procedure_presentation.errors if procedure_presentation&.invalid? + rescue ActiveRecord::RecordNotFound => e + [e.message] + end - errors = procedure_presentation.errors + if errors.present? Sentry.capture_message( "Destroying invalid ProcedurePresentation", - extra: { procedure_presentation: procedure_presentation.as_json } + extra: { procedure_presentation_id: procedure_presentation.id, errors: } ) self.procedure_presentation = nil - errors end + + errors end end diff --git a/app/models/column.rb b/app/models/column.rb index 1a6871c65..2e95a9c3e 100644 --- a/app/models/column.rb +++ b/app/models/column.rb @@ -1,6 +1,11 @@ # frozen_string_literal: true class Column + # include validations to enable procedure_presentation.validate_associate, + # which enforces the deserialization of columns in the displayed_columns attribute + # and raises an error if a column is not found + include ActiveModel::Validations + TYPE_DE_CHAMP_TABLE = 'type_de_champ' attr_reader :table, :column, :label, :type, :scope, :value_column, :filterable, :displayable diff --git a/app/models/procedure_presentation.rb b/app/models/procedure_presentation.rb index 57f61d796..37247174a 100644 --- a/app/models/procedure_presentation.rb +++ b/app/models/procedure_presentation.rb @@ -24,8 +24,8 @@ class ProcedurePresentation < ApplicationRecord before_create { self.displayed_columns = procedure.default_displayed_columns } - validates_associated :a_suivre_filters, :suivis_filters, :traites_filters, - :tous_filters, :supprimes_filters, :expirant_filters, :archives_filters + validates_associated :displayed_columns, :sorted_column, :a_suivre_filters, :suivis_filters, + :traites_filters, :tous_filters, :supprimes_filters, :expirant_filters, :archives_filters def filters_for(statut) send(filters_name_for(statut)) diff --git a/app/models/sorted_column.rb b/app/models/sorted_column.rb index d254405d7..d92036885 100644 --- a/app/models/sorted_column.rb +++ b/app/models/sorted_column.rb @@ -1,6 +1,11 @@ # frozen_string_literal: true class SortedColumn + # include validations to enable procedure_presentation.validate_associate, + # which enforces the deserialization of columns in the sorted_column attribute + # and raises an error if a column is not found + include ActiveModel::Validations + attr_reader :column, :order def initialize(column:, order:) diff --git a/spec/models/assign_to_spec.rb b/spec/models/assign_to_spec.rb index 8147d5bd0..59aaf55dd 100644 --- a/spec/models/assign_to_spec.rb +++ b/spec/models/assign_to_spec.rb @@ -9,32 +9,43 @@ describe AssignTo, type: :model do let(:procedure_presentation_or_default) { procedure_presentation_and_errors.first } let(:errors) { procedure_presentation_and_errors.second } - context "without a procedure_presentation" do - it { expect(procedure_presentation_or_default).to be_persisted } - it { expect(procedure_presentation_or_default).to be_valid } - it { expect(errors).to be_nil } + context "without a preexisting procedure_presentation" do + it 'creates a default pp' do + expect(procedure_presentation_or_default).to be_persisted + expect(procedure_presentation_or_default).to be_valid + expect(errors).to be_nil + end end - context "with a procedure_presentation" do - let!(:procedure_presentation) { ProcedurePresentation.create(assign_to: assign_to) } + context "with a preexisting procedure_presentation" do + let!(:procedure_presentation) { ProcedurePresentation.create(assign_to:) } - it { expect(procedure_presentation_or_default).to eq(procedure_presentation) } - it { expect(procedure_presentation_or_default).to be_valid } - it { expect(errors).to be_nil } + it 'returns the preexisting pp' do + expect(procedure_presentation_or_default).to eq(procedure_presentation) + expect(procedure_presentation_or_default).to be_valid + expect(errors).to be_nil + end end context "with an invalid procedure_presentation" do let!(:procedure_presentation) do - pp = ProcedurePresentation.new(assign_to: assign_to, displayed_fields: [{ 'table' => 'invalid', 'column' => 'random' }]) - pp.save(validate: false) - pp + pp = ProcedurePresentation.create(assign_to: assign_to) + + sql = <<-SQL.squish + UPDATE procedure_presentations + SET displayed_columns = ARRAY['{\"procedure_id\":666}'::jsonb] + WHERE id = #{pp.id} ; + SQL + + pp.class.connection.execute(sql) + + assign_to.reload end - it { expect(procedure_presentation_or_default).to be_persisted } - it { expect(procedure_presentation_or_default).to be_valid } - it { expect(errors).to be_present } it do - procedure_presentation_or_default + expect(procedure_presentation_or_default).to be_persisted + expect(procedure_presentation_or_default).to be_valid + expect(errors).to be_present expect(assign_to.procedure_presentation).not_to be(procedure_presentation) end end