Merge pull request #6065 from betagouv/main

2021-04-07-01

app/assets/stylesheets/common.scss

@@ -49,6 +49,11 @@ a {
   color: $blue;
 }
 
+a[target="_blank"]::after {
+  content: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAAAKCAYAAACNMs+9AAAAQElEQVR42qXKwQkAIAxDUUdxtO6/RBQkQZvSi8I/pL4BoGw/XPkh4XigPmsUgh0626AjRsgxHTkUThsG2T/sIlzdTsp52kSS1wAAAABJRU5ErkJggg==);
+  margin: 0 3px 0 5px;
+}
+
 em {
   font-style: italic;
 }

app/controllers/application_controller.rb

@@ -11,7 +11,7 @@ class ApplicationController < ActionController::Base
   before_action :set_current_roles
   before_action :set_sentry_user
   before_action :redirect_if_untrusted
-  before_action :reject, if: -> { feature_enabled?(:maintenance_mode) }
+  before_action :reject, if: -> { ENV.fetch("MAINTENANCE_MODE", 'false') == 'true' }
   before_action :configure_permitted_parameters, if: :devise_controller?
 
   before_action :staging_authenticate
@@ -309,7 +309,7 @@ class ApplicationController < ActionController::Base
   end
 
   def set_locale
-    if feature_enabled?(:localization)
+    if ENV.fetch('LOCALIZATION_ENABLED', 'false') == 'true'
       I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales)
     end
   end

app/controllers/new_administrateur/experts_procedures_controller.rb

@@ -1,8 +1,16 @@
 module NewAdministrateur
   class ExpertsProceduresController < AdministrateurController
-    before_action :retrieve_procedure, only: [:add_expert_to_procedure, :revoke_expert_from_procedure]
+    before_action :retrieve_procedure
 
-    def add_expert_to_procedure
+    def index
+      @experts_procedure = @procedure
+        .experts_procedures
+        .where(revoked_at: nil)
+        .sort_by { |expert_procedure| expert_procedure.expert.email }
+      @experts_emails = experts_procedure_emails
+    end
+
+    def create
       emails = params['emails'].presence || [].to_json
       emails = JSON.parse(emails).map(&:strip).map(&:downcase)
 
@@ -29,15 +37,32 @@ module NewAdministrateur
           value: valid_users.map(&:email).join(', '),
           procedure: @procedure.id)
       end
-      redirect_to admin_procedure_invited_expert_list_path(@procedure)
+      redirect_to admin_procedure_experts_path(@procedure)
     end
 
-    def revoke_expert_from_procedure
+    def update
-      expert_procedure = ExpertsProcedure.find_by!(procedure: @procedure, id: params[:expert_procedure][:id])
+      @procedure
+        .experts_procedures
+        .find(params[:id])
+        .update!(expert_procedure_params)
+    end
+
+    def destroy
+      expert_procedure = ExpertsProcedure.find_by!(procedure: @procedure, id: params[:id])
       expert_email = expert_procedure.expert.email
       expert_procedure.update!(revoked_at: Time.zone.now)
       flash[:notice] = "#{expert_email} a été révoqué de la démarche et ne pourra plus déposer d'avis."
-      redirect_to admin_procedure_invited_expert_list_path(@procedure)
+      redirect_to admin_procedure_experts_path(@procedure)
+    end
+
+    private
+
+    def experts_procedure_emails
+      @procedure.experts.map(&:email).sort
+    end
+
+    def expert_procedure_params
+      params.require(:experts_procedure).permit(:allow_decision_access)
     end
   end
 end

app/controllers/new_administrateur/procedures_controller.rb

@@ -1,6 +1,6 @@
 module NewAdministrateur
   class ProceduresController < AdministrateurController
-    before_action :retrieve_procedure, only: [:champs, :annotations, :edit, :monavis, :update_monavis, :jeton, :update_jeton, :publication, :publish, :transfert, :allow_expert_review, :invited_expert_list, :update_allow_decision_access]
+    before_action :retrieve_procedure, only: [:champs, :annotations, :edit, :monavis, :update_monavis, :jeton, :update_jeton, :publication, :publish, :transfert, :allow_expert_review]
     before_action :procedure_locked?, only: [:champs, :annotations]
 
     ITEMS_PER_PAGE = 25
@@ -185,24 +185,8 @@ module NewAdministrateur
       end
     end
 
-    def invited_expert_list
-      @experts_procedure = @procedure.experts_procedures.where(revoked_at: nil).sort_by { |expert_procedure| expert_procedure.expert.email }
-      @experts_emails = experts_procedure_emails
-    end
-
-    def update_allow_decision_access
-      @procedure
-        .experts_procedures
-        .find(params[:expert_procedure])
-        .update!(allow_decision_access_params)
-    end
-
     private
 
-    def experts_procedure_emails
-      @procedure.experts.map(&:email).sort
-    end
-
     def apercu_tab
       params[:tab] || 'dossier'
     end

app/jobs/virus_scanner_job.rb

@@ -11,17 +11,32 @@ class VirusScannerJob < ApplicationJob
   # If the file is not analyzed yet, retry later (to avoid clobbering metadata)
   retry_on FileNotAnalyzedYetError, wait: :exponentially_longer, attempts: 10
   # If for some reason the file appears invalid, retry for a while
-  retry_on ActiveStorage::IntegrityError, attempts: 10, wait: 5.seconds
+  retry_on(ActiveStorage::IntegrityError, attempts: 5, wait: 5.seconds) do |job, _error|
+    blob = job.arguments.first
+
+    metadata = {
+      virus_scan_result: ActiveStorage::VirusScanner::INTEGRITY_ERROR,
+      scanned_at: Time.zone.now
+    }
+
+    merge_and_update_metadata(blob, metadata)
+  end
 
   def perform(blob)
     if !blob.analyzed? then raise FileNotAnalyzedYetError end
     if blob.virus_scanner.done? then return end
 
     metadata = extract_metadata_via_virus_scanner(blob)
-    blob.update!(metadata: blob.metadata.merge(metadata))
+    VirusScannerJob.merge_and_update_metadata(blob, metadata)
   end
 
   def extract_metadata_via_virus_scanner(blob)
     ActiveStorage::VirusScanner.new(blob).metadata
   end
+
+  private
+
+  def self.merge_and_update_metadata(blob, metadata)
+    blob.update!(metadata: blob.metadata.merge(metadata))
+  end
 end

app/lib/active_storage/virus_scanner.rb

@@ -8,6 +8,7 @@ class ActiveStorage::VirusScanner
   PENDING = 'pending'
   INFECTED = 'infected'
   SAFE = 'safe'
+  INTEGRITY_ERROR = 'integrity_error'
 
   def pending?
     blob.metadata[:virus_scan_result] == PENDING
@@ -21,6 +22,10 @@ class ActiveStorage::VirusScanner
     blob.metadata[:virus_scan_result] == SAFE
   end
 
+  def corrupt?
+    blob.metadata[:virus_scan_result] == INTEGRITY_ERROR
+  end
+
   def done?
     started? && blob.metadata[:virus_scan_result] != PENDING
   end

app/lib/api_entreprise/api.rb

@@ -113,13 +113,7 @@ class APIEntreprise::API
   end
 
   def self.url(resource_name, siret_or_siren)
-    base_url = [API_ENTREPRISE_URL, resource_name, siret_or_siren].join("/")
+    [API_ENTREPRISE_URL, resource_name, siret_or_siren].join("/")
-
-    if Flipper.enabled?(:insee_api_v3)
-      base_url += "?with_insee_v3=true"
-    end
-
-    base_url
   end
 
   def self.params(siret_or_siren, procedure_id, user_id)

app/models/champ.rb

@@ -88,7 +88,7 @@ class Champ < ApplicationRecord
   end
 
   def sections
-    siblings.filter(&:header_section?)
+    siblings&.filter(&:header_section?)
   end
 
   def mandatory_and_blank?

app/models/champs/header_section_champ.rb

@@ -23,7 +23,7 @@ class Champs::HeaderSectionChamp < Champ
   end
 
   def libelle_with_section_index
-    if sections.none?(&:libelle_with_section_index?)
+    if sections&.none?(&:libelle_with_section_index?)
       "#{section_index}. #{libelle}"
     else
       libelle

app/views/layouts/_coronavirus_banner.html.haml

@@ -1,8 +0,0 @@
-#strike-banner.site-banner
-  .container
-    .site-banner-icon ⚠️
-    .site-banner-text
-      %strong
-        En raison de l’épidémie du coronavirus, les services fonctionnent en mode dégradé.
-      %br
-      Les délais de prise en charge des dossiers ou de réponses aux questions pourront être perturbés durant cette période.

app/views/layouts/_header.haml

@@ -3,14 +3,6 @@
 - dossier = controller.try(:dossier_for_help)
 - procedure = controller.try(:procedure_for_help)
 
--# only display the coronavirus to usagers (instructeurs know there are delays) when they are logged in, or on the public pages.
-- if user_signed_in?
-  - if dossier.present? && dossier.procedure.feature_enabled?(:coronavirus_banner)
-    = render partial: 'layouts/coronavirus_banner'
-- else
-  - if procedure.present? && procedure.feature_enabled?(:coronavirus_banner)
-    = render partial: 'layouts/coronavirus_banner'
-
 %header.new-header{ class: current_page?(root_path) ? nil : "new-header-with-border", role: 'banner' }
   .header-inner-content
 

app/views/layouts/application.html.haml

@@ -23,9 +23,6 @@
 
     = Gon::Base.render_data(camel_case: true, init: true, nonce:  request.content_security_policy_nonce)
 
-    - if Rails.env.development? && feature_enabled?(:xray)
-      = stylesheet_link_tag :xray
-
   %body{ id: content_for(:page_id), class: browser.platform.ios? ? 'ios' : nil }
     .page-wrapper
       = render partial: "layouts/outdated_browser_banner"
@@ -43,7 +40,4 @@
       - if content_for?(:footer)
         = content_for(:footer)
 
-      - if Rails.env.development? && feature_enabled?(:xray)
-        = javascript_include_tag :xray
-
       = yield :charts_js

app/views/new_administrateur/experts_procedures/index.html.haml

@@ -11,7 +11,7 @@
     .card
       .card-title Affecter des experts à la démarche
       = form_for :experts_procedure,
-        url: admin_procedure_add_expert_to_procedure_path(@procedure),
+        url: admin_procedure_experts_path(@procedure),
         html: { class: 'form' } do |f|
 
         .instructeur-wrapper
@@ -46,7 +46,7 @@
             - if @procedure.feature_enabled?(:admin_affect_experts_to_avis)
               %td.text-center
                 = form_for expert_procedure,
-                  url: admin_procedure_update_allow_decision_access_path(expert_procedure: expert_procedure),
+                  url: admin_procedure_expert_path(id: expert_procedure),
                   remote: true,
                   method: :put,
                   authenticity_token: true,
@@ -57,13 +57,11 @@
                     %span.toggle-switch-label.on
                     %span.toggle-switch-label.off
             %td.actions= button_to 'retirer',
-              { action: "revoke_expert_from_procedure", :controller=>"new_administrateur/experts_procedures" },
+              admin_procedure_expert_path(id: expert_procedure, procedure: @procedure),
-              { method: :put,
+              method: :delete,
-                data: { confirm: "Êtes-vous sûr de vouloir révoquer l'expert « #{expert_procedure.expert.email} » de la démarche #{expert_procedure.procedure.libelle} ? Les instructeurs ne pourront plus lui demander d'avis" },
+              data: { confirm: "Êtes-vous sûr de vouloir révoquer l'expert « #{expert_procedure.expert.email} » de la démarche #{expert_procedure.procedure.libelle} ? Les instructeurs ne pourront plus lui demander d'avis" },
-                params: { expert_procedure: { id: expert_procedure.id }},
+              class: 'button'
-                class: 'button' }
   - else
     .blank-tab
       %h2.empty-text Aucun expert invité pour le moment.
       %p.empty-text-details Les instructeurs de cette démarche n'ont pas encore fait appel aux experts.
-

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

@@ -167,7 +167,7 @@
           %p.card-admin-subtitle Liste des experts invités par les instructeurs
 
         .card-admin-action
-          = link_to "Voir", admin_procedure_invited_expert_list_path(@procedure), class: 'button'
+          = link_to "Voir", admin_procedure_experts_path(@procedure), class: 'button'
 
 
     .card-admin

app/views/root/_footer.html.haml

@@ -45,6 +45,7 @@
           %li.footer-link
             = link_to "FAQ", FAQ_URL, :class => "footer-link", :target => "_blank", rel: "noopener noreferrer"
           %li.footer-link
-            = link_to "Accessibilité", ACCESSIBILITE_URL, :class => "footer-link", :target => "_blank", rel: "noopener noreferrer"
+            = link_to ACCESSIBILITE_URL, :class => "footer-link", :target => "_blank", rel: "noopener noreferrer" do
+              Accessibilité : non conforme
           %li.footer-link
             = link_to "Disponibilité", STATUS_PAGE_URL, :class => "footer-link", :target => "_blank", rel: "noopener noreferrer"

app/views/shared/attachment/_show.html.haml

@@ -28,3 +28,9 @@
         (virus détecté, merci d’envoyer un autre fichier)
       - else
         (virus détecté, le téléchargement de ce fichier est bloqué)
+    - elsif attachment.virus_scanner.corrupt?
+      - if user_can_upload
+        (le fichier est corrompu, merci d’envoyer un autre fichier)
+      - else
+        (le fichier est corrompu, le téléchargement est bloqué)
+

app/views/users/_general_footer_row.html.haml

@@ -1,7 +1,9 @@
 %ul.footer-row.footer-bottom-line.footer-site-links
-  %li.footer-link-accessibilite>= link_to "Accessibilité", ACCESSIBILITE_URL, target: "_blank", rel: "noopener noreferrer"
+  %li.footer-link-accessibilite>
+    = link_to ACCESSIBILITE_URL, target: "_blank", rel: "noopener noreferrer" do
+      Accessibilité : non conforme
   %li.footer-link-cgu>= link_to "CGU", CGU_URL, target: "_blank", rel: "noopener noreferrer"
   %li.footer-link-mentions-legales>= link_to "Mentions légales", MENTIONS_LEGALES_URL, target: "_blank", rel: "noopener noreferrer"
-  %li.footer-link-doc>= link_to 'Documentation', DOC_URL
+  %li.footer-link-doc>= link_to 'Documentation', DOC_URL, target: "_blank", rel: "noopener noreferrer"
   %li.footer-link-contact>= contact_link "Contact technique", dossier_id: dossier&.id
-  %li.footer-link-aide>= link_to 'Aide', FAQ_URL
+  %li.footer-link-aide>= link_to 'Aide', FAQ_URL, target: "_blank", rel: "noopener noreferrer"

config/application.rb

@@ -77,7 +77,8 @@ module TPS
     end
 
     config.middleware.use Rack::Attack
-    config.middleware.use Flipper::Middleware::Memoizer, preload_all: true
+    config.middleware.use Flipper::Middleware::Memoizer,
+      preload: [:instructeur_bypass_email_login_token]
 
     config.ds_env = ENV.fetch('DS_ENV', Rails.env)
 

config/env.example.optional

@@ -67,3 +67,8 @@ DS_ENV="staging"
 # Personnalisation d'instance - fichier utilisé pour poser un filigrane sur les pièces d'identité
 # WATERMARK_FILE=""
 
+# Active le mode maintenance
+# MAINTENANCE_MODE="true"
+
+# Active la localisation
+# LOCALIZATION_ENABLED="true"

config/initializers/flipper.rb

@@ -28,18 +28,12 @@ features = [
   :administrateur_routage,
   :administrateur_web_hook,
   :carte_ign,
-  :coronavirus_banner,
   :dossier_pdf_vide,
   :expert_not_allowed_to_invite,
   :hide_instructeur_email,
-  :insee_api_v3,
   :instructeur_bypass_email_login_token,
-  :localization,
-  :maintenance_mode,
   :make_experts_notifiable,
-  :mini_profiler,
+  :procedure_routage_api
-  :procedure_routage_api,
-  :xray
 ]
 
 def database_exists?

config/routes.rb

@@ -406,10 +406,6 @@ Rails.application.routes.draw do
       put 'publish' => 'procedures#publish', as: :publish
       get 'transfert' => 'procedures#transfert', as: :transfert
       post 'transfer' => 'procedures#transfer', as: :transfer
-      get 'invited_expert_list'
-      put 'update_allow_decision_access' => 'procedures#update_allow_decision_access', as: :update_allow_decision_access
-      post 'add_expert_to_procedure' => 'experts_procedures#add_expert_to_procedure', as: :add_expert_to_procedure
-      put 'revoke_expert_from_procedure' => 'experts_procedures#revoke_expert_from_procedure', as: :revoke_expert_from_procedure
       resources :mail_templates, only: [:edit, :update]
 
       resources :groupe_instructeurs, only: [:index, :show, :create, :update, :destroy] do
@@ -427,6 +423,8 @@ Rails.application.routes.draw do
 
       resources :administrateurs, controller: 'procedure_administrateurs', only: [:index, :create, :destroy]
 
+      resources :experts, controller: 'experts_procedures', only: [:index, :create, :update, :destroy]
+
       resources :types_de_champ, only: [:create, :update, :destroy] do
         member do
           patch :move

spec/controllers/new_administrateur/experts_procedures_controller_spec.rb

@@ -1,17 +1,32 @@
 describe NewAdministrateur::ExpertsProceduresController, type: :controller do
   let(:admin) { create(:administrateur) }
+  let(:procedure) { create :procedure, administrateur: admin }
 
   before do
     sign_in(admin.user)
   end
-  describe '#add_expert_to_procedure' do
+
-    let(:procedure) { create :procedure, administrateur: admin }
+  describe '#index' do
+    subject do
+      get :index, params: { procedure_id: procedure.id }
+    end
+
+    before do
+      subject
+    end
+
+    it { expect(response.status).to eq 200 }
+  end
+
+  describe '#create' do
     let(:expert) { create(:expert) }
     let(:expert2) { create(:expert) }
 
     subject do
-      post :add_expert_to_procedure,
+      post :create, params: {
-      params: { procedure_id: procedure.id, emails: "[\"#{expert.email}\",\"#{expert2.email}\"]" }
+        procedure_id: procedure.id,
+        emails: "[\"#{expert.email}\",\"#{expert2.email}\"]"
+      }
     end
 
     before do
@@ -22,17 +37,41 @@ describe NewAdministrateur::ExpertsProceduresController, type: :controller do
       it { expect(procedure.experts.include?(expert)).to be_truthy }
       it { expect(procedure.experts.include?(expert2)).to be_truthy }
       it { expect(flash.notice).to be_present }
-      it { expect(response).to redirect_to(admin_procedure_invited_expert_list_path(procedure)) }
+      it { expect(response).to redirect_to(admin_procedure_experts_path(procedure)) }
     end
   end
 
-  describe '#revoke_expert_from_procedure' do
+  describe '#update' do
-    let(:procedure) { create :procedure, administrateur: admin }
+    let(:expert) { create(:expert) }
+    let(:expert_procedure) { create(:experts_procedure, procedure: procedure, expert: expert) }
+
+    subject do
+      put :update, params: {
+        id: expert_procedure.id,
+        procedure_id: procedure.id,
+        experts_procedure: {
+          allow_decision_access: true
+        }
+      }, format: :js
+    end
+
+    before do
+      subject
+    end
+
+    it 'updates the record' do
+      expect(expert_procedure.allow_decision_access).to be false
+      subject
+      expect(expert_procedure.reload.allow_decision_access).to be true
+    end
+  end
+
+  describe '#delete' do
     let(:expert) { create(:expert) }
     let(:expert_procedure) { ExpertsProcedure.create(expert: expert, procedure: procedure) }
 
     subject do
-      put :revoke_expert_from_procedure, params: { procedure_id: procedure.id, expert_procedure: { id: expert_procedure.id } }
+      delete :destroy, params: { procedure_id: procedure.id, id: expert_procedure.id }
     end
 
     before do
@@ -43,7 +82,7 @@ describe NewAdministrateur::ExpertsProceduresController, type: :controller do
     context 'of multiple experts' do
       it { expect(expert_procedure.revoked_at).to be_present }
       it { expect(flash.notice).to be_present }
-      it { expect(response).to redirect_to(admin_procedure_invited_expert_list_path(procedure)) }
+      it { expect(response).to redirect_to(admin_procedure_experts_path(procedure)) }
     end
   end
 end

spec/controllers/new_administrateur/procedures_controller_spec.rb

@@ -532,34 +532,4 @@ describe NewAdministrateur::ProceduresController, type: :controller do
       it { expect(procedure.allow_expert_review).to be_truthy }
     end
   end
-
-  describe 'PUT #update_allow_decision_access' do
-    let!(:procedure) { create :procedure, :with_service, administrateur: admin }
-    let(:expert) { create(:expert) }
-    let(:expert_procedure) { create(:experts_procedure, procedure: procedure, expert: expert) }
-
-    subject do
-      put :update_allow_decision_access, params: { procedure_id: procedure.id, experts_procedure: { allow_decision_access: !expert_procedure.allow_decision_access }, expert_procedure: expert_procedure }, format: :js
-    end
-
-    context 'when the experts_procedure is true' do
-      let(:expert_procedure) { create(:experts_procedure, procedure: procedure, expert: expert, allow_decision_access: true) }
-
-      before do
-        subject
-        expert_procedure.reload
-      end
-
-      it { expect(expert_procedure.allow_decision_access).to be_falsy }
-    end
-
-    context 'when the experts_procedure is false' do
-      before do
-        subject
-        expert_procedure.reload
-      end
-
-      it { expect(expert_procedure.allow_decision_access).to be_truthy }
-    end
-  end
 end

spec/jobs/virus_scanner_job_spec.rb

@@ -18,6 +18,20 @@ describe VirusScannerJob, type: :job do
       blob.analyze
     end
 
+    context "when there is an integrity error" do
+      before do
+        blob.update_column('checksum', 'integrity error')
+
+        assert_performed_jobs(5) do
+          VirusScannerJob.perform_later(blob)
+        end
+      end
+
+      it do
+        expect(blob.reload.virus_scanner.corrupt?).to be_truthy
+      end
+    end
+
     context "when no virus is found" do
       before do
         allow(ClamavService).to receive(:safe_file?).and_return(true)

spec/views/new_administrateur/experts_procedures/index.html.haml_spec.rb

@@ -1,4 +1,4 @@
-describe 'new_administrateur/procedures/invited_expert_list.html.haml', type: :view do
+describe 'new_administrateur/experts_procedures/index.html.haml', type: :view do
   let!(:procedure) { create(:procedure, :published) }
 
   before do

spec/views/shared/attachment/_show.html.haml_spec.rb

@@ -55,4 +55,14 @@ describe 'shared/attachment/_show.html.haml', type: :view do
       expect(subject).to have_text('virus détecté')
     end
   end
+
+  context 'when the file is corrupted' do
+    let(:virus_scan_result) { ActiveStorage::VirusScanner::INTEGRITY_ERROR }
+
+    it 'displays the filename, but doesn’t allow to download the file' do
+      expect(subject).to have_text(champ.piece_justificative_file.filename.to_s)
+      expect(subject).not_to have_link(champ.piece_justificative_file.filename.to_s)
+      expect(subject).to have_text('corrompu')
+    end
+  end
 end