Bug 35582 - Advanced search languages should be listed with selected UI language descriptions shown first if available
Summary: Advanced search languages should be listed with selected UI language descript...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Bugs List
QA Contact: Kyle M Hall (khall)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-15 13:29 UTC by Lari Strand
Modified: 2024-05-28 21:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12
Circulation function:


Attachments
Bug 35582 - Show advanced search languages with selected intranet language descriptions first (1.54 KB, patch)
2023-12-15 13:48 UTC, Lari Strand
Details | Diff | Splinter Review
Bug 35582: Advanced search languages should be listed with selected UI language descriptions shown first if available (1.57 KB, patch)
2024-01-08 12:11 UTC, Lari Strand
Details | Diff | Splinter Review
Bug 35582: Show advanced search languages with selected intranet language descriptions first (1.97 KB, patch)
2024-02-20 11:01 UTC, Lari Strand
Details | Diff | Splinter Review
Bug 35582: Show advanced search languages with selected intranet language descriptions first (2.03 KB, patch)
2024-02-20 16:30 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35582: Show advanced search languages with selected intranet language descriptions first (2.08 KB, patch)
2024-04-26 18:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 35582: Supress possible warning (1.17 KB, patch)
2024-04-26 18:42 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Strand 2023-12-15 13:29:17 UTC
Languages are being showed by their localized name first in the advanced search language limit dropdowns and the user's intranet language selection's description follows next (or defaults to English). It's hard to select the language you want from the list if the languages' descriptions are written in the way they are written in the country the language is being used, Suomi (Finnish) or ལྷ་སའི་སྐད་ (Standard Tibetan) for example. They should be listed by the language description that matches the user's selected language in the intranet first and then the local name, like Finnish (Suomi), Standard Tibetan (ལྷ་སའི་སྐད)་ if the intranet language selection is English for example.
Comment 1 Lari Strand 2023-12-15 13:48:38 UTC
Created attachment 159892 [details] [review]
Bug 35582 - Show advanced search languages with selected intranet language descriptions first
Comment 2 Lari Strand 2023-12-15 13:53:15 UTC
We are also using an intranetUserJS-string to alphabetize and prioritize the languages which obviously does not work as intended if the native description is showing first instead of the selected UI language's description.
Comment 3 Lucas Gass (lukeg) 2023-12-15 21:58:42 UTC
Lari,

The patch seems to work but could you provide a test plan? 


Also, please mind the the QA tool:

Processing additional checks

	* Commit title does not start with 'Bug XXXXX: ' - 80ed34f406
Comment 4 Katrin Fischer 2023-12-16 13:12:24 UTC
I agree with the idea, but there is a problem: The translations are are not handled by po files, but are stored in the database and we only have very few translations in the standard files. And there is no way to add translations apart from using SQL. 

So a lot of people might see a list sorted by the English translations/descriptions.

Just to be aware of. For testing French and German should work reasonably well.
Comment 5 Lari Strand 2024-01-08 12:11:42 UTC
Created attachment 160621 [details] [review]
Bug 35582: Advanced search languages should be listed with selected UI language descriptions shown first if available
Comment 6 Lari Strand 2024-01-08 12:18:05 UTC
To test:

1) Go to advanced search and bring up the language pulldowns from the Limits section
2) Notice the languages are shown with their native description first
3) Apply patch
4) Bring up the languages dropdowns and notice the localized language translations are shown first if the language (selected intranet language) has a local translation in language_descriptions table, otherwise defaulting to the English translation.
Comment 7 Lari Strand 2024-01-08 12:30:59 UTC
I can provide full Finnish and Swedish translations for the language codes if interested. I seems a couple of language codes are missing altogether from the English language descriptions in 23x. Here's a few:

afa | Afroasiatic languages
bua | Buriat
fro | French, Old (ca. 842-1400)
haw | Hawaiian
lad | Ladino
nai | North American Indian languages
tmh | Tamashek
wol | Wolof

I can open another ticket for this / these.
Comment 8 Lari Strand 2024-01-08 12:41:38 UTC
Seems a couple of language codes are missing altogether
> from the English language descriptions in 23x. Here's a few:
> 
> afa | Afroasiatic languages
> bua | Buriat
> fro | French, Old (ca. 842-1400)
> haw | Hawaiian
> lad | Ladino
> nai | North American Indian languages
> tmh | Tamashek
> wol | Wolof
> 
Sorry these were missing from our Finnish/Swedish translations.

English translations that seem to be missing:

afa, chu, den, haw, sqn
Comment 9 Lari Strand 2024-01-10 14:48:28 UTC
Just found the ticket about adding more language codes/translations:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33134

I'll join/follow the discussion there.
Comment 10 Owen Leonard 2024-01-11 12:37:08 UTC
The patch still needs the test plan to be added to the commit message. 

Can you take a look at the perltidy warning shown by the qa tool?
Comment 11 Lari Strand 2024-01-16 10:01:00 UTC
The qa tool complains about this row:

$language_subtag_registry->{language_description}.=" ($native_description)";
If I delete the row from the commit, the qa passes.

The line is like a copy-paste from the old code line:

$language_subtag_registry->{language_description}.=" ($language_descriptions->{description})" if $language_descriptions->{description};

I can't get it to pass validation without breaking the code. It's still valid perl and perltidy in VSCode doesn't want to touch the row (using perltidyrc from https://git.koha-community.org/Koha-community/Koha/src/commit/7e8f41cce9353a1aeda333906db0355b7066ab66/.perltidyrc if I recall).

How do we move forward from this?
Comment 12 Lari Strand 2024-01-16 10:43:07 UTC
If I run the whole file through pertidy the qa tool is happy. There's 211 dirty lines in the file according to qa tool before the commit. Don't know how many messy lines got "fixed" by perltidy, but the qa tool is happy if there's less of them than before the commit it seems. Isn't this a problem with the qa tool for falsely reporting my committed line as messy rather that it actually being a messy new line?
Comment 13 Katrin Fischer 2024-01-22 22:00:18 UTC
(In reply to Lari Strand from comment #12)
> If I run the whole file through pertidy the qa tool is happy. There's 211
> dirty lines in the file according to qa tool before the commit. Don't know
> how many messy lines got "fixed" by perltidy, but the qa tool is happy if
> there's less of them than before the commit it seems. Isn't this a problem
> with the qa tool for falsely reporting my committed line as messy rather
> that it actually being a messy new line?

I think the truth is in the middle. If I tidy just the if-else block the tests pass too.
The lines show as changed in the patch - I guess that's what triggers.
Comment 14 Lari Strand 2024-02-20 11:01:49 UTC
Created attachment 162293 [details] [review]
Bug 35582: Show advanced search languages with selected intranet language descriptions first

To test:

1. Check language dropdowns in advanced search, notice localized
   names of the languages are shown first in the menus, then the
selected ui language translations if available or the english translation
2. Apply patch
3. Check language dropdowns, notice the selected ui language's
   translations are shown first for the languages (if missing, english
translation), then the localized
language's name.
Comment 15 Pedro Amorim 2024-02-20 16:30:44 UTC
Created attachment 162302 [details] [review]
Bug 35582: Show advanced search languages with selected intranet language descriptions first

To test:

1. Check language dropdowns in advanced search, notice localized
   names of the languages are shown first in the menus, then the
selected ui language translations if available or the english translation
2. Apply patch
3. Check language dropdowns, notice the selected ui language's
   translations are shown first for the languages (if missing, english
translation), then the localized
language's name.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 16 Pedro Amorim 2024-02-20 16:31:52 UTC
prove t/db_dependent/Languages.t pass

Had to install arab, finish and french, added "ara|fin|fre" to AdvancedSearchLanguages, only then was able to test the advanced search language dropdown.
Everything works as described. Makes sense.

Perhaps this same enhancement could/should be considered for INTRA/OPAC (req opaclanguagesdisplay) language pickers (default at bottom of page)
Comment 17 Kyle M Hall (khall) 2024-04-26 18:42:16 UTC
Created attachment 165692 [details] [review]
Bug 35582: Show advanced search languages with selected intranet language descriptions first

To test:

1. Check language dropdowns in advanced search, notice localized
   names of the languages are shown first in the menus, then the
selected ui language translations if available or the english translation
2. Apply patch
3. Check language dropdowns, notice the selected ui language's
   translations are shown first for the languages (if missing, english
translation), then the localized
language's name.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Kyle M Hall (khall) 2024-04-26 18:42:23 UTC
Created attachment 165693 [details] [review]
Bug 35582: Supress possible warning

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 19 Katrin Fischer 2024-04-29 16:55:26 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 20 Fridolin Somers 2024-05-24 12:07:15 UTC
Nice small enhancement, I backport

Pushed to 23.11.x for 23.11.06
Comment 21 Lucas Gass (lukeg) 2024-05-28 21:09:27 UTC
Backported to 23.05.x for upcoming 23.05.12