Bug 30028

Summary: Patron message delete confirmation untranslatable
Product: Koha Reporter: Caroline Cyr La Rose <caroline.cyr-la-rose>
Component: I18N/L10NAssignee: Shi Yao Wang <shi-yao.wang>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, f.demians, jonathan.druart, lucas, matted-34813, shi-yao.wang, victor
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes the patron delete messages dialogue box to make the message shown translatable.
Version(s) released in:
22.11.00,22.05.06,21.11.12
Attachments: Bug 30028: Fix patron message delete confirmation translation (fr-CA)
Bug 30028: Fix patron message delete confirmation translation (fr-CA)
Bug 30028: Fix patron message delete confirmation translation (fr-CA)
Bug 30028: Fix patron message delete confirmation to be translatable
Bug 30028: Fix patron message delete confirmation to be translatable
Bug 30028: Fix patron message delete confirmation to be translatable
Bug 30028: Fix patron message delete confirmation to be translatable
Bug 30028: Fix patron message delete confirmation to be translatable
Bug 30028: Fix patron message delete confirmation to be translatable
Bug 30028: Fix patron message delete confirmation to be translatable

Description Caroline Cyr La Rose 2022-02-04 16:12:54 UTC
When we delete a patron message, there is a confirmation message that is untranslatable. It may be because it's a browser message or something, rather than a Koha modal?

To get to this message:
1) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
2) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".

This sentence is in koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc
but it is not in any .po file.

Caroline
Comment 1 Shi Yao Wang 2022-03-25 17:52:11 UTC Comment hidden (obsolete)
Comment 2 Shi Yao Wang 2022-03-25 17:57:28 UTC Comment hidden (obsolete)
Comment 3 Shi Yao Wang 2022-05-27 12:45:52 UTC
Created attachment 135419 [details] [review]
Bug 30028: Fix patron message delete confirmation translation (fr-CA)

When we delete a patron message, there is a confirmation message that is
not translated. Here is the fix (for fr-CA translations)

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
6) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"
Comment 4 Owen Leonard 2022-05-27 13:25:35 UTC
I guess it's not in the Coding Guidelines, but we shouldn't use "javascript:" to handle events. The template before your patch *also* wasn't doing it correctly (see https://wiki.koha-community.org/wiki/Coding_Guidelines#JS9:_Avoid_the_use_of_event_attributes_like_.22onclick.22_to_attach_events).

See members-menu.js, where click handlers are added for the various controls in the patron toolbar. I recommend adding an ID attribute to the deletion link so that you can target it in members-menu.js

Also, changes to the po file should not be included in the patch. Patch testers will need to update the translations themselves.
Comment 5 Shi Yao Wang 2022-05-27 14:59:46 UTC
Created attachment 135432 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-staff-prog.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"
Comment 6 David Nind 2022-05-28 23:24:32 UTC
Created attachment 135441 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-staff-prog.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2022-05-28 23:48:57 UTC
Testing notes (using koha-testing-docker):

1. Before applying the patch, I grepped for the missing phrase - there were no occurrences:
   - git grep "Are you sure you want to delete this message? This cannot be undone." misc/translator/po/fr-CA-*.po 

2. Sequence for updating a translation, after patch applied:
   - cd misc/translator
   - ./translate update fr-CA
   - update the strings in po file: vi po/fr-CA-staff-prog.po 
   - install the updated translation: ./translate install fr-CA
   - file ownership was changed to root, I needed to change so that updated messages appear: sudo chown kohadev-koha:kohadev-koha po/fr-CA-*.po
   - test that updated translation works
   - clean up files so patch can be signed off rather than having to individually delete unstaged files and check out modified files: 
     . change back to the koha directory cd ../../ 
     . git checkout
     . git clean -fd

3. When editing the translations, I deleted the extra lines 13586 and 13589 - not sure whether that was necessary or not, but it worked for me! Lines before updated:

13584 #. SCRIPT
13585 #: koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc:230
13586 #, fuzzy
13587 msgid "Are you sure you want to delete this message? This cannot be undone."
13588 msgstr ""
13589 "Voulez-vous vraiment supprimer cette règle? L'opération est irréversible."

4. I did notice that there were a few examples of empty "" after msgstr and a translation straight after that (as above). I'm not familiar enough with translations to know whether this is something that is expected.

5. I also noticed that after adding a patron message, going to delete it, and pressing cancel - the message is deleted when it shouldn't be. I have logged bug 30863 for this.
Comment 8 Jonathan Druart 2022-06-01 11:44:54 UTC
Why did you split the 2 JS code?
Comment 9 Shi Yao Wang 2022-06-01 12:44:31 UTC
(In reply to Jonathan Druart from comment #8)
> Why did you split the 2 JS code?

The popup couldn't be translated if I put everything in members-menu.js
The .po files only affect .tt and .inc files no?
Comment 10 Jonathan Druart 2022-06-01 13:02:46 UTC
Use __("") (2 underscores) from .js files, it should then work.
Comment 11 Shi Yao Wang 2022-06-01 13:47:42 UTC
Created attachment 135548 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-staff-prog.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Shi Yao Wang 2022-06-01 13:54:52 UTC
(In reply to Jonathan Druart from comment #10)
> Use __("") (2 underscores) from .js files, it should then work.

I tried it and the popup is still in english.
The .po files generates a copy of .tt and .inc files and replaces every matching msgid by their respective msgstr, but it doesn't do that with .js files.
Comment 13 Shi Yao Wang 2022-06-01 13:59:33 UTC
Nevermind, I looked a bit more and I see koha-tmpl/intranet-tmpl/prog/fr-CA/js/locale_data.js was generated(?) by the translations.
Comment 14 David Nind 2022-06-01 21:52:47 UTC
Created attachment 135562 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-staff-prog.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 Jonathan Druart 2022-06-02 05:48:27 UTC
Nothing has changed here.
Comment 16 Shi Yao Wang 2022-06-02 12:44:37 UTC
I actually added a "return" in front of "confirm_message_deletion()" because otherwise it would always delete the message even if "No" was selected.

Working on a solution where the JS is not split.
Comment 17 Shi Yao Wang 2022-06-02 13:27:10 UTC
Created attachment 135584 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-messages-js.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 18 Shi Yao Wang 2022-06-02 13:32:32 UTC
Finally got it.

(In reply to Jonathan Druart from comment #10)
> Use __("") (2 underscores) from .js files, it should then work.

You were right, but the translations had to be in misc/translator/po/fr-CA-messages-js.po rather than in misc/translator/po/fr-CA-staff-prog.po
Comment 19 wajasu 2022-07-08 20:41:15 UTC
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135584
This patch remove the popup that has the "Are you sure you want to delete this message? This cannot be undone." english text. But I think they wanted it translated to french.
Adding it to: fr-CA-messages-js.po
Some message id coded to use some method/tool(template tooklit?) to help lookup the translation when french is selected.

DavidNind mentioned in IRC:
translations are done at translate.koha-community.org, and will picked up at release time
16:11 < davidnind[m]> https://wiki.koha-community.org/wiki/Release_maintenance#Push_updated_translation_files
Comment 20 Shi Yao Wang 2022-07-08 21:30:14 UTC
(In reply to wajasu from comment #19)
> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135584
> This patch remove the popup that has the "Are you sure you want to delete
> this message? This cannot be undone." english text. But I think they wanted
> it translated to french.
> Adding it to: fr-CA-messages-js.po
> Some message id coded to use some method/tool(template tooklit?) to help
> lookup the translation when french is selected.
> 
> DavidNind mentioned in IRC:
> translations are done at translate.koha-community.org, and will picked up at
> release time
> 16:11 < davidnind[m]>
> https://wiki.koha-community.org/wiki/
> Release_maintenance#Push_updated_translation_files

This patch does not remove the popup, it moves it from koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc to koha-tmpl/intranet-tmpl/prog/js/members-menu.js so it can be translated using translations written in misc/translator/po/fr-CA-messages-js.po
Comment 21 David Nind 2022-07-08 22:55:49 UTC
With the latest patch: when you go to delete the message, the popup doesn't appear (no message and buttons) and the message is deleted.
Comment 22 Shi Yao Wang 2022-07-11 13:28:12 UTC
It still works well for me...

Not sure where the problem comes from. I tested with chrome and firefox.
I tried blocking prompts on firefox and the popup does not appear, but the message is not deleted.

Any ideas?
Comment 23 David Nind 2022-07-12 00:35:30 UTC
(In reply to Shi Yao Wang from comment #22)
> It still works well for me...
> 
> Not sure where the problem comes from. I tested with chrome and firefox.
> I tried blocking prompts on firefox and the popup does not appear, but the
> message is not deleted.
> 
> Any ideas?

I retested, and it all works now... not sure what I did to make it not work!
Comment 24 David Nind 2022-07-12 00:41:12 UTC
Created attachment 137597 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-messages-js.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 Jonathan Druart 2022-08-01 09:17:05 UTC
Created attachment 138453 [details] [review]
Bug 30028: Fix patron message delete confirmation to be translatable

Make the message deletion confirmation popup translatable. Test plan
uses fr-CA translations.

Test plan:
1) Switch language to french (fr-CA)
If you do not have the translations, go to
  misc/translator
then do
  ./translate install fr-CA
After it is done, go to administration > global system preference >
l18N/L10N then select Français (fr-CA) under language preference.
Refresh then you should be able to switch languages.
2) Add a message to a patron account from your own branch (or make sure AllowAllMessageDeletion is on)
3) Click "Delete" next to the message
--> a confirmation message appears "Are you sure you want to delete this message? This cannot be undone." with options "Cancel" and "OK".
4) Apply the patch
5) Write a translation in misc/translator/po/fr-CA-messages-js.po :
msgid "Are you sure you want to delete this message? This cannot be undone."
msgstr "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible."
6) Refresh the translations by going to misc/translator and execute ./translate install fr-CA
7) Refresh and click "Delete" again
--> now the confirmation message is "Êtes-vous sûr de vouloir supprimer ce message? Cette opération est irréversible." with options "Cancel" and "OK"

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Tomás Cohen Arazi 2022-08-01 12:10:59 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 27 Lucas Gass 2022-09-29 15:18:11 UTC
Backported to 22.05.x for 22.05.06
Comment 28 Victor Grousset/tuxayo 2022-10-13 15:57:49 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 29 Arthur Suzuki 2022-10-22 22:07:52 UTC
Thanks!

Pushed to 21.11 for 21.11.12