VRFS-4249 - fix for stripe error handling in charge method

ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/teacher_distribution_fail.html.erb

@@ -9,11 +9,11 @@
 <% else %>
 <p>
   <% if @card_declined %>
-    When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by stripe. Can you please check your stripe account status? Thank you!
+    When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by Stripe. Can you please check your Stripe account status? Thank you!
   <% elsif @card_expired %>
-    When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by stripe due to a card expiration. Can you please check your stripe account status? Thank you!
+    When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by Stripe due to a card expiration. Can you please check your stripe account status? Thank you!
   <% else %>
-    For some reason, when we tried to distribute a payment to you on <%= @bill_date %>, the charge failed. Can you please check your stripe account status?  Thank you!
+    For some reason, when we tried to distribute a payment to you on <%= @bill_date %>, the charge failed. Can you please check your Stripe account status?  Thank you!
   <% end %>
 </p>
 <% end %>

ruby/lib/jam_ruby/app/views/jam_ruby/user_mailer/teacher_distribution_fail.text.erb

@@ -5,11 +5,11 @@ Hello <%= @name %>,
   We attempted to process a payment via your Stripe account for <%= @distribution.real_distribution_display %> for this lesson, but the payment failed. Please sign into your Stripe account, and verify that everything there is working properly. We’ll try again to process this payment in about 24 hours.
 <% else %>
   <% if @card_declined %>
-When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by stripe. Can you please check your stripe account status? Thank you!
+When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by Stripe. Can you please check your Stripe account status? Thank you!
   <% elsif @card_expired %>
-When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by stripe due to a card expiration. Can you please check your stripe account status? Thank you!
+When we tried to distribute a payment to you on <%= @bill_date %>, the charge was declined by Stripe due to a card expiration. Can you please check your Stripe account status? Thank you!
   <% else %>
-For some reason, when we tried to distribute a payment to you on <%= @bill_date %>, the charge failed. Can you please check your stripe account status?  Thank you!
+For some reason, when we tried to distribute a payment to you on <%= @bill_date %>, the charge failed. Can you please check your Stripe account status?  Thank you!
   <% end %>
 <% end %>
 

ruby/lib/jam_ruby/models/charge.rb

@@ -1,6 +1,8 @@
 module JamRuby
   class  Charge < ActiveRecord::Base
 
+    attr_accessor :stripe_charge
+
     belongs_to :user, class_name: "JamRuby::User"
 
     validates :sent_billing_notices, inclusion: {in: [true, false]}
@@ -24,9 +26,16 @@ module JamRuby
       raise "not implemented"
     end
 
+    def record_charge(stripe_charge)
+      self.stripe_charge_id = stripe_charge.id
+      self.billed = true
+      self.billed_at = Time.now
+      self.save(validate: false)
+    end
+
     def charge(force = false)
 
-      stripe_charge = nil
+      @stripe_charge = nil
 
       if !self.billed
 
@@ -49,10 +58,10 @@ module JamRuby
         begin
 
           stripe_charge = do_charge(force)
-          self.stripe_charge_id = stripe_charge.id
-          self.billed = true
-          self.billed_at = Time.now
-          self.save(validate: false)
+
+          # record the charge in this context (meaning, in our transaction)
+          record_charge(@stripe_charge) if @stripe_charge
+
         rescue Stripe::StripeError => e
 
           stripe_handler(e)
@@ -64,10 +73,15 @@ module JamRuby
 
           return false
         rescue Exception => e
+
+          # record the charge even if there was an unhandled exception at some point
+          record_charge(@stripe_charge) if @stripe_charge
+
+          unhandled_handler(e)
+
           subject = "Unable to charge user #{charged_user.email} for lesson #{self.id} (unhandled)"
           body = "user=#{charged_user.email}\n\nbilling_error_reason=#{billing_error_reason}\n\nbilling_error_detail = #{billing_error_detail}"
           AdminMailer.alerts({subject: subject, body: body}).deliver
-          unhandled_handler(e)
           return false
         end
 
@@ -89,8 +103,8 @@ module JamRuby
       return stripe_charge
     end
 
-    def unhandled_handler(e, reason = 'unhandled_exception')
-      self.billing_error_reason = reason
+    def unhandled_handler(e)
+      self.billing_error_reason = e.to_s
       if e.cause
         self.billing_error_detail = e.cause.to_s + "\n" + e.cause.backtrace.join("\n\t") if e.cause.backtrace
         self.billing_error_detail << "\n\n"
@@ -98,7 +112,7 @@ module JamRuby
       else
         self.billing_error_detail = e.to_s + "\n" + e.backtrace.join("\n\t") if e.backtrace
       end
-      puts "Charge: unhandled exception #{billing_error_reason}, #{billing_error_detail}"
+      #puts "Charge: unhandled exception #{billing_error_reason}, #{billing_error_detail}"
       self.save(validate: false)
     end
 

ruby/lib/jam_ruby/models/lesson_payment_charge.rb

@@ -39,12 +39,18 @@ module JamRuby
       !lesson_session.nil?
     end
 
+
+    # stupid way to inject a failure
+    def post_sale_test_failure
+      return true
+    end
+
     def do_charge(force)
 
       if is_lesson?
-        result = Sale.purchase_lesson(student, lesson_booking, lesson_booking.lesson_package_type, lesson_session)
+        result = Sale.purchase_lesson(self, student, lesson_booking, lesson_booking.lesson_package_type, lesson_session)
       else
-        result = Sale.purchase_lesson(student, lesson_booking, lesson_booking.lesson_package_type, nil, lesson_package_purchase, force)
+        result = Sale.purchase_lesson(self, student, lesson_booking, lesson_booking.lesson_package_type, nil, lesson_package_purchase, force)
         lesson_booking.unsuspend! if lesson_booking.is_suspended?
       end
 
@@ -55,6 +61,8 @@ module JamRuby
 
       # update teacher distribution, because it's now ready to be given to them!
 
+      post_sale_test_failure
+
       distribution = target.teacher_distribution
       if distribution # not all lessons/payment charges have a distribution
         distribution.ready = true

ruby/lib/jam_ruby/models/lesson_session.rb

@@ -932,7 +932,7 @@ module JamRuby
     end
 
     def description(lesson_booking)
-      lesson_booking.lesson_package_type.description(lesson_booking)
+      lesson_booking.lesson_package_type.description(lesson_booking) + " with #{teacher.admin_name}"
     end
 
     def timed_description

ruby/lib/jam_ruby/models/sale.rb

@@ -82,26 +82,26 @@ module JamRuby
       price = price_data['product_price'].to_f * 100.0
 
       price_info = {
-        subtotal_in_cents: price, 
-        total_in_cents: price, 
-        tax_in_cents: nil, 
-        currency: price_data['product_currency']
+          subtotal_in_cents: price,
+          total_in_cents: price,
+          tax_in_cents: nil,
+          currency: price_data['product_currency']
       }
-      response = IosReceiptValidator.post('/verifyReceipt', 
-                                          body: { 'receipt-data' => receipt }.to_json,
-                                          headers: { 'Content-Type' => 'application/json' })
+      response = IosReceiptValidator.post('/verifyReceipt',
+                                          body: {'receipt-data' => receipt}.to_json,
+                                          headers: {'Content-Type' => 'application/json'})
       json_resp = JSON.parse(response.body)
 
       #  https://developer.apple.com/library/ios/releasenotes/General/ValidateAppStoreReceipt/Chapters/ReceiptFields.html#//apple_ref/doc/uid/TP40010573-CH106-SW1
 
       if 0 != json_resp['status']
         err_msgs = {
-          21000 => 'The App Store could not read the JSON object you provided.',
-          21002 => 'The data in the receipt-data property was malformed or missing.',
-          21003 => 'The receipt could not be authenticated.',
-          21005 => 'The receipt server is not currently available.',
-          21007 => 'This receipt is from the test environment, but it was sent to the production environment for verification. Send it to the test environment instead.',
-          21008 => 'This receipt is from the production environment, but it was sent to the test environment for verification. Send it to the production environment instead.'
+            21000 => 'The App Store could not read the JSON object you provided.',
+            21002 => 'The data in the receipt-data property was malformed or missing.',
+            21003 => 'The receipt could not be authenticated.',
+            21005 => 'The receipt server is not currently available.',
+            21007 => 'This receipt is from the test environment, but it was sent to the production environment for verification. Send it to the test environment instead.',
+            21008 => 'This receipt is from the production environment, but it was sent to the test environment for verification. Send it to the production environment instead.'
         }
         raise err_msgs[json_resp['status']]
       else
@@ -209,63 +209,63 @@ module JamRuby
     end
 
     def self.purchase_test_drive(current_user, lesson_package_type, booking = nil)
-      self.purchase_lesson(current_user, booking, lesson_package_type)
+      self.purchase_lesson(nil, current_user, booking, lesson_package_type)
     end
 
-    def self.purchase_normal(current_user, booking)
-      self.purchase_lesson(current_user, booking, LessonPackageType.single, booking.lesson_sessions[0])
+    def self.post_sale_test_failure
+      return true
     end
-
     # this is easy to make generic, but right now, it just purchases lessons
-    def self.purchase_lesson(current_user, lesson_booking, lesson_package_type, lesson_session = nil, lesson_package_purchase = nil, force = false)
+    def self.purchase_lesson(charge, current_user, lesson_booking, lesson_package_type, lesson_session = nil, lesson_package_purchase = nil, force = false)
       stripe_charge = nil
       sale = nil
       purchase = nil
       # everything needs to go into a transaction!  If anything goes wrong, we need to raise an exception to break it
       Sale.transaction(:requires_new => true) do
 
-        sale = create_lesson_sale(current_user)
+          sale = create_lesson_sale(current_user)
 
-        if sale.valid?
+          if sale.valid?
 
-          if lesson_booking
-            lesson_booking.current_lesson = lesson_session
-            lesson_booking.current_purchase = lesson_package_purchase
-          end
+            if lesson_booking
+              lesson_booking.current_lesson = lesson_session
+              lesson_booking.current_purchase = lesson_package_purchase
+            end
 
-          sale_line_item = SaleLineItem.create_from_lesson_package(current_user, sale, lesson_package_type, lesson_booking)
+            sale_line_item = SaleLineItem.create_from_lesson_package(current_user, sale, lesson_package_type, lesson_booking)
 
-          price_info = charge_stripe_for_lesson(current_user, lesson_booking, lesson_package_type, sale_line_item, lesson_session, lesson_package_purchase, force)
+            price_info = charge_stripe_for_lesson(charge, current_user, lesson_booking, lesson_package_type, sale_line_item, lesson_session, lesson_package_purchase, force)
 
-          if price_info[:purchase] && price_info[:purchase].errors.any?
+            post_stripe_test_failure
+
+            if price_info[:purchase] && price_info[:purchase].errors.any?
+              purchase = price_info[:purchase]
+              raise ActiveRecord::Rollback
+            end
+
+            if !sale_line_item.valid?
+              raise "invalid sale_line_item object for user #{current_user.email} and lesson_booking #{lesson_booking.id}"
+            end
+            # sale.source = 'stripe'
+            sale.recurly_subtotal_in_cents = price_info[:subtotal_in_cents]
+            sale.recurly_tax_in_cents = price_info[:tax_in_cents]
+            sale.recurly_total_in_cents = price_info[:total_in_cents]
+            sale.recurly_currency = price_info[:currency]
+            sale.stripe_charge_id = price_info[:charge_id]
+            sale.save
+            stripe_charge = price_info[:charge]
             purchase = price_info[:purchase]
-            raise ActiveRecord::Rollback
+          else
+            # should not get out of testing.  This would be very rare (i.e., from a big regression).  Sale is always valid at this point.
+            puts "invalid sale object"
+            raise "invalid sale object"
           end
-
-          if !sale_line_item.valid?
-            raise "invalid sale_line_item object for user #{current_user.email} and lesson_booking #{lesson_booking.id}"
-          end
-          # sale.source = 'stripe'
-          sale.recurly_subtotal_in_cents = price_info[:subtotal_in_cents]
-          sale.recurly_tax_in_cents = price_info[:tax_in_cents]
-          sale.recurly_total_in_cents = price_info[:total_in_cents]
-          sale.recurly_currency = price_info[:currency]
-          sale.stripe_charge_id = price_info[:charge_id]
-          sale.save
-          stripe_charge = price_info[:charge]
-          purchase = price_info[:purchase]
-        else
-          # should not get out of testing.  This would be very rare (i.e., from a big regression).  Sale is always valid at this point.
-          puts "invalid sale object"
-          raise "invalid sale object"
-        end
-
       end
 
       {sale: sale, stripe_charge: stripe_charge, purchase: purchase}
     end
 
-    def self.charge_stripe_for_lesson(current_user, lesson_booking, lesson_package_type, sale_line_item, lesson_session = nil, lesson_package_purchase = nil, force = false)
+    def self.charge_stripe_for_lesson(charge, current_user, lesson_booking, lesson_package_type, sale_line_item, lesson_session = nil, lesson_package_purchase = nil, force = false)
       if lesson_package_purchase
         target = lesson_package_purchase
       elsif lesson_session
@@ -304,12 +304,24 @@ module JamRuby
       tax_in_cents = (subtotal_in_cents * tax_percent).round
       total_in_cents = subtotal_in_cents + tax_in_cents
 
+      lesson_id = lesson_session.id if lesson_session # not set if test drive
+      charge_id = charge.id if charge # not set if test drive
+
       stripe_charge = Stripe::Charge.create(
           :amount => total_in_cents,
           :currency => "usd",
           :customer => current_user.stripe_customer_id,
-          :description => target.stripe_description(lesson_booking)
+          :description => target.stripe_description(lesson_booking),
+          :metadata => {
+              lesson_package: purchase.id,
+              lesson_session: lesson_id,
+              charge: charge_id,
+              user: current_user.id
+          }
       )
+      if charge
+        charge.stripe_charge = stripe_charge
+      end
 
       sale_line_item.lesson_package_purchase = purchase
       sale_line_item.save

ruby/lib/jam_ruby/models/teacher_payment_charge.rb

@@ -23,7 +23,7 @@ module JamRuby
 
       # source will let you supply a token.  But... how to get a token in this case?
 
-      stripe_charge = Stripe::Charge.create(
+      @stripe_charge = Stripe::Charge.create(
           :amount => amount_in_cents,
           :currency => "usd",
           :customer => APP_CONFIG.stripe[:source_customer],

ruby/lib/jam_ruby/models/user.rb

@@ -1882,6 +1882,9 @@ module JamRuby
       verifier.generate(user.id)
     end
 
+    def admin_name
+      "#{name} (#{(email)})"
+    end
     # URL to jam-admin
     def admin_url
       APP_CONFIG.admin_root_url + "/admin/users/" + id

ruby/spec/jam_ruby/flows/normal_lesson_spec.rb

@@ -179,6 +179,8 @@ describe "Normal Lesson Flow" do
       Timecop.freeze((24 + 24 + 24 + 3).hours.from_now)
       StripeMock.clear_errors
 
+      UserMailer.deliveries.clear
+
       # finally the user will get billed!
       LessonSession.hourly_check
 
@@ -206,7 +208,6 @@ describe "Normal Lesson Flow" do
       user.reload
       user.lesson_purchases.length.should eql 1
 
-
       LessonBooking.hourly_check
       payment.reload
       payment.amount_in_cents.should eql 3248

ruby/spec/jam_ruby/models/lesson_payment_charge_spec.rb

@@ -0,0 +1,59 @@
+require 'spec_helper'
+
+describe TeacherPaymentCharge, no_transaction: true do
+
+  let(:user) { FactoryGirl.create(:user) }
+  let(:user2) { FactoryGirl.create(:user) }
+  let(:teacher1_auth) { UserAuthorization.create(provider: 'stripe_connect', uid: stripe_account1_id, token: 'abc', refresh_token: 'abc', token_expiration: Date.today + 365, secret: 'secret') }
+  let(:teacher2_auth) { UserAuthorization.create(provider: 'stripe_connect', uid: stripe_account2_id, token: 'abc', refresh_token: 'abc', token_expiration: Date.today + 365, secret: 'secret') }
+  let(:teacher) { FactoryGirl.create(:user) }
+  let(:teacher2) { FactoryGirl.create(:user) }
+  let(:teacher_obj) { FactoryGirl.create(:teacher, user: teacher) }
+  let(:teacher_obj2) { FactoryGirl.create(:teacher, user: teacher2) }
+
+  let(:lesson) { normal_lesson(user, teacher, finish: true, accept: true, student_show: true, no_after_logic: true) }
+
+  describe "error behavior" do
+
+    before(:each) do
+      teacher_obj.touch
+      teacher_obj2.touch
+      teacher.teacher.stripe_account_id = stripe_account1_id
+      teacher2.teacher.stripe_account_id = stripe_account2_id
+    end
+
+    it "fails after stripe communication (in transaction)" do
+      LessonPaymentCharge.transaction do
+
+        Sale.stub(:post_stripe_test_failure).and_raise('bad logic after stripe call')
+
+        lesson.analyse
+
+        LessonPaymentCharge.count.should eql 1
+
+        charge = LessonPaymentCharge.first
+        charge.billing_attempts.should eql 1
+        charge.billed.should be_true
+        charge.billing_error_reason.should eql 'bad logic after stripe call'
+
+        Sale.count.should eql 0
+      end
+    end
+
+    it "fails after stripe communication (no transaction)" do
+
+      Sale.stub(:post_stripe_test_failure).and_raise('bad logic after stripe call')
+
+      lesson.analyse
+
+      LessonPaymentCharge.count.should eql 1
+
+      charge = LessonPaymentCharge.first
+      charge.billing_attempts.should eql 1
+      charge.billed.should be_true
+      charge.billing_error_reason.should eql 'bad logic after stripe call'
+
+      Sale.count.should eql 0
+    end
+  end
+end

ruby/spec/support/lesson_session.rb

@@ -116,8 +116,12 @@ def book_lesson(user, teacher, options)
     lesson.music_session.session_removed_at = end_time
     lesson.music_session.save!
     Timecop.travel(end_time + 1)
-    lesson.analyse
-    lesson.session_completed
+
+    unless options[:no_after_logic]
+      lesson.analyse
+      lesson.session_completed
+    end
+
   elsif options[:finish]
     # teacher & student get into session
     uh2 = FactoryGirl.create(:music_session_user_history, user: teacher, history: lesson.music_session, created_at: start, session_removed_at: end_time)
@@ -130,9 +134,10 @@ def book_lesson(user, teacher, options)
 
     Timecop.travel(end_time + 1)
 
-
-    lesson.analyse
-    lesson.session_completed
+    unless options[:no_after_logic]
+      lesson.analyse
+      lesson.session_completed
+    end
 
     if options[:monthly]
       LessonBooking.hourly_check

web/config/application.rb

@@ -428,6 +428,7 @@ if defined?(Bundler)
     config.stripe = {
         :publishable_key => 'pk_test_9vO8ZnxBpb9Udb0paruV3qLv',
         :secret_key      => 'sk_test_cPVRbtr9xbMiqffV8jwibwLA',
+        :source_customer => 'cus_8J2sI8iOHFEl2u',
         :client_id       => 'ca_8CgkjoHvfRMVqoQkcKdPt5Riy3dSPIlg',
         :ach_pct         => 0.008
     }

web/config/environments/test.rb

@@ -119,7 +119,8 @@ SampleApp::Application.configure do
   config.stripe = {
       :publishable_key => 'pk_test_HLTvioRAxN3hr5fNfrztZeoX',
       :secret_key      => 'sk_test_OkjoIF7FmdjunyNsdVqJD02D',
-      :source_customer => 'cus_88Vp44SLnBWMXq'
+      :source_customer => 'cus_88Vp44SLnBWMXq',
+      :ach_pct         => 0.008
   }
   config.jamclass_enabled = true
 end

web/lib/tasks/lesson.rake

@@ -31,7 +31,6 @@ namespace :lessons do
       puts lesson.errors.inspect
       raise "lesson failed"
     end
-    lesson = booking.lesson_sessions[0]
 
     puts "http://localhost:3000/client#/jamclass/lesson-booking/#{lesson.id}"
   end