Merge pull request #1962 from betagouv/frederic/fix-1946/edit-retention-time

Frederic/fix 1946/edit retention time

app/controllers/admin/procedures_controller.rb

@@ -237,7 +237,7 @@ class Admin::ProceduresController < AdminController
     if @procedure&.locked?
       params.require(:procedure).permit(*editable_params)
     else
-      params.require(:procedure).permit(*editable_params, :lien_demarche, :cerfa_flag, :for_individual, :individual_with_siret, :ask_birthday, module_api_carto_attributes: [:id, :use_api_carto, :quartiers_prioritaires, :cadastre]).merge(administrateur_id: current_administrateur.id)
+      params.require(:procedure).permit(*editable_params, :duree_conservation_dossiers_dans_ds, :duree_conservation_dossiers_hors_ds, :lien_demarche, :cerfa_flag, :for_individual, :individual_with_siret, :ask_birthday, module_api_carto_attributes: [:id, :use_api_carto, :quartiers_prioritaires, :cadastre]).merge(administrateur_id: current_administrateur.id)
     end
   end
 

app/models/procedure.rb

@@ -1,4 +1,6 @@
 class Procedure < ApplicationRecord
+  MAX_DUREE_CONSERVATION = 36
+
   has_many :types_de_piece_justificative, -> { order "order_place ASC" }, dependent: :destroy
   has_many :types_de_champ, -> { public_only }, dependent: :destroy
   has_many :types_de_champ_private, -> { private_only }, class_name: 'TypeDeChamp', dependent: :destroy
@@ -47,8 +49,14 @@ class Procedure < ApplicationRecord
   validates :libelle, presence: true, allow_blank: false, allow_nil: false
   validates :description, presence: true, allow_blank: false, allow_nil: false
   validate :check_juridique
+  # FIXME: remove duree_conservation_required flag once all procedures are converted to the new style
+  validates :duree_conservation_dossiers_dans_ds, allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: MAX_DUREE_CONSERVATION }, if: :durees_conservation_required
+  validates :duree_conservation_dossiers_hors_ds, allow_nil: false, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, if: :durees_conservation_required
+  validates :duree_conservation_dossiers_dans_ds, allow_nil: true, numericality: { only_integer: true, greater_than_or_equal_to: 1, less_than_or_equal_to: MAX_DUREE_CONSERVATION }, unless: :durees_conservation_required
+  validates :duree_conservation_dossiers_hors_ds, allow_nil: true, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, unless: :durees_conservation_required
 
   before_save :update_juridique_required
+  before_save :update_durees_conservation_required
 
   include AASM
 
@@ -376,4 +384,9 @@ class Procedure < ApplicationRecord
       'column' => column
     }
   end
+
+  def update_durees_conservation_required
+    self.durees_conservation_required ||= duree_conservation_dossiers_hors_ds.present? && duree_conservation_dossiers_dans_ds.present?
+    true
+  end
 end

app/views/admin/procedures/_informations.html.haml

@@ -10,6 +10,15 @@
   %h4 Description*
   = f.text_area :description, rows: '6', placeholder: 'Description du projet', class: 'form-control'
 
+- if !@procedure.locked?
+  .form-group
+    %h4 Conservation des données
+    = f.label :duree_conservation_dossiers_dans_ds, "Sur demarches-simplifiees.fr* (durée en mois après le début de l’instruction)"
+    = f.number_field :duree_conservation_dossiers_dans_ds, class: 'form-control', placeholder: '6', required: true
+
+    = f.label :duree_conservation_dossiers_hors_ds, "Hors demarches-simplifiees.fr* (durée en mois)"
+    = f.number_field :duree_conservation_dossiers_hors_ds, class: 'form-control', placeholder: '6', required: true
+
 .form-group
   %h4 Lien site internet
   = f.text_field :lien_site_web, class: 'form-control', placeholder: 'https://www.exemple.fr/'

config/locales/fr.yml

@@ -88,6 +88,7 @@ fr:
     errors:
       messages:
         blank: "doit être rempli"
+        not_a_number: 'doit être un nombre'
       models:
         attestation_template:
           attributes:

config/locales/models/procedure/fr.yml

@@ -3,3 +3,5 @@ fr:
     attributes:
       procedure:
         organisation: Organisme
+        duree_conservation_dossiers_dans_ds: Durée de conservation des dossiers sur demarches-simplifiees.fr
+        duree_conservation_dossiers_hors_ds: Durée de conservation des dossiers hors demarches-simplifiees.fr

db/migrate/20180524201142_add_durees_conservation_required_to_procedure.rb

@@ -0,0 +1,5 @@
+class AddDureesConservationRequiredToProcedure < ActiveRecord::Migration[5.2]
+  def change
+    add_column :procedures, :durees_conservation_required, :boolean, default: true
+  end
+end

db/schema.rb

@@ -483,6 +483,7 @@ ActiveRecord::Schema.define(version: 2018_06_01_084546) do
     t.integer "duree_conservation_dossiers_hors_ds"
     t.string "cadre_juridique"
     t.boolean "juridique_required", default: true
+    t.boolean "durees_conservation_required", default: true
     t.index ["hidden_at"], name: "index_procedures_on_hidden_at"
     t.index ["parent_procedure_id"], name: "index_procedures_on_parent_procedure_id"
     t.index ["service_id"], name: "index_procedures_on_service_id"

lib/tasks/2018_05_24_optional_durees_conservation_for_legacy_procedures.rake

@@ -0,0 +1,5 @@
+namespace :'2018_05_24_optional_durees_conservation_for_legacy_procedures' do
+  task set: :environment do
+    Procedure.all.update_all(durees_conservation_required: false)
+  end
+end

spec/controllers/admin/procedures_controller_spec.rb

@@ -16,6 +16,8 @@ describe Admin::ProceduresController, type: :controller do
   let(:cadastre) { '0' }
   let(:cerfa_flag) { true }
   let(:cadre_juridique) { 'cadre juridique' }
+  let(:duree_conservation_dossiers_dans_ds) { 3 }
+  let(:duree_conservation_dossiers_hors_ds) { 6 }
 
   let(:procedure_params) {
     {
@@ -26,6 +28,8 @@ describe Admin::ProceduresController, type: :controller do
       lien_demarche: lien_demarche,
       cerfa_flag: cerfa_flag,
       cadre_juridique: cadre_juridique,
+      duree_conservation_dossiers_dans_ds: duree_conservation_dossiers_dans_ds,
+      duree_conservation_dossiers_hors_ds: duree_conservation_dossiers_hors_ds,
       module_api_carto_attributes: {
         use_api_carto: use_api_carto,
         quartiers_prioritaires: quartiers_prioritaires,
@@ -196,6 +200,8 @@ describe Admin::ProceduresController, type: :controller do
           it { expect(subject.direction).to eq(direction) }
           it { expect(subject.lien_demarche).to eq(lien_demarche) }
           it { expect(subject.administrateur_id).to eq(admin.id) }
+          it { expect(subject.duree_conservation_dossiers_dans_ds).to eq(duree_conservation_dossiers_dans_ds) }
+          it { expect(subject.duree_conservation_dossiers_hors_ds).to eq(duree_conservation_dossiers_hors_ds) }
         end
 
         describe 'procedure module api carto attributs in database' do
@@ -267,6 +273,8 @@ describe Admin::ProceduresController, type: :controller do
         let(:lien_demarche) { 'http://plip.com' }
         let(:use_api_carto) { '1' }
         let(:cadastre) { '1' }
+        let(:duree_conservation_dossiers_dans_ds) { 7 }
+        let(:duree_conservation_dossiers_hors_ds) { 5 }
 
         describe 'procedure attributs in database' do
           subject { procedure }
@@ -276,6 +284,8 @@ describe Admin::ProceduresController, type: :controller do
           it { expect(subject.organisation).to eq(organisation) }
           it { expect(subject.direction).to eq(direction) }
           it { expect(subject.lien_demarche).to eq(lien_demarche) }
+          it { expect(subject.duree_conservation_dossiers_dans_ds).to eq(duree_conservation_dossiers_dans_ds) }
+          it { expect(subject.duree_conservation_dossiers_hors_ds).to eq(duree_conservation_dossiers_hors_ds) }
         end
 
         describe 'procedure module api carto attributs in database' do

spec/factories/procedure.rb

@@ -10,6 +10,8 @@ FactoryBot.define do
     published_at nil
     cerfa_flag false
     administrateur { create(:administrateur) }
+    duree_conservation_dossiers_dans_ds 3
+    duree_conservation_dossiers_hors_ds 6
 
     after(:build) do |procedure, _evaluator|
       if procedure.module_api_carto.nil?

spec/features/admin/procedure_cloning_spec.rb

@@ -1,6 +1,9 @@
 require 'spec_helper'
+require 'features/admin/procedure_spec_helper'
 
 feature 'As an administrateur I wanna clone a procedure', js: true do
+  include ProcedureSpecHelper
+
   let(:administrateur) { create(:administrateur) }
 
   before do
@@ -12,9 +15,7 @@ feature 'As an administrateur I wanna clone a procedure', js: true do
     before 'Create procedure' do
       page.find_by_id('new-procedure').click
       page.find_by_id('from-scratch').click
-      fill_in 'procedure_libelle', with: 'libelle de la procedure'
-      page.execute_script("$('#procedure_description').val('description de la procedure')")
-      fill_in 'procedure_cadre_juridique', with: 'cadre juridique'
+      fill_in_dummy_procedure_details
       page.find_by_id('save-procedure').click
     end
 

spec/features/admin/procedure_creation_spec.rb

@@ -1,6 +1,9 @@
 require 'spec_helper'
+require 'features/admin/procedure_spec_helper'
 
 feature 'As an administrateur I wanna create a new procedure', js: true do
+  include ProcedureSpecHelper
+
   let(:administrateur) { create(:administrateur) }
 
   before do
@@ -35,11 +38,11 @@ feature 'As an administrateur I wanna create a new procedure', js: true do
     scenario 'Finding save button for new procedure, libelle, description and cadre_juridique required' do
       page.find_by_id('new-procedure').click
       page.find_by_id('from-scratch').click
+      fill_in 'procedure_duree_conservation_dossiers_dans_ds', with: '3'
+      fill_in 'procedure_duree_conservation_dossiers_hors_ds', with: '6'
       page.find_by_id('save-procedure').click
       page.find_by_id('flash_message').visible?
-      fill_in 'procedure_libelle', with: 'libelle de la procedure'
-      page.execute_script("$('#procedure_description').val('description de la procedure')")
-      fill_in 'procedure_cadre_juridique', with: 'cadre juridique'
+      fill_in_dummy_procedure_details
       page.find_by_id('save-procedure').click
       expect(page).to have_current_path(admin_procedure_types_de_champ_path(Procedure.first.id.to_s))
     end
@@ -49,9 +52,7 @@ feature 'As an administrateur I wanna create a new procedure', js: true do
     before 'Create procedure' do
       page.find_by_id('new-procedure').click
       page.find_by_id('from-scratch').click
-      fill_in 'procedure_libelle', with: 'libelle de la procedure'
-      page.execute_script("$('#procedure_description').val('description de la procedure')")
-      fill_in 'procedure_cadre_juridique', with: 'cadre juridique'
+      fill_in_dummy_procedure_details
       page.find_by_id('save-procedure').click
 
       procedure = Procedure.last

spec/features/admin/procedure_spec_helper.rb

@@ -0,0 +1,9 @@
+module ProcedureSpecHelper
+  def fill_in_dummy_procedure_details
+    fill_in 'procedure_libelle', with: 'libelle de la procedure'
+    page.execute_script("$('#procedure_description').val('description de la procedure')")
+    fill_in 'procedure_cadre_juridique', with: 'cadre juridique'
+    fill_in 'procedure_duree_conservation_dossiers_dans_ds', with: '3'
+    fill_in 'procedure_duree_conservation_dossiers_hors_ds', with: '6'
+  end
+end

spec/models/procedure_spec.rb

@@ -199,6 +199,63 @@ describe Procedure do
 
       it { expect(procedure.valid?).to eq(true) }
     end
+
+    shared_examples 'duree de conservation' do
+      context 'duree_conservation_required it true, the field gets validated' do
+        before { subject.durees_conservation_required = true }
+
+        it { is_expected.not_to allow_value(nil).for(field_name) }
+        it { is_expected.not_to allow_value('').for(field_name) }
+        it { is_expected.not_to allow_value('trois').for(field_name) }
+        it { is_expected.to allow_value(3).for(field_name) }
+      end
+
+      context 'duree_conservation_required is false, the field doesn’t get validated' do
+        before { subject.durees_conservation_required = false }
+
+        it { is_expected.to allow_value(nil).for(field_name) }
+        it { is_expected.to allow_value('').for(field_name) }
+        it { is_expected.not_to allow_value('trois').for(field_name) }
+        it { is_expected.to allow_value(3).for(field_name) }
+      end
+    end
+
+    describe 'duree de conservation dans ds' do
+      let(:field_name) { :duree_conservation_dossiers_dans_ds }
+
+      it_behaves_like 'duree de conservation'
+    end
+
+    describe 'duree de conservation hors ds' do
+      let(:field_name) { :duree_conservation_dossiers_hors_ds }
+
+      it_behaves_like 'duree de conservation'
+    end
+  end
+
+  describe '#duree_de_conservation_required' do
+    it 'automatically jumps to true once both durees de conservation have been set' do
+      p = build(
+        :procedure,
+        durees_conservation_required: false,
+        duree_conservation_dossiers_dans_ds: nil,
+        duree_conservation_dossiers_hors_ds: nil
+      )
+      p.save
+      expect(p.durees_conservation_required).to be_falsey
+
+      p.duree_conservation_dossiers_hors_ds = 3
+      p.save
+      expect(p.durees_conservation_required).to be_falsey
+
+      p.duree_conservation_dossiers_dans_ds = 6
+      p.save
+      expect(p.durees_conservation_required).to be_truthy
+
+      p.duree_conservation_dossiers_dans_ds = nil
+      p.save
+      expect(p.durees_conservation_required).to be_truthy
+    end
   end
 
   describe '#types_de_champ_ordered' do