Merge pull request #4456 from betagouv/dev

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

Gemfile.lock

@@ -1,14 +1,3 @@
-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
@@ -75,6 +64,11 @@ 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)
@@ -214,7 +208,7 @@ GEM
     ethon (0.11.0)
       ffi (>= 1.3.0)
     eventmachine (1.2.7)
-    excon (0.62.0)
+    excon (0.68.0)
     execjs (2.7.0)
     factory_bot (4.11.1)
       activesupport (>= 3.0.0)
@@ -238,7 +232,7 @@ GEM
     fog-json (1.2.0)
       fog-core
       multi_json (~> 1.10)
-    fog-openstack (1.0.6)
+    fog-openstack (1.0.10)
       fog-core (~> 2.1)
       fog-json (>= 1.0)
       ipaddress (>= 0.8)
@@ -325,7 +319,7 @@ GEM
       rails-dom-testing (>= 1, < 3)
       railties (>= 4.2.0)
       thor (>= 0.14, < 2.0)
-    json (2.1.0)
+    json (2.2.0)
     json-jwt (1.10.0)
       activesupport (>= 4.2)
       aes_key_wrap
@@ -375,16 +369,16 @@ GEM
     marcel (0.3.3)
       mimemagic (~> 0.3.2)
     method_source (0.9.2)
-    mime-types (3.2.2)
+    mime-types (3.3)
       mime-types-data (~> 3.2015)
-    mime-types-data (3.2018.0812)
+    mime-types-data (3.2019.1009)
     mimemagic (0.3.3)
     mini_mime (1.0.2)
     mini_portile2 (2.4.0)
     minitest (5.11.3)
     momentjs-rails (2.20.1)
       railties (>= 3.1)
-    multi_json (1.13.1)
+    multi_json (1.14.1)
     multi_xml (0.6.0)
     multipart-post (2.0.0)
     mustermann (1.0.3)
@@ -425,7 +419,7 @@ GEM
       validate_email
       validate_url
       webfinger (>= 1.0.1)
-    openstack (3.3.20)
+    openstack (3.3.21)
       json
     orm_adapter (0.5.0)
     parallel (1.12.1)
@@ -717,7 +711,7 @@ DEPENDENCIES
   actiontext!
   active_link_to
   active_model_serializers
-  activestorage-openstack!
+  activestorage-openstack
   administrate
   after_party
   anchored

app/controllers/instructeurs/groupe_instructeurs_controller.rb

@@ -0,0 +1,98 @@
+module Instructeurs
+  class GroupeInstructeursController < InstructeurController
+    ITEMS_PER_PAGE = 25
+
+    def index
+      @procedure = procedure
+      @groupes_instructeurs = paginated_groupe_instructeurs
+    end
+
+    def show
+      @procedure = procedure
+      @groupe_instructeur = groupe_instructeur
+      @instructeurs = paginated_instructeurs
+    end
+
+    def add_instructeur
+      @instructeur = Instructeur.find_by(email: instructeur_email) ||
+        create_instructeur(instructeur_email)
+
+      if groupe_instructeur.instructeurs.include?(@instructeur)
+        flash[:alert] = "L’instructeur « #{instructeur_email} » est déjà dans le groupe."
+
+      else
+        groupe_instructeur.instructeurs << @instructeur
+        flash[:notice] = "L’instructeur « #{instructeur_email} » a été affecté au groupe."
+        GroupeInstructeurMailer
+          .add_instructeur(groupe_instructeur, @instructeur, current_user.email)
+          .deliver_later
+      end
+
+      redirect_to instructeur_groupe_path(procedure, groupe_instructeur)
+    end
+
+    def remove_instructeur
+      if groupe_instructeur.instructeurs.one?
+        flash[:alert] = "Suppression impossible : il doit y avoir au moins un instructeur dans le groupe"
+
+      else
+        @instructeur = Instructeur.find(instructeur_id)
+        groupe_instructeur.instructeurs.destroy(@instructeur)
+        flash[:notice] = "L’instructeur « #{@instructeur.email} » a été retiré du groupe."
+        GroupeInstructeurMailer
+          .remove_instructeur(groupe_instructeur, @instructeur, current_user.email)
+          .deliver_later
+      end
+
+      redirect_to instructeur_groupe_path(procedure, groupe_instructeur)
+    end
+
+    private
+
+    def create_instructeur(email)
+      user = User.create_or_promote_to_instructeur(
+        email,
+        SecureRandom.hex,
+        administrateurs: [procedure.administrateurs.first]
+      )
+      user.invite!
+      user.instructeur
+    end
+
+    def procedure
+      current_instructeur
+        .procedures
+        .includes(:groupe_instructeurs)
+        .find(params[:procedure_id])
+    end
+
+    def groupe_instructeur
+      current_instructeur.groupe_instructeurs.find(params[:id])
+    end
+
+    def paginated_groupe_instructeurs
+      current_instructeur
+        .groupe_instructeurs
+        .where(procedure: procedure)
+        .page(params[:page])
+        .per(ITEMS_PER_PAGE)
+        .order(:label)
+    end
+
+    def paginated_instructeurs
+      groupe_instructeur
+        .instructeurs
+        .page(params[:page])
+        .per(ITEMS_PER_PAGE)
+        .order(:email)
+    end
+
+    def instructeur_email
+      params[:instructeur][:email].strip.downcase
+    end
+
+    def instructeur_id
+      params[:instructeur][:id]
+    end
+  end
+end

app/lib/active_storage/service/ds_proxy_service.rb

@@ -1,57 +0,0 @@
-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/views/instructeurs/groupe_instructeurs/index.html.haml

@@ -0,0 +1,20 @@
+- content_for(:title, "Notifications pour #{@procedure.libelle}")
+
+= render partial: 'new_administrateur/breadcrumbs',
+  locals: { steps: [link_to(@procedure.libelle, procedure_path(@procedure)),
+                    'Groupes d’instructeurs'] }
+
+.container.groupe-instructeur
+  .card
+    .card-title Gestion des Groupes
+    %table.table.mt-2
+      %thead
+        %tr
+          %th{ colspan: 2 } Liste des groupes
+      %tbody
+        - @groupes_instructeurs.each do |group|
+          %tr
+            %td= group.label
+            %td.actions= link_to "voir", instructeur_groupe_path(@procedure, group)
+
+    = paginate @groupes_instructeurs

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

@@ -0,0 +1,37 @@
+- content_for(:title, "Instructeurs du groupe #{@groupe_instructeur.label}")
+
+= render partial: 'new_administrateur/breadcrumbs',
+  locals: { steps: [link_to(@procedure.libelle, procedure_path(@procedure)),
+                    link_to('Groupes d’instructeurs', instructeur_groupes_path(@procedure)),
+                    @groupe_instructeur.label] }
+
+.container.groupe-instructeur
+  %h1 Groupe « #{@groupe_instructeur.label} »
+
+  .card.mt-2
+    .card-title Gestion des instructeurs
+    = form_for :instructeur,
+      url: { action: :add_instructeur },
+      html: { class: 'form' } do |f|
+
+      = f.label :email do
+        Affecter un nouvel instructeur
+      = f.email_field :email, placeholder: 'marie.dupont@exemple.fr', required: true
+      = f.submit 'Affecter', class: 'button primary send'
+
+    %table.table.mt-2
+      %thead
+        %tr
+          %th{ colspan: 2 } Instructeurs affectés
+      %tbody
+        - @instructeurs.each do |instructeur|
+          %tr
+            %td= instructeur.email
+            %td.actions= button_to 'retirer',
+              { action: :remove_instructeur },
+              { method: :delete,
+                data: { confirm: "Êtes-vous sûr de vouloir retirer l’instructeur « #{instructeur.email} » du groupe  « #{@groupe_instructeur.label} » ?" },
+                params: { instructeur: { id: instructeur.id }},
+                class: 'button' }
+
+    = paginate @instructeurs

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

@@ -13,6 +13,12 @@
         |
         = link_to 'statistiques', stats_instructeur_procedure_path(@procedure), class: 'header-link', data: { turbolinks: false } # Turbolinks disabled for Chartkick. See Issue #350
 
+        - if @procedure.routee?
+          |
+          - if current_administrateur.present? && current_administrateur.owns?(@procedure)
+            = link_to 'instructeurs', procedure_groupe_instructeurs_path(@procedure), class: 'header-link'
+          - else
+            = link_to 'instructeurs', instructeur_groupes_path(@procedure), class: 'header-link'
 
         %ul.tabs
           = tab_item('à suivre',

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

@@ -5,9 +5,7 @@
                     @groupe_instructeur.label] }
 
 .container.groupe-instructeur
-  .rename_form_block
-    .flex.baseline-start
-      %h1 Groupe « #{@groupe_instructeur.label} »
+  %h1 Groupe « #{@groupe_instructeur.label} »
 
   .card.mt-2
     = form_for @groupe_instructeur,

config/env.example

@@ -24,6 +24,7 @@ FOG_OPENSTACK_REGION=""
 FOG_DIRECTORY=""
 FOG_ENABLED=""
 CARRIERWAVE_CACHE_DIR="/tmp/tps-local-cache"
+DS_PROXY_URL=""
 
 FC_PARTICULIER_ID=""
 FC_PARTICULIER_SECRET=""

config/environments/development.rb

@@ -22,7 +22,7 @@ Rails.application.configure do
   # Don't care if the mailer can't send.
   config.action_mailer.raise_delivery_errors = false
 
-  config.active_storage.service = :local
+  config.active_storage.service = ENV['FOG_ENABLED'] == 'enabled' ? :openstack : :local
 
   # Print deprecation notices to the Rails logger.
   config.active_support.deprecation = :log

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 = :proxied
+  config.active_storage.service = :openstack
 
   # Send deprecation notices to registered listeners.
   config.active_support.deprecation = :notify

config/initializers/active_storage.rb

@@ -7,3 +7,33 @@ 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 = "#{ENV['DS_PROXY_URL']}/"
+      url.gsub(search, replace)
+    end
+  end
+end

config/routes.rb

@@ -288,6 +288,13 @@ Rails.application.routes.draw do
   scope module: 'instructeurs', as: 'instructeur' do
     resources :procedures, only: [:index, :show], param: :procedure_id do
       member do
+        resources :groupes, only: [:index, :show], controller: 'groupe_instructeurs' do
+          member do
+            post 'add_instructeur'
+            delete 'remove_instructeur'
+          end
+        end
+
         patch 'update_displayed_fields'
         get 'update_sort/:table/:column' => 'procedures#update_sort', as: 'update_sort'
         post 'add_filter'

config/storage.yml

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

spec/controllers/instructeurs/groupe_instructeurs_controller_spec.rb

@@ -0,0 +1,100 @@
+describe Instructeurs::GroupeInstructeursController, type: :controller do
+  render_views
+
+  let(:instructeur) { create(:instructeur) }
+  let(:procedure) { create(:procedure, :published) }
+  let!(:gi_1_1) { procedure.defaut_groupe_instructeur }
+  let!(:gi_1_2) { procedure.groupe_instructeurs.create(label: 'groupe instructeur 2') }
+
+  let(:procedure2) { create(:procedure, :published) }
+  let!(:gi_2_2) { procedure2.groupe_instructeurs.create(label: 'groupe instructeur 2 2') }
+
+  before do
+    gi_1_2.instructeurs << instructeur
+    sign_in(instructeur.user)
+  end
+
+  describe '#index' do
+    context 'of a procedure I own' do
+      before do
+        get :index, params: { procedure_id: procedure.id }
+      end
+
+      context 'when a procedure has multiple groups' do
+        it { expect(response).to have_http_status(:ok) }
+        it { expect(response.body).to include(gi_1_2.label) }
+        it { expect(response.body).not_to include(gi_1_1.label) }
+        it { expect(response.body).not_to include(gi_2_2.label) }
+      end
+    end
+  end
+
+  describe '#show' do
+    context 'of a group I belong to' do
+      before { get :show, params: { procedure_id: procedure.id, id: gi_1_2.id } }
+
+      it { expect(response).to have_http_status(:ok) }
+    end
+  end
+
+  describe '#add_instructeur' do
+    before do
+      post :add_instructeur,
+        params: {
+          procedure_id: procedure.id,
+          id: gi_1_2.id,
+          instructeur: { email: new_instructeur_email }
+        }
+    end
+
+    context 'of a new instructeur' do
+      let(:new_instructeur_email) { 'new_instructeur@mail.com' }
+
+      it { expect(gi_1_2.instructeurs.pluck(:email)).to include(new_instructeur_email) }
+      it { expect(flash.notice).to be_present }
+      it { expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) }
+    end
+
+    context 'of an instructeur already in the group' do
+      let(:new_instructeur_email) { instructeur.email }
+
+      it { expect(flash.alert).to be_present }
+      it { expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_2)) }
+    end
+  end
+
+  describe '#remove_instructeur' do
+    let!(:new_instructeur) { create(:instructeur) }
+
+    before { gi_1_1.instructeurs << instructeur << new_instructeur }
+
+    def remove_instructeur(instructeur)
+      delete :remove_instructeur,
+        params: {
+          procedure_id: procedure.id,
+          id: gi_1_1.id,
+          instructeur: { id: instructeur.id }
+        }
+    end
+
+    context 'when there are many instructeurs' do
+      before { remove_instructeur(new_instructeur) }
+
+      it { expect(gi_1_1.instructeurs).to include(instructeur) }
+      it { expect(gi_1_1.reload.instructeurs.count).to eq(1) }
+      it { expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_1)) }
+    end
+
+    context 'when there is only one instructeur' do
+      before do
+        remove_instructeur(new_instructeur)
+        remove_instructeur(instructeur)
+      end
+
+      it { expect(gi_1_1.instructeurs).to include(instructeur) }
+      it { expect(gi_1_1.instructeurs.count).to eq(1) }
+      it { expect(flash.alert).to eq('Suppression impossible : il doit y avoir au moins un instructeur dans le groupe') }
+      it { expect(response).to redirect_to(instructeur_groupe_path(procedure, gi_1_1)) }
+    end
+  end
+end

spec/controllers/new_administrateur/groupe_instructeurs_controller_spec.rb

@@ -118,17 +118,17 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do
 
     before { gi_1_1.instructeurs << admin.instructeur << instructeur }
 
-    def remove_instructeur(email)
+    def remove_instructeur(instructeur)
       delete :remove_instructeur,
         params: {
           procedure_id: procedure.id,
           id: gi_1_1.id,
-          instructeur: { id: admin.instructeur.id }
+          instructeur: { id: instructeur.id }
         }
     end
 
     context 'when there are many instructeurs' do
-      before { remove_instructeur(admin.user.email) }
+      before { remove_instructeur(admin.instructeur) }
 
       it { expect(gi_1_1.instructeurs).to include(instructeur) }
       it { expect(gi_1_1.reload.instructeurs.count).to eq(1) }
@@ -137,8 +137,8 @@ describe NewAdministrateur::GroupeInstructeursController, type: :controller do
 
     context 'when there is only one instructeur' do
       before do
-        remove_instructeur(admin.user.email)
-        remove_instructeur(instructeur.email)
+        remove_instructeur(admin.instructeur)
+        remove_instructeur(instructeur)
       end
 
       it { expect(gi_1_1.instructeurs).to include(instructeur) }