Merge pull request #6834 from tchak/revert-attestations

Revert attestation templates revisions

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('<br>')
@@ -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?

app/models/attestation_template.rb

@@ -15,8 +15,7 @@ class AttestationTemplate < ApplicationRecord
   include ActionView::Helpers::NumberHelper
   include TagsSubstitutionConcern
 
-  belongs_to :procedure, optional: true
+  belongs_to :procedure, optional: false
-  has_many :revisions, class_name: 'ProcedureRevision', inverse_of: :attestation_template, dependent: :nullify
 
   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

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?
+    if procedure.attestation_template&.activated?
-      attestation_template.attestation_for(self)
+      procedure.attestation_template.attestation_for(self)
     end
   end
 

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

app/models/procedure_revision.rb

@@ -2,17 +2,15 @@
 #
 # Table name: procedure_revisions
 #
-#  id                      :bigint           not null, primary key
+#  id           :bigint           not null, primary key
-#  published_at            :datetime
+#  published_at :datetime
-#  created_at              :datetime         not null
+#  created_at   :datetime         not null
-#  updated_at              :datetime         not null
+#  updated_at   :datetime         not null
-#  attestation_template_id :bigint
+#  procedure_id :bigint           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)

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

db/migrate/20211214145059_add_attestation_template_id_to_procedure_revisions.rb

@@ -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

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"

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

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.attestation_template).to have_attributes(attestation_params) }
-      it { expect(procedure.draft_attestation_template.activated).to be true }
+      it { expect(procedure.attestation_template.activated).to be true }
-      it { expect(procedure.draft_attestation_template.logo.download).to eq(logo2.read) }
+      it { expect(procedure.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.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.attestation_template).to have_attributes(attestation_params) }
-      it { expect(procedure.draft_attestation_template.logo.download).to eq(logo2.read) }
+      it { expect(procedure.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.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

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

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)
 

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 }

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(:procedure) { create(:procedure, closed_mail: closed_mail) }
-  let(:attestation_template) { nil }
 
   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)) }