On the advanced search page the shelving locations don't seem to be in alphabetical order by description as they should be to match the other tabs.
Created attachment 19978 [details] one example
Created attachment 19979 [details] another example
Tested in 3.10.6 and master.
Comment on attachment 19978 [details] one example there was a space in the description of periodicals on this one.
In the second example the lib is set to the same as the code - the lib_opac is set to the text. The search seems to be sorting on lib not lib_opac.
Created attachment 20039 [details] [review] Bug 10656 - shelving locations not alpha on search Collection codes and shelving locations are displayed in the OPAC and staff client via GetAuthorisedValues which currently sorts results by "lib, lib_opac." Consequently if lib (the description for the staff client) doesn't match lib_opac (the description for the OPAC) sorting will appear to be nonsensical in the OPAC. GetAuthorisedValues can be passed an $opac parameter, so this should be used to switch how reuslts are sorted. This patch implements such a switch. To test, modify your collection code or shelving location authorized values so that lib and lib_opac do not match. Set your AdvancedSearchTypes system preference to display the modified authorized values and view the advanced search page in the OPAC and staff client. Sorting should be correct in each case according to the correct value (lib in the staff client, lib_opac in the OPAC).
Created attachment 20040 [details] [review] [SIGNED-OFF] Bug 10656 - shelving locations not alpha on search Collection codes and shelving locations are displayed in the OPAC and staff client via GetAuthorisedValues which currently sorts results by "lib, lib_opac." Consequently if lib (the description for the staff client) doesn't match lib_opac (the description for the OPAC) sorting will appear to be nonsensical in the OPAC. GetAuthorisedValues can be passed an $opac parameter, so this should be used to switch how reuslts are sorted. This patch implements such a switch. To test, modify your collection code or shelving location authorized values so that lib and lib_opac do not match. Set your AdvancedSearchTypes system preference to display the modified authorized values and view the advanced search page in the OPAC and staff client. Sorting should be correct in each case according to the correct value (lib in the staff client, lib_opac in the OPAC). Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested in staff and opac and it works perfectly!
Created attachment 20502 [details] [review] Bug 10656 - shelving locations not alpha on search Collection codes and shelving locations are displayed in the OPAC and staff client via GetAuthorisedValues which currently sorts results by "lib, lib_opac." Consequently if lib (the description for the staff client) doesn't match lib_opac (the description for the OPAC) sorting will appear to be nonsensical in the OPAC. GetAuthorisedValues can be passed an $opac parameter, so this should be used to switch how reuslts are sorted. This patch implements such a switch. To test, modify your collection code or shelving location authorized values so that lib and lib_opac do not match. Set your AdvancedSearchTypes system preference to display the modified authorized values and view the advanced search page in the OPAC and staff client. Sorting should be correct in each case according to the correct value (lib in the staff client, lib_opac in the OPAC). Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested in staff and opac and it works perfectly! Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> I did not test the patch but the change looks logical.
Created attachment 20507 [details] [review] Bug 10656: add regression test This patch adds regression tests to verify the sort order when fetching authorised values in either staff or OPAC mode. This patch also wraps the Koha.t tests in a transaction, and better handles skipping tests if Test::Deep is not installed. To test: [1] Verify that prove -v t/db_dependent/Koha.t passes Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Created attachment 20508 [details] [review] Bug 10656: improve sorting of shelving location and collections on OPAC advanced search form Collection codes and shelving locations are displayed in the OPAC and staff client via GetAuthorisedValues which currently sorts results by "lib, lib_opac." Consequently if lib (the description for the staff client) doesn't match lib_opac (the description for the OPAC) sorting will appear to be nonsensical in the OPAC. GetAuthorisedValues can be passed an $opac parameter, so this should be used to switch how reuslts are sorted. This patch implements such a switch. To test, modify your collection code or shelving location authorized values so that lib and lib_opac do not match. Set your AdvancedSearchTypes system preference to display the modified authorized values and view the advanced search page in the OPAC and staff client. Sorting should be correct in each case according to the correct value (lib in the staff client, lib_opac in the OPAC). Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested in staff and opac and it works perfectly! Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Created attachment 20509 [details] [review] Bug 10656: (follow-up) handle OPAC sorting of authvals where lib_opac is NULL The OPAC description for an authorized value is not required to be populated. In particular, if it is NULL, the staff description is displayed instead. This match makes sure that the sort order (in OPAC mode) uses either the staff description or the OPAC description as needed for each value. To test: [1] Make sure that AdvancedSearchTypes includes "ccode" [1] Arrange your CCODE values so the sort order for staff labels is different from the sort order for OPAC descriptions. Also, ensure that one of the OPAC descriptions is NULL. For example, authorised_value | lib | lib_opac -------------------------------------- ZZZ | A_STAFF | Z_PUBLIC DDD | D_STAFF | NULL AAA | Z_STAFF | A_PUBLIC [2] Prior to the patch, any CCODE values where the OPAC description is NULL will sort first in the OPAC advanced search page, even if the displayed label shouldn't come first. [3] Apply the patch. [4] Verify that the collection list on the OPAC advanced search page is now correct. [5] Verify that the sort order on the staff advanced search page has not changed. Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Sometimes patches that appear to be trivially correct ... aren't. On the other hand, if you really want to get my attention, combining a signoff with a note that one has not actually tested the patch in question is a very good way to do so. :) I've added a regression test and a follow-up that handles the case when lib_opac is null. Setting to signed-off.
Created attachment 20512 [details] [review] [SIGNED-OFF] Bug 10656: add regression test This patch adds regression tests to verify the sort order when fetching authorised values in either staff or OPAC mode. This patch also wraps the Koha.t tests in a transaction, and better handles skipping tests if Test::Deep is not installed. To test: [1] Verify that prove -v t/db_dependent/Koha.t passes Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tests pass with all patches applied. http://bugs.koha-community.org/show_bug.cgi?id=10565
Created attachment 20513 [details] [review] [SIGNED OFF] Bug 10656: improve sorting of shelving location and collections on OPAC advanced search form Collection codes and shelving locations are displayed in the OPAC and staff client via GetAuthorisedValues which currently sorts results by "lib, lib_opac." Consequently if lib (the description for the staff client) doesn't match lib_opac (the description for the OPAC) sorting will appear to be nonsensical in the OPAC. GetAuthorisedValues can be passed an $opac parameter, so this should be used to switch how reuslts are sorted. This patch implements such a switch. To test, modify your collection code or shelving location authorized values so that lib and lib_opac do not match. Set your AdvancedSearchTypes system preference to display the modified authorized values and view the advanced search page in the OPAC and staff client. Sorting should be correct in each case according to the correct value (lib in the staff client, lib_opac in the OPAC). Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested in staff and opac and it works perfectly! Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 20514 [details] [review] [SIGNED OFF] Bug 10656: (follow-up) handle OPAC sorting of authvals where lib_opac is NULL The OPAC description for an authorized value is not required to be populated. In particular, if it is NULL, the staff description is displayed instead. This match makes sure that the sort order (in OPAC mode) uses either the staff description or the OPAC description as needed for each value. To test: [1] Make sure that AdvancedSearchTypes includes "ccode" [1] Arrange your CCODE values so the sort order for staff labels is different from the sort order for OPAC descriptions. Also, ensure that one of the OPAC descriptions is NULL. For example, authorised_value | lib | lib_opac -------------------------------------- ZZZ | A_STAFF | Z_PUBLIC DDD | D_STAFF | NULL AAA | Z_STAFF | A_PUBLIC [2] Prior to the patch, any CCODE values where the OPAC description is NULL will sort first in the OPAC advanced search page, even if the displayed label shouldn't come first. [3] Apply the patch. [4] Verify that the collection list on the OPAC advanced search page is now correct. [5] Verify that the sort order on the staff advanced search page has not changed. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works nicely, tested in staff and OPAC.
Created attachment 20825 [details] [review] Bug 10656: add regression test This patch adds regression tests to verify the sort order when fetching authorised values in either staff or OPAC mode. This patch also wraps the Koha.t tests in a transaction, and better handles skipping tests if Test::Deep is not installed. To test: [1] Verify that prove -v t/db_dependent/Koha.t passes Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Tests pass with all patches applied. http://bugs.koha-community.org/show_bug.cgi?id=10565 Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 20826 [details] [review] Bug 10656: improve sorting of shelving location and collections on OPAC advanced search form Collection codes and shelving locations are displayed in the OPAC and staff client via GetAuthorisedValues which currently sorts results by "lib, lib_opac." Consequently if lib (the description for the staff client) doesn't match lib_opac (the description for the OPAC) sorting will appear to be nonsensical in the OPAC. GetAuthorisedValues can be passed an $opac parameter, so this should be used to switch how reuslts are sorted. This patch implements such a switch. To test, modify your collection code or shelving location authorized values so that lib and lib_opac do not match. Set your AdvancedSearchTypes system preference to display the modified authorized values and view the advanced search page in the OPAC and staff client. Sorting should be correct in each case according to the correct value (lib in the staff client, lib_opac in the OPAC). Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Tested in staff and opac and it works perfectly! Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 20827 [details] [review] Bug 10656: (follow-up) handle OPAC sorting of authvals where lib_opac is NULL The OPAC description for an authorized value is not required to be populated. In particular, if it is NULL, the staff description is displayed instead. This match makes sure that the sort order (in OPAC mode) uses either the staff description or the OPAC description as needed for each value. To test: [1] Make sure that AdvancedSearchTypes includes "ccode" [1] Arrange your CCODE values so the sort order for staff labels is different from the sort order for OPAC descriptions. Also, ensure that one of the OPAC descriptions is NULL. For example, authorised_value | lib | lib_opac -------------------------------------- ZZZ | A_STAFF | Z_PUBLIC DDD | D_STAFF | NULL AAA | Z_STAFF | A_PUBLIC [2] Prior to the patch, any CCODE values where the OPAC description is NULL will sort first in the OPAC advanced search page, even if the displayed label shouldn't come first. [3] Apply the patch. [4] Verify that the collection list on the OPAC advanced search page is now correct. [5] Verify that the sort order on the staff advanced search page has not changed. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works nicely, tested in staff and OPAC. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master. Thanks, Owen!
This patch has been pushed to 3.12.x, will be in 3.12.5. Thanks Owen!