Move payment method fetching logic to Model, rewrite tests for EveryPay component

app/controllers/registrar/payments_controller.rb

@@ -10,11 +10,9 @@ class Registrar
 
     def pay
       invoice = Invoice.find(params[:invoice_id])
-      payment_type = params[:bank]
+      channel = params[:bank]
 
-      channel = PaymentOrder.type_from_shortname(payment_type)
-
-      @payment_order = PaymentOrder.new(type: channel, invoice: invoice)
+      @payment_order = PaymentOrder.create_with_type(type: channel, invoice: invoice)
       @payment_order.save && @payment_order.reload
 
       @payment_order.return_url = registrar_return_payment_with_url(@payment_order)

app/models/payment_order.rb

@@ -29,6 +29,12 @@ class PaymentOrder < ApplicationRecord
     supported
   end
 
+  def self.create_with_type(type:, invoice:)
+    channel = PaymentOrder.type_from_shortname(type)
+
+    PaymentOrder.new(type: channel, invoice: invoice)
+  end
+
   # Name of configuration namespace
   def self.config_namespace_name; end
 
@@ -67,11 +73,11 @@ class PaymentOrder < ApplicationRecord
   end
 
   def complete_transaction
+    return NoMethodError unless payment_received?
+
     paid!
     transaction = composed_transaction
-    transaction.save!
-    transaction.bind_invoice(invoice.number)
-
+    transaction.save! && transaction.bind_invoice(invoice.number)
     return unless transaction.errors.any?
 
     worded_errors = 'Failed to bind. '

app/models/payment_orders/every_pay.rb

@@ -76,6 +76,8 @@ module PaymentOrders
       hmac_string = hmac_hash.map { |key, _v| "#{key}=#{hmac_hash[key]}" }.join('&')
       expected_hmac = OpenSSL::HMAC.hexdigest('sha1', KEY, hmac_string)
       expected_hmac == response['hmac']
+    rescue NoMethodError
+      false
     end
 
     def valid_amount?

test/fixtures/invoice_items.yml

@@ -4,3 +4,10 @@ one:
   quantity: 1
   unit: pc
   invoice: one
+
+two:
+  description: Acme services
+  price: 5
+  quantity: 1
+  unit: pc
+  invoice: unpaid

test/fixtures/invoices.yml

@@ -24,3 +24,30 @@ one:
   reference_no: 13
   number: 1
   description: Order nr 1 from registrar 1234567 second number 2345678
+
+unpaid:
+  issue_date: <%= Date.parse '2010-07-05' %>
+  due_date: <%= Date.parse '2010-07-06' %>
+  currency: EUR
+  seller_name: Seller Ltd
+  seller_reg_no: 1234
+  seller_iban: US75512108001245126199
+  seller_bank: Main Bank
+  seller_swift: swift
+  seller_email: info@seller.test
+  seller_country_code: US
+  seller_street: Main Street 1
+  seller_city: New York
+  seller_contact_name: John Doe
+  buyer: bestnames
+  buyer_name: Buyer Ltd
+  buyer_reg_no: 12345
+  buyer_email: info@buyer.test
+  buyer_country_code: GB
+  buyer_street: Main Street 2
+  buyer_city: London
+  vat_rate: 0.1
+  total: 16.50
+  reference_no: 13
+  number: 2
+  description: Order nr 2 from registrar 1234567 second number 2345678

test/fixtures/payment_orders.yml

@@ -0,0 +1,20 @@
+issued:
+  type: PaymentOrders::EveryPay
+  status: issued
+  invoice: one
+  response:
+  notes:
+
+paid:
+  type: PaymentOrders::EveryPay
+  status: paid
+  invoice: one
+  response: "{}"
+  notes:
+
+cancelled:
+  type: PaymentOrders::Seb
+  status: cancelled
+  invoice: one
+  response: "{}"
+  notes: User failed to make payment. Bank responded with code 1911

test/integration/registrar_area/invoices/payment_callback_test.rb

@@ -12,7 +12,7 @@ class PaymentCallbackTest < ApplicationIntegrationTest
     invoice = payable_invoice
     assert_matching_bank_transaction_exists(invoice)
 
-    request_params = every_pay_request_params.merge(invoice_id: invoice.id)
+    request_params = every_pay_request_params.merge(payment_order: invoice.id)
     post "/registrar/pay/callback/every_pay", params: request_params
 
     assert_response :ok

test/models/payment_orders/bank_link_test.rb

@@ -36,11 +36,11 @@ class BankLinkTest < ActiveSupport::TestCase
       'VK_MAC': 'CZZvcptkxfuOxRR88JmT4N+Lw6Hs4xiQfhBWzVYldAcRTQbcB/lPf9MbJzBE4e1/HuslQgkdCFt5g1xW2lJwrVDBQTtP6DAHfvxU3kkw7dbk0IcwhI4whUl68/QCwlXEQTAVDv1AFnGVxXZ40vbm/aLKafBYgrirB5SUe8+g9FE=',
       'VK_ENCODING': 'UTF-8',
       'VK_LANG': 'ENG'
-    }.with_indifferent_access
+    }.as_json
 
-    @completed_bank_link = PaymentOrders::BankLink.new(
-      'seb', @invoice, { response: params }
-    )
+    @completed_bank_link = PaymentOrder.new(type: 'PaymentOrders::Seb',
+                                            invoice: @invoice,
+                                            response: params)
   end
 
   def create_cancelled_bank_link
@@ -55,16 +55,18 @@ class BankLinkTest < ActiveSupport::TestCase
       'VK_MAC': 'PElE2mYXXN50q2UBvTuYU1rN0BmOQcbafPummDnWfNdm9qbaGQkGyOn0XaaFGlrdEcldXaHBbZKUS0HegIgjdDfl2NOk+wkLNNH0Iu38KzZaxHoW9ga7vqiyKHC8dcxkHiO9HsOnz77Sy/KpWCq6cz48bi3fcMgo+MUzBMauWoQ=',
       'VK_ENCODING': 'UTF-8',
       'VK_LANG': 'ENG'
-    }.with_indifferent_access
+    }.as_json
 
-    @cancelled_bank_link = PaymentOrders::BankLink.new(
-      'seb', @invoice, { response: params }
-    )
+    @cancelled_bank_link = PaymentOrder.new(type: 'PaymentOrders::Seb',
+                                            invoice: @invoice,
+                                            response: params)
   end
 
   def create_new_bank_link
     params = { return_url: 'return.url', response_url: 'response.url' }
-    @new_bank_link = PaymentOrders::BankLink.new('seb', @invoice, params)
+    @new_bank_link = PaymentOrder.new(type: 'PaymentOrders::Seb', invoice: @invoice)
+    @new_bank_link.return_url = 'return.url'
+    @new_bank_link.response_url = 'response.url'
   end
 
   def test_response_is_not_valid_when_it_is_missing

test/models/payment_orders/every_pay_test.rb

@@ -4,36 +4,38 @@ class EveryPayTest < ActiveSupport::TestCase
   def setup
     super
 
-    @invoice = invoices(:one)
+    @invoice = invoices(:unpaid)
     @invoice.update!(total: 12)
 
-    params = {
-      response:
-        {
-          utf8:                '✓',
-          _method:             'put',
-          authenticity_token:  'OnA69vbccQtMt3C9wxEWigs5Gpf/7z+NoxRCMkFPlTvaATs8+OgMKF1I4B2f+vuK37zCgpWZaWWtyuslRRSwkw==',
-          nonce:               '392f2d7748bc8cb0d14f263ebb7b8932',
-          timestamp:           '1524136727',
-          api_username:        'ca8d6336dd750ddb',
-          transaction_result:  'completed',
-          payment_reference:   'fd5d27b59a1eb597393cd5ff77386d6cab81ae05067e18d530b10f3802e30b56',
-          payment_state:       'settled',
-          amount:              '12.00',
-          order_reference:     'e468a2d59a731ccc546f2165c3b1a6',
-          account_id:          'EUR3D1',
-          cc_type:             'master_card',
-          cc_last_four_digits: '0487',
-          cc_month:            '10',
-          cc_year:             '2018',
-          cc_holder_name:      'John Doe',
-          hmac_fields:         'account_id,amount,api_username,cc_holder_name,cc_last_four_digits,cc_month,cc_type,cc_year,hmac_fields,nonce,order_reference,payment_reference,payment_state,timestamp,transaction_result',
-          hmac: 'efac1c732835668cd86023a7abc140506c692f0d',
-          invoice_id:          '1',
-        },
-    }
-    @every_pay = PaymentOrders::EveryPay.new('every_pay', @invoice, params)
-    @other_pay = PaymentOrders::EveryPay.new('every_pay', @invoice, {})
+    response = {
+      "utf8": '✓',
+      "_method": 'put',
+      "authenticity_token": 'OnA69vbccQtMt3C9wxEWigs5Gpf/7z+NoxRCMkFPlTvaATs8+OgMKF1I4B2f+vuK37zCgpWZaWWtyuslRRSwkw=="',
+      "nonce": '392f2d7748bc8cb0d14f263ebb7b8932',
+      "timestamp": '1524136727',
+      "api_username": 'ca8d6336dd750ddb',
+      "transaction_result": 'completed',
+      "payment_reference": 'fd5d27b59a1eb597393cd5ff77386d6cab81ae05067e18d530b10f3802e30b56',
+      "payment_state": 'settled',
+      "amount": '12.00',
+      "order_reference": 'e468a2d59a731ccc546f2165c3b1a6',
+      "account_id": 'EUR3D1',
+      "cc_type": 'master_card',
+      "cc_last_four_digits": '0487',
+      "cc_month": '10',
+      "cc_year": '2018',
+      "cc_holder_name": 'John Doe',
+      "hmac_fields": 'account_id,amount,api_username,cc_holder_name,cc_last_four_digits,cc_month,cc_type,cc_year,hmac_fields,nonce,order_reference,payment_reference,payment_state,timestamp,transaction_result',
+      "hmac": 'efac1c732835668cd86023a7abc140506c692f0d',
+      "invoice_id": '2'
+    }.as_json
+
+    @successful_payment = PaymentOrder.new(type: 'PaymentOrders::EveryPay',
+                                           invoice: @invoice,
+                                           response: response)
+
+    @failed_payment = @successful_payment.dup
+    @failed_payment.response['payment_state'] = 'cancelled'
 
     travel_to Time.zone.parse('2018-04-01 00:30:00 +0000')
   end
@@ -47,37 +49,37 @@ class EveryPayTest < ActiveSupport::TestCase
       transaction_type: 'charge',
       hmac_fields: 'account_id,amount,api_username,callback_url,customer_url,hmac_fields,nonce,order_reference,timestamp,transaction_type'
     }
-    form_fields = @every_pay.form_fields
+    form_fields = @successful_payment.form_fields
     expected_fields.each do |k, v|
       assert_equal(v, form_fields[k])
     end
   end
 
   def test_valid_response_from_intermediary?
-    assert(@every_pay.valid_response_from_intermediary?)
-    refute(@other_pay.valid_response_from_intermediary?)
+    assert(@successful_payment.valid_response_from_intermediary?)
+
+    @failed_payment.response = { 'what': 'definitely not valid everypay response' }
+    refute(@failed_payment.valid_response_from_intermediary?)
+  end
+
+  def test_valid_and_successful_payment_is_determined
+    assert(@successful_payment.payment_received?)
+    refute(@failed_payment.payment_received?)
   end
 
   def test_settled_payment?
-    assert(@every_pay.settled_payment?)
-    other_pay = PaymentOrders::EveryPay.new(
-      'every_pay', @invoice, {response: {payment_state: 'CANCELLED'}}
-    )
-    refute(other_pay.settled_payment?)
+    assert(@successful_payment.settled_payment?)
+    refute(@failed_payment.settled_payment?)
   end
 
-  def test_complete_transaction_calls_methods_on_transaction
-    mock_transaction = MiniTest::Mock.new
-    mock_transaction.expect(:sum= , '12.00', ['12.00'])
-    mock_transaction.expect(:paid_at= , Date.strptime('1524136727', '%s'), [Date.strptime('1524136727', '%s')])
-    mock_transaction.expect(:buyer_name=, 'John Doe', ['John Doe'])
-    mock_transaction.expect(:save!, true)
-    mock_transaction.expect(:autobind_invoice, AccountActivity.new)
+  def test_successful_payment_creates_bank_transaction
+    @successful_payment.complete_transaction
 
-    BankTransaction.stub(:find_by, mock_transaction) do
-      @every_pay.complete_transaction
-    end
+    transaction = BankTransaction.find_by(
+      sum: @successful_payment.response['amount'],
+      buyer_name: @successful_payment.response['cc_holder_name']
+    )
 
-    mock_transaction.verify
+    assert transaction.present?
   end
 end

test/models/payment_orders_test.rb

@@ -5,23 +5,21 @@ class PaymentOrdersTest < ActiveSupport::TestCase
     super
 
     @original_methods = ENV['payment_methods']
-    @original_seb_URL = ENV['seb_payment_url']
-    ENV['payment_methods'] = 'seb, swed, credit_card'
+    @original_seb_url = ENV['seb_payment_url']
+    ENV['payment_methods'] = 'seb, swed, every_pay'
     ENV['seb_payment_url'] = nil
-    @not_implemented_payment = PaymentOrders::Base.new(
-      'not_implemented', Invoice.new
-    )
+    @not_implemented_payment = PaymentOrder.new(invoice: Invoice.new)
   end
 
   def teardown
     super
 
     ENV['payment_methods'] = @original_methods
-    ENV['seb_payment_url'] = @original_seb_URL
+    ENV['seb_payment_url'] = @original_seb_url
   end
 
   def test_variable_assignment
-    assert_equal 'not_implemented', @not_implemented_payment.type
+    assert_nil @not_implemented_payment.type
     assert_nil @not_implemented_payment.response_url
     assert_nil @not_implemented_payment.return_url
     assert_nil @not_implemented_payment.form_url
@@ -45,14 +43,14 @@ class PaymentOrdersTest < ActiveSupport::TestCase
     end
   end
 
-  def test_that_create_with_type_raises_argument_error
-    assert_raise ArgumentError do
-      PaymentOrders.create_with_type("not_implemented", Invoice.new)
+  def test_can_not_create_order_with_invalid_type
+    assert_raise NameError do
+      PaymentOrder.create_with_type(type: 'not_implemented', invoice: Invoice.new)
     end
   end
 
-  def test_create_with_correct_subclass
-    payment = PaymentOrders.create_with_type('seb', Invoice.new)
-    assert_equal PaymentOrders::BankLink, payment.class
+  def test_can_create_with_correct_subclass
+    payment = PaymentOrder.create_with_type(type: 'seb', invoice: Invoice.new)
+    assert_equal PaymentOrders::Seb, payment.class
   end
 end