Bug 38903

Summary: getTranslatedLanguages is still called with wrong theme
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: I18N/L10NAssignee: Marcel de Rooy <m.de.rooy>
Status: Pushed to main --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: f.demians, jonathan.druart, julian.maurice, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on: 38630    
Bug Blocks: 38904    
Attachments: Bug 38903: Fix theme in some getTranslatedLanguages calls
Bug 38903: Fix theme in some getTranslatedLanguages calls
Bug 38903: Fix theme in some getTranslatedLanguages calls

Description Marcel de Rooy 2025-01-16 08:19:48 UTC
Four years after bug 29245. Triggered by work on bug 38630. Another try ;)
Comment 1 Marcel de Rooy 2025-01-16 10:33:57 UTC
Created attachment 176621 [details] [review]
Bug 38903: Fix theme in some getTranslatedLanguages calls

Combining opac with pref template is wrong. This pref should
actually be renamed to something like intranetTheme(s) or so.

Replacing the obsolete prog theme in Languages.t by undef. This
achieves the same: getting all themes for that interface.

Test plan:
Add some languages for opac and intranet. Do not enable exactly
the same set.
Enable TranslateNotices.
Verify that you have all OPAC languages on memberentry and
opac-messaging. And all languages on additional contents.
Run t/db_dependent/Languages.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with:
OPAC  languages: en, nl-NL, de-DE
Staff languages: en, de-DE, fr-FR
Comment 2 Marcel de Rooy 2025-01-16 10:35:29 UTC
Trivial adding self SO. You might view them as QA follow-ups for bug 38630 too.

Julian: Could you QA here ?
Comment 3 Julian Maurice 2025-01-20 10:55:59 UTC
I've tried different combinations of sysprefs, and even tried the exact same syspref values as in comment 1. All installed languages (even those that are disabled in both interfaces) show up in language selectors on memberentry and opac-messaging.
Applying the patch does not change anything for me.
The changes make sense to me but I cannot validate them because of that.
Comment 4 Marcel de Rooy 2025-01-20 14:25:47 UTC
(In reply to Julian Maurice from comment #3)
> I've tried different combinations of sysprefs, and even tried the exact same
> syspref values as in comment 1. All installed languages (even those that are
> disabled in both interfaces) show up in language selectors on memberentry
> and opac-messaging.
> Applying the patch does not change anything for me.
> The changes make sense to me but I cannot validate them because of that.

Strange. Would restart / flushing cache make a difference?
Comment 5 Julian Maurice 2025-01-20 14:31:33 UTC
I restarted koha & memcached each time I switched branch (which I did a few times) so cache was not the problem
Comment 6 Julian Maurice 2025-01-31 08:35:13 UTC
After looking at the code, I think this patch would make a difference only if the language directories in koha-tmpl/intranet-tmpl/prog/ are different from the language directories in koha-tmpl/opac-tmpl/bootstrap/
But how could we end up in this situation as the `translate install` command always install languages in both locations ?
Comment 7 Marcel de Rooy 2025-01-31 08:53:07 UTC
(In reply to Julian Maurice from comment #6)
> After looking at the code, I think this patch would make a difference only
> if the language directories in koha-tmpl/intranet-tmpl/prog/ are different
> from the language directories in koha-tmpl/opac-tmpl/bootstrap/
> But how could we end up in this situation as the `translate install` command
> always install languages in both locations ?

Yes thats an important observation. Look at my test description:
OPAC  languages: en, nl-NL, de-DE
Staff languages: en, de-DE, fr-FR

Note that we locally do not translate staff. So after e.g. installing nl-NL I am removing those folders on intranet-tmpl.
Comment 8 Julian Maurice 2025-01-31 09:07:29 UTC
(In reply to Marcel de Rooy from comment #7)
> Yes thats an important observation. Look at my test description:
> OPAC  languages: en, nl-NL, de-DE
> Staff languages: en, de-DE, fr-FR
> 
> Note that we locally do not translate staff. So after e.g. installing nl-NL
> I am removing those folders on intranet-tmpl.
I thought those were values of systempreferences OPACLanguages and StaffInterfaceLanguages... >_<
Comment 9 Marcel de Rooy 2025-02-14 08:34:52 UTC
(In reply to Julian Maurice from comment #8)
> (In reply to Marcel de Rooy from comment #7)
> > Yes thats an important observation. Look at my test description:
> > OPAC  languages: en, nl-NL, de-DE
> > Staff languages: en, de-DE, fr-FR
> > 
> > Note that we locally do not translate staff. So after e.g. installing nl-NL
> > I am removing those folders on intranet-tmpl.
> I thought those were values of systempreferences OPACLanguages and
> StaffInterfaceLanguages... >_<

Could you QA please?
Comment 10 Marcel de Rooy 2025-02-14 08:36:15 UTC
Wait, i need to rebase first ;) tidy all..
Comment 11 Marcel de Rooy 2025-02-19 12:58:07 UTC
Created attachment 178330 [details] [review]
Bug 38903: Fix theme in some getTranslatedLanguages calls

Combining opac with pref template is wrong. This pref should
actually be renamed to something like intranetTheme(s) or so.

Replacing the obsolete prog theme in Languages.t by undef. This
achieves the same: getting all themes for that interface.

Test plan:
Add some languages for opac and intranet. Do not enable exactly
the same set.
Enable TranslateNotices.
Verify that you have all OPAC languages on memberentry and
opac-messaging. And all languages on additional contents.
Run t/db_dependent/Languages.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with:
OPAC  languages: en, nl-NL, de-DE
Staff languages: en, de-DE, fr-FR
Comment 12 Marcel de Rooy 2025-02-19 12:58:24 UTC
(In reply to Marcel de Rooy from comment #10)
> Wait, i need to rebase first ;) tidy all..

Rebased.
Comment 13 Martin Renvoize (ashimema) 2025-03-07 12:15:10 UTC
Created attachment 179069 [details] [review]
Bug 38903: Fix theme in some getTranslatedLanguages calls

Combining opac with pref template is wrong. This pref should
actually be renamed to something like intranetTheme(s) or so.

Replacing the obsolete prog theme in Languages.t by undef. This
achieves the same: getting all themes for that interface.

Test plan:
Add some languages for opac and intranet. Do not enable exactly
the same set.
Enable TranslateNotices.
Verify that you have all OPAC languages on memberentry and
opac-messaging. And all languages on additional contents.
Run t/db_dependent/Languages.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with:
OPAC  languages: en, nl-NL, de-DE
Staff languages: en, de-DE, fr-FR
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2025-03-07 12:16:10 UTC
Took me a few minutes here to wrap my head around the translation picking code again, but I agree with the changes.

No regressions and barely noticeable change in most cases.. but in the few cases it counts this is an important change.

Passing QA
Comment 15 Katrin Fischer 2025-03-07 14:42:51 UTC
Pushed for 25.05!

Well done everyone, thank you!