From 4e7733e5710acd83a3dc889fcf86e14420295bbc Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 Jan 2022 18:26:34 +0100 Subject: [PATCH 1/2] Revert "feat(attestation): revise attestations" This reverts commit e413872530a2cfe13dcc12cdc59f32f886b926c1. --- .../attestation_templates_controller.rb | 20 ++++------------ app/models/attestation_template.rb | 22 +---------------- app/models/dossier.rb | 10 ++++---- app/models/procedure.rb | 14 +++-------- app/models/procedure_revision.rb | 16 ++++--------- .../administrateurs/procedures/show.html.haml | 2 +- ...tion_template_id_to_procedure_revisions.rb | 5 ---- db/schema.rb | 5 +--- .../attestation_templates_controller_spec.rb | 24 +++++++++---------- spec/factories/attestation_template.rb | 2 ++ spec/factories/procedure.rb | 3 +-- spec/models/procedure_spec.rb | 10 +++++--- ...tion_inconsistency_alert.html.haml_spec.rb | 5 ++-- 13 files changed, 44 insertions(+), 94 deletions(-) delete mode 100644 db/migrate/20211214145059_add_attestation_template_id_to_procedure_revisions.rb diff --git a/app/controllers/administrateurs/attestation_templates_controller.rb b/app/controllers/administrateurs/attestation_templates_controller.rb index d1f66ebad..aff0bb793 100644 --- a/app/controllers/administrateurs/attestation_templates_controller.rb +++ b/app/controllers/administrateurs/attestation_templates_controller.rb @@ -3,17 +3,12 @@ module Administrateurs before_action :retrieve_procedure def edit - @attestation_template = build_attestation + @attestation_template = @procedure.attestation_template || AttestationTemplate.new(procedure: @procedure) end def update - attestation_template = @procedure.draft_attestation_template.revise! - + attestation_template = @procedure.attestation_template if attestation_template.update(activated_attestation_params) - AttestationTemplate - .where(id: @procedure.revisions.pluck(:attestation_template_id).compact) - .update_all(activated: attestation_template.activated?) - flash.notice = "L'attestation a bien été modifiée" else flash.alert = attestation_template.errors.full_messages.join('
') @@ -23,7 +18,7 @@ module Administrateurs end def create - attestation_template = build_attestation(activated_attestation_params) + attestation_template = AttestationTemplate.new(activated_attestation_params.merge(procedure_id: @procedure.id)) if attestation_template.save flash.notice = "L'attestation a bien été sauvegardée" @@ -35,19 +30,14 @@ module Administrateurs end def preview - @attestation = build_attestation.render_attributes_for({}) + attestation = @procedure.attestation_template || AttestationTemplate.new + @attestation = attestation.render_attributes_for({}) render 'administrateurs/attestation_templates/show', formats: [:pdf] end private - def build_attestation(attributes = {}) - attestation_template = @procedure.draft_attestation_template || @procedure.draft_revision.build_attestation_template - attestation_template.attributes = attributes - attestation_template - end - def activated_attestation_params # cache result to avoid multiple uninterlaced computations if @activated_attestation_params.nil? diff --git a/app/models/attestation_template.rb b/app/models/attestation_template.rb index deab7b0a2..6062589e8 100644 --- a/app/models/attestation_template.rb +++ b/app/models/attestation_template.rb @@ -15,8 +15,7 @@ class AttestationTemplate < ApplicationRecord include ActionView::Helpers::NumberHelper include TagsSubstitutionConcern - belongs_to :procedure, optional: true - has_many :revisions, class_name: 'ProcedureRevision', inverse_of: :attestation_template, dependent: :nullify + belongs_to :procedure, optional: false has_one_attached :logo has_one_attached :signature @@ -104,25 +103,6 @@ class AttestationTemplate < ApplicationRecord } end - def revise! - if revisions.size > 1 - attestation_template = dup - attestation_template.save! - revisions - .last - .procedure - .draft_revision - .update!(attestation_template: attestation_template) - attestation_template - else - self - end - end - - def procedure - revisions.last&.procedure || super - end - private def used_tags diff --git a/app/models/dossier.rb b/app/models/dossier.rb index 6e1a5adfc..b1dcc6b0a 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -720,11 +720,9 @@ class Dossier < ApplicationRecord { lon: lon, lat: lat, zoom: zoom } end - def attestation_template - revision.attestation_template - end - def unspecified_attestation_champs + attestation_template = procedure.attestation_template + if attestation_template&.activated? attestation_template.unspecified_champs_for_dossier(self) else @@ -733,8 +731,8 @@ class Dossier < ApplicationRecord end def build_attestation - if attestation_template&.activated? - attestation_template.attestation_for(self) + if procedure.attestation_template&.activated? + procedure.attestation_template.attestation_for(self) end end diff --git a/app/models/procedure.rb b/app/models/procedure.rb index a3de8ada3..99482064b 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -77,15 +77,12 @@ class Procedure < ApplicationRecord has_many :published_types_de_champ_private, through: :published_revision, source: :types_de_champ_private has_many :draft_types_de_champ, through: :draft_revision, source: :types_de_champ has_many :draft_types_de_champ_private, through: :draft_revision, source: :types_de_champ_private - has_one :draft_attestation_template, through: :draft_revision, source: :attestation_template - has_one :published_attestation_template, through: :published_revision, source: :attestation_template has_many :experts_procedures, dependent: :destroy has_many :experts, through: :experts_procedures has_one :module_api_carto, dependent: :destroy has_one :attestation_template, dependent: :destroy - has_many :attestation_templates, through: :revisions, source: :attestation_template belongs_to :parent_procedure, class_name: 'Procedure', optional: true belongs_to :canonical_procedure, class_name: 'Procedure', optional: true @@ -437,8 +434,7 @@ class Procedure < ApplicationRecord }, revision_types_de_champ_private: { type_de_champ: :types_de_champ - }, - attestation_template: [] + } } } include_list[:groupe_instructeurs] = :instructeurs if !is_different_admin @@ -576,17 +572,13 @@ class Procedure < ApplicationRecord touch(:whitelisted_at) end - def active_attestation_template - published_attestation_template || draft_attestation_template - end - def closed_mail_template_attestation_inconsistency_state # As an optimization, don’t check the predefined templates (they are presumed correct) if closed_mail.present? tag_present = closed_mail.body.to_s.include?("--lien attestation--") - if active_attestation_template&.activated? && !tag_present + if attestation_template&.activated? && !tag_present :missing_tag - elsif !active_attestation_template&.activated? && tag_present + elsif !attestation_template&.activated? && tag_present :extraneous_tag end end diff --git a/app/models/procedure_revision.rb b/app/models/procedure_revision.rb index da60bb296..7c0a19c48 100644 --- a/app/models/procedure_revision.rb +++ b/app/models/procedure_revision.rb @@ -2,17 +2,15 @@ # # Table name: procedure_revisions # -# id :bigint not null, primary key -# published_at :datetime -# created_at :datetime not null -# updated_at :datetime not null -# attestation_template_id :bigint -# procedure_id :bigint not null +# id :bigint not null, primary key +# published_at :datetime +# created_at :datetime not null +# updated_at :datetime not null +# procedure_id :bigint not null # class ProcedureRevision < ApplicationRecord self.implicit_order_column = :created_at belongs_to :procedure, -> { with_discarded }, inverse_of: :revisions, optional: false - belongs_to :attestation_template, inverse_of: :revisions, optional: true, dependent: :destroy has_many :dossiers, inverse_of: :revision, foreign_key: :revision_id @@ -127,10 +125,6 @@ class ProcedureRevision < ApplicationRecord ) end - def attestation_template - super || procedure.attestation_template - end - private def compare_types_de_champ(from_tdc, to_tdc) diff --git a/app/views/administrateurs/procedures/show.html.haml b/app/views/administrateurs/procedures/show.html.haml index 7c328396e..3c5059cfb 100644 --- a/app/views/administrateurs/procedures/show.html.haml +++ b/app/views/administrateurs/procedures/show.html.haml @@ -138,7 +138,7 @@ .procedure-grid = link_to edit_admin_procedure_attestation_template_path(@procedure), class: 'card-admin' do - - if @procedure.draft_attestation_template&.activated? + - if @procedure.attestation_template.present? && @procedure.attestation_template.activated %div %span.icon.accept %p.card-admin-status-accept Activée diff --git a/db/migrate/20211214145059_add_attestation_template_id_to_procedure_revisions.rb b/db/migrate/20211214145059_add_attestation_template_id_to_procedure_revisions.rb deleted file mode 100644 index b0ffc5b82..000000000 --- a/db/migrate/20211214145059_add_attestation_template_id_to_procedure_revisions.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddAttestationTemplateIdToProcedureRevisions < ActiveRecord::Migration[6.1] - def change - add_reference :procedure_revisions, :attestation_template, foreign_key: { to_table: :attestation_templates }, null: true, index: true - end -end diff --git a/db/schema.rb b/db/schema.rb index aac61da7a..aef752fc6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_01_10_133139) do +ActiveRecord::Schema.define(version: 2021_12_02_133139) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -593,8 +593,6 @@ ActiveRecord::Schema.define(version: 2022_01_10_133139) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.datetime "published_at" - t.bigint "attestation_template_id" - t.index ["attestation_template_id"], name: "index_procedure_revisions_on_attestation_template_id" t.index ["procedure_id"], name: "index_procedure_revisions_on_procedure_id" end @@ -874,7 +872,6 @@ ActiveRecord::Schema.define(version: 2022_01_10_133139) do add_foreign_key "procedure_revision_types_de_champ", "procedure_revision_types_de_champ", column: "parent_id" add_foreign_key "procedure_revision_types_de_champ", "procedure_revisions", column: "revision_id" add_foreign_key "procedure_revision_types_de_champ", "types_de_champ" - add_foreign_key "procedure_revisions", "attestation_templates" add_foreign_key "procedure_revisions", "procedures" add_foreign_key "procedures", "procedure_revisions", column: "draft_revision_id" add_foreign_key "procedures", "procedure_revisions", column: "published_revision_id" diff --git a/spec/controllers/administrateurs/attestation_templates_controller_spec.rb b/spec/controllers/administrateurs/attestation_templates_controller_spec.rb index 451bd30f0..401eb0be1 100644 --- a/spec/controllers/administrateurs/attestation_templates_controller_spec.rb +++ b/spec/controllers/administrateurs/attestation_templates_controller_spec.rb @@ -1,8 +1,8 @@ include ActionDispatch::TestProcess describe Administrateurs::AttestationTemplatesController, type: :controller do + let!(:attestation_template) { create(:attestation_template) } let(:admin) { create(:administrateur) } - let(:attestation_template) { build(:attestation_template) } let!(:procedure) { create :procedure, administrateur: admin, attestation_template: attestation_template } let(:logo) { fixture_file_upload('spec/fixtures/files/white.png', 'image/png') } let(:logo2) { fixture_file_upload('spec/fixtures/files/white.png', 'image/png') } @@ -41,7 +41,7 @@ describe Administrateurs::AttestationTemplatesController, type: :controller do end context 'if an attestation template exists on the procedure' do - after { procedure.draft_revision.attestation_template&.destroy } + after { procedure.attestation_template.destroy } context 'with images' do let!(:attestation_template) do @@ -115,14 +115,14 @@ describe Administrateurs::AttestationTemplatesController, type: :controller do procedure.reload end - it { expect(procedure.draft_attestation_template).to have_attributes(attestation_params) } - it { expect(procedure.draft_attestation_template.activated).to be true } - it { expect(procedure.draft_attestation_template.logo.download).to eq(logo2.read) } - it { expect(procedure.draft_attestation_template.signature.download).to eq(signature2.read) } + it { expect(procedure.attestation_template).to have_attributes(attestation_params) } + it { expect(procedure.attestation_template.activated).to be true } + it { expect(procedure.attestation_template.logo.download).to eq(logo2.read) } + it { expect(procedure.attestation_template.signature.download).to eq(signature2.read) } it { expect(response).to redirect_to edit_admin_procedure_attestation_template_path(procedure) } it { expect(flash.notice).to eq("L'attestation a bien été sauvegardée") } - after { procedure.draft_attestation_template.destroy } + after { procedure.attestation_template.destroy } end context 'when something wrong happens in the attestation template creation' do @@ -140,7 +140,7 @@ describe Administrateurs::AttestationTemplatesController, type: :controller do it { expect(response).to redirect_to edit_admin_procedure_attestation_template_path(procedure) } it { expect(flash.alert).to be_present } - it { expect(procedure.draft_attestation_template).to be nil } + it { expect(procedure.attestation_template).to be nil } end end @@ -158,13 +158,13 @@ describe Administrateurs::AttestationTemplatesController, type: :controller do procedure.reload end - it { expect(procedure.draft_attestation_template).to have_attributes(attestation_params) } - it { expect(procedure.draft_attestation_template.logo.download).to eq(logo2.read) } - it { expect(procedure.draft_attestation_template.signature.download).to eq(signature2.read) } + it { expect(procedure.attestation_template).to have_attributes(attestation_params) } + it { expect(procedure.attestation_template.logo.download).to eq(logo2.read) } + it { expect(procedure.attestation_template.signature.download).to eq(signature2.read) } it { expect(response).to redirect_to edit_admin_procedure_attestation_template_path(procedure) } it { expect(flash.notice).to eq("L'attestation a bien été modifiée") } - after { procedure.draft_attestation_template&.destroy } + after { procedure.attestation_template.destroy } end context 'when something wrong happens in the attestation template creation' do diff --git a/spec/factories/attestation_template.rb b/spec/factories/attestation_template.rb index 6e6110363..eb3e98727 100644 --- a/spec/factories/attestation_template.rb +++ b/spec/factories/attestation_template.rb @@ -4,6 +4,8 @@ FactoryBot.define do body { 'body' } footer { 'footer' } activated { true } + + association :procedure end trait :with_files do diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index e23650d82..75919801c 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -23,11 +23,10 @@ FactoryBot.define do types_de_champ { [] } types_de_champ_private { [] } updated_at { nil } - attestation_template { } end after(:build) do |procedure, evaluator| - initial_revision = build(:procedure_revision, procedure: procedure, attestation_template: evaluator.attestation_template) + initial_revision = build(:procedure_revision, procedure: procedure) add_types_de_champs(evaluator.types_de_champ, to: initial_revision, scope: :public) add_types_de_champs(evaluator.types_de_champ_private, to: initial_revision, scope: :private) diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index 6789080ed..d940f07a3 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -66,12 +66,16 @@ describe Procedure do end describe '#closed_mail_template_attestation_inconsistency_state' do - let(:procedure_without_attestation) { create(:procedure, closed_mail: closed_mail, attestation_template: nil) } + let(:procedure_without_attestation) { create(:procedure, closed_mail: closed_mail) } let(:procedure_with_active_attestation) do - create(:procedure, closed_mail: closed_mail, attestation_template: build(:attestation_template, activated: true)) + procedure = create(:procedure, closed_mail: closed_mail) + create(:attestation_template, procedure: procedure, activated: true) + procedure end let(:procedure_with_inactive_attestation) do - create(:procedure, closed_mail: closed_mail, attestation_template: build(:attestation_template, activated: false)) + procedure = create(:procedure, closed_mail: closed_mail) + create(:attestation_template, procedure: procedure, activated: false) + procedure end subject { procedure.closed_mail_template_attestation_inconsistency_state } diff --git a/spec/views/administrateurs/_closed_mail_template_attestation_inconsistency_alert.html.haml_spec.rb b/spec/views/administrateurs/_closed_mail_template_attestation_inconsistency_alert.html.haml_spec.rb index d4c49f6c2..642952738 100644 --- a/spec/views/administrateurs/_closed_mail_template_attestation_inconsistency_alert.html.haml_spec.rb +++ b/spec/views/administrateurs/_closed_mail_template_attestation_inconsistency_alert.html.haml_spec.rb @@ -1,6 +1,5 @@ describe 'admin/_closed_mail_template_attestation_inconsistency_alert.html.haml', type: :view do - let(:procedure) { create(:procedure, closed_mail: closed_mail, attestation_template: attestation_template) } - let(:attestation_template) { nil } + let(:procedure) { create(:procedure, closed_mail: closed_mail) } def alert assign(:procedure, procedure) @@ -24,7 +23,7 @@ describe 'admin/_closed_mail_template_attestation_inconsistency_alert.html.haml' context 'when there is an active attestation but the closed mail template does not mention it' do let(:closed_mail) { create(:closed_mail) } - let(:attestation_template) { build(:attestation_template) } + let!(:attestation_template) { create(:attestation_template, procedure: procedure, activated: true) } it { expect(alert).to include("Cette démarche comporte une attestation, mais l’accusé d’acceptation ne la mentionne pas") } it { expect(alert).to include(edit_admin_procedure_mail_template_path(procedure, Mails::ClosedMail::SLUG)) } From 80125cbed1f63252e11bd4fa53fa954a0ba54d03 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 Jan 2022 18:43:26 +0100 Subject: [PATCH 2/2] fix attestation templates --- lib/tasks/fix_attestations.rake | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 lib/tasks/fix_attestations.rake diff --git a/lib/tasks/fix_attestations.rake b/lib/tasks/fix_attestations.rake new file mode 100644 index 000000000..09bce879c --- /dev/null +++ b/lib/tasks/fix_attestations.rake @@ -0,0 +1,19 @@ +namespace :hotfix do + desc 'Fix attestation templates' + task attestation_templates: :environment do + attestation_templates = Rails.root.join('lib', 'tasks', 'attestation_templates.json') + file = File.read attestation_templates + json = JSON.parse file + progress = ProgressReport.new(json.size) + + json.each do |row| + attestation_template = AttestationTemplate.find_by(id: row['id']) + procedure = Procedure.find_by(id: row['procedure_id']) + if attestation_template.present? && procedure.present? + attestation_template.update_column(:procedure_id, procedure.id) + end + progress.inc + end + progress.finish + end +end