From 8973fbcf08fcbfb7af8ad9fd9465ff3c2de06437 Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 10:38:51 -0400
Subject: Simplify incineration

---
 .../inbound_email/incineration_job.rb              | 11 -----------
 app/jobs/action_mailbox/incineration_job.rb        | 11 +++++++++++
 app/models/action_mailbox/inbound_email.rb         |  4 ++++
 .../action_mailbox/inbound_email/incineratable.rb  | 23 +++++-----------------
 .../inbound_email/incineratable/incineration.rb    |  4 ++--
 .../action_mailbox/inbound_email/routable.rb       |  2 +-
 test/unit/inbound_email/incineration_test.rb       |  6 +++---
 7 files changed, 26 insertions(+), 35 deletions(-)
 delete mode 100644 app/jobs/action_mailbox/inbound_email/incineration_job.rb
 create mode 100644 app/jobs/action_mailbox/incineration_job.rb

diff --git a/app/jobs/action_mailbox/inbound_email/incineration_job.rb b/app/jobs/action_mailbox/inbound_email/incineration_job.rb
deleted file mode 100644
index d9422a3fa8..0000000000
--- a/app/jobs/action_mailbox/inbound_email/incineration_job.rb
+++ /dev/null
@@ -1,11 +0,0 @@
-class ActionMailbox::InboundEmail::IncinerationJob < ApplicationJob
-  queue_as :action_mailbox_incineration
-
-  def self.schedule(inbound_email)
-    set(wait: ActionMailbox.incinerate_after).perform_later(inbound_email)
-  end
-
-  def perform(inbound_email)
-    inbound_email.incinerate
-  end
-end
diff --git a/app/jobs/action_mailbox/incineration_job.rb b/app/jobs/action_mailbox/incineration_job.rb
new file mode 100644
index 0000000000..e9f0c78e0f
--- /dev/null
+++ b/app/jobs/action_mailbox/incineration_job.rb
@@ -0,0 +1,11 @@
+class ActionMailbox::IncinerationJob < ApplicationJob
+  queue_as :action_mailbox_incineration
+
+  def self.schedule(inbound_email)
+    set(wait: ActionMailbox.incinerate_after).perform_later(inbound_email)
+  end
+
+  def perform(inbound_email)
+    inbound_email.incinerate
+  end
+end
diff --git a/app/models/action_mailbox/inbound_email.rb b/app/models/action_mailbox/inbound_email.rb
index f2589d7429..7d1a36b705 100644
--- a/app/models/action_mailbox/inbound_email.rb
+++ b/app/models/action_mailbox/inbound_email.rb
@@ -19,4 +19,8 @@ class ActionMailbox::InboundEmail < ActiveRecord::Base
   def source
     @source ||= raw_email.download
   end
+
+  def processed?
+    delivered? || failed? || bounced?
+  end
 end
diff --git a/app/models/action_mailbox/inbound_email/incineratable.rb b/app/models/action_mailbox/inbound_email/incineratable.rb
index 364231a443..198846422c 100644
--- a/app/models/action_mailbox/inbound_email/incineratable.rb
+++ b/app/models/action_mailbox/inbound_email/incineratable.rb
@@ -2,27 +2,14 @@ module ActionMailbox::InboundEmail::Incineratable
   extend ActiveSupport::Concern
 
   included do
-    before_update :remember_to_incinerate_later
-    after_update_commit :incinerate_later, if: :incinerating_later?
+    after_update_commit :incinerate_later, if: -> { status_previously_changed? && processed? }
+  end
+
+  def incinerate_later
+    ActionMailbox::IncinerationJob.schedule self
   end
 
   def incinerate
     Incineration.new(self).run
   end
-
-  private
-    # TODO: Use enum change tracking once merged into Active Support
-    def remember_to_incinerate_later
-      if status_changed? && (delivered? || bounced? || failed?)
-        @incinerating_later = true
-      end
-    end
-
-    def incinerating_later?
-      @incinerating_later ||= false
-    end
-
-    def incinerate_later
-      ActionMailbox::InboundEmail::IncinerationJob.schedule(self)
-    end
 end
diff --git a/app/models/action_mailbox/inbound_email/incineratable/incineration.rb b/app/models/action_mailbox/inbound_email/incineratable/incineration.rb
index ab9311edfb..801cc0c8b9 100644
--- a/app/models/action_mailbox/inbound_email/incineratable/incineration.rb
+++ b/app/models/action_mailbox/inbound_email/incineratable/incineration.rb
@@ -4,7 +4,7 @@ class ActionMailbox::InboundEmail::Incineratable::Incineration
   end
 
   def run
-    @inbound_email.destroy if due? && processed?
+    @inbound_email.destroy! if due? && processed?
   end
 
   private
@@ -13,6 +13,6 @@ class ActionMailbox::InboundEmail::Incineratable::Incineration
     end
 
     def processed?
-      @inbound_email.delivered? || @inbound_email.bounced? || @inbound_email.failed?
+      @inbound_email.processed?
     end
 end
diff --git a/app/models/action_mailbox/inbound_email/routable.rb b/app/models/action_mailbox/inbound_email/routable.rb
index 48b357af45..8f5b0ddd39 100644
--- a/app/models/action_mailbox/inbound_email/routable.rb
+++ b/app/models/action_mailbox/inbound_email/routable.rb
@@ -2,7 +2,7 @@ module ActionMailbox::InboundEmail::Routable
   extend ActiveSupport::Concern
 
   included do
-    after_create_commit :route_later, if: ->(inbound_email) { inbound_email.pending? }
+    after_create_commit :route_later, if: :pending?
   end
 
   def route_later
diff --git a/test/unit/inbound_email/incineration_test.rb b/test/unit/inbound_email/incineration_test.rb
index 72378df05a..a3267afac9 100644
--- a/test/unit/inbound_email/incineration_test.rb
+++ b/test/unit/inbound_email/incineration_test.rb
@@ -4,7 +4,7 @@ class ActionMailbox::InboundEmail::IncinerationTest < ActiveSupport::TestCase
   test "incinerating 30 days after delivery" do
     freeze_time
 
-    assert_enqueued_with job: ActionMailbox::InboundEmail::IncinerationJob, at: 30.days.from_now do
+    assert_enqueued_with job: ActionMailbox::IncinerationJob, at: 30.days.from_now do
       create_inbound_email_from_fixture("welcome.eml").delivered!
     end
   end
@@ -12,7 +12,7 @@ class ActionMailbox::InboundEmail::IncinerationTest < ActiveSupport::TestCase
   test "incinerating 30 days after bounce" do
     freeze_time
 
-    assert_enqueued_with job: ActionMailbox::InboundEmail::IncinerationJob, at: 30.days.from_now do
+    assert_enqueued_with job: ActionMailbox::IncinerationJob, at: 30.days.from_now do
       create_inbound_email_from_fixture("welcome.eml").bounced!
     end
   end
@@ -20,7 +20,7 @@ class ActionMailbox::InboundEmail::IncinerationTest < ActiveSupport::TestCase
   test "incinerating 30 days after failure" do
     freeze_time
 
-    assert_enqueued_with job: ActionMailbox::InboundEmail::IncinerationJob, at: 30.days.from_now do
+    assert_enqueued_with job: ActionMailbox::IncinerationJob, at: 30.days.from_now do
       create_inbound_email_from_fixture("welcome.eml").failed!
     end
   end
-- 
cgit v1.2.3


From 89ada1977ce648b2121cad7cfb95af9686ea016b Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 10:54:58 -0400
Subject: Remove unnecessary TODO

Exception handlers are executed in the context of a mailbox instance and already have access to the inbound email.
---
 lib/action_mailbox/base.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/action_mailbox/base.rb b/lib/action_mailbox/base.rb
index 3b12493662..103bbc544c 100644
--- a/lib/action_mailbox/base.rb
+++ b/lib/action_mailbox/base.rb
@@ -27,7 +27,6 @@ class ActionMailbox::Base
       end
     end
   rescue => exception
-    # TODO: Include a reference to the inbound_email in the exception raised so error handling becomes easier
     rescue_with_handler(exception) || raise
   end
 
-- 
cgit v1.2.3


From 7903e3ada80d7938b1b78b06d0f678e95367a56b Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 12:38:26 -0400
Subject: Use a class attribute

---
 lib/action_mailbox/routing.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/action_mailbox/routing.rb b/lib/action_mailbox/routing.rb
index b40e2774e4..d258b632f9 100644
--- a/lib/action_mailbox/routing.rb
+++ b/lib/action_mailbox/routing.rb
@@ -2,15 +2,17 @@ module ActionMailbox
   module Routing
     extend ActiveSupport::Concern
 
-    class_methods do
-      attr_reader :router
+    included do
+      cattr_accessor :router, default: ActionMailbox::Router.new
+    end
 
+    class_methods do
       def routing(routes)
-        (@router ||= ActionMailbox::Router.new).add_routes(routes)
+        router.add_routes(routes)
       end
 
       def route(inbound_email)
-        @router.route(inbound_email)
+        router.route(inbound_email)
       end
     end
   end
-- 
cgit v1.2.3


From 2b09cbbe4407ce84b23450b27b2c3b6f35e8ef1b Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 12:42:54 -0400
Subject: Shush various interpreter warnings

---
 app/models/action_mailbox/inbound_email/message_id.rb | 2 +-
 lib/action_mailbox/base.rb                            | 2 +-
 lib/action_mailbox/test_helper.rb                     | 2 +-
 test/unit/mailbox/routing_test.rb                     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/models/action_mailbox/inbound_email/message_id.rb b/app/models/action_mailbox/inbound_email/message_id.rb
index 590dbfc4d7..a1ec5c0437 100644
--- a/app/models/action_mailbox/inbound_email/message_id.rb
+++ b/app/models/action_mailbox/inbound_email/message_id.rb
@@ -13,7 +13,7 @@ module ActionMailbox::InboundEmail::MessageId
     private
       def extract_message_id(raw_email)
         mail_from_source(raw_email.read).message_id
-      rescue => e
+      rescue
         # FIXME: Add logging with "Couldn't extract Message ID, so will generating a new random ID instead"
       end
   end
diff --git a/lib/action_mailbox/base.rb b/lib/action_mailbox/base.rb
index 103bbc544c..01ddc9c59b 100644
--- a/lib/action_mailbox/base.rb
+++ b/lib/action_mailbox/base.rb
@@ -49,7 +49,7 @@ class ActionMailbox::Base
       inbound_email.processing!
       yield
       inbound_email.delivered! unless inbound_email.bounced?
-    rescue => exception
+    rescue
       inbound_email.failed!
       raise
     end
diff --git a/lib/action_mailbox/test_helper.rb b/lib/action_mailbox/test_helper.rb
index b1d7af33f9..a74ea8ef57 100644
--- a/lib/action_mailbox/test_helper.rb
+++ b/lib/action_mailbox/test_helper.rb
@@ -9,7 +9,7 @@ module ActionMailbox
     end
 
     def create_inbound_email_from_mail(status: :processing, **mail_options)
-      raw_email = Tempfile.new.tap { |raw_email| raw_email.write Mail.new(mail_options).to_s }
+      raw_email = Tempfile.new.tap { |io| io.write Mail.new(mail_options).to_s }
       create_inbound_email(raw_email, status: status)
     end
 
diff --git a/test/unit/mailbox/routing_test.rb b/test/unit/mailbox/routing_test.rb
index 320dee6aab..f30199b7af 100644
--- a/test/unit/mailbox/routing_test.rb
+++ b/test/unit/mailbox/routing_test.rb
@@ -23,7 +23,7 @@ class ActionMailbox::Base::RoutingTest < ActiveSupport::TestCase
 
   test "delayed routing" do
     perform_enqueued_jobs only: ActionMailbox::RoutingJob do
-      another_inbound_email = create_inbound_email_from_fixture("welcome.eml", status: :pending)
+      create_inbound_email_from_fixture "welcome.eml", status: :pending
       assert_equal "Discussion: Let's debate these attachments", $processed
     end
   end
-- 
cgit v1.2.3


From 5a8939447b2ae655f0c304ebb226eb5e6f9d8496 Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 12:48:30 -0400
Subject: Bump mail to shush mismatched indentation warnings

---
 Gemfile.lock | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 868d7f0819..d01cb7bd76 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -83,7 +83,7 @@ GEM
     loofah (2.2.2)
       crass (~> 1.0.2)
       nokogiri (>= 1.5.9)
-    mail (2.7.0)
+    mail (2.7.1)
       mini_mime (>= 0.1.1)
     marcel (0.3.3)
       mimemagic (~> 0.3.2)
@@ -131,4 +131,4 @@ DEPENDENCIES
   sqlite3
 
 BUNDLED WITH
-   1.16.5
+   1.16.6
-- 
cgit v1.2.3


From 1c4a57e0e5b9666f0a05eb4fbdad09a6998ef54b Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 12:48:45 -0400
Subject: Remove needless autoloads

ActionMailbox::Callbacks and ActionMailbox::Routing are eagerly loaded where they're used.
---
 lib/action_mailbox.rb | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/action_mailbox.rb b/lib/action_mailbox.rb
index 01cc38ec4a..ae37cb84ed 100644
--- a/lib/action_mailbox.rb
+++ b/lib/action_mailbox.rb
@@ -5,8 +5,6 @@ module ActionMailbox
 
   autoload :Base
   autoload :Router
-  autoload :Callbacks
-  autoload :Routing
 
   mattr_accessor :logger
   mattr_accessor :incinerate_after, default: 30.days
-- 
cgit v1.2.3


From cf8d76fdb42ab33c778b1787fb2ebe06481e2e3f Mon Sep 17 00:00:00 2001
From: George Claghorn <george@basecamp.com>
Date: Wed, 17 Oct 2018 13:22:52 -0400
Subject: Revert "Remove unnecessary TODO"

This reverts commit 89ada1977ce648b2121cad7cfb95af9686ea016b.
---
 lib/action_mailbox/base.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/action_mailbox/base.rb b/lib/action_mailbox/base.rb
index 01ddc9c59b..30ecc4b623 100644
--- a/lib/action_mailbox/base.rb
+++ b/lib/action_mailbox/base.rb
@@ -27,6 +27,7 @@ class ActionMailbox::Base
       end
     end
   rescue => exception
+    # TODO: Include a reference to the inbound_email in the exception raised so error handling becomes easier
     rescue_with_handler(exception) || raise
   end
 
-- 
cgit v1.2.3