Bug 24172

I hadn't noticed, but everything works a-ok for month and day names. The problem happens only when using season names.

Applied it on 19.05 for our customer, it works in French.

Sponsored By: Collecto   (formely known as CCSR)

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

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.

Waiting for an answer.

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

I FQA it to get an answer...
Will come back later.

You are welcome for the follow-up.

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?

(I did not test that it works as I described it, but guessing it is how it works reading the code)

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.

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?

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.

(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.

(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.

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....

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.

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...

(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.

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.

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.

I can confirm this in 20.11 still.

Bug 20511 may fix this.

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.

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.

(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.

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.

(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?

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

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

(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" />

Previous comment needs to be addressed before another signoff attempt, in case that require changes and thus being tested again.

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

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;

Created attachment 170319 [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

Created attachment 171104 [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

Created attachment 172223 [details] [review]
Bug 24172: [WIP alternate] Process seasons using locale when generating

FIXME: I like this path, however, what we really need is a way to get localized seasons in the perl.
DateTime handles months and daynames, but has no support for seasons. This patch won't work because it
relies on having the languages installed and being able to translate locale 'fre' to language dir 'fr-FR'
Attaching only for possible future reference.

Currently we are always generating season strings as English, and translating
in the templates.

Compare this to handling of day/month names which are translated when generated.

The numbering for a serial issue is expected to be static as printed - i.e. the French version
of an issue will always say 'Hiver' while the English will always say 'Winter'.
Translating depending on the interface language will make it harder for users to locate
issues in a bilingual context

This patch relies on bug 38034 allowing the processing of includes in process_tt

If this patch is acceptable we will need a follow-up with a script to correct older serials to the
correct locale in the DB - we should be able to run them through process_tt with the correct locale

To test:
0 - Apply patch
1 - Make sure you have at least one language other than English installed, we're using French here
2 - Ensure translations are updated after this patch, or install the patch with the fr-FR localizer.inc
2 - Create 2 subscriptions, using numbering 'Seasonal', 1 with locale in english, 1 with locale French
3 - Ensure the prediction is in correct language for the locale
4 - Receive some issues
5 - Confirm the issues are generated with the correct locale for each subscription
6 - Ensure that changing the interface for the staff client does not affect the numering for any views

 Author:    Nick <nick@bywatersolutions.com>

Created attachment 172230 [details] [review]
Bug 24172: (follow-up) Split serial sequence upon editing and server

This patch checks whether we find a season match in the numeration when editing/receiving a serial. If so,
we split the input into three boxes, the before string, the season as a dropdown, and the after string.

The dropdown visible values wil be transalated, however, the values will be in English.
It's a bit ugly, but it works.

To test:
1 - Apply patch, update translation
2 - Receive some issues of the serial
3 - Try receiving in English or your other lang
4 - Confirm values are saved in English in the DB either way
5 - Confirm issues show in the currently selected language either way
6 - Test with a non-seasonal serial, confirm no changes

I tried my hand here - when editing we split the value and provide a dropdown that is only visibly translated.

My WIP patch would be a better solution, translating the values when generating, but it needs more infrastructure.

If InLibro accepts my patch I think we can possibly move this one forward - or anyone can feel free toed ti what I have done.

Created attachment 174095 [details] [review]
Bug 24172: Add missing localizer.inc for opac