Bug 25024 - OPAC incorrectly marks branch as invalid pickup location when similarly named branch is blocked
Summary: OPAC incorrectly marks branch as invalid pickup location when similarly named...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 17453
Blocks:
  Show dependency treegraph
 
Reported: 2020-03-31 13:32 UTC by Andrew Fuerste-Henry
Modified: 2020-05-11 21:12 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.06, 19.05.11


Attachments
Bug 25024: Make grep match full string (2.31 KB, patch)
2020-03-31 14:00 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25024: Make grep match full string (2.37 KB, patch)
2020-03-31 14:38 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25024: Make grep match full string (2.43 KB, patch)
2020-03-31 15:27 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25024: Make grep match full string (2.50 KB, patch)
2020-04-01 08:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25024: Do not escape $ (1.92 KB, patch)
2020-04-01 08:59 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2020-03-31 13:32:28 UTC
When you've got branches with similar branchcodes, the OPAC can get confused about which should be blocked by transfer limits.

To test:
- have a branch whose code contains the code for another branch, like "TOWN" and "TOWNSCHOOL"
- Have a third branch with an unrealated code. I'll use "COUNTRY"
- Have an item of item type BOOK, owned by branch COUNTRY
- Set branch transfer limit to not allow BOOKs to go from COUNTRY to TOWNSCHOOL
- confirm that no transfer limit exists for BOOKS from COUNTRY to TOWN
- On staff client, confirm item allows hold for pickup at TOWN but not at TOWNSCHOOL
- On OPAC, confirm hold is not allowed for pickup at TOWN or TOWNSCHOOL

This seems to be the problematic bit of code:
[% SET pickup_available_at = bibitemloo.not_available_at.grep(library.branchcode).size ? 0 : 1 %]

So when we grep "TOWN" it also finds "TOWNSCHOOL," sees that block for TOWNSCHOOL, and applies it to TOWN in error.

This has come to light as library systems deal with closures of some of their libraries due to coronavirus and start using branch transfer limits more intensively.
Comment 1 Nick Clemens 2020-03-31 14:00:36 UTC
Created attachment 102139 [details] [review]
Bug 25024: Make grep match full string

To test:
1 - Create or have a branch with code CPL
2 - Create two new branches 'ACPL' and 'CPLA'
3 - In branch transfer limits disallow some itype or collection from another branch (FPL) to ACPL and CPLA
4 - Set UseBranchTransferLimits to 'Enforce'
5 - Find a biblio with one item belonging to FPL
6 - On opac, attempt to place a hold
7 - Note that ACPL and CPLA don't show in dropdown, OK
8 - Note that CPL is disabled, NOT OK
9 - Apply patch
10 - Place hold
11 - CPL is now enabled
Comment 2 Andrew Fuerste-Henry 2020-03-31 14:38:13 UTC
Created attachment 102142 [details] [review]
Bug 25024: Make grep match full string

To test:
1 - Create or have a branch with code CPL
2 - Create two new branches 'ACPL' and 'CPLA'
3 - In branch transfer limits disallow some itype or collection from another branch (FPL) to ACPL and CPLA
4 - Set UseBranchTransferLimits to 'Enforce'
5 - Find a biblio with one item belonging to FPL
6 - On opac, attempt to place a hold
7 - Note that ACPL and CPLA don't show in dropdown, OK
8 - Note that CPL is disabled, NOT OK
9 - Apply patch
10 - Place hold
11 - CPL is now enabled

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Jonathan Druart 2020-03-31 14:56:36 UTC
Nick, what about the existing syntax:

koha-tmpl/intranet-tmpl/prog/en/modules/admin/additional-fields.tt:[% show_marcfield = marcfield_tables.grep('^' _ tablename _ '$').size ? 1 : 0 %]


The '^' _ var _ '$' seems more readable to me, don't you think?
Comment 4 Andrew Fuerste-Henry 2020-03-31 15:27:15 UTC
Created attachment 102146 [details] [review]
Bug 25024: Make grep match full string

To test:
1 - Create or have a branch with code CPL
2 - Create two new branches 'ACPL' and 'CPLA'
3 - In branch transfer limits disallow some itype or collection from another branch (FPL) to ACPL and CPLA
4 - Set UseBranchTransferLimits to 'Enforce'
5 - Find a biblio with one item belonging to FPL
6 - On opac, attempt to place a hold
7 - Note that ACPL and CPLA don't show in dropdown, OK
8 - Note that CPL is disabled, NOT OK
9 - Apply patch
10 - Place hold
11 - CPL is now enabled

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Comment 5 Jonathan Druart 2020-04-01 08:59:02 UTC
Created attachment 102177 [details] [review]
Bug 25024: Make grep match full string

To test:
1 - Create or have a branch with code CPL
2 - Create two new branches 'ACPL' and 'CPLA'
3 - In branch transfer limits disallow some itype or collection from another branch (FPL) to ACPL and CPLA
4 - Set UseBranchTransferLimits to 'Enforce'
5 - Find a biblio with one item belonging to FPL
6 - On opac, attempt to place a hold
7 - Note that ACPL and CPLA don't show in dropdown, OK
8 - Note that CPL is disabled, NOT OK
9 - Apply patch
10 - Place hold
11 - CPL is now enabled

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2020-04-01 08:59:06 UTC
Created attachment 102178 [details] [review]
Bug 25024: Do not escape $

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2020-04-01 08:59:40 UTC
The test plan should have contained testing OPACHoldsIfAvailableAtPickup to be complete.
Comment 8 Martin Renvoize 2020-04-03 13:24:53 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 9 Joy Nelson 2020-04-29 19:29:34 UTC
rebased (thanks nick) and backported to 19.11.x for 19.11.06
Comment 10 Lucas Gass 2020-05-11 21:12:40 UTC
backported to 19.05.x for 19.05.11