Bug 30359 reduces the amount of work by removing the calculation of spent/ordered totals. We can go one step further here and obtain the entire list in one call, rather than manually calling for each budget
Created attachment 139767 [details] [review] Bug 31459: Use the same dropdown as when adding orders to a basket Current order receive code loops over each budget period and gets the hierarchy, this is slow when systems have many budgets. Additionally, the list includes inactive items by default We can switch to a single call, and add consistency by adopting the same dropdown as used when placing an order To test: 1 - Add some budgets with funds - ensure you have at least one inactive budget and fund 2 - Place an order, observe the fund selection menu 3 - Close basket, receive order, observe the fund menu 4 - Apply patch 5 - Cancel receipt 6 - Receive again 7 - Observe fund menu, confirm it matches the order placing menu 8 - Confirm inactive are not displayed by default, but are when button is checked
Need to investigate more tomorrow but it seems like if I select an inactive fund and save then on the invoice it shows the fund that was listed in the text "(Current: - Main fund)". Could be an indexing problem? My inactive fund was the first fund in the list, Main fund the second. Just a guess, will look into this.
I have confirmed the bug: changing active or inactive fund in acqui/orderreceive.pl doesn't work. The fund that was selected when adding the order line stays there and cannot be changed in the receive order step.
Created attachment 140065 [details] [review] Bug 31459: Use the same dropdown as when adding orders to a basket Current order receive code loops over each budget period and gets the hierarchy, this is slow when systems have many budgets. Additionally, the list includes inactive items by default We can switch to a single call, and add consistency by adopting the same dropdown as used when placing an order To test: 1 - Add some budgets with funds - ensure you have at least one inactive budget and fund 2 - Place an order, observe the fund selection menu 3 - Close basket, receive order, observe the fund menu 4 - Apply patch 5 - Cancel receipt 6 - Receive again 7 - Observe fund menu, confirm it matches the order placing menu 8 - Confirm inactive are not displayed by default, but are when button is checked
(In reply to Joonas Kylmälä from comment #3) > I have confirmed the bug: changing active or inactive fund in > acqui/orderreceive.pl doesn't work. The fund that was selected when adding > the order line stays there and cannot be changed in the receive order step. I had changed the name of the input, without changing code to read that, amended to use original name and selection now works
Created attachment 140168 [details] [review] Bug 31459: Use the same dropdown as when adding orders to a basket Current order receive code loops over each budget period and gets the hierarchy, this is slow when systems have many budgets. Additionally, the list includes inactive items by default We can switch to a single call, and add consistency by adopting the same dropdown as used when placing an order To test: 1 - Add some budgets with funds - ensure you have at least one inactive budget and fund 2 - Place an order, observe the fund selection menu 3 - Close basket, receive order, observe the fund menu 4 - Apply patch 5 - Cancel receipt 6 - Receive again 7 - Observe fund menu, confirm it matches the order placing menu 8 - Confirm inactive are not displayed by default, but are when button is checked Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
I think while it's faster, for the UX it's a regression :( Before this patch: The budget is used as top level entry and the funds show below it. This made it really easy to navigate even a long list here for libraries with various open/inactive budgets. Also the pull down stretched to accomodate longer entries. I actually liked this pull down best of all the ones we have (we should be more consistent there..) With the patch: On the sample data now even the short fund names are cut off. The budget level is missing, making orientation much harder. There are also some strange line breaks in the code: + >
Created attachment 141733 [details] [review] Bug 31459: (follow-up) Preserve budget period display This patch adds a level to the budget_loops and displays the budgets as optgroups for the funds
Created attachment 141734 [details] [review] Bug 31459: (follow-up) Preserve budget period display This patch adds a level to the budget_loops and displays the budgets as optgroups for the funds
Thank you, Nick :)
I am sorry, but I found another regression: If the user doesn't have permission for a fund, it doesn't show in the list. So far so good. But: if the order has been made for a fund the user has no permission for, currently we have an entry in the list as: Keep current ... With the patch set applied, the list jumps to the first 'allowed' fund in the list, so on saving you will change the fund and cannot keep the existing one. I was recently made aware of this feature, which is quite cool as we avoid data loss here in an efficient way. We need to keep doing that. For testing: make sure you remove budget_manage_all permission, then you can limit some funds to a library for example.
Created attachment 141960 [details] [review] Bug 31459: (follow-up) Return 'Keep current' option This patch restores the option to keep current budget, in the case that receiver can receive, but cannot order on current budget
Back here...
Created attachment 142019 [details] [review] Bug 31459: Use the same dropdown as when adding orders to a basket Current order receive code loops over each budget period and gets the hierarchy, this is slow when systems have many budgets. Additionally, the list includes inactive items by default We can switch to a single call, and add consistency by adopting the same dropdown as used when placing an order To test: 1 - Add some budgets with funds - ensure you have at least one inactive budget and fund 2 - Place an order, observe the fund selection menu 3 - Close basket, receive order, observe the fund menu 4 - Apply patch 5 - Cancel receipt 6 - Receive again 7 - Observe fund menu, confirm it matches the order placing menu 8 - Confirm inactive are not displayed by default, but are when button is checked Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> https://bugs.koha-community.org/show_bug.cgi?id=31559
Created attachment 142020 [details] [review] Bug 31459: (follow-up) Preserve budget period display This patch adds a level to the budget_loops and displays the budgets as optgroups for the funds Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> https://bugs.koha-community.org/show_bug.cgi?id=31559
Created attachment 142021 [details] [review] Bug 31459: (follow-up) Return 'Keep current' option This patch restores the option to keep current budget, in the case that receiver can receive, but cannot order on current budget Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> https://bugs.koha-community.org/show_bug.cgi?id=31559
Created attachment 142022 [details] [review] Bug 31459: (QA follow-up) Add missing TT filters Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> https://bugs.koha-community.org/show_bug.cgi?id=31559
Love. Thx, Nick! Btw... I filed bug 13956 in 2015...
I think this should be backported.
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for upcoming 22.05.08
applied to 21.11 for 21.11.15
Not backported to 21.05.x