openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.1k stars 714 forks source link

Snail when running Payment Totals Report #4977

Closed emilyjeanrogers closed 1 year ago

emilyjeanrogers commented 4 years ago

Description

Aus customer Prom Coast Wholefoods Collective are getting a snail when running the Payment Totals Report. Snail is generated when including a date more than 12 months before today (11th March 2020). The snail comes up when trying to run this report as both a Manager of the Customer and as Superadmin. Have also tested for other Enterprises with no problems.

Expected Behavior

Report should run successfully for any date range.

Actual Behaviour

It shows a snail. The error has been logged: https://app.bugsnag.com/yaycode/openfoodnetwork/errors/5e68221b18c0ef00177d7370

Steps to Reproduce

  1. Log in as Superadmin (or add yourself as a manager for Prom Coast Wholefoods Collective)
  2. Go to Reports
  3. Select Payment Totals Report
  4. Set Date range for a date including more than 12 months prior to today's date
    OR
    set date range to include dates prior to 11th March 2019

Workaround

No workaround.

Severity

S2

bug-s2: a non-critical feature is broken, no workaround

Possible Fix

It looks like a payment method is missing. The safe navigation operator &. may help here:

NoMethodError · undefined method `name' for nil:NilClass 
lib/open_food_network/payments_report.rb:106
proc { |orders| orders.sum { |o| o.payments.select { |payment| payment.completed? && (payment.payment_method.name.to_s.include? "EFT") }.sum(&:amount) } },
RachL commented 4 years ago

@emilyjeanrogers I already had this because there was a particular problem with an order. Have you tried narrowing the search and see for which set of dates the error occurs?

kirstenalarsen commented 4 years ago

@emilyjeanrogers is this an s2? or is an s3 - not sure why it got downgraded? Seems likely connected to https://github.com/openfoodfoundation/openfoodnetwork/issues/5572

lin-d-hop commented 4 years ago

Very unlikely to be connected to #5572

5572 is specific to payment methods report and is related to an order not having a payment associated. It is a data corruption issue and renders this report totally unusable ALWAYS for any user that is associated with that order.

EDIT:

Could well be related and due to an order not having a payment associated: From Payments Report code -> payments.first.payment_method.name

emilyjeanrogers commented 4 years ago

I have reproduced this error today and been able to narrow it to a more specific date range. The snail occurs for any Payment Totals Report run for Prom Coast Food Collective prior to March 12th 2019.

sigmundpetersen commented 4 years ago

The prioritization was discussed here https://openfoodnetwork.slack.com/archives/CAVTM01QB/p1587133992174900

However, @filipefurtad0 has observed this when testing #5595 and I think is in favor of upping it to an S2. Filipe, maybe you could summarize your findings here?

lin-d-hop commented 4 years ago

There are two things happening here: 1) Inconsistent data on orders 2) Reports that won't load if there is not payment

@sauloperez helped us find the orders with inconsistent data in #5572 so we could remove them and make the report work again.

I would say that you could do an s2 for that, and that since this seems to occur very rarely we could make an s3 for the underlying issues.

@sauloperez Are you able to help find the broken orders for Prom Coast before 12th march 2019? And document the query you use to find them so we can fix these quickly when they come up in the future?

filipefurtad0 commented 4 years ago

Yes, as @sigmundpetersen points out, I bumped into this issue while checking something else.

I agree with points 1. and 2. @lin-d-hop. So I tried to narrow it down to such cases, and cancel these orders, so they don't appear in the reports - and this works to some extent, but not always. There is something else breaking the report other than the integrity of orders (points 1. and 2.) and the time-frame (in the description of this PR).

For example, in staging-katuma, these orders were placed in May, and have a cash payment, customer info, they seem like regular orders: image

However, setting the date range in the Payment Totals to include only these orders breaks the report anyway.

In staging-UK I tried to cancel out apparently broken orders, but same thing - report was broken anyway for all the dates. So, going day-by-day, I narrowed down to this day, which has the order(s) breaking the report: image

I can't explain these so far - these look like perfectly regular orders - so that makes me argue for an S2. Perhaps this is what @emilyjeanrogers observed by increasing the time range to generate the report?

filipefurtad0 commented 4 years ago

Below are the entries on DB from the 3 orders from staging-uk. I can't spot anything unusual here either. Thoughts?

   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 | currency | last_ip_address | order_cycle_id | customer_id | created_by_id 
--------+------------+------------+-------+----------+------------------+---------+----------------------------+----------------------------+---------------------------+-----------------+-----------------+---------------+----------------+---------------+-----------------------------------+----------------------+----------------+----------+-----------------+----------------+-------------+---------------
 701995 | R254238635 |      80.00 | 80.00 | complete |             0.00 |  208000 | 2020-05-17 21:36:38.923795 | 2020-05-17 21:37:13.847052 | 2020-05-17 21:37:13.56881 |          424100 |          424102 |         80.00 | ready          | paid          | filipefurtado+haciendac@gmail.com |                      |         201389 | GBP      | 127.0.0.1       |         203888 |        7395 |        208000
 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 | currency | last_ip_address | order_cycle_id | customer_id | created_by_id 
--------+------------+------------+-------+----------+------------------+---------+----------------------------+----------------------------+----------------------------+-----------------+-----------------+---------------+----------------+---------------+-----------------------------------+----------------------+----------------+----------+-----------------+----------------+-------------+---------------
 701994 | R740217288 |      20.00 | 20.00 | complete |             0.00 |  208000 | 2020-05-17 21:36:03.377958 | 2020-05-17 21:36:38.844988 | 2020-05-17 21:36:38.596334 |          424097 |          424099 |         20.00 | ready          | paid          | filipefurtado+haciendac@gmail.com |                      |         201389 | GBP      | 127.0.0.1       |         203888 |        7395 |        208000
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 | currency | last_ip_address | order_cycle_id | customer_id | created_by_id 
--------+------------+------------+-------+----------+------------------+---------+----------------------------+----------------------------+----------------------------+-----------------+-----------------+---------------+----------------+---------------+-----------------------------------+----------------------+----------------+----------+-----------------+----------------+-------------+---------------
 701993 | R822265384 |      40.00 | 40.00 | complete |             0.00 |  208000 | 2020-05-17 21:35:07.035622 | 2020-05-17 21:36:03.254395 | 2020-05-17 21:36:02.868835 |          424091 |          424095 |          0.00 | pending        | balance_due   | filipefurtado+haciendac@gmail.com |                      |         201389 | GBP      | 127.0.0.1       |         203888 |        7395 |        208000
sauloperez commented 4 years ago

Thanks this Sherlock work @filipefurtad0 ! it made my job a piece of cake!

It turns out that the only failure in UK, Katuma and FR stagings is

NoMethodError (undefined method `name' for nil:NilClass):
  lib/open_food_network/payments_report.rb:106:in `block (3 levels) in columns'

Particularly, from these orders from UK staging that you report, it's only due to R740217288. Its only payment has a deleted payment method. Therefore, our reports don't handle soft-deleted payment methods. I can confirm that because undeleting the method 200359 brought the report back.

In FR the issue is that the order R805323111 has no payment at all.

I'll check payments to Prom Coast Wholefoods Collective later.

Matt-Yorkley commented 4 years ago

Nice, we can just adapt it for soft-deletion :smile:

sauloperez commented 4 years ago

yes, that's what I'm about to do but first I'm finding out whether this is the issue this hub is experiencing. It could also be because a) payment is missing b) payment has a missing method.

sauloperez commented 4 years ago

@emilyjeanrogers I couldn't find of the potential root causes directly affecting orders distributed by either Prom Coast Food Collective or Prom Coast Food Collective Wholesale Market.

However, I did find that these orders are affected by a deleted payment method. So, I'm moving on with the fix for it.

sauloperez commented 4 years ago

For the record, this is how I found the potential root causes of this failure:

Complete orders without payment

select count(*)                                                                                 
from spree_orders                                                                               
left join spree_payments on spree_payments.order_id = spree_orders.id
where spree_payments.id is null
and spree_orders.state = 'complete';

Payments whose method is deleted

select count(*)                                                                                 
from spree_payments                                                                             
inner join spree_payment_methods on spree_payment_methods.id = spree_payments.payment_method_id
where spree_payment_methods.deleted_at is not null;

Payments without method

select count(*)
from spree_payments
left join spree_payment_methods on spree_payment_methods.id = spree_payments.payment_method_id
where spree_payment_methods.id is null;
emilyjeanrogers commented 4 years ago

@emilyjeanrogers I couldn't find of the potential root causes directly affecting orders distributed by either Prom Coast Food Collective or Prom Coast Food Collective Wholesale Market.

However, I did find that these orders are affected by a deleted payment method. So, I'm moving on with the fix for it.

@sauloperez So, does this mean we have identified a fix that is helpful in other cases, but not in the case of this specific customer (Prom Coast Food Collective) ie. reports will still fail for this customer?

sauloperez commented 4 years ago

sorry for my late response @emilyjeanrogers . Yes, that's it. Chances are that they were affected by this and we missed it though. Is this still happening?

RachL commented 4 years ago

@emilyjeanrogers Pau will be on holiday tonight and won't look at this for the next 2 weeks, so I'm unassigning him and move this back to all the things. Please feel free to close if this is solved. Or raise to s2 if this is really a blocker. Thanks!

emilyjeanrogers commented 4 years ago

I have reproduced this error again today and it hasn't changed. Attempting to run the Payment Totals Report for Prom Coast Food Collective for any date prior to March 12th 2019 will generate a snail.

Screen Shot 2020-08-12 at 11 38 49 AM

lin-d-hop commented 1 year ago

Closing. Can be reopened if it pops up but I suspect resolved in reports refactorring