Finalize account invoices

Product Development
1

Yesterday I stumbled upon https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/jobs/finalize_account_invoices.rb which updates shipping_method_id by setting the default shipping method to order, thus directly related to the changes on Order in Spree 2.0.

I can’t quite understand what is the intent of that class by checking the git history. Aren’t we overriding the payment and shipping methods that the customer specified to place the order? Can you @oeoeaio shed some light on this?

2

This class is part of the system that charges hubs for using OFN. It does so by creating orders. These are not orders for food that customers place, but instead are orders for OFN usage, so to speak. Thus, they’re technically not “shipped” anywhere, so they’re just given a default shipping method. @oeoeaio wrote this and should be able to confirm.

3

Hey @sauloperez,

The “orders” used by this class are not real orders, they are basically “invoices” that enterprise owners need to pay for use of an open food network instance. The amount that the enterprise owner is charged is configurable by the instance, and can be based on a combination of a percentage of their turnover, fixed fees, caps and floors and trial periods. The “orders” you are seeing here are just objects to hold the billing information for a particular enterprise owner for a given month. From memory, the only reason they have a shipping method at all is because there is a validation on Spree::Order that requires it. It is not important for the functionality provided, so I think if we get to that point the code that updates shipping_method_id can be safely removed.

Does that answer your question?

4

This was truly unexpected. Needless to say someday we’ll have to implement this in a proper way or we’ll end up extending the Order model in strange ways for this feature to work.

I will add some documentation to the class so the next reader is aware and make my way for the upgrade.

Thanks guys, this is a lot clearer now.

5

This might have changed lately:

irb(main):008:0> FactoryGirl.create(:order) 
...
irb(main):010:0> order             
=> #<Spree::Order id: 1, number: "R464722464", item_total: #<BigDecimal:55903b9c6820,'0.0',9(27)>, total: #<BigDecimal:55903c386400,'0.0',9(18)>, state: "cart", adjustment_total: #<BigDecimal:55903b9c60a0,'0.0',9(27)>, user_id: 2, created_at: "2017-07-06 07:19:24", updated_at: "2017-07-06 07:19:24", completed_at: nil, bill_address_id: nil, ship_address_id: nil, payment_total: #<BigDecimal:55903b9c5060,'0.0',9(27)>, shipping_method_id: nil, shipment_state: nil, payment_state: nil, email: "llewellyn.hahn@kerlukelueilwitz.uk", special_instructions: nil, distributor_id: nil, order_cycle_id: nil, currency: "AUD", last_ip_address: nil, cart_id: nil, customer_id: nil>                               
irb(main):011:0> order.shipping_method                                
=> nil                             
irb(main):012:0> order.valid?
...
=> true

So, orders do not require a shipping method.

6

PR opened https://github.com/openfoodfoundation/openfoodnetwork/pull/1662 If you give your :thumbsup: we can merge it.

7

Validations on orders are dependent on the order state so maybe it just isn’t being triggered? I am fairly sure that some kind of error will be encountered when trying to process an order to completion if it doesn’t have a shipping method, though it may not be a validation error, I can’t recall…

8

True. It’s due to a validates :shipping_method, :presence => true in Spree’s core/app/models/spree/shipment.rb

When sending a request with nil shipping_method_id I get:

ActiveRecord::RecordInvalid (Validation failed: Shipping method can't be blank):                                                            
  app/controllers/checkout_controller.rb:122:in `advance_order_state' 
  app/controllers/checkout_controller.rb:30:in `update'

The app depends indeed on having a shipping method. If I remove that validation this is what I get:

NoMethodError (undefined method `adjustment_label' for nil:NilClass): 
  app/models/spree/shipment_decorator.rb:4:in `ensure_correct_adjustment_with_included_tax'                                                 
  app/controllers/checkout_controller.rb:122:in `advance_order_state' 
  app/controllers/checkout_controller.rb:30:in `update'

I’ll update the PR accordingly

9

This is now outdated, we dont have account invoices anymore.