Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once.
Created attachment 131391 [details] [review] Bug 30232: Logged in user performance for CanUserManageBasket() Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once. This patch optimises call for CanUserManageBasket(). You may benchmark on a vendor with a lot of baskets. Test plan 1) 1.1) Log in to staff interface as user A with all acquisition permissions 1.2) Set system preference 'AcqViewBaskets' to 'created or managed by staff member' 1.3) Create a new basket 1.4) Copy URL, ie /cgi-bin/koha/acqui/basket.pl?basketno=2 1.5) Click on 'Baskets' tab, you see the basket in table 2) 2.1) Log in to staff interface as user B with acquisition permissions but not 'order_manage_all' 2.2) Go to URL => You don't have access to basket 2.3) Click on 'Baskets' tab => you don't see the basket in table
Created attachment 131392 [details] [review] Bug 30232: Logged in user performance for CanUserManageBasket() Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once. This patch optimises call for CanUserManageBasket(). Also removes a useless refetch of logged in patron. You may benchmark on a vendor with a lot of baskets. Test plan 1) 1.1) Log in to staff interface as user A with all acquisition permissions 1.2) Set system preference 'AcqViewBaskets' to 'created or managed by staff member' 1.3) Create a new basket 1.4) Copy URL, ie /cgi-bin/koha/acqui/basket.pl?basketno=2 1.5) Click on 'Baskets' tab, you see the basket in table 2) 2.1) Log in to staff interface as user B with acquisition permissions but not 'order_manage_all' 2.2) Go to URL => You don't have access to basket 2.3) Click on 'Baskets' tab => you don't see the basket in table
Patch "Logged in user performance for CanUserManageBasket()" Page /cgi-bin/koha/acqui/booksellers.pl tested with 3000 baskets. Time goes from 30s to 20s.
Created attachment 131393 [details] [review] Bug 30232: Logged in user performance for CanUserUseBudget() Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once. This patch optimises call for CanUserUseBudget(); You may benchmark on a user with a lot of budgets acess. Test plan : Play with impacted pages and check budgets list is the same than wihtout patch
Created attachment 131395 [details] [review] Bug 30232: Logged in user performance for CanUserManageBasket() Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once. This patch optimises call for CanUserManageBasket(). Also removes a useless refetch of logged in patron. You may benchmark on a vendor with a lot of baskets. Test plan 1) 1.1) Log in to staff interface as user A with all acquisition permissions 1.2) Set system preference 'AcqViewBaskets' to 'created or managed by staff member' 1.3) Create a new basket 1.4) Copy URL, ie /cgi-bin/koha/acqui/basket.pl?basketno=2 1.5) Click on 'Baskets' tab, you see the basket in table 2) 2.1) Log in to staff interface as user B with acquisition permissions but not 'order_manage_all' 2.2) Go to URL => You don't have access to basket 2.3) Click on 'Baskets' tab => you don't see the basket in table
Created attachment 131396 [details] [review] Bug 30232: Logged in user performance for CanUserUseBudget() and CanUserModifyBudget() Some acquisition methods can take borrowernumber or a Koha::Patron->unblessed. When called with logged in user, sometimes in a loop, performance will be better if fetching Koha::Patron of logged in user only once. This patch optimises call for CanUserUseBudget() and CanUserModifyBudget(); You may benchmark on a user with a lot of budgets acess. Test plan : Play with impacted pages and check budgets list is the same than wihtout patch
/cgi-bin/koha/acqui/acqui-home.pl with 300 funds : From 13 seconds to 10 seconds
I think it would be better if you change those C4/Budgets.pm functions accept directly a Koha::Patron, reading the code there is now a lot of duplication regarding the unblessing of the patron. Haven't checked all cases but if you check the code for CanUserUseBudget it's perfectly fine to just use the Koha::Patron methods, just remove the curly braces from the current code basically to make it work. I'd like to mark this as FQA if you don't mind.
Indeed the idea was not that good. I close invalid > it would be better if you change those C4/Budgets.pm functions accept directly a Koha::Patron This will be for another bug report