From fdde3d62b59b07448c67fbdb80e5ee9f173d7441 Mon Sep 17 00:00:00 2001
From: Armando Fox <fox@cs.berkeley.edu>
Date: Thu, 9 Jun 2022 12:58:47 -0700
Subject: [PATCH] move newrelic gem to all groups to allow testing; basic specs
 for noticing stale orders

---
 Gemfile                                   |  2 +-
 Guardfile                                 | 16 +++++------
 app/services/stale_order_sweeper.rb       | 16 +++++++++--
 spec/services/stale_order_sweeper_spec.rb | 35 +++++++++++++++++++++++
 4 files changed, 57 insertions(+), 12 deletions(-)
 create mode 100644 spec/services/stale_order_sweeper_spec.rb

diff --git a/Gemfile b/Gemfile
index 31be844af..0a8c308e5 100644
--- a/Gemfile
+++ b/Gemfile
@@ -19,6 +19,7 @@ gem 'i18n'
 gem 'jbuilder', '~> 2.0'
 gem 'jquery-rails', '= 4.0.5'
 gem 'jquery-ui-rails', '= 5.0.5'
+gem 'newrelic_rpm'
 gem 'nokogiri'
 gem 'protected_attributes'      # remove once we migrate to Strong Parameters
 gem 'responders', '~> 2.0'
@@ -34,7 +35,6 @@ gem 'uglifier'
 
 group :production do
   gem 'rack-timeout'              # prevent Heroku dynos from hanging up on timeout
-  gem 'newrelic_rpm'
   gem 'puma-heroku'
   gem 'puma', '>= 4.3.8'
   gem 'rails_12factor'
diff --git a/Guardfile b/Guardfile
index 8714fa234..b1a1981ff 100644
--- a/Guardfile
+++ b/Guardfile
@@ -31,14 +31,14 @@ group 'cucumber' do
     # notification: false
   }
 
-  guard "cucumber", cucumber_options do
-    watch(%r{^features/.+\.feature$})
-    watch(%r{^features/support/.+$}) { "features" }
-
-    watch(%r{^features/step_definitions/(.+)_steps\.rb$}) do |m|
-      Dir[File.join("**/#{m[1]}.feature")][0] || "features"
-    end
-  end
+  # guard "cucumber", cucumber_options do
+  #   watch(%r{^features/.+\.feature$})
+  #   watch(%r{^features/support/.+$}) { "features" }
+
+  #   watch(%r{^features/step_definitions/(.+)_steps\.rb$}) do |m|
+  #     Dir[File.join("**/#{m[1]}.feature")][0] || "features"
+  #   end
+  # end
 end
 # Note: The cmd option is now required due to the increasing number of ways
 #       rspec may be run, below are examples of the most common uses.
diff --git a/app/services/stale_order_sweeper.rb b/app/services/stale_order_sweeper.rb
index 13d153221..543290d06 100644
--- a/app/services/stale_order_sweeper.rb
+++ b/app/services/stale_order_sweeper.rb
@@ -5,9 +5,13 @@ class StaleOrderSweeper
   # There's only one class method, designed to be called from a periodic runner or similar.
 
   def self.sweep!
-    # destroy stale orders and update sweep timer
-    stale_order_date = (Option.order_timeout + 1).minutes.ago # just to be on the safe side
-    stale_import_date = (Option.import_timeout + 1).minutes.ago
+    # raise error for orders where CC charge & order proc didn't happen atomically
+    self.notice_failed_orders!
+    # destroy stale (customer-abandoned) orders and update sweep timer
+    self.sweep_abandoned_orders_and_imports
+  end
+
+  def self.notice_failed_orders!
     # first check for 'pending' CC orders and raise an alarm if any are found, since no
     # order should ever be 'pending' for more than a few seconds
     if (pending = Order.pending_but_paid.abandoned_since(2.minutes.ago))
@@ -17,10 +21,16 @@ def self.sweep!
       # mark those order(s) as errored, so they don't trigger the warning again
       pending.update_all(:authorization => Order::ERRORED)
     end
+  end
+
+  def self.sweep_abandoned_orders_and_imports
+    stale_order_date = (Option.order_timeout + 1).minutes.ago # just to be on the safe side
+    stale_import_date = (Option.import_timeout + 1).minutes.ago
     ActiveRecord::Base.transaction do
       Order.where(:type => 'Order').abandoned_since(stale_order_date).destroy_all
       TicketSalesImport.abandoned_since(stale_import_date).destroy_all
       Option.first.update_attribute(:last_sweep, Time.current)
     end
   end
+
 end
diff --git a/spec/services/stale_order_sweeper_spec.rb b/spec/services/stale_order_sweeper_spec.rb
new file mode 100644
index 000000000..16a5edb65
--- /dev/null
+++ b/spec/services/stale_order_sweeper_spec.rb
@@ -0,0 +1,35 @@
+describe StaleOrderSweeper do
+
+  describe 'abandoned orders'  do
+    before(:each) do
+      @old_order = create(:order)
+      @old_id = old_order.id
+    end
+    specify 'are swept if too old' do
+      @old_order.update_attribute(:updated_at, 1.day.ago)
+      StaleOrderSweeper.sweep_abandoned_orders_and_imports
+      expect(Order.find_by(:id => @old_id)).to be_nil
+    end
+    specify 'are not swept if within abandon threshold' do
+      Option.first.update_attribute(:stale_order_timeout, 10) # minutes
+      StaleOrderSweeper.sweep_abandoned_orders_and_imports
+      expect(Order.find_by(:id => @old_id)).to eq(@old_order)
+    end
+  end
+
+  describe 'when an order has failed', focus: true do
+    before(:each) do
+      @o = create(:order)
+      @o.update_attribute(:authorization, Order::PENDING)
+      @o.update_attribute(:updated_at, 3.minutes.ago)
+    end
+    it 'notifies of error' do
+      expect(NewRelic::Agent).to receive(:notice_error)
+      StaleOrderSweeper.notice_failed_orders!
+    end
+    it 'changes order status to errored' do
+      StaleOrderSweeper.notice_failed_orders!
+      expect(@o.reload.authorization).to eq(Order::ERRORED)
+    end
+  end
+end