fix broken custom payment history #448 #449
Conversation
| @@ -36,7 +36,7 @@ public function transaction(): BelongsTo { | |||
| public function getFlow(string $payer_type, int $payer_id, string $period, $limit = null, string $order = 'ASC') { | |||
| $sql = 'SELECT sum(amount) as amount, payment_type, CONCAT_WS("/", '.$period.') as aperiod from'. | |||
| ' payment_histories where payer_id=:payer_id and payer_type=:payer_type '. | |||
| 'GROUP by concat( '.$period.'), payment_type ORDER BY created_at '.$order; | |||
| 'GROUP by aperiod, payment_type ORDER BY aperiod '.$order; | |||
There was a problem hiding this comment.
I don't think we should change the ORDER BY clause here. Because then the result is weird. For example:
It get's ordered lexiographically. And then it doesn't match a timeline
(12-2023 appearing in the middle of 2024)
There was a problem hiding this comment.
Good catch! Had to remove created_at to get the query to work. Now that I understand this better I think I could get this work.
There was a problem hiding this comment.
Yeah, the error is very misleading in this situation. The ORDER BY created_at has nothing to do with the error, even though the error suggest it 😛.
In fact, if you remove the ORDER BY created_at entirely, the error still persists.
It's only about the GROUP BY clause.
There was a problem hiding this comment.
Updated to order by created_at, the bar chart is now orderly correctly.
|
Adding a little more context as to why this issue started appearing. In #385 we introduced That's why started to appear. |
beesaferoot
force-pushed
the
fix-customer-page-payment-history
branch
from
3d7ecea to
b8d131c
Compare
|
@munyanezaarmel Please help review this fix. |
Brief summary of the change made
Fixes payment history display on customer detail page. Closes #448
Are there any other side effects of this change that we should be aware of?
Describe how you tested your changes?
Pull Request checklist
Please confirm you have completed any of the necessary steps below.