Merge pull request #4169 from betagouv/dev

2019-08-01-01

Gemfile

@@ -1,7 +1,7 @@
 source 'https://rubygems.org'
 
 gem 'aasm'
-gem 'actiontext', github: 'kobaltz/actiontext', branch: 'archive', require: 'action_text'
+gem 'actiontext', git: 'https://github.com/kobaltz/actiontext.git', branch: 'archive', require: 'action_text' # Port of ActionText to Rails 5
 gem 'active_link_to' # Automatically set a class on active links
 gem 'active_model_serializers'
 gem 'activestorage-openstack', git: 'https://github.com/fredZen/activestorage-openstack.git', branch: 'frederic/fix_upload_signature'

Gemfile.lock

@@ -1,12 +1,3 @@
-GIT
-  remote: git://github.com/kobaltz/actiontext.git
-  revision: ef59c4ba99d1b7614dd47f5a294eef553224db88
-  branch: archive
-  specs:
-    actiontext (0.1.0)
-      nokogiri
-      rails (>= 5.2.0)
-
 GIT
   remote: https://github.com/fredZen/activestorage-openstack.git
   revision: c71d5107a51701eab9d9267dd0000e6c1cf3e39a
@@ -18,6 +9,15 @@ GIT
       mime-types
       rails (~> 5.2.0)
 
+GIT
+  remote: https://github.com/kobaltz/actiontext.git
+  revision: ef59c4ba99d1b7614dd47f5a294eef553224db88
+  branch: archive
+  specs:
+    actiontext (0.1.0)
+      nokogiri
+      rails (>= 5.2.0)
+
 GIT
   remote: https://github.com/mina-deploy/mina.git
   revision: 0dd5fdb8bb82a180d35e1fc033de2fac48257e30

app/controllers/application_controller.rb

@@ -45,13 +45,11 @@ class ApplicationController < ActionController::Base
   helper_method :logged_in?
 
   def pundit_user
-    if administrateur_signed_in?
-      current_administrateur
-    elsif gestionnaire_signed_in?
-      current_gestionnaire
-    else
-      current_user
-    end
+    {
+      administrateur: current_administrateur,
+      gestionnaire: current_gestionnaire,
+      user: current_user
+    }.compact
   end
 
   protected

app/models/gestionnaire.rb

@@ -225,10 +225,6 @@ class Gestionnaire < ApplicationRecord
     end
   end
 
-  def user
-    User.find_by(email: email)
-  end
-
   private
 
   def annotations_hash(demande, annotations_privees, avis, messagerie)

app/policies/application_policy.rb

@@ -35,10 +35,12 @@ class ApplicationPolicy
   end
 
   class Scope
-    attr_reader :user, :scope
+    attr_reader :user, :gestionnaire, :administrateur, :scope
 
-    def initialize(user, scope)
-      @user = user
+    def initialize(account, scope)
+      @user = account[:user]
+      @gestionnaire = account[:gestionnaire]
+      @administrateur = account[:administrateur]
       @scope = scope
     end
 

app/policies/champ_policy.rb

@@ -1,21 +1,10 @@
 class ChampPolicy < ApplicationPolicy
   class Scope < Scope
     def resolve
-      if user.is_a?(User)
+      if user.present?
         scope
           .joins(:dossier)
           .where({ dossiers: { user_id: user.id } })
-      elsif user.is_a?(Gestionnaire)
-        scope_with_join = scope.joins(dossier: :follows)
-        scope_with_left_join = scope.left_joins(dossier: :follows)
-
-        if user.user
-          scope_with_left_join
-            .where({ dossiers: { user_id: user.user.id } })
-            .or(scope_with_left_join.where(dossiers: { follows: { gestionnaire_id: user.id } }))
-        else
-          scope_with_join.where(dossiers: { follows: { gestionnaire_id: user.id } })
-        end
       else
         scope.none
       end

app/policies/type_de_champ_policy.rb

@@ -1,10 +1,10 @@
 class TypeDeChampPolicy < ApplicationPolicy
   class Scope < Scope
     def resolve
-      if user.is_a?(Administrateur)
+      if administrateur.present?
         scope
           .joins(procedure: [:administrateurs])
-          .where({ administrateurs: { id: user.id } })
+          .where({ administrateurs: { id: administrateur.id } })
       else
         scope.none
       end

app/serializers/dossier_serializer.rb

@@ -58,7 +58,7 @@ class DossierSerializer < ActiveModel::Serializer
   end
 
   def attestation
-    object.attestation.pdf_url
+    object.attestation&.pdf_url
   end
 
   def justificatif_motivation

db/migrate/20190730153555_drop_deprecated_piece_justificative_tables.rb

@@ -0,0 +1,40 @@
+class DropDeprecatedPieceJustificativeTables < ActiveRecord::Migration[5.2]
+  def change
+    assert_empty_table!(:types_de_piece_justificative)
+    assert_empty_table!(:pieces_justificatives)
+
+    drop_table :types_de_piece_justificative do |t|
+      t.string "libelle"
+      t.string "description"
+      t.boolean "api_entreprise", default: false
+      t.datetime "created_at", null: false
+      t.datetime "updated_at", null: false
+      t.integer "procedure_id"
+      t.integer "order_place"
+      t.string "lien_demarche"
+      t.boolean "mandatory", default: false
+      t.index ["procedure_id"], name: "index_types_de_piece_justificative_on_procedure_id"
+    end
+
+    drop_table :pieces_justificatives do |t|
+      t.string "content"
+      t.integer "dossier_id"
+      t.integer "type_de_piece_justificative_id"
+      t.datetime "created_at"
+      t.integer "user_id"
+      t.string "original_filename"
+      t.string "content_secure_token"
+      t.datetime "updated_at"
+      t.index ["dossier_id"], name: "index_pieces_justificatives_on_dossier_id"
+      t.index ["type_de_piece_justificative_id"], name: "index_pieces_justificatives_on_type_de_piece_justificative_id"
+    end
+  end
+
+  def assert_empty_table!(table)
+    results = ActiveRecord::Base.connection.exec_query("SELECT COUNT(*) FROM #{table}")
+    records_count = results.first['count']
+    if records_count > 0
+      raise "Abord dropping `#{table}` table: it still contains #{records_count} records."
+    end
+  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: 2019_07_17_151228) do
+ActiveRecord::Schema.define(version: 2019_07_30_153555) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -456,19 +456,6 @@ ActiveRecord::Schema.define(version: 2019_07_17_151228) do
     t.index ["procedure_id"], name: "index_module_api_cartos_on_procedure_id", unique: true
   end
 
-  create_table "pieces_justificatives", id: :serial, force: :cascade do |t|
-    t.string "content"
-    t.integer "dossier_id"
-    t.integer "type_de_piece_justificative_id"
-    t.datetime "created_at"
-    t.integer "user_id"
-    t.string "original_filename"
-    t.string "content_secure_token"
-    t.datetime "updated_at"
-    t.index ["dossier_id"], name: "index_pieces_justificatives_on_dossier_id"
-    t.index ["type_de_piece_justificative_id"], name: "index_pieces_justificatives_on_type_de_piece_justificative_id"
-  end
-
   create_table "procedure_presentations", id: :serial, force: :cascade do |t|
     t.integer "assign_to_id"
     t.jsonb "sort", default: {"order"=>"desc", "table"=>"notifications", "column"=>"notifications"}, null: false
@@ -587,19 +574,6 @@ ActiveRecord::Schema.define(version: 2019_07_17_151228) do
     t.index ["stable_id"], name: "index_types_de_champ_on_stable_id"
   end
 
-  create_table "types_de_piece_justificative", id: :serial, force: :cascade do |t|
-    t.string "libelle"
-    t.string "description"
-    t.boolean "api_entreprise", default: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-    t.integer "procedure_id"
-    t.integer "order_place"
-    t.string "lien_demarche"
-    t.boolean "mandatory", default: false
-    t.index ["procedure_id"], name: "index_types_de_piece_justificative_on_procedure_id"
-  end
-
   create_table "users", id: :serial, force: :cascade do |t|
     t.string "email", default: "", null: false
     t.string "encrypted_password", default: "", null: false

lib/tasks/2019_05_29_migrate_commentaire_pj.rake

@@ -14,12 +14,13 @@ namespace :'2019_05_29_migrate_commentaire_pj' do
     progress = ProgressReport.new(commentaires.count)
     commentaires.find_each do |commentaire|
       if commentaire.file.present?
+        dossier = Dossier.unscope(where: :hidden_at).find(commentaire.dossier_id)
         uri = URI.parse(URI.escape(commentaire.file_url))
         response = Typhoeus.get(uri)
         if response.success?
           filename = commentaire.file.filename || commentaire.file_identifier
           updated_at = commentaire.updated_at
-          dossier_updated_at = commentaire.dossier.updated_at
+          dossier_updated_at = dossier.updated_at
           commentaire.piece_jointe.attach(
             io: StringIO.new(response.body),
             filename: filename,
@@ -27,7 +28,7 @@ namespace :'2019_05_29_migrate_commentaire_pj' do
             metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE }
           )
           commentaire.update_column(:updated_at, updated_at)
-          commentaire.dossier.update_column(:updated_at, dossier_updated_at)
+          dossier.update_column(:updated_at, dossier_updated_at)
         end
       end
       progress.inc

spec/controllers/api/v1/dossiers_controller_spec.rb

@@ -151,10 +151,25 @@ describe API::V1::DossiersController do
         it { expect(subject.code).to eq('404') }
       end
 
+      context 'when dossier (with attestation) exists and belongs to procedure' do
+        let(:procedure_id) { procedure.id }
+        let(:dossier_id) { dossier.id }
+        let!(:dossier) { create(:dossier, :with_entreprise, :with_attestation, :accepte, procedure: procedure, motivation: "Motivation") }
+        let(:body) { JSON.parse(retour.body, symbolize_names: true) }
+        subject { body[:dossier] }
+
+        it {
+          expect(retour.code).to eq('200')
+          expect(subject[:id]).to eq(dossier.id)
+          expect(subject[:state]).to eq('closed')
+          expect(subject[:attestation]).to_not be_nil
+        }
+      end
+
       context 'when dossier exists and belongs to procedure' do
         let(:procedure_id) { procedure.id }
         let(:date_creation) { Time.zone.local(2008, 9, 1, 10, 5, 0) }
-        let!(:dossier) { Timecop.freeze(date_creation) { create(:dossier, :with_entreprise, :with_attestation, :accepte, procedure: procedure, motivation: "Motivation") } }
+        let!(:dossier) { Timecop.freeze(date_creation) { create(:dossier, :with_entreprise, :accepte, procedure: procedure, motivation: "Motivation") } }
         let(:dossier_id) { dossier.id }
         let(:body) { JSON.parse(retour.body, symbolize_names: true) }
         let(:field_list) { [:id, :created_at, :updated_at, :archived, :individual, :entreprise, :etablissement, :cerfa, :types_de_piece_justificative, :pieces_justificatives, :champs, :champs_private, :commentaires, :state, :simplified_state, :initiated_at, :processed_at, :received_at, :motivation, :email, :instructeurs, :attestation, :avis] }

spec/factories/dossier.rb

@@ -39,6 +39,10 @@ FactoryBot.define do
       archived { false }
     end
 
+    trait :hidden do
+      hidden_at { Time.zone.now }
+    end
+
     trait :with_dossier_link do
       after(:create) do |dossier, _evaluator|
         linked_dossier = create(:dossier)

spec/lib/tasks/2019_05_29_migrate_commentaire_pj_spec.rb

@@ -1,14 +1,16 @@
 describe '2019_05_29_migrate_commentaire_pj.rake' do
   let(:rake_task) { Rake::Task['2019_05_29_migrate_commentaire_pj:run'] }
 
-  let!(:commentaires) do
-    create(:commentaire)
-    create(:commentaire, :with_file)
-    create(:commentaire, :with_file)
+  let(:commentaires) do
+    [
+      create(:commentaire),
+      create(:commentaire, :with_file),
+      create(:commentaire, :with_file)
+    ]
   end
 
   before do
-    Commentaire.all.each do |commentaire|
+    commentaires.each do |commentaire|
       if commentaire.file.present?
         stub_request(:get, commentaire.file_url)
           .to_return(status: 200, body: File.read(commentaire.file.path))
@@ -39,4 +41,22 @@ describe '2019_05_29_migrate_commentaire_pj.rake' do
     expect(Commentaire.where(file: nil).count).to eq(1)
     expect(Commentaire.all.map(&:piece_jointe).map(&:attached?)).to eq([false, true, false])
   end
+
+  context 'when a commentaire’s dossier is hidden' do
+    let(:hidden_dossier) { create(:dossier, :en_construction, :hidden) }
+    let(:commentaire) { create(:commentaire, :with_file, dossier: hidden_dossier) }
+    let(:commentaires) { [commentaire] }
+
+    it 'should migrate the pj' do
+      comment_updated_at = commentaire.reload.updated_at
+      dossier_updated_at = hidden_dossier.reload.updated_at
+
+      rake_task.invoke
+      commentaires.each(&:reload)
+
+      expect(commentaire.piece_jointe.attached?).to be true
+      expect(commentaire.updated_at).to eq(comment_updated_at)
+      expect(hidden_dossier.updated_at).to eq(dossier_updated_at)
+    end
+  end
 end

spec/policies/champ_policy_spec.rb

@@ -5,12 +5,13 @@ describe ChampPolicy do
   let(:dossier) { create(:dossier, user: user) }
   let!(:champ) { create(:champ_text, dossier: dossier) }
 
-  let(:pundit_user) { user }
-  subject { Pundit.policy_scope(pundit_user, Champ) }
+  let(:account) { { user: user } }
+
+  subject { Pundit.policy_scope(account, Champ) }
 
   context 'when the user has only user rights' do
     context 'cannot access champs for other dossiers' do
-      let(:pundit_user) { create(:user) }
+      let(:account) { { user: create(:user) } }
 
       it { expect(subject.find_by(id: champ.id)).to eq(nil) }
     end
@@ -21,36 +22,4 @@ describe ChampPolicy do
       }
     end
   end
-
-  context 'when the user has only gestionnaire rights' do
-    context 'can access champs for dossiers it follows' do
-      let(:dossier) { create(:dossier, :followed) }
-      let(:pundit_user) { dossier.followers_gestionnaires.first }
-
-      it { expect(subject.find(champ.id)).to eq(champ) }
-    end
-  end
-
-  context 'when the user has user and gestionnaire rights' do
-    let(:pundit_user) { dossier.followers_gestionnaires.first }
-    let(:dossier) { create(:dossier, :followed) }
-
-    let(:user) { create(:user, email: pundit_user.email) }
-    let(:dossier2) { create(:dossier, user: user) }
-    let!(:champ_2) { create(:champ_text, dossier: dossier2) }
-
-    context 'can access champs for dossiers it follows' do
-      it do
-        expect(pundit_user.user).to eq(user)
-        expect(subject.find(champ.id)).to eq(champ)
-      end
-    end
-
-    context 'can access champs for its own dossiers' do
-      it do
-        expect(pundit_user.user).to eq(user)
-        expect(subject.find(champ_2.id)).to eq(champ_2)
-      end
-    end
-  end
 end

spec/policies/type_de_champ_policy_spec.rb

@@ -4,8 +4,17 @@ describe TypeDeChampPolicy do
   let(:procedure) { create(:procedure) }
   let!(:type_de_champ) { create(:type_de_champ_text, procedure: procedure) }
 
-  let(:pundit_user) { create(:user) }
-  subject { Pundit.policy_scope(pundit_user, TypeDeChamp) }
+  let(:user) { create(:user) }
+  let(:administrateur) { nil }
+
+  let(:account) do
+    {
+      user: user,
+      administrateur: administrateur
+    }.compact
+  end
+
+  subject { Pundit.policy_scope(account, TypeDeChamp) }
 
   context 'when the user has only user rights' do
     it 'can not access' do
@@ -14,7 +23,7 @@ describe TypeDeChampPolicy do
   end
 
   context 'when the user has administrateur rights' do
-    let(:pundit_user) { procedure.administrateurs.first }
+    let(:administrateur) { procedure.administrateurs.first }
 
     it 'can access' do
       expect(subject.find(type_de_champ.id)).to eq(type_de_champ)