Bug 27565

I'm pretty sure these dropdowns used to be sorted alphabetically by description.  I'm not sure when they switched, but they all appear to be sorted in the order in which they were added to the system. 

They really need to go back to alpha by description.

To clarify, the url is /cgi-bin/koha/catalogue/itemsearch.pl, and in some cases it looks like they are organized by when they were added, and in other cases, it looks like they are sorted by code.

This DOES need to be fixed.

I can confirm, at the very least, that this is happening in master for Shelving Location on the staff interface's /cgi-bin/koha/catalogue/itemsearch.pl page.

It appears to be sorting by the primary key instead of the description. 

Some relevant lines from the itemsearch.tt file:

[% locations = AuthorisedValues.GetDescriptionsByKohaField({ kohafield = 'items.location' }) %]
[% FOREACH loc IN locations %]
    [% loc.value = loc.authorised_value %]
    [% loc.label = loc.lib %]
[% END %]

Koha::Template::Plugin::AuthorisedValues leads to Koha::AuthorisedValues->get_descriptions_by_koha_field

I'm seeing no sorting...

But I also don't see any obvious changes to this code in years...

By contrast, the Shelving Location is sorted on /cgi-bin/koha/cataloguing/additem.pl as it uses a different module. It uses C4::Koha::GetAuthorisedValues.

Looking at other instances of Koha::AuthorisedValues->get_descriptions_by_koha_field, I see the method is typically used to build an unsorted hashmap, and then that is used to build a sorted array. I'm not sure why Matthias did it that way back in 2016, as it seems inefficient for no gain. Also potentially adds some syntactical issues...

So there are 2 solutions:

1. Add sorting to Koha::AuthorisedValues
2. Add sorting to itemsearch.tt and itemsearch.pl (the data is fetched for different dropdowns in different places)

The most efficient option would be to sort at the database level, so Koha::AuthorisedValues, but it's the heaviest touch, and would require the most testing. Changing itemsearch.pl would be easier although 

For what it's worth, I think that this has been an issue for at least 4 years. 

This is a bit too systemic to write a quick patch, so I'll leave this one for someone with more time.

Hmm looking back at 19.11 and it seems like this wasn't an issue there. 

Ahhh I was misreading the commit history. The patch was from 2017 but it wasn't pushed to a release until 2020. 

Bug 18433 caused this regression.

Created attachment 120333 [details] [review]
Bug 27565: Add sorting to search_by_koha_field

This patch adds sorting by 'category', 'lib' and finally 'lib_opac' as
found in other AV search methods.

Test plan
1

I'm not entirely sure I'm looking at the right things.. I've implemented an initial sort going on Davids suggestions.. It would be great if someone could varify whether this helps or not and we can come up with a proper test plan from that.

We should add tests to t/db_dependent/AuthorisedValues.t no ?

And also impact search_by_marc_field() right ?

I dont see a call on search_by_koha_field() with category param.
Obsolete ?

I see some places call get_descriptions_by_koha_field() with :
{ order_by => ['description'] }

https://git.koha-community.org/Koha-community/Koha/src/commit/4a6dd5f82da1f45456b953421e699abc179a6fb9/reports/catalogue_stats.pl#L120

Does this even work ?

(In reply to Fridolin Somers from comment #7)
> We should add tests to t/db_dependent/AuthorisedValues.t no ?

Definitely.. I just wanted some feedback initially to see if the end result was what people were looking for.

> And also impact search_by_marc_field() right ?

Yeah, this is pretty low level, its effect could be fairly widespread. (But hopefully in a positive way)

> 
> I dont see a call on search_by_koha_field() with category param.
> Obsolete ?

I also wondered that.. nice to see you noticed that too.. I'll investigate to see where it's hanging around from.

(In reply to Fridolin Somers from comment #8)
> I see some places call get_descriptions_by_koha_field() with :
> { order_by => ['description'] }
> 
> https://git.koha-community.org/Koha-community/Koha/src/commit/
> 4a6dd5f82da1f45456b953421e699abc179a6fb9/reports/catalogue_stats.pl#L120
> 
> Does this even work ?

Yeah, I really don't think that works.. Unless I'm missing something.. 'description' is the field name in the UI, not a field name in the database.

(In reply to Martin Renvoize from comment #10)
> (In reply to Fridolin Somers from comment #8)
> > I see some places call get_descriptions_by_koha_field() with :
> > { order_by => ['description'] }
> > 
> > https://git.koha-community.org/Koha-community/Koha/src/commit/
> > 4a6dd5f82da1f45456b953421e699abc179a6fb9/reports/catalogue_stats.pl#L120
> > 
> > Does this even work ?
> 
> Yeah, I really don't think that works.. Unless I'm missing something..
> 'description' is the field name in the UI, not a field name in the database.

Also get_descriptions_by_koha_field() only uses the first given hash as $param, I think second hash is ignored.

Maybe caused by replacement of search_by_koha_field() with get_descriptions_by_koha_field() :
https://git.koha-community.org/Koha-community/Koha/commit/a16a75018063477cf13fdbdad60152bbb27a7616

Is this a duplicate of Bug 28554  ?

(In reply to Fridolin Somers from comment #12)
> Is this a duplicate of Bug 28554  ?

Looks like it could be.. it's been a while since I wrote this patch though so would need to investigate a little deeper.

I believe it's a duplicate that goes a quite similar route - as the newer patches appear to be more current, complete and include unit tests, I am marking this one as the duplicate.

*** This bug has been marked as a duplicate of bug 28554 ***