Allow unsubscribe to work for logged in user

Author: rossta

app/controllers/users/newsletter_subscriptions_controller.rb

@@ -51,9 +51,16 @@ def create
   end
 
   def unsubscribe
-    subscription = find_subscription or raise ActiveRecord::RecordNotFound
-
-    subscription.destroy
+    if params[:token]
+      subscription = NewsletterSubscription.find_by_token_for(:unsubscribe, params[:token]) or raise ActiveRecord::RecordNotFound
+      subscription.destroy
+    elsif current_user
+      # Even though we model the subscription as a has_one, we should destroy
+      # all because has_one is not enforced as a constraint
+      NewsletterSubscription.where(subscriber: current_user).destroy_all
+    else
+      raise ActiveRecord::RecordNotFound
+    end
 
     if request.post? && params["List-Unsubscribe"] == "One-Click"
       # must not redirect according to RFC 8058
@@ -63,14 +70,4 @@ def unsubscribe
       redirect_to root_path, notice: "You have been unsubscribed"
     end
   end
-
-  private
-
-  def find_subscription
-    if params[:token]
-      NewsletterSubscription.find_by_token_for(:unsubscribe, params[:token])
-    elsif current_user&.subscribed_to_newsletter?
-      current_user.newsletter_subscription
-    end
-  end
 end

config/routes.rb

@@ -39,6 +39,7 @@
 
     resources :newsletter_subscriptions, only: [:new, :create, :show]
     resources :newsletter_subscriptions, only: [], param: :token do
+      match :unsubscribe, on: :collection, via: [:get, :post, :delete]
       match :unsubscribe, on: :member, via: [:get, :post, :delete]
     end
 

spec/requests/users/newsletter_subscriptions_spec.rb

@@ -170,5 +170,31 @@
       expect(response).to have_http_status(:not_found)
       expect(user.newsletter_subscription).to be_present
     end
+
+    it "unsubscribes a logged in user with an existing subscription on collection route" do
+      user = FactoryBot.create(:user, :subscribed)
+      login_user user
+
+      expect(user.newsletter_subscription).to be_present
+
+      expect {
+        delete unsubscribe_users_newsletter_subscriptions_path
+      }.to change(NewsletterSubscription, :count).by(-1)
+
+      user.reload
+
+      expect(response).to redirect_to(root_path)
+      expect(flash[:notice]).to eq("You have been unsubscribed")
+
+      expect(user.newsletter_subscription).to be_nil
+    end
+
+    it "disallows when not logged in on collection route" do
+      expect {
+        post unsubscribe_users_newsletter_subscriptions_path
+      }.not_to change(NewsletterSubscription, :count)
+
+      expect(response).to have_http_status(:not_found)
+    end
   end
 end