Bug 21953

Summary: Term "Lost item" is untranslatable
Product: Koha Reporter: Caroline Cyr La Rose <caroline.cyr-la-rose>
Component: PatronsAssignee: Caroline Cyr La Rose <caroline.cyr-la-rose>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: gmcharlt, hayleypelham, jonathan.druart, katrin.fischer, kyle.m.hall, marjorie.barry-vila, martin.renvoize, nick, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17413
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12166
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.05.00
Bug Depends on: 22628    
Bug Blocks: 22521    
Attachments: pay.pl in French
Bug 21953 - Term 'Lost item' untranslatable
Bug 21953 - Term 'Lost item' untranslatable
Bug 21953: Term 'Lost item' untranslatable
Bug 21953: Add tests
Bug 21953: Update existing rows in DB
Bug 21953: Add tests
Bug 21953: Update existing rows in DB
Bug 21953: Update existing rows in DB
Bug 21953: Term 'Lost item' untranslatable
Bug 21953: Add tests
Bug 21953: Update existing rows in DB
Bug 21953: (QA follow-up) Corrections to DB Update
Bug 21953: Clarify PF = Lost item processing fee
Bug 21953: Term 'Lost item' untranslatable
Bug 21953: Add tests
Bug 21953: Update existing rows in DB
Bug 21953: (QA follow-up) Corrections to DB Update
Bug 21953: Clarify PF = Lost item processing fee
Bug 21953: Term 'Lost item' untranslatable
Bug 21953: Add tests
Bug 21953: Update existing rows in DB
Bug 21953: (QA follow-up) Corrections to DB Update
Bug 21953: Clarify PF = Lost item processing fee
Bug 21953: Term 'Lost item' untranslatable
Bug 21953: Add tests
Bug 21953: Update existing rows in DB
Bug 21953: (QA follow-up) Corrections to DB Update
Bug 21953: Clarify PF = Lost item processing fee

Description Caroline Cyr La Rose 2018-12-05 14:54:31 UTC
Created attachment 82872 [details]
pay.pl in French

When I mark an item as lost and the fee is charged to the user, the description contains the translated string for "Lost item", followed by "Lost item" in English, followed by the title, the barcode and the title again (?) in parentheses.

In English, "Lost item" appears twice: "Lost item, Lost item title barcode (title)".

Could it be possible to remove the untranslatable "Lost item" part?

To reproduce:
1) Make sure the system preference WhenLostChargeReplacementFee is set to "Charge"
2) Optional: In Administration > Item types, add a default replacement cost to your item types or the item type you plan to use
3) Loan an item to a patron
4) If there is no default replacement cost, make sure your item has a replacement price
5) In the patron's account > Details > Loans, click on the item's barcode
6) Set the Lost status to Lost
7) Go back in the patron's account > Fines
8) Notice it is written "Lost item, Lost item title barcode (title)"
9) If you have another language installed, switch to the other language
10) Notice it is written "Translated string, Lost item title barcode (title)"
11) Click "Pay"
12) Notice it is written "Lost item title barcode"
13) If you switch to your other language, it is still written in English
14) Click on "Account"
15) Notice the description is again "Lost item, Lost item title barcode (title)"
Comment 1 Caroline Cyr La Rose 2018-12-20 22:13:12 UTC
Created attachment 83433 [details] [review]
Bug 21953 - Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string
Comment 2 Katrin Fischer 2018-12-20 23:34:57 UTC
Created attachment 83439 [details] [review]
Bug 21953 - Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 3 Katrin Fischer 2018-12-20 23:35:52 UTC
2 things:

- Joubu already pointed out on IRC that there should be a test (change in C4)
- Should we update existing data and remove the String there to have a clean state? It's mostly a display issue.
Comment 4 Tomás Cohen Arazi 2019-01-15 18:47:07 UTC
(In reply to Katrin Fischer from comment #3)
> 2 things:
> 
> - Joubu already pointed out on IRC that there should be a test (change in C4)
> - Should we update existing data and remove the String there to have a clean
> state? It's mostly a display issue.

In my opinion, this has been wrong from the begining: the information should be built for display using the account_type and the biblio information, but instead we hardcoded those values.

Once Koha::Account::add_debit and friends is pushed, I'd review this one.
Comment 5 Caroline Cyr La Rose 2019-01-17 17:29:41 UTC
1. I don't know how to do tests, I'll know in the future not to touch anything in C4 even if it's a simple string patch. I feel it's a bit overkill to do tests just for removing two displayed words. But if someone wants to code tests, it's fine with me. I will de-assign myself the bug since I can't do more than submit the string patch.

2. I'm not sure how I would go about to update the existing data. I'm guessing it's an UPDATE using regex. 

3. From Tomás' comment, I understand that this bug is put on the back burner. Should we change it to "in discussion"?
Comment 6 Tomás Cohen Arazi 2019-01-17 17:32:49 UTC
(In reply to Caroline Cyr La Rose from comment #5)
> 1. I don't know how to do tests, I'll know in the future not to touch
> anything in C4 even if it's a simple string patch. I feel it's a bit
> overkill to do tests just for removing two displayed words. But if someone
> wants to code tests, it's fine with me. I will de-assign myself the bug
> since I can't do more than submit the string patch.
> 
> 2. I'm not sure how I would go about to update the existing data. I'm
> guessing it's an UPDATE using regex. 
> 
> 3. From Tomás' comment, I understand that this bug is put on the back
> burner. Should we change it to "in discussion"?

I will try to take a look at the add_debit bugs again and see how it fits.
Comment 7 Jonathan Druart 2019-03-06 19:09:58 UTC
Created attachment 86209 [details] [review]
Bug 21953: Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Jonathan Druart 2019-03-06 19:10:03 UTC
Created attachment 86210 [details] [review]
Bug 21953: Add tests
Comment 9 Jonathan Druart 2019-03-06 19:10:08 UTC
Created attachment 86211 [details] [review]
Bug 21953: Update existing rows in DB
Comment 10 Hayley Pelham 2019-03-08 00:43:57 UTC
Patch doesn't apply.

Applying: Bug 21953: Term 'Lost item' untranslatable
Applying: Bug 21953: Add tests
Using index info to reconstruct a base tree...
M	t/db_dependent/Circulation.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Circulation.t
CONFLICT (content): Merge conflict in t/db_dependent/Circulation.t
error: Failed to merge in the changes.
Patch failed at 0001 Bug 21953: Add tests
Comment 11 Jonathan Druart 2019-03-08 21:55:46 UTC
Created attachment 86405 [details] [review]
Bug 21953: Add tests
Comment 12 Jonathan Druart 2019-03-08 21:55:50 UTC
Created attachment 86406 [details] [review]
Bug 21953: Update existing rows in DB
Comment 13 Hayley Pelham 2019-03-10 21:06:57 UTC
Created attachment 86418 [details] [review]
Bug 21953: Update existing rows in DB

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Test plan passes.
Comment 14 Martin Renvoize 2019-03-12 14:14:45 UTC
Sorry Caroline but...

There's a weird case here where a Lost item could receive two types of charge depending on what system preferences were in play. One with the code 'PF' for "Processing Fee" and another with the code "L" for Lost Item..  The DB update needs to catch both before I can PQA the bug..

Also, do we need to somehow distinguish that the Processing Fee line is attributed to the Lost item?  With this patch we're losing that I think.
Comment 15 Martin Renvoize 2019-04-09 09:26:38 UTC
*** Bug 11573 has been marked as a duplicate of this bug. ***
Comment 16 Martin Renvoize 2019-04-09 11:06:40 UTC
Created attachment 87593 [details] [review]
Bug 21953: Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2019-04-09 11:06:43 UTC
Created attachment 87594 [details] [review]
Bug 21953: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2019-04-09 11:06:47 UTC
Created attachment 87595 [details] [review]
Bug 21953: Update existing rows in DB

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Test plan passes.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2019-04-09 11:06:51 UTC
Created attachment 87596 [details] [review]
Bug 21953: (QA follow-up) Corrections to DB Update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2019-04-09 11:06:54 UTC
Created attachment 87597 [details] [review]
Bug 21953: Clarify PF = Lost item processing fee

Currently PF is only ever used for lost item processing fees. This patch
alters the end user faceing description to clarify that.

Note: It may be nice to make the code more descriptive too at some
point, but that can happen in another bug in my opinion.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize 2019-04-09 11:08:32 UTC
OK, I've added the required followups myself... back to SO in the hopes I can get a quick QA from someone else on the team.
Comment 22 Martin Renvoize 2019-04-09 12:11:03 UTC
Created attachment 87599 [details] [review]
Bug 21953: Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Martin Renvoize 2019-04-09 12:11:07 UTC
Created attachment 87600 [details] [review]
Bug 21953: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2019-04-09 12:11:10 UTC
Created attachment 87601 [details] [review]
Bug 21953: Update existing rows in DB

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Test plan passes.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2019-04-09 12:11:14 UTC
Created attachment 87602 [details] [review]
Bug 21953: (QA follow-up) Corrections to DB Update

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2019-04-09 12:11:17 UTC
Created attachment 87603 [details] [review]
Bug 21953: Clarify PF = Lost item processing fee

Currently PF is only ever used for lost item processing fees. This patch
alters the end user faceing description to clarify that.

Note: It may be nice to make the code more descriptive too at some
point, but that can happen in another bug in my opinion.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Katrin Fischer 2019-04-09 18:33:34 UTC
Created attachment 87659 [details] [review]
Bug 21953: Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2019-04-09 18:33:39 UTC
Created attachment 87660 [details] [review]
Bug 21953: Add tests

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 29 Katrin Fischer 2019-04-09 18:33:43 UTC
Created attachment 87661 [details] [review]
Bug 21953: Update existing rows in DB

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Test plan passes.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 30 Katrin Fischer 2019-04-09 18:33:47 UTC
Created attachment 87662 [details] [review]
Bug 21953: (QA follow-up) Corrections to DB Update

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 31 Katrin Fischer 2019-04-09 18:33:51 UTC
Created attachment 87663 [details] [review]
Bug 21953: Clarify PF = Lost item processing fee

Currently PF is only ever used for lost item processing fees. This patch
alters the end user faceing description to clarify that.

Note: It may be nice to make the code more descriptive too at some
point, but that can happen in another bug in my opinion.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2019-04-09 18:34:01 UTC
Thx Caroline and Martin!
Comment 33 Nick Clemens 2019-04-11 13:46:16 UTC
please rebase on top of 12166
Comment 34 Martin Renvoize 2019-04-11 18:16:41 UTC
Created attachment 87782 [details] [review]
Bug 21953: Term 'Lost item' untranslatable

This patch removes the hardcoded untranslatable string
'Lost item' from C4/Circulation.pm.

To test:
1) Make sure the system preference WhenLostChargeReplacementFee is set to Charge
2) Optional: in Administration > Item types, add a default replacement cost to
the item type you plan to use
3) Loan an item out to a patron
(If there is no default replacement cost, make sure the item has a replacement
price)
4) In the patron's account > Details > Loans, click on the item's barcode
5) Set the lost status to Lost
6) Go back to the patron's account > Fines
7) Notice it is written 'Lost item , Lost item title barcode (title)'
8) If you have another language installed, switch to the other language
and notice the second Lost item is still in English
9) Apply the patch
10) Redo steps 3-6
11) Notice it is written 'Lost item, title barcode (title)'
12) Optional: switch to another language, notice there is no English string

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Martin Renvoize 2019-04-11 18:16:45 UTC
Created attachment 87783 [details] [review]
Bug 21953: Add tests

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Martin Renvoize 2019-04-11 18:16:49 UTC
Created attachment 87784 [details] [review]
Bug 21953: Update existing rows in DB

Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz>
Test plan passes.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 37 Martin Renvoize 2019-04-11 18:16:53 UTC
Created attachment 87785 [details] [review]
Bug 21953: (QA follow-up) Corrections to DB Update

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 38 Martin Renvoize 2019-04-11 18:16:56 UTC
Created attachment 87786 [details] [review]
Bug 21953: Clarify PF = Lost item processing fee

Currently PF is only ever used for lost item processing fees. This patch
alters the end user faceing description to clarify that.

Note: It may be nice to make the code more descriptive too at some
point, but that can happen in another bug in my opinion.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 39 Martin Renvoize 2019-04-11 18:17:19 UTC
Rebased as requested.
Comment 40 Nick Clemens 2019-04-12 11:50:13 UTC
Awesome work all!

Pushed to master for 19.05
Comment 41 Martin Renvoize 2019-04-25 13:12:13 UTC
Long standing bug depends on bug 22628 not in 18.11.x series.. will not be backported.