Merge pull request #4452 from betagouv/dev

2019-10-29-01

Gemfile

@@ -4,7 +4,7 @@ gem 'aasm'
 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'
+gem 'activestorage-openstack', git: 'https://github.com/fredZen/activestorage-openstack.git', branch: 'frederic/fix_upload_signature'
 gem 'administrate'
 gem 'after_party'
 gem 'anchored'

Gemfile.lock

@@ -1,3 +1,14 @@
+GIT
+  remote: https://github.com/fredZen/activestorage-openstack.git
+  revision: c71d5107a51701eab9d9267dd0000e6c1cf3e39a
+  branch: frederic/fix_upload_signature
+  specs:
+    activestorage-openstack (0.5.0)
+      fog-openstack (~> 1.0)
+      marcel
+      mime-types
+      rails (~> 5.2.0)
+
 GIT
   remote: https://github.com/kobaltz/actiontext.git
   revision: ef59c4ba99d1b7614dd47f5a294eef553224db88
@@ -64,11 +75,6 @@ GEM
       actionpack (= 5.2.2.1)
       activerecord (= 5.2.2.1)
       marcel (~> 0.3.1)
-    activestorage-openstack (1.0.0)
-      fog-openstack (~> 1.0)
-      marcel
-      mime-types
-      rails (<= 6)
     activesupport (5.2.2.1)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 0.7, < 2)
@@ -711,7 +717,7 @@ DEPENDENCIES
   actiontext!
   active_link_to
   active_model_serializers
-  activestorage-openstack
+  activestorage-openstack!
   administrate
   after_party
   anchored

app/assets/images/icons/add.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" aria-hidden="true" width="24" height="24"><path d="M18 10.5h-4.5V6a1 1 0 0 0-1-1h-1a1 1 0 0 0-1 1v4.5H6a1 1 0 0 0-1 1v1a1 1 0 0 0 1 1h4.5V18a1 1 0 0 0 1 1h1a1 1 0 0 0 1-1v-4.5H18a1 1 0 0 0 1-1v-1a1 1 0 0 0-1-1z" fill="#333"/></svg>

app/assets/stylesheets/new_design/icons.scss

@@ -79,11 +79,16 @@
     background-image: image-url("icons/lock.svg");
   }
 
+  &.add {
+    background-image: image-url("icons/add.svg");
+    margin-left: -5px;
+    margin-right: 0px;
+  }
+
   &.justificatif {
     background-image: image-url("icons/justificatif.svg");
   }
 
-
   &.printer {
     background-image: image-url("icons/printer.svg");
   }

app/controllers/application_controller.rb

@@ -250,7 +250,7 @@ class ApplicationController < ActionController::Base
         payload: {
           DS_SIGN_IN_COUNT: current_user&.sign_in_count,
           DS_CREATED_AT: current_administrateur&.created_at,
-          DS_ACTIVE: current_administrateur&.active,
+          DS_ACTIVE: current_administrateur&.active?,
           DS_ID: current_administrateur&.id,
           DS_GESTIONNAIRE_ID: current_instructeur&.id,
           DS_ROLES: current_user_roles

app/controllers/instructeurs/dossiers_controller.rb

@@ -49,7 +49,7 @@ module Instructeurs
       @previous_following_instructeurs_emails = previous_followers.pluck(:email)
       @avis_emails = dossier.avis.includes(:instructeur).map(&:email_to_display)
       @invites_emails = dossier.invites.map(&:email)
-      @potential_recipients = procedure.defaut_groupe_instructeur.instructeurs.reject { |g| g == current_instructeur }
+      @potential_recipients = dossier.groupe_instructeur.instructeurs.reject { |g| g == current_instructeur }
     end
 
     def send_to_instructeurs

app/controllers/root_controller.rb

@@ -17,15 +17,15 @@ class RootController < ApplicationController
   end
 
   def patron
-    description = 'aller voir le super site : https://demarches-simplifiees.fr'
+    description = 'Aller voir le super site : https://demarches-simplifiees.fr'
 
     all_champs = TypeDeChamp.type_champs
-      .map { |name, _| TypeDeChamp.new(type_champ: name, private: false, libelle: name, description: description, mandatory: true) }
+      .map { |name, _| TypeDeChamp.new(type_champ: name, private: false, libelle: name.humanize, description: description, mandatory: true) }
       .map.with_index { |type_de_champ, i| type_de_champ.champ.build(id: i) }
 
     all_champs
       .filter { |champ| champ.type_champ == TypeDeChamp.type_champs.fetch(:header_section) }
-      .each { |champ| champ.type_de_champ.libelle = 'un super titre de section' }
+      .each { |champ| champ.type_de_champ.libelle = 'Un super titre de section' }
 
     all_champs
       .filter { |champ| [TypeDeChamp.type_champs.fetch(:drop_down_list), TypeDeChamp.type_champs.fetch(:multiple_drop_down_list)].include?(champ.type_champ) }
@@ -39,6 +39,16 @@ class RootController < ApplicationController
         champ.value = '["option B", "option C"]'
       end
 
+    all_champs
+      .filter { |champ| champ.type_champ == TypeDeChamp.type_champs.fetch(:repetition) }
+      .each do |champ_repetition|
+        libelles = ['Prénom', 'Nom'];
+        champ_repetition.champs << libelles.map.with_index do |libelle, i|
+          text_tdc = TypeDeChamp.new(type_champ: :text, private: false, libelle: libelle, description: description, mandatory: true)
+          text_tdc.champ.build(id: all_champs.length + i)
+        end
+      end
+
     type_champ_values = {
       TypeDeChamp.type_champs.fetch(:date)      => '2016-07-26',
       TypeDeChamp.type_champs.fetch(:datetime)  => '26/07/2016 07:35',

app/lib/active_storage/service/ds_proxy_service.rb

@@ -0,0 +1,57 @@
+module ActiveStorage
+  # Wraps an ActiveStorage::Service to route direct upload and direct download URLs through our proxy,
+  # thus avoiding exposing the storage provider’s URL to our end-users.
+  class Service::DsProxyService < SimpleDelegator
+    attr_reader :wrapped
+
+    def self.build(wrapped:, configurator:, **options)
+      new(wrapped: configurator.build(wrapped))
+    end
+
+    def initialize(wrapped:)
+      @wrapped = wrapped
+      super(wrapped)
+    end
+
+    def url(*args)
+      url = wrapped.url(*args)
+      publicize(url)
+    end
+
+    def url_for_direct_upload(*args)
+      url = wrapped.url_for_direct_upload(*args)
+      publicize(url)
+    end
+
+    private
+
+    def object_for(key, &block)
+      blob_url = url(key)
+      if block_given?
+        request = Typhoeus::Request.new(blob_url)
+        request.on_headers do |response|
+          if response.code != 200
+            raise Fog::OpenStack::Storage::NotFound.new
+          end
+        end
+        request.on_body do |chunk|
+          yield chunk
+        end
+        request.run
+      else
+        response = Typhoeus.get(blob_url)
+        if response.success?
+          response
+        else
+          raise Fog::OpenStack::Storage::NotFound.new
+        end
+      end
+    end
+
+    def publicize(url)
+      search = %r{^https://[^/]+/v1/AUTH_[a-f0-9]{32}}
+      replace = 'https://static.demarches-simplifiees.fr'
+      url.gsub(search, replace)
+    end
+  end
+end

app/models/administrateur.rb

@@ -56,7 +56,7 @@ class Administrateur < ApplicationRecord
   end
 
   def invitation_expired?
-    !active && !user.reset_password_period_valid?
+    !active? && !user.reset_password_period_valid?
   end
 
   def self.reset_password(reset_password_token, password)
@@ -66,10 +66,6 @@ class Administrateur < ApplicationRecord
       reset_password_token: reset_password_token
     })
 
-    if administrateur && administrateur.errors.empty?
-      administrateur.update_column(:active, true)
-    end
-
     administrateur
   end
 
@@ -84,4 +80,8 @@ class Administrateur < ApplicationRecord
   def can_be_deleted?
     dossiers.state_instruction_commencee.none? && procedures.none?
   end
+
+  def active?
+    user.last_sign_in_at.present?
+  end
 end

app/models/instructeur.rb

@@ -110,8 +110,10 @@ class Instructeur < ApplicationRecord
   end
 
   def notifications_for_procedure(procedure, scope)
-    procedure
-      .defaut_groupe_instructeur.dossiers
+    target_groupes = groupe_instructeurs.where(procedure: procedure)
+
+    Dossier
+      .where(groupe_instructeur: target_groupes)
       .send(scope) # :en_cours or :termine or :not_archived (or any other Dossier scope)
       .merge(followed_dossiers)
       .with_notifications

app/services/administrateur_usage_statistics_service.rb

@@ -29,7 +29,7 @@ class AdministrateurUsageStatisticsService
     result = {
       ds_sign_in_count: administrateur.user.sign_in_count,
       ds_created_at: administrateur.created_at,
-      ds_active: administrateur.active,
+      ds_active: administrateur.active?,
       ds_id: administrateur.id,
       nb_services: nb_services_by_administrateur_id[administrateur.id],
       nb_instructeurs: nb_instructeurs_by_administrateur_id[administrateur.id],

app/views/root/patron.html.haml

@@ -1,6 +1,6 @@
 .patron
   .container
-    %h1 Icones
+    %h1 Icônes
 
     %span.icon.follow
     %span.icon.unfollow
@@ -16,6 +16,7 @@
     %span.icon.bubble
     %span.icon.attachment
     %span.icon.lock
+    %span.icon.add
     %span.icon.justificatif
     %span.icon.printer
     %span.icon.account
@@ -47,7 +48,9 @@
           = render partial: "shared/dossiers/editable_champs/editable_champ",
             locals: { champ: champ, form: champ_form, seen_at: nil }
 
+        %label Mot de passe
         %input{ type: "password", value: "12345678" }
+
         .send-wrapper
           = f.submit 'Enregistrer un brouillon (formnovalidate)', formnovalidate: true, class: 'button send'
           = f.submit 'Envoyer', class: 'button send primary'

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

@@ -14,7 +14,10 @@
             Supprimer
 
 - if champ.persisted?
-  = link_to "Ajouter une ligne pour « #{champ.libelle} »", champs_repetition_path(form.index), class: 'button add-row', data: { remote: true, method: 'POST', params: { champ_id: champ&.id }.to_query }
+  = link_to champs_repetition_path(form.index), class: 'button add-row', data: { remote: true, method: 'POST', params: { champ_id: champ&.id }.to_query } do
+    %span.icon.add
+    Ajouter une ligne pour « #{champ.libelle} »
 - else
-  %button.button{ type: :button }
-    = "Ajouter une ligne pour « #{champ.libelle} »"
+  %a.button.add-row
+    %span.icon.add
+    Ajouter une ligne pour « #{champ.libelle} »

config/environments/production.rb

@@ -93,7 +93,7 @@ Rails.application.configure do
   # the I18n.default_locale when a translation cannot be found).
   config.i18n.fallbacks = true
 
-  config.active_storage.service = :openstack
+  config.active_storage.service = :proxied
 
   # Send deprecation notices to registered listeners.
   config.active_support.deprecation = :notify

config/initializers/active_storage.rb

@@ -7,33 +7,3 @@ ActiveStorage::Service.url_expires_in = 1.hour
 # cleaner (as it allows to enqueue the virus scan on attachment creation, rather
 # than on blob creation).
 ActiveSupport.on_load(:active_storage_blob) { include BlobVirusScanner }
-
-# When an OpenStack service is initialized it makes a request to fetch
-# `publicURL` to use for all operations. We intercept the method that reads
-# this url and replace the host with DS_Proxy host. This way all the operation
-# are performed through DS_Proxy.
-#
-# https://github.com/fog/fog-openstack/blob/37621bb1d5ca78d037b3c56bd307f93bba022ae1/lib/fog/openstack/auth/catalog/v2.rb#L16
-require 'fog/openstack/auth/catalog/v2'
-
-module Fog::OpenStack::Auth::Catalog
-  class V2
-    def endpoint_url(endpoint, interface)
-      url = endpoint["#{interface}URL"]
-
-      if interface == 'public'
-        publicize(url)
-      else
-        url
-      end
-    end
-
-    private
-
-    def publicize(url)
-      search = %r{^https://[^/]+/}
-      replace = 'https://static.demarches-simplifiees.fr/'
-      url.gsub(search, replace)
-    end
-  end
-end

config/locales/fr.yml

@@ -189,14 +189,14 @@ fr:
       procedure_archived: "Cette démarche en ligne a été fermée, il n'est plus possible de déposer de dossier."
       procedure_not_draft: "Cette démarche n’est maintenant plus en brouillon."
       cadastres_empty:
-        one: "Aucune parcelle cadastrale sur la zone séléctionnée"
-        other: "Aucune parcelle cadastrale sur les zones séléctionnées"
+        one: "Aucune parcelle cadastrale sur la zone sélectionnée"
+        other: "Aucune parcelle cadastrale sur les zones sélectionnées"
       quartiers_prioritaires_empty:
-        one: "Aucun quartier prioritaire sur la zone séléctionnée"
-        other: "Aucun quartier prioritaire sur les zones séléctionnées"
+        one: "Aucun quartier prioritaire sur la zone sélectionnée"
+        other: "Aucun quartier prioritaire sur les zones sélectionnées"
       parcelles_agricoles_empty:
-        one: "Aucune parcelle agricole sur la zone séléctionnée"
-        other: "Aucune parcelle agricole sur les zones séléctionnées"
+        one: "Aucune parcelle agricole sur la zone sélectionnée"
+        other: "Aucune parcelle agricole sur les zones sélectionnées"
 
   date:
     abbr_day_names:

config/storage.yml

@@ -4,6 +4,9 @@ local:
 test:
   service: Disk
   root: <%= Rails.root.join("tmp/storage") %>
+proxied:
+  service: DsProxy
+  wrapped: openstack
 openstack:
   service: OpenStack
   container: "<%= ENV['FOG_ACTIVESTORAGE_DIRECTORY'] %>"

spec/features/admin/admin_creation_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 feature 'As an administrateur', js: true do
   let(:administration) { create(:administration) }
   let(:admin_email) { 'new_admin@gouv.fr' }
+  let(:new_admin) { Administrateur.find_by(email: admin_email) }
 
   before do
     perform_enqueued_jobs do
@@ -11,6 +12,8 @@ feature 'As an administrateur', js: true do
   end
 
   scenario 'I can register' do
+    expect(new_admin.reload.active?).to be(false)
+
     confirmation_email = open_email(admin_email)
     token_params = confirmation_email.body.match(/token=[^"]+/)
 
@@ -20,5 +23,7 @@ feature 'As an administrateur', js: true do
     click_button 'Continuer'
 
     expect(page).to have_content 'Mot de passe enregistré'
+
+    expect(new_admin.reload.active?).to be(true)
   end
 end

spec/features/routing/full_scenario_spec.rb

@@ -1,10 +1,11 @@
 require 'spec_helper'
 
 feature 'The routing' do
-  let(:procedure) { create(:procedure, :with_service, :for_individual) }
+  let(:password) { 'a very complicated password' }
+  let(:procedure) { create(:procedure, :with_type_de_champ, :with_service, :for_individual) }
   let(:administrateur) { create(:administrateur, procedures: [procedure]) }
-  let(:scientifique_user) { create(:user) }
-  let(:litteraire_user) { create(:user) }
+  let(:scientifique_user) { create(:user, password: password) }
+  let(:litteraire_user) { create(:user, password: password) }
 
   before { Flipper.enable_actor(:routage, administrateur.user) }
 
@@ -67,6 +68,9 @@ feature 'The routing' do
     click_on litteraire_user.email
     expect(page).to have_current_path(instructeur_dossier_path(procedure, litteraire_user.dossiers.first))
 
+    # follow the dossier
+    click_on 'Suivre le dossier'
+
     log_out
 
     # the scientifiques instructeurs only manage the scientifiques dossiers
@@ -74,9 +78,50 @@ feature 'The routing' do
     click_on procedure.libelle
     expect(page).not_to have_text(litteraire_user.email)
     expect(page).to have_text(scientifique_user.email)
+
+    # follow the dossier
+    click_on scientifique_user.email
+    click_on 'Suivre le dossier'
+
     log_out
 
-    # TODO: notifications tests
+    # litteraire_user change its dossier
+    visit root_path
+    click_on 'Connexion'
+    sign_in_with litteraire_user.email, password
+
+    click_on litteraire_user.dossiers.first.id
+    click_on 'Modifier mon dossier'
+
+    fill_in 'dossier_champs_attributes_0_value', with: 'some value'
+    click_on 'Enregistrer les modifications du dossier'
+    log_out
+
+    # the litteraires instructeurs should have a notification
+    visit root_path
+    click_on 'Connexion'
+    sign_in_with victor.user.email, password
+
+    ## on the procedures list
+    visit instructeur_procedures_path
+    expect(page).to have_css("span.notifications")
+
+    ## on the dossiers list
+    click_on procedure.libelle
+    expect(page).to have_css("span.notifications")
+
+    ## on the dossier it self
+    click_on 'suivi'
+    click_on litteraire_user.email
+    expect(page).to have_css("span.notifications")
+
+    log_out
+
+    # the scientifiques instructeurs should not have a notification
+    login_as marie.user, scope: :user
+    visit instructeur_procedures_path
+    expect(page).not_to have_css("span.notifications")
+    log_out
   end
 
   def publish_procedure(procedure)
@@ -113,7 +158,7 @@ feature 'The routing' do
     token_params = confirmation_email.body.match(/token=[^"]+/)
 
     visit "users/activate?#{token_params}"
-    fill_in :user_password, with: 'démarches-simplifiées-pwd'
+    fill_in :user_password, with: password
 
     click_button 'Définir le mot de passe'
 

spec/jobs/administrateurs/activate_before_expiration_job_spec.rb

@@ -2,7 +2,8 @@ require 'rails_helper'
 
 RSpec.describe Administrateurs::ActivateBeforeExpirationJob, type: :job do
   describe 'perform' do
-    let(:administrateur) { create(:administrateur, active: active) }
+    let(:administrateur) { create(:administrateur) }
+    let(:user) { administrateur.user }
     let(:mailer_double) { double('mailer', deliver_later: true) }
 
     subject { Administrateurs::ActivateBeforeExpirationJob.perform_now }
@@ -16,7 +17,7 @@ RSpec.describe Administrateurs::ActivateBeforeExpirationJob, type: :job do
     after { Timecop.return }
 
     context "with an inactive administrateur" do
-      let(:active) { false }
+      before { user.update(last_sign_in_at: nil) }
 
       context "created now" do
         before { subject }
@@ -43,7 +44,7 @@ RSpec.describe Administrateurs::ActivateBeforeExpirationJob, type: :job do
     end
 
     context "with an active administrateur" do
-      let(:active) { true }
+      before { user.update(last_sign_in_at: Time.zone.now) }
 
       context "created now" do
         before { subject }

spec/lib/tasks/deployment/20191007124230_update_admin_last_sign_in_at.rake_spec.rb

@@ -1,27 +0,0 @@
-describe '20191007124230_update_admin_last_sign_in_at.rake' do
-  let(:rake_task) { Rake::Task['after_party:update_admin_last_sign_in_at'] }
-
-  subject { rake_task.invoke }
-  after { rake_task.reenable }
-
-  context 'with 2 administrateurs' do
-    let!(:admin) { create(:administrateur, active: true) }
-    let(:user) { admin.user }
-    let!(:admin2) { create(:administrateur, active: false) }
-    let(:user2) { admin2.user }
-
-    before do
-    end
-
-    it do
-      expect(admin.active).to be true
-      expect(user.last_sign_in_at).to be_nil
-      expect(admin.updated_at).not_to be_nil
-
-      subject
-
-      expect(user.reload.last_sign_in_at).to eq(admin.reload.updated_at)
-      expect(user2.reload.last_sign_in_at).to be_nil
-    end
-  end
-end

spec/models/administrateur_spec.rb

@@ -50,4 +50,22 @@ describe Administrateur, type: :model do
   #     it { expect(subject).to eq([]) }
   #   end
   # end
+
+  describe '#active?' do
+    let!(:administrateur) { create(:administrateur) }
+
+    subject { administrateur.active? }
+
+    context 'when the user has never signed in' do
+      before { administrateur.user.update(last_sign_in_at: nil) }
+
+      it { is_expected.to be false }
+    end
+
+    context 'when the user has already signed in' do
+      before { administrateur.user.update(last_sign_in_at: Time.zone.now) }
+
+      it { is_expected.to be true }
+    end
+  end
 end

spec/models/concern/tags_substitution_concern_spec.rb

@@ -119,8 +119,8 @@ describe TagsSubstitutionConcern, type: :model do
       let(:types_de_champ) do
         [
           create(:type_de_champ_repetition, libelle: 'Répétition', types_de_champ: [
-            create(:type_de_champ_text, libelle: 'Nom'),
-            create(:type_de_champ_text, libelle: 'Prénom')
+            create(:type_de_champ_text, libelle: 'Nom', order_place: 1),
+            create(:type_de_champ_text, libelle: 'Prénom', order_place: 2)
           ])
         ]
       end

spec/models/instructeur_spec.rb

@@ -236,15 +236,16 @@ describe Instructeur, type: :model do
   end
 
   describe '#notifications_for_procedure' do
-    let!(:dossier) { create(:dossier, :followed, state: Dossier.states.fetch(:en_construction)) }
+    let(:procedure) { create(:simple_procedure, :routee, :with_type_de_champ_private) }
+    let!(:dossier) { create(:dossier, :followed, groupe_instructeur: procedure.groupe_instructeurs.last, state: Dossier.states.fetch(:en_construction)) }
     let(:instructeur) { dossier.follows.first.instructeur }
-    let(:procedure) { dossier.procedure }
-    let!(:instructeur_2) { create(:instructeur, groupe_instructeurs: [procedure.defaut_groupe_instructeur]) }
+    let!(:instructeur_2) { create(:instructeur, groupe_instructeurs: [procedure.groupe_instructeurs.last]) }
 
     let!(:dossier_on_procedure_2) { create(:dossier, :followed, state: Dossier.states.fetch(:en_construction)) }
     let!(:instructeur_on_procedure_2) { dossier_on_procedure_2.follows.first.instructeur }
 
     before do
+      procedure.groupe_instructeurs.last.instructeurs << instructeur
       instructeur_2.followed_dossiers << dossier
     end
 

spec/services/administrateur_usage_statistics_service_spec.rb

@@ -39,7 +39,6 @@ describe AdministrateurUsageStatisticsService do
       let(:administrateur) do
         create(:administrateur,
           user: create(:user, sign_in_count: 17, current_sign_in_at: Time.zone.local(2019, 3, 7), last_sign_in_at: Time.zone.local(2019, 2, 27)),
-          active: true,
           services: [create(:service)],
           instructeurs: [create(:instructeur)])
       end