Remove N+1 fetching payments in report

[sauloperez]

What? Why?

Removes an N+1 related to payments

What should we test?

The payments report should work as expected while being a little faster.

Release notes

Remove N+1 from payments report
Changelog Category: Changed

[filipefurtad0]

Hey @sauloperez,
This relates to #5492, right? I'd test one after the other then.

[sauloperez]

I can merge them together if that's going to make it easier for you by they touch two different reports.

[filipefurtad0]

Thanks @sauloperez, not necessary. Thought this related to eventual performance issues introduced by the PR I mentioned. No need to merge them 👍

[filipefurtad0]

Hey @sauloperez ,

Tested this in Staging-UK, as superadmin.
In Payments by Type there is a decrease of around 3 sec. load time 🎉

However - and not introduced by this PR - the Payment Totals appears to be broken, across staging servers (tested in UK, FR, ES):

https://openfoodnetwork.slack.com/archives/CEF14NU3V/p1592334661341000

Perhaps related with #5572?

Update:
I see now I haven't checked Payment Totals, while testing PR #5573 - sorry for that. This needs a second look. I'm wondering if the original issue should be re-opened? Adding the feedback-label here.

[sigmundpetersen]

@filipefurtad0 there's an issue with snail on Payment Totals #4977.
It has been switching between s3 and s2. What do you think? S2?

[Matt-Yorkley]

Looking at it again, I think something like this might be significantly more efficient:

payments = Spree::Payment.completed.where(order_id: orders.select('spree_orders.id'))

[sauloperez]

Yes, I also wanted to get rid of this map but it was so quick and non-distracting to do this that I thought to improve in a later PR.