http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6934
--- Comment #23 from Katrin Fischer <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
