Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finance ordergroup balance sums are misleading + performance issue #1044

Closed
twothreenine opened this issue Feb 17, 2024 · 2 comments · Fixed by #1051
Closed

Finance ordergroup balance sums are misleading + performance issue #1044

twothreenine opened this issue Feb 17, 2024 · 2 comments · Fixed by #1051

Comments

@twothreenine
Copy link
Contributor

The feature introduced in #1017 as requested in #382 takes the sum of all transactions of a certain transaction class.
However, these include foodcoop transactions as well as transactions from deleted ordergroups, with both aren't listed in the table.

I've reproduced this in the international demo by

  1. creating a foodcoop transaction of 20 € (transaction class Standard)
  2. creating an ordergroup deletion test, granting 100 € (transaction class Standard), then deleting the ordergroup

Now, the sum of Standard doesn't match the values above it (should be 110 €, but is 230 €).
grafik

The "total" value suggests to be the sum of the values above, so I think both foodcoop transactions and transactions from deleted ordergroups should be excluded from this calculation. For my understanding of foodcoop transactions, it wouldn't make sense to include a Foodcoop ordergroup in this table.

Performance:
I also noticed that, in Foodsofts with a lot of transactions over many years, this menu loads significantly slower now (takes more than 20 s in ours.) Also each time when I switch between page sizes (20/50/100/500) or between pages.

@yksflip
Copy link
Member

yksflip commented Mar 7, 2024

wow, thank you. No idea why I didn't think about that. Also I completely ignored the filtering/pagination ... I think I found a good solution in #1051 for that. It should also improve the performance.
Would you like to give that a test before I merge it? :)

@twothreenine
Copy link
Contributor Author

Awesome!
I didn't test for pagination and performance, but it works now for foodcoop transactions and deleted ordergroups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants