[Spree Upgrade] Subs - Remove process_payments! override

[mkllnk]

Description

As summarised in https://github.com/openfoodfoundation/openfoodnetwork/wiki/Spree-Upgrade:-Checkout-customisations, we backported functionality from Spree 2 into our code. With the Spree upgrade, we can get rid of the code:

openfoodnetwork/app/models/spree/order_decorator.rb

Lines 327 to 344 in 32846af

	# Override of existing Spree method. Can remove when we reach 2-0-stable
	# See commit: https://github.com/spree/spree/commit/5fca58f658273451193d5711081d018c317814ed
	# Allows GatewayError to show useful error messages in checkout
	def process_payments!
	pending_payments.each do |payment|
	break if payment_total >= total
	payment.process!
	if payment.completed?
	self.payment_total += payment.amount
	end
	end
	rescue Spree::Core::GatewayError => e # This section changed
	result = !!Spree::Config[:allow_checkout_on_gateway_error]
	errors.add(:base, e.message) and return result
	end

Expected Behavior

Less code, but everything still works.

Actual Behavior

Duplicate code. We override Spree's code with the same functionality.

Steps to Reproduce

1. Look at the code. ;-)

This is part of the Spree upgrade and the proposed change should be merged into the 2-0-stable branch.

[mkllnk]

The Spree 2 method is almost exactly the same as our override. The only difference is an update in Spree 2 which would be good to have as well: openfoodfoundation/spree@884c0ff

[mkllnk]

It seems like removing the override breaks a few tests, probably due to the new check for the presence of pending payments.

# Semaphore run 2
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:418 # Admin::SubscriptionsController cancel json as an enterprise user with authorisation when at least one associated order is still 'open' when no 'open_orders' directive has been provided renders an error, asking what to do
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:429 # Admin::SubscriptionsController cancel json as an enterprise user with authorisation when at least one associated order is still 'open' when 'keep' has been provided as the 'open_orders' directive renders the cancelled subscription as json, and does not cancel the open order
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:449 # Admin::SubscriptionsController cancel json as an enterprise user with authorisation when at least one associated order is still 'open' when 'cancel' has been provided as the 'open_orders' directive renders the cancelled subscription as json, and cancels the open order
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:517 # Admin::SubscriptionsController pause json as an enterprise user with authorisation when at least one associated order is still 'open' when no 'open_orders' directive has been provided renders an error, asking what to do
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:528 # Admin::SubscriptionsController pause json as an enterprise user with authorisation when at least one associated order is still 'open' when 'keep' has been provided as the 'open_orders' directive renders the paused subscription as json, and does not cancel the open order
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:548 # Admin::SubscriptionsController pause json as an enterprise user with authorisation when at least one associated order is still 'open' when 'cancel' has been provided as the 'open_orders' directive renders the paused subscription as json, and cancels the open order
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:616 # Admin::SubscriptionsController unpause json as an enterprise user with authorisation when at least one order in an open order cycle is 'complete' when no associated orders are 'canceled' renders the unpaused subscription as json, leaves the order untouched
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:634 # Admin::SubscriptionsController unpause json as an enterprise user with authorisation when at least one order in an open order cycle is 'complete' when at least one associate orders is 'canceled' when no 'canceled_orders' directive has been provided renders a message, informing the user that canceled order can be resumed
rspec ./spec/controllers/admin/subscriptions_controller_spec.rb:645 # Admin::SubscriptionsController unpause json as an enterprise user with authorisation when at least one order in an open order cycle is 'complete' when at least one associate orders is 'canceled' when 'notified' has been provided as the 'canceled_orders' directive renders the unpaused subscription as json, leaves the order untouched

# Semaphore run 3
rspec ./spec/controllers/admin/proxy_orders_controller_spec.rb:89 # Admin::ProxyOrdersController resume json as a regular user redirects to unauthorized
rspec ./spec/controllers/admin/proxy_orders_controller_spec.rb:100 # Admin::ProxyOrdersController resume json as an enterprise user without authorisation redirects to unauthorized
rspec ./spec/controllers/admin/proxy_orders_controller_spec.rb:110 # Admin::ProxyOrdersController resume json as an enterprise user with authorisation when resuming succeeds renders the resumed proxy_order as json
rspec ./spec/controllers/admin/proxy_orders_controller_spec.rb:122 # Admin::ProxyOrdersController resume json as an enterprise user with authorisation when resuming fails shows an error
rspec ./spec/jobs/subscription_placement_job_spec.rb:140 # SubscriptionPlacementJob processing a subscription order when the order is already complete records an issue and ignores it

The tests break around subscriptions. The problem is that subscriptions override pending_payments to return an empty array unless the order cycle is closed. This is a hack we need to solve before we can remove our override.

[mkllnk]

I just had a look. This pull request removed the override of process_payments! which was based on v1. Spree v2 has an additional check: an order can't require payment but then has no payment to process.

 StateMachine::InvalidTransition:
   Cannot transition state via :next from :payment (Reason(s): No pending payments)

A lot of our specs didn't care and don't create payments before transitioning them to complete. They all fail now.

rspec ./spec/models/proxy_order_spec.rb:145
rspec ./spec/models/proxy_order_spec.rb:156
rspec ./spec/controllers/spree/admin/orders_controller_spec.rb:12
rspec ./spec/services/advance_order_service_spec.rb:13
rspec ./spec/services/advance_order_service_spec.rb:39
rspec ./spec/controllers/spree/admin/orders/customer_details_controller_spec.rb:42