Merge pull request #1702 from betagouv/frederic/fix_1677-view_invite_as_usager_instructeur

#1677 allow invites to edit dossiers

app/controllers/new_user/dossiers_controller.rb

@@ -1,6 +1,8 @@
 module NewUser
   class DossiersController < UserController
-    before_action :ensure_ownership!, except: [:index]
+    before_action :ensure_ownership!, except: [:index, :modifier, :update]
+    before_action :ensure_ownership_or_invitation!, only: [:modifier, :update]
+    before_action :forbid_invite_submission!, only: [:update]
 
     def attestation
       send_data(dossier.attestation.pdf.read, filename: 'attestation.pdf', type: 'application/pdf')
@@ -67,15 +69,14 @@ module NewUser
       elsif draft?
         flash.now.notice = 'Votre brouillon a bien été sauvegardé.'
         render :modifier
+      elsif @dossier.brouillon?
+        @dossier.en_construction!
+        NotificationMailer.send_notification(@dossier, @dossier.procedure.initiated_mail_template).deliver_now!
+        redirect_to merci_dossier_path(@dossier)
+      elsif owns_dossier?
+        redirect_to users_dossier_recapitulatif_path(@dossier)
       else
-        if @dossier.brouillon?
-          @dossier.en_construction!
-          NotificationMailer.send_notification(@dossier, @dossier.procedure.initiated_mail_template).deliver_now!
-          redirect_to merci_dossier_path(@dossier)
-        else
-          @dossier.en_construction!
-          redirect_to users_dossier_recapitulatif_path(@dossier)
-        end
+        redirect_to users_dossiers_invite_path(@dossier.invite_for_user(current_user))
       end
     end
 
@@ -99,16 +100,32 @@ module NewUser
     end
 
     def dossier_with_champs
-      @dossier_with_champs ||= current_user.dossiers.with_ordered_champs.find(params[:id])
+      @dossier_with_champs ||= Dossier.with_ordered_champs.find(params[:id])
     end
 
     def ensure_ownership!
-      if dossier.user_id != current_user.id
-        flash[:alert] = "Vous n'avez pas accès à ce dossier"
-        redirect_to root_path
+      if !owns_dossier?
+        forbidden!
       end
     end
 
+    def ensure_ownership_or_invitation!
+      if !dossier.owner_or_invite?(current_user)
+        forbidden!
+      end
+    end
+
+    def forbid_invite_submission!
+      if passage_en_construction? && !owns_dossier?
+        forbidden!
+      end
+    end
+
+    def forbidden!
+      flash[:alert] = "Vous n'avez pas accès à ce dossier"
+      redirect_to root_path
+    end
+
     def individual_params
       params.require(:individual).permit(:gender, :nom, :prenom, :birthdate)
     end
@@ -117,6 +134,14 @@ module NewUser
       params.require(:dossier).permit(:autorisation_donnees)
     end
 
+    def owns_dossier?
+      dossier.user_id == current_user.id
+    end
+
+    def passage_en_construction?
+      dossier.brouillon? && !draft?
+    end
+
     def draft?
       params[:submit_action] == 'draft'
     end

app/controllers/users_controller.rb

@@ -10,9 +10,11 @@ class UsersController < ApplicationController
 
     dossier = Dossier.find(dossier_id)
 
-    return dossier if dossier.owner?(current_user.email) || dossier.invite_by_user?(current_user.email)
+    if !dossier.owner_or_invite?(current_user)
+      raise ActiveRecord::RecordNotFound
+    end
 
-    raise ActiveRecord::RecordNotFound
+    dossier
   end
 
   def authorized_routes?(controller)

app/models/dossier.rb

@@ -190,12 +190,12 @@ class Dossier < ApplicationRecord
     en_instruction? || accepte? || refuse? || sans_suite?
   end
 
-  def owner?(email)
-    user.email == email
+  def owner_or_invite?(user)
+    self.user == user || invite_for_user(user).present?
   end
 
-  def invite_by_user?(email)
-    (invites_user.pluck :email).include? email
+  def invite_for_user(user)
+    invites_user.find_by(user_id: user.id)
   end
 
   def can_be_en_construction?

app/views/dossiers/_edit_carto.html.haml

@@ -1,5 +1,5 @@
 - if !@facade.dossier.read_only?
-  - if user_signed_in? && (@facade.dossier.owner?(current_user.email) || @facade.dossier.invite_by_user?(current_user.email))
+  - if user_signed_in? && (@facade.dossier.owner_or_invite?(current_user))
     %a#maj_carte.action{ href: "/users/dossiers/#{@facade.dossier.id}/carte" }
       .col-lg-2.col-md-2.col-sm-2.col-xs-2.action
         = 'éditer'.upcase

app/views/dossiers/_edit_dossier.html.haml

@@ -1,5 +1,5 @@
 - if !@facade.dossier.read_only?
-  - if user_signed_in? && (@facade.dossier.owner?(current_user.email) || @facade.dossier.invite_by_user?(current_user.email))
+  - if user_signed_in? && (@facade.dossier.owner_or_invite?(current_user))
     = link_to modifier_dossier_path(@facade.dossier), class: 'action', id: 'maj_infos' do
       #edit-dossier.col-lg-2.col-md-2.col-sm-2.col-xs-2.action
         = "éditer".upcase

app/views/dossiers/_edit_pieces_jointes.html.haml

@@ -1,5 +1,5 @@
 - if !@facade.dossier.read_only?
-  - if user_signed_in? && (@facade.dossier.owner?(current_user.email) || @facade.dossier.invite_by_user?(current_user.email))
+  - if user_signed_in? && (@facade.dossier.owner_or_invite?(current_user))
     - if @facade.procedure.cerfa_flag? || @facade.dossier.types_de_piece_justificative.size > 0
       .col-lg-4.col-md-4.col-sm-4.col-xs-4.action
         %a#maj_pj.action{ "data-target" => "#upload-pj-modal",

app/views/dossiers/_infos_dossier.html.haml

@@ -101,7 +101,7 @@
                     Pièce non fournie
 
     - if !@facade.dossier.read_only?
-      - if user_signed_in? && (@facade.dossier.owner?(current_user.email) || @facade.dossier.invite_by_user?(current_user.email))
+      - if user_signed_in? && (@facade.dossier.owner_or_invite?(current_user))
         - if @facade.procedure.cerfa_flag? || @facade.dossier.types_de_piece_justificative.size > 0
           .row
             .col-xs-4

app/views/new_user/dossiers/modifier.html.haml

@@ -52,9 +52,10 @@
             class: 'button send',
             data: { action: 'draft', disable_with: 'Envoi...' }
 
-          = f.button 'Soumettre le dossier',
-            class: 'button send primary',
-            data: { action: 'submit', disable_with: 'Envoi...' }
+          - if @dossier.user == current_user
+            = f.button 'Soumettre le dossier',
+              class: 'button send primary',
+              data: { action: 'submit', disable_with: 'Envoi...' }
 
         - else
           = f.button 'Modifier le dossier',

spec/controllers/new_user/dossiers_controller_spec.rb

@@ -3,44 +3,96 @@ require 'spec_helper'
 describe NewUser::DossiersController, type: :controller do
   let(:user) { create(:user) }
 
-  describe 'before_action: ensure_ownership!' do
-    it 'is present' do
+  describe 'before_actions' do
+    it 'are present' do
       before_actions = NewUser::DossiersController
         ._process_action_callbacks
         .find_all{ |process_action_callbacks| process_action_callbacks.kind == :before }
         .map(&:filter)
 
-      expect(before_actions).to include(:ensure_ownership!)
+      expect(before_actions).to include(:ensure_ownership!, :ensure_ownership_or_invitation!, :forbid_invite_submission!)
     end
   end
 
-  describe 'ensure_ownership!' do
+  shared_examples_for 'does not redirect nor flash' do
+    before { @controller.send(ensure_authorized) }
+
+    it { expect(@controller).not_to have_received(:redirect_to) }
+    it { expect(flash.alert).to eq(nil) }
+  end
+
+  shared_examples_for 'redirects and flashes' do
+    before { @controller.send(ensure_authorized) }
+
+    it { expect(@controller).to have_received(:redirect_to).with(root_path) }
+    it { expect(flash.alert).to eq("Vous n'avez pas accès à ce dossier") }
+  end
+
+  describe '#ensure_ownership!' do
     let(:user) { create(:user) }
+    let(:asked_dossier) { create(:dossier) }
+    let(:ensure_authorized) { :ensure_ownership! }
 
     before do
       @controller.params = @controller.params.merge(dossier_id: asked_dossier.id)
       expect(@controller).to receive(:current_user).and_return(user)
       allow(@controller).to receive(:redirect_to)
-
-      @controller.send(:ensure_ownership!)
     end
 
-    context 'when a user asks for its dossier' do
+    context 'when a user asks for their own dossier' do
       let(:asked_dossier) { create(:dossier, user: user) }
 
-      it 'does not redirects nor flash' do
-        expect(@controller).not_to have_received(:redirect_to)
-        expect(flash.alert).to eq(nil)
-      end
+      it_behaves_like 'does not redirect nor flash'
     end
 
     context 'when a user asks for another dossier' do
-      let(:asked_dossier) { create(:dossier) }
+      it_behaves_like 'redirects and flashes'
+    end
 
-      it 'redirects and flash' do
-        expect(@controller).to have_received(:redirect_to).with(root_path)
-        expect(flash.alert).to eq("Vous n'avez pas accès à ce dossier")
-      end
+    context 'when an invite asks for a dossier where they were invited' do
+      before { create(:invite, dossier: asked_dossier, user: user, type: 'InviteUser') }
+
+      it_behaves_like 'redirects and flashes'
+    end
+
+    context 'when an invite asks for another dossier' do
+      before { create(:invite, dossier: create(:dossier), user: user, type: 'InviteUser') }
+
+      it_behaves_like 'redirects and flashes'
+    end
+  end
+
+  describe '#ensure_ownership_or_invitation!' do
+    let(:user) { create(:user) }
+    let(:asked_dossier) { create(:dossier) }
+    let(:ensure_authorized) { :ensure_ownership_or_invitation! }
+
+    before do
+      @controller.params = @controller.params.merge(dossier_id: asked_dossier.id)
+      expect(@controller).to receive(:current_user).and_return(user)
+      allow(@controller).to receive(:redirect_to)
+    end
+
+    context 'when a user asks for their own dossier' do
+      let(:asked_dossier) { create(:dossier, user: user) }
+
+      it_behaves_like 'does not redirect nor flash'
+    end
+
+    context 'when a user asks for another dossier' do
+      it_behaves_like 'redirects and flashes'
+    end
+
+    context 'when an invite asks for a dossier where they were invited' do
+      before { create(:invite, dossier: asked_dossier, user: user, type: 'InviteUser') }
+
+      it_behaves_like 'does not redirect nor flash'
+    end
+
+    context 'when an invite asks for another dossier' do
+      before { create(:invite, dossier: create(:dossier), user: user, type: 'InviteUser') }
+
+      it_behaves_like 'redirects and flashes'
     end
   end
 
@@ -242,5 +294,43 @@ describe NewUser::DossiersController, type: :controller do
         expect(response).to redirect_to(merci_dossier_path(dossier))
       end
     end
+
+    context 'when the user has an invitation but is not the owner' do
+      let(:dossier) { create(:dossier) }
+      let!(:invite) { create(:invite, dossier: dossier, user: user, type: 'InviteUser') }
+
+      context 'and the invite saves a draft' do
+        let(:payload) { submit_payload.merge(submit_action: 'draft') }
+
+        before do
+          first_champ.type_de_champ.update(mandatory: true, libelle: 'l')
+          allow(PiecesJustificativesService).to receive(:missing_pj_error_messages).and_return(['pj'])
+
+          subject
+        end
+
+        it { expect(response).to render_template(:modifier) }
+        it { expect(flash.notice).to eq('Votre brouillon a bien été sauvegardé.') }
+        it { expect(dossier.reload.state).to eq('brouillon') }
+      end
+
+      context 'and the invite tries to submit the dossier' do
+        before { subject }
+
+        it { expect(response).to redirect_to(root_path) }
+        it { expect(flash.alert).to eq("Vous n'avez pas accès à ce dossier") }
+      end
+
+      context 'and the invite updates a dossier en constructions' do
+        before do
+          dossier.en_construction!
+          subject
+        end
+
+        it { expect(first_champ.reload.value).to eq('beautiful value') }
+        it { expect(dossier.reload.state).to eq('en_construction') }
+        it { expect(response).to redirect_to(users_dossiers_invite_path(invite)) }
+      end
+    end
   end
 end

spec/models/dossier_spec.rb

@@ -404,28 +404,39 @@ describe Dossier do
     end
   end
 
-  describe '#invite_by_user?' do
-    let(:dossier) { create :dossier }
-    let(:invite_user) { create :user, email: user_invite_email }
-    let(:invite_gestionnaire) { create :user, email: gestionnaire_invite_email }
-    let(:user_invite_email) { 'plup@plop.com' }
-    let(:gestionnaire_invite_email) { 'plap@plip.com' }
+  describe '#owner_or_invite?' do
+    let(:owner) { create(:user) }
+    let(:dossier) { create(:dossier, user: owner) }
+    let(:invite_user) { create(:user) }
+    let(:invite_gestionnaire) { create(:user) }
 
     before do
-      create :invite, dossier: dossier, user: invite_user, email: invite_user.email, type: 'InviteUser'
-      create :invite, dossier: dossier, user: invite_gestionnaire, email: invite_gestionnaire.email, type: 'InviteGestionnaire'
+      create(:invite, dossier: dossier, user: invite_user, type: 'InviteUser')
+      create(:invite, dossier: dossier, user: invite_gestionnaire, type: 'InviteGestionnaire')
     end
 
-    subject { dossier.invite_by_user? email }
+    subject { dossier.owner_or_invite?(user) }
 
-    context 'when email is present on invite list' do
-      let(:email) { user_invite_email }
+    context 'when user is owner' do
+      let(:user) { owner }
 
       it { is_expected.to be_truthy }
     end
 
-    context 'when email is present on invite list' do
-      let(:email) { gestionnaire_invite_email }
+    context 'when user was invited by user' do
+      let(:user) { invite_user }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when user was invited by gestionnaire (legacy, no new invitations happen)' do
+      let(:user) { invite_gestionnaire }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when user is quidam' do
+      let(:user) { create(:user) }
 
       it { is_expected.to be_falsey }
     end