Merge pull request #5407 from betagouv/dev

2020-07-22-01

app/assets/stylesheets/new_design/labels.scss

@@ -55,3 +55,17 @@
     @extend .without-continuation;
   }
 }
+
+// Labels that we only want for screen readers
+// https://www.coolfields.co.uk/2016/05/text-for-screen-readers-only-updated/
+.screen-reader-text {
+  border: none;
+  clip: rect(1px, 1px, 1px, 1px);
+  clip-path: inset(50%);
+  height: 1px;
+  margin: -1px;
+  width: 1px;
+  overflow: hidden;
+  position: absolute !important;
+  word-wrap: normal !important;
+}

app/controllers/new_administrateur/procedures_controller.rb

@@ -33,6 +33,8 @@ module NewAdministrateur
       else
         flash.notice = 'Démarche enregistrée.'
         current_administrateur.instructeur.assign_to_procedure(@procedure)
+        # FIXUP: needed during transition to revisions
+        RevisionsMigration.add_revisions(@procedure)
 
         redirect_to champs_admin_procedure_path(@procedure)
       end

app/controllers/new_administrateur/types_de_champ_controller.rb

@@ -2,11 +2,12 @@ module NewAdministrateur
   class TypesDeChampController < AdministrateurController
     before_action :retrieve_procedure, only: [:create, :update, :move, :destroy]
     before_action :procedure_locked?, only: [:create, :update, :move, :destroy]
+    before_action :revisions_migration
 
     def create
-      type_de_champ = TypeDeChamp.new(type_de_champ_create_params)
+      type_de_champ = @procedure.draft_revision.add_type_de_champ(type_de_champ_create_params)
 
-      if type_de_champ.save
+      if type_de_champ.valid?
         reset_procedure
         render json: serialize_type_de_champ(type_de_champ), status: :created
       else
@@ -15,7 +16,7 @@ module NewAdministrateur
     end
 
     def update
-      type_de_champ = TypeDeChamp.where(procedure: @procedure).find(params[:id])
+      type_de_champ = @procedure.draft_revision.find_or_clone_type_de_champ(type_de_champ_stable_id)
 
       if type_de_champ.update(type_de_champ_update_params)
         reset_procedure
@@ -26,18 +27,13 @@ module NewAdministrateur
     end
 
     def move
-      type_de_champ = TypeDeChamp.where(procedure: @procedure).find(params[:id])
+      @procedure.draft_revision.move_type_de_champ(type_de_champ_stable_id, (params[:position] || params[:order_place]).to_i)
-      new_index = params[:order_place].to_i
-
-      @procedure.move_type_de_champ(type_de_champ, new_index)
 
       head :no_content
     end
 
     def destroy
-      type_de_champ = TypeDeChamp.where(procedure: @procedure).find(params[:id])
+      @procedure.draft_revision.remove_type_de_champ(type_de_champ_stable_id)
-
-      type_de_champ.destroy!
       reset_procedure
 
       head :no_content
@@ -45,6 +41,15 @@ module NewAdministrateur
 
     private
 
+    def type_de_champ_stable_id
+      TypeDeChamp.find(params[:id]).stable_id
+    end
+
+    def revisions_migration
+      # FIXUP: needed during transition to revisions
+      RevisionsMigration.add_revisions(@procedure)
+    end
+
     def serialize_type_de_champ(type_de_champ)
       {
         type_de_champ: type_de_champ.as_json(
@@ -55,6 +60,7 @@ module NewAdministrateur
             :parent_id,
             :private,
             :procedure_id,
+            :revision_id,
             :stable_id,
             :type,
             :updated_at
@@ -72,7 +78,7 @@ module NewAdministrateur
     end
 
     def type_de_champ_create_params
-      params.required(:type_de_champ).permit(:cadastres,
+      type_de_champ_params = params.required(:type_de_champ).permit(:cadastres,
         :description,
         :drop_down_list_value,
         :libelle,
@@ -83,7 +89,13 @@ module NewAdministrateur
         :piece_justificative_template,
         :private,
         :quartiers_prioritaires,
-        :type_champ).merge(procedure: @procedure)
+        :type_champ)
+
+      if type_de_champ_params[:parent_id].present?
+        type_de_champ_params[:parent_id] = TypeDeChamp.find(type_de_champ_params[:parent_id]).stable_id
+      end
+
+      type_de_champ_params
     end
 
     def type_de_champ_update_params

app/controllers/users/dossiers_controller.rb

@@ -258,7 +258,10 @@ module Users
         return redirect_to url_for dossiers_path
       end
 
+      # FIXUP: needed during transition to revisions
+      RevisionsMigration.add_revisions(procedure)
       dossier = Dossier.new(
+        revision: procedure.active_revision,
         groupe_instructeur: procedure.defaut_groupe_instructeur,
         user: current_user,
         state: Dossier.states.fetch(:brouillon)

app/javascript/components/TypesDeChampEditor/components/TypeDeChampCarteOptions.js

@@ -15,7 +15,7 @@ function TypeDeChampCarteOptions({ isVisible, children }) {
 
 TypeDeChampCarteOptions.propTypes = {
   isVisible: PropTypes.bool,
-  children: PropTypes.array
+  children: PropTypes.node
 };
 
 export default TypeDeChampCarteOptions;

app/jobs/tmp_dossiers_migrate_revisions_job.rb

@@ -0,0 +1,22 @@
+class TmpDossiersMigrateRevisionsJob < ApplicationJob
+  def perform(except)
+    dossiers = Dossier.with_discarded.where(revision_id: nil)
+
+    dossiers.where
+      .not(id: except)
+      .includes(procedure: [:draft_revision, :published_revision])
+      .limit(2000)
+      .find_each do |dossier|
+        if dossier.procedure.present?
+          dossier.revision = dossier.procedure.active_revision
+          dossier.save!(validate: false)
+        else
+          except << dossier.id
+        end
+      end
+
+    if dossiers.where.not(id: except).exists?
+      TmpDossiersMigrateRevisionsJob.perform_later(except)
+    end
+  end
+end

app/lib/active_storage/downloadable_file.rb

@@ -24,11 +24,14 @@ class ActiveStorage::DownloadableFile
   private
 
   def self.timestamped_filename(piece_justificative)
+    # we pad the original file name with a timestamp
+    # and a short id in order to help identify multiple versions and avoid name collisions
     extension = File.extname(piece_justificative.filename.to_s)
     basename = File.basename(piece_justificative.filename.to_s, extension)
-    timestamp = piece_justificative.created_at.strftime("%d-%m-%Y-%H-%S")
+    timestamp = piece_justificative.created_at.strftime("%d-%m-%Y-%H-%M")
+    id = piece_justificative.id % 10000
 
-    "#{basename}-#{timestamp}#{extension}"
+    "#{basename}-#{timestamp}-#{id}#{extension}"
   end
 
   def using_local_backend?

app/models/dossier.rb

@@ -48,6 +48,7 @@ class Dossier < ApplicationRecord
 
   belongs_to :groupe_instructeur
   has_one :procedure, through: :groupe_instructeur
+  belongs_to :revision, class_name: 'ProcedureRevision', optional: true
   belongs_to :user
 
   accepts_nested_attributes_for :champs

app/models/procedure.rb

@@ -14,6 +14,9 @@ class Procedure < ApplicationRecord
 
   has_many :types_de_champ, -> { root.public_only.ordered }, inverse_of: :procedure, dependent: :destroy
   has_many :types_de_champ_private, -> { root.private_only.ordered }, class_name: 'TypeDeChamp', inverse_of: :procedure, dependent: :destroy
+  has_many :revisions, class_name: 'ProcedureRevision', inverse_of: :procedure, dependent: :destroy
+  belongs_to :draft_revision, class_name: 'ProcedureRevision', optional: true
+  belongs_to :published_revision, class_name: 'ProcedureRevision', optional: true
   has_many :deleted_dossiers, dependent: :destroy
 
   has_one :module_api_carto, dependent: :destroy
@@ -23,6 +26,10 @@ class Procedure < ApplicationRecord
   belongs_to :canonical_procedure, class_name: 'Procedure'
   belongs_to :service
 
+  def active_revision
+    brouillon? ? draft_revision : published_revision
+  end
+
   has_many :administrateurs_procedures
   has_many :administrateurs, through: :administrateurs_procedures, after_remove: -> (procedure, _admin) { procedure.validate! }
   has_many :groupe_instructeurs, dependent: :destroy
@@ -239,6 +246,7 @@ class Procedure < ApplicationRecord
   def new_dossier
     Dossier.new(
       procedure: self,
+      revision: active_revision,
       champs: build_champs,
       champs_private: build_champs_private,
       groupe_instructeur: defaut_groupe_instructeur
@@ -265,37 +273,23 @@ class Procedure < ApplicationRecord
     publiees.find(id)
   end
 
-  def switch_types_de_champ(index_of_first_element)
-    switch_list_order(types_de_champ, index_of_first_element)
-  end
-
-  def switch_types_de_champ_private(index_of_first_element)
-    switch_list_order(types_de_champ_private, index_of_first_element)
-  end
-
-  def switch_list_order(list, index_of_first_element)
-    if index_of_first_element < 0 ||
-      index_of_first_element == list.count - 1 ||
-      list.count < 1
-
-      false
-    else
-      list[index_of_first_element].update(order_place: index_of_first_element + 1)
-      list[index_of_first_element + 1].update(order_place: index_of_first_element)
-      reload
-
-      true
-    end
-  end
-
   def clone(admin, from_library)
+    # FIXUP: needed during transition to revisions
+    RevisionsMigration.add_revisions(self)
+
     is_different_admin = !admin.owns?(self)
 
     populate_champ_stable_ids
     include_list = {
-      attestation_template: nil,
+      attestation_template: [],
-      types_de_champ: [:types_de_champ],
+      draft_revision: {
-      types_de_champ_private: [:types_de_champ]
+        revision_types_de_champ: {
+          type_de_champ: :types_de_champ
+        },
+        revision_types_de_champ_private: {
+          type_de_champ: :types_de_champ
+        }
+      }
     }
     include_list[:groupe_instructeurs] = :instructeurs if !is_different_admin
     procedure = self.deep_clone(include: include_list, &method(:clone_attachments))
@@ -305,10 +299,7 @@ class Procedure < ApplicationRecord
     procedure.unpublished_at = nil
     procedure.published_at = nil
     procedure.lien_notice = nil
-
+    procedure.draft_revision.procedure = procedure
-    if is_different_admin || from_library
-      procedure.types_de_champ.each { |tdc| tdc.options&.delete(:old_pj) }
-    end
 
     if is_different_admin
       procedure.administrateurs = [admin]
@@ -336,6 +327,18 @@ class Procedure < ApplicationRecord
 
     procedure.save
 
+    # FIXUP: needed during transition to revisions
+    procedure.draft_revision.types_de_champ.each do |type_de_champ|
+      procedure.types_de_champ << type_de_champ
+    end
+    procedure.draft_revision.types_de_champ_private.each do |type_de_champ|
+      procedure.types_de_champ_private << type_de_champ
+    end
+
+    if is_different_admin || from_library
+      procedure.types_de_champ.each { |tdc| tdc.options&.delete(:old_pj) }
+    end
+
     procedure
   end
 
@@ -472,34 +475,6 @@ class Procedure < ApplicationRecord
     result
   end
 
-  def move_type_de_champ(type_de_champ, new_index)
-    types_de_champ, collection_attribute_name = if type_de_champ.parent&.repetition?
-      if type_de_champ.parent.private?
-        [type_de_champ.parent.types_de_champ, :types_de_champ_private_attributes]
-      else
-        [type_de_champ.parent.types_de_champ, :types_de_champ_attributes]
-      end
-    elsif type_de_champ.private?
-      [self.types_de_champ_private, :types_de_champ_private_attributes]
-    else
-      [self.types_de_champ, :types_de_champ_attributes]
-    end
-
-    attributes = move_type_de_champ_attributes(types_de_champ.to_a, type_de_champ, new_index)
-
-    if type_de_champ.parent&.repetition?
-      attributes = [
-        {
-          id: type_de_champ.parent.id,
-          libelle: type_de_champ.parent.libelle,
-          types_de_champ_attributes: attributes
-        }
-      ]
-    end
-
-    update!(collection_attribute_name => attributes)
-  end
-
   def process_dossiers!
     case declarative_with_state
     when Procedure.declarative_with_states.fetch(:en_instruction)
@@ -575,38 +550,34 @@ class Procedure < ApplicationRecord
     ApiEntrepriseToken.new(api_entreprise_token).expired?
   end
 
-  private
+  def create_new_revision
-
+    draft_revision.deep_clone(include: [:revision_types_de_champ, :revision_types_de_champ_private])
-  def move_type_de_champ_attributes(types_de_champ, type_de_champ, new_index)
-    old_index = types_de_champ.index(type_de_champ)
-    if types_de_champ.delete_at(old_index)
-      types_de_champ.insert(new_index, type_de_champ)
-        .map.with_index do |type_de_champ, index|
-          {
-            id: type_de_champ.id,
-            libelle: type_de_champ.libelle,
-            order_place: index
-          }
-        end
-    else
-      []
-    end
   end
 
+  private
+
   def before_publish
     update!(closed_at: nil, unpublished_at: nil)
   end
 
   def after_publish(canonical_procedure = nil)
-    update!(published_at: Time.zone.now, canonical_procedure: canonical_procedure)
+    # FIXUP: needed during transition to revisions
+    if RevisionsMigration.add_revisions(self)
+      update!(published_at: Time.zone.now, canonical_procedure: canonical_procedure)
+    else
+      update!(published_at: Time.zone.now, canonical_procedure: canonical_procedure, draft_revision: create_new_revision, published_revision: draft_revision)
+    end
   end
 
   def after_close
-    now = Time.zone.now
+    # FIXUP: needed during transition to revisions
-    update!(closed_at: now)
+    RevisionsMigration.add_revisions(self)
+    update!(closed_at: Time.zone.now)
   end
 
   def after_unpublish
+    # FIXUP: needed during transition to revisions
+    RevisionsMigration.add_revisions(self)
     update!(unpublished_at: Time.zone.now)
   end
 

app/models/procedure_revision.rb

@@ -0,0 +1,114 @@
+class ProcedureRevision < ApplicationRecord
+  self.implicit_order_column = :created_at
+  belongs_to :procedure, -> { with_discarded }, inverse_of: :revisions
+
+  has_many :revision_types_de_champ, -> { public_only.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision
+  has_many :revision_types_de_champ_private, -> { private_only.ordered }, class_name: 'ProcedureRevisionTypeDeChamp', foreign_key: :revision_id, dependent: :destroy, inverse_of: :revision
+  has_many :types_de_champ, through: :revision_types_de_champ, source: :type_de_champ
+  has_many :types_de_champ_private, through: :revision_types_de_champ_private, source: :type_de_champ
+
+  def add_type_de_champ(params)
+    params[:procedure] = procedure
+    params[:revision] = self
+
+    if params[:parent_id]
+      find_or_clone_type_de_champ(params.delete(:parent_id))
+        .types_de_champ
+        .tap do |types_de_champ|
+          params[:order_place] = types_de_champ.present? ? types_de_champ.last.order_place + 1 : 0
+        end.create(params)
+    elsif params[:private]
+      types_de_champ_private.tap do |types_de_champ|
+        # FIXUP: needed during transition to revisions
+        params[:order_place] = types_de_champ.present? ? types_de_champ.last.order_place + 1 : 0
+      end.create(params)
+    else
+      types_de_champ.tap do |types_de_champ|
+        # FIXUP: needed during transition to revisions
+        params[:order_place] = types_de_champ.present? ? types_de_champ.last.order_place + 1 : 0
+      end.create(params)
+    end
+  end
+
+  def find_or_clone_type_de_champ(id)
+    type_de_champ = find_type_de_champ_by_id(id)
+
+    if type_de_champ.revision == self
+      type_de_champ
+    elsif type_de_champ.parent.present?
+      find_or_clone_type_de_champ(type_de_champ.parent.stable_id).types_de_champ.find_by!(stable_id: id)
+    else
+      type_de_champ.revise!
+    end
+  end
+
+  def move_type_de_champ(id, position)
+    type_de_champ = find_type_de_champ_by_id(id)
+
+    if type_de_champ.parent.present?
+      repetition_type_de_champ = find_or_clone_type_de_champ(id).parent
+
+      move_type_de_champ_hash(repetition_type_de_champ.types_de_champ.to_a, type_de_champ, position).each do |(id, position)|
+        repetition_type_de_champ.types_de_champ.find(id).update!(order_place: position)
+      end
+    elsif type_de_champ.private?
+      move_type_de_champ_hash(types_de_champ_private.to_a, type_de_champ, position).each do |(id, position)|
+        revision_types_de_champ_private.find_by!(type_de_champ_id: id).update!(position: position)
+      end
+    else
+      move_type_de_champ_hash(types_de_champ.to_a, type_de_champ, position).each do |(id, position)|
+        revision_types_de_champ.find_by!(type_de_champ_id: id).update!(position: position)
+      end
+    end
+  end
+
+  def remove_type_de_champ(id)
+    type_de_champ = find_type_de_champ_by_id(id)
+
+    if type_de_champ.revision == self
+      type_de_champ.destroy
+    elsif type_de_champ.parent.present?
+      find_or_clone_type_de_champ(id).destroy
+    elsif type_de_champ.private?
+      types_de_champ_private.delete(type_de_champ)
+    else
+      types_de_champ.delete(type_de_champ)
+    end
+  end
+
+  def draft?
+    procedure.draft_revision == self
+  end
+
+  def locked?
+    !draft?
+  end
+
+  private
+
+  def find_type_de_champ_by_id(id)
+    types_de_champ.find_by(stable_id: id) ||
+      types_de_champ_private.find_by(stable_id: id) ||
+      types_de_champ_in_repetition.find_by!(stable_id: id)
+  end
+
+  def types_de_champ_in_repetition
+    parent_ids = types_de_champ.repetition.ids + types_de_champ_private.repetition.ids
+    TypeDeChamp.where(parent_id: parent_ids)
+  end
+
+  def move_type_de_champ_hash(types_de_champ, type_de_champ, new_index)
+    old_index = types_de_champ.index(type_de_champ)
+
+    if types_de_champ.delete_at(old_index)
+      types_de_champ.insert(new_index, type_de_champ)
+        .map.with_index do |type_de_champ, index|
+          # FIXUP: needed during transition to revisions
+          type_de_champ.update!(order_place: index)
+          [type_de_champ.id, index]
+        end
+    else
+      []
+    end
+  end
+end

app/models/procedure_revision_type_de_champ.rb

@@ -0,0 +1,32 @@
+class ProcedureRevisionTypeDeChamp < ApplicationRecord
+  belongs_to :revision, class_name: 'ProcedureRevision'
+  belongs_to :type_de_champ
+
+  scope :ordered, -> { order(:position) }
+  scope :public_only, -> { joins(:type_de_champ).where(types_de_champ: { private: false }) }
+  scope :private_only, -> { joins(:type_de_champ).where(types_de_champ: { private: true }) }
+
+  before_create :set_position
+
+  def private?
+    type_de_champ.private?
+  end
+
+  private
+
+  def set_position
+    self.position ||= if private?
+      if revision.types_de_champ_private.present?
+        revision.revision_types_de_champ_private.filter(&:persisted?).last.position + 1
+      else
+        0
+      end
+    else
+      if revision.types_de_champ.present?
+        revision.revision_types_de_champ.filter(&:persisted?).last.position + 1
+      else
+        0
+      end
+    end
+  end
+end

app/models/type_de_champ.rb

@@ -31,11 +31,14 @@ class TypeDeChamp < ApplicationRecord
   }
 
   belongs_to :procedure
+  belongs_to :revision, class_name: 'ProcedureRevision', optional: true
 
   belongs_to :parent, class_name: 'TypeDeChamp'
   has_many :types_de_champ, -> { ordered }, foreign_key: :parent_id, class_name: 'TypeDeChamp', inverse_of: :parent, dependent: :destroy
 
   store_accessor :options, :cadastres, :quartiers_prioritaires, :parcelles_agricoles, :old_pj, :drop_down_options, :skip_pj_validation
+  has_many :revision_types_de_champ, class_name: 'ProcedureRevisionTypeDeChamp', dependent: :destroy, inverse_of: :type_de_champ
+
   delegate :tags_for_template, to: :dynamic_type
 
   class WithIndifferentAccess
@@ -60,6 +63,7 @@ class TypeDeChamp < ApplicationRecord
   scope :private_only, -> { where(private: true) }
   scope :ordered, -> { order(order_place: :asc) }
   scope :root, -> { where(parent_id: nil) }
+  scope :repetition, -> { where(type_champ: type_champs.fetch(:repetition)) }
 
   has_many :champ, inverse_of: :type_de_champ, dependent: :destroy do
     def build(params = {})
@@ -225,6 +229,13 @@ class TypeDeChamp < ApplicationRecord
     GraphQL::Schema::UniqueWithinType.encode('Champ', stable_id)
   end
 
+  def revise!
+    types_de_champ_association = private? ? :revision_types_de_champ_private : :revision_types_de_champ
+    association = revision.send(types_de_champ_association).find_by!(type_de_champ: self)
+    association.update!(type_de_champ: deep_clone(include: [:types_de_champ], &method(:clone_attachments)))
+    association.type_de_champ
+  end
+
   FEATURE_FLAGS = {}
 
   def self.type_de_champ_types_for(procedure, user)
@@ -250,6 +261,7 @@ class TypeDeChamp < ApplicationRecord
       :parent_id,
       :private,
       :procedure_id,
+      :revision_id,
       :stable_id,
       :type,
       :updated_at
@@ -309,4 +321,23 @@ class TypeDeChamp < ApplicationRecord
       types_de_champ.destroy_all
     end
   end
+
+  def clone_attachments(original, kopy)
+    if original.is_a?(TypeDeChamp)
+      clone_attachment(:piece_justificative_template, original, kopy)
+    end
+  end
+
+  def clone_attachment(attribute, original, kopy)
+    original_attachment = original.send(attribute)
+    if original_attachment.attached?
+      kopy.send(attribute).attach({
+        io: StringIO.new(original_attachment.download),
+        filename: original_attachment.filename,
+        content_type: original_attachment.content_type,
+        # we don't want to run virus scanner on cloned file
+        metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE }
+      })
+    end
+  end
 end

app/services/revisions_migration.rb

@@ -0,0 +1,36 @@
+class RevisionsMigration
+  def self.add_revisions(procedure)
+    if procedure.draft_revision.present?
+      return false
+    end
+
+    procedure.draft_revision = procedure.revisions.create
+    procedure.save!(validate: false)
+
+    add_types_de_champs_to_revision(procedure, :types_de_champ)
+    add_types_de_champs_to_revision(procedure, :types_de_champ_private)
+
+    if !procedure.brouillon?
+      published_revision = procedure.draft_revision
+
+      procedure.draft_revision = procedure.create_new_revision
+      procedure.published_revision = published_revision
+      procedure.save!(validate: false)
+    end
+
+    true
+  end
+
+  def self.add_types_de_champs_to_revision(procedure, types_de_champ_scope)
+    types_de_champ = procedure.send(types_de_champ_scope)
+    types_de_champ.where(revision_id: nil).update_all(revision_id: procedure.draft_revision.id)
+
+    types_de_champ.each.with_index do |type_de_champ, index|
+      type_de_champ.types_de_champ.where(revision_id: nil).update_all(revision_id: procedure.draft_revision.id)
+      procedure.draft_revision.send(:"revision_#{types_de_champ_scope}").create!(
+        type_de_champ: type_de_champ,
+        position: index
+      )
+    end
+  end
+end

app/views/new_administrateur/procedures/show.html.haml

@@ -35,12 +35,19 @@
 
     - if !@procedure.locked?
       .card-admin
+        - if @procedure.types_de_champ.count > 0
+          %div
+            %span.icon.accept
+            %p.card-admin-status-accept Validé
+        - else
+          %div
+            %span.icon.clock
+            %p.card-admin-status-todo À faire
         %div
-          %span.icon.clock
+          %p.card-admin-title
-          %p.card-admin-status-todo À faire
+            %span.badge.baseline= @procedure.types_de_champ.count
-        %div
+            Champs du formulaire
-          %p.card-admin-title Formulaire
+          %p.card-admin-subtitle À remplir par les usagers
-          %p.card-admin-subtitle Champs à remplir par les usagers
         .card-admin-action
           = link_to 'Modifier', champs_admin_procedure_path(@procedure), class: 'button'
 

app/views/shared/dossiers/editable_champs/_champ_label.html.haml

@@ -3,7 +3,7 @@
   = form.label champ.main_value_name do
     = render partial: 'shared/dossiers/editable_champs/champ_label_content', locals: { champ: champ, seen_at: seen_at }
 - else
-  %h4.form-label
+  .form-label.mb-4
     = render partial: 'shared/dossiers/editable_champs/champ_label_content', locals: { champ: champ, seen_at: seen_at }
 
 - if champ.description.present?

config/initializers/date_select.rb

@@ -25,7 +25,7 @@ module ActionView
         end
         field_for = "#{prefix.join('_')}_#{@options[:field_name]}"
 
-        "<span class='hidden'><label for='#{field_for}_#{n}i'>#{label}</label></span>"
+        "<label class='screen-reader-text' for='#{field_for}_#{n}i'>#{label}</label>"
       end
 
       # Returns the separator for a given datetime component.

config/initializers/inflections.rb

@@ -12,6 +12,9 @@ ActiveSupport::Inflector.inflections(:en) do |inflect|
   inflect.acronym 'RNA'
   inflect.irregular 'type_de_champ', 'types_de_champ'
   inflect.irregular 'type_de_champ_private', 'types_de_champ_private'
+  inflect.irregular 'procedure_revision_type_de_champ', 'procedure_revision_types_de_champ'
+  inflect.irregular 'revision_type_de_champ', 'revision_types_de_champ'
+  inflect.irregular 'revision_type_de_champ_private', 'revision_types_de_champ_private'
   inflect.irregular 'assign_to', 'assign_tos'
   inflect.uncountable(['avis', 'pays'])
 end

db/migrate/20200707082260_create_procedure_revisions.rb

@@ -0,0 +1,24 @@
+class CreateProcedureRevisions < ActiveRecord::Migration[5.2]
+  def change
+    create_table :procedure_revisions do |t|
+      t.references :procedure, foreign_key: true, null: false, index: true
+
+      t.timestamps
+    end
+
+    add_column :dossiers, :revision_id, :bigint
+    add_column :types_de_champ, :revision_id, :bigint
+    add_column :procedures, :draft_revision_id, :bigint
+    add_column :procedures, :published_revision_id, :bigint
+
+    add_foreign_key :dossiers, :procedure_revisions, column: :revision_id
+    add_foreign_key :types_de_champ, :procedure_revisions, column: :revision_id
+    add_foreign_key :procedures, :procedure_revisions, column: :draft_revision_id
+    add_foreign_key :procedures, :procedure_revisions, column: :published_revision_id
+
+    add_index :dossiers, :revision_id
+    add_index :types_de_champ, :revision_id
+    add_index :procedures, :draft_revision_id
+    add_index :procedures, :published_revision_id
+  end
+end

db/migrate/20200707082261_create_procedure_revision_types_de_champ.rb

@@ -0,0 +1,14 @@
+class CreateProcedureRevisionTypesDeChamp < ActiveRecord::Migration[5.2]
+  def change
+    create_table :procedure_revision_types_de_champ do |t|
+      t.references :revision, null: false, index: true
+      t.references :type_de_champ, foreign_key: true, null: false, index: true
+
+      t.integer :position, null: false
+
+      t.timestamps
+    end
+
+    add_foreign_key :procedure_revision_types_de_champ, :procedure_revisions, column: :revision_id
+  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: 2020_07_15_143010) do
+ActiveRecord::Schema.define(version: 2020_07_16_143010) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -259,9 +259,11 @@ ActiveRecord::Schema.define(version: 2020_07_15_143010) do
     t.datetime "termine_close_to_expiration_notice_sent_at"
     t.index "to_tsvector('french'::regconfig, (search_terms || private_search_terms))", name: "index_dossiers_on_search_terms_private_search_terms", using: :gin
     t.index "to_tsvector('french'::regconfig, search_terms)", name: "index_dossiers_on_search_terms", using: :gin
+    t.bigint "revision_id"
     t.index ["archived"], name: "index_dossiers_on_archived"
     t.index ["groupe_instructeur_id"], name: "index_dossiers_on_groupe_instructeur_id"
     t.index ["hidden_at"], name: "index_dossiers_on_hidden_at"
+    t.index ["revision_id"], name: "index_dossiers_on_revision_id"
     t.index ["state"], name: "index_dossiers_on_state"
     t.index ["user_id"], name: "index_dossiers_on_user_id"
   end
@@ -476,6 +478,23 @@ ActiveRecord::Schema.define(version: 2020_07_15_143010) do
     t.index ["assign_to_id"], name: "index_procedure_presentations_on_assign_to_id", unique: true
   end
 
+  create_table "procedure_revision_types_de_champ", force: :cascade do |t|
+    t.bigint "revision_id", null: false
+    t.bigint "type_de_champ_id", null: false
+    t.integer "position", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["revision_id"], name: "index_procedure_revision_types_de_champ_on_revision_id"
+    t.index ["type_de_champ_id"], name: "index_procedure_revision_types_de_champ_on_type_de_champ_id"
+  end
+
+  create_table "procedure_revisions", force: :cascade do |t|
+    t.bigint "procedure_id", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["procedure_id"], name: "index_procedure_revisions_on_procedure_id"
+  end
+
   create_table "procedures", id: :serial, force: :cascade do |t|
     t.string "libelle"
     t.string "description"
@@ -517,10 +536,14 @@ ActiveRecord::Schema.define(version: 2020_07_15_143010) do
     t.datetime "unpublished_at"
     t.bigint "canonical_procedure_id"
     t.string "api_entreprise_token"
+    t.bigint "draft_revision_id"
+    t.bigint "published_revision_id"
     t.index ["declarative_with_state"], name: "index_procedures_on_declarative_with_state"
+    t.index ["draft_revision_id"], name: "index_procedures_on_draft_revision_id"
     t.index ["hidden_at"], name: "index_procedures_on_hidden_at"
     t.index ["parent_procedure_id"], name: "index_procedures_on_parent_procedure_id"
     t.index ["path", "closed_at", "hidden_at"], name: "index_procedures_on_path_and_closed_at_and_hidden_at", unique: true
+    t.index ["published_revision_id"], name: "index_procedures_on_published_revision_id"
     t.index ["service_id"], name: "index_procedures_on_service_id"
   end
 
@@ -592,9 +615,11 @@ ActiveRecord::Schema.define(version: 2020_07_15_143010) do
     t.jsonb "options"
     t.bigint "stable_id"
     t.bigint "parent_id"
+    t.bigint "revision_id"
     t.index ["parent_id"], name: "index_types_de_champ_on_parent_id"
     t.index ["private"], name: "index_types_de_champ_on_private"
     t.index ["procedure_id"], name: "index_types_de_champ_on_procedure_id"
+    t.index ["revision_id"], name: "index_types_de_champ_on_revision_id"
     t.index ["stable_id"], name: "index_types_de_champ_on_stable_id"
   end
 
@@ -660,18 +685,25 @@ ActiveRecord::Schema.define(version: 2020_07_15_143010) do
   add_foreign_key "dossier_operation_logs", "dossiers"
   add_foreign_key "dossier_operation_logs", "instructeurs"
   add_foreign_key "dossiers", "groupe_instructeurs"
+  add_foreign_key "dossiers", "procedure_revisions", column: "revision_id"
   add_foreign_key "dossiers", "users"
   add_foreign_key "feedbacks", "users"
   add_foreign_key "geo_areas", "champs"
   add_foreign_key "groupe_instructeurs", "procedures"
   add_foreign_key "initiated_mails", "procedures"
   add_foreign_key "procedure_presentations", "assign_tos"
+  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", "procedures"
+  add_foreign_key "procedures", "procedure_revisions", column: "draft_revision_id"
+  add_foreign_key "procedures", "procedure_revisions", column: "published_revision_id"
   add_foreign_key "procedures", "services"
   add_foreign_key "received_mails", "procedures"
   add_foreign_key "refused_mails", "procedures"
   add_foreign_key "services", "administrateurs"
   add_foreign_key "traitements", "dossiers"
   add_foreign_key "trusted_device_tokens", "instructeurs"
+  add_foreign_key "types_de_champ", "procedure_revisions", column: "revision_id"
   add_foreign_key "types_de_champ", "types_de_champ", column: "parent_id"
   add_foreign_key "users", "administrateurs"
   add_foreign_key "users", "instructeurs"

lib/tasks/deployment/20200625113026_migrate_revisions.rake

@@ -0,0 +1,22 @@
+namespace :after_party do
+  desc 'Deployment task: migrate_revisions'
+  task migrate_revisions: :environment do
+    puts "Running deploy task 'migrate_revisions'"
+
+    procedures = Procedure.with_discarded.where(draft_revision_id: nil)
+    progress = ProgressReport.new(procedures.count)
+
+    puts "Processing procedures"
+    procedures.find_each do |procedure|
+      RevisionsMigration.add_revisions(procedure)
+      progress.inc
+    end
+    progress.finish
+
+    TmpDossiersMigrateRevisionsJob.perform_later([])
+
+    # Update task as completed.  If you remove the line below, the task will
+    # run with every deploy (or every time you call after_party:run).
+    AfterParty::TaskRecord.create version: '20200625113026'
+  end
+end

spec/features/accessibilite/wcag_usager_spec.rb

@@ -62,7 +62,7 @@ feature 'wcag rules for usager', js: true do
       fill_in('individual_nom', with: 'nom')
       click_on 'Continuer'
 
-      expect(page).to be_accessible.skipping :'aria-input-field-name', :'heading-order', :label
+      expect(page).to be_accessible.skipping :'aria-input-field-name'
     end
   end
 
@@ -93,7 +93,7 @@ feature 'wcag rules for usager', js: true do
 
     scenario 'dossier' do
       visit dossier_path(dossier)
-      expect(page).to be_accessible.skipping :'heading-order', :label, :'aria-input-field-name'
+      expect(page).to be_accessible.skipping :'aria-input-field-name'
     end
 
     scenario 'merci' do
@@ -113,12 +113,12 @@ feature 'wcag rules for usager', js: true do
 
     scenario 'modifier' do
       visit modifier_dossier_path(dossier)
-      expect(page).to be_accessible.skipping :'aria-input-field-name', :'heading-order', :label
+      expect(page).to be_accessible.skipping :'aria-input-field-name'
     end
 
     scenario 'brouillon' do
       visit brouillon_dossier_path(dossier)
-      expect(page).to be_accessible.skipping :'aria-input-field-name', :'heading-order', :label
+      expect(page).to be_accessible.skipping :'aria-input-field-name'
     end
   end
 end

spec/features/instructeurs/instruction_spec.rb

@@ -138,6 +138,55 @@ feature 'Instructing a dossier:' do
     expect(page).to have_text("Dossier envoyé")
   end
 
+  context 'with dossiers having attached files', js: true do
+    let(:procedure) { create(:procedure, :published, :with_piece_justificative, instructeurs: [instructeur]) }
+    let(:dossier) { create(:dossier, :en_construction, procedure: procedure) }
+    let(:champ) { dossier.champs.first }
+    let(:path) { 'spec/fixtures/files/piece_justificative_0.pdf' }
+    let(:commentaire) { create(:commentaire, instructeur: instructeur, dossier: dossier) }
+
+    before do
+      champ.piece_justificative_file.attach(io: File.open(path), filename: "piece_justificative_0.pdf", content_type: "application/pdf")
+
+      log_in(instructeur.email, password)
+      visit instructeur_dossier_path(procedure, dossier)
+    end
+
+    scenario 'A instructeur can download an archive containing a single attachment' do
+      find(:css, '.attached').click
+      click_on 'Télécharger toutes les pièces jointes'
+      # For some reason, clicking the download link does not trigger the download in the headless browser ;
+      # So we need to go to the download link directly
+      visit telecharger_pjs_instructeur_dossier_path(procedure, dossier)
+
+      DownloadHelpers.wait_for_download
+      files = ZipTricks::FileReader.read_zip_structure(io: File.open(DownloadHelpers.download))
+
+      expect(DownloadHelpers.download).to include "dossier-#{dossier.id}.zip"
+      expect(files.size).to be 1
+      expect(files[0].filename.include?('piece_justificative_0')).to be_truthy
+      expect(files[0].uncompressed_size).to be File.size(path)
+    end
+
+    scenario 'A instructeur can download an archive containing several identical attachments' do
+      commentaire.piece_jointe.attach(io: File.open(path), filename: "piece_justificative_0.pdf", content_type: "application/pdf")
+
+      visit telecharger_pjs_instructeur_dossier_path(procedure, dossier)
+      DownloadHelpers.wait_for_download
+      files = ZipTricks::FileReader.read_zip_structure(io: File.open(DownloadHelpers.download))
+
+      expect(DownloadHelpers.download).to include "dossier-#{dossier.id}.zip"
+      expect(files.size).to be 2
+      expect(files[0].filename.include?('piece_justificative_0')).to be_truthy
+      expect(files[1].filename.include?('piece_justificative_0')).to be_truthy
+      expect(files[0].filename).not_to eq files[1].filename
+      expect(files[0].uncompressed_size).to be File.size(path)
+      expect(files[1].uncompressed_size).to be File.size(path)
+    end
+
+    after { DownloadHelpers.clear_downloads }
+  end
+
   def log_in(email, password, check_email: true)
     visit '/'
     click_on 'Connexion'

spec/features/users/brouillon_spec.rb

@@ -325,7 +325,7 @@ feature 'The user' do
     #     </select>
     #     <!-- … 4 other selects for month, year, minute and seconds -->
     # </div>
-    e = find(:xpath, ".//h4[contains(text()[normalize-space()], '#{libelle}')]")
+    e = find(:xpath, ".//div[contains(text()[normalize-space()], '#{libelle}')]")
     e.sibling('.datetime').first('select')[:id][0..-4]
   end
 

spec/models/procedure_revision_spec.rb

@@ -0,0 +1,164 @@
+describe ProcedureRevision do
+  let(:procedure) { create(:procedure, :with_type_de_champ, :with_type_de_champ_private, :with_repetition) }
+  let(:revision) { procedure.active_revision }
+  let(:type_de_champ) { revision.types_de_champ.first }
+  let(:type_de_champ_private) { revision.types_de_champ_private.first }
+  let(:type_de_champ_repetition) do
+    type_de_champ = revision.types_de_champ.repetition.first
+    type_de_champ.update(stable_id: 3333)
+    type_de_champ
+  end
+
+  before do
+    RevisionsMigration.add_revisions(procedure)
+  end
+
+  describe '#add_type_de_champ' do
+    it 'type_de_champ' do
+      expect(revision.types_de_champ.size).to eq(2)
+      new_type_de_champ = revision.add_type_de_champ({
+        type_champ: TypeDeChamp.type_champs.fetch(:text),
+        libelle: "Un champ text"
+      })
+      procedure.reload
+      expect(revision.types_de_champ.size).to eq(3)
+      expect(procedure.types_de_champ.size).to eq(3)
+
+      expect(procedure.types_de_champ.last).to eq(new_type_de_champ)
+      expect(revision.types_de_champ.last).to eq(new_type_de_champ)
+      expect(revision.revision_types_de_champ.last.position).to eq(2)
+      expect(revision.revision_types_de_champ.last.type_de_champ).to eq(new_type_de_champ)
+    end
+
+    it 'type_de_champ_private' do
+      expect(revision.types_de_champ_private.size).to eq(1)
+      revision.add_type_de_champ({
+        type_champ: TypeDeChamp.type_champs.fetch(:text),
+        libelle: "Un champ text",
+        private: true
+      })
+      procedure.reload
+      expect(revision.types_de_champ_private.size).to eq(2)
+      expect(procedure.types_de_champ_private.size).to eq(2)
+    end
+
+    it 'type_de_champ_repetition' do
+      expect(type_de_champ_repetition.types_de_champ.size).to eq(1)
+      revision.add_type_de_champ({
+        type_champ: TypeDeChamp.type_champs.fetch(:text),
+        libelle: "Un champ text",
+        parent_id: type_de_champ_repetition.stable_id
+      })
+      type_de_champ_repetition.reload
+      expect(type_de_champ_repetition.types_de_champ.size).to eq(2)
+    end
+  end
+
+  describe '#move_type_de_champ' do
+    let(:procedure) { create(:procedure, :with_type_de_champ, types_de_champ_count: 4) }
+    let(:last_type_de_champ) { revision.types_de_champ.last }
+
+    it 'move down' do
+      expect(revision.types_de_champ.index(type_de_champ)).to eq(0)
+      revision.move_type_de_champ(type_de_champ.stable_id, 2)
+      revision.reload
+      expect(revision.types_de_champ.index(type_de_champ)).to eq(2)
+    end
+
+    it 'move up' do
+      expect(revision.types_de_champ.index(last_type_de_champ)).to eq(3)
+      revision.move_type_de_champ(last_type_de_champ.stable_id, 0)
+      revision.reload
+      expect(revision.types_de_champ.index(last_type_de_champ)).to eq(0)
+    end
+
+    context 'repetition' do
+      let(:procedure) { create(:procedure, :with_repetition) }
+      let(:type_de_champ) { type_de_champ_repetition.types_de_champ.first }
+      let(:last_type_de_champ) { type_de_champ_repetition.types_de_champ.last }
+
+      before do
+        revision.add_type_de_champ({
+          type_champ: TypeDeChamp.type_champs.fetch(:text),
+          libelle: "Un champ text",
+          parent_id: type_de_champ_repetition.stable_id
+        })
+        revision.add_type_de_champ({
+          type_champ: TypeDeChamp.type_champs.fetch(:text),
+          libelle: "Un champ text",
+          parent_id: type_de_champ_repetition.stable_id
+        })
+        type_de_champ_repetition.reload
+      end
+
+      it 'move down' do
+        expect(type_de_champ_repetition.types_de_champ.index(type_de_champ)).to eq(0)
+        revision.move_type_de_champ(type_de_champ.stable_id, 2)
+        type_de_champ_repetition.reload
+        expect(type_de_champ_repetition.types_de_champ.index(type_de_champ)).to eq(2)
+      end
+
+      it 'move up' do
+        expect(type_de_champ_repetition.types_de_champ.index(last_type_de_champ)).to eq(2)
+        revision.move_type_de_champ(last_type_de_champ.stable_id, 0)
+        type_de_champ_repetition.reload
+        expect(type_de_champ_repetition.types_de_champ.index(last_type_de_champ)).to eq(0)
+      end
+    end
+  end
+
+  describe '#remove_type_de_champ' do
+    it 'type_de_champ' do
+      expect(revision.types_de_champ.size).to eq(2)
+      revision.remove_type_de_champ(type_de_champ.stable_id)
+      procedure.reload
+      expect(revision.types_de_champ.size).to eq(1)
+      expect(procedure.types_de_champ.size).to eq(1)
+    end
+
+    it 'type_de_champ_private' do
+      expect(revision.types_de_champ_private.size).to eq(1)
+      revision.remove_type_de_champ(type_de_champ_private.stable_id)
+      procedure.reload
+      expect(revision.types_de_champ_private.size).to eq(0)
+      expect(procedure.types_de_champ_private.size).to eq(0)
+    end
+
+    it 'type_de_champ_repetition' do
+      expect(type_de_champ_repetition.types_de_champ.size).to eq(1)
+      expect(revision.types_de_champ.size).to eq(2)
+      revision.remove_type_de_champ(type_de_champ_repetition.types_de_champ.first.stable_id)
+      procedure.reload
+      type_de_champ_repetition.reload
+      expect(type_de_champ_repetition.types_de_champ.size).to eq(0)
+      expect(revision.types_de_champ.size).to eq(2)
+      expect(procedure.types_de_champ.size).to eq(2)
+    end
+  end
+
+  describe '#create_new_revision' do
+    let(:new_revision) { procedure.create_new_revision }
+
+    before { new_revision.save }
+
+    it 'should be part of procedure' do
+      expect(new_revision.procedure).to eq(revision.procedure)
+      expect(procedure.revisions.count).to eq(2)
+      expect(procedure.revisions).to eq([revision, new_revision])
+    end
+
+    it 'should have types_de_champ' do
+      expect(new_revision.types_de_champ.count).to eq(2)
+      expect(new_revision.types_de_champ_private.count).to eq(1)
+      expect(new_revision.types_de_champ).to eq(revision.types_de_champ)
+      expect(new_revision.types_de_champ_private).to eq(revision.types_de_champ_private)
+
+      expect(new_revision.revision_types_de_champ.count).to eq(2)
+      expect(new_revision.revision_types_de_champ_private.count).to eq(1)
+      expect(new_revision.revision_types_de_champ.count).to eq(revision.revision_types_de_champ.count)
+      expect(new_revision.revision_types_de_champ_private.count).to eq(revision.revision_types_de_champ_private.count)
+      expect(new_revision.revision_types_de_champ).not_to eq(revision.revision_types_de_champ)
+      expect(new_revision.revision_types_de_champ_private).not_to eq(revision.revision_types_de_champ_private)
+    end
+  end
+end

spec/models/procedure_spec.rb

@@ -288,40 +288,6 @@ describe Procedure do
     it { expect(subject.last).to eq(type_de_champ_0) }
   end
 
-  describe '#switch_types_de_champ' do
-    let(:procedure) { create(:procedure) }
-    let(:index) { 0 }
-    subject { procedure.switch_types_de_champ(index) }
-
-    context 'when procedure has no types_de_champ' do
-      it { expect(subject).to eq(false) }
-    end
-    context 'when procedure has 3 types de champ' do
-      let!(:type_de_champ_0) { create(:type_de_champ, procedure: procedure, order_place: 0) }
-      let!(:type_de_champ_1) { create(:type_de_champ, procedure: procedure, order_place: 1) }
-      let!(:type_de_champ_2) { create(:type_de_champ, procedure: procedure, order_place: 2) }
-      context 'when index is not the last element' do
-        it { expect(subject).to eq(true) }
-        it 'switches the position of the champ N and N+1' do
-          subject
-          expect(procedure.types_de_champ[0]).to eq(type_de_champ_1)
-          expect(procedure.types_de_champ[0].order_place).to eq(0)
-          expect(procedure.types_de_champ[1]).to eq(type_de_champ_0)
-          expect(procedure.types_de_champ[1].order_place).to eq(1)
-        end
-        it 'doesn’t move other types de champ' do
-          subject
-          expect(procedure.types_de_champ[2]).to eq(type_de_champ_2)
-          expect(procedure.types_de_champ[2].order_place).to eq(2)
-        end
-      end
-      context 'when index is the last element' do
-        let(:index) { 2 }
-        it { expect(subject).to eq(false) }
-      end
-    end
-  end
-
   describe 'active' do
     let(:procedure) { create(:procedure) }
     subject { Procedure.active(procedure.id) }
@@ -417,14 +383,22 @@ describe Procedure do
       expect(subject.types_de_champ_private.size).to eq procedure.types_de_champ_private.size
       expect(subject.types_de_champ.map(&:drop_down_options).compact.size).to eq procedure.types_de_champ.map(&:drop_down_options).compact.size
       expect(subject.types_de_champ_private.map(&:drop_down_options).compact.size).to eq procedure.types_de_champ_private.map(&:drop_down_options).compact.size
+      expect(subject.draft_revision.types_de_champ.size).to eq(procedure.draft_revision.types_de_champ.size)
+      expect(subject.draft_revision.types_de_champ_private.size).to eq(procedure.draft_revision.types_de_champ_private.size)
 
       procedure.types_de_champ.zip(subject.types_de_champ).each do |ptc, stc|
         expect(stc).to have_same_attributes_as(ptc)
       end
+      procedure.types_de_champ.zip(procedure.draft_revision.types_de_champ).each do |ptc, rtc|
+        expect(ptc).to eq(rtc)
+      end
 
       subject.types_de_champ_private.zip(procedure.types_de_champ_private).each do |stc, ptc|
         expect(stc).to have_same_attributes_as(ptc)
       end
+      procedure.types_de_champ_private.zip(procedure.draft_revision.types_de_champ_private).each do |ptc, rtc|
+        expect(ptc).to eq(rtc)
+      end
 
       expect(subject.attestation_template.title).to eq(procedure.attestation_template.title)
 
@@ -432,7 +406,7 @@ describe Procedure do
 
       cloned_procedure = subject
       cloned_procedure.parent_procedure_id = nil
-      expect(cloned_procedure).to have_same_attributes_as(procedure, except: ["path"])
+      expect(cloned_procedure).to have_same_attributes_as(procedure, except: ["path", "draft_revision_id"])
     end
 
     context 'when the procedure is cloned from the library' do
@@ -598,6 +572,13 @@ describe Procedure do
         expect(Procedure.find_by(path: "example-path")).to eq(procedure)
         expect(Procedure.find_by(path: "example-path").administrateurs).to eq(procedure.administrateurs)
       end
+
+      it 'creates a new draft revision' do
+        expect(procedure.published_revision).not_to be_nil
+        expect(procedure.draft_revision).not_to be_nil
+        expect(procedure.revisions.count).to eq(2)
+        expect(procedure.revisions).to eq([procedure.published_revision, procedure.draft_revision])
+      end
     end
 
     context 'when publishing over a previous canonical procedure' do
@@ -648,6 +629,13 @@ describe Procedure do
       it 'unpublishes the canonical procedure' do
         expect(canonical_procedure.unpublished_at).to eq(now)
       end
+
+      it 'creates a new draft revision' do
+        expect(procedure.published_revision).not_to be_nil
+        expect(procedure.draft_revision).not_to be_nil
+        expect(procedure.revisions.count).to eq(2)
+        expect(procedure.revisions).to eq([procedure.published_revision, procedure.draft_revision])
+      end
     end
 
     context 'when publishing over a previous procedure with canonical procedure' do
@@ -694,6 +682,13 @@ describe Procedure do
       expect(procedure.published_at).not_to be_nil
       expect(procedure.unpublished_at).to eq(now)
     }
+
+    it 'sets published revision' do
+      expect(procedure.published_revision).not_to be_nil
+      expect(procedure.draft_revision).not_to be_nil
+      expect(procedure.revisions.count).to eq(2)
+      expect(procedure.revisions).to eq([procedure.published_revision, procedure.draft_revision])
+    end
   end
 
   describe "#brouillon?" do
@@ -768,6 +763,13 @@ describe Procedure do
 
     it { expect(procedure.close?).to be_truthy }
     it { expect(procedure.closed_at).to eq(now) }
+
+    it 'sets published revision' do
+      expect(procedure.published_revision).not_to be_nil
+      expect(procedure.draft_revision).not_to be_nil
+      expect(procedure.revisions.count).to eq(2)
+      expect(procedure.revisions).to eq([procedure.published_revision, procedure.draft_revision])
+    end
   end
 
   describe 'path_customized?' do
@@ -1015,82 +1017,6 @@ describe Procedure do
     end
   end
 
-  describe '#move_type_de_champ' do
-    let(:procedure) { create(:procedure) }
-
-    context 'type_de_champ' do
-      let(:type_de_champ) { create(:type_de_champ_text, order_place: 0, procedure: procedure) }
-      let!(:type_de_champ1) { create(:type_de_champ_text, order_place: 1, procedure: procedure) }
-      let!(:type_de_champ2) { create(:type_de_champ_text, order_place: 2, procedure: procedure) }
-
-      it 'move down' do
-        procedure.move_type_de_champ(type_de_champ, 2)
-
-        type_de_champ.reload
-        procedure.reload
-
-        expect(procedure.types_de_champ.index(type_de_champ)).to eq(2)
-        expect(type_de_champ.order_place).to eq(2)
-      end
-
-      context 'repetition' do
-        let!(:type_de_champ_repetition) do
-          create(:type_de_champ_repetition, types_de_champ: [
-            type_de_champ,
-            type_de_champ1,
-            type_de_champ2
-          ], procedure: procedure)
-        end
-
-        it 'move down' do
-          procedure.move_type_de_champ(type_de_champ, 2)
-
-          type_de_champ.reload
-          procedure.reload
-
-          expect(type_de_champ.parent.types_de_champ.index(type_de_champ)).to eq(2)
-          expect(type_de_champ.order_place).to eq(2)
-        end
-
-        context 'private' do
-          let!(:type_de_champ_repetition) do
-            create(:type_de_champ_repetition, types_de_champ: [
-              type_de_champ,
-              type_de_champ1,
-              type_de_champ2
-            ], private: true, procedure: procedure)
-          end
-
-          it 'move down' do
-            procedure.move_type_de_champ(type_de_champ, 2)
-
-            type_de_champ.reload
-            procedure.reload
-
-            expect(type_de_champ.parent.types_de_champ.index(type_de_champ)).to eq(2)
-            expect(type_de_champ.order_place).to eq(2)
-          end
-        end
-      end
-    end
-
-    context 'private' do
-      let(:type_de_champ) { create(:type_de_champ_text, order_place: 0, private: true, procedure: procedure) }
-      let!(:type_de_champ1) { create(:type_de_champ_text, order_place: 1, private: true, procedure: procedure) }
-      let!(:type_de_champ2) { create(:type_de_champ_text, order_place: 2, private: true, procedure: procedure) }
-
-      it 'move down' do
-        procedure.move_type_de_champ(type_de_champ, 2)
-
-        type_de_champ.reload
-        procedure.reload
-
-        expect(procedure.types_de_champ_private.index(type_de_champ)).to eq(2)
-        expect(type_de_champ.order_place).to eq(2)
-      end
-    end
-  end
-
   describe '.ensure_a_groupe_instructeur_exists' do
     let!(:procedure) { create(:procedure) }
 

spec/spec_helper.rb

@@ -45,10 +45,19 @@ Capybara.register_driver :headless_chrome do |app|
     chromeOptions: { args: ['disable-dev-shm-usage', 'disable-software-rasterizer', 'mute-audio', 'window-size=1440,900'] }
   )
 
-  Capybara::Selenium::Driver.new app,
+  download_path = Capybara.save_path
+  # Chromedriver 77 requires setting this for headless mode on linux
+  # Different versions of Chrome/selenium-webdriver require setting differently - just set them all
+  options.add_preference('download.default_directory', download_path)
+  options.add_preference(:download, default_directory: download_path)
+
+  Capybara::Selenium::Driver.new(app,
     browser: :chrome,
     desired_capabilities: capabilities,
-    options: options
+    options: options).tap do |driver|
+    # Set download dir for Chrome < 77
+    driver.browser.download_path = download_path
+  end
 end
 
 # FIXME: remove this line when https://github.com/rspec/rspec-rails/issues/1897 has been fixed

spec/support/download_helpers.rb

@@ -0,0 +1,36 @@
+module DownloadHelpers
+  TIMEOUT = 2
+
+  extend self
+
+  def downloads
+    Dir[Capybara.save_path.join("*.zip")]
+  end
+
+  def download
+    downloads.first
+  end
+
+  def download_content
+    wait_for_download
+    File.read(download)
+  end
+
+  def wait_for_download
+    Timeout.timeout(TIMEOUT) do
+      sleep 0.1 until downloaded?
+    end
+  end
+
+  def downloaded?
+    !downloading? && downloads.any?
+  end
+
+  def downloading?
+    downloads.grep(/\.part$/).any?
+  end
+
+  def clear_downloads
+    FileUtils.rm_f(downloads)
+  end
+end