Merge pull request #2727 from betagouv/fix-destroy

Fix scope for logo/notice/deliberation deletion

app/controllers/admin/procedures_controller.rb

@@ -2,7 +2,7 @@ class Admin::ProceduresController < AdminController
   include SmartListing::Helper::ControllerExtensions
   helper SmartListing::Helper
 
-  before_action :retrieve_procedure, only: [:show, :edit]
+  before_action :retrieve_procedure, only: [:show, :edit, :delete_logo, :delete_deliberation, :delete_notice]
 
   def index
     @procedures = smart_listing_create :procedures,
@@ -252,31 +252,25 @@ class Admin::ProceduresController < AdminController
   end
 
   def delete_logo
-    procedure = Procedure.find(params[:id])
+    @procedure.remove_logo!
-
+    @procedure.save
-    procedure.remove_logo!
-    procedure.save
 
     flash.notice = 'le logo a bien été supprimé'
-    redirect_to edit_admin_procedure_path(procedure)
+    redirect_to edit_admin_procedure_path(@procedure)
   end
 
   def delete_deliberation
-    procedure = Procedure.find(params[:id])
+    @procedure.deliberation.purge_later
-
-    procedure.deliberation.purge_later
 
     flash.notice = 'la délibération a bien été supprimée'
-    redirect_to edit_admin_procedure_path(procedure)
+    redirect_to edit_admin_procedure_path(@procedure)
   end
 
   def delete_notice
-    procedure = Procedure.find(params[:id])
+    @procedure.notice.purge_later
-
-    procedure.notice.purge_later
 
     flash.notice = 'la notice a bien été supprimée'
-    redirect_to edit_admin_procedure_path(procedure)
+    redirect_to edit_admin_procedure_path(@procedure)
   end
 
   private

spec/controllers/admin/procedures_controller_spec.rb

@@ -713,15 +713,53 @@ describe Admin::ProceduresController, type: :controller do
   end
 
   describe "DELETE #delete_deliberation" do
-    let(:procedure) { create(:procedure, :with_deliberation) }
+    context "with a demarche the admin owns" do
+      let(:procedure) { create(:procedure, :with_deliberation, administrateur: admin) }
 
-    before do
+      before do
-      delete :delete_deliberation, params: { id: procedure.id }
+        delete :delete_deliberation, params: { id: procedure.id }
-      procedure.reload
+        procedure.reload
+      end
+
+      it { expect(procedure.deliberation.attached?).to eq(false) }
+      it { expect(response).to redirect_to(edit_admin_procedure_path(procedure)) }
     end
 
-    it { expect(procedure.deliberation.attached?).to eq(false) }
+    context "with a demarche the admin does not own" do
-    it { expect(response).to redirect_to(edit_admin_procedure_path(procedure)) }
+      let(:procedure) { create(:procedure, :with_deliberation) }
+
+      before do
+        delete :delete_deliberation, params: { id: procedure.id }
+        procedure.reload
+      end
+
+      it { expect(response.status).to eq(404) }
+    end
+  end
+
+  describe "DELETE #delete_notice" do
+    context "with a demarche the admin owns" do
+      let(:procedure) { create(:procedure, :with_notice, administrateur: admin) }
+
+      before do
+        delete :delete_notice, params: { id: procedure.id }
+        procedure.reload
+      end
+
+      it { expect(procedure.notice.attached?).to eq(false) }
+      it { expect(response).to redirect_to(edit_admin_procedure_path(procedure)) }
+    end
+
+    context "with a demarche the admin does not own" do
+      let(:procedure) { create(:procedure, :with_notice) }
+
+      before do
+        delete :delete_notice, params: { id: procedure.id }
+        procedure.reload
+      end
+
+      it { expect(response.status).to eq(404) }
+    end
   end
 
   describe "GET #check_availability" do