Fix fatal error in reports helper for orders without payments

[Matt-Yorkley]

What? Why?

Closes #5572

A form helper was trying to write create options for a dropdown of payment methods on orders, but the orders did not necessarily have payments (or payment methods), resulting in a slug.

What should we test?

The payment methods report page loads correctly if the hub has orders without payments

Release notes

Fixed an issue in payments report for orders without payments.

[luisramos0]

👍

[filipefurtad0]

Hey @Matt-Yorkley,

After a chat in Slack, @sauloperez made some changes in the DB creating a similar order to the one causing the issue originally. The idea was trying to reproduce this in staging, to make sure the PR solves it. This was order R805323111:

This indeed triggered a snail/bugsnag before:
https://openfoodnetwork.slack.com/archives/CEF14NU3V/p1591805246325400

but also after staging this PR:
https://openfoodnetwork.slack.com/archives/CEF14NU3V/p1591805913325700

Adding the feedback-needed label for further checks, and access whether further dev work is required.

[sauloperez]

I managed to reproduce it and fix it on my machine. I simply removed the payment for one of the orders that showed up in the report and my patched worked. Could you take another look @filipefurtad0 ?

[Matt-Yorkley]

@sauloperez it looks like we jumped on this at the same time...

[sauloperez]

and my change is now gone 😅

I don't see what you changed with your force-push. Mine was

commit f322bac7668d91658c4a0c3c8c4a1f3be5428a2d
Author: Pau Perez <[email protected]>
Date:   Thu Jun 11 11:17:25 2020 +0200

    Defend from an order w/o payments building table

diff --git a/lib/open_food_network/order_cycle_management_report.rb b/lib/open_food_network/order_cycle_management_report.rb
index 49fb2e736..73a0ea34d 100644
--- a/lib/open_food_network/order_cycle_management_report.rb
+++ b/lib/open_food_network/order_cycle_management_report.rb
@@ -77,7 +77,7 @@ module OpenFoodNetwork
        ba.phone,
        order.shipping_method.andand.name,
        order.payments.first.andand.payment_method.andand.name,
-       order.payments.first.amount,
+       order.payments.first.andand.amount,
        OpenFoodNetwork::UserBalanceCalculator.new(order.email, order.distributor).balance]
     end
 
@@ -92,7 +92,7 @@ module OpenFoodNetwork
        sa.phone,
        order.shipping_method.andand.name,
        order.payments.first.andand.payment_method.andand.name,
-       order.payments.first.amount,
+       order.payments.first.andand.amount,
        OpenFoodNetwork::UserBalanceCalculator.new(order.email, order.distributor).balance,
        has_temperature_controlled_items?(order),
        order.special_instructions]

Would you mind applying it?

[filipefurtad0]

Hey @Matt-Yorkley and @sauloperez,

The report still is broken, after staging:

Moving back to dev.

[sauloperez]

@filipefurtad0 this time we encountered another issue that is being discussed in #5561. Some orders didn't have billing address and that's something we assumed existed.

This was it: https://app.bugsnag.com/open-food-france/open-food-france-prod/errors/5ee107d3dd8f6c0018b1c294?filters[event.since][0]=1d&filters[error.status][0][type]=eq&filters[error.status][0][value]=open&filters[app.release_stage][0][value]=staging&filters[app.release_stage][0][type]=eq

It surprised me that I could do the following from the console without any failure:

irb(main):002:0> order.bill_address = nil       
=> nil                                          
irb(main):003:0> order.save!        
   (1.3ms)  COMMIT                              
=> true      

This makes me think we don't have any validation for this either at app or DB level. In any case, this needs a deeper look.

[sauloperez]

Something else I just spot is that I can do a order.payments.first.destroy while order.state = 'complete'.

IMO, we shouldn't let this happen in that state. I believe that from an accounting perspective this is a red flag although it removes some flexibility that may come in handy when "fixing" some edge cases as a hub manager.

[filipefurtad0]

Hi @sauloperez and @Matt-Yorkley,

This PR is working as expected and preventing the report to break.

This was tested after manually corrupting orders both via psql (removing payments - thanks @sauloperez 👍 ), in FR (R805323111, now deleted) and ES-staging (R620504317, id=1979) - see below note empty shipment_state as well:

  id  |   number   | item_total | total |  state   | adjustment_total | user_id |         created_at         |         updated_at         |        completed_at        | bill_address_id | ship_address_id | payment_total | shipment_state | payment_state |                 email                  | special_instructions | distributor_id | order_cycle_id | currency | last_ip_address | customer_id | created_by_id 
------+------------+------------+-------+----------+------------------+---------+----------------------------+----------------------------+----------------------------+-----------------+-----------------+---------------+----------------+---------------+----------------------------------------+----------------------+----------------+----------------+----------+-----------------+-------------+---------------
 1979 | R620504317 |       0.00 |  0.00 | complete |             0.00 |      41 | 2020-06-11 15:55:52.315506 | 2020-06-11 16:07:31.662329 | 2020-06-11 16:07:31.662329 |            1413 |            1414 |         10.00 |                | balance_due   | [email protected] | 

Before the PR the Payment Methods Report was breaking - after staging it isn't. The page renders well and Order R620504317 now shows up:

As agreed in Slack, the remaining issues concerning this will be tackled in other PRs.
Ready to go.