Bug 13961 - Option to include patron's total amount of fines in notices
Summary: Option to include patron's total amount of fines in notices
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-07 13:37 UTC by Katrin Fischer
Modified: 2020-03-24 11:09 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13961: Option to include patron's total amount of fines in notices (1.88 KB, patch)
2015-08-25 06:29 UTC, j.kylmala
Details | Diff | Splinter Review
Screenshot from diff view - white chars in empty line 648 in C4/letters.pm (9.61 KB, image/png)
2015-08-28 13:04 UTC, Marc Véron
Details
[Follow-up] Remove whitespace from line 648 in C4/Letters.pm (911 bytes, patch)
2015-08-28 15:34 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 13961: Option to include patron's total amount of fines in notices (2.10 KB, patch)
2016-04-21 14:22 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 13961: Option to include patron's total amount of fines in notices (2.21 KB, patch)
2020-02-13 12:42 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 13961: Add currency format handling (2.27 KB, patch)
2020-02-13 12:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 13961: Add unit test (3.15 KB, patch)
2020-02-13 12:45 UTC, Emmi Takkinen
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2015-04-07 13:37:07 UTC
It would be nice if we had the option to include information about the patron's total of fines in notices.

- overdues (fine per item can be printed, but no total)
- checkin
- checkout
- predue
- due
Comment 1 j.kylmala 2015-08-20 11:25:17 UTC
Koha -> Tools -> Notices, you mean? Would there be one one row per patron and there would be all the info? I'm not sure if I understood right what you suggested.
Comment 2 Katrin Fischer 2015-08-24 13:25:15 UTC
Just looking for a new placeholder. We got <<items.fine>> to display the fine per item. And we have something like <<today>> to display today's date. The suggestion would be to have something like <<totalfine>> or similar to print the total amount owed by a patron in the letter. So a single placeholder - not one to be repeated in the letter.
Comment 3 j.kylmala 2015-08-24 13:47:45 UTC
(In reply to Katrin Fischer from comment #2)
> Just looking for a new placeholder. We got <<items.fine>> to display the
> fine per item. And we have something like <<today>> to display today's date.
> The suggestion would be to have something like <<totalfine>> or similar to
> print the total amount owed by a patron in the letter. So a single
> placeholder - not one to be repeated in the letter.

Yeah, that would great. Actually we just spoke about that kind of feature in our library. I think I could add that quickly there. :)
Comment 4 j.kylmala 2015-08-25 06:29:17 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2015-08-28 08:42:38 UTC
Works as expected.
However, C4/Letters.pm line 648 has forbidden pattern (trailing space chars).
Comment 6 j.kylmala 2015-08-28 09:15:44 UTC
(In reply to Marc Véron from comment #5)
> Works as expected.
> However, C4/Letters.pm line 648 has forbidden pattern (trailing space chars).

I don't see any in that line.
Comment 7 Marc Véron 2015-08-28 13:04:54 UTC Comment hidden (obsolete)
Comment 8 j.kylmala 2015-08-28 13:42:09 UTC
(In reply to Marc Véron from comment #7)
> Created attachment 42077 [details]
> Screenshot from diff view - white chars in empty line 648 in C4/letters.pm
> 
> Hi Joonas,
> 
> There are some space chars in an empty line, see screenshot from diff view.
> 
> Marc

When I apply the patch and try to edit with Gedit, Atom, Nano none of these editors see the whitespace. Can you please make a follow-up where you remove it because I'm not able to.
Comment 9 j.kylmala 2015-08-28 13:46:30 UTC
(In reply to Joonas Kylmälä from comment #8)
> (In reply to Marc Véron from comment #7)
> > Created attachment 42077 [details]
> > Screenshot from diff view - white chars in empty line 648 in C4/letters.pm
> > 
> > Hi Joonas,
> > 
> > There are some space chars in an empty line, see screenshot from diff view.
> > 
> > Marc
> 
> When I apply the patch and try to edit with Gedit, Atom, Nano none of these
> editors see the whitespace. Can you please make a follow-up where you remove
> it because I'm not able to.

I think the problem is with the bugzilla's diff shower because in my patch I didn't even modify that line.
Comment 10 Marc Véron 2015-08-28 15:34:00 UTC Comment hidden (obsolete)
Comment 11 Marc Véron 2016-04-21 14:22:35 UTC
Created attachment 50517 [details] [review]
Bug 13961: Option to include patron's total amount of fines in notices

Possibility to show patrons's total amount of fines in notices.

Test plan:
1. Apply patch
2. Modify some notice in Tools -> notices & slips
3. See that in the left side of where one can edit message's body is now
   also a new attribute that one can include to the text:
   borrowers.totalfine
4. Include that to the text and see it showing the patron's total amount
   of fines.

Sponsored-by: Vaara-kirjastot

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 12 Marc Véron 2016-04-21 14:28:02 UTC
Comment on attachment 42089 [details] [review]
[Follow-up] Remove whitespace from line 648 in C4/Letters.pm

QA tools no longer complain regarding first patch. Obsoleting this follow-up.
Comment 13 Katrin Fischer 2016-04-28 12:48:28 UTC
Testing this on top of bug 16378
Comment 14 Kyle M Hall 2016-04-28 13:03:50 UTC
Please add a unit test for this addition.

Also, could you please change the key needed from borrowers.totalfine to something like borrowers.account_balance? I think that would be much more readable and explanatory.
Comment 15 Katrin Fischer 2016-04-28 13:15:18 UTC
I was also looking into this and have another note: Please take a look in Overdues.pm at how items.fine is formatted:

            $item->{'fine'} = currency_format($currency_format, "$fine", FMT_SYMBOL);
            # if active currency isn't correct ISO code fallback to sprintf
            $item->{'fine'} = sprintf('%.2f', $fine) unless $item->{'fine'};

I think we should use the same formatting for all money amounts if possible - especially as it's very likely that people will want to use those 2 together!
Comment 16 Ray Delahunty 2016-05-11 14:45:33 UTC
Are we sure that <<items.fines>> is working as it should? We are on 3.20.01 and we recently changed ODUE to use <item></item> allowing nice layout rather than the horridly formatted <<items.content>> variable. But often (not always) the fine amount wasn’t pulling through. Also to our horror, after the change sometimes only the first overdue would be listed, and not the full list. The consensus here is that the <<items.fine>> tag can cause problems when generating notices and we have reverted to <<items.content>>. This has resolved issues with notices being truncated after the first item. (We can find no pattern for why some of these fail but we suspect it is related to the <<items.fine>> variable.)
Ray Delahunty
University of the Arts London
Comment 17 Katrin Fischer 2016-05-11 16:18:15 UTC
Hi, this bug is about a different feature, so it would be good to open a separate bug report, maybe with a 'see also' link to this one if you want to.

We have been using the items.fine variable without any problems for quite some time now - although not in 3.20, but in 3.18 and 3.22.

One reson for the fine not printing can be the sequence the cronjobs are being run. fines.pl should be run before overdue_notices.pl (currently not the default, see bug 11144). Or that you haven't defined a proper ISO code for your active currency.
Comment 18 Katrin Fischer 2016-05-11 16:19:08 UTC
Btw: you can use <item></item> and leave the <<items.fine>> out too!
Comment 19 Josef Moravec 2019-03-25 15:27:14 UTC
In ISSUESLIP ans ISSUEQSLIP is possible to do this:


[% USE Price %]

# your normal issue slip content

[% SET balance = borrower.account.balance %]
[% IF balance > 0 %]
Patron has debit of [% balance | $Price %].
[% END %]
[% IF balance < 0 %]
Patron has credit of [% balance | $Price %].
[% END %]
Comment 20 Emmi Takkinen 2020-02-13 12:42:06 UTC
Created attachment 98819 [details] [review]
Bug 13961: Option to include patron's total amount of fines in notices

Fixed patch to apply master version.
Comment 21 Emmi Takkinen 2020-02-13 12:45:14 UTC
Created attachment 98820 [details] [review]
Bug 13961: Add currency format handling

Added currency format handling proposed in comments.
Comment 22 Emmi Takkinen 2020-02-13 12:45:58 UTC
Created attachment 98821 [details] [review]
Bug 13961: Add unit test