search

frappe / books

Free Accounting Software
https://frappe.io/books
GNU Affero General Public License v3.0
2.68k stars 614 forks source link

Payments not showing up #808

Closed 18alantom closed 5 months ago

18alantom commented 5 months ago

https://github.com/frappe/books/assets/29507195/7a1aedbb-a984-44c7-88ac-9fd852f3d5ea

Payments don't show up in the List view when navigating to using the sidebar. Individual payments can be viewed when navigating to using Invoice links.

Isaac-GC commented 5 months ago

Hi @18alantom!

I tried to replicate the issue above but wasn't able to.

Could you provide some more information about what version of FrappeBooks you are using along with what OS/OS-Version you are using?

Thanks!

mildred commented 5 months ago

I'm having the issue, but it seems the issue lies in the database, in the Payment table the referenceType column is empty.

mildred commented 5 months ago

This is probably a migration issue that removed this value. new payments do not have this problem.

mildred commented 5 months ago

This is 67f9232160052a76ddf8e16a45918eac7b5a9f60 that introduced the referenceType column. The column value was marked as required but this prevented the migration to succeed so the required setting was removed and migrated payments now have a NULL value for referenceType.

We should add a migration that run UPDATE Payment SET referenceType = 'SalesInvoice' WHERE referenceType IS NULL

mildred commented 5 months ago

The required was removed in 2d52ce8a25fe07bd2578e9164265ebe4fb7a0260

pulsar17 commented 5 months ago

Hi @mildred, facing the same issue that the "Sales Payments" page does not show any entries after upgrade to 0.20.0.

Wouldn't the proposed migration change the referenceType for all Payments which currently have it null to SalesInvoice?

That would mean all the "Purchase Payments" would also show up in "Sales Payments" after the migration.

The offending commit changed filters for both the Payment types:

https://github.com/frappe/books/commit/67f9232160052a76ddf8e16a45918eac7b5a9f60#diff-aa9c6d8495fb7b57d6dbc231fcf18a654ed1e4c98ed91739ebdb3e85e65e96ab

and the migration should also consider that and instead update the referenceType based on paymentType field:

UPDATE Payment SET referenceType = 'SalesInvoice' WHERE paymentType = 'Receive';
UPDATE Payment SET referenceType = 'PurchaseInvoice' WHERE paymentType = 'Pay';

I might be wrong but this is what I could infer from taking a cursory look at the code.

mildred commented 5 months ago

Good point. I was under the impression that new payments since paymentType was introduced always filled the paymentType (since it was marked as required, it would have been a bug not to fill it).

So the migration not only needs to differenciate the paymentType as you suggest for existing payments, but also ensure that all future payments will not have a NULL paymentType.

mildred commented 5 months ago

Ready for release a fix soon in v0.20.1