Bug 26403 - Move debit and credit types to YAML files and fix other related translation issues
Summary: Move debit and credit types to YAML files and fix other related translation i...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on: 26418
Blocks: 14825 33935
  Show dependency treegraph
 
Reported: 2020-09-07 21:41 UTC by Katrin Fischer
Modified: 2023-12-28 20:47 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
With this patch the descriptions of system internal credit and debit types will be translated into the selected language at installation time. This will only affect new installations and SQL reporting. If you are building your own SQL reports, you'll be able to pull the descriptions from the tables. It also makes sure, that all system internal debit and credit types appear translated in the GUI. This now also includes the administration pages for managing credit and debit types. Some descriptions for discount, payout, purchase, and void were missing. These have now also been added.
Version(s) released in:
23.05.00,22.11.06


Attachments
Account summary showing English descriptions (77.60 KB, image/png)
2020-09-09 13:57 UTC, Katrin Fischer
Details
YAML file for credit types (2.23 KB, application/x-yaml)
2023-03-29 21:23 UTC, Caroline Cyr La Rose
Details
Bug 26403: "Issue refund" is not fully translatable (8.75 KB, patch)
2023-04-03 19:38 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 26403: (follow-up) Deleted old SQL files (2.66 KB, patch)
2023-04-03 19:38 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 26403: "Issue refund" is not fully translatable (8.80 KB, patch)
2023-05-05 12:19 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 26403: (follow-up) Deleted old SQL files (2.76 KB, patch)
2023-05-05 12:19 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 26403: Move credit and debit types into translatable YAML files (8.87 KB, patch)
2023-05-09 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26403: (follow-up) Deleted old SQL files (2.81 KB, patch)
2023-05-09 21:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26403: Use accounts include in account management pages (5.19 KB, patch)
2023-05-10 06:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26403: Use accounts include in account management pages (5.45 KB, patch)
2023-05-10 06:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26403: Move credit and debit types into translatable YAML files (8.93 KB, patch)
2023-05-10 08:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26403: (follow-up) Deleted old SQL files (2.87 KB, patch)
2023-05-10 08:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26403: Use accounts include in account management pages (5.50 KB, patch)
2023-05-10 08:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26403: (QA follow-up) Add missing debit and credit types to includes (2.63 KB, patch)
2023-05-10 08:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26403: (QA follow-up) Add missing debit and credit types to includes (4.69 KB, patch)
2023-05-10 08:51 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2020-09-07 21:41:18 UTC
The new "issue refund" that is available for payments is not fully translatable. This is a big issue.

To test:
- Make sure a language with a complete translation is installed
- Switch to the language
- Go to any user account
- Add a manual invoice
- Pay it off fully or partially
- Click on "issue refund"
- Verify the list of "Transaction types" is not translated
- Check the information in the patron account. It will show:

Payment from library to patron
A refund applied to a patrons fine

- Check in the OPAC - same issue there.

There is no option to edit the hardcoded credit and debit type descriptions?
Comment 1 Martin Renvoize 2020-09-09 12:37:46 UTC
(In reply to Katrin Fischer from comment #0)
> The new "issue refund" that is available for payments is not fully
> translatable. This is a big issue.
> 
> To test:
> - Make sure a language with a complete translation is installed
> - Switch to the language
> - Go to any user account
> - Add a manual invoice
> - Pay it off fully or partially
> - Click on "issue refund"
> - Verify the list of "Transaction types" is not translated
> - Check the information in the patron account. It will show:
> 
> Payment from library to patron
> A refund applied to a patrons fine
> 
> - Check in the OPAC - same issue there.
> 
> There is no option to edit the hardcoded credit and debit type descriptions?

The reason one cannot edit the descriptions for these hardcoded types is precisely for translation reasons.  If it's a system code it should appear in the translation template and thus the description in the DB will never get called.
Comment 2 Martin Renvoize 2020-09-09 12:50:57 UTC
These are not credit types, but rather the 'payment_type' authorized values... as such to be translatable we need translatable AV's first.... or to merge payment_types into credit_types.. but I'm still not entirely sure about that one.
Comment 3 Katrin Fischer 2020-09-09 13:52:35 UTC
(In reply to Martin Renvoize from comment #2)
> These are not credit types, but rather the 'payment_type' authorized
> values... as such to be translatable we need translatable AV's first.... or
> to merge payment_types into credit_types.. but I'm still not entirely sure
> about that one.

Hm, it was labelled credit types - so maybe the label is wrong then?

But what you are saying is good for the descriptions in the "history" - we "just" need to add them to the templates.
Comment 4 Katrin Fischer 2020-09-09 13:53:48 UTC
I am not sure why this is BLOCKED? We can at least fix a part of this now and we should - translations for authorised values doesn't apply to all points here and it's not around the corner.
Comment 5 Katrin Fischer 2020-09-09 13:57:29 UTC
Created attachment 109816 [details]
Account summary showing English descriptions
Comment 6 Martin Renvoize 2020-09-09 14:23:55 UTC
I've split this bug in two.. one for the description which is fixable and leaving this one here for the transaction type which is blocked by AV translations.
Comment 7 Caroline Cyr La Rose 2023-03-29 21:04:13 UTC
Is this the bug for account_credit_types and account_debit_types being installed in English only?

I have to go into every one of my installations and manually change the database descriptions... My problem is specifically when installing in another language, not switching between languages.

My hypothesis is that those values are in installer/data/mysql/mandatory rather than installer/data/mysql/en/mandatory

If they were in /en/, they would show up in the translation files. Does that make sense? Could we consider moving them?
Comment 8 Caroline Cyr La Rose 2023-03-29 21:23:39 UTC
Created attachment 148940 [details]
YAML file for credit types

I tried to transfer the credit types into a yaml file to put into the /en/ directory. Didn't have time to do the rest (CASH payment type, which is also untranslated, and account debit types, and remove the sql files).

Let me know if this is OK and I'll be able to continue another day
Comment 9 Caroline Cyr La Rose 2023-04-03 19:38:10 UTC Comment hidden (obsolete)
Comment 10 Caroline Cyr La Rose 2023-04-03 19:38:12 UTC Comment hidden (obsolete)
Comment 11 Magnus Enger 2023-05-05 12:04:23 UTC
Trying to apply the patches, I get this: 

$ git bz apply 26403
Bug 26403 Depends on bug 20307 (BLOCKED)

As far as I can tell "Bug 20307 - Language overlay for authorized values" is for getting translateable authorised values in the interface, which would be another solution to this problem? Or is there a connection between these two bugs that I am not seeing?
Comment 12 Magnus Enger 2023-05-05 12:19:21 UTC
Created attachment 150744 [details] [review]
Bug 26403: "Issue refund" is not fully translatable

This patch moves account debit and credit types, as well as the CASH
payment type, into YAML files under
installer/data/en/mandatory. This should make the debit and credit types
translatable (for new installations only).

To test:
1. Apply patch
2. Update po files
   ./misc/translator/translate update xx-XX
3. View the xx-XX-installer.po file
   -->The default account debit and credit types (and cash payment type)
      should be available to be translated
4. Translate the strings
5. Apply the translations
   ./misc/translator/translate install xx-XX
6. Load the new translated YAML files (with a fresh install, or by deleting
   the values in the db and using ./misc/load_yaml.pl to load the files)
7. In the staff interface, go to Administration > Credit types
   --> They should be translated
8. Go to Administration > Debit types
   --> They should be translated

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 13 Magnus Enger 2023-05-05 12:19:24 UTC
Created attachment 150745 [details] [review]
Bug 26403: (follow-up) Deleted old SQL files

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan. Works as advertised.
Comment 14 Magnus Enger 2023-05-05 12:20:34 UTC
Worked nicely without 20307 for me, so I am taking the liberty of changing that to a "see also".
Comment 15 Katrin Fischer 2023-05-09 21:55:21 UTC
The templates use .inc files to make the system internal debit and credit types multi-lingual. So this will only affect reports or places where the description is pulled from the database directly (which we shouldn't do... but...)

My initial bug was about things missing from the include files, this might be still something to investigate, but repurposing this one now.
Comment 16 Katrin Fischer 2023-05-09 21:55:55 UTC
Created attachment 150912 [details] [review]
Bug 26403: Move credit and debit types into translatable YAML files

This patch moves account debit and credit types, as well as the CASH
payment type, into YAML files under
installer/data/en/mandatory. This should make the debit and credit types
translatable (for new installations only).

To test:
1. Apply patch
2. Update po files
   ./misc/translator/translate update xx-XX
3. View the xx-XX-installer.po file
   -->The default account debit and credit types (and cash payment type)
      should be available to be translated
4. Translate the strings
5. Apply the translations
   ./misc/translator/translate install xx-XX
6. Load the new translated YAML files (with a fresh install, or by deleting
   the values in the db and using ./misc/load_yaml.pl to load the files)
7. In the staff interface, go to Administration > Credit types
   --> They should be translated
8. Go to Administration > Debit types
   --> They should be translated

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

150744 - Bug 26403: "Issue refund" is not fully translatable
Comment 17 Katrin Fischer 2023-05-09 21:55:58 UTC
Created attachment 150913 [details] [review]
Bug 26403: (follow-up) Deleted old SQL files

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan. Works as advertised.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Martin Renvoize 2023-05-10 06:09:44 UTC
The first two patches look like duplicates to me and I don't understand how the first one fixes the issue refund issue initially reported.

Also, this encourages bad practice in so much as the DB level descriptions are not meant it be used.

Has someone broken the use of the include somewhere to make this work.. the DB fields should ONLY be getting used for reports.. nowhere else
Comment 19 Martin Renvoize 2023-05-10 06:21:01 UTC
Your just introducing double translation here. The "correct" patch would change the .description calls in the two tables to use the include as intended.  I have a feeling that got undone during QA of the original patchsets that added these management pages.
Comment 20 Martin Renvoize 2023-05-10 06:39:41 UTC
Created attachment 150925 [details] [review]
Bug 26403: Use accounts include in account management pages

This patch updates the credit and debit types management pages to use
the proper includes for handling system type translations.  This is how
it should have been done in the first place on the original bugs that
introduced the management of these types but for some reason I think it
got dropped during the QA process leading to confusion around these
system types with their fixed descriptions.
Comment 21 Martin Renvoize 2023-05-10 06:40:51 UTC
Please try the final patch as an alternative.

With this version you do not need to translate the strings again, they should already all be in appearance in the po files so we're no longer doubling up.
Comment 22 Martin Renvoize 2023-05-10 06:44:44 UTC Comment hidden (obsolete)
Comment 23 Martin Renvoize 2023-05-10 06:46:30 UTC
Created attachment 150932 [details] [review]
Bug 26403: Use accounts include in account management pages

This patch updates the credit and debit types management pages to use
the proper includes for handling system type translations.  This is how
it should have been done in the first place on the original bugs that
introduced the management of these types but for some reason I think it
got dropped during the QA process leading to confusion around these
system types with their fixed descriptions.

To test:
1. Apply patch
2. Apply the translations
   ./misc/translator/translate install xx-XX
3. In the staff interface, go to Administration > Credit types
   --> They should be translated
4. Go to Administration > Debit types
   --> They should be translated
Comment 24 Katrin Fischer 2023-05-10 07:52:47 UTC
I think we should do both. 

1) Web installer: It will help with SQL reporting where you can't use the includes. I have translated ours in the database for this reason and it has no bad side effects.

2) Template fixes: These are great, because they fix what I initially had reported.
Comment 25 Katrin Fischer 2023-05-10 07:55:10 UTC
Actually, I think we might need some more template fixes, but are on a good road here. 

Looking at my original test plan:

To test:
- Make sure a language with a complete translation is installed
- Switch to the language
- Go to any user account
- Add a manual invoice
- Pay it off fully or partially
- Click on "issue refund"
- Verify the list of "Transaction types" is not translated
- Check the information in the patron account. It will show:

I'll try to test this now.
Comment 26 Katrin Fischer 2023-05-10 08:27:38 UTC
(In reply to Katrin Fischer from comment #25)
> Actually, I think we might need some more template fixes, but are on a good
> road here. 
> 
> Looking at my original test plan:
> 
> To test:
> - Make sure a language with a complete translation is installed
> - Switch to the language
> - Go to any user account
> - Add a manual invoice
> - Pay it off fully or partially
> - Click on "issue refund"
> - Verify the list of "Transaction types" is not translated

Only Cash stands out there, the other entries are translated now. This will be fixed by Caroline's adding Cash to the web installer files, because this list is part hard coded and part coming from AV:

[% FOREACH pt IN payment_types %]
  <option value="[% pt.authorised_value | html %]">[% pt.lib | html %]</option>
[% END %]

> - Check the information in the patron account. It will show:

Some debit and credit types are missing from the includes. I'll provide a follow-up.
Comment 27 Katrin Fischer 2023-05-10 08:44:35 UTC
Created attachment 150941 [details] [review]
Bug 26403: Move credit and debit types into translatable YAML files

This patch moves account debit and credit types, as well as the CASH
payment type, into YAML files under
installer/data/en/mandatory. This should make the debit and credit types
translatable (for new installations only).

To test:
1. Apply patch
2. Update po files
   ./misc/translator/translate update xx-XX
3. View the xx-XX-installer.po file
   -->The default account debit and credit types (and cash payment type)
      should be available to be translated
4. Translate the strings
5. Apply the translations
   ./misc/translator/translate install xx-XX
6. Load the new translated YAML files (with a fresh install, or by deleting
   the values in the db and using ./misc/load_yaml.pl to load the files)
7. In the staff interface, go to Administration > Credit types
   --> They should be translated
8. Go to Administration > Debit types
   --> They should be translated

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 28 Katrin Fischer 2023-05-10 08:44:38 UTC
Created attachment 150942 [details] [review]
Bug 26403: (follow-up) Deleted old SQL files

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan. Works as advertised.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 29 Katrin Fischer 2023-05-10 08:44:40 UTC
Created attachment 150943 [details] [review]
Bug 26403: Use accounts include in account management pages

This patch updates the credit and debit types management pages to use
the proper includes for handling system type translations.  This is how
it should have been done in the first place on the original bugs that
introduced the management of these types but for some reason I think it
got dropped during the QA process leading to confusion around these
system types with their fixed descriptions.

To test:
1. Apply patch
2. Apply the translations
   ./misc/translator/translate install xx-XX
3. In the staff interface, go to Administration > Credit types
   --> They should be translated
4. Go to Administration > Debit types
   --> They should be translated

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 30 Katrin Fischer 2023-05-10 08:44:43 UTC
Created attachment 150944 [details] [review]
Bug 26403: (QA follow-up) Add missing debit and credit types to includes

This adds the missing debit and credit types to the includes, namely:
DISCOUNT, PURCHASE, PAYOUT, and VOID.

I sorted them as they appear in the GUI (alphabetically) so it's a little
easier to spot missing ones.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 31 Katrin Fischer 2023-05-10 08:51:00 UTC
Created attachment 150946 [details] [review]
Bug 26403: (QA follow-up) Add missing debit and credit types to includes

This adds the missing debit and credit types to the includes, namely:
DISCOUNT, PURCHASE, PAYOUT, and VOID.

I sorted them as they appear in the GUI (alphabetically) so it's a little
easier to spot missing ones.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 32 Tomás Cohen Arazi 2023-05-10 18:28:24 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 33 Martin Renvoize 2023-05-10 18:33:08 UTC
Nice one everyone, thanks for the further follow-up Katrin :)
Comment 34 Matt Blenkinsop 2023-05-15 12:27:18 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 35 Jonathan Druart 2023-06-07 09:13:44 UTC
Nobody tried the installer process? There are issues reported on the mailing list.

There are still occurrences of the sql files!
% git grep account_credit_types.sql
Comment 36 Jonathan Druart 2023-06-07 09:18:19 UTC
(In reply to Jonathan Druart from comment #35)
> Nobody tried the installer process? There are issues reported on the mailing
> list.
> 
> There are still occurrences of the sql files!
> % git grep account_credit_types.sql

See 33935.