From 3c2515ce6db01d74f4853fe2a5c21352cb0491b4 Mon Sep 17 00:00:00 2001
From: Paul Chavard <github@paul.chavard.net>
Date: Thu, 28 Oct 2021 22:05:11 +0200
Subject: [PATCH 01/14] feat(graphql): add graphql_operation to rails logs

---
 app/controllers/api/v2/graphql_controller.rb | 41 ++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/app/controllers/api/v2/graphql_controller.rb b/app/controllers/api/v2/graphql_controller.rb
index 2bfb6d3c4..1bebb48e7 100644
--- a/app/controllers/api/v2/graphql_controller.rb
+++ b/app/controllers/api/v2/graphql_controller.rb
@@ -18,6 +18,47 @@ class API::V2::GraphqlController < API::V2::BaseController
 
   private
 
+  def append_info_to_payload(payload)
+    super
+
+    payload.merge!({
+      graphql_operation: operation_log(params[:query], params[:operationName], params[:variables])
+    })
+  end
+
+  def operation_log(query, operation_name, variables)
+    return "NoQuery" if query.nil?
+
+    operation = GraphQL.parse(query).children.find do |node|
+      if node.is_a?(GraphQL::Language::Nodes::OperationDefinition)
+        node.name == operation_name
+      end
+    end
+
+    return "InvalidQuery" if operation.nil?
+    return "IntrospectionQuery" if operation.name == "IntrospectionQuery"
+
+    message = operation.operation_type
+    if operation.name
+      message += ": #{operation.name} { "
+    end
+    message += operation.selections.map(&:name).join(', ')
+    message += " }"
+    if variables.present?
+      message += " "
+      message += variables.to_unsafe_h.flat_map do |(name, value)|
+        if name == "input"
+          value.map do |(name, value)|
+            "#{name}: \"#{value.to_s.truncate(10)}\""
+          end
+        else
+          "#{name}: \"#{value.to_s.truncate(10)}\""
+        end
+      end.join(', ')
+    end
+    message
+  end
+
   def process_action(*args)
     super
   rescue ActionDispatch::Http::Parameters::ParseError => exception

From 7ef73f13e4cb3d27ae0bfef5330ee36a327ab0c5 Mon Sep 17 00:00:00 2001
From: Paul Chavard <github@paul.chavard.net>
Date: Thu, 28 Oct 2021 16:23:05 +0200
Subject: [PATCH 02/14] fix(grope_instructeur): can not destroy
 groupe_instructeur with discarded dossiers

---
 .../new_administrateur/groupe_instructeurs_controller.rb        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/controllers/new_administrateur/groupe_instructeurs_controller.rb b/app/controllers/new_administrateur/groupe_instructeurs_controller.rb
index fd787c840..abb161874 100644
--- a/app/controllers/new_administrateur/groupe_instructeurs_controller.rb
+++ b/app/controllers/new_administrateur/groupe_instructeurs_controller.rb
@@ -63,7 +63,7 @@ module NewAdministrateur
     end
 
     def destroy
-      if !groupe_instructeur.dossiers.empty?
+      if !groupe_instructeur.dossiers.with_discarded.empty?
         flash[:alert] = "Impossible de supprimer un groupe avec des dossiers. Il faut le réaffecter avant"
       elsif procedure.groupe_instructeurs.one?
         flash[:alert] = "Suppression impossible : il doit y avoir au moins un groupe instructeur sur chaque procédure"

From 0e2f09dd6fdbc0dc3aaddf44a1e31cba938a2f03 Mon Sep 17 00:00:00 2001
From: Paul Chavard <github@paul.chavard.net>
Date: Wed, 27 Oct 2021 10:02:56 +0200
Subject: [PATCH 03/14] fix(dossiers): wrap dossier discard in a transaction

By doing this we ensure that deleted_dossier are not created when dossier is not discarded
---
 app/models/deleted_dossier.rb                 |   9 +-
 app/models/dossier.rb                         |  74 ++++++----
 app/models/procedure.rb                       |   2 +-
 .../expired_dossiers_deletion_service.rb      | 133 ++++++++----------
 4 files changed, 111 insertions(+), 107 deletions(-)

diff --git a/app/models/deleted_dossier.rb b/app/models/deleted_dossier.rb
index 687ec9458..7ba5c6603 100644
--- a/app/models/deleted_dossier.rb
+++ b/app/models/deleted_dossier.rb
@@ -17,8 +17,6 @@
 class DeletedDossier < ApplicationRecord
   belongs_to :procedure, -> { with_discarded }, inverse_of: :deleted_dossiers, optional: false
 
-  validates :dossier_id, uniqueness: true
-
   scope :order_by_updated_at, -> (order = :desc) { order(created_at: order) }
   scope :deleted_since,       -> (since) { where('deleted_dossiers.deleted_at >= ?', since) }
 
@@ -32,16 +30,17 @@ class DeletedDossier < ApplicationRecord
   }
 
   def self.create_from_dossier(dossier, reason)
-    create!(
+    # We have some bad data because of partially deleted dossiers in the past.
+    # For now use find_or_create_by! to avoid errors.
+    create_with(
       reason: reasons.fetch(reason),
-      dossier_id: dossier.id,
       groupe_instructeur_id: dossier.groupe_instructeur_id,
       revision_id: dossier.revision_id,
       user_id: dossier.user_id,
       procedure: dossier.procedure,
       state: dossier.state,
       deleted_at: Time.zone.now
-    )
+    ).create_or_find_by!(dossier_id: dossier.id)
   end
 
   def procedure_removed?
diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index 8f3f2bb13..c583c535d 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -606,7 +606,7 @@ class Dossier < ApplicationRecord
   end
 
   def keep_track_on_deletion?
-    !procedure.brouillon?
+    !procedure.brouillon? && !brouillon?
   end
 
   def expose_legacy_carto_api?
@@ -645,56 +645,68 @@ class Dossier < ApplicationRecord
     end
   end
 
-  def expired_keep_track!
-    if keep_track_on_deletion?
-      DeletedDossier.create_from_dossier(self, :expired)
-      log_automatic_dossier_operation(:supprimer, self)
+  def expired_keep_track_and_destroy!
+    transaction do
+      if keep_track_on_deletion?
+        DeletedDossier.create_from_dossier(self, :expired)
+        log_automatic_dossier_operation(:supprimer, self)
+      end
+      destroy!
     end
+    true
+  rescue
+    false
   end
 
   def discard_and_keep_track!(author, reason)
-    if keep_track_on_deletion?
-      if en_construction?
-        deleted_dossier = DeletedDossier.create_from_dossier(self, reason)
+    user_email = user_deleted? ? nil : user_email_for(:notification)
+    deleted_dossier = nil
 
+    transaction do
+      if keep_track_on_deletion?
+        log_dossier_operation(author, :supprimer, self)
+        deleted_dossier = DeletedDossier.create_from_dossier(self, reason)
+      end
+
+      update!(dossier_transfer_id: nil)
+      discard!
+    end
+
+    if deleted_dossier.present?
+      if en_construction?
         administration_emails = followers_instructeurs.present? ? followers_instructeurs.map(&:email) : procedure.administrateurs.map(&:email)
         administration_emails.each do |email|
           DossierMailer.notify_deletion_to_administration(deleted_dossier, email).deliver_later
         end
+      end
 
-        if !user_deleted?
-          DossierMailer.notify_deletion_to_user(deleted_dossier, user_email_for(:notification)).deliver_later
+      if user_email.present?
+        if reason == :user_request
+          DossierMailer.notify_deletion_to_user(deleted_dossier, user_email).deliver_later
+        else
+          DossierMailer.notify_instructeur_deletion_to_user(deleted_dossier, user_email).deliver_later
         end
-
-        log_dossier_operation(author, :supprimer, self)
-      elsif termine?
-        deleted_dossier = DeletedDossier.create_from_dossier(self, reason)
-
-        if !user_deleted?
-          DossierMailer.notify_instructeur_deletion_to_user(deleted_dossier, user_email_for(:notification)).deliver_later
-        end
-
-        log_dossier_operation(author, :supprimer, self)
       end
     end
-
-    update!(dossier_transfer_id: nil)
-    discard!
   end
 
-  def restore(author, only_discarded_with_procedure = false)
+  def restore(author)
     if discarded?
-      deleted_dossier = DeletedDossier.find_by(dossier_id: id)
-
-      if !only_discarded_with_procedure || deleted_dossier&.procedure_removed?
-        if undiscard && keep_track_on_deletion? && en_construction?
-          deleted_dossier&.destroy
+      transaction do
+        if undiscard && keep_track_on_deletion?
+          deleted_dossier&.destroy!
           log_dossier_operation(author, :restaurer, self)
         end
       end
     end
   end
 
+  def restore_if_discarded_with_procedure(author)
+    if deleted_dossier&.procedure_removed?
+      restore(author)
+    end
+  end
+
   def after_passer_en_construction
     update!(conservation_extension: 0.days)
     update!(en_construction_at: Time.zone.now) if self.en_construction_at.nil?
@@ -977,6 +989,10 @@ class Dossier < ApplicationRecord
 
   private
 
+  def deleted_dossier
+    @deleted_dossier ||= DeletedDossier.find_by(dossier_id: id)
+  end
+
   def defaut_groupe_instructeur?
     groupe_instructeur == procedure.defaut_groupe_instructeur
   end
diff --git a/app/models/procedure.rb b/app/models/procedure.rb
index ebd09ad1b..bdec28614 100644
--- a/app/models/procedure.rb
+++ b/app/models/procedure.rb
@@ -672,7 +672,7 @@ class Procedure < ApplicationRecord
   def restore(author)
     if discarded? && undiscard
       dossiers.with_discarded.discarded.find_each do |dossier|
-        dossier.restore(author, true)
+        dossier.restore_if_discarded_with_procedure(author)
       end
     end
   end
diff --git a/app/services/expired_dossiers_deletion_service.rb b/app/services/expired_dossiers_deletion_service.rb
index fbf2c3752..a7916d23c 100644
--- a/app/services/expired_dossiers_deletion_service.rb
+++ b/app/services/expired_dossiers_deletion_service.rb
@@ -19,22 +19,16 @@ class ExpiredDossiersDeletionService
       .brouillon_close_to_expiration
       .without_brouillon_expiration_notice_sent
 
-    dossiers_close_to_expiration
-      .with_notifiable_procedure
-      .includes(:user, :procedure)
-      .group_by(&:user)
-      .each do |(user, dossiers)|
-        DossierMailer.notify_brouillon_near_deletion(
-          dossiers,
-          user.email
-        ).deliver_later
+    user_notifications = group_by_user_email(dossiers_close_to_expiration)
 
-        # mark as sent dossiers from current notification
-        Dossier.where(id: dossiers).update_all(brouillon_close_to_expiration_notice_sent_at: Time.zone.now)
-      end
-
-    # mark as sent dossiers without notification
     dossiers_close_to_expiration.update_all(brouillon_close_to_expiration_notice_sent_at: Time.zone.now)
+
+    user_notifications.each do |(email, dossiers)|
+      DossierMailer.notify_brouillon_near_deletion(
+        dossiers,
+        email
+      ).deliver_later
+    end
   end
 
   def self.send_en_construction_expiration_notices
@@ -52,24 +46,17 @@ class ExpiredDossiersDeletionService
   end
 
   def self.delete_expired_brouillons_and_notify
-    dossiers_to_remove = Dossier.brouillon_expired
+    user_notifications = group_by_user_email(Dossier.brouillon_expired)
+      .map { |(email, dossiers)| [email, dossiers.map(&:hash_for_deletion_mail)] }
 
-    dossiers_to_remove
-      .with_notifiable_procedure
-      .includes(:user, :procedure)
-      .group_by(&:user)
-      .each do |(user, dossiers)|
-        DossierMailer.notify_brouillon_deletion(
-          dossiers.map(&:hash_for_deletion_mail),
-          user.email
-        ).deliver_later
+    Dossier.brouillon_expired.destroy_all
 
-        # destroy dossiers from current notification
-        Dossier.where(id: dossiers).destroy_all
-      end
-
-    # destroy dossiers without notification
-    dossiers_to_remove.destroy_all
+    user_notifications.each do |(email, dossiers_hash)|
+      DossierMailer.notify_brouillon_deletion(
+        dossiers_hash,
+        email
+      ).deliver_later
+    end
   end
 
   def self.delete_expired_en_construction_and_notify
@@ -83,57 +70,58 @@ class ExpiredDossiersDeletionService
   private
 
   def self.send_expiration_notices(dossiers_close_to_expiration, close_to_expiration_flag)
-    dossiers_close_to_expiration
-      .with_notifiable_procedure
-      .includes(:user)
-      .group_by(&:user)
-      .each do |(user, dossiers)|
-        DossierMailer.notify_near_deletion_to_user(
-          dossiers,
-          user.email
-        ).deliver_later
-      end
+    user_notifications = group_by_user_email(dossiers_close_to_expiration)
+    administration_notifications = group_by_fonctionnaire_email(dossiers_close_to_expiration)
 
-    group_by_fonctionnaire_email(dossiers_close_to_expiration).each do |(email, dossiers)|
-      DossierMailer.notify_near_deletion_to_administration(
-        dossiers.to_a,
-        email
-      ).deliver_later
-
-      # mark as sent dossiers from current notification
-      Dossier.where(id: dossiers.to_a).update_all(close_to_expiration_flag => Time.zone.now)
-    end
-
-    # mark as sent dossiers without notification
     dossiers_close_to_expiration.update_all(close_to_expiration_flag => Time.zone.now)
+
+    user_notifications.each do |(email, dossiers)|
+      DossierMailer.notify_near_deletion_to_user(dossiers, email).deliver_later
+    end
+    administration_notifications.each do |(email, dossiers)|
+      DossierMailer.notify_near_deletion_to_administration(dossiers, email).deliver_later
+    end
   end
 
   def self.delete_expired_and_notify(dossiers_to_remove, notify_on_closed_procedures_to_user: false)
-    dossiers_to_remove.each(&:expired_keep_track!)
+    user_notifications = group_by_user_email(dossiers_to_remove, notify_on_closed_procedures_to_user: notify_on_closed_procedures_to_user)
+      .map { |(email, dossiers)| [email, dossiers.map(&:id)] }
+    administration_notifications = group_by_fonctionnaire_email(dossiers_to_remove)
+      .map { |(email, dossiers)| [email, dossiers.map(&:id)] }
 
-    dossiers_to_remove
-      .with_notifiable_procedure(notify_on_closed: notify_on_closed_procedures_to_user)
-      .includes(:user)
-      .group_by(&:user)
-      .each do |(user, dossiers)|
-        DossierMailer.notify_automatic_deletion_to_user(
-          DeletedDossier.where(dossier_id: dossiers.map(&:id)).to_a,
-          user.email
-        ).deliver_later
+    deleted_dossier_ids = []
+    dossiers_to_remove.find_each do |dossier|
+      if dossier.expired_keep_track_and_destroy!
+        deleted_dossier_ids << dossier.id
       end
-
-    self.group_by_fonctionnaire_email(dossiers_to_remove).each do |(email, dossiers)|
-      DossierMailer.notify_automatic_deletion_to_administration(
-        DeletedDossier.where(dossier_id: dossiers.map(&:id)).to_a,
-        email
-      ).deliver_later
-
-      # destroy dossiers from current notification
-      Dossier.where(id: dossiers.to_a).destroy_all
     end
 
-    # destroy dossiers without notification
-    dossiers_to_remove.destroy_all
+    user_notifications.each do |(email, dossier_ids)|
+      dossier_ids = dossier_ids.intersection(deleted_dossier_ids)
+      if dossier_ids.present?
+        DossierMailer.notify_automatic_deletion_to_user(
+          DeletedDossier.where(dossier_id: dossier_ids).to_a,
+          email
+        ).deliver_later
+      end
+    end
+    administration_notifications.each do |(email, dossier_ids)|
+      dossier_ids = dossier_ids.intersection(deleted_dossier_ids)
+      if dossier_ids.present?
+        DossierMailer.notify_automatic_deletion_to_administration(
+          DeletedDossier.where(dossier_id: dossier_ids).to_a,
+          email
+        ).deliver_later
+      end
+    end
+  end
+
+  def self.group_by_user_email(dossiers, notify_on_closed_procedures_to_user: false)
+    dossiers
+      .with_notifiable_procedure(notify_on_closed: notify_on_closed_procedures_to_user)
+      .includes(:user, :procedure)
+      .group_by(&:user)
+      .map { |(user, dossiers)| [user.email, dossiers] }
   end
 
   def self.group_by_fonctionnaire_email(dossiers)
@@ -143,5 +131,6 @@ class ExpiredDossiersDeletionService
       .each_with_object(Hash.new { |h, k| h[k] = Set.new }) do |dossier, h|
         (dossier.followers_instructeurs + dossier.procedure.administrateurs).each { |destinataire| h[destinataire.email] << dossier }
       end
+      .map { |(email, dossiers)| [email, dossiers.to_a] }
   end
 end

From f6b8689a97688bd0581547e08134e7618706c00c Mon Sep 17 00:00:00 2001
From: Paul Chavard <github@paul.chavard.net>
Date: Fri, 22 Oct 2021 12:18:43 +0200
Subject: [PATCH 04/14] fix(revisions): fix repetitions export with revisions

---
 app/models/champs/repetition_champ.rb         | 20 ++---------
 app/models/dossier.rb                         |  6 ++--
 app/models/procedure.rb                       | 14 ++++----
 app/models/type_de_champ.rb                   | 20 +++++++++++
 .../repetition_type_de_champ.rb               |  7 ++++
 .../types_de_champ/type_de_champ_base.rb      |  6 ++--
 app/services/procedure_export_service.rb      | 35 ++++++++++---------
 spec/models/dossier_spec.rb                   | 21 +++++++----
 ...ocedure_presentation_and_revisions_spec.rb |  2 +-
 spec/models/procedure_revision_spec.rb        |  4 +--
 10 files changed, 78 insertions(+), 57 deletions(-)

diff --git a/app/models/champs/repetition_champ.rb b/app/models/champs/repetition_champ.rb
index 3390bb1ec..25ed1700d 100644
--- a/app/models/champs/repetition_champ.rb
+++ b/app/models/champs/repetition_champ.rb
@@ -21,6 +21,7 @@
 #
 class Champs::RepetitionChamp < Champ
   accepts_nested_attributes_for :champs, allow_destroy: true
+  delegate :libelle_for_export, to: :type_de_champ
 
   def rows
     champs.group_by(&:row).values
@@ -57,13 +58,6 @@ class Champs::RepetitionChamp < Champ
     end
   end
 
-  # We have to truncate the label here as spreadsheets have a (30 char) limit on length.
-  def libelle_for_export
-    str = "(#{stable_id}) #{libelle}"
-    # /\*?[] are invalid Excel worksheet characters
-    ActiveStorage::Filename.new(str.delete('[]*?')).sanitized
-  end
-
   class Row < Hashie::Dash
     property :index
     property :dossier_id
@@ -73,19 +67,11 @@ class Champs::RepetitionChamp < Champ
       self[attribute]
     end
 
-    def spreadsheet_columns
+    def spreadsheet_columns(types_de_champ)
       [
         ['Dossier ID', :dossier_id],
         ['Ligne', :index]
-      ] + exported_champs
-    end
-
-    private
-
-    def exported_champs
-      champs.reject(&:exclude_from_export?).map do |champ|
-        [champ.libelle, champ.for_export]
-      end
+      ] + Dossier.champs_for_export(champs, types_de_champ)
     end
   end
 end
diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index c583c535d..d458dafca 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -918,12 +918,12 @@ class Dossier < ApplicationRecord
       columns << ['Groupe instructeur', groupe_instructeur.label]
     end
 
-    columns + champs_for_export(types_de_champ)
+    columns + self.class.champs_for_export(champs + champs_private, types_de_champ)
   end
 
-  def champs_for_export(types_de_champ)
+  def self.champs_for_export(champs, types_de_champ)
     # Index values by stable_id
-    values = (champs + champs_private).reject(&:exclude_from_export?)
+    values = champs.reject(&:exclude_from_export?)
       .index_by(&:stable_id)
       .transform_values(&:for_export)
 
diff --git a/app/models/procedure.rb b/app/models/procedure.rb
index bdec28614..0bfdc51e5 100644
--- a/app/models/procedure.rb
+++ b/app/models/procedure.rb
@@ -99,20 +99,16 @@ class Procedure < ApplicationRecord
   end
 
   def types_de_champ_for_procedure_presentation
-    explanatory_types_de_champ = [:header_section, :explication, :repetition].map { |k| TypeDeChamp.type_champs.fetch(k) }
-
     if brouillon?
-      TypeDeChamp
-        .joins(:revisions)
-        .where.not(type_champ: explanatory_types_de_champ)
-        .where(procedure_revisions: { id: draft_revision_id })
+      TypeDeChamp.fillable
+        .joins(:revision_types_de_champ)
+        .where(revision_types_de_champ: { revision: draft_revision })
         .order(:private, :position)
     else
       # fetch all type_de_champ.stable_id for all the revisions expect draft
       # and for each stable_id take the bigger (more recent) type_de_champ.id
-      recent_ids = TypeDeChamp
+      recent_ids = TypeDeChamp.fillable
         .joins(:revisions)
-        .where.not(type_champ: explanatory_types_de_champ)
         .where(procedure_revisions: { procedure_id: id })
         .where.not(procedure_revisions: { id: draft_revision_id })
         .group(:stable_id)
@@ -139,6 +135,7 @@ class Procedure < ApplicationRecord
     else
       TypeDeChamp.root
         .public_only
+        .fillable
         .where(revision: revisions - [draft_revision])
         .order(:created_at)
         .uniq
@@ -151,6 +148,7 @@ class Procedure < ApplicationRecord
     else
       TypeDeChamp.root
         .private_only
+        .fillable
         .where(revision: revisions - [draft_revision])
         .order(:created_at)
         .uniq
diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb
index a415595ec..81767fd72 100644
--- a/app/models/type_de_champ.rb
+++ b/app/models/type_de_champ.rb
@@ -86,6 +86,8 @@ class TypeDeChamp < ApplicationRecord
   scope :ordered, -> { order(order_place: :asc) }
   scope :root, -> { where(parent_id: nil) }
   scope :repetition, -> { where(type_champ: type_champs.fetch(:repetition)) }
+  scope :not_repetition, -> { where.not(type_champ: type_champs.fetch(:repetition)) }
+  scope :fillable, -> { where.not(type_champ: [type_champs.fetch(:header_section), type_champs.fetch(:explication)]) }
 
   has_many :champ, inverse_of: :type_de_champ, dependent: :destroy do
     def build(params = {})
@@ -287,6 +289,24 @@ class TypeDeChamp < ApplicationRecord
     options.slice(*TypesDeChamp::CarteTypeDeChamp::LAYERS)
   end
 
+  def types_de_champ_for_revision(revision)
+    if revision.draft?
+      # if we are asking for children on a draft revision, just use current child types_de_champ
+      types_de_champ.fillable
+    else
+      # otherwise return all types_de_champ in their latest state
+      types_de_champ = TypeDeChamp
+        .fillable
+        .joins(:parent)
+        .where(parent: { stable_id: stable_id })
+
+      TypeDeChamp
+        .where(id: types_de_champ.group(:stable_id).select('MAX(types_de_champ.id)'))
+        .joins(parent: :revision_types_de_champ)
+        .order(:order_place, 'procedure_revision_types_de_champ.revision_id': :desc)
+    end
+  end
+
   FEATURE_FLAGS = {}
 
   def self.type_de_champ_types_for(procedure, user)
diff --git a/app/models/types_de_champ/repetition_type_de_champ.rb b/app/models/types_de_champ/repetition_type_de_champ.rb
index c3328850a..4a17ae4bf 100644
--- a/app/models/types_de_champ/repetition_type_de_champ.rb
+++ b/app/models/types_de_champ/repetition_type_de_champ.rb
@@ -4,4 +4,11 @@ class TypesDeChamp::RepetitionTypeDeChamp < TypesDeChamp::TypeDeChampBase
     champ.add_row
     champ
   end
+
+  # We have to truncate the label here as spreadsheets have a (30 char) limit on length.
+  def libelle_for_export(index = 0)
+    str = "(#{stable_id}) #{libelle}"
+    # /\*?[] are invalid Excel worksheet characters
+    ActiveStorage::Filename.new(str.delete('[]*?')).sanitized
+  end
 end
diff --git a/app/models/types_de_champ/type_de_champ_base.rb b/app/models/types_de_champ/type_de_champ_base.rb
index 57f9e0370..56404ae2a 100644
--- a/app/models/types_de_champ/type_de_champ_base.rb
+++ b/app/models/types_de_champ/type_de_champ_base.rb
@@ -1,14 +1,14 @@
 class TypesDeChamp::TypeDeChampBase
   include ActiveModel::Validations
 
-  delegate :description, :libelle, to: :@type_de_champ
+  delegate :description, :libelle, :stable_id, to: :@type_de_champ
 
   def initialize(type_de_champ)
     @type_de_champ = type_de_champ
   end
 
   def tags_for_template
-    stable_id = @type_de_champ.stable_id
+    stable_id = self.stable_id
     [
       {
         libelle: libelle,
@@ -21,7 +21,7 @@ class TypesDeChamp::TypeDeChampBase
     ]
   end
 
-  def libelle_for_export(index)
+  def libelle_for_export(index = 0)
     libelle
   end
 
diff --git a/app/services/procedure_export_service.rb b/app/services/procedure_export_service.rb
index 2de0cf91c..7381b32ea 100644
--- a/app/services/procedure_export_service.rb
+++ b/app/services/procedure_export_service.rb
@@ -39,21 +39,22 @@ class ProcedureExportService
     @avis ||= dossiers.flat_map(&:avis)
   end
 
-  def champs_repetables
-    @champs_repetables ||= dossiers.flat_map do |dossier|
-      [dossier.champs, dossier.champs_private]
-        .flatten
-        .filter { |champ| champ.is_a?(Champs::RepetitionChamp) }
-    end.group_by(&:libelle_for_export)
-  end
-
   def champs_repetables_options
-    champs_repetables.map do |libelle, champs|
-      [
-        libelle,
-        champs.flat_map(&:rows_for_export)
-      ]
-    end
+    revision = @procedure.active_revision
+    champs_by_stable_id = dossiers
+      .flat_map { |dossier| (dossier.champs + dossier.champs_private).filter(&:repetition?) }
+      .group_by(&:stable_id)
+
+    @procedure.types_de_champ_for_procedure_presentation.repetition
+      .map { |type_de_champ_repetition| [type_de_champ_repetition, type_de_champ_repetition.types_de_champ_for_revision(revision).to_a] }
+      .filter { |(_, types_de_champ)| types_de_champ.present? }
+      .map do |(type_de_champ_repetition, types_de_champ)|
+        {
+          sheet_name: type_de_champ_repetition.libelle_for_export,
+          instances: champs_by_stable_id.fetch(type_de_champ_repetition.stable_id, []).flat_map(&:rows_for_export),
+          spreadsheet_columns: Proc.new { |instance| instance.spreadsheet_columns(types_de_champ) }
+        }
+      end
   end
 
   DEFAULT_STYLES = {
@@ -69,8 +70,8 @@ class ProcedureExportService
       { instances: etablissements.to_a, sheet_name: 'Etablissements' }
     when :avis
       { instances: avis.to_a, sheet_name: 'Avis' }
-    when Array
-      { instances: table.last, sheet_name: table.first }
+    when Hash
+      table
     end.merge(DEFAULT_STYLES)
 
     # transliterate: convert to ASCII characters
@@ -84,7 +85,7 @@ class ProcedureExportService
   end
 
   def spreadsheet_columns(format)
-    types_de_champ = @procedure.types_de_champ_for_procedure_presentation.to_a
+    types_de_champ = @procedure.types_de_champ_for_procedure_presentation.not_repetition.to_a
 
     Proc.new do |instance|
       instance.send(:"spreadsheet_columns_#{format}", types_de_champ: types_de_champ)
diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb
index 426ab34b9..b3d1d0980 100644
--- a/spec/models/dossier_spec.rb
+++ b/spec/models/dossier_spec.rb
@@ -1341,14 +1341,20 @@ describe Dossier do
   end
 
   describe "champs_for_export" do
-    let(:procedure) { create(:procedure, :with_type_de_champ, :with_datetime, :with_yes_no, :with_explication, :with_commune) }
+    let(:procedure) { create(:procedure, :with_type_de_champ, :with_datetime, :with_yes_no, :with_explication, :with_commune, :with_repetition) }
     let(:text_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:text) } }
     let(:yes_no_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:yes_no) } }
     let(:datetime_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:datetime) } }
     let(:explication_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:explication) } }
     let(:commune_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:communes) } }
+    let(:repetition_type_de_champ) { procedure.types_de_champ.find { |type_de_champ| type_de_champ.type_champ == TypeDeChamp.type_champs.fetch(:repetition) } }
+    let(:repetition_champ) { dossier.champs.find(&:repetition?) }
+    let(:repetition_second_revision_champ) { dossier_second_revision.champs.find(&:repetition?) }
     let(:dossier) { create(:dossier, procedure: procedure) }
     let(:dossier_second_revision) { create(:dossier, procedure: procedure) }
+    let(:dossier_champs_for_export) { Dossier.champs_for_export(dossier.champs, procedure.types_de_champ_for_procedure_presentation.not_repetition) }
+    let(:dossier_second_revision_champs_for_export) { Dossier.champs_for_export(dossier_second_revision.champs, procedure.types_de_champ_for_procedure_presentation.not_repetition) }
+    let(:repetition_second_revision_champs_for_export) { Dossier.champs_for_export(repetition_second_revision_champ.champs, procedure.types_de_champ_for_procedure_presentation.repetition) }
 
     context "when procedure published" do
       before do
@@ -1358,16 +1364,19 @@ describe Dossier do
         procedure.draft_revision.add_type_de_champ(type_champ: TypeDeChamp.type_champs.fetch(:text), libelle: 'New text field')
         procedure.draft_revision.find_or_clone_type_de_champ(yes_no_type_de_champ.stable_id).update(libelle: 'Updated yes/no')
         procedure.draft_revision.find_or_clone_type_de_champ(commune_type_de_champ.stable_id).update(libelle: 'Commune de naissance')
+        procedure.draft_revision.find_or_clone_type_de_champ(repetition_type_de_champ.stable_id).update(libelle: 'Repetition')
         procedure.update(published_revision: procedure.draft_revision, draft_revision: procedure.create_new_revision)
         dossier.reload
         procedure.reload
       end
 
       it "should have champs from all revisions" do
-        expect(dossier.types_de_champ.map(&:libelle)).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Yes/no", explication_type_de_champ.libelle, commune_type_de_champ.libelle])
-        expect(dossier_second_revision.types_de_champ.map(&:libelle)).to eq([datetime_type_de_champ.libelle, "Updated yes/no", explication_type_de_champ.libelle, 'Commune de naissance', "New text field"])
-        expect(dossier.champs_for_export(dossier.procedure.types_de_champ_for_procedure_presentation).map { |(libelle)| libelle }).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Updated yes/no", "Commune de naissance", "Commune de naissance (Code insee)", "New text field"])
-        expect(dossier.champs_for_export(dossier.procedure.types_de_champ_for_procedure_presentation)).to eq(dossier_second_revision.champs_for_export(dossier_second_revision.procedure.types_de_champ_for_procedure_presentation))
+        expect(dossier.types_de_champ.map(&:libelle)).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Yes/no", explication_type_de_champ.libelle, commune_type_de_champ.libelle, repetition_type_de_champ.libelle])
+        expect(dossier_second_revision.types_de_champ.map(&:libelle)).to eq([datetime_type_de_champ.libelle, "Updated yes/no", explication_type_de_champ.libelle, 'Commune de naissance', "Repetition", "New text field"])
+        expect(dossier_champs_for_export.map { |(libelle)| libelle }).to eq([text_type_de_champ.libelle, datetime_type_de_champ.libelle, "Updated yes/no", "Commune de naissance", "Commune de naissance (Code insee)", "New text field"])
+        expect(dossier_champs_for_export).to eq(dossier_second_revision_champs_for_export)
+        expect(repetition_second_revision_champs_for_export.map { |(libelle)| libelle }).to eq(procedure.types_de_champ_for_procedure_presentation.repetition.map(&:libelle_for_export))
+        expect(repetition_second_revision_champs_for_export.first.size).to eq(2)
       end
     end
 
@@ -1375,7 +1384,7 @@ describe Dossier do
       let(:procedure) { create(:procedure, :with_type_de_champ, :with_explication) }
 
       it "should not contain non-exportable types de champ" do
-        expect(dossier.champs_for_export(dossier.procedure.types_de_champ_for_procedure_presentation).map { |(libelle)| libelle }).to eq([text_type_de_champ.libelle])
+        expect(dossier_champs_for_export.map { |(libelle)| libelle }).to eq([text_type_de_champ.libelle])
       end
     end
   end
diff --git a/spec/models/procedure_presentation_and_revisions_spec.rb b/spec/models/procedure_presentation_and_revisions_spec.rb
index 087b2e17e..4f9432d31 100644
--- a/spec/models/procedure_presentation_and_revisions_spec.rb
+++ b/spec/models/procedure_presentation_and_revisions_spec.rb
@@ -1,6 +1,6 @@
 describe ProcedurePresentation do
   describe "#types_de_champ_for_procedure_presentation" do
-    subject { procedure.types_de_champ_for_procedure_presentation.pluck(:libelle) }
+    subject { procedure.types_de_champ_for_procedure_presentation.not_repetition.pluck(:libelle) }
 
     context 'for a draft procedure' do
       let(:procedure) { create(:procedure) }
diff --git a/spec/models/procedure_revision_spec.rb b/spec/models/procedure_revision_spec.rb
index efdc05dc1..469da0ef4 100644
--- a/spec/models/procedure_revision_spec.rb
+++ b/spec/models/procedure_revision_spec.rb
@@ -56,7 +56,7 @@ describe ProcedureRevision do
       revision.reload
       expect(revision.types_de_champ.index(type_de_champ)).to eq(2)
       expect(revision.procedure.types_de_champ.index(type_de_champ)).to eq(2)
-      expect(revision.procedure.types_de_champ_for_procedure_presentation.index(type_de_champ)).to eq(2)
+      expect(revision.procedure.types_de_champ_for_procedure_presentation.not_repetition.index(type_de_champ)).to eq(2)
     end
 
     it 'move up' do
@@ -66,7 +66,7 @@ describe ProcedureRevision do
       revision.reload
       expect(revision.types_de_champ.index(last_type_de_champ)).to eq(0)
       expect(revision.procedure.types_de_champ.index(last_type_de_champ)).to eq(0)
-      expect(revision.procedure.types_de_champ_for_procedure_presentation.index(last_type_de_champ)).to eq(0)
+      expect(revision.procedure.types_de_champ_for_procedure_presentation.not_repetition.index(last_type_de_champ)).to eq(0)
     end
 
     context 'repetition' do

From 6c60c940d4855f5d6955f86c11e45ce93b8fc3c9 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 16:17:19 +0100
Subject: [PATCH 05/14] gems: update aasm

---
 Gemfile.lock | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index c06ad62b4..fdf3237a6 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -18,7 +18,7 @@ GIT
 GEM
   remote: https://rubygems.org/
   specs:
-    aasm (5.1.1)
+    aasm (5.2.0)
       concurrent-ruby (~> 1.0)
     acsv (0.0.1)
     actioncable (6.1.4.1)

From b06183d7bf0faac0cb5989f4607b881e730370cf Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 14:52:53 +0000
Subject: [PATCH 06/14] specs: rename Dossier's `with_all_champs` to
 `with_populated_champs`

This better reflects what the trait is used for, and disambiguate it
from the similarly named trait in the Procedure factory.
---
 spec/controllers/api/v1/dossiers_controller_spec.rb  |  2 +-
 spec/controllers/api/v2/graphql_controller_spec.rb   |  4 ++--
 .../instructeurs/dossiers_controller_spec.rb         |  6 +++---
 spec/controllers/recherche_controller_spec.rb        |  4 ++--
 spec/controllers/users/dossiers_controller_spec.rb   |  2 +-
 spec/factories/dossier.rb                            |  4 ++--
 spec/services/procedure_export_service_spec.rb       | 12 ++++++------
 spec/system/users/list_dossiers_spec.rb              |  4 ++--
 8 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/spec/controllers/api/v1/dossiers_controller_spec.rb b/spec/controllers/api/v1/dossiers_controller_spec.rb
index 75426fe7b..152b2f38d 100644
--- a/spec/controllers/api/v1/dossiers_controller_spec.rb
+++ b/spec/controllers/api/v1/dossiers_controller_spec.rb
@@ -256,7 +256,7 @@ describe API::V1::DossiersController do
 
           describe 'repetition' do
             let(:procedure) { create(:procedure, :with_repetition, administrateur: admin) }
-            let(:dossier) { create(:dossier, :en_construction, :with_all_champs, procedure: procedure) }
+            let(:dossier) { create(:dossier, :en_construction, :with_populated_champs, procedure: procedure) }
 
             subject { super().first[:rows] }
 
diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
index dffe42533..82cd93dc7 100644
--- a/spec/controllers/api/v2/graphql_controller_spec.rb
+++ b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -5,8 +5,8 @@ describe API::V2::GraphqlController do
   let(:dossier) do
     dossier = create(:dossier,
       :en_construction,
-      :with_all_champs,
-      :with_all_annotations,
+      :with_populated_champs,
+      :with_populated_annotations,
       :with_individual,
       procedure: procedure)
     create(:commentaire, :with_file, dossier: dossier, email: 'test@test.com')
diff --git a/spec/controllers/instructeurs/dossiers_controller_spec.rb b/spec/controllers/instructeurs/dossiers_controller_spec.rb
index dc69d2bd0..4f0f2f88f 100644
--- a/spec/controllers/instructeurs/dossiers_controller_spec.rb
+++ b/spec/controllers/instructeurs/dossiers_controller_spec.rb
@@ -560,8 +560,8 @@ describe Instructeurs::DossiersController, type: :controller do
       let(:dossier) do
         create(:dossier,
           :accepte,
-          :with_all_champs,
-          :with_all_annotations,
+          :with_populated_champs,
+          :with_populated_annotations,
           :with_motivation,
           :with_entreprise,
           :with_commentaires, procedure: procedure)
@@ -593,7 +593,7 @@ describe Instructeurs::DossiersController, type: :controller do
         build(:type_de_champ_repetition, :with_types_de_champ, position: 3)
       ], instructeurs: instructeurs)
     end
-    let(:dossier) { create(:dossier, :en_construction, :with_all_annotations, procedure: procedure) }
+    let(:dossier) { create(:dossier, :en_construction, :with_populated_annotations, procedure: procedure) }
     let(:another_instructeur) { create(:instructeur) }
     let(:now) { Time.zone.parse('01/01/2100') }
 
diff --git a/spec/controllers/recherche_controller_spec.rb b/spec/controllers/recherche_controller_spec.rb
index 2e643bd84..985e4dd3e 100644
--- a/spec/controllers/recherche_controller_spec.rb
+++ b/spec/controllers/recherche_controller_spec.rb
@@ -1,10 +1,10 @@
 describe RechercheController, type: :controller do
-  let(:dossier) { create(:dossier, :en_construction, :with_all_annotations) }
+  let(:dossier) { create(:dossier, :en_construction, :with_populated_annotations) }
   let(:dossier2) { create(:dossier, :en_construction, procedure: dossier.procedure) }
   let(:instructeur) { create(:instructeur) }
 
   let(:dossier_with_expert) { avis.dossier }
-  let(:avis) { create(:avis, dossier: create(:dossier, :en_construction, :with_all_annotations)) }
+  let(:avis) { create(:avis, dossier: create(:dossier, :en_construction, :with_populated_annotations)) }
 
   let(:user) { instructeur.user }
 
diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb
index ec6e67aa7..7b41bf6be 100644
--- a/spec/controllers/users/dossiers_controller_spec.rb
+++ b/spec/controllers/users/dossiers_controller_spec.rb
@@ -879,7 +879,7 @@ describe Users::DossiersController, type: :controller do
       let(:dossier) do
         create(:dossier,
           :accepte,
-          :with_all_champs,
+          :with_populated_champs,
           :with_motivation,
           :with_commentaires,
           procedure: procedure,
diff --git a/spec/factories/dossier.rb b/spec/factories/dossier.rb
index ec747fcc8..59f4abc37 100644
--- a/spec/factories/dossier.rb
+++ b/spec/factories/dossier.rb
@@ -212,7 +212,7 @@ FactoryBot.define do
       end
     end
 
-    trait :with_all_champs do
+    trait :with_populated_champs do
       after(:create) do |dossier, _evaluator|
         dossier.champs = dossier.types_de_champ.map do |type_de_champ|
           build(:"champ_#{type_de_champ.type_champ}", dossier: dossier, type_de_champ: type_de_champ)
@@ -221,7 +221,7 @@ FactoryBot.define do
       end
     end
 
-    trait :with_all_annotations do
+    trait :with_populated_annotations do
       after(:create) do |dossier, _evaluator|
         dossier.champs_private = dossier.types_de_champ_private.map do |type_de_champ|
           build(:"champ_#{type_de_champ.type_champ}", private: true, dossier: dossier, type_de_champ: type_de_champ)
diff --git a/spec/services/procedure_export_service_spec.rb b/spec/services/procedure_export_service_spec.rb
index 6888e77e1..36c35a3fa 100644
--- a/spec/services/procedure_export_service_spec.rb
+++ b/spec/services/procedure_export_service_spec.rb
@@ -32,7 +32,7 @@ describe ProcedureExportService do
     end
 
     describe 'Dossiers sheet' do
-      let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) }
+      let!(:dossier) { create(:dossier, :en_instruction, :with_populated_champs, :with_individual, procedure: procedure) }
 
       let(:nominal_headers) do
         [
@@ -119,7 +119,7 @@ describe ProcedureExportService do
 
     describe 'Etablissement sheet' do
       let(:procedure) { create(:procedure, :published, :with_all_champs) }
-      let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_entreprise, procedure: procedure) }
+      let!(:dossier) { create(:dossier, :en_instruction, :with_populated_champs, :with_entreprise, procedure: procedure) }
 
       let(:dossier_etablissement) { etablissements_sheet.data[1] }
       let(:champ_etablissement) { etablissements_sheet.data[0] }
@@ -309,7 +309,7 @@ describe ProcedureExportService do
     end
 
     describe 'Avis sheet' do
-      let!(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) }
+      let!(:dossier) { create(:dossier, :en_instruction, :with_populated_champs, :with_individual, procedure: procedure) }
       let!(:avis) { create(:avis, :with_answer, dossier: dossier) }
 
       it 'should have headers' do
@@ -332,8 +332,8 @@ describe ProcedureExportService do
     describe 'Repetitions sheet' do
       let!(:dossiers) do
         [
-          create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure),
-          create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure)
+          create(:dossier, :en_instruction, :with_populated_champs, :with_individual, procedure: procedure),
+          create(:dossier, :en_instruction, :with_populated_champs, :with_individual, procedure: procedure)
         ]
       end
       let(:champ_repetition) { dossiers.first.champs.find { |champ| champ.type_champ == 'repetition' } }
@@ -381,7 +381,7 @@ describe ProcedureExportService do
       end
 
       context 'with non unique labels' do
-        let(:dossier) { create(:dossier, :en_instruction, :with_all_champs, :with_individual, procedure: procedure) }
+        let(:dossier) { create(:dossier, :en_instruction, :with_populated_champs, :with_individual, procedure: procedure) }
         let(:champ_repetition) { dossier.champs.find { |champ| champ.type_champ == 'repetition' } }
         let(:type_de_champ_repetition) { create(:type_de_champ_repetition, procedure: procedure, libelle: champ_repetition.libelle) }
         let!(:another_champ_repetition) { create(:champ_repetition, type_de_champ: type_de_champ_repetition, dossier: dossier) }
diff --git a/spec/system/users/list_dossiers_spec.rb b/spec/system/users/list_dossiers_spec.rb
index 445f5b12a..42733b337 100644
--- a/spec/system/users/list_dossiers_spec.rb
+++ b/spec/system/users/list_dossiers_spec.rb
@@ -1,7 +1,7 @@
 describe 'user access to the list of their dossiers' do
   let(:user) { create(:user) }
   let!(:dossier_brouillon)       { create(:dossier, user: user) }
-  let!(:dossier_en_construction) { create(:dossier, :with_all_champs, :en_construction, user: user) }
+  let!(:dossier_en_construction) { create(:dossier, :with_populated_champs, :en_construction, user: user) }
   let!(:dossier_en_instruction)  { create(:dossier, :en_instruction, user: user) }
   let!(:dossier_archived)        { create(:dossier, :en_instruction, :archived, user: user) }
   let(:dossiers_per_page) { 25 }
@@ -121,7 +121,7 @@ describe 'user access to the list of their dossiers' do
     end
 
     context "when user search for something inside the dossier" do
-      let(:dossier_en_construction2) { create(:dossier, :with_all_champs, :en_construction, user: user) }
+      let(:dossier_en_construction2) { create(:dossier, :with_populated_champs, :en_construction, user: user) }
       before do
         page.find_by_id('q').set(dossier_en_construction.champs.first.value)
       end

From 04159056af93d849ff39f51025c49f36d0fbd899 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 15:04:50 +0000
Subject: [PATCH 07/14] specs: clarify spec by replacing some `context` by
 `describe`

---
 .../api/v2/graphql_controller_spec.rb         | 30 ++++++++++---------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
index 82cd93dc7..1e4b2a3cf 100644
--- a/spec/controllers/api/v2/graphql_controller_spec.rb
+++ b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -116,7 +116,7 @@ describe API::V2::GraphqlController do
       request.env['HTTP_AUTHORIZATION'] = authorization_header
     end
 
-    context "demarche" do
+    describe "demarche" do
       it "should be returned" do
         expect(gql_errors).to eq(nil)
         expect(gql_data).to eq(demarche: {
@@ -166,7 +166,7 @@ describe API::V2::GraphqlController do
         })
       end
 
-      context "filter dossiers" do
+      describe "filter dossiers" do
         let(:query) do
           "{
             demarche(number: #{procedure.id}) {
@@ -193,7 +193,7 @@ describe API::V2::GraphqlController do
         end
       end
 
-      context "filter archived dossiers" do
+      describe "filter archived dossiers" do
         let(:query) do
           "{
             demarche(number: #{procedure.id}) {
@@ -210,7 +210,8 @@ describe API::V2::GraphqlController do
 
         context 'with archived=true' do
           let(:archived_filter) { 'true' }
-          it "only archived dossiers should be returned" do
+
+          it 'returns only archived dossiers' do
             expect(gql_errors).to eq(nil)
             expect(gql_data).to eq(demarche: {
               id: procedure.to_typed_id,
@@ -224,7 +225,8 @@ describe API::V2::GraphqlController do
 
         context 'with archived=false' do
           let(:archived_filter) { 'false' }
-          it "only not archived dossiers should be returned" do
+
+          it 'returns only non-archived dossiers' do
             expect(gql_errors).to eq(nil)
             expect(gql_data).to eq(demarche: {
               id: procedure.to_typed_id,
@@ -237,7 +239,7 @@ describe API::V2::GraphqlController do
         end
       end
 
-      context "filter by minRevision" do
+      describe "filter by minRevision" do
         let(:query) do
           "{
             demarche(number: #{procedure.id}) {
@@ -266,7 +268,7 @@ describe API::V2::GraphqlController do
         end
       end
 
-      context "filter by maxRevision" do
+      describe "filter by maxRevision" do
         let(:query) do
           "{
             demarche(number: #{procedure.id}) {
@@ -296,7 +298,7 @@ describe API::V2::GraphqlController do
       end
     end
 
-    context "dossier" do
+    describe "dossier" do
       context "with individual" do
         let(:query) do
           "{
@@ -661,7 +663,7 @@ describe API::V2::GraphqlController do
       end
     end
 
-    context "deletedDossiers" do
+    describe "deletedDossiers" do
       let(:query) do
         "{
           demarche(number: #{procedure.id}) {
@@ -699,7 +701,7 @@ describe API::V2::GraphqlController do
       end
     end
 
-    context "champ" do
+    describe "champ" do
       let(:champ) { create(:champ_piece_justificative, dossier: dossier) }
       let(:byte_size) { 2712286911 }
 
@@ -766,7 +768,7 @@ describe API::V2::GraphqlController do
       end
     end
 
-    context "groupeInstructeur" do
+    describe "groupeInstructeur" do
       let(:groupe_instructeur) { procedure.groupe_instructeurs.first }
       let(:query) do
         "{
@@ -796,7 +798,7 @@ describe API::V2::GraphqlController do
       end
     end
 
-    context "mutations" do
+    describe "mutations" do
       describe 'dossierEnvoyerMessage' do
         context 'success' do
           let(:query) do
@@ -1563,7 +1565,7 @@ describe API::V2::GraphqlController do
       expect(gql_errors).not_to eq(nil)
     end
 
-    context "dossier" do
+    describe "dossier" do
       let(:query) { "{ dossier(number: #{dossier.id}) { id number usager { email } } }" }
 
       it "should return error" do
@@ -1572,7 +1574,7 @@ describe API::V2::GraphqlController do
       end
     end
 
-    context "mutation" do
+    describe "mutation" do
       let(:query) do
         "mutation {
           dossierEnvoyerMessage(input: {

From bb1bc9140dd223e22ed4330ac2b2ff5824cf43bc Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 15:33:42 +0000
Subject: [PATCH 08/14] graphql_controller_spec: remove unused line

---
 spec/controllers/api/v2/graphql_controller_spec.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
index 1e4b2a3cf..6a9dd2ac7 100644
--- a/spec/controllers/api/v2/graphql_controller_spec.rb
+++ b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -14,7 +14,6 @@ describe API::V2::GraphqlController do
   end
   let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 1.day.ago) }
   let(:dossier2) { create(:dossier, :en_construction, :with_individual, :archived, procedure: procedure, en_construction_at: 3.days.ago) }
-  let(:dossier_brouillon) { create(:dossier, :with_individual, procedure: procedure) }
   let(:dossiers) { [dossier2, dossier1, dossier] }
   let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) }
 

From e02251c0e5b47c263c4a87ee38903e04ffa896fe Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 17:04:38 +0100
Subject: [PATCH 09/14] graphql_controller_spec: populate all champs only when
 required

Creating a dossier with available champs populated is slow. We can
create simpler dossiers in the default case, and only populate all
champs when the example requires it.

Speeds up this spec from 2mn 20s to 1m 55s.
---
 .../api/v2/graphql_controller_spec.rb         | 22 ++++++++++---------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
index 6a9dd2ac7..37288ece4 100644
--- a/spec/controllers/api/v2/graphql_controller_spec.rb
+++ b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -2,16 +2,7 @@ describe API::V2::GraphqlController do
   let(:admin) { create(:administrateur) }
   let(:token) { admin.renew_api_token }
   let(:procedure) { create(:procedure, :published, :for_individual, :with_service, :with_all_champs, :with_all_annotations, administrateurs: [admin]) }
-  let(:dossier) do
-    dossier = create(:dossier,
-      :en_construction,
-      :with_populated_champs,
-      :with_populated_annotations,
-      :with_individual,
-      procedure: procedure)
-    create(:commentaire, :with_file, dossier: dossier, email: 'test@test.com')
-    dossier
-  end
+  let(:dossier)  { create(:dossier, :en_construction, :with_individual, procedure: procedure) }
   let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 1.day.ago) }
   let(:dossier2) { create(:dossier, :en_construction, :with_individual, :archived, procedure: procedure, en_construction_at: 3.days.ago) }
   let(:dossiers) { [dossier2, dossier1, dossier] }
@@ -298,6 +289,17 @@ describe API::V2::GraphqlController do
     end
 
     describe "dossier" do
+      let(:dossier) do
+        dossier = create(:dossier,
+                         :en_construction,
+                         :with_populated_champs,
+                         :with_populated_annotations,
+                         :with_individual,
+                         procedure: procedure)
+        create(:commentaire, :with_file, dossier: dossier, email: 'test@test.com')
+        dossier
+      end
+
       context "with individual" do
         let(:query) do
           "{

From f0e045ad250c731ff739c379a3e5d72bacbad830 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 16:00:40 +0000
Subject: [PATCH 10/14] graphql_controller_spec: create all types de champs
 only when required

Creating a procedure with all available types de champ is slow. We can
create a simpler procedure in the default case, and only create all
types de champs when the example requires it.

Speeds up this spec from 1m 55s to 0m 57s.
---
 .../api/v2/graphql_controller_spec.rb         | 104 ++++++++++--------
 1 file changed, 56 insertions(+), 48 deletions(-)

diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
index 37288ece4..855d66361 100644
--- a/spec/controllers/api/v2/graphql_controller_spec.rb
+++ b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -1,10 +1,11 @@
 describe API::V2::GraphqlController do
   let(:admin) { create(:administrateur) }
   let(:token) { admin.renew_api_token }
-  let(:procedure) { create(:procedure, :published, :for_individual, :with_service, :with_all_champs, :with_all_annotations, administrateurs: [admin]) }
+  let(:procedure) { create(:procedure, :published, :for_individual, :with_service, administrateurs: [admin]) }
   let(:dossier)  { create(:dossier, :en_construction, :with_individual, procedure: procedure) }
   let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 1.day.ago) }
   let(:dossier2) { create(:dossier, :en_construction, :with_individual, :archived, procedure: procedure, en_construction_at: 3.days.ago) }
+  #let(:dossiers) { [dossier2, dossier1, dossier] }
   let(:dossiers) { [dossier2, dossier1, dossier] }
   let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) }
 
@@ -107,53 +108,57 @@ describe API::V2::GraphqlController do
     end
 
     describe "demarche" do
-      it "should be returned" do
-        expect(gql_errors).to eq(nil)
-        expect(gql_data).to eq(demarche: {
-          id: procedure.to_typed_id,
-          number: procedure.id,
-          title: procedure.libelle,
-          description: procedure.description,
-          state: 'publiee',
-          dateFermeture: nil,
-          dateCreation: procedure.created_at.iso8601,
-          dateDerniereModification: procedure.updated_at.iso8601,
-          groupeInstructeurs: [
-            {
-              instructeurs: [{ email: instructeur.email }],
-              label: "défaut"
-            }
-          ],
-          revisions: procedure.revisions.map { |revision| { id: revision.to_typed_id } },
-          draftRevision: { id: procedure.draft_revision.to_typed_id },
-          publishedRevision: {
-            id: procedure.published_revision.to_typed_id,
-            champDescriptors: procedure.published_types_de_champ.map do |tdc|
+      describe "query a demarche" do
+        let(:procedure) { create(:procedure, :published, :for_individual, :with_service, :with_all_champs, :with_all_annotations, administrateurs: [admin]) }
+
+        it "returns the demarche" do
+          expect(gql_errors).to eq(nil)
+          expect(gql_data).to eq(demarche: {
+            id: procedure.to_typed_id,
+            number: procedure.id,
+            title: procedure.libelle,
+            description: procedure.description,
+            state: 'publiee',
+            dateFermeture: nil,
+            dateCreation: procedure.created_at.iso8601,
+            dateDerniereModification: procedure.updated_at.iso8601,
+            groupeInstructeurs: [
               {
-                type: tdc.type_champ
+                instructeurs: [{ email: instructeur.email }],
+                label: "défaut"
               }
-            end
-          },
-          service: {
-            nom: procedure.service.nom,
-            typeOrganisme: procedure.service.type_organisme,
-            organisme: procedure.service.organisme
-          },
-          champDescriptors: procedure.types_de_champ.map do |tdc|
-            {
-              id: tdc.to_typed_id,
-              label: tdc.libelle,
-              type: tdc.type_champ,
-              description: tdc.description,
-              required: tdc.mandatory?,
-              champDescriptors: tdc.repetition? ? tdc.reload.types_de_champ.map { |tdc| { id: tdc.to_typed_id, type: tdc.type_champ } } : nil,
-              options: tdc.drop_down_list? ? tdc.drop_down_list_options.reject(&:empty?) : nil
+            ],
+            revisions: procedure.revisions.map { |revision| { id: revision.to_typed_id } },
+            draftRevision: { id: procedure.draft_revision.to_typed_id },
+            publishedRevision: {
+              id: procedure.published_revision.to_typed_id,
+              champDescriptors: procedure.published_types_de_champ.map do |tdc|
+                {
+                  type: tdc.type_champ
+                }
+              end
+            },
+            service: {
+              nom: procedure.service.nom,
+              typeOrganisme: procedure.service.type_organisme,
+              organisme: procedure.service.organisme
+            },
+            champDescriptors: procedure.types_de_champ.map do |tdc|
+              {
+                id: tdc.to_typed_id,
+                label: tdc.libelle,
+                type: tdc.type_champ,
+                description: tdc.description,
+                required: tdc.mandatory?,
+                champDescriptors: tdc.repetition? ? tdc.reload.types_de_champ.map { |tdc| { id: tdc.to_typed_id, type: tdc.type_champ } } : nil,
+                options: tdc.drop_down_list? ? tdc.drop_down_list_options.reject(&:empty?) : nil
+              }
+            end,
+            dossiers: {
+              nodes: dossiers.map { |dossier| { id: dossier.to_typed_id } }
             }
-          end,
-          dossiers: {
-            nodes: dossiers.map { |dossier| { id: dossier.to_typed_id } }
-          }
-        })
+          })
+        end
       end
 
       describe "filter dossiers" do
@@ -300,7 +305,8 @@ describe API::V2::GraphqlController do
         dossier
       end
 
-      context "with individual" do
+      context "for individual" do
+        let(:procedure) { create(:procedure, :published, :for_individual, :with_service, :with_all_champs, :with_all_annotations, administrateurs: [admin]) }
         let(:query) do
           "{
             dossier(number: #{dossier.id}) {
@@ -452,7 +458,7 @@ describe API::V2::GraphqlController do
         end
       end
 
-      context "with entreprise" do
+      context "for entreprise" do
         let(:procedure_for_entreprise) { create(:procedure, :published, administrateurs: [admin]) }
         let(:dossier) { create(:dossier, :en_construction, :with_entreprise, procedure: procedure_for_entreprise) }
 
@@ -725,7 +731,7 @@ describe API::V2::GraphqlController do
         }
       end
 
-      context "when file is really big" do
+      context "when the file is really big" do
         before do
           champ.piece_justificative_file.blob.update(byte_size: byte_size)
         end
@@ -1325,6 +1331,8 @@ describe API::V2::GraphqlController do
       end
 
       describe 'dossierModifierAnnotation' do
+        let(:procedure) { create(:procedure, :published, :for_individual, :with_service, :with_all_annotations, administrateurs: [admin]) }
+
         describe 'text' do
           let(:query) do
             "mutation {

From 0a541d678d4d96f41b6adf9218f035dc957eb046 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 17:12:14 +0100
Subject: [PATCH 11/14] graphql_controller_spec: create several dossiers only
 when required

Creating dossiers is faster than creating a procedure, but still slow.
We can create a single dossier in the default case, and only create
several others when the example requires it.

Speeds up this spec from 0m 57s to 0m 49s.
---
 spec/controllers/api/v2/graphql_controller_spec.rb | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb
index 855d66361..e88feeac8 100644
--- a/spec/controllers/api/v2/graphql_controller_spec.rb
+++ b/spec/controllers/api/v2/graphql_controller_spec.rb
@@ -5,8 +5,7 @@ describe API::V2::GraphqlController do
   let(:dossier)  { create(:dossier, :en_construction, :with_individual, procedure: procedure) }
   let(:dossier1) { create(:dossier, :en_construction, :with_individual, procedure: procedure, en_construction_at: 1.day.ago) }
   let(:dossier2) { create(:dossier, :en_construction, :with_individual, :archived, procedure: procedure, en_construction_at: 3.days.ago) }
-  #let(:dossiers) { [dossier2, dossier1, dossier] }
-  let(:dossiers) { [dossier2, dossier1, dossier] }
+  let(:dossiers) { [dossier] }
   let(:instructeur) { create(:instructeur, followed_dossiers: dossiers) }
 
   def compute_checksum_in_chunks(io)
@@ -162,6 +161,8 @@ describe API::V2::GraphqlController do
       end
 
       describe "filter dossiers" do
+        let(:dossiers) { [dossier, dossier1, dossier2] }
+
         let(:query) do
           "{
             demarche(number: #{procedure.id}) {
@@ -189,6 +190,7 @@ describe API::V2::GraphqlController do
       end
 
       describe "filter archived dossiers" do
+        let(:dossiers) { [dossier, dossier1, dossier2] }
         let(:query) do
           "{
             demarche(number: #{procedure.id}) {
@@ -911,6 +913,7 @@ describe API::V2::GraphqlController do
       end
 
       describe 'dossierPasserEnInstruction' do
+        let(:dossiers) { [dossier2, dossier1, dossier] }
         let(:dossier) { create(:dossier, :en_construction, :with_individual, procedure: procedure) }
         let(:instructeur_id) { instructeur.to_typed_id }
         let(:disable_notification) { false }

From 5f2233d07dbd49414779a5dc5684c7a52ac26877 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Wed, 3 Nov 2021 19:37:43 +0100
Subject: [PATCH 12/14] specs: reduce BCrypt complexity during tests

BCrypt is used to compute Instructeur tokens, and takes a surprisingly
ong time during specs.

Reducing the complexity to speed it up.

Speeds up this spec from 0m 57s to 0m 20s.
---
 config/environments/test.rb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/config/environments/test.rb b/config/environments/test.rb
index 4a8e4e0d9..540394959 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -76,4 +76,10 @@ Rails.application.configure do
     debounce_delay: 500,
     status_visible_duration: 500
   }
+
+  # BCrypt is slow by design - but during tests we want to make it faster
+  # to compute hashes of passwords.
+  silence_warnings do
+    BCrypt::Engine::DEFAULT_COST = BCrypt::Engine::MIN_COST
+  end
 end

From e4317e8c7ef917e43779d6eb560e45093c4d42ca Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Wed, 3 Nov 2021 16:21:20 +0100
Subject: [PATCH 13/14] task(dossiers): delete MAE procedures expired dossiers

---
 lib/tasks/ds/destroy_expired_dossiers_mae.rake | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 lib/tasks/ds/destroy_expired_dossiers_mae.rake

diff --git a/lib/tasks/ds/destroy_expired_dossiers_mae.rake b/lib/tasks/ds/destroy_expired_dossiers_mae.rake
new file mode 100644
index 000000000..67d44858f
--- /dev/null
+++ b/lib/tasks/ds/destroy_expired_dossiers_mae.rake
@@ -0,0 +1,16 @@
+namespace :ds do
+  desc 'DS task: destroy_expired_dossiers_mae'
+  task destroy_expired_dossiers_mae: :environment do
+    dossiers = Dossier.state_termine
+      .where("termine_close_to_expiration_notice_sent_at + INTERVAL :expires_in < :now", { now: Time.zone.now, expires_in: '30 days' })
+      .joins(:groupe_instructeur)
+      .where(groupe_instructeur: { procedure_id: [47787, 47844, 47478, 47865] })
+    progress = ProgressReport.new(dossiers.count)
+
+    dossiers.find_each do |dossier|
+      dossier.expired_keep_track_and_destroy!
+      progress.inc
+    end
+    progress.finish
+  end
+end

From 122cdacbc2b161ef933f9aaa601fc0946d9cbfd7 Mon Sep 17 00:00:00 2001
From: Paul Chavard <pro@paul.chavard.net>
Date: Wed, 3 Nov 2021 21:29:00 +0100
Subject: [PATCH 14/14] fix(avis): destroy avis for discarded en_construction
 dossiers

---
 app/models/avis.rb          |  1 +
 app/models/dossier.rb       |  1 +
 spec/models/dossier_spec.rb | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/app/models/avis.rb b/app/models/avis.rb
index bd23547cd..2f56b1b89 100644
--- a/app/models/avis.rb
+++ b/app/models/avis.rb
@@ -50,6 +50,7 @@ class Avis < ApplicationRecord
   scope :by_latest, -> { order(updated_at: :desc) }
   scope :updated_since?, -> (date) { where('avis.updated_at > ?', date) }
   scope :discarded_termine_expired, -> { unscope(:joins).where(dossier: Dossier.discarded_termine_expired) }
+  scope :discarded_en_construction_expired, -> { unscope(:joins).where(dossier: Dossier.discarded_en_construction_expired) }
 
   # The form allows subtmitting avis requests to several emails at once,
   # hence this virtual attribute.
diff --git a/app/models/dossier.rb b/app/models/dossier.rb
index d458dafca..cc278f844 100644
--- a/app/models/dossier.rb
+++ b/app/models/dossier.rb
@@ -977,6 +977,7 @@ class Dossier < ApplicationRecord
 
     transaction do
       DossierOperationLog.discarded_en_construction_expired.destroy_all
+      Avis.discarded_en_construction_expired.destroy_all
       discarded_en_construction_expired.destroy_all
     end
 
diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb
index b3d1d0980..b25fb4f25 100644
--- a/spec/models/dossier_spec.rb
+++ b/spec/models/dossier_spec.rb
@@ -1461,6 +1461,42 @@ describe Dossier do
       expect(dossier.destroy).to be_truthy
       expect(transfer.reload).not_to be_nil
     end
+
+    context 'discarded' do
+      context 'en_construction' do
+        let(:dossier) { create(:dossier, :en_construction) }
+
+        before do
+          create(:avis, dossier: dossier)
+          Timecop.travel(2.weeks.ago) do
+            dossier.discard!
+          end
+          dossier.reload
+        end
+
+        it "can destroy dossier with avis" do
+          Avis.discarded_en_construction_expired.destroy_all
+          expect(dossier.destroy).to be_truthy
+        end
+      end
+
+      context 'termine' do
+        let(:dossier) { create(:dossier, :accepte) }
+
+        before do
+          create(:avis, dossier: dossier)
+          Timecop.travel(2.weeks.ago) do
+            dossier.discard!
+          end
+          dossier.reload
+        end
+
+        it "can destroy dossier with avis" do
+          Avis.discarded_termine_expired.destroy_all
+          expect(dossier.destroy).to be_truthy
+        end
+      end
+    end
   end
 
   describe "#spreadsheet_columns" do