https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11371
--- Comment #39 from Katrin Fischer <katrin.fisc...@bsz-bw.de> --- Comment on attachment 45982 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45982 Bug 11371 - Add a new report : Orders by budget Review of attachment 45982: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=11371&attachment=45982) ----------------------------------------------------------------- I feel we are getting closer with this, even if it might not feel like it. Can you please take a look at my new comments and the one Jonathan has made? There is one additional thing not noted in the splitter comments: The new report should also be added to the reports module navigation (I think it's reports-menu.inc), that shows up on the left side once you have opened a reports page. ::: koha-tmpl/intranet-tmpl/prog/en/modules/reports/orders_by_budget.tt @@ +38,5 @@ > + <thead> > + <tr> > + <th>Fund</th> > + <th>Basket</th> > + <th>Basket Name</th> Please fix capitalization in the list of column names. We have agreed that only the first word should be capitalized. @@ +42,5 @@ > + <th>Basket Name</th> > + <th>Basket By</th> > + <th>Title</th> > + <th>Currency</th> > + <th>Vendor Price</th> Please use list price to match acquisition terminology. ::: reports/orders_by_fund.pl @@ +26,5 @@ > +=cut > + > +use strict; > +use warnings; > + Please use Modern::Perl. @@ +111,5 @@ > + > + # Format the dates and currencies correctly > + $order->{'datereceived'} = > Koha::DateUtils::output_pref(Koha::DateUtils::dt_from_string($order->{'datereceived'})); > + $order->{'entrydate'} = > Koha::DateUtils::output_pref(Koha::DateUtils::dt_from_string($order->{'entrydate'})); > + $order->{'listprice'} = sprintf( "%.2f", $order->{'listprice'} ); It would be nicer to handle the display on template level with the new Price TT plugin. @@ +221,5 @@ > + foreach my $thisbudget ( sort {$a->{budget_name} cmp $b->{budget_name}} > @non_active_budgets ) { > + my $budget_period_desc = > C4::Budgets::GetBudgetPeriodDescription($thisbudget->{budget_id}); > + my %row = ( > + value => $thisbudget->{budget_id}, > + description => "[i] ". $thisbudget->{budget_name}, The [i] is a translation issue. I think as it's in the template now it's not translatable at all, but I would also prefer another way of displaying it, as [i] is not very clear and will probably cause confusion for translators. Looking in other spots of the acquisition module I have some suggestions: - Don't show the inactive funds to begin with, but only, when a checkbox is checked (see neworderempty.pl) and then put (inactive) behind the inactiv fund name. - Use a hierarchical display for more than one budget and it's funds like on histsearch.pl ::: t/db_dependent/Budgets.t @@ +2,2 @@ > use Modern::Perl; > +use Test::More ; Please don't remove the number of tests to run. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list Koha-bugs@lists.koha-community.org 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/