Bug 18861

Created attachment 64676 [details] [review]
Bug 18861 - branch_transfer_limits needs keys

The branch transfers limits are edited in : /cgi-bin/koha/admin/branch_transfer_limits.pl
This page calls a loop on IsBranchTransferAllowed with all destination branches and codes (ccodes or itemtypes).

This can takes a huge time because table branch_transfer_limits has no keys.

Since ccode or itemtype may be used, this patch adds a key on both combinaison : ccode/fromBranch/toBranch and itemtype/fromBranch/toBranch.

Tested on a table with 66000 entries, time whent from 26 second to less than 1 second.

Test plan :
- Go to Administration > Library transfer limits
- Check page load is faster
- Uncheck a checkbox
- Save
=> Verify save is OK
- Change branch and same tests

Created attachment 64764 [details] [review]
Bug 18861 - branch_transfer_limits needs keys

The branch transfers limits are edited in : /cgi-bin/koha/admin/branch_transfer_limits.pl
This page calls a loop on IsBranchTransferAllowed with all destination branches and codes (ccodes or itemtypes).

This can takes a huge time because table branch_transfer_limits has no keys.

Since ccode or itemtype may be used, this patch adds a key on both combinaison : ccode/fromBranch/toBranch and itemtype/fromBranch/toBranch.

Tested on a table with 66000 entries, time whent from 26 second to less than 1 second.

Test plan :
- Go to Administration > Library transfer limits
- Check page load is faster
- Uncheck a checkbox
- Save
=> Verify save is OK
- Change branch and same tests

Followed test plan which worked as intended
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Hi Frindolin 

Your patch works well, one thing I noticed was that it would be logical for the checkbox of the current branch not to be displayed.

i.e. if I am looking at the Fairfield checkin and transfer policy then the fairfield checkbox should not be displayed because when you uncheck the fairfield box then save the page it reloads with fairfield checked again.

Perhaps this could be a future patch :)

What is the status of this bug? Signed off?

(In reply to Alex Buckley from comment #3)
> Perhaps this could be a future patch :)
Indeed, another bug report.

Back to signed off

Created attachment 114686 [details] [review]
Bug 18861 - branch_transfer_limits needs keys

The branch transfers limits are edited in : /cgi-bin/koha/admin/branch_transfer_limits.pl
This page calls a loop on IsBranchTransferAllowed with all destination branches and codes (ccodes or itemtypes).

This can takes a huge time because table branch_transfer_limits has no keys.

Since ccode or itemtype may be used, this patch adds a key on both combinaison : ccode/fromBranch/toBranch and itemtype/fromBranch/toBranch.

Tested on a table with 66000 entries, time whent from 26 second to less than 1 second.

Test plan :
- Go to Administration > Library transfer limits
- Check page load is faster
- Uncheck a checkbox
- Save
=> Verify save is OK
- Change branch and same tests

Followed test plan which worked as intended
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

I updated the patch, but in testing I didn't see any difference.

I added many types and branches:
INSERT INTO itemtypes (itemtype,description) SELECT CONCAT(itemtype,"A"),CONCAT(description,"A") FROM itemtypes;
INSERT INTO itemtypes (itemtype,description) SELECT CONCAT(itemtype,"B"),CONCAT(description,"B") FROM itemtypes;
INSERT INTO itemtypes (itemtype,description) SELECT CONCAT(itemtype,"C"),CONCAT(description,"C") FROM itemtypes;
INSERT INTO itemtypes (itemtype,description) SELECT CONCAT(itemtype,"D"),CONCAT(description,"D") FROM itemtypes;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"A"),CONCAT(branchname,"A"),pickup_location FROM branches;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"B"),CONCAT(branchname,"B"),pickup_location FROM branches;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"C"),CONCAT(branchname,"C"),pickup_location FROM branches;
INSERT INTO branches (branchcode,branchname,pickup_location) SELECT CONCAT(branchcode,"D"),CONCAT(branchname,"D"),pickup_location FROM branches;

Before and after the indexes it took about 30 seconds for the page to load

(In reply to Nick Clemens from comment #8)
> I updated the patch, but in testing I didn't see any difference.
> 
> I added many types and branches:
> INSERT INTO itemtypes (itemtype,description) SELECT
> CONCAT(itemtype,"A"),CONCAT(description,"A") FROM itemtypes;
> INSERT INTO itemtypes (itemtype,description) SELECT
> CONCAT(itemtype,"B"),CONCAT(description,"B") FROM itemtypes;
> INSERT INTO itemtypes (itemtype,description) SELECT
> CONCAT(itemtype,"C"),CONCAT(description,"C") FROM itemtypes;
> INSERT INTO itemtypes (itemtype,description) SELECT
> CONCAT(itemtype,"D"),CONCAT(description,"D") FROM itemtypes;
> INSERT INTO branches (branchcode,branchname,pickup_location) SELECT
> CONCAT(branchcode,"A"),CONCAT(branchname,"A"),pickup_location FROM branches;
> INSERT INTO branches (branchcode,branchname,pickup_location) SELECT
> CONCAT(branchcode,"B"),CONCAT(branchname,"B"),pickup_location FROM branches;
> INSERT INTO branches (branchcode,branchname,pickup_location) SELECT
> CONCAT(branchcode,"C"),CONCAT(branchname,"C"),pickup_location FROM branches;
> INSERT INTO branches (branchcode,branchname,pickup_location) SELECT
> CONCAT(branchcode,"D"),CONCAT(branchname,"D"),pickup_location FROM branches;
> 
> Before and after the indexes it took about 30 seconds for the page to load

How many entries do you have in table branch_transfer_limits ?