Bug 12285

Summary: Allow easy printing of patron's fines
Product: Koha Reporter: paxed <pasi.kallinen>
Component: Staff ClientAssignee: Emmi Takkinen <emmi.takkinen>
Status: Needs Signoff --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: barton, emmi.takkinen, gmcharlt, josef.moravec, katrin.fischer, kelly, kyle, lisetteslatah, marjorie.barry-vila, nkeener, sandboxes, veron
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=1232
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10708
Change sponsored?: --- Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
Attachments: Add a printing option to the staff client patron details page to print a slip of patron's fines.
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Fix QA issues
Bug 12285: Fix QA issues
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Fix QA issues
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Fix QA issues
Bug 12285: Use accountlines date if issue is not found
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Allow easy printing of patron's fines.
Bug 12285: Fix QA issues
Bug 12285: Use accountlines date if issue is not found
Bug 12285: (follow-up) Correct extra quote in yml file
Bug 12285: Don't print fines with undef amountoutstanding
Bug 12285: Determine patrons preferred languge

Description paxed 2014-05-19 10:29:26 UTC
There should be an easy way to print a slip of patron's fines.
Comment 1 paxed 2014-05-19 10:34:44 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2014-06-09 19:18:59 UTC
I tested. 
After applying the patch the menu item appeared as expected, but the slip did not display properly. Ic checked FINESLIP in Home › Tools › Notices & Slips. It had missing angle brackets (<) at two places. After fixing it the slip printed as expected.

The HTML in FINESLIP inserted by the patch was:

<<borrowers.firstname>> <<borrowers.surname>><br>
<<borrowers.cardnumber>><br>
Fines: <<total.fines>>
<ul>
<fines>
<li><<fines.date_due>>, 1001<fines.amount>><br>
Bar code: <<items.barcode>><br>
<<fines.description>>
</fines>
</ul>
Total: 1001<total.amount>>

If I look at the patch, the strange 1001's appear at the places where, in both SQL statemens, we have $<<fines.amount>> and $<<total.amount>>
Could it be that the $ signes are superfluous?
Comment 3 Nicole C. Engard 2014-06-20 19:00:34 UTC
Changing status so Marc's concerns are addressed in a new patch.
Comment 4 Marc Véron 2014-06-22 14:21:10 UTC Comment hidden (obsolete)
Comment 5 sandboxes@biblibre.com 2014-07-15 01:51:39 UTC
Patch tested with a sandbox, by Aleisha <aleishaamohia@hotmail.com>
Comment 6 sandboxes@biblibre.com 2014-07-15 01:51:57 UTC
Created attachment 29692 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Add a printing option to the staff client patron details page
to print a slip of patron's fines.

To test:
1) Apply patch.
2) Check in tools > Notices & Slips that you have FINESLIP slip,
   and a print message there. If not, run updatedatabase.pl
3) Either create some fines for a patron, or look up a patron
   with existing fines.
4) From that patron's detail page, and the Print-menu in
   the toolbar, select "Print fines"
5) You should get a slip of the patron's fines.

I removed superfluous $ signs in sample_notices.sql and updatedatabase.pl
Patch works now as expected. However, it needs now a Sign-off by somebody else.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Comment 7 Katrin Fischer 2014-07-20 18:48:42 UTC
Comment on attachment 29692 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Review of attachment 29692 [details] [review]:
-----------------------------------------------------------------

Hi Pasi,

I have added my comments using Splinter review, hope everything is clear. Again, I really like the idea of this and hope you can provide a follow-up.

Addition to Splinter comments:
Please include the new notice template in the translated sample_notices.sql as well. Leaving them in English is fine.

::: C4/Members.pm
@@ +2406,5 @@
>      );
>  }
>  
> +
> +sub GetBorrowerFines {

I am not sure this one is needed - why not use GetMemberAccountRecords? If we are to keep this one, please point out why it's more useful and add unit tests and documentation (POD).

@@ +2422,5 @@
> +    my $data = $sth->fetchall_arrayref({});
> +    return $data;
> +}
> +
> +sub FineSlip {

Needs unit tests.

::: installer/data/mysql/en/mandatory/sample_notices.sql
@@ +143,5 @@
>  Your library.'
>  );
> +
> +INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type)
> +VALUES ( 'circulation', 'FINESLIP', '', 'Patron fines -slip', '1', 'Fines', '<<borrowers.firstname>> <<borrowers.surname>><br>

I think 'Fines and fees slip' would maybe be better.

@@ +149,5 @@
> +Fines: <<total.fines>>
> +<ul>
> +<fines>
> +<li><<fines.date_due>>, <<fines.amount>><br>
> +Bar code: <<items.barcode>><br>

Small typo - barcode, please fix.

::: installer/data/mysql/updatedatabase.pl
@@ +8573,5 @@
> +$DBversion = "3.17.00.XXX";
> +if ( CheckVersion($DBversion) ) {
> +    $dbh->do("
> +INSERT INTO  letter (module, code, branchcode, name, is_html, title, content, message_transport_type)
> +VALUES ( 'circulation', 'FINESLIP', '', 'Patron fines -slip', '1', 'Fines', '<<borrowers.firstname>> <<borrowers.surname>><br>

Please fix the name here too.

@@ +8579,5 @@
> +Fines: <<total.fines>>
> +<ul>
> +<fines>
> +<li><<fines.date_due>>, <<fines.amount>><br>
> +Bar code: <<items.barcode>><br>

Small typo, please fix.

::: koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc
@@ +129,4 @@
>                  [% IF ( CAN_user_borrowers ) %]<li><a id="printsummary" href="#">Print summary</a></li>[% END %]
>                  <li><a id="printslip" href="#">Print slip</a></li>
>                  <li><a id="printquickslip" href="#">Print quick slip</a></li>
> +                <li><a id="printfineslip" href="#">Print fines</a></li>

Maybe print fines and fees? Just a suggestion, not blocker.
Comment 8 NancyK. 2016-09-06 17:32:25 UTC
Hi, is anyone still considering his bug.  I have had a request from staff for this very feature.
Comment 9 Katrin Fischer 2017-02-23 10:36:13 UTC
Pasi, is this still on your todo list or would it be ok to reset the Assignee?
Comment 10 Emmi Takkinen 2021-07-27 10:19:46 UTC
Created attachment 123202 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Add a printing option to the staff client patron details page
to print a slip of patron's fines.

To test:
1) Apply patch.
2) Check in tools > Notices & Slips that you have FINESLIP slip,
   and a print message there. If not, run updatedatabase.pl
3) Either create some fines for a patron, or look up a patron
   with existing fines.
4) From that patron's detail page, and the Print-menu in
   the toolbar, select "Print fines"
5) You should get a slip of the patron's fines.

I removed superfluous $ signs in sample_notices.sql and updatedatabase.pl
Patch works now as expected. However, it needs now a Sign-off by somebody else.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 11 Emmi Takkinen 2021-07-27 10:20:22 UTC
Created attachment 123203 [details] [review]
Bug 12285: Fix QA issues

This patch:
- removes GetBorrowerFines and reworks Fineslip to use Koha objects
- adds the new notice template in the translated sample_notices.sql
files
- adds unit tests

To test follow test plan from previous patch and confirm everything
works as intented. Also prove t/db_dependent/Members/FineSlip.t.

Sponsored-by: Koha-Suomi Oy
Comment 12 Owen Leonard 2021-07-27 11:28:51 UTC
The print slip page gives me an error:

Can't call method "unblessed" on an undefined value at /kohadevbox/koha/C4/Members.pm line 692

QA tool reports the following failures:

 FAIL	C4/Members.pm
   FAIL	  pod coverage
		POD is missing for FineSlip
		POD is missing for checkcardnumber

 FAIL	t/db_dependent/Members/FineSlip.t
   FAIL	  file permissions
		File must have the exec flag
Comment 13 Emmi Takkinen 2021-07-28 07:57:09 UTC
Created attachment 123243 [details] [review]
Bug 12285: Fix QA issues

This patch:
- removes GetBorrowerFines and reworks Fineslip to use Koha objects
- adds the new notice template in the translated sample_notices.sql
files
- adds unit tests

Sponsored-by: Koha-Suomi Oy
Comment 14 Kelly McElligott 2021-08-10 16:34:09 UTC
Hi ! Tried to sign off on this bug and received this error:

Can't call method "date_due" on an undefined value at /kohadevbox/koha/C4/Members.pm line 702

Thanks!
Comment 15 Emmi Takkinen 2022-01-24 13:56:11 UTC
Created attachment 129723 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Add a printing option to the staff client patron details page
to print a slip of patron's fines.

To test:
1) Apply patch.
2) Check in tools > Notices & Slips that you have FINESLIP slip,
   and a print message there. If not, run updatedatabase.pl
3) Either create some fines for a patron, or look up a patron
   with existing fines.
4) From that patron's detail page, and the Print-menu in
   the toolbar, select "Print fines"
5) You should get a slip of the patron's fines.

I removed superfluous $ signs in sample_notices.sql and updatedatabase.pl
Patch works now as expected. However, it needs now a Sign-off by somebody else.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 16 Emmi Takkinen 2022-01-24 13:56:42 UTC
Created attachment 129724 [details] [review]
Bug 12285: Fix QA issues

This patch:
- removes GetBorrowerFines and reworks Fineslip to use Koha objects
- adds the new notice template in the translated sample_notices.sql
files
- adds unit tests

Sponsored-by: Koha-Suomi Oy
Comment 17 Owen Leonard 2022-01-25 14:38:24 UTC
I'm still getting warnings from the QA tool:

 FAIL	installer/data/mysql/en/mandatory/sample_notices.yml
   FAIL	  yaml_valid
		ARRAY(0x55d88d804738)

The change to sample_notices.yml also lacks a value for "module," which is required.

After applying the patch and running "reset_all" in koha-testing-docker I see this error in the output:

C4::Installer::load_sql returned the following errors while attempting to load /kohadevbox/koha/installer/data/mysql/en/mandatory/sample_notices.yml:
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`letter`, CONSTRAINT `message_transport_type_fk` FOREIGN KEY (`message_transport_type`) REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE CASCADE ON UPDA) at /kohadevbox/koha/C4/Installer.pm line 574.
Comment 18 Emmi Takkinen 2022-01-28 11:48:24 UTC
Created attachment 129922 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Add a printing option to the staff client patron details page
to print a slip of patron's fines.

To test:
1) Apply patch.
2) Check in tools > Notices & Slips that you have FINESLIP slip,
   and a print message there. If not, run updatedatabase.pl
3) Either create some fines for a patron, or look up a patron
   with existing fines.
4) From that patron's detail page, and the Print-menu in
   the toolbar, select "Print fines"
5) You should get a slip of the patron's fines.

I removed superfluous $ signs in sample_notices.sql and updatedatabase.pl
Patch works now as expected. However, it needs now a Sign-off by somebody else.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 19 Emmi Takkinen 2022-01-28 11:49:00 UTC
Created attachment 129923 [details] [review]
Bug 12285: Fix QA issues

This patch:
- removes GetBorrowerFines and reworks Fineslip to use Koha objects
- adds the new notice template in the translated sample_notices.sql
files
- adds unit tests

Sponsored-by: Koha-Suomi Oy
Comment 20 Emmi Takkinen 2022-01-28 11:53:16 UTC
(In reply to Owen Leonard from comment #17)
> I'm still getting warnings from the QA tool:
> 
>  FAIL	installer/data/mysql/en/mandatory/sample_notices.yml
>    FAIL	  yaml_valid
> 		ARRAY(0x55d88d804738)
> 
> The change to sample_notices.yml also lacks a value for "module," which is
> required.
> 
> After applying the patch and running "reset_all" in koha-testing-docker I
> see this error in the output:
> 
> C4::Installer::load_sql returned the following errors while attempting to
> load /kohadevbox/koha/installer/data/mysql/en/mandatory/sample_notices.yml:
> DBD::mysql::db do failed: Cannot add or update a child row: a foreign key
> constraint fails (`koha_kohadev`.`letter`, CONSTRAINT
> `message_transport_type_fk` FOREIGN KEY (`message_transport_type`)
> REFERENCES `message_transport_types` (`message_transport_type`) ON DELETE
> CASCADE ON UPDA) at /kohadevbox/koha/C4/Installer.pm line 574.

I was unable to produce these errors, but fixed invalid yaml syntax and missing "module" value to the first patch. Also fixed atomicupdate files name (there was cap between 12285 and -allow) and added IGNORE to INSERT statement (my wild guess is that this caused that last error).
Comment 21 Owen Leonard 2022-01-28 12:24:46 UTC
> >  FAIL	installer/data/mysql/en/mandatory/sample_notices.yml
> >    FAIL	  yaml_valid
> > 		ARRAY(0x55d88d804738)

Line 1594 has an extra quote:

            - "<li>""

I get an error when printing a fine slip for a charge which isn't associated with a checkout:

Can't call method "date_due" on an undefined value at /kohadevbox/koha/C4/Members.pm line 711

I triggered this by going to Accounting -> Create manual invoice and adding an arbitrary amount.
Comment 22 Emmi Takkinen 2022-01-28 13:32:55 UTC
Created attachment 129937 [details] [review]
Bug 12285: Use accountlines date if issue is not found

Printing fines slip would raise error if patron has
manually created invoice. In these cases use accountlines
date instead of issues date_due. Also silence error:
"Argument "" isn't numeric in numeric comparison (<=>)"

Sponsored-by: Koha-Suomi Oy
Comment 23 Owen Leonard 2022-01-28 14:00:43 UTC
Created attachment 129943 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Add a printing option to the staff client patron details page
to print a slip of patron's fines.

To test:
1) Apply patch.
2) Check in tools > Notices & Slips that you have FINESLIP slip,
   and a print message there. If not, run updatedatabase.pl
3) Either create some fines for a patron, or look up a patron
   with existing fines.
4) From that patron's detail page, and the Print-menu in
   the toolbar, select "Print fines"
5) You should get a slip of the patron's fines.

I removed superfluous $ signs in sample_notices.sql and updatedatabase.pl
Patch works now as expected. However, it needs now a Sign-off by somebody else.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 24 Owen Leonard 2022-01-28 14:04:57 UTC
Created attachment 129944 [details] [review]
Bug 12285: Allow easy printing of patron's fines.

Add a printing option to the staff client patron details page
to print a slip of patron's fines.

To test:
1) Apply patch.
2) Check in tools > Notices & Slips that you have FINESLIP slip,
   and a print message there. If not, run updatedatabase.pl
3) Either create some fines for a patron, or look up a patron
   with existing fines.
4) From that patron's detail page, and the Print-menu in
   the toolbar, select "Print fines"
5) You should get a slip of the patron's fines.

I removed superfluous $ signs in sample_notices.sql and updatedatabase.pl
Patch works now as expected. However, it needs now a Sign-off by somebody else.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 25 Owen Leonard 2022-01-28 14:05:03 UTC
Created attachment 129945 [details] [review]
Bug 12285: Fix QA issues

This patch:
- removes GetBorrowerFines and reworks Fineslip to use Koha objects
- adds the new notice template in the translated sample_notices.sql
files
- adds unit tests

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 26 Owen Leonard 2022-01-28 14:05:10 UTC
Created attachment 129946 [details] [review]
Bug 12285: Use accountlines date if issue is not found

Printing fines slip would raise error if patron has
manually created invoice. In these cases use accountlines
date instead of issues date_due. Also silence error:
"Argument "" isn't numeric in numeric comparison (<=>)"

Sponsored-by: Koha-Suomi Oy

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 27 Owen Leonard 2022-01-28 14:05:16 UTC
Created attachment 129947 [details] [review]
Bug 12285: (follow-up) Correct extra quote in yml file

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 28 Kyle M Hall 2022-03-31 15:12:22 UTC
t/db_dependent/Members/FineSlip.t is failing
Comment 29 Emmi Takkinen 2022-04-01 05:47:48 UTC
(In reply to Kyle M Hall from comment #28)
> t/db_dependent/Members/FineSlip.t is failing

Tested and tests pass fine. Could you take a look where it exactly fails for you?
Comment 30 Emmi Takkinen 2022-04-08 09:53:05 UTC
Just noticed that accountlines with undefined amountoutstanding are printed in fineslip.
Comment 31 Emmi Takkinen 2022-04-08 10:06:26 UTC
Created attachment 133112 [details] [review]
Bug 12285: Don't print fines with undef amountoutstanding

Accountlines with undefined amountoutstading value are
printed in fineslip.

To test:
1) Find patron with credit type accountlines.
2) Edit some of those lines from database, set their
amountoutstanding as NULL.
3) Print fineslip.
=> Note that edited credit lines are displayed in the
fineslip.
4) Apply patch.
5) Print fineslip.
=> Note that lines are no longer displayed in the
fineslip.

Sponsored-by: Koha-Suomi Oy
Comment 32 Emmi Takkinen 2022-04-11 10:40:55 UTC
Created attachment 133147 [details] [review]
Bug 12285: Determine patrons preferred languge

Fineslip is not printed by patrons preferred
language choice.

To test:
1) Add content to fineslip under "en" tab.
2) Find patron with fines.
3) Set patrons preferred language as "en".
4) Print fines.
=> Fineslip has default content.
5) Apply patch.
6) Print slip.
=> Fineslip has "en" content.

Sponsored-by: Koha-Suomi Oy