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)"
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
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>
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 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.
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"?
(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.
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>
Created attachment 86210 [details] [review] Bug 21953: Add tests
Created attachment 86211 [details] [review] Bug 21953: Update existing rows in DB
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
Created attachment 86405 [details] [review] Bug 21953: Add tests
Created attachment 86406 [details] [review] Bug 21953: Update existing rows in DB
Created attachment 86418 [details] [review] Bug 21953: Update existing rows in DB Signed-off-by: Hayley Mapley <hayleymapley@catalyst.net.nz> Test plan passes.
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.
*** Bug 11573 has been marked as a duplicate of this bug. ***
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>
Created attachment 87594 [details] [review] Bug 21953: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Created attachment 87596 [details] [review] Bug 21953: (QA follow-up) Corrections to DB Update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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.
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>
Created attachment 87600 [details] [review] Bug 21953: Add tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Created attachment 87602 [details] [review] Bug 21953: (QA follow-up) Corrections to DB Update Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
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>
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>
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>
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>
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>
Thx Caroline and Martin!
please rebase on top of 12166
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>
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>
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>
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>
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>
Rebased as requested.
Awesome work all! Pushed to master for 19.05
Long standing bug depends on bug 22628 not in 18.11.x series.. will not be backported.