|
Description
Lari Strand
2023-12-15 13:29:17 UTC
Created attachment 159892 [details] [review] Bug 35582 - Show advanced search languages with selected intranet language descriptions first 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. 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 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. Created attachment 160621 [details] [review] Bug 35582: Advanced search languages should be listed with selected UI language descriptions shown first if available 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. 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. 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
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. 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? 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?
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? (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. 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. 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> 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) 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> Created attachment 165693 [details] [review] Bug 35582: Supress possible warning Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Pushed for 24.05! Well done everyone, thank you! Nice small enhancement, I backport Pushed to 23.11.x for 23.11.06 Backported to 23.05.x for upcoming 23.05.12 |