Bug 12285 - Allow easy printing of patron's fines
Summary: Allow easy printing of patron's fines
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-19 10:29 UTC by paxed
Modified: 2021-08-10 16:34 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
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. (7.06 KB, patch)
2014-05-19 10:34 UTC, paxed
Details | Diff | Splinter Review
Bug 12285: Allow easy printing of patron's fines. (7.19 KB, patch)
2014-06-22 14:21 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 12285: Allow easy printing of patron's fines. (7.24 KB, patch)
2014-07-15 01:51 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 12285: Allow easy printing of patron's fines. (8.00 KB, patch)
2021-07-27 10:19 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12285: Fix QA issues (10.65 KB, patch)
2021-07-27 10:20 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 12285: Fix QA issues (10.69 KB, patch)
2021-07-28 07:57 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 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!