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

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

Any chance we can move this along?

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

Rebased against master on 16th of sept 2018.

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

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

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

I have also amended the test plan in the commit accordingly

Hi Michael, 

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

Cheers,
Alex

Created attachment 83518 [details] [review]
Bug 21249: Made the SQL INSERT statement idempotent

Sponsored-BY: Brimbank Library, Australia

Any updates on bug 21249?

Carnegie team

Patch tested with a sandbox, by Amandine Zocca <azocca@ville-montauban.fr>

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>

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>

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.

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>

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>

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>

(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!

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 :(

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 :(

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>

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>

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>

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.

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

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

(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.

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>

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>

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>

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

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

This is on my test list. Just haven't gotten to it yet.

"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)"

Created attachment 120481 [details] [review]
Bug 21249: (follow-up) Adding parentheses to fix query

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>

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>

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>

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>

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>

Created attachment 120572 [details] [review]
Bug 21249: (follow-up) Adding parentheses to fix query

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

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

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!

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>

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>

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>

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>

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>

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>

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.

Great to have this going :D

btw, great test plan Aleisha it helped a lot

Pushed to master for 21.05, thanks to everybody involved!

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 ?

(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.