http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6934 --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 36476 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36476 [SIGNED-OFF] Bug 6934 New report Cash Register Statistics Review of attachment 36476: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=6934&attachment=36476) ----------------------------------------------------------------- Hi Simith, I have added a few comments from code review - please fix in a follow-up. I have only marked the first occurrence of each, please also look below and above to make sure to get them all. Thx! ::: koha-tmpl/intranet-tmpl/prog/en/includes/reports-menu.inc @@ +10,4 @@
<li><a href="/cgi-bin/koha/reports/catalogue_stats.pl">Catalog</a></li> <li><a href="/cgi-bin/koha/reports/issues_stats.pl">Circulation</a></li> <li><a href="/cgi-bin/koha/reports/serials_stats.pl">Serials</a></li> + <li><a href="/cgi-bin/koha/reports/cash_register_stats.pl">Cash Register</a></li>
Please fix capitalization here and in the other template files to obey our rules. Thx! ::: koha-tmpl/intranet-tmpl/prog/en/modules/reports/cash_register_stats.tt @@ +7,5 @@
+<script type="text/javascript" src="[% themelang %]/js/datatables.js"></script> +<script type="text/javascript" id="js">$(document).ready(function() { + $(document).ready(function() { + $("#tbl_cash_register_stats").dataTable($.extend(true, {}, dataTablesDefaults, { + "iDisplayLength": 50
Be careful, without adding a paging option here, the paging on the table is broken (see recent 'paging' bugs for examples on how to fix) @@ +143,5 @@
+ <option value="F">Fine</option> + [% END %] + + [% IF transaction_type == "FU" %] + <option value="FU" selected="selected">Fine - long period</option>
Please check fine descriptions with those in the other templates - I think this one is not quite right. @@ +233,5 @@
+ <thead> + <tr> + <th>Manager name</th> + <th>Borrower cardnumber</th> + <th>Borrower name</th>
Please use patron instead of borrower. @@ +234,5 @@
+ <tr> + <th>Manager name</th> + <th>Borrower cardnumber</th> + <th>Borrower name</th> + <th>Branch</th>
Please use library instead of branch. @@ +240,5 @@
+ <th>Transaction type</th> + <th>Amount</th> + <th>Biblio title</th> + <th>Barcode</th> + <th>Document type</th>
Please use item type. ::: reports/cash_register_stats.pl @@ +14,5 @@
+# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use strict; +use warnings;
Please use Modern::Perl; instead. @@ +22,5 @@
+use C4::Reports; +use C4::Output; +use C4::Koha; +use C4::Circulation; +use C4::Dates qw/format_date format_date_in_iso/;
Please avoid use of C4::Dates as it's deprecated. @@ +45,5 @@
+my $output = $input->param("output"); +my $basename = $input->param("basename"); +my $transaction_type = $input->param("transaction_type") || 'ACT'; +my $branchcode = $input->param("branch") || C4::Context->userenv->{'branch'}; +our $sep = ",";
I think the separator should not be hardcoded, it would be better to use a pull down like on the other reports or using the delimiter system preference. @@ +49,5 @@
+our $sep = ","; + +$template->param( + do_it => $do_it, + DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(),
We are no longer using DHTML calendar, please use the TT Dates plugin instead. -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.