From 58b2013377b46f01a2b4d4d259e6738b599a259c Mon Sep 17 00:00:00 2001
From: Anna Buianova <lightallloy@gmail.com>
Date: Sun, 24 Feb 2019 05:16:20 +0300
Subject: [PATCH] Fix delayed job errors related to follows #1621 (#1835)

* Add delayed_job_web to dev environment for debugging

* Add specs for the current Follow callbacks implementation

* Move follower touching to ActiveJob

* Spec for the touch followers job

* Move Follow#create_chat_channel to ActiveJob and make the job safe

* Add ActiveJob to send email notifications about follows

* Enqueue SendEmailNotificationJob after the follow is created

* Specs to wnsure jobs are enqueued on Follow creation

* Make CreateChatChannelJob queue name more specific
---
 Gemfile                                       |  1 +
 Gemfile.lock                                  |  6 ++
 app/jobs/follows/create_chat_channel_job.rb   | 12 ++++
 .../follows/send_email_notification_job.rb    | 15 +++++
 app/jobs/follows/touch_follower_job.rb        | 13 ++++
 app/models/follow.rb                          | 30 ++++++---
 config/initializers/delayed_job.rb            |  2 +-
 config/routes.rb                              |  4 ++
 spec/factories/follow.rb                      |  6 ++
 .../follows/create_chat_channel_job_spec.rb   | 42 +++++++++++++
 .../send_email_notification_job_spec.rb       | 46 ++++++++++++++
 spec/jobs/follows/touch_follower_job_spec.rb  | 29 +++++++++
 spec/models/follow_spec.rb                    | 63 +++++++++++++++++++
 13 files changed, 260 insertions(+), 9 deletions(-)
 create mode 100644 app/jobs/follows/create_chat_channel_job.rb
 create mode 100644 app/jobs/follows/send_email_notification_job.rb
 create mode 100644 app/jobs/follows/touch_follower_job.rb
 create mode 100644 spec/factories/follow.rb
 create mode 100644 spec/jobs/follows/create_chat_channel_job_spec.rb
 create mode 100644 spec/jobs/follows/send_email_notification_job_spec.rb
 create mode 100644 spec/jobs/follows/touch_follower_job_spec.rb

diff --git a/Gemfile b/Gemfile
index 0d14f281a..da2228b72 100644
--- a/Gemfile
+++ b/Gemfile
@@ -108,6 +108,7 @@ group :development do
   gem "brakeman", "~> 4.4", require: false
   gem "bullet", "~> 5.9"
   gem "bundler-audit", "~> 0.6"
+  gem "delayed_job_web", "~> 1.4"
   gem "derailed_benchmarks", "~> 1.3"
   gem "guard", "~> 2.15", require: false
   gem "guard-livereload", "~> 2.5", require: false
diff --git a/Gemfile.lock b/Gemfile.lock
index 5a6272f9d..4279566cf 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -235,6 +235,11 @@ GEM
     delayed_job_active_record (4.1.3)
       activerecord (>= 3.0, < 5.3)
       delayed_job (>= 3.0, < 5)
+    delayed_job_web (1.4.3)
+      activerecord (> 3.0.0)
+      delayed_job (> 2.0.3)
+      rack-protection (>= 1.5.5)
+      sinatra (>= 1.4.4)
     derailed (0.1.0)
       derailed_benchmarks
     derailed_benchmarks (1.3.5)
@@ -969,6 +974,7 @@ DEPENDENCIES
   dalli (~> 2.7)
   database_cleaner (~> 1.7)
   delayed_job_active_record (~> 4.1)
+  delayed_job_web (~> 1.4)
   derailed (~> 0.1)
   derailed_benchmarks (~> 1.3)
   devise (~> 4.6)
diff --git a/app/jobs/follows/create_chat_channel_job.rb b/app/jobs/follows/create_chat_channel_job.rb
new file mode 100644
index 000000000..bf2477943
--- /dev/null
+++ b/app/jobs/follows/create_chat_channel_job.rb
@@ -0,0 +1,12 @@
+module Follows
+  class CreateChatChannelJob < ApplicationJob
+    queue_as :create_chat_channel_after_follow
+
+    def perform(follow_id)
+      follow = Follow.includes(:follower, :followable).find_by(id: follow_id, follower_type: "User", followable_type: "User")
+      return unless follow&.followable&.following?(follow.follower)
+
+      ChatChannel.create_with_users([follow.followable, follow.follower])
+    end
+  end
+end
diff --git a/app/jobs/follows/send_email_notification_job.rb b/app/jobs/follows/send_email_notification_job.rb
new file mode 100644
index 000000000..9f0a7eae7
--- /dev/null
+++ b/app/jobs/follows/send_email_notification_job.rb
@@ -0,0 +1,15 @@
+module Follows
+  class SendEmailNotificationJob < ApplicationJob
+    queue_as :send_follow_email_notification
+
+    def perform(follow_id, mailer = NotifyMailer)
+      follow = Follow.find_by(id: follow_id, followable_type: "User")
+      return unless follow&.followable&.email? && follow.followable.email_follower_notifications
+      return if EmailMessage.where(user_id: follow.followable_id).
+        where("sent_at > ?", rand(15..35).hours.ago).
+        where("subject LIKE ?", "%followed you on dev.to%").any?
+
+      mailer.new_follower_email(follow).deliver
+    end
+  end
+end
diff --git a/app/jobs/follows/touch_follower_job.rb b/app/jobs/follows/touch_follower_job.rb
new file mode 100644
index 000000000..aa6573e47
--- /dev/null
+++ b/app/jobs/follows/touch_follower_job.rb
@@ -0,0 +1,13 @@
+module Follows
+  class TouchFollowerJob < ApplicationJob
+    queue_as :touch_follower
+
+    def perform(follow_id)
+      follow = Follow.find_by(id: follow_id)
+      return unless follow
+
+      follow.follower.touch
+      follow.follower.touch(:last_followed_at)
+    end
+  end
+end
diff --git a/app/models/follow.rb b/app/models/follow.rb
index ae3069ccd..85ac57e06 100644
--- a/app/models/follow.rb
+++ b/app/models/follow.rb
@@ -20,16 +20,31 @@ class Follow < ApplicationRecord
     ["follows.followable_type = ?", "Organization"] => "following_orgs_count",
     ["follows.followable_type = ?", "ActsAsTaggableOn::Tag"] => "following_tags_count"
   }
-  after_save :touch_user
-  after_save :touch_user_followed_at
-  after_create :send_email_notification
-  after_create :create_chat_channel
+  after_save :touch_follower
+  after_create :send_email_notification, :create_chat_channel
   before_destroy :modify_chat_channel_status
 
   validates :followable_id, uniqueness: { scope: %i[followable_type follower_id] }
 
   private
 
+  def touch_follower
+    Follows::TouchFollowerJob.perform_later(id)
+  end
+
+  def create_chat_channel
+    return unless followable_type == "User"
+
+    Follows::CreateChatChannelJob.perform_later(id)
+  end
+
+  def send_email_notification
+    return unless followable.class.name == "User" && followable.email?
+
+    Follows::SendEmailNotificationJob.perform_later(id)
+  end
+
+  # TODO: remove methods #touch_user, #touch_user_followed_at, #create_chat_channel_without_delay, #send_email_notification_without_delay
   def touch_user
     follower.touch
   end
@@ -40,14 +55,14 @@ class Follow < ApplicationRecord
   end
   handle_asynchronously :touch_user_followed_at
 
-  def create_chat_channel
+  # *_without_delay method will be used if there're jobs created before introducing ActiveJob
+  def create_chat_channel_without_delay
     if followable_type == "User" && followable.following?(follower)
       ChatChannel.create_with_users([followable, follower])
     end
   end
-  handle_asynchronously :create_chat_channel
 
-  def send_email_notification
+  def send_email_notification_without_delay
     if followable.class.name == "User" && followable.email.present? && followable.email_follower_notifications
       return if EmailMessage.where(user_id: followable.id).
         where("sent_at > ?", rand(15..35).hours.ago).
@@ -56,7 +71,6 @@ class Follow < ApplicationRecord
       NotifyMailer.new_follower_email(self).deliver
     end
   end
-  handle_asynchronously :send_email_notification
 
   def modify_chat_channel_status
     if followable_type == "User" && followable.following?(follower)
diff --git a/config/initializers/delayed_job.rb b/config/initializers/delayed_job.rb
index 29985f0c0..4c0241a01 100644
--- a/config/initializers/delayed_job.rb
+++ b/config/initializers/delayed_job.rb
@@ -1,4 +1,4 @@
-Delayed::Worker.destroy_failed_jobs = true
+Delayed::Worker.destroy_failed_jobs = !Rails.env.development?
 Delayed::Worker.sleep_delay = 60
 Delayed::Worker.max_attempts = 10
 Delayed::Worker.max_run_time = 30.minutes
diff --git a/config/routes.rb b/config/routes.rb
index 995efd930..4e5f1c04c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -7,6 +7,10 @@ Rails.application.routes.draw do
     registrations: "registrations"
   }
 
+  if Rails.env.development?
+    match "/delayed_job" => DelayedJobWeb, :anchor => false, :via => [:get, :post]
+  end
+
   devise_scope :user do
     delete "/sign_out" => "devise/sessions#destroy"
     get "/enter" => "registrations#new", as: :new_user_registration_path
diff --git a/spec/factories/follow.rb b/spec/factories/follow.rb
new file mode 100644
index 000000000..b8395c9f2
--- /dev/null
+++ b/spec/factories/follow.rb
@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :follow do
+    association :follower, factory: :user
+    association :followable, factory: :organization
+  end
+end
diff --git a/spec/jobs/follows/create_chat_channel_job_spec.rb b/spec/jobs/follows/create_chat_channel_job_spec.rb
new file mode 100644
index 000000000..375f73de6
--- /dev/null
+++ b/spec/jobs/follows/create_chat_channel_job_spec.rb
@@ -0,0 +1,42 @@
+require "rails_helper"
+
+RSpec.describe Follows::CreateChatChannelJob, type: :job do
+  describe "#perform_later" do
+    it "enqueues the job" do
+      ActiveJob::Base.queue_adapter = :test
+      expect do
+        described_class.perform_later(3)
+      end.to have_enqueued_job.with(3).on_queue("create_chat_channel_after_follow")
+    end
+  end
+
+  describe "#perform_now" do
+    let(:user) { create(:user) }
+    let(:user2) { create(:user) }
+    let!(:follow) { create(:follow, follower: user, followable: user2) }
+
+    it "creates a chat channel when mutual followers" do
+      follow2 = create(:follow, follower: user2, followable: user)
+      expect do
+        described_class.perform_now(follow2.id)
+      end.to change(ChatChannel, :count).by(1)
+    end
+
+    it "doesn't create a chat channel when the follow is not mutual" do
+      expect do
+        described_class.perform_now(follow.id)
+      end.not_to change(ChatChannel, :count)
+    end
+
+    it "doesn't fail if follow doesn't exist" do
+      described_class.perform_now(Follow.maximum(:id).to_i + 1)
+    end
+
+    it "doesn't do anything if follow is not from user to user" do
+      org_follow = create(:follow, follower: user, followable: create(:organization))
+      expect do
+        described_class.perform_now(org_follow.id)
+      end.not_to change(ChatChannel, :count)
+    end
+  end
+end
diff --git a/spec/jobs/follows/send_email_notification_job_spec.rb b/spec/jobs/follows/send_email_notification_job_spec.rb
new file mode 100644
index 000000000..5e575f4a0
--- /dev/null
+++ b/spec/jobs/follows/send_email_notification_job_spec.rb
@@ -0,0 +1,46 @@
+require "rails_helper"
+
+RSpec.describe Follows::SendEmailNotificationJob, type: :job do
+  describe "#perform_later" do
+    it "enqueues the job" do
+      ActiveJob::Base.queue_adapter = :test
+      expect do
+        described_class.perform_later(3)
+      end.to have_enqueued_job.with(3).on_queue("send_follow_email_notification")
+    end
+  end
+
+  describe "#perform_now" do
+    let(:user) { create(:user) }
+    let(:user2) { create(:user) }
+    let!(:follow) { create(:follow, follower: user, followable: user2) }
+    let(:mailer) { double }
+
+    before do
+      deliverer = double
+      allow(deliverer).to receive(:deliver)
+      allow(mailer).to receive(:new_follower_email).and_return(deliverer)
+    end
+
+    it "sends a new_follower_email" do
+      user2.update_column(:email_follower_notifications, true)
+      described_class.new(follow.id, mailer).perform_now
+      expect(mailer).to have_received(:new_follower_email).once
+    end
+
+    it "doesn't create an EmailMessage if it already exists" do
+      EmailMessage.create!(user_id: user2.id, sent_at: Time.now, subject: "#{user.username} followed you on dev.to")
+      described_class.new(follow.id, mailer).perform_now
+      expect(mailer).not_to have_received(:new_follower_email)
+    end
+
+    it "doesn't send an email if user has disabled notifications" do
+      user2.update_column(:email_follower_notifications, false)
+      expect(mailer).not_to have_received(:new_follower_email)
+    end
+
+    it "doesn't fail if follow doesn't exist" do
+      described_class.perform_now(Follow.maximum(:id).to_i + 1)
+    end
+  end
+end
diff --git a/spec/jobs/follows/touch_follower_job_spec.rb b/spec/jobs/follows/touch_follower_job_spec.rb
new file mode 100644
index 000000000..1b81795f6
--- /dev/null
+++ b/spec/jobs/follows/touch_follower_job_spec.rb
@@ -0,0 +1,29 @@
+require "rails_helper"
+
+RSpec.describe Follows::TouchFollowerJob, type: :job do
+  describe "#perform_later" do
+    it "enqueues the job" do
+      ActiveJob::Base.queue_adapter = :test
+      expect do
+        described_class.perform_later(3)
+      end.to have_enqueued_job.with(3).on_queue("touch_follower")
+    end
+
+    it "touches a follower" do
+      user = create(:user)
+      user.update_columns(updated_at: Time.now - 1.day, last_followed_at: Time.now - 1.day)
+      now = Time.now
+      follow = create(:follow, follower: user)
+
+      described_class.perform_now(follow.id)
+      user.reload
+
+      expect(user.updated_at).to be >= now
+      expect(user.last_followed_at).to be >= now
+    end
+
+    it "doesn't fail if follow doesn't exist" do
+      described_class.perform_now(Follow.maximum(:id).to_i + 1)
+    end
+  end
+end
diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb
index 7da2c8a29..613620b9a 100644
--- a/spec/models/follow_spec.rb
+++ b/spec/models/follow_spec.rb
@@ -8,4 +8,67 @@ RSpec.describe Follow, type: :model do
     user.follow(user_2)
     expect(user.following?(user_2)).to eq(true)
   end
+
+  context "when enqueuing jobs" do
+    before { ActiveJob::Base.queue_adapter = :test }
+
+    it "enqueues touch follower job on creation" do
+      expect do
+        Follow.create(follower: user, followable: user_2)
+      end.to have_enqueued_job(Follows::TouchFollowerJob)
+    end
+
+    it "enqueues create channel job" do
+      expect do
+        Follow.create(follower: user, followable: user_2)
+      end.to have_enqueued_job(Follows::CreateChatChannelJob)
+    end
+
+    it "enqueues send notification job" do
+      expect do
+        Follow.create(follower: user, followable: user_2)
+      end.to have_enqueued_job(Follows::SendEmailNotificationJob)
+    end
+  end
+
+  context "when creating and inline" do
+    before { ActiveJob::Base.queue_adapter = :inline }
+
+    it "touches the follower user while creating" do
+      user.update_columns(updated_at: Time.now - 1.day, last_followed_at: Time.now - 1.day)
+      now = Time.now
+      Follow.create!(follower: user, followable: user_2)
+      user.reload
+      expect(user.updated_at).to be >= now
+      expect(user.last_followed_at).to be >= now
+    end
+
+    it "doesn't create a channel when a followable is an org" do
+      expect do
+        Follow.create!(follower: user, followable: create(:organization))
+      end.not_to change(ChatChannel, :count)
+    end
+
+    it "doesn't create a chat channel when users don't follow mutually" do
+      expect do
+        Follow.create!(follower: user, followable: user_2)
+      end.not_to change(ChatChannel, :count)
+    end
+
+    it "creates a chat channel when users follow mutually" do
+      Follow.create!(follower: user_2, followable: user)
+      expect do
+        Follow.create!(follower: user, followable: user_2)
+      end.to change(ChatChannel, :count).by(1)
+    end
+
+    it "sends an email notification" do
+      user_2.update_column(:email_follower_notifications, true)
+      expect do
+        run_background_jobs_immediately do
+          Follow.create!(follower: user, followable: user_2)
+        end
+      end.to change(EmailMessage, :count).by(1)
+    end
+  end
 end