Merge branch '199034-nomethoderror-undefined-method-unsubscribe-for-nil-nilclass' into 'master'

Resolve "NoMethodError: undefined method `unsubscribe' for nil:NilClass"

Closes #199034

See merge request gitlab-org/gitlab!23747

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