Bug 24172 - Locale is ignored when saving season name in the database
Summary: Locale is ignored when saving season name in the database
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Blou
QA Contact:
URL:
Keywords: no-sandbox
Depends on:
Blocks:
 
Reported: 2019-12-04 15:23 UTC by Caroline Cyr La Rose
Modified: 2024-02-24 07:28 UTC (History)
11 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (6.85 KB, patch)
2019-12-04 18:07 UTC, Blou
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in details.tt and opac-details.tt (5.91 KB, patch)
2019-12-04 18:46 UTC, Blou
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in details.tt and opac-details.tt (5.93 KB, patch)
2019-12-05 19:44 UTC, Blou
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (6.91 KB, patch)
2020-03-09 10:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in details.tt and opac-details.tt (6.06 KB, patch)
2020-03-09 10:08 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24172: Move duplicated code to an include file (13.95 KB, patch)
2020-03-17 11:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (7.01 KB, patch)
2022-08-25 19:51 UTC, Blou
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in details.tt and opac-details.tt (6.25 KB, patch)
2022-08-25 19:51 UTC, Blou
Details | Diff | Splinter Review
Bug 24172: Move duplicated code to an include file (17.82 KB, patch)
2022-08-25 20:56 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 24172: Move duplicated code to an include file (17.82 KB, patch)
2022-08-25 20:58 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in details.tt and opac-details.tt (6.36 KB, patch)
2022-08-25 20:58 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 24172: Move duplicated code to an include file (17.82 KB, patch)
2022-08-25 20:58 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (14.16 KB, patch)
2022-08-25 21:04 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (15.18 KB, patch)
2023-03-29 13:59 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (15.21 KB, patch)
2023-05-17 12:21 UTC, Hammat wele
Details | Diff | Splinter Review
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt (15.23 KB, patch)
2023-11-08 19:51 UTC, Hammat wele
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caroline Cyr La Rose 2019-12-04 15:23:37 UTC
Hi All, 

it seems the locale is completely ignored when creating numbering with season/month/day names.

What I'm getting on master is that the season name is saved in English in the database, ignoring the locale completely, and depending on the page, the season name will be translated or not for display purposes in the staff interface language. (Steps 1-4 of the test plan are related to bug #20511, but I feel they are related since both have to do with locale being ignored.)

Here's what I did :
0- Make sure you have at least one language other than English installed
1- With staff interface in English, create a subscription with a seasonal numbering pattern and frequency, with locale in your other language
2- Check the prediction pattern, note that the season names are in English not the locale language
3- Change the staff interface to other language, redo the prediction pattern test with locale in other language, note that the season names are in other language
4- Save the subscription
5- Receive an issue, note that the season name in the "Numbered" column on serials-edit.pl is in English
6- Save your issue
7- Switch interface in other language
8- Note that the season name on serials-collection is in other language
9- If you check the database, you will see the season name is in English, even though the subscription.locale is something else

Navigating through different pages, here's what I noticed:
- serials-collection.pl: display is translated
- serials-edit.pl: name pulled directly from db
- subscription-detail.pl: name pulled directly from db
- detail.pl: name pulled directly from db
- opac-detail.pl: name pulled directly from db

I think the locale should be used when saving the season name in the db ensuring all displays are the same and in accordance with the option specified when creating the subscription.

P.S. I'm not sure it has anything to do with it, but I noticed when choosing "French" as locale, the value saved in the db (subscription.locale) is "fre" and not "fr-FR" or "fr-CA". Would that have to do with Koha not being able to find the right translations?
Comment 1 Caroline Cyr La Rose 2019-12-04 15:45:30 UTC
I hadn't noticed, but everything works a-ok for month and day names. The problem happens only when using season names.
Comment 2 Blou 2019-12-04 18:07:20 UTC Comment hidden (obsolete)
Comment 3 Blou 2019-12-04 18:46:28 UTC Comment hidden (obsolete)
Comment 4 Blou 2019-12-04 19:07:33 UTC
Applied it on 19.05 for our customer, it works in French.

Sponsored By: Collecto   (formely known as CCSR)
Comment 5 Blou 2019-12-05 19:44:03 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize 2020-03-09 10:08:37 UTC Comment hidden (obsolete)
Comment 7 Martin Renvoize 2020-03-09 10:08:40 UTC Comment hidden (obsolete)
Comment 8 Martin Renvoize 2020-03-09 10:10:37 UTC
This works as expected so I'm signing it off...

But.. I would really like to see the logic factored out into an include file. It seems ripe for such a factoring task and would lead to less code to maintain and thus a lower likelihood of future mistakes in this area.

I'd really like to see the above addressed as part of QA
Comment 9 Jonathan Druart 2020-03-17 11:48:54 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2020-03-17 11:55:31 UTC
I should have tested the patches before writing the follow-up.
I do not understand what is done here, why do we want to modify the value on editing?
It means we are going to have "Winter" and "Hiver" at the same time in the DB?

The serial will be created with "Winter" before receiving, then "Hiver" once they are received?
That does not seem correct to me.
Comment 11 Jonathan Druart 2020-03-24 09:28:35 UTC
Waiting for an answer.
Comment 12 Blou 2020-04-01 14:53:58 UTC
Don't know what to tell you.

When the user gets to receive the damn serial (bulletiner, en français), it shows up in English, even though all their environment is in French. 

I explained the logic in Comment 2.  It's just a few pages that are lazy and needed to be improved like the others had already been.

It works, it's done like the other pages.  It shouldn't be FQA
Comment 13 Jonathan Druart 2020-04-01 15:31:49 UTC
I FQA it to get an answer...
Will come back later.

You are welcome for the follow-up.
Comment 14 Jonathan Druart 2020-04-08 07:31:01 UTC
Had a look again at this.

I agree with the changes in subscription-detail.tt, catalogue/detail.tt and opac-detail.tt
However I am still not sure what we are doing in serials-edit.tt is correct:

(In reply to Jonathan Druart from comment #10)
> I do not understand what is done here, why do we want to modify the value on
> editing?
> It means we are going to have "Winter" and "Hiver" at the same time in the
> DB?

So basically we have "Winter" in DB, we are editing a serial, "Hiver" is put in the input text letting the user think that it is the value in DB, you save, and "Hiver" is inserted in DB.

Could you confirm it is the behaviour you expect?
Comment 15 Jonathan Druart 2020-04-08 07:33:57 UTC
(I did not test that it works as I described it, but guessing it is how it works reading the code)
Comment 16 Blou 2020-04-08 11:40:54 UTC
I wanted the user with the experience to come explain, but the Situation prevents it.  I'll take time this week to put my head back into it and explain it.
Comment 17 Katrin Fischer 2020-04-12 19:04:04 UTC
It looks like we should always store the English value in the database so that the template code can then translte it to the currently selected language?
Comment 18 Katrin Fischer 2020-04-12 19:05:06 UTC
Blou can you please double check and come back to us? I am not sure about storing 'translated' values in the db if this wasn't done before.
Comment 19 Blou 2020-04-14 12:16:43 UTC
(Sorry, still hadn't come back to it as promessed)

The goal is really that it it stored in English, always.  Then translated on display.  That's what my patches presume.  

The problem with saving in a given Locale, is that it does not always correspond to the current user's preference. I can tell you, "locales" are really non-standard in systems here (let's call that French Canada) and most interfaces are bilingual, always.

Seasons are always saved in English, or at least that's what my French/English testing demonstrated.

I can keep digging for more, if you want.
Comment 20 Jonathan Druart 2020-04-14 12:29:05 UTC
(In reply to Blou from comment #19)
> (Sorry, still hadn't come back to it as promessed)
> 
> The goal is really that it it stored in English, always. 

So we are saying the same thing, maybe you should read again comment 14.
Comment 21 Blou 2020-04-28 20:02:51 UTC
It's very hard to handle <input/> fields with translations.

Without modification, it will display English words.  Definitively what the user does not want.

The alternative (here coded) display in French, and then yes, would save the modification in French in the DB.  The display would then be French everywhere after.  It is NOT a win-win, I admit, but it's certainly closer to the user's needs than the current situation where there's no translation.

I know because I have to handle those requests from many different institutions...

Now, brainstorming, I suppose a solution could be to have the display separated from an empty edit-box, which, if data is entered, would be saved as-is in the DB.  Again, not translatable, though.  Really I don't see an edit option where French doesn't end up in the DB.

I just think it's the least bad option.  

But I'm listening, I admit to limited imagination right now....
Comment 22 Jonathan Druart 2020-04-29 08:16:13 UTC
I don't think this specific needs should be merged with the Koha codebase.
If you really want it you can handle it with some JS code.
Comment 23 Caroline Cyr La Rose 2020-04-29 13:04:20 UTC
Hi Jonathan,
I don't understand why you would be against this. The community certainly has several institutions who have serials in different languages (here, it's mostly french and english, and some spanish too). 

When creating the subscription, we specify which locale the numbering is in. I could have for example National Geographic in English and National Geographic in French. One would have the english numbering and the other the french numbering (ok, in the case of National Geographic it's just Vol. No. but let's say it's seasonal! hehe!)

I don't get why we want this to be translatable (and you know I'm usually for translatability). The numbering is written on the actual physical issue of the serial. It's either going to be in one language or another.

I definitely think this should be in the codebase and not corrected per institution in an ad hoc method with JS...
Comment 24 Jonathan Druart 2020-04-29 13:16:37 UTC
(In reply to Jonathan Druart from comment #14)
> So basically we have "Winter" in DB, we are editing a serial, "Hiver" is put
> in the input text letting the user think that it is the value in DB, you
> save, and "Hiver" is inserted in DB.

Maybe you are right, but this behaviour is totally wrong. We should translate it from the very beginning in that case (and update the DB data with a DB update?).

I am letting another QAer tackling this one as it seems we are stuck.
Comment 25 Blou 2020-07-06 15:20:26 UTC
No DB update will know to which language translate things.  Or not translate, for that matter.

I'm putting it back into "Signed Off" to see if another QAer will tackle it in current form, or provide different feedback.
Comment 26 Nick Clemens 2020-08-28 12:48:53 UTC
I think I agree with Jonathan and Katrin - we should not store the translated value in the DB if we can avoid it.

The rest of these patches have no problems, they look good.

I think we do need to pursue the idea of a separate edit box, perhaps using the same splitting as in the translation - get matches before season name, that is 1 inoutm the season is a translated dropdown with english in the values, then a second input for trailing.

Adding a 'blank' option to dropdown will allow removing season name for special issues.

I would suggest removing the changes to the edit screen and tackling the editing on a new bug.
Comment 27 Katrin Fischer 2021-10-29 16:35:50 UTC
I can confirm this in 20.11 still.
Comment 28 Jonathan Druart 2022-02-17 15:22:58 UTC
Bug 20511 may fix this.
Comment 29 Blou 2022-08-25 19:47:22 UTC
Bug 20511 doesn't help.

Since this patch won't get any support, but we need it, we'll rebase it for our own needs.  Including a squash for simplicity sake.
Comment 30 Blou 2022-08-25 19:51:11 UTC Comment hidden (obsolete)
Comment 31 Blou 2022-08-25 19:51:24 UTC Comment hidden (obsolete)
Comment 32 Katrin Fischer 2022-08-25 19:53:27 UTC
I can try to look into this one, as I've also tested bug 20511, but since I'll be on vacation next week, it won't be quick. Please switch to needs signoff or signed off (depending on the nature of the rebase) when you think this is ready for testing again.
Comment 33 Katrin Fischer 2022-08-25 19:54:50 UTC
(In reply to Katrin Fischer from comment #18)
> Blou can you please double check and come back to us? I am not sure about
> storing 'translated' values in the db if this wasn't done before.

Looking back, we need to store the translated value in some cases. Maybe what I was thinking about was the transition - how to handle existing subscriptions.
Comment 34 Shi Yao Wang 2022-08-25 20:56:02 UTC Comment hidden (obsolete)
Comment 35 Shi Yao Wang 2022-08-25 20:58:07 UTC Comment hidden (obsolete)
Comment 36 Shi Yao Wang 2022-08-25 20:58:39 UTC Comment hidden (obsolete)
Comment 37 Shi Yao Wang 2022-08-25 20:58:44 UTC Comment hidden (obsolete)
Comment 38 Shi Yao Wang 2022-08-25 21:04:40 UTC Comment hidden (obsolete)
Comment 39 Blou 2023-02-07 14:58:44 UTC
Hello Katrin,

I noticed you made some slight turn around on this bug.  Is there anything else that could be done?  

Although Nick sided with Jonathan, I don't think anyone refuted Caroline's arguments ont comment 23.  This needs to be stored.

So if we're talking transition, we're all ear!   Here, and at Collecto (so about 100 large institutions total), we just apply the patch.  As it was mostly (and wrongly) shown in English beforehand, it still is.  At least until the user can now edit it as needed.  I have a hard time imagining an atomic update that could fix that.
Comment 40 Magnus Enger 2023-03-29 12:10:13 UTC Comment hidden (obsolete)
Comment 41 Hammat wele 2023-03-29 13:59:58 UTC Comment hidden (obsolete)
Comment 42 Hammat wele 2023-03-29 14:50:49 UTC Comment hidden (obsolete)
Comment 43 Magnus Enger 2023-03-30 08:38:19 UTC
(In reply to Caroline Cyr La Rose from comment #23)
> I don't get why we want this to be translatable (and you know I'm usually
> for translatability). The numbering is written on the actual physical issue
> of the serial. It's either going to be in one language or another.

One one hand I think this makes a lot of sense. Just write the season name in the language of the journal/periodical. 

But I guess the problem is with predicting the numbering/naming patterns? 

I think someone said somwhere that names for months and weekdays are working. Does seasons have to be handled differently than those? Could we do things with months in the same way?
Comment 44 Hammat wele 2023-05-17 12:21:18 UTC
Created attachment 151330 [details] [review]
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt

Seasons are not handled by locales.  Those generated are stored in
English in the database, and the display in serials-collection.tt is
translating them.  But once you click on "Edit serials" or if you go
straight to the subscription's display, those are displayed as in the
database: in English.

This patch reproduces the same trick as in serial-collection.tt and
allow for the display to be translated.

NOTE: this patch's testing require knowledge of translation tools.

Reproducing Caroline's test:
0- Make sure you have at least one language other than English installed
1- With staff interface in English, create a subscription with a seasonal numbering pattern and frequency, with locale in your other language
2- Check the prediction pattern, note that the season names are in English not the locale language
3- Change the staff interface to other language, redo the prediction pattern test with locale in other language, note that the season names are in other language
4- Save the subscription
5- Receive an issue, note that the season name in the "Numbered" column on serials-edit.pl is in English
6- Save your issue
7- Switch interface in other language
8- Note that the season name on serials-collection is in other language
9- If you check the database, you will see the season name is in English, even though the subscription.locale is something else
10- validate the different pages:
- serials-collection.pl: display is translated
- serials-edit.pl: name pulled directly from db
- subscription-detail.pl: name pulled directly from db
- detail.pl: name pulled directly from db
- opac-detail.pl: name pulled directly from db
11- APPLY THE PATCH
12- translate the string
  a) ./translate update <some locale>
  b) find the strings, translate them
  c) ./translate install <same locale>
13- restart Plack ?
14- validate the pages again.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24172: Allow translation of seasons in details.tt and opac-details.tt

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24172: Move duplicated code to an include file
Comment 45 Hammat wele 2023-11-08 19:51:29 UTC
Created attachment 158671 [details] [review]
Bug 24172: Allow translation of seasons in serials-edit.tt and subscription-detail.tt

Seasons are not handled by locales.  Those generated are stored in
English in the database, and the display in serials-collection.tt is
translating them.  But once you click on "Edit serials" or if you go
straight to the subscription's display, those are displayed as in the
database: in English.

This patch reproduces the same trick as in serial-collection.tt and
allow for the display to be translated.

NOTE: this patch's testing require knowledge of translation tools.

Reproducing Caroline's test:
0- Make sure you have at least one language other than English installed
1- With staff interface in English, create a subscription with a seasonal numbering pattern and frequency, with locale in your other language
2- Check the prediction pattern, note that the season names are in English not the locale language
3- Change the staff interface to other language, redo the prediction pattern test with locale in other language, note that the season names are in other language
4- Save the subscription
5- Receive an issue, note that the season name in the "Numbered" column on serials-edit.pl is in English
6- Save your issue
7- Switch interface in other language
8- Note that the season name on serials-collection is in other language
9- If you check the database, you will see the season name is in English, even though the subscription.locale is something else
10- validate the different pages:
- serials-collection.pl: display is translated
- serials-edit.pl: name pulled directly from db
- subscription-detail.pl: name pulled directly from db
- detail.pl: name pulled directly from db
- opac-detail.pl: name pulled directly from db
11- APPLY THE PATCH
12- translate the string
  a) ./translate update <some locale>
  b) find the strings, translate them
  c) ./translate install <same locale>
13- restart Plack ?
14- validate the pages again.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24172: Allow translation of seasons in details.tt and opac-details.tt

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 24172: Move duplicated code to an include file
Comment 46 Jonathan Druart 2023-11-08 20:12:20 UTC
(In reply to Jonathan Druart from comment #14)
> Had a look again at this.
> 
> I agree with the changes in subscription-detail.tt, catalogue/detail.tt and
> opac-detail.tt
> However I am still not sure what we are doing in serials-edit.tt is correct:
> 
> (In reply to Jonathan Druart from comment #10)
> > I do not understand what is done here, why do we want to modify the value on
> > editing?
> > It means we are going to have "Winter" and "Hiver" at the same time in the
> > DB?
> 
> So basically we have "Winter" in DB, we are editing a serial, "Hiver" is put
> in the input text letting the user think that it is the value in DB, you
> save, and "Hiver" is inserted in DB.
> 
> Could you confirm it is the behaviour you expect?

Is this fixed?

I am still setting that:
+            Issue <input type="text" name="serialseq" id="serialseq[% serialslis.serialid | html %]"
+            value="[% PROCESS localize_numbering_pattern_season season => serialslis.serialseq %]"
+            size="20" maxlength="100" />
Comment 47 Victor Grousset/tuxayo 2024-02-22 20:33:17 UTC
Previous comment needs to be addressed before another signoff attempt, in case that require changes and thus being tested again.
Comment 48 Blou 2024-02-22 20:54:51 UTC
The previous comment is not clear on what needs to be corrected.  Considering Jonathan's objections were previously on architectural principles, I do not think those lines suddenly present the last hurdle for this to pass.

Hammat rebased it so that we can have it working for the latest version (us, Collecto, and anyone else interested), but I don't think any serious change will be made to our solution without a clear path to Passed QA.  The person in charge will rebase it again in a few months and put it as "Need signoff" out of habit.

I leave it as Failed QA
Comment 49 Victor Grousset/tuxayo 2024-02-24 07:28:24 UTC
Ok this is a though one ^^"

After rereading most stuff here: comment 1 mentions that days and months work.
After testing:
a. the localized value is saved directly in the database[1]
b. which is likely why all pages of test plan step 10 are following subscription.locale for days and months

So questions for everyone depending on how you view the current patches proposed here:
- related to a. : what's the issue with the proposed patches if day and month already store a localized value?
- related to b. : do seasons need to be handled differently than day and month? (asked in comment 43) Because the patches are going to a different logic: following current UI locale IIUC. Even though days and months at first seemed satisfactory (comment 1)


[1] is that the right place to look? : select * from subscriptionhistory;