Fix 500 error when trying to unsubscribe from an already deleted entity

This change fixes 500 error when trying to unsubscribe
from an already deleted entity by checking if the noteable exists
before rendering the unsubscribe page.

app/controllers/sent_notifications_controller.rb

@@ -6,14 +6,24 @@ class SentNotificationsController < ApplicationController
   def unsubscribe
     @sent_notification = SentNotification.for(params[:id])
 
-    return render_404 unless @sent_notification && @sent_notification.unsubscribable?
+    return render_404 unless unsubscribe_prerequisites_met?
+
     return unsubscribe_and_redirect if current_user || params[:force]
   end
 
   private
 
+  def unsubscribe_prerequisites_met?
+    @sent_notification.present? &&
+    @sent_notification.unsubscribable? &&
+    noteable.present?
+  end
+
+  def noteable
+    @sent_notification.noteable
+  end
+
   def unsubscribe_and_redirect
-    noteable = @sent_notification.noteable
     noteable.unsubscribe(@sent_notification.recipient, @sent_notification.project)
 
     flash[:notice] = _("You have been unsubscribed from this thread.")

changelogs/unreleased/199034-nomethoderror-undefined-method-unsubscribe-for-nil-nilclass.yml

+---
+title: Fix 500 error when trying to unsubscribe from an already deleted entity
+merge_request: 23747
+author:
+type: fixed

spec/controllers/sent_notifications_controller_spec.rb

@@ -30,6 +30,16 @@ describe SentNotificationsController do
   let(:target_project) { project }
 
   describe 'GET unsubscribe' do
+    shared_examples 'returns 404' do
+      it 'does not set the flash message' do
+        expect(controller).not_to set_flash[:notice]
+      end
+
+      it 'returns a 404' do
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
     context 'when the user is not logged in' do
       context 'when the force param is passed' do
         before do
@@ -156,6 +166,16 @@ describe SentNotificationsController do
           end
         end
       end
+
+      context 'when the noteable associated to the notification has been deleted' do
+        before do
+          sent_notification.noteable.destroy!
+
+          get(:unsubscribe, params: { id: sent_notification.reply_key })
+        end
+
+        it_behaves_like 'returns 404'
+      end
     end
 
     context 'when the user is logged in' do
@@ -168,17 +188,7 @@ describe SentNotificationsController do
           get(:unsubscribe, params: { id: sent_notification.reply_key.reverse })
         end
 
-        it 'does not unsubscribe the user' do
-          expect(issue.subscribed?(user, project)).to be_truthy
-        end
-
-        it 'does not set the flash message' do
-          expect(controller).not_to set_flash[:notice]
-        end
-
-        it 'returns a 404' do
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
+        it_behaves_like 'returns 404'
       end
 
       context 'when the force param is passed' do
@@ -254,6 +264,16 @@ describe SentNotificationsController do
           end
         end
       end
+
+      context 'when the noteable associated to the notification has been deleted' do
+        before do
+          sent_notification.noteable.destroy!
+
+          get(:unsubscribe, params: { id: sent_notification.reply_key })
+        end
+
+        it_behaves_like 'returns 404'
+      end
     end
   end
 end