|
Description
Alex Buckley
2018-08-20 07:11:16 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 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. |