Merge pull request #4245 from betagouv/dev

2019-08-27-01

app/controllers/admin/procedures_controller.rb

@@ -249,8 +249,11 @@ class Admin::ProceduresController < AdminController
   end
 
   def delete_logo
+    if @procedure.logo
       @procedure.remove_logo!
       @procedure.save
+    end
+    @procedure.logo_active_storage.purge_later
 
     flash.notice = 'le logo a bien été supprimé'
     redirect_to edit_admin_procedure_path(@procedure)
@@ -278,10 +281,12 @@ class Admin::ProceduresController < AdminController
 
   def procedure_params
     editable_params = [:libelle, :description, :organisation, :direction, :lien_site_web, :cadre_juridique, :deliberation, :notice, :web_hook_url, :euro_flag, :logo, :auto_archive_on, :monavis_embed]
-    if @procedure&.locked?
+    permited_params = if @procedure&.locked?
       params.require(:procedure).permit(*editable_params)
     else
-      params.require(:procedure).permit(*editable_params, :duree_conservation_dossiers_dans_ds, :duree_conservation_dossiers_hors_ds, :for_individual, :ask_birthday, :path)
+      params.require(:procedure).permit(*editable_params, :duree_conservation_dossiers_dans_ds, :duree_conservation_dossiers_hors_ds, :for_individual, :path)
     end
+    permited_params[:logo_active_storage] = permited_params.delete(:logo)
+    permited_params
   end
 end

app/controllers/new_administrateur/mail_templates_controller.rb

@@ -7,7 +7,7 @@ module NewAdministrateur
       dossier = Dossier.new(id: '1', procedure: procedure)
 
       @dossier = dossier
-      @logo_url = procedure.logo.url
+      @logo_url = procedure.logo_url
       @service = procedure.service
       @rendered_template = sanitize(mail_template.body)
       @actions = mail_template.actions_for_dossier(dossier)

app/controllers/users/sessions_controller.rb

@@ -51,7 +51,11 @@ class Users::SessionsController < Devise::SessionsController
       .trusted_device_tokens
       .find_by(token: params[:jeton])
 
-    if trusted_device_token&.token_valid?
+    if trusted_device_token.nil?
+      flash[:alert] = 'Votre lien est invalide.'
+
+      redirect_to root_path
+    elsif trusted_device_token.token_valid?
       trust_device(trusted_device_token.created_at)
 
       period = ((trusted_device_token.created_at + TRUSTED_DEVICE_PERIOD) - Time.zone.now).to_i / ActiveSupport::Duration::SECONDS_PER_DAY
@@ -67,7 +71,7 @@ class Users::SessionsController < Devise::SessionsController
         redirect_to new_user_session_path
       end
     else
-      flash[:alert] = 'Votre lien est invalide ou expiré, un nouveau vient de vous être envoyé.'
+      flash[:alert] = 'Votre lien est expiré, un nouveau vient de vous être envoyé.'
 
       send_login_token_or_bufferize(instructeur)
       redirect_to link_sent_path(email: instructeur.email)

app/helpers/procedure_helper.rb

@@ -20,20 +20,6 @@ module ProcedureHelper
     t(action, scope: [:modal, :publish, key])
   end
 
-  def logo_img(procedure)
-    logo = procedure.logo
-
-    if logo.blank?
-      ActionController::Base.helpers.image_url("marianne.svg")
-    else
-      if Rails.application.secrets.fog[:enabled]
-        RemoteDownloader.new(logo.filename).url
-      else
-        LocalDownloader.new(logo.path, 'logo').url
-      end
-    end
-  end
-
   def types_de_champ_data(procedure)
     {
       isAnnotation: false,

app/mailers/application_mailer.rb

@@ -9,8 +9,13 @@ class ApplicationMailer < ActionMailer::Base
     return nil if !procedure.logo?
 
     begin
+      if procedure.logo_active_storage.attached?
+        logo_filename = procedure.logo_active_storage.filename
+        attachments.inline[logo_filename] = procedure.logo_active_storage.download
+      else
         logo_filename = procedure.logo.filename
         attachments.inline[logo_filename] = procedure.logo.read
+      end
       attachments[logo_filename].url
 
     rescue StandardError => e

app/models/procedure.rb

@@ -25,6 +25,7 @@ class Procedure < ApplicationRecord
   has_one :refused_mail, class_name: "Mails::RefusedMail", dependent: :destroy
   has_one :without_continuation_mail, class_name: "Mails::WithoutContinuationMail", dependent: :destroy
 
+  has_one_attached :logo_active_storage
   has_one_attached :notice
   has_one_attached :deliberation
 
@@ -215,7 +216,9 @@ class Procedure < ApplicationRecord
     procedure.archived_at = nil
     procedure.published_at = nil
     procedure.logo_secure_token = nil
+    if logo.present?
       procedure.remote_logo_url = self.logo_url
+    end
     procedure.lien_notice = nil
 
     if is_different_admin || from_library
@@ -233,6 +236,7 @@ class Procedure < ApplicationRecord
     procedure.closed_mail = closed_mail&.dup
     procedure.refused_mail = refused_mail&.dup
     procedure.without_continuation_mail = without_continuation_mail&.dup
+    procedure.ask_birthday = false # see issue #4242
 
     procedure.cloned_from_library = from_library
     procedure.parent_procedure = self
@@ -252,6 +256,7 @@ class Procedure < ApplicationRecord
     if original.is_a?(TypeDeChamp)
       clone_attachment(:piece_justificative_template, original, kopy)
     elsif original.is_a?(Procedure)
+      clone_attachment(:logo_active_storage, original, kopy)
       clone_attachment(:notice, original, kopy)
       clone_attachment(:deliberation, original, kopy)
     end
@@ -452,6 +457,25 @@ class Procedure < ApplicationRecord
     end
   end
 
+  def logo?
+    logo.present? || logo_active_storage.attached?
+  end
+
+  def logo_url
+    if !logo?
+      ActionController::Base.helpers.image_url("marianne.svg")
+    elsif logo_active_storage.attached?
+      Rails.application.routes.url_helpers.url_for(logo_active_storage)
+    else
+      if Rails.application.secrets.fog[:enabled]
+        RemoteDownloader.new(logo.filename).url
+      else
+        # FIXME: this is horrible but used only in dev and will be removed after migration
+        File.join(LOCAL_DOWNLOAD_URL, logo.url)
+      end
+    end
+  end
+
   private
 
   def move_type_de_champ_attributes(types_de_champ, type_de_champ, new_index)

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

@@ -77,13 +77,16 @@
 .row
   .col-md-6
     %h4 Logo de la démarche
-    - if @procedure.logo.present?
-      = image_tag logo_img(@procedure), { style: 'height: 40px; display: inline; margin-right: 6px;', id: 'preview_procedure_logo' }
+    - if @procedure.logo?
+      = image_tag @procedure.logo_url, { style: 'height: 40px; display: inline; margin-right: 6px;', id: 'preview_procedure_logo' }
       \-
 
       - if @procedure.persisted?
         = link_to 'supprimer', delete_logo_admin_procedure_path(@procedure), method: :delete
-    = f.file_field :logo, accept: 'image/png, image/jpg, image/jpeg', style: 'display: inline'
+    = f.file_field :logo,
+      direct_upload: true,
+      accept: 'image/png, image/jpg, image/jpeg',
+      style: 'display: inline'
 
     %div{ style: 'margin-top: 5px;' }
       %i
@@ -118,12 +121,6 @@
       %b
         Si votre démarche s’adresse indifféremment à une personne morale ou un particulier choisissez l'option "particuliers". Vous pourrez utilisez le champ SIRET directement dans le formulaire.
 
-      %ul#individual-with-siret
-        %li
-          .checkbox
-            %label
-              = f.check_box :ask_birthday
-              Demander la date de naissance.
 .row
   .col-md-6
     %h4 Options avancées

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

@@ -4,7 +4,7 @@
   .sub-header
     .container.flex
 
-      .procedure-logo{ style: @procedure.logo.present? ? "background-image: url(#{@procedure.logo.url})" : nil,
+      .procedure-logo{ style: @procedure.logo? ? "background-image: url(#{@procedure.logo_url})" : nil,
         role: 'img', 'aria-label': "logo de la démarche #{@procedure.libelle}" }
 
       .procedure-header

app/views/layouts/commencer/_procedure_description.html.haml

@@ -1,5 +1,5 @@
 .procedure-logos
-  = image_tag logo_img(procedure)
+  = image_tag procedure.logo_url
   - if procedure.euro_flag
     = image_tag "flag_of_europe.svg"
 %h2.procedure-title

app/views/layouts/left_panels/_left_panel_admin_procedurescontroller_navbar.html.haml

@@ -1,7 +1,7 @@
 #first-block
   .en-cours
-    - if @procedure.logo.present?
-      = image_tag logo_img(@procedure), style: 'width: 30px;'
+    - if @procedure.logo?
+      = image_tag @procedure.logo_url, style: 'width: 30px;'
     %b
       = @procedure.libelle
 

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

@@ -1,6 +1,6 @@
 .toolbar
   %button.button.primary.new-area Ajouter une zone
-  %input.address{ data: { address: true, autocomplete: 'address' }, placeholder: 'Saissisez une adresse ou positionner la carte' }
+  %input.address{ data: { address: true, autocomplete: 'address' }, placeholder: 'Saisissez une adresse ou positionner la carte' }
 
 .carte.edit{ data: { geo: geo_data(champ) }, class: "carte-#{form.index}" }
 

app/views/users/sessions/_resume_procedure.html.haml

@@ -8,7 +8,7 @@
           = image_tag('drapeau_europe.png')
 
       #logo_procedure.flag
-        = image_tag(logo_img(dossier.procedure))
+        = image_tag(dossier.procedure.logo_url)
 
       %h2#titre-procedure.text-info
         = @dossier.procedure.libelle

app/views/users/sessions/new.html.haml

@@ -1,12 +1,6 @@
 = content_for(:page_id, 'auth')
 
 .auth-form.sign-in-form
-  %p.register
-    %span
-      Nouveau sur demarches‑simplifiees.fr ?
-    = link_to "Créer un compte", new_user_registration_path, class: "button primary auth-signup-button"
-
-  %hr
 
   = form_for User.new, url: user_session_path, html: { class: "form" } do |f|
     %h1 Connectez-vous
@@ -28,3 +22,10 @@
     = f.submit "Se connecter", class: "button large primary expand"
 
   = render partial: 'shared/france_connect_login', locals: { url: france_connect_particulier_path }
+
+  %hr
+  %p.center
+    %span Vous êtes nouveau sur demarches‑simplifiees.fr ?
+    %br
+    %br
+    = link_to "Trouvez votre démarche", "https://faq.demarches-simplifiees.fr/article/59-comment-trouver-ma-demarche", target: "_blank", class: "button expend secondary"

config/initializers/content_security_policy.rb

@@ -17,5 +17,5 @@ Rails.application.config.content_security_policy do |policy|
   policy.connect_src :self, "wss://*.crisp.chat", "*.crisp.chat", "*.demarches-simplifiees.fr", "in-automate.sendinblue.com", "app.franceconnect.gouv.fr", "sentry.io"
   # Pour tout le reste, par défaut on accepte uniquement ce qui vient de chez nous
   # et dans la notification on inclue la source de l'erreur
-  policy.default_src :self, :data, :report_sample, "fonts.gstatic.com", "in-automate.sendinblue.com", "player.vimeo.com", "app.franceconnect.gouv.fr", "sentry.io", "static.demarches-simplifiees.fr", "*.crisp.chat", "crisp.chat", "*.sibautomation.com", "sibautomation.com", "data"
+  policy.default_src :self, :data, :report_sample, "fonts.gstatic.com", "in-automate.sendinblue.com", "player.vimeo.com", "app.franceconnect.gouv.fr", "sentry.io", "static.demarches-simplifiees.fr", "*.crisp.chat", "crisp.chat", "*.crisp.help", "*.sibautomation.com", "sibautomation.com", "data"
 end

lib/tasks/2019_08_20_migrate_procedure_logo.rake

@@ -0,0 +1,33 @@
+namespace :'2019_08_20_migrate_procedure_logo' do
+  task run: :environment do
+    procedures = Procedure.unscope(where: :hidden_at)
+      .where.not(logo: nil)
+      .left_joins(:logo_active_storage_attachment)
+      .where('active_storage_attachments.id IS NULL')
+      .order(:created_at)
+
+    limit = ENV['LIMIT']
+    if limit
+      procedures.limit!(limit.to_i)
+    end
+
+    progress = ProgressReport.new(procedures.count)
+    procedures.find_each do |procedure|
+      if procedure.logo.present?
+        uri = URI.parse(URI.escape(procedure.logo_url))
+        response = Typhoeus.get(uri)
+        if response.success?
+          filename = procedure.logo.filename || procedure.logo_identifier
+          procedure.logo_active_storage.attach(
+            io: StringIO.new(response.body),
+            filename: filename,
+            content_type: procedure.logo.content_type,
+            metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE }
+          )
+        end
+      end
+      progress.inc
+    end
+    progress.finish
+  end
+end

spec/controllers/new_administrateur/mail_templates_controller_spec.rb

@@ -14,7 +14,7 @@ describe NewAdministrateur::MailTemplatesController, type: :controller do
     it { expect(response).to have_http_status(:ok) }
 
     it 'displays the procedure logo' do
-      expect(response.body).to have_css("img[src*='#{procedure.logo.filename}']")
+      expect(response.body).to have_css("img[src*='#{procedure.logo_url}']")
     end
 
     it 'displays the action buttons' do

spec/controllers/users/sessions_controller_spec.rb

@@ -136,7 +136,9 @@ describe Users::SessionsController, type: :controller do
     context 'when the instructeur has non other account' do
       let(:instructeur) { create(:instructeur) }
       let!(:good_jeton) { instructeur.create_trusted_device_token }
+      let(:jeton) { good_jeton }
       let(:logged) { false }
+      let(:valid_token) { true }
 
       before do
         if logged
@@ -144,34 +146,41 @@ describe Users::SessionsController, type: :controller do
         end
         allow(controller).to receive(:trust_device)
         allow(controller).to receive(:send_login_token_or_bufferize)
+        allow_any_instance_of(TrustedDeviceToken).to receive(:token_valid?).and_return(valid_token)
         post :sign_in_by_link, params: { id: instructeur.id, jeton: jeton }
       end
 
       context 'when the instructeur is not logged in' do
         context 'when the token is valid' do
-          let(:jeton) { good_jeton }
-
           it { is_expected.to redirect_to new_user_session_path }
           it { expect(controller.current_instructeur).to be_nil }
           it { expect(controller).to have_received(:trust_device) }
         end
 
         context 'when the token is invalid' do
-          let(:jeton) { 'invalid_token' }
+          let(:valid_token) { false }
 
           it { is_expected.to redirect_to link_sent_path(email: instructeur.email) }
           it { expect(controller.current_instructeur).to be_nil }
           it { expect(controller).not_to have_received(:trust_device) }
           it { expect(controller).to have_received(:send_login_token_or_bufferize) }
         end
+
+        context 'when the token does not exist' do
+          let(:jeton) { 'I do not exist' }
+
+          it { is_expected.to redirect_to root_path }
+          it { expect(controller.current_instructeur).to be_nil }
+          it { expect(controller).not_to have_received(:trust_device) }
+          it { expect(controller).not_to have_received(:send_login_token_or_bufferize) }
+          it { expect(flash.alert).to eq('Votre lien est invalide.') }
+        end
       end
 
       context 'when the instructeur is logged in' do
         let(:logged) { true }
 
         context 'when the token is valid' do
-          let(:jeton) { good_jeton }
-
           # redirect to root_path, then redirect to instructeur_procedures_path (see root_controller)
           it { is_expected.to redirect_to root_path }
           it { expect(controller.current_instructeur).to eq(instructeur) }
@@ -179,7 +188,7 @@ describe Users::SessionsController, type: :controller do
         end
 
         context 'when the token is invalid' do
-          let(:jeton) { 'invalid_token' }
+          let(:valid_token) { false }
 
           it { is_expected.to redirect_to link_sent_path(email: instructeur.email) }
           it { expect(controller.current_instructeur).to eq(instructeur) }

spec/factories/procedure.rb

@@ -43,6 +43,10 @@ FactoryBot.define do
     end
 
     trait :with_logo do
+      logo_active_storage { Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'image/png') }
+    end
+
+    trait :with_legacy_logo do
       logo { Rack::Test::UploadedFile.new("./spec/fixtures/files/logo_test_procedure.png", 'image/png') }
     end
 

spec/features/users/sign_up_spec.rb

@@ -3,24 +3,23 @@ require 'spec_helper'
 feature 'Signing up:' do
   let(:user_email) { generate :user_email }
   let(:user_password) { 'démarches-simplifiées-pwd' }
+  let(:procedure) { create :simple_procedure, :with_service }
 
   scenario 'a new user can sign-up' do
-    visit root_path
-    click_on 'Connexion'
-    click_on 'Créer un compte'
+    visit commencer_path(path: procedure.path)
+    click_on 'Créer un compte demarches-simplifiees.fr'
 
     sign_up_with user_email, user_password
     expect(page).to have_content "nous avons besoin de vérifier votre adresse #{user_email}"
 
     click_confirmation_link_for user_email
     expect(page).to have_content 'Votre compte a été activé'
-    expect(page).to have_current_path dossiers_path
+    expect(page).to have_current_path commencer_path(path: procedure.path)
   end
 
-  scenario 'a new user can’t sign-up with too short password' do
-    visit root_path
-    click_on 'Connexion'
-    click_on 'Créer un compte'
+  scenario 'a new user can’t sign-up with too short password when visiting a procedure' do
+    visit commencer_path(path: procedure.path)
+    click_on 'Créer un compte demarches-simplifiees.fr'
 
     expect(page).to have_current_path new_user_registration_path
     sign_up_with user_email, '1234567'
@@ -61,9 +60,8 @@ feature 'Signing up:' do
 
   context 'when a user is not confirmed yet' do
     before do
-      visit root_path
-      click_on 'Connexion'
-      click_on 'Créer un compte'
+      visit commencer_path(path: procedure.path)
+      click_on 'Créer un compte demarches-simplifiees.fr'
 
       sign_up_with user_email, user_password
     end

spec/helpers/procedure_helper_spec.rb

@@ -1,9 +0,0 @@
-RSpec.describe ProcedureHelper, type: :helper do
-  let(:procedure) { create(:procedure) }
-
-  describe ".logo_img" do
-    subject { logo_img(procedure) }
-
-    it { is_expected.to match(/#{ActionController::Base.helpers.image_url("marianne.svg")}/) }
-  end
-end

spec/lib/tasks/2019_08_20_migrate_procedure_logo_spec.rb

@@ -0,0 +1,60 @@
+describe '2019_08_20_migrate_procedure_logo.rake' do
+  let(:rake_task) { Rake::Task['2019_08_20_migrate_procedure_logo:run'] }
+
+  let(:procedures) do
+    [
+      create(:procedure),
+      create(:procedure, :with_legacy_logo),
+      create(:procedure, :with_legacy_logo)
+    ]
+  end
+
+  let(:run_task) do
+    rake_task.invoke
+    procedures.each(&:reload)
+  end
+
+  before do
+    procedures.each do |procedure|
+      if procedure.logo.present?
+        stub_request(:get, procedure.logo_url)
+          .to_return(status: 200, body: File.read(procedure.logo.path))
+      end
+    end
+  end
+
+  after do
+    ENV['LIMIT'] = nil
+    rake_task.reenable
+  end
+
+  it 'should migrate logo' do
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, false, false])
+
+    run_task
+
+    expect(Procedure.where(logo: nil).count).to eq(1)
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, true, true])
+  end
+
+  it 'should migrate logo within limit' do
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, false, false])
+
+    ENV['LIMIT'] = '1'
+    run_task
+
+    expect(Procedure.where(logo: nil).count).to eq(1)
+    expect(procedures.map(&:logo_active_storage).map(&:attached?)).to eq([false, true, false])
+  end
+
+  context 'when a procedure is hidden' do
+    let(:hidden_procedure) { create(:procedure, :hidden, :with_legacy_logo) }
+    let(:procedures) { [hidden_procedure] }
+
+    it 'should migrate logo' do
+      run_task
+
+      expect(hidden_procedure.logo_active_storage.attached?).to be true
+    end
+  end
+end

spec/models/procedure_spec.rb

@@ -448,6 +448,18 @@ describe Procedure do
       it 'should have one administrateur' do
         expect(subject.administrateurs).to eq([administrateur])
       end
+
+      it 'should set ask_birthday to false' do
+        expect(subject.ask_birthday?).to eq(false)
+      end
+    end
+
+    context 'when the procedure is cloned from the library' do
+      let(:procedure) { create(:procedure, received_mail: received_mail, service: service, ask_birthday: true) }
+
+      it 'should set ask_birthday to false' do
+        expect(subject.ask_birthday?).to eq(false)
+      end
     end
 
     it 'should keep service_id' do

spec/spec_helper.rb

@@ -39,13 +39,18 @@ Capybara.register_driver :chrome do |app|
 end
 
 Capybara.register_driver :headless_chrome do |app|
+  options = Selenium::WebDriver::Chrome::Options.new
+  options.add_argument('--headless')
+  options.add_argument('--window-size=1440,900')
+
   capabilities = Selenium::WebDriver::Remote::Capabilities.chrome(
     chromeOptions: { args: ['headless', 'disable-dev-shm-usage', 'disable-software-rasterizer', 'mute-audio', 'window-size=1440,900'] }
   )
 
   Capybara::Selenium::Driver.new app,
     browser: :chrome,
-    desired_capabilities: capabilities
+    desired_capabilities: capabilities,
+    options: options
 end
 
 # FIXME: remove this line when https://github.com/rspec/rspec-rails/issues/1897 has been fixed

yarn.lock

@@ -3224,14 +3224,16 @@ eslint-scope@^4.0.0, eslint-scope@^4.0.3:
     estraverse "^4.1.1"
 
 eslint-utils@^1.3.1:
-  version "1.3.1"
-  resolved "https://registry.yarnpkg.com/eslint-utils/-/eslint-utils-1.3.1.tgz#9a851ba89ee7c460346f97cf8939c7298827e512"
-  integrity sha512-Z7YjnIldX+2XMcjr7ZkgEsOj/bREONV60qYeB/bjMAqqqZ4zxKyWX+BOUkdmRmA9riiIPVvo5x86m5elviOk0Q==
+  version "1.4.2"
+  resolved "https://registry.yarnpkg.com/eslint-utils/-/eslint-utils-1.4.2.tgz#166a5180ef6ab7eb462f162fd0e6f2463d7309ab"
+  integrity sha512-eAZS2sEUMlIeCjBeubdj45dmBHQwPHWyBcT1VSYB7o9x9WRRqKxyUoiXlRjyAwzN7YEzHJlYg0NmzDRWx6GP4Q==
+  dependencies:
+    eslint-visitor-keys "^1.0.0"
 
 eslint-visitor-keys@^1.0.0:
-  version "1.0.0"
-  resolved "https://registry.yarnpkg.com/eslint-visitor-keys/-/eslint-visitor-keys-1.0.0.tgz#3f3180fb2e291017716acb4c9d6d5b5c34a6a81d"
-  integrity sha512-qzm/XxIbxm/FHyH341ZrbnMUpe+5Bocte9xkmFMzPMjRaZMcXww+MpBptFvtU+79L362nqiLhekCxCxDPaUMBQ==
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/eslint-visitor-keys/-/eslint-visitor-keys-1.1.0.tgz#e2a82cea84ff246ad6fb57f9bde5b46621459ec2"
+  integrity sha512-8y9YjtM1JBJU/A9Kc+SbaOV4y29sSWckBwMHa+FGtVj5gN/sbnKDf6xJUl+8g7FAij9LVaP8C24DUiH/f/2Z9A==
 
 eslint@^5.16.0:
   version "5.16.0"