Delayed Jobs Investigation (#929)

* disallow nil reactables to be saved

* investigation wip

* passes deleted comment info to async bust cache

* fixes failing cache buster spec

* Create user_delete_a_comment_spec.rb

* Fix broken spec

* Binstub RuboCop

* Do not save comments when they are getting destroyed

* break update_reactable into smaller methods

app/labor/cache_buster.rb

@@ -11,29 +11,29 @@ class CacheBuster
     headers: { "Fastly-Key" => ApplicationConfig["FASTLY_API_KEY"] })
   end
 
-  def bust_comment(comment)
-    if comment.commentable.featured_number.to_i > (Time.now.to_i - 5.hours.to_i)
+  def bust_comment(commentable, username)
+    if commentable.featured_number.to_i > (Time.now.to_i - 5.hours.to_i)
       bust("/")
       bust("/?i=i")
       bust("?i=i")
     end
-    if comment.commentable.decorate.cached_tag_list_array.include?("discuss") &&
-        comment.commentable.featured_number.to_i > (Time.now.to_i - 35.hours.to_i)
+    if commentable.decorate.cached_tag_list_array.include?("discuss") &&
+        commentable.featured_number.to_i > (Time.now.to_i - 35.hours.to_i)
       bust("/")
       bust("/?i=i")
       bust("?i=i")
     end
-    bust("#{comment.commentable.path}/comments/")
-    bust(comment.commentable.path.to_s)
-    comment.commentable.comments.each do |c|
+    bust("#{commentable.path}/comments/")
+    bust(commentable.path.to_s)
+    commentable.comments.each do |c|
       bust(c.path)
       bust(c.path + "?i=i")
     end
-    bust("#{comment.commentable.path}/comments/*")
-    bust("/#{comment.user.username}")
-    bust("/#{comment.user.username}/comments")
-    bust("/#{comment.user.username}/comments?i=i")
-    bust("/#{comment.user.username}/comments/?i=i")
+    bust("#{commentable.path}/comments/*")
+    bust("/#{username}")
+    bust("/#{username}/comments")
+    bust("/#{username}/comments?i=i")
+    bust("/#{username}/comments/?i=i")
   end
 
   def bust_article(article)

app/models/comment.rb

@@ -226,6 +226,13 @@ class Comment < ApplicationRecord
     )
   end
 
+  def self.comment_async_bust(commentable, username)
+    commentable.touch
+    commentable.touch(:last_comment_at)
+    CacheBuster.new.bust_comment(commentable, username)
+    commentable.index!
+  end
+
   private
 
   def send_to_moderator
@@ -299,6 +306,7 @@ class Comment < ApplicationRecord
 
   def before_destroy_actions
     bust_cache
+    Comment.delay.comment_async_bust(commentable, user.username)
     remove_algolia_index
     reactions.destroy_all
   end
@@ -308,18 +316,8 @@ class Comment < ApplicationRecord
     cache_buster = CacheBuster.new
     cache_buster.bust(commentable.path.to_s) if commentable
     cache_buster.bust("#{commentable.path}/comments") if commentable
-    async_bust
   end
 
-  def async_bust
-    expire_root_fragment
-    commentable.touch
-    commentable.touch(:last_comment_at)
-    CacheBuster.new.bust_comment(self)
-    commentable.index!
-  end
-  handle_asynchronously :async_bust
-
   def send_email_notification
     NotifyMailer.new_reply_email(self).deliver
   end

app/models/reaction.rb

@@ -74,20 +74,30 @@ class Reaction < ApplicationRecord
   private
 
   def update_reactable
-    cache_buster = CacheBuster.new
     if reactable_type == "Article"
-      reactable.async_score_calc
-      reactable.index!
-      cache_buster.bust "/reactions?article_id=#{reactable_id}"
-    elsif reactable_type == "Comment"
-      reactable.save
-      cache_buster.bust "/reactions?commentable_id=#{reactable.commentable_id}&commentable_type=#{reactable.commentable_type}"
+      update_article
+    elsif reactable_type == "Comment" && reactable
+      update_comment
     end
-    cache_buster.bust user.path
     occasionally_sync_reaction_counts
   end
   handle_asynchronously :update_reactable
 
+  def update_article
+    cache_buster = CacheBuster.new
+    reactable.async_score_calc
+    reactable.index!
+    cache_buster.bust "/reactions?article_id=#{reactable_id}"
+    cache_buster.bust user.path
+  end
+
+  def update_comment
+    cache_buster = CacheBuster.new
+    reactable.save unless destroyed_by_association
+    cache_buster.bust "/reactions?commentable_id=#{reactable.commentable_id}&commentable_type=#{reactable.commentable_type}"
+    cache_buster.bust user.path
+  end
+
   def touch_user
     user.touch
   end

bin/rubocop

@@ -0,0 +1,29 @@
+#!/usr/bin/env ruby
+# frozen_string_literal: true
+
+#
+# This file was generated by Bundler.
+#
+# The application 'rubocop' is installed as part of a gem, and
+# this file is here to facilitate running it.
+#
+
+require "pathname"
+ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile",
+  Pathname.new(__FILE__).realpath)
+
+bundle_binstub = File.expand_path("../bundle", __FILE__)
+
+if File.file?(bundle_binstub)
+  if File.read(bundle_binstub, 300) =~ /This file was generated by Bundler/
+    load(bundle_binstub)
+  else
+    abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run.
+Replace `bin/bundle` by running `bundle binstubs bundler --force`, then run this command again.")
+  end
+end
+
+require "rubygems"
+require "bundler/setup"
+
+load Gem.bin_path("rubocop", "rubocop")

package.json

@@ -23,7 +23,7 @@
       "git add"
     ],
     "{app,spec}/**/*.rb": [
-      "bundle exec rubocop --require rubocop-rspec --auto-correct",
+      "bin/rubocop --require rubocop-rspec --auto-correct",
       "git add"
     ],
     "*.json": [

spec/features/user_delete_a_comment_spec.rb

@@ -0,0 +1,20 @@
+require "rails_helper"
+
+RSpec.describe "Deleting Comment", type: :feature, js: true do
+  let(:user) { create(:user) }
+  let(:raw_comment) { Faker::Lorem.paragraph }
+  let(:article) do
+    create(:article, user_id: user.id, show_comments: true)
+  end
+  let(:comment) { create(:comment, commentable: article, commentable_type: "Article", user: user) }
+
+  before do
+    sign_in user
+  end
+
+  it "works" do
+    visit comment.path + "/delete_confirm"
+    click_link("DELETE")
+    expect(page).to have_current_path(article.path)
+  end
+end

spec/labor/cache_buster_spec.rb

@@ -6,7 +6,9 @@ RSpec.describe CacheBuster do
   let(:comment) { create(:comment, user_id: user.id, commentable_id: article.id) }
 
   it "busts comment" do
-    described_class.new.bust_comment(comment)
+    commentable = Article.find(comment.commentable_id)
+    username = User.find(comment.user_id).username
+    described_class.new.bust_comment(commentable, username)
   end
   it "busts article" do
     described_class.new.bust_article(article)