Refactor Payment.risky

Use AR and existing scopes to build the query instead of SQL.

Add a spec ensuring failed payments are considered risky.

Author: elia

core/app/models/spree/payment.rb

@@ -54,7 +54,7 @@ class Payment < Spree::Base
     scope :processing, -> { with_state('processing') }
     scope :failed, -> { with_state('failed') }
 
-    scope :risky, -> { where("avs_response IN (?) OR (cvv_response_code IS NOT NULL and cvv_response_code != 'M') OR state = 'failed'", RISKY_AVS_CODES) }
+    scope :risky, -> { failed.or(where(avs_response: RISKY_AVS_CODES)).or(where.not(cvv_response_code: [nil, '', 'M'])) }
     scope :valid, -> { where.not(state: %w(failed invalid void)) }
 
     scope :store_credits, -> { where(source_type: Spree::StoreCredit.to_s) }

core/spec/models/spree/payment_spec.rb

@@ -46,14 +46,19 @@
     )
   end
 
-  context '.risky' do
+  context 'risk analysis' do
     let!(:payment_1) { create(:payment, avs_response: 'Y', cvv_response_code: 'M', cvv_response_message: 'Match') }
     let!(:payment_2) { create(:payment, avs_response: 'Y', cvv_response_code: 'M', cvv_response_message: '') }
     let!(:payment_3) { create(:payment, avs_response: 'A', cvv_response_code: 'M', cvv_response_message: 'Match') }
     let!(:payment_4) { create(:payment, avs_response: 'Y', cvv_response_code: 'N', cvv_response_message: 'No Match') }
+    let!(:payment_5) { create(:payment, avs_response: 'Y', cvv_response_code: 'M', cvv_response_message: '', state: 'failed') }
+
+    describe '.risky' do
+      it 'fetches only risky payments' do
+        expect(subject.class.risky.to_a).to match_array([payment_3, payment_4, payment_5])
+      end
+    end
 
-    it 'should not return successful responses' do
-      expect(subject.class.risky.to_a).to match_array([payment_3, payment_4])
     end
   end