handle deleted article's comments page with a 404 response (#15351)

* Add a failing test for comments index of deleted article

After https://github.com/forem/forem/pull/15052 removed the (also
broken) deleted commentable template and the redirect, requests for
comments from deleted articles raise no method errors (initially
trying to set the page title to "Comments for commentable.title" but
the assumption that @commentable is non-nil is present in several
other places.

This test currently fails (by design). Either we want to update the
controller so that comments from deleted articles are no longer
viewable (returning not found, as we did prior to
https://github.com/forem/forem/pull/5199 or making the view safe for
the case where @commentable is nil and no @root_comment is present.

These requests are happening quite frequently (@maestromac and I
suspect the sitemap may contain these pages and crawlers are visiting
the site from a published link), as evidenced by
https://app.honeybadger.io/projects/66984/faults/81994265

* Add request spec for deleted article scenario

Additionally, relabel the legacy spec (updated during #15052) to
clarify we do not render the deleted_commentable_comment view (this is
testing the comment.path, rather than the article.path/comments index

The scenario with comment.path sets `@root_comment` in the controller,
so does not trigger errors on the `@commentable` method calls.

* Update view

pass 1: get the errors to stop (need to check the rendered page is
also usable, the comment tree is not shown when commentable is nil and
this might be a huge usability/correctness issue).

* extract logic from comments index to methods and update tests

We no longer expect deleted article's comments index to render (should
return 404), only direct links to comments of deleted articles.

Only assign @article in the index for the view if it is in fact an
article (not a podcast episode, it's possible `@root_comment` was for
a podcast episode).

* remove unneeded (and soon to be incorrect spec)

No longer want or need to test for the case where root comment is nil
and so is commentable. This was an exploratory spec (scaffolding) and
can be removed.

* Undo nil safety changes to comments index

and fix typo in comments spec

app/controllers/comments_controller.rb

@@ -21,14 +21,9 @@ class CommentsController < ApplicationController
       @user = @podcast
       @commentable = @user.podcast_episodes.find_by(slug: params[:slug]) if @user.podcast_episodes
     else
-      @user = User.find_by(username: params[:username]) ||
-        Organization.find_by(slug: params[:username]) ||
-        not_found
-      @commentable = @root_comment&.commentable ||
-        @user.articles.find_by(slug: params[:slug]) || nil
-      @article = @commentable
-
-      not_found if @commentable && !@commentable.published
+      set_user
+      set_commentable
+      not_found unless comment_should_be_visible?
     end
 
     @commentable_type = @commentable.class.name if @commentable
@@ -274,6 +269,26 @@ class CommentsController < ApplicationController
 
   private
 
+  def comment_should_be_visible?
+    if @article
+      @article.published?
+    else
+      @root_comment
+    end
+  end
+
+  def set_user
+    @user = User.find_by(username: params[:username]) ||
+      Organization.find_by(slug: params[:username]) ||
+      not_found
+  end
+
+  def set_commentable
+    @commentable = @root_comment&.commentable ||
+      @user.articles.find_by(slug: params[:slug]) || nil
+    @article = @commentable if @commentable.is_a?(Article)
+  end
+
   # Use callbacks to share common setup or constraints between actions.
   def set_comment
     @comment = Comment.find(params[:id])

spec/requests/comments_spec.rb

@@ -172,12 +172,17 @@ RSpec.describe "Comments", type: :request do
     end
 
     context "when the article is deleted" do
-      it "index action renders deleted_commentable_comment view" do
-        article = create(:article)
-        comment = create(:comment, commentable: article)
+      it "raises not found when listing article comments" do
+        path = "#{article.path}/comments"
 
         article.destroy
 
+        expect { get path }.to raise_error(ActiveRecord::RecordNotFound)
+      end
+
+      it "shows comment from a deleted post" do
+        article.destroy
+
         get comment.path
         expect(response.body).to include("Comment from a deleted post")
       end