Bug 30517 - Translation breaks editing parent type circulation rule
Summary: Translation breaks editing parent type circulation rule
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Shi Yao Wang
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 34269
  Show dependency treegraph
 
Reported: 2022-04-12 19:27 UTC by Caroline Cyr La Rose
Modified: 2023-07-13 12:57 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.08, 21.11.15


Attachments
Bug 30517: Translation breaks editing parent type circulation rule fix (1.98 KB, patch)
2022-04-15 21:20 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 30517: Translation breaks editing parent type circulation rule fix (2.57 KB, patch)
2022-04-19 15:55 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bug 30517: Translation breaks editing parent type circulation rule fix (2.69 KB, patch)
2022-08-24 15:23 UTC, Shi Yao Wang
Details | Diff | Splinter Review
Bottom line doesn't edit the chosen line (14.43 KB, image/png)
2022-09-02 09:07 UTC, Emmanuel Bétemps
Details
Bug 30517: Translation breaks editing parent type circulation rule fix (2.83 KB, patch)
2022-09-03 08:25 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 30517: Translation breaks editing parent type circulation rule fix (2.88 KB, patch)
2022-10-02 09:15 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 Caroline Cyr La Rose 2022-04-12 19:27:24 UTC
If using Koha in a language other than English, and using parent item types, you can't easily edit a rule for the parent type. When you click on "Edit" next to the rule, the line at the bottom keeps "All" as the item type and if you're not careful, it doesn't modify the parent type rule, but the All rule.

I can't explain it well, but here's how to replicate this problem:

1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type (I created a 'Children's books' type and assigned 'Books' as its parent)
3. Create a circulation rule for the parent type (I created All/Books, with 10 checkouts allowed)
4. Create a circulation rule for All/All (I created All/All with 30 checkouts allowed)
5. In English, click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
--> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row stays on 'All'
9. Modify the number of checkouts allowed (e.g. 88)
--> The All/All rule is modified

I hope my steps are clear...

I tried this in fr-CA and de-DE, and the behaviour is the same.
Comment 1 Caroline Cyr La Rose 2022-04-12 19:31:22 UTC
This is the string that is causing the problem

#. %1$s:  itemtypeloo.translated_description | html 
#: koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:374
#, c-format
msgid "%s (All)"
msgstr "%s (Toutes)"

If I add fuzzy to the last commented line and reapply the translation, the problem goes away.

#. %1$s:  itemtypeloo.translated_description | html 
#: koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt:374
#, fuzzy,c-format
msgid "%s (All)"
msgstr "%s (Toutes)"
Comment 2 Shi Yao Wang 2022-04-15 21:20:49 UTC
Created attachment 133364 [details] [review]
Bug 30517: Translation breaks editing parent type circulation rule fix

Test plan:
1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type (I created a 'Children's books' type and assigned 'Books' as its parent)
3. Create a circulation rule for the parent type (I created All/Books, with 10 checkouts allowed)
4. Create a circulation rule for All/All (I created All/All with 30 checkouts allowed)
5. In English, click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
--> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row stays on 'All'
9. Modify the number of checkouts allowed (e.g. 88)
--> The All/All rule is modified
10. Apply the patch and refresh
11. Do step 8-9 again and notice it now behaves as it should
Comment 3 Shi Yao Wang 2022-04-19 15:55:44 UTC
Created attachment 133404 [details] [review]
Bug 30517: Translation breaks editing parent type circulation rule fix

Test plan:
1. Install another language in the staff interface
   1. ./translate install xx-XX
   2. Check the box of the language in the 'language' system preference
   3. Refresh
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type (I created a 'Children's books' type and assigned 'Books' as its parent)
3. Create a circulation rule for the parent type (I created All/Books, with 10 checkouts allowed)
4. Create a circulation rule for All/All (I created All/All with 30 checkouts allowed)
5. In English, click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
--> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row stays on 'All'
9. Modify the number of checkouts allowed (e.g. 88)
--> The All/All rule is modified
10. Apply the patch and refresh
11. Do step 8-9 again and notice it now behaves as it should
Comment 4 Katrin Fischer 2022-06-17 23:23:12 UTC
Hi, should this be Needs Signoff?
Comment 5 Blou 2022-08-23 15:59:10 UTC
no interest?
Comment 6 Emmanuel Bétemps 2022-08-24 09:56:49 UTC
Hi,

I just testried on a sandbox and it doesn't work

step 11 displays the same results as in steps 8 & 9 : All/All rule is modified when I edit the All/Books rule
Comment 7 Katrin Fischer 2022-08-24 11:08:48 UTC
Did you install the translations before or after the patch was applied?
Comment 8 Emmanuel Bétemps 2022-08-24 12:34:03 UTC
(In reply to Katrin Fischer from comment #7)
> Did you install the translations before or after the patch was applied?


I just followed the test plan : translation (french) installed before applying the patch.


I can try applying the patch before installing the language.
Comment 9 Emmanuel Bétemps 2022-08-24 14:57:51 UTC
I tested applying the patch and, after, installing the language.

Patch works better but some things bother me


1.I click on "Circulation and fines rules"
2.I click on the Edit button to modify "All / Books"
3. I save my changes
-> I really modify "All / Books"
-> the bottom line (i.e the one where we write our changes) resets

But sometimes, if I don't want to save my changes, I click on another line to edit (Koha asks me then if I am sure) but it doesn't change the bottom line.
Comment 10 Shi Yao Wang 2022-08-24 15:23:37 UTC
Created attachment 139718 [details] [review]
Bug 30517: Translation breaks editing parent type circulation rule fix

Test plan:
1. Install another language in the staff interface
   1. in commandline: `cd misc/translator/; ./translate install xx-XX`
   2. Check the box of the language in the 'language' system preference
and save
   3. Refresh and you should be able to choose languages
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type (I created a 'Children's books' type and assigned 'Books' as its parent)
3. Create a circulation rule for the parent type (I created All/Books, with 10 checkouts allowed)
4. Create a circulation rule for All/All (I created All/All with 30 checkouts allowed)
5. In English, click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
--> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row stays on 'All'
9. Modify the number of checkouts allowed (e.g. 88)
--> The All/All rule is modified
10. Apply the patch, translate again and refresh the page
11. Do step 8-9 again and notice it now behaves as it should
Comment 11 Shi Yao Wang 2022-08-24 15:35:11 UTC
Updated the test plan to make it a bit more clear.

(In reply to Emmanuel Bétemps from comment #9)
> I tested applying the patch and, after, installing the language.
> 
> Patch works better but some things bother me
> 
> 
> 1.I click on "Circulation and fines rules"
> 2.I click on the Edit button to modify "All / Books"
> 3. I save my changes
> -> I really modify "All / Books"
> -> the bottom line (i.e the one where we write our changes) resets
> 
> But sometimes, if I don't want to save my changes, I click on another line
> to edit (Koha asks me then if I am sure) but it doesn't change the bottom
> line.

I do not seem to have this problem.

What I did after the test plan:
1- Go to Administration -> Circulation and fines rules
2- Click edit button for "All / Books"
3- Modify "Checkouts allowed" to a different number
4- Click on edit button for "All / All" without saving
5- Click ok on the confirmation popup
6- Bottom line's information change to "All / All" circulation rule values
Comment 12 Katrin Fischer 2022-08-24 15:56:26 UTC
(In reply to Emmanuel Bétemps from comment #9)
> I tested applying the patch and, after, installing the language.
> 
> Patch works better but some things bother me
> 
> 
> 1.I click on "Circulation and fines rules"
> 2.I click on the Edit button to modify "All / Books"
> 3. I save my changes
> -> I really modify "All / Books"
> -> the bottom line (i.e the one where we write our changes) resets
> 
> But sometimes, if I don't want to save my changes, I click on another line
> to edit (Koha asks me then if I am sure) but it doesn't change the bottom
> line.

When you install a language it copies the templates and files. So if you install the patch first, you'll look at unchanged/old templates that don't have the patch.
You probably also need to make sure that "all" has been translated to something else in that language.
Comment 13 Katrin Fischer 2022-08-24 15:58:31 UTC
> 
> When you install a language it copies the templates and files. So if you
> install the patch first, you'll look at unchanged/old templates that don't
> have the patch.
> You probably also need to make sure that "all" has been translated to
> something else in that language.

That should have read...: so if you install the language first... because installing the patch will then only update the English files and not trigger updating the translated files as well.
Comment 14 Emmanuel Bétemps 2022-09-02 09:06:08 UTC
Hi,

Patch doesn't apply (I followed the test plan in comment #10)
Screenshot confirms

I (really) just need to know if I must install language or patch first :)
Comment 15 Emmanuel Bétemps 2022-09-02 09:07:13 UTC
Created attachment 140091 [details]
Bottom line doesn't edit the chosen line
Comment 16 Shi Yao Wang 2022-09-02 13:00:03 UTC
(In reply to Emmanuel Bétemps from comment #14)
> Hi,
> 
> Patch doesn't apply (I followed the test plan in comment #10)
> Screenshot confirms
> 
> I (really) just need to know if I must install language or patch first :)

Hi, thanks for testing again.

TLDR: `./translate install xx-XX` before and after the patch

You need to install the language at step 1:
>1. Install another language in the staff interface
>   1. in commandline: `cd misc/translator/; ./translate install xx-XX`
>   2. Check the box of the language in the 'language' system preference
>and save
>   3. Refresh and you should be able to choose languages

apply the patch at step 10:
>10. Apply the patch, translate again and refresh the page

and do step 1.1 again to refresh translations (that's what I meant by "translate again" here).
Comment 17 ByWater Sandboxes 2022-09-03 08:25:39 UTC
Created attachment 140193 [details] [review]
Bug 30517: Translation breaks editing parent type circulation rule fix

Test plan:
1. Install another language in the staff interface
   1. in commandline: `cd misc/translator/; ./translate install xx-XX`
   2. Check the box of the language in the 'language' system preference
and save
   3. Refresh and you should be able to choose languages
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type (I created a 'Children's books' type and assigned 'Books' as its parent)
3. Create a circulation rule for the parent type (I created All/Books, with 10 checkouts allowed)
4. Create a circulation rule for All/All (I created All/All with 30 checkouts allowed)
5. In English, click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
--> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row stays on 'All'
9. Modify the number of checkouts allowed (e.g. 88)
--> The All/All rule is modified
10. Apply the patch, translate again and refresh the page
11. Do step 8-9 again and notice it now behaves as it should

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>
Comment 18 Emmanuel Bétemps 2022-09-03 08:30:59 UTC
(In reply to Shi Yao Wang from comment #16)

> apply the patch at step 10:
> >10. Apply the patch, translate again and refresh the page
> 
> and do step 1.1 again to refresh translations (that's what I meant by
> "translate again" here).

my bad !
I didn't understand I had to install translation twice (once before and once after installing the patch)

'translate again' meant to me 'change the language of the interface'

thanks for your patience & explanations :)


Emmanuel Bétemps
Comment 19 Katrin Fischer 2022-10-02 09:15:10 UTC
Created attachment 141202 [details] [review]
Bug 30517: Translation breaks editing parent type circulation rule fix

Test plan:
1. Install another language in the staff interface
   1. in commandline: `cd misc/translator/; ./translate install xx-XX`
   2. Check the box of the language in the 'language' system preference
and save
   3. Refresh and you should be able to choose languages
2. Create an item type with a parent
   1. Go to Administration > Item types
   2. Create a new item type or modify an existing one, assigning a parent type (I created a 'Children's books' type and assigned 'Books' as its parent)
3. Create a circulation rule for the parent type (I created All/Books, with 10 checkouts allowed)
4. Create a circulation rule for All/All (I created All/All with 30 checkouts allowed)
5. In English, click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row (the modifiable row) is changed to 'Books (All)'
6. Modify the number of checkouts allowed (e.g. 99)
--> The All/Books rule is modified
7. Switch the interface to the other language
8. Click on "Edit" next to the parent type rule (All/Books)
--> Note that the item type in the bottom row stays on 'All'
9. Modify the number of checkouts allowed (e.g. 88)
--> The All/All rule is modified
10. Apply the patch, translate again and refresh the page
11. Do step 8-9 again and notice it now behaves as it should

Signed-off-by: Emmanuel Bétemps <e.betemps@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 20 Katrin Fischer 2022-10-02 09:18:28 UTC
Overall I am not super happy about the way we rely on a regex here, but have not managed to break behavior in my tests. So this fix works and that's the most important as this bug has been there for too long causing potential data loss.
Comment 21 Tomás Cohen Arazi 2022-10-03 15:42:06 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 22 Lucas Gass 2022-12-05 16:10:25 UTC
Backported to 22.05.x for upcoming 22.05.08 release
Comment 23 Arthur Suzuki 2022-12-14 11:44:54 UTC
applied to 21.11 for 21.11.15
Comment 24 wainuiwitikapark 2022-12-20 01:42:04 UTC
Backported to 21.05.x for 21.05.22
Comment 25 wainuiwitikapark 2022-12-23 00:22:57 UTC
Not released in 21.05.22
Comment 26 wainuiwitikapark 2023-03-15 00:43:25 UTC
(In reply to wainuiwitikapark from comment #25)
> Not released in 21.05.22

Reverted from 21.05 branch