https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11371 --- Comment #39 from Katrin Fischer <katrin.fischer@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.