Bug 31459 - Make order receive page faster on systems with many budgets
Summary: Make order receive page faster on systems with many budgets
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 30359
Blocks: 32694 34006
  Show dependency treegraph
 
Reported: 2022-08-25 11:27 UTC by Nick Clemens
Modified: 2023-12-28 20:47 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.08, 21.11.15


Attachments
Bug 31459: Use the same dropdown as when adding orders to a basket (7.93 KB, patch)
2022-08-25 11:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31459: Use the same dropdown as when adding orders to a basket (7.93 KB, patch)
2022-09-02 06:09 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31459: Use the same dropdown as when adding orders to a basket (8.07 KB, patch)
2022-09-02 17:20 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 31459: (follow-up) Preserve budget period display (7.54 KB, patch)
2022-10-12 14:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31459: (follow-up) Preserve budget period display (7.46 KB, patch)
2022-10-12 14:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31459: (follow-up) Return 'Keep current' option (1.44 KB, patch)
2022-10-17 11:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31459: Use the same dropdown as when adding orders to a basket (8.18 KB, patch)
2022-10-17 19:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31459: (follow-up) Preserve budget period display (7.57 KB, patch)
2022-10-17 19:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31459: (follow-up) Return 'Keep current' option (1.55 KB, patch)
2022-10-17 19:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31459: (QA follow-up) Add missing TT filters (1.49 KB, patch)
2022-10-17 19:32 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-08-25 11:27:41 UTC
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
Comment 1 Nick Clemens 2022-08-25 11:32:53 UTC
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
Comment 2 Joonas Kylmälä 2022-08-31 20:21:43 UTC
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.
Comment 3 Joonas Kylmälä 2022-09-01 16:30:53 UTC
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.
Comment 4 Nick Clemens 2022-09-02 06:09:10 UTC
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
Comment 5 Nick Clemens 2022-09-02 06:11:14 UTC
(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
Comment 6 Joonas Kylmälä 2022-09-02 17:20:20 UTC
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>
Comment 7 Katrin Fischer 2022-10-03 14:52:28 UTC
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:
+                    >
Comment 8 Nick Clemens 2022-10-12 14:57:30 UTC Comment hidden (obsolete)
Comment 9 Nick Clemens 2022-10-12 14:58:35 UTC
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
Comment 10 Katrin Fischer 2022-10-16 11:40:49 UTC
Thank you, Nick :)
Comment 11 Katrin Fischer 2022-10-16 12:08:11 UTC
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.
Comment 12 Nick Clemens 2022-10-17 11:58:08 UTC
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
Comment 13 Katrin Fischer 2022-10-17 19:12:18 UTC
Back here...
Comment 14 Katrin Fischer 2022-10-17 19:32:12 UTC
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
Comment 15 Katrin Fischer 2022-10-17 19:32:17 UTC
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
Comment 16 Katrin Fischer 2022-10-17 19:32:22 UTC
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
Comment 17 Katrin Fischer 2022-10-17 19:32:29 UTC
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
Comment 18 Katrin Fischer 2022-10-17 19:39:32 UTC
Love. Thx, Nick!

Btw... I filed bug 13956 in 2015...
Comment 19 Tomás Cohen Arazi 2022-10-24 18:43:06 UTC
I think this should be backported.
Comment 20 Tomás Cohen Arazi 2022-10-24 18:44:39 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 21 Lucas Gass 2022-12-05 22:50:33 UTC
Backported to 22.05.x for upcoming 22.05.08
Comment 22 Arthur Suzuki 2022-12-16 15:51:25 UTC
applied to 21.11 for 21.11.15
Comment 23 wainuiwitikapark 2023-03-15 01:05:32 UTC
Not backported to 21.05.x