Bug 24865 - Customize the accountlines description
Summary: Customize the accountlines description
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Kyle M Hall (khall)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 31170
  Show dependency treegraph
 
Reported: 2020-03-13 16:22 UTC by Lisette Scheer
Modified: 2024-11-07 16:08 UTC (History)
12 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/632
Text to go in the release notes:
This patch adds a new notice/slip: OVERDUE_FINE_DESC This can be used to customize the accountlines description for overdue fines that can access objects for the checkout, the item, and the borrower
Version(s) released in:
22.11.00
Circulation function:


Attachments
Bug 24865: Customize the Accountlines Description (5.01 KB, patch)
2022-02-17 14:27 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 24865: Customize the Accountlines Description (5.07 KB, patch)
2022-03-02 16:35 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 24865: Customize the Accountlines Description (5.13 KB, patch)
2022-04-15 09:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24865: Customize the Accountlines Description (5.19 KB, patch)
2022-04-19 11:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24865: (QA follow-up) Remove hardcoded notice name from protected_letters (2.25 KB, patch)
2022-04-19 11:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lisette Scheer 2020-03-13 16:22:53 UTC
It would be great if we could customize what information was added to the "Description of charges" field when a fine was made so data could be stored even when the item is deleted. 

For example if you would like to be able to store the home library code, title, and barcode even if the item was deleted, you could have it e entered automatically when the fine was created. 

This would be very helpful to us.  
Lisette
Comment 1 Katrin Fischer 2020-04-05 13:47:37 UTC
The information for deleted items is still in Koha - maybe we could solve this without changing the description. It could also avoid display/linking isses.
Comment 2 Katrin Fischer 2020-05-24 00:23:45 UTC
The main issue I have with the description field is translatability. In the past we often stored strings there that would always remain in English and could not be changed. Nowadays we usually pull the information about the items using the itemnumbers and other identifiers, which is much nicer. But I am still finding things like bug 25596. I am just a little worried this could bring back the old behavior. 

Maybe we could add additional columns for the information instead?
Comment 3 Maria 2020-05-25 13:36:22 UTC
I can make a solution where the item id that is removed is saved in a new column in accountlines. 

Addition of functionality in the staff fines view to see owning library etc also for removed items.
Comment 4 Kyle M Hall (khall) 2022-02-17 14:27:46 UTC
Created attachment 130749 [details] [review]
Bug 24865: Customize the Accountlines Description

It would be great if we could customize what information was added to the "Description of charges" field when a fine was made so data could be stored even when the item is deleted.

Test Plan:
1) Create an overdue checkout that will get a fine
2) Run fines.pl
3) Note the description for the fine
4) Delete the fine from the database
5) Apply this patch
6) Run updatedatabase.pl
7) Restart all the things!
8) Run fines.pl
9) Note the description of the fine is unchanged
10) Delete the fine again
11) Browse to Slips & Notices
12) Edit the new notice OVERDUE_FINE_DESC
    You will have access to the objects checkout, item, and borrower
13) Run fines.pl
14) Note your new description was used
Comment 5 Andrew Fuerste-Henry 2022-03-01 13:43:49 UTC
A very minor quibble, the current behavior is to give the due date including the time. The default notice in this patch calls the due date without the time. It should be changed to maintain current behavior.
Comment 6 Andrew Fuerste-Henry 2022-03-02 16:35:55 UTC
Created attachment 131286 [details] [review]
Bug 24865: Customize the Accountlines Description

It would be great if we could customize what information was added to the "Description of charges" field when a fine was made so data could be stored even when the item is deleted.

Test Plan:
1) Create an overdue checkout that will get a fine
2) Run fines.pl
3) Note the description for the fine
4) Delete the fine from the database
5) Apply this patch
6) Run updatedatabase.pl
7) Restart all the things!
8) Run fines.pl
9) Note the description of the fine is unchanged
10) Delete the fine again
11) Browse to Slips & Notices
12) Edit the new notice OVERDUE_FINE_DESC
    You will have access to the objects checkout, item, and borrower
13) Run fines.pl
14) Note your new description was used

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 7 Marcel de Rooy 2022-04-15 09:02:15 UTC
I have my doubts about whats going on here. Maybe I dont fully understand.

[1] Isnt it a very expensive way to go thru notices to concatenate two fields:
[% item.biblio.title %] [% checkout.date_due | $KohaDates %]

[2] In the description a reference is made to deleted items. If an item is deleted, there is no issue anymore too. I dont see how you create a better description here? Can you explain in which exact situation you reach the else block where you call the letter? (No accountline found, amount>0, item deleted?)

Note: commit message refers to item deletion, but test plan does not include it.

Changing status: need for feedback.
Comment 8 Marcel de Rooy 2022-04-15 09:16:37 UTC
Created attachment 133344 [details] [review]
Bug 24865: Customize the Accountlines Description

It would be great if we could customize what information was added to the "Description of charges" field when a fine was made so data could be stored even when the item is deleted.

Test Plan:
1) Create an overdue checkout that will get a fine
2) Run fines.pl
3) Note the description for the fine
4) Delete the fine from the database
5) Apply this patch
6) Run updatedatabase.pl
7) Restart all the things!
8) Run fines.pl
9) Note the description of the fine is unchanged
10) Delete the fine again
11) Browse to Slips & Notices
12) Edit the new notice OVERDUE_FINE_DESC
    You will have access to the objects checkout, item, and borrower
13) Run fines.pl
14) Note your new description was used

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 9 Nick Clemens (kidclamp) 2022-04-15 12:21:53 UTC
1 - The idea though is adding the ability to customize which fields are being concatenated there, to allow librarians to add their own values in an understandable format. Additionally, we are now adding the ability to add translation here by getting patron language. I think it is worth the cost for new functionality

2 - The code addresses the creation of new overdue accountlines - the code is hit whenever a new overdue amount is generated. The note about deleted refers to the fact the info like 'Barcode' is calculated on display from information in the accountlines - when an item is deleted that link is broken and that info is not displayed. If [% item.barcode %] is added to the description it will be retained after deletion
Comment 10 Martin Renvoize (ashimema) 2022-04-19 10:48:53 UTC
Hmm, this is clever and I generally like it.

It feels like a bit of a hack being in letters/notices though, which is a bit of a shame, but I can appreciate why you added it there.

I wonder if this might helpful elsewhere too and a future work might split it out of notices and into a simplified interface a little more like the News and HTML Content stuff.. i.e. without the superfluous transport types handling.

We almost need a base class for the template handling and then more specific classes for the use cases like letters/notices vs blocks/pages.
Comment 11 Kyle M Hall (khall) 2022-04-19 10:58:25 UTC
(In reply to Martin Renvoize from comment #10)
> Hmm, this is clever and I generally like it.
> 
> It feels like a bit of a hack being in letters/notices though, which is a
> bit of a shame, but I can appreciate why you added it there.
> 
> I wonder if this might helpful elsewhere too and a future work might split
> it out of notices and into a simplified interface a little more like the
> News and HTML Content stuff.. i.e. without the superfluous transport types
> handling.
> 
> We almost need a base class for the template handling and then more specific
> classes for the use cases like letters/notices vs blocks/pages.

FWIW Bug 27334 adds template toolkit support to news. If that makes it in we could move this template from slips to there.
Comment 12 Marcel de Rooy 2022-04-19 11:38:43 UTC
(In reply to Nick Clemens from comment #9)
> 1 - The idea though is adding the ability to customize which fields are
> being concatenated there, to allow librarians to add their own values in an
> understandable format. Additionally, we are now adding the ability to add
> translation here by getting patron language. I think it is worth the cost
> for new functionality
> 
> 2 - The code addresses the creation of new overdue accountlines - the code
> is hit whenever a new overdue amount is generated. The note about deleted
> refers to the fact the info like 'Barcode' is calculated on display from
> information in the accountlines - when an item is deleted that link is
> broken and that info is not displayed. If [% item.barcode %] is added to the
> description it will be retained after deletion

Thx. Will finish it this one now.
Comment 13 Marcel de Rooy 2022-04-19 11:40:25 UTC
(In reply to Martin Renvoize from comment #10)
> It feels like a bit of a hack being in letters/notices though, which is a
> bit of a shame, but I can appreciate why you added it there.

True. We might resolve that with a small follow-up.
Comment 14 Marcel de Rooy 2022-04-19 11:59:50 UTC
Created attachment 133402 [details] [review]
Bug 24865: Customize the Accountlines Description

It would be great if we could customize what information was added to the "Description of charges" field when a fine was made so data could be stored even when the item is deleted.

Test Plan:
1) Create an overdue checkout that will get a fine
2) Run fines.pl
3) Note the description for the fine
4) Delete the fine from the database
5) Apply this patch
6) Run updatedatabase.pl
7) Restart all the things!
8) Run fines.pl
9) Note the description of the fine is unchanged
10) Delete the fine again
11) Browse to Slips & Notices
12) Edit the new notice OVERDUE_FINE_DESC
    You will have access to the objects checkout, item, and borrower
13) Run fines.pl
14) Note your new description was used

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2022-04-19 11:59:55 UTC
Created attachment 133403 [details] [review]
Bug 24865: (QA follow-up) Remove hardcoded notice name from protected_letters

If the letter has been removed, fall back to itemnumber/due date. (Title is
no longer fetched.) We may assume that the notice is present.

Note: The option to 'protect' a notice may need some more thought. Perhaps
it needs to be an attribute on itself.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested by deleting notice, running fines again.
Comment 16 Martin Renvoize (ashimema) 2022-04-19 12:34:10 UTC
Good call on QA.. I was about to say, lets be pragmatic and handle follow-ups and follow-ups :)
Comment 17 Fridolin Somers 2022-05-11 06:18:02 UTC
This new notice is missing from fr-FR/1-Obligatoire/sample_notices.sql

Note there is a work in progress Bug 27619
Comment 18 Martin Renvoize (ashimema) 2022-05-11 06:24:37 UTC
At this point in the cycle now I think I'd hold off and push this early next cycle..

I'm going to take another look at bug 27334 to see what's holding it up and as we've got collisions with bug 27619 too I think it's sensible to postpone this one a couple of weeks ;)
Comment 19 Tomás Cohen Arazi (tcohen) 2022-07-05 14:52:22 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 20 Lucas Gass (lukeg) 2022-08-12 15:32:37 UTC
Enhancement will not be backported to 22.05.x series