Bug 14605 - The description on pay/write off individual fine is wrong
Summary: The description on pay/write off individual fine is wrong
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Blou
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-27 20:24 UTC by Blou
Modified: 2017-12-07 22:15 UTC (History)
12 users (show)

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


Attachments
Bug 14605 - Corrects the individual fine's description (3.03 KB, patch)
2015-07-27 20:26 UTC, Blou
Details | Diff | Splinter Review
display after patch (19.40 KB, image/png)
2015-07-28 15:15 UTC, Blou
Details
Bug 14605 - Corrects the individual fine's description (3.10 KB, patch)
2015-07-28 15:37 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 14605 - Corrects the individual fine's description (3.07 KB, patch)
2016-05-03 19:31 UTC, Blou
Details | Diff | Splinter Review
Bug 14605 - Corrects the individual fine's description (3.22 KB, patch)
2016-05-31 14:42 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14605 - Corrects the individual fine's description (3.26 KB, patch)
2016-06-10 16:07 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Blou 2015-07-27 20:24:33 UTC
When paying a fine, the description is built out of the borrower's type and title, instead of the actual fine's description.

This has two causes: the information was not passed, and then it was overriden by some line
<<
#workaround for Bug 4041 to get rid of duplicated code in circ-menu.tt, use circ-menu.inc instad
   $template->param(%$borrower);
>>

which basically replace the accountline's description with the borrower's ?!?!

Anyway, steps to reproduce:
1) in the staff, go to a user with a fine to pay.
   a) if none, go to a user and Fines>>"Create manual invoice" with a distinctive description.
2) In Fines>>Pay fines, click the "Pay" button on the line of the fine.
3) This will take you a "Pay an individual fine" screen, where the description will look like "Staff Mrs", for the category and title of the borrower.

Apply the patch, and do it again.  Voilà.
Comment 1 Blou 2015-07-27 20:26:23 UTC Comment hidden (obsolete)
Comment 2 Mirko Tietgen 2015-07-28 14:47:21 UTC
It looks like »Description« and »Account type« were mixed up. When I apply the patch, I have the fine description in »Description«, but also in »Account type«. I suppose this is where the user category should go. Can you confirm and adjust the patch?
Comment 3 Blou 2015-07-28 15:15:52 UTC
Created attachment 41216 [details]
display after patch

I do not reproduce your issue, unless I'm misunderstanding.
In "Account type", I only get the "F" as I had before.

I checked again the diff's code, and can't see how it would have modified the account_type display.
Comment 4 Mirko Tietgen 2015-07-28 15:34:29 UTC
(In reply to Blou from comment #3)

> I do not reproduce your issue, unless I'm misunderstanding.
> In "Account type", I only get the "F" as I had before.

Maybe I don't understand what the account type column is actually for. I assumed it is for the patron category (which accidentally ended up in description before your patch).

I left F as description before and thought it is the same now in both fields. After I used a different description, I see the fields have different contents. I am still not sure what account type is supposed to mean though. If it is the type of invoice, it would probably be better to display the description (Fine), not the code (F), and label the column »Invoice type«?
Comment 5 Mirko Tietgen 2015-07-28 15:37:04 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-08-05 08:55:12 UTC
I would say that the expected behavior was to display the description (what you did) and the title of the biblio, don't you think?
Comment 7 Jonathan Druart 2015-08-05 08:55:45 UTC
And what about existing rows in the DB? Do you think it's possible to update them?
Comment 8 Blou 2016-05-03 19:31:01 UTC Comment hidden (obsolete)
Comment 9 Blou 2016-05-03 19:36:54 UTC
I rebased.
Regarding the "issues": The biblio's title was not part of the previous functionality, and I don't think it's right anyway, since I presume fines can be for things other than associated to a biblio.

As for correcting the current DB, that's feasible, but a bit dodgy.  I would really fear to replace perfectly valid subjects (corrected manually by the employee), and validating what is right or not might be a bit too much work for what it's worth.  This patch is here to correct make life easier in the future, but it's not correcting a major bug that's worth a big discussion on "How to fix the current data", don't you think?
Comment 10 Bernardo Gonzalez Kriegel 2016-05-04 00:04:43 UTC
(In reply to Blou from comment #9)
> I rebased.
> Regarding the "issues": The biblio's title was not part of the previous
> functionality

That's correct.
It can be done, but I think in another bug.

> As for correcting the current DB, that's feasible, but a bit dodgy.
I don't know if it's possible, description in accountlines must be set initially. A fee entry does not reference the original biblio.

I think that correct Status for this is Signed off, no problem if you need another sign but it's already signed by Mirko.
Comment 11 Marc Véron 2016-05-31 14:42:54 UTC Comment hidden (obsolete)
Comment 12 Nick Clemens 2016-06-10 16:07:43 UTC
Created attachment 52248 [details] [review]
Bug 14605 - Corrects the individual fine's description

When paying a fine, the description is built out of the borrower's type and title, instead of the actual fine's description.

STEPS:
1) in the staff, go to a user with a fine to pay.
   a) if none, go to a user and Fines>>"Create manual invoice" with a distinctive description.
2) In Fines>>Pay fines, click the "Pay" button on the line of the fine.
3) This will take you a "Pay an individual fine" screen, where the description will look like "Staff Mrs", for the category and title of the borrower.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Kyle M Hall 2016-06-10 17:34:42 UTC
Pushed to master for 16.11, thanks Philippe!
Comment 14 Frédéric Demians 2016-06-15 07:45:51 UTC
Pushed in 16.05. Will be in 16.05.01.
Comment 15 Julian Maurice 2016-06-16 09:15:53 UTC
Patch pushed to 3.22.x, will be in 3.22.8