From ff44b7a6002805ac221b7b1f447c27bf87fa6e1b Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 29 May 2019 12:05:28 +0200 Subject: [PATCH] Refactor purge pj to be more generic --- app/controllers/attachments_controller.rb | 7 +++ .../gestionnaires/dossiers_controller.rb | 7 --- app/controllers/users/dossiers_controller.rb | 12 +---- app/views/attachments/destroy.js.erb | 3 ++ .../gestionnaires/avis/instruction.html.haml | 2 +- .../purge_champ_piece_justificative.js.erb | 3 -- app/views/shared/attachment/_update.html.haml | 22 +++++--- .../_piece_justificative.html.haml | 25 +-------- .../purge_champ_piece_justificative.js.erb | 3 -- config/routes.rb | 9 +--- .../attachments_controller_spec.rb | 51 +++++++++++++++++++ .../gestionnaires/dossiers_controller_spec.rb | 40 --------------- .../users/dossiers_controller_spec.rb | 41 --------------- 13 files changed, 82 insertions(+), 143 deletions(-) create mode 100644 app/views/attachments/destroy.js.erb delete mode 100644 app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb delete mode 100644 app/views/users/dossiers/purge_champ_piece_justificative.js.erb create mode 100644 spec/controllers/attachments_controller_spec.rb diff --git a/app/controllers/attachments_controller.rb b/app/controllers/attachments_controller.rb index c2edd378c..93f86f718 100644 --- a/app/controllers/attachments_controller.rb +++ b/app/controllers/attachments_controller.rb @@ -6,4 +6,11 @@ class AttachmentsController < ApplicationController @attachment = @blob.attachments.find(params[:id]) @user_can_upload = params[:user_can_upload] end + + def destroy + attachment = @blob.attachments.find(params[:id]) + @attachment_id = attachment.id + attachment.purge_later + flash.now.notice = 'La pièce jointe a bien été supprimée.' + end end diff --git a/app/controllers/gestionnaires/dossiers_controller.rb b/app/controllers/gestionnaires/dossiers_controller.rb index 02512b173..7a36bd6d0 100644 --- a/app/controllers/gestionnaires/dossiers_controller.rb +++ b/app/controllers/gestionnaires/dossiers_controller.rb @@ -141,13 +141,6 @@ module Gestionnaires redirect_to annotations_privees_gestionnaire_dossier_path(procedure, dossier) end - def purge_champ_piece_justificative - @champ = dossier.champs_private.find(params[:champ_id]) - @champ.piece_justificative_file.purge_later - - flash.notice = 'La pièce jointe a bien été supprimée.' - end - def print @dossier = dossier render layout: "print" diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 420a81891..9e599a194 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -6,11 +6,11 @@ module Users layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret] ACTIONS_ALLOWED_TO_ANY_USER = [:index, :recherche, :new] - ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire, :purge_champ_piece_justificative] + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire] before_action :ensure_ownership!, except: ACTIONS_ALLOWED_TO_ANY_USER + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE before_action :ensure_ownership_or_invitation!, only: ACTIONS_ALLOWED_TO_OWNER_OR_INVITE - before_action :ensure_dossier_can_be_updated, only: [:update_identite, :update_brouillon, :modifier, :update, :purge_champ_piece_justificative] + before_action :ensure_dossier_can_be_updated, only: [:update_identite, :update_brouillon, :modifier, :update] before_action :forbid_invite_submission!, only: [:update_brouillon] before_action :forbid_closed_submission!, only: [:update_brouillon] before_action :show_demarche_en_test_banner @@ -236,14 +236,6 @@ module Users redirect_to url_for dossiers_path end - def purge_champ_piece_justificative - @champ = dossier.champs.find(params[:champ_id]) - - @champ.piece_justificative_file.purge_later - - flash.notice = 'La pièce jointe a bien été supprimée.' - end - def dossier_for_help dossier_id = params[:id] || params[:dossier_id] @dossier || (dossier_id.present? && Dossier.find_by(id: dossier_id.to_i)) diff --git a/app/views/attachments/destroy.js.erb b/app/views/attachments/destroy.js.erb new file mode 100644 index 000000000..ca79c22a1 --- /dev/null +++ b/app/views/attachments/destroy.js.erb @@ -0,0 +1,3 @@ +<%= render_flash(timeout: 5000, sticky: true) %> +<%= remove_element("#piece_justificative_#{@attachment_id}") %> +<%= show_element("#piece_justificative_file_#{@attachment_id}") %> diff --git a/app/views/gestionnaires/avis/instruction.html.haml b/app/views/gestionnaires/avis/instruction.html.haml index 25f03d299..dadee5f12 100644 --- a/app/views/gestionnaires/avis/instruction.html.haml +++ b/app/views/gestionnaires/avis/instruction.html.haml @@ -13,7 +13,7 @@ = form_for @avis, url: gestionnaire_avis_path(@avis), html: { class: 'form' } do |f| = f.text_area :answer, rows: 3, placeholder: 'Votre avis', required: true - = render partial: "shared/attachment/update", locals: { pj: @avis.piece_justificative_file, object: @avis, form: f } + = render partial: "shared/attachment/update", locals: { attachment: @avis.piece_justificative_file.attachment, user_can_destroy: true, form: f } .flex.justify-between.align-baseline %p.confidentiel.flex diff --git a/app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb b/app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb deleted file mode 100644 index 9d8807116..000000000 --- a/app/views/gestionnaires/dossiers/purge_champ_piece_justificative.js.erb +++ /dev/null @@ -1,3 +0,0 @@ -<%= render_flash(timeout: 5000, sticky: true) %> -<%= remove_element("#piece_justificative_#{@champ.id}") %> -<%= show_element("#champs_#{@champ.id}") %> diff --git a/app/views/shared/attachment/_update.html.haml b/app/views/shared/attachment/_update.html.haml index eb5c9dc83..ee86ddd7b 100644 --- a/app/views/shared/attachment/_update.html.haml +++ b/app/views/shared/attachment/_update.html.haml @@ -1,12 +1,22 @@ .piece-justificative - - if pj.attached? - .piece-justificative-actions{ id: "piece_justificative_#{object.id}" } + - if defined?(template) && template.attached? + %p.edit-pj-template.mb-1 + Veuillez télécharger, remplir et joindre + = link_to('le modèle suivant', url_for(template), target: '_blank', rel: 'noopener') + + - attachment_id = attachment ? attachment.id : SecureRandom.uuid + - user_can_destroy = defined?(user_can_destroy) ? user_can_destroy : false + - if attachment + .piece-justificative-actions{ id: "piece_justificative_#{attachment_id}" } .piece-justificative-action - = render partial: "shared/attachment/show", locals: { attachment: pj.attachment, user_can_upload: true } + = render partial: "shared/attachment/show", locals: { attachment: attachment, user_can_upload: true } + - if user_can_destroy + .piece-justificative-action + = link_to 'Supprimer', attachment_url(attachment.id, { signed_id: attachment.blob.signed_id }), remote: true, method: :delete, class: 'button small danger' .piece-justificative-action - = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#champs_#{object.id}" } + = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#piece_justificative_file_#{attachment_id}" } = form.file_field :piece_justificative_file, - id: "champs_#{object.id}", - class: "piece-justificative-input #{'hidden' if pj.attached?}", + id: "piece_justificative_file_#{attachment_id}", + class: "piece-justificative-input #{'hidden' if attachment}", direct_upload: true diff --git a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml index 346317669..3fefd0238 100644 --- a/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml +++ b/app/views/shared/dossiers/editable_champs/_piece_justificative.html.haml @@ -1,24 +1 @@ -- pj = champ.piece_justificative_file - -.piece-justificative - - if champ.type_de_champ.piece_justificative_template.attached? - %p.edit-pj-template.mb-1 - Veuillez télécharger, remplir et joindre - = link_to('le modèle suivant', url_for(champ.type_de_champ.piece_justificative_template), target: '_blank', rel: 'noopener') - - - if pj.attached? - .piece-justificative-actions{ id: "piece_justificative_#{champ.id}" } - .piece-justificative-action - = render partial: "shared/attachment/show", locals: { attachment: pj.attachment, user_can_upload: true } - .piece-justificative-action - - if champ.private? - = link_to 'Supprimer', gestionnaire_champ_purge_champ_piece_justificative_path(procedure_id: champ.dossier.procedure_id, dossier_id: champ.dossier_id, champ_id: champ.id), remote: true, method: :delete, class: 'button small danger' - - else - = link_to 'Supprimer', champ_purge_champ_piece_justificative_path(id: champ.dossier_id, champ_id: champ.id), remote: true, method: :delete, class: 'button small danger' - .piece-justificative-action - = button_tag 'Remplacer', type: 'button', class: 'button small', data: { 'toggle-target': "#champs_#{champ.id}" } - - = form.file_field :piece_justificative_file, - id: "champs_#{champ.id}", - class: "piece-justificative-input #{'hidden' if pj.attached?}", - direct_upload: true += render partial: "shared/attachment/update", locals: { attachment: champ.piece_justificative_file.attachment, template: champ.type_de_champ.piece_justificative_template, user_can_destroy: true, form: form } diff --git a/app/views/users/dossiers/purge_champ_piece_justificative.js.erb b/app/views/users/dossiers/purge_champ_piece_justificative.js.erb deleted file mode 100644 index 9d8807116..000000000 --- a/app/views/users/dossiers/purge_champ_piece_justificative.js.erb +++ /dev/null @@ -1,3 +0,0 @@ -<%= render_flash(timeout: 5000, sticky: true) %> -<%= remove_element("#piece_justificative_#{@champ.id}") %> -<%= show_element("#champs_#{@champ.id}") %> diff --git a/config/routes.rb b/config/routes.rb index 16c0d618a..da1c66ecc 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -134,6 +134,7 @@ Rails.application.routes.draw do end get 'attachments/:id', to: 'attachments#show', as: :attachment + delete 'attachments/:id', to: 'attachments#destroy' get 'tour-de-france' => 'root#tour_de_france' get "patron" => "root#patron" @@ -281,10 +282,6 @@ Rails.application.routes.draw do post 'commentaire' => 'dossiers#create_commentaire' post 'ask_deletion' get 'attestation' - - resources :champs, only: [] do - delete 'purge_champ_piece_justificative' => 'dossiers#purge_champ_piece_justificative' - end end collection do @@ -330,10 +327,6 @@ Rails.application.routes.draw do post 'send-to-instructeurs' => 'dossiers#send_to_instructeurs' post 'avis' => 'dossiers#create_avis' get 'print' => 'dossiers#print' - - resources :champs, only: [] do - delete 'purge_champ_piece_justificative' => 'dossiers#purge_champ_piece_justificative' - end end end end diff --git a/spec/controllers/attachments_controller_spec.rb b/spec/controllers/attachments_controller_spec.rb new file mode 100644 index 000000000..c9b732ca5 --- /dev/null +++ b/spec/controllers/attachments_controller_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe AttachmentsController, type: :controller do + let(:user) { create(:user) } + + describe '#destroy' do + render_views + + let(:attachment) { champ.piece_justificative_file.attachment } + let(:dossier) { create(:dossier, user: user) } + let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } + let(:signed_id) { attachment.blob.signed_id } + + subject do + delete :destroy, params: { id: attachment.id, signed_id: signed_id }, format: :js + end + + context "when authenticated" do + before { sign_in(user) } + + context 'and dossier is owned by user' do + it { is_expected.to have_http_status(200) } + + it do + subject + expect(champ.reload.piece_justificative_file.attached?).to be(false) + end + end + + context 'and signed_id is invalid' do + let(:signed_id) { 'yolo' } + + it { is_expected.to have_http_status(404) } + + it do + subject + expect(champ.reload.piece_justificative_file.attached?).to be(true) + end + end + end + + context 'when not authenticated' do + it { is_expected.to have_http_status(401) } + + it do + subject + expect(champ.reload.piece_justificative_file.attached?).to be(true) + end + end + end +end diff --git a/spec/controllers/gestionnaires/dossiers_controller_spec.rb b/spec/controllers/gestionnaires/dossiers_controller_spec.rb index ad7415874..570f6b3c4 100644 --- a/spec/controllers/gestionnaires/dossiers_controller_spec.rb +++ b/spec/controllers/gestionnaires/dossiers_controller_spec.rb @@ -469,44 +469,4 @@ describe Gestionnaires::DossiersController, type: :controller do it { expect(champ_repetition.champs.first.value).to eq('text') } it { expect(response).to redirect_to(annotations_privees_gestionnaire_dossier_path(dossier.procedure, dossier)) } end - - describe '#purge_champ_piece_justificative' do - before { sign_in(gestionnaire) } - - subject { delete :purge_champ_piece_justificative, params: { procedure_id: champ.dossier.procedure.id, dossier_id: champ.dossier.id, champ_id: champ.id }, format: :js } - - context 'when gestionnaire can process dossier' do - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id, private: true) } - - it { is_expected.to have_http_status(200) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(false) - end - - context 'but champ is not linked to this dossier' do - let(:champ) { create(:champ_piece_justificative, dossier: create(:dossier), private: true) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end - - context 'when gestionnaire cannot process dossier' do - let(:dossier) { create(:dossier, procedure: create(:procedure)) } - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id, private: true) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end end diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index 351e2fae7..9d68d2003 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -919,47 +919,6 @@ describe Users::DossiersController, type: :controller do end end - describe '#purge_champ_piece_justificative' do - before { sign_in(user) } - - subject { delete :purge_champ_piece_justificative, params: { id: champ.dossier.id, champ_id: champ.id }, format: :js } - - context 'when dossier is owned by user' do - let(:dossier) { create(:dossier, user: user) } - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } - - it { is_expected.to have_http_status(200) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(false) - end - - context 'but champ is not linked to this dossier' do - let(:champ) { create(:champ_piece_justificative, dossier: create(:dossier)) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end - - context 'when dossier is not owned by user' do - let(:dossier) { create(:dossier, user: create(:user)) } - let(:champ) { create(:champ_piece_justificative, dossier_id: dossier.id) } - - it { is_expected.to redirect_to(root_path) } - - it do - subject - expect(champ.reload.piece_justificative_file.attached?).to be(true) - end - end - end - describe "#dossier_for_help" do before do sign_in(user)