Bug 21249 - Syspref to choose whether to search homebranch, holding branch or both for library groups in advanced search
Summary: Syspref to choose whether to search homebranch, holding branch or both for li...
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: David Cook
URL:
Keywords:
Depends on:
Blocks: 28847 28295
  Show dependency treegraph
 
Reported: 2018-08-20 07:11 UTC by Alex Buckley
Modified: 2021-08-12 11:06 UTC (History)
12 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Text to go in the release notes:
This patch adds a system preference, SearchLimitLibrary, to be used in the advanced search on the staff client and OPAC, and the OPAC masthead search. When limiting search results with a library or library group, the SearchLimitLibrary system preference can be set to limit using the item's holding branch, home branch, or both.
Version(s) released in:
21.05.00


Attachments
Bug 21249 - New syspref controlling branch fields queried against selected library group value in advanced search (8.43 KB, patch)
2018-08-20 11:48 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21249 - New syspref controlling branch fields queried against selected library group value in advanced search (8.43 KB, patch)
2018-08-20 11:52 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21249 - New syspref controlling branch fields queried against selected library group value in advanced search (8.30 KB, patch)
2018-09-16 00:58 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (8.07 KB, patch)
2018-11-09 04:38 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.54 KB, patch)
2018-12-27 01:57 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (8.12 KB, patch)
2019-10-01 08:38 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.60 KB, patch)
2019-10-01 08:38 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (8.22 KB, patch)
2019-11-06 11:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.67 KB, patch)
2019-11-06 11:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21249: Code cleaning (4.49 KB, patch)
2019-11-06 11:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (8.32 KB, patch)
2020-04-24 03:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.68 KB, patch)
2020-04-24 03:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: Code cleaning (4.57 KB, patch)
2020-04-24 03:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search (6.52 KB, patch)
2020-04-24 03:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search (6.55 KB, patch)
2020-04-24 03:07 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (9.35 KB, patch)
2021-02-23 22:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.67 KB, patch)
2021-02-23 22:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: Code cleaning (4.57 KB, patch)
2021-02-23 22:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search (6.57 KB, patch)
2021-02-23 22:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: (follow-up) Implement SearchLibraryLimit for individual library (2.43 KB, patch)
2021-02-23 22:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: (follow-up) Adding parentheses to fix query (1.38 KB, patch)
2021-05-05 05:22 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (9.38 KB, patch)
2021-05-05 21:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.72 KB, patch)
2021-05-05 21:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 21249: Code cleaning (4.62 KB, patch)
2021-05-05 21:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search (6.58 KB, patch)
2021-05-05 21:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 21249: (follow-up) Implement SearchLibraryLimit for individual library (2.49 KB, patch)
2021-05-05 21:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 21249: (follow-up) Adding parentheses to fix query (1.43 KB, patch)
2021-05-05 21:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search (9.46 KB, patch)
2021-05-06 03:45 UTC, David Cook
Details | Diff | Splinter Review
Bug 21249: Made the SQL INSERT statement idempotent (1.78 KB, patch)
2021-05-06 03:45 UTC, David Cook
Details | Diff | Splinter Review
Bug 21249: Code cleaning (4.68 KB, patch)
2021-05-06 03:45 UTC, David Cook
Details | Diff | Splinter Review
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search (6.68 KB, patch)
2021-05-06 03:45 UTC, David Cook
Details | Diff | Splinter Review
Bug 21249: (follow-up) Implement SearchLibraryLimit for individual library (2.54 KB, patch)
2021-05-06 03:45 UTC, David Cook
Details | Diff | Splinter Review
Bug 21249: (follow-up) Adding parentheses to fix query (1.49 KB, patch)
2021-05-06 03:46 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Buckley 2018-08-20 07:11:16 UTC
Implement a system preference for librarians to choose whether they want the library or library group selected in the advanced search to be checked against biblios homebranch, holdingbranch or both branches.
Comment 1 Alex Buckley 2018-08-20 11:48:22 UTC
Created attachment 78013 [details] [review]
Bug 21249 - New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group (if you do not already have one) and add
several library branches to the group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child branch of the library group created in step 4

5. Perform an Advanced search in the staff client and write in the
keyword as the barcode of the item and select the library group in the
'Location and availability' section.

6. Submit the search query and notice the item with a homebranch
existing in the selected library group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

7. Now edit the item making sure the homebranch branch is not in the
library group, and making sure the holdingbranch is in the library
group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'AdvancedSearchBranchFieldToUse'.

8. Repeat step 5 and 6 and notice the item with a holdingbranch existing
in the selected library group is returned. This proves that when the
AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the holdingbranch is checked.

9. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 8 and again notice the item with a
holdingbranch existing in the selected library group is returned so we
know the holdingbranch is being chcked when the 'holdingbranch' value
for the syspref is set.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

10. Change the homebranch back to a branch that is in the library group
and change the holdingbranch value to a library branch that is not in
the library group.

11. Repeat step 5 and 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked when the 'homebranch' value for the syspref is set.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library
Comment 2 Alex Buckley 2018-08-20 11:52:52 UTC
Created attachment 78014 [details] [review]
Bug 21249 - New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group (if you do not already have one) and add
several library branches to the group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child branch of the library group created in step 4

5. Perform an Advanced search in the staff client and write in the
keyword as the barcode of the item and select the library group in the
'Location and availability' section.

6. Submit the search query and notice the item with a homebranch
existing in the selected library group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

7. Now edit the item making sure the homebranch branch is not in the
library group, and making sure the holdingbranch is in the library
group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'AdvancedSearchBranchFieldToUse'.

8. Repeat step 5 and 6 and notice the item with a holdingbranch existing
in the selected library group is returned. This proves that when the
AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the holdingbranch is checked.

9. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 8 and again notice the item with a
holdingbranch existing in the selected library group is returned so we
know the holdingbranch is being chcked when the 'holdingbranch' value
for the syspref is set.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

10. Change the homebranch back to a branch that is in the library group
and change the holdingbranch value to a library branch that is not in
the library group.

11. Repeat step 5 and 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked when the 'homebranch' value for the syspref is set.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library
Comment 3 Alex Buckley 2018-09-11 22:21:59 UTC
Any chance we can move this along?
Comment 4 Alex Buckley 2018-09-16 00:58:17 UTC
Created attachment 78921 [details] [review]
Bug 21249 - New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group (if you do not already have one) and add
several library branches to the group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child branch of the library group created in step 4

5. Perform an Advanced search in the staff client and write in the
keyword as the barcode of the item and select the library group in the
'Location and availability' section.

6. Submit the search query and notice the item with a homebranch
existing in the selected library group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

7. Now edit the item making sure the homebranch branch is not in the
library group, and making sure the holdingbranch is in the library
group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'AdvancedSearchBranchFieldToUse'.

8. Repeat step 5 and 6 and notice the item with a holdingbranch existing
in the selected library group is returned. This proves that when the
AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the holdingbranch is checked.

9. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 8 and again notice the item with a
holdingbranch existing in the selected library group is returned so we
know the holdingbranch is being chcked when the 'holdingbranch' value
for the syspref is set.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

10. Change the homebranch back to a branch that is in the library group
and change the holdingbranch value to a library branch that is not in
the library group.

11. Repeat step 5 and 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked when the 'homebranch' value for the syspref is set.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library
Comment 5 Alex Buckley 2018-09-16 00:58:38 UTC
Rebased against master on 16th of sept 2018.
Comment 6 Michal Denar 2018-10-31 20:43:02 UTC
Hi Alex,
I can't select group at poitn 5. http://prntscr.com/lcuq55

Missed I something?

Is this setting of group correct? http://prntscr.com/lcuqxf

Thank You.


Mike
Comment 7 Alex Buckley 2018-11-09 04:38:21 UTC
Created attachment 82138 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group. Create a subgroup from the library group and add several library branches to the sub group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child of the sub group created in step 4

6. Perform an Advanced search in the staff client and write in the item
barcode as the keyword and select the sub group in the
'Location and availability' section.

7. Submit the search query and notice the item with a homebranch
existing in the selected sub group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

8. Now edit the item making sure the homebranch branch is not in the
sub group, and making sure the holdingbranch is in the sub group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'homebranch and holdingbranch'

9. Repeat step 6 and notice the item with a holdingbranch existing
in the selected library group is returned.

10. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 6 and again notice the item with a
holdingbranch existing in the selected library group is returned.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

11. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'homebranch'

12. Change the item homebranch back to a branch that is in the sub group
and change the holdingbranch value to a library branch that is not in
the sub group.

13. Repeat step 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library
Comment 8 Alex Buckley 2018-11-09 04:41:25 UTC
Hi Michal

I have found what you have to do is create a Library group then click 'Actions->Add sub-group'

Give your sub group a name and then add the library branches to the subgroup. The subgroup name will show up in the 'Groups of libraries' dropdown. 

If you try that it should work, please let me know if you have any issues.

Cheers,
Alex
Comment 9 Alex Buckley 2018-11-09 04:43:15 UTC
I have also amended the test plan in the commit accordingly
Comment 10 Alex Buckley 2018-12-12 21:18:30 UTC
Hi Michael, 

Are you able to retest please,? This still applies cleanly on master.

Cheers,
Alex
Comment 11 Alex Buckley 2018-12-27 01:57:37 UTC
Created attachment 83518 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia
Comment 12 Brian 2019-01-14 20:03:46 UTC
Any updates on bug 21249?

Carnegie team
Comment 13 sandboxes@biblibre.com 2019-10-01 08:38:12 UTC
Patch tested with a sandbox, by Amandine Zocca <azocca@ville-montauban.fr>
Comment 14 sandboxes@biblibre.com 2019-10-01 08:38:44 UTC
Created attachment 93345 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group. Create a subgroup from the library group and add several library branches to the sub group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child of the sub group created in step 4

6. Perform an Advanced search in the staff client and write in the item
barcode as the keyword and select the sub group in the
'Location and availability' section.

7. Submit the search query and notice the item with a homebranch
existing in the selected sub group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

8. Now edit the item making sure the homebranch branch is not in the
sub group, and making sure the holdingbranch is in the sub group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'homebranch and holdingbranch'

9. Repeat step 6 and notice the item with a holdingbranch existing
in the selected library group is returned.

10. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 6 and again notice the item with a
holdingbranch existing in the selected library group is returned.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

11. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'homebranch'

12. Change the item homebranch back to a branch that is in the sub group
and change the holdingbranch value to a library branch that is not in
the sub group.

13. Repeat step 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>
Comment 15 sandboxes@biblibre.com 2019-10-01 08:38:48 UTC
Created attachment 93346 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>
Comment 16 Katrin Fischer 2019-10-07 22:20:09 UTC
Hi Alex,

some small stuff:

1) The pref name and pref description use 'branch' according to our terminology guidelines 'library' should be used. This would also be consistent with other prefs using 'home library' and 'holding library' (see OPAC results library, OpacLocationOnDetail, ...)

2) The pref description always talks about the advanced search, but I think this applies to the branch selection on the simple search (OpacAddMastheadLibraryPulldown) too or should apply to it to have consistent behaviour - can you clarify?

3) You are missing the change to sysprefs.sql.
Comment 17 Jonathan Druart 2019-11-06 11:21:16 UTC
Created attachment 95099 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group. Create a subgroup from the library group and add several library branches to the sub group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child of the sub group created in step 4

6. Perform an Advanced search in the staff client and write in the item
barcode as the keyword and select the sub group in the
'Location and availability' section.

7. Submit the search query and notice the item with a homebranch
existing in the selected sub group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

8. Now edit the item making sure the homebranch branch is not in the
sub group, and making sure the holdingbranch is in the sub group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'homebranch and holdingbranch'

9. Repeat step 6 and notice the item with a holdingbranch existing
in the selected library group is returned.

10. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 6 and again notice the item with a
holdingbranch existing in the selected library group is returned.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

11. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'homebranch'

12. Change the item homebranch back to a branch that is in the sub group
and change the holdingbranch value to a library branch that is not in
the sub group.

13. Repeat step 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2019-11-06 11:21:21 UTC
Created attachment 95100 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2019-11-06 11:21:27 UTC
Created attachment 95101 [details] [review]
Bug 21249: Code cleaning

Trying to make the code a bit more readable removing unecessary
variables and parenthesis.

Code is duplicated but no idea where we could move it to.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Jonathan Druart 2019-11-06 11:22:33 UTC
(In reply to Katrin Fischer from comment #16)
> Hi Alex,
> 
> some small stuff:
> 
> 1) The pref name and pref description use 'branch' according to our
> terminology guidelines 'library' should be used. This would also be
> consistent with other prefs using 'home library' and 'holding library' (see
> OPAC results library, OpacLocationOnDetail, ...)
> 
> 2) The pref description always talks about the advanced search, but I think
> this applies to the branch selection on the simple search
> (OpacAddMastheadLibraryPulldown) too or should apply to it to have
> consistent behaviour - can you clarify?
> 
> 3) You are missing the change to sysprefs.sql.

Sorry, I missed that comment!
Comment 21 Martin Renvoize 2019-11-06 17:36:04 UTC
Would love to see this picked back up near the beginning of the next cycle.. I'm afraid it's just missed the freeze for 19.11 :(
Comment 22 Martin Renvoize 2020-04-15 16:07:01 UTC
Failed and patch no longer applies.. moving to 20.11 target.  I would love to have seen this one looked at early in the 20.05 cycle :(
Comment 23 Aleisha Amohia 2020-04-24 03:06:18 UTC
Created attachment 103618 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library group search condition in the staff client and OPAC advanced
searches.

Test plan:
1. Apply patch and run ./updatedatabase.pl in the koha shell

2. Restart memcached and plack

3. In the staff client go to Administration->Global system preferences
and search for the systempreference: AdvancedSearchBranchFieldToUse and
notice by default it is set to 'homebranch and holdingbranch'

4. Create a library group. Create a subgroup from the library group and add several library branches to the sub group.

5. Note down the barcode of an item to use for testing searching. Make
sure the items homebranch and holdingbranch are different and the items
homebranch is a child of the sub group created in step 4

6. Perform an Advanced search in the staff client and write in the item
barcode as the keyword and select the sub group in the
'Location and availability' section.

7. Submit the search query and notice the item with a homebranch
existing in the selected sub group is returned. This proves that
when the AdvancedSearchBranchFieldToUse syspref is set to 'homebranch and
holdingbranch' the homebranch value is checked.

Also note at the top of the search result page the search query is listed:  Results of search
with limit(s): '(homebranch: <branch1> or holdingbranch: <branch2> or homebranch:
<homebranch1> or holdingbranch: <branch2>)'

8. Now edit the item making sure the homebranch branch is not in the
sub group, and making sure the holdingbranch is in the sub group. This is to test that holdingbranch is indeed being checked when
the AdvancedSearchBranchFieldToUse is set to
'homebranch and holdingbranch'

9. Repeat step 6 and notice the item with a holdingbranch existing
in the selected library group is returned.

10. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'holdingbranch' and repeat step 6 and again notice the item with a
holdingbranch existing in the selected library group is returned.

Also note the search query at the top of the page is: "Results of search
with limit(s): '(holdingbranch: <branch1> or holdingbranch: <branch2>)'"
this shows holdingbranch not homebranch is a condition in the query

11. Change the value of the AdvancedSearchBranchFieldToUse syspref to
'homebranch'

12. Change the item homebranch back to a branch that is in the sub group
and change the holdingbranch value to a library branch that is not in
the sub group.

13. Repeat step 6 and notice the item with the homebranch in the
selected library group is returned, so we know the homebranch is being
checked.

Also note the search query at the top of the page is: " Results of
search  with limit(s): '(homebranch: <branch1> or homebranch: <branch2>)'" this shows homebranch not holdingbranch is a condition in the query.

Sponsored-By: Brimbank Library

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Aleisha Amohia 2020-04-24 03:06:23 UTC
Created attachment 103619 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Aleisha Amohia 2020-04-24 03:06:27 UTC
Created attachment 103620 [details] [review]
Bug 21249: Code cleaning

Trying to make the code a bit more readable removing unecessary
variables and parenthesis.

Code is duplicated but no idea where we could move it to.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Aleisha Amohia 2020-04-24 03:06:31 UTC
Created attachment 103621 [details] [review]
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search

Syspref is now called SearchLimitLibrary, and the description better
explains what the feature does. It works with the advanced search on the
staff client and OPAC, and the masthead search on the OPAC when
OpacAddMastheadLibraryPulldown is enabled.
Comment 27 Aleisha Amohia 2020-04-24 03:07:34 UTC
Created attachment 103622 [details] [review]
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search

Syspref is now called SearchLimitLibrary, and the description better
explains what the feature does. It works with the advanced search on the
staff client and OPAC, and the masthead search on the OPAC when
OpacAddMastheadLibraryPulldown is enabled.

Sponsored-by: Catalyst IT
Comment 28 Andrew Fuerste-Henry 2020-07-23 16:47:54 UTC
This doesn't seem to be working in all cases when picking a specific library rather than a group. 

- Have an item with homebranch A and holdingbranch B
- Set SearchLimitLibrary to home library
- search for your barcode, limited to branch A
- no results

I get the same on the staff client and the OPAC
Comment 29 David Cook 2020-12-18 06:22:52 UTC
(In reply to Aleisha Amohia from comment #25)
>
> Code is duplicated but no idea where we could move it to.
> 

I'd suggest something like Koha::Search or a Koha::Search::Something submodule and then add unit tests for it. It's late on a Friday but I could have more of a think about this one.
Comment 30 Aleisha Amohia 2021-02-23 22:04:17 UTC
Created attachment 117220 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library or library group search condition in the staff client and OPAC
advanced searches.

Test plan:
1. Apply patches, update database, restart services
2. Set up a record with one item. Edit the item so that:
home branch = Branch A
holding/current branch = Branch B.
Note the barcode of your item.
3. Go to Administration -> Library Groups. Create a library group that
only contains Branch A.

= homebranch and holdingbranch =

4. Go to Administration -> System preferences and find the new
SearchLimitLibrary syspref. Confirm it is set to 'homebranch and
holdingbranch' by default. Keep this tab open.
5. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
6. Submit the search and confirm you are taken to your item as expected.
7. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
8. Submit the search and confirm you are taken to your item as expected.
9. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
10. Submit the search and confirm you are taken to your item as expected.

= homebranch only =

11. Go back to your System preferences tab. Set the SearchLimitLibrary
syspref to 'homebranch'. keep this tab open.
12. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
13. Submit the search and confirm you are taken to your item as expected,
because the syspref is set to homebranch and the library group contains our item's
homebranch.
14. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
15. Submit the search and confirm you are taken to your item as expected.
16. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
17. Submit the search and confirm you are NOT taken to your item and your item
does not show in the search results.

= holdingbranch only =

18. Go back to your System preferences tab. Set the SearchLimitLibrary
syspref to 'holdingbranch'. keep this tab open.
19. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
20. Submit the search and confirm you are NOT taken to your item and your
item does not show in the search results, because the syspref is set to
holdingbranch and the library group does not contain our item's holdingbranch.
21. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
22. Submit the search and confirm you are taken to your item as expected.
23. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
24. Submit the search and confirm you are NOT taken to your item and your item
does not show in the search results.

25. Repeat the above test plan but do your searching with the OPAC
advanced search.

Sponsored-By: Brimbank Library

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 31 Aleisha Amohia 2021-02-23 22:04:22 UTC
Created attachment 117221 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 32 Aleisha Amohia 2021-02-23 22:04:26 UTC
Created attachment 117222 [details] [review]
Bug 21249: Code cleaning

Trying to make the code a bit more readable removing unecessary
variables and parenthesis.

Code is duplicated but no idea where we could move it to.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 33 Aleisha Amohia 2021-02-23 22:04:31 UTC
Created attachment 117223 [details] [review]
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search

Syspref is now called SearchLimitLibrary, and the description better
explains what the feature does. It works with the advanced search on the
staff client and OPAC, and the masthead search on the OPAC when
OpacAddMastheadLibraryPulldown is enabled.

Sponsored-by: Catalyst IT
Comment 34 Aleisha Amohia 2021-02-23 22:04:35 UTC
Created attachment 117224 [details] [review]
Bug 21249: (follow-up) Implement SearchLibraryLimit for individual library

before this patch, the limit only applies to groups of libraries
Comment 35 David Cook 2021-03-03 05:03:52 UTC
This is on my test list. Just haven't gotten to it yet.
Comment 36 David Cook 2021-05-03 06:04:40 UTC
"home library and holding library" doesn't work for individual libraries.

It's generating the following PQF:
@attrset Bib-1 @or @and @attr 1=8023 @attr 4=6 @attr 5=1 @attr 2=102 39999000001310 @attr 1=homebranch CPL @attr 1=8012 CPL

The Library Group option generates the following which is correct:
@attrset Bib-1 @and @attr 1=8023 @attr 4=6 @attr 5=1 @attr 2=102 39999000001310 @or @attr 1=homebranch CPL @attr 1=8012 CPL

The key is the parentheses in your query.

For the group you're doing this:
"(%s or %s)"

For the individual you're doing this:
"$homebranchlimit or $holdingbranchlimit"

If you change that last one to the following it should work:
"($homebranchlimit or $holdingbranchlimit)"
Comment 37 Aleisha Amohia 2021-05-05 05:22:56 UTC
Created attachment 120481 [details] [review]
Bug 21249: (follow-up) Adding parentheses to fix query
Comment 38 Victor Grousset/tuxayo 2021-05-05 21:17:59 UTC
Created attachment 120567 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library or library group search condition in the staff client and OPAC
advanced searches.

Test plan:
1. Apply patches, update database, restart services
2. Set up a record with one item. Edit the item so that:
home branch = Branch A
holding/current branch = Branch B.
Note the barcode of your item.
3. Go to Administration -> Library Groups. Create a library group that
only contains Branch A.

= homebranch and holdingbranch =

4. Go to Administration -> System preferences and find the new
SearchLimitLibrary syspref. Confirm it is set to 'homebranch and
holdingbranch' by default. Keep this tab open.
5. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
6. Submit the search and confirm you are taken to your item as expected.
7. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
8. Submit the search and confirm you are taken to your item as expected.
9. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
10. Submit the search and confirm you are taken to your item as expected.

= homebranch only =

11. Go back to your System preferences tab. Set the SearchLimitLibrary
syspref to 'homebranch'. keep this tab open.
12. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
13. Submit the search and confirm you are taken to your item as expected,
because the syspref is set to homebranch and the library group contains our item's
homebranch.
14. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
15. Submit the search and confirm you are taken to your item as expected.
16. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
17. Submit the search and confirm you are NOT taken to your item and your item
does not show in the search results.

= holdingbranch only =

18. Go back to your System preferences tab. Set the SearchLimitLibrary
syspref to 'holdingbranch'. keep this tab open.
19. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
20. Submit the search and confirm you are NOT taken to your item and your
item does not show in the search results, because the syspref is set to
holdingbranch and the library group does not contain our item's holdingbranch.
21. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
22. Submit the search and confirm you are taken to your item as expected.
23. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
24. Submit the search and confirm you are NOT taken to your item and your item
does not show in the search results.

25. Repeat the above test plan but do your searching with the OPAC
advanced search.

Sponsored-By: Brimbank Library

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 39 Victor Grousset/tuxayo 2021-05-05 21:18:09 UTC
Created attachment 120568 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 40 Victor Grousset/tuxayo 2021-05-05 21:18:19 UTC
Created attachment 120569 [details] [review]
Bug 21249: Code cleaning

Trying to make the code a bit more readable removing unecessary
variables and parenthesis.

Code is duplicated but no idea where we could move it to.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 41 Victor Grousset/tuxayo 2021-05-05 21:18:27 UTC
Created attachment 120570 [details] [review]
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search

Syspref is now called SearchLimitLibrary, and the description better
explains what the feature does. It works with the advanced search on the
staff client and OPAC, and the masthead search on the OPAC when
OpacAddMastheadLibraryPulldown is enabled.

Sponsored-by: Catalyst IT
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 42 Victor Grousset/tuxayo 2021-05-05 21:18:33 UTC
Created attachment 120571 [details] [review]
Bug 21249: (follow-up) Implement SearchLibraryLimit for individual library

before this patch, the limit only applies to groups of libraries

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 43 Victor Grousset/tuxayo 2021-05-05 21:18:39 UTC
Created attachment 120572 [details] [review]
Bug 21249: (follow-up) Adding parentheses to fix query

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 44 Victor Grousset/tuxayo 2021-05-05 21:21:57 UTC
It works! :D

Oops forgot to edit the test plan.
Testing note:

> 3. Go to Administration -> Library Groups. Create a library group that
> only contains Branch A.

Tick the checkbox to have it visible in staff and OPAC adv search
Comment 45 David Cook 2021-05-06 03:44:37 UTC
I am getting 1 fail in the QA tools but it seems minor to me. I'm leaving it to the RM to squash if necessary.

--

root@kohadevbox:koha(bug_21249)$ qa
testing 6 commit(s) (applied to 640ba47 'cf Bug 28288: Prevent XSISBN.t to fai')

Processing files before patches
|========================>| 4 / 4 (100.00%)
Processing files after patches
|========================>| 4 / 4 (100.00%)

 OK     catalogue/search.pl
 FAIL   installer/data/mysql/atomicupdate/bug_21249-Branch_to_limit_advanced_search_results.sql
   FAIL   git manipulation
                The file has been added and deleted in the same patchset

 OK     installer/data/mysql/mandatory/sysprefs.sql
 OK     opac/opac-search.pl

Processing additional checks OK!
Comment 46 David Cook 2021-05-06 03:45:39 UTC
Created attachment 120575 [details] [review]
Bug 21249: New syspref controlling branch fields queried against selected library group value in advanced search

This commit allows librarians to choose whether they want to query the
homebranch, holdingbranch or homebranch AND holdingbranch when they set
a library or library group search condition in the staff client and OPAC
advanced searches.

Test plan:
1. Apply patches, update database, restart services
2. Set up a record with one item. Edit the item so that:
home branch = Branch A
holding/current branch = Branch B.
Note the barcode of your item.
3. Go to Administration -> Library Groups. Create a library group that
only contains Branch A.

= homebranch and holdingbranch =

4. Go to Administration -> System preferences and find the new
SearchLimitLibrary syspref. Confirm it is set to 'homebranch and
holdingbranch' by default. Keep this tab open.
5. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
6. Submit the search and confirm you are taken to your item as expected.
7. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
8. Submit the search and confirm you are taken to your item as expected.
9. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
10. Submit the search and confirm you are taken to your item as expected.

= homebranch only =

11. Go back to your System preferences tab. Set the SearchLimitLibrary
syspref to 'homebranch'. keep this tab open.
12. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
13. Submit the search and confirm you are taken to your item as expected,
because the syspref is set to homebranch and the library group contains our item's
homebranch.
14. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
15. Submit the search and confirm you are taken to your item as expected.
16. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
17. Submit the search and confirm you are NOT taken to your item and your item
does not show in the search results.

= holdingbranch only =

18. Go back to your System preferences tab. Set the SearchLimitLibrary
syspref to 'holdingbranch'. keep this tab open.
19. Go to Advanced Search in the staff client in another tab. Under 'location and
availability', select your library group from the dropdown. Under
'search for', select the barcode option and enter your item's barcode.
20. Submit the search and confirm you are NOT taken to your item and your
item does not show in the search results, because the syspref is set to
holdingbranch and the library group does not contain our item's holdingbranch.
21. Go back to Advanced Search. Under 'location and availability', select
Branch B from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
22. Submit the search and confirm you are taken to your item as expected.
23. Go back to Advanced Search. Under 'location and availability', select
Branch A from the individual libraries dropdown. Under 'search for',
select the barcode option and enter your item's barcode.
24. Submit the search and confirm you are NOT taken to your item and your item
does not show in the search results.

25. Repeat the above test plan but do your searching with the OPAC
advanced search.

Sponsored-By: Brimbank Library

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 47 David Cook 2021-05-06 03:45:44 UTC
Created attachment 120576 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Signed-off-by: Amandine Zocca <azocca@ville-montauban.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 48 David Cook 2021-05-06 03:45:49 UTC
Created attachment 120577 [details] [review]
Bug 21249: Code cleaning

Trying to make the code a bit more readable removing unecessary
variables and parenthesis.

Code is duplicated but no idea where we could move it to.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 49 David Cook 2021-05-06 03:45:54 UTC
Created attachment 120578 [details] [review]
Bug 21249: (follow-up) Fixing syspref name, description, get working with OPAC masthead search

Syspref is now called SearchLimitLibrary, and the description better
explains what the feature does. It works with the advanced search on the
staff client and OPAC, and the masthead search on the OPAC when
OpacAddMastheadLibraryPulldown is enabled.

Sponsored-by: Catalyst IT
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 50 David Cook 2021-05-06 03:45:59 UTC
Created attachment 120579 [details] [review]
Bug 21249: (follow-up) Implement SearchLibraryLimit for individual library

before this patch, the limit only applies to groups of libraries

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 51 David Cook 2021-05-06 03:46:04 UTC
Created attachment 120580 [details] [review]
Bug 21249: (follow-up) Adding parentheses to fix query

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 52 Jonathan Druart 2021-05-06 13:35:18 UTC
I am willing to push this enhancement as it, given its age. But please keep in mind that we must avoid copy/paste, here we have ~40 lines of the exact same code in 2 different files. That must be part of a module and covered by tests.
Comment 53 Victor Grousset/tuxayo 2021-05-06 15:10:26 UTC
Great to have this going :D

btw, great test plan Aleisha it helped a lot
Comment 54 Jonathan Druart 2021-05-07 12:41:07 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 55 Fridolin Somers 2021-05-11 14:27:34 UTC
Nice.

Just, preference description seems incorect :
  both: "home library and holding library."

https://git.koha-community.org/Koha-community/Koha/src/commit/21658717de3d8da3516eb23d2f7869fc6fe88922/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/searching.pref#L176

  is should be "home library or holding library." no ?
Comment 56 David Cook 2021-05-11 23:29:42 UTC
(In reply to Fridolin Somers from comment #55)
> Nice.
> 
> Just, preference description seems incorect :
>   both: "home library and holding library."
> 
> https://git.koha-community.org/Koha-community/Koha/src/commit/
> 21658717de3d8da3516eb23d2f7869fc6fe88922/koha-tmpl/intranet-tmpl/prog/en/
> modules/admin/preferences/searching.pref#L176
> 
>   is should be "home library or holding library." no ?

I suppose that either option could be technically correct, but I think that you're right that "home library or holding library" might be more obvious to users.