From 9ca213316c8f0c03fbedc26f9a75042cce37c550 Mon Sep 17 00:00:00 2001 From: Chris Cormack Date: Fri, 13 Feb 2015 14:19:54 +1300 Subject: [PATCH] Revert "Bug 13352: On editing, prices should not be formatted" This reverts commit abc46657e5039ba9d40637624390c0a28a664c8e. --- Koha/Number/Price.pm | 15 - Koha/Template/Plugin/Price.pm | 9 +- admin/aqbudgetperiods.pl | 47 +- admin/aqbudgets.pl | 1 + .../prog/en/modules/admin/aqbudgetperiods.tt | 7 +- .../prog/en/modules/admin/aqbudgets.tt | 4 +- t/Prices.t | 412 ------------------ 7 files changed, 35 insertions(+), 460 deletions(-) delete mode 100644 t/Prices.t diff --git a/Koha/Number/Price.pm b/Koha/Number/Price.pm index f0da1e051f..80854ab68c 100644 --- a/Koha/Number/Price.pm +++ b/Koha/Number/Price.pm @@ -44,21 +44,6 @@ sub format { return Number::Format->new(%$format_params)->format_price($self->value); } -sub format_for_editing { - my ( $self, $params ) = @_; - return unless defined $self->value; - - my $format_params = $self->_format_params( $params ); - $format_params = { - %$format_params, - int_curr_symbol => '', - mon_thousands_sep => '', - mon_decimal_point => '.', - }; - - return Number::Format->new(%$format_params)->format_price($self->value); -} - sub unformat { my ( $self, $params ) = @_; return unless defined $self->value; diff --git a/Koha/Template/Plugin/Price.pm b/Koha/Template/Plugin/Price.pm index 807f30189e..f5d2a421ac 100644 --- a/Koha/Template/Plugin/Price.pm +++ b/Koha/Template/Plugin/Price.pm @@ -23,16 +23,11 @@ use Template::Plugin::Filter; use base qw( Template::Plugin::Filter ); use Koha::Number::Price; -our $DYNAMIC = 1; sub filter { - my ( $self, $value, $args, $config ) = @_; + my ( $self, $value ) = @_; $value ||= 0; - $config->{on_editing} //= 0; - my $formatted_price; - return $config->{on_editing} - ? Koha::Number::Price->new( $value )->format_for_editing - : Koha::Number::Price->new( $value )->format; + return Koha::Number::Price->new( $value )->format; } 1; diff --git a/admin/aqbudgetperiods.pl b/admin/aqbudgetperiods.pl index b452642768..119884ad99 100755 --- a/admin/aqbudgetperiods.pl +++ b/admin/aqbudgetperiods.pl @@ -21,32 +21,32 @@ =head1 admin/aqbudgetperiods.pl script to administer the budget periods table -This software is placed under the gnu General Public License, v2 (http://www.gnu.org/licenses/gpl.html) - -ALGO : -this script use an $op to know what to do. -if $op is empty or none of the above values, -- the default screen is build (with all records, or filtered datas). -- the user can clic on add, modify or delete record. -if $op=add_form -- if primkey exists, this is a modification,so we read the $primkey record -- builds the add/modify form -if $op=add_validate -- the user has just send datas, so we create/modify the record -if $op=delete_confirm -- we show the record having primkey=$primkey and ask for deletion validation form -if $op=delete_confirmed -- we delete the record having primkey=$primkey -if $op=duplicate_form -- displays the duplication of budget period form (allowing specification of dates) -if $op=duplicate_budget -- we perform the duplication of the budget period specified as budget_period_id + This software is placed under the gnu General Public License, v2 (http://www.gnu.org/licenses/gpl.html) + + ALGO : + this script use an $op to know what to do. + if $op is empty or none of the above values, + - the default screen is build (with all records, or filtered datas). + - the user can clic on add, modify or delete record. + if $op=add_form + - if primkey exists, this is a modification,so we read the $primkey record + - builds the add/modify form + if $op=add_validate + - the user has just send datas, so we create/modify the record + if $op=delete_confirm + - we show the record having primkey=$primkey and ask for deletion validation form + if $op=delete_confirmed + - we delete the record having primkey=$primkey + if $op=duplicate_form + - displays the duplication of budget period form (allowing specification of dates) + if $op=duplicate_budget + - we perform the duplication of the budget period specified as budget_period_id =cut use Modern::Perl; -use CGI qw ( -utf8 ); +use CGI; use List::Util qw/min/; use Koha::DateUtils; use Koha::Database; @@ -58,6 +58,8 @@ use C4::Acquisition; use C4::Budgets; use C4::Debug; +use Koha::Number::Price; + my $dbh = C4::Context->dbh; my $input = new CGI; @@ -105,6 +107,9 @@ if ( $op eq 'add_form' ) { my $budgetperiod_hash=GetBudgetPeriod($budget_period_id); # get dropboxes + $budgetperiod_hash->{budget_period_total} = + Koha::Number::Price->new( $budgetperiod_hash->{budget_period_total} ) + ->format; $template->param( %$budgetperiod_hash ); diff --git a/admin/aqbudgets.pl b/admin/aqbudgets.pl index 5efe1f6c13..0a71ff26d9 100755 --- a/admin/aqbudgets.pl +++ b/admin/aqbudgets.pl @@ -130,6 +130,7 @@ if ($op eq 'add_form') { $dropbox_disabled = BudgetHasChildren($budget_id); my $borrower = &GetMember( borrowernumber=>$budget->{budget_owner_id} ); $budget->{budget_owner_name} = ( $borrower ? $borrower->{'firstname'} . ' ' . $borrower->{'surname'} : '' ); + $$budget{$_}= sprintf("%.2f", $budget->{$_}) for grep{ /amount|encumb|expend/ } keys %$budget; } # build budget hierarchy diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt index 6e78916dd3..a0a811a097 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt @@ -1,4 +1,5 @@ [% USE KohaDates %] +[% USE format %] [% USE Price %] [%- BLOCK action_menu %] @@ -359,7 +360,7 @@ + size="10" maxlength="80" value="[% budget_period_total %]" />
  • @@ -497,7 +498,7 @@ [% IF r.orders_moved.size > 0 %] [% FOR order IN r.orders_moved %] - [% r.budget.budget_name %] (id=[% r.budget.budget_id %]) Amount=[% r.budget.budget_amount | $Price %][% IF r.unspent_moved %] ([% r.unspent_moved | $Price %] remaining has been moved)[% END %] + [% r.budget.budget_name %] (id=[% r.budget.budget_id %]) Amount=[% r.budget.budget_amount | format ("%.2f") %][% IF r.unspent_moved %] ([% r.unspent_moved | format ("%.2f")%] remaining has been moved)[% END %] [% order.basketname %] [% order.ordernumber %] Moved! @@ -514,7 +515,7 @@ [% ELSE %] [% IF r.error == 'budget_code_not_exists' %] - [% r.budget.budget_id %] [% r.budget.budget_amount | $Price %][% IF r.unspent_moved %] ([% r.unspent_moved | $Price %] remaining has been moved)[% END %] + [% r.budget.budget_id %] [% r.budget.budget_amount | format ("%.2f") %][% IF r.unspent_moved %] ([% r.unspent_moved | format ("%.2f") %] remaining has been moved)[% END %] This fund code does not exist in the destination budget. diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt index cd39d175d8..fe67194e45 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt @@ -458,7 +458,7 @@ var MSG_PARENT_BENEATH_BUDGET = "- " + _("New budget-parent is beneath budget")
  • - +
  • @@ -469,7 +469,7 @@ var MSG_PARENT_BENEATH_BUDGET = "- " + _("New budget-parent is beneath budget")
  • - + 0 to disable
    • diff --git a/t/Prices.t b/t/Prices.t deleted file mode 100644 index aa864d26a6..0000000000 --- a/t/Prices.t +++ /dev/null @@ -1,412 +0,0 @@ -use Modern::Perl; -use Test::More tests => 16; -use Test::MockModule; - -use t::lib::Mocks; - -BEGIN { - my $context_module = t::lib::Mocks::mock_dbh; - use_ok('C4::Acquisition'); - use_ok('C4::Bookseller'); - use_ok('C4::Context'); - use_ok('Koha::Number::Price'); -}; - -t::lib::Mocks::mock_preference( 'gist', '0.02|0.05|0.196' ); - -my $bookseller_module = Test::MockModule->new('Koha::Acquisition::Bookseller'); - -my ( $basketno_0_0, $basketno_1_1, $basketno_1_0, $basketno_0_1 ); -my ( $invoiceid_0_0, $invoiceid_1_1, $invoiceid_1_0, $invoiceid_0_1 ); -my $today; - -for my $currency_format ( qw( US FR ) ) { - t::lib::Mocks::mock_preference( 'CurrencyFormat', $currency_format ); - subtest 'Configuration 1: 0 0' => sub { - plan tests => 7; - $bookseller_module->mock( - 'fetch', - sub { - return { listincgst => 0, invoiceincgst => 0 }; - } - ); - - my $biblionumber_0_0 = 42; - - my $order_0_0 = { - biblionumber => $biblionumber_0_0, - quantity => 2, - listprice => 82.000000, - unitprice => 73.80000, - quantityreceived => 2, - basketno => $basketno_0_0, - invoiceid => $invoiceid_0_0, - rrp => 82.00, - ecost => 73.80, - gstrate => 0.0500, - discount => 10.0000, - datereceived => $today - }; - $order_0_0 = C4::Acquisition::populate_order_with_prices( - { - order => $order_0_0, - booksellerid => 'just_something', - ordering => 1, - } - ); - - # Note that this configuration is correct \o/ - compare( - { - got => $order_0_0->{rrpgsti}, - expected => 86.10, - conf => '0 0', - field => 'rrpgsti' - } - ); - compare( - { - got => $order_0_0->{rrpgste}, - expected => 82.00, - conf => '0 0', - field => 'rrpgste' - } - ); - compare( - { - got => $order_0_0->{ecostgsti}, - expected => 77.49, - conf => '0 0', - field => 'ecostgsti' - } - ); - compare( - { - got => $order_0_0->{ecostgste}, - expected => 73.80, - conf => '0 0', - field => 'ecostgste' - } - ); - compare( - { - got => $order_0_0->{gstvalue}, - expected => 7.38, - conf => '0 0', - field => 'gstvalue' - } - ); - compare( - { - got => $order_0_0->{totalgsti}, - expected => 154.98, - conf => '0 0', - field => 'totalgsti' - } - ); - compare( - { - got => $order_0_0->{totalgste}, - expected => 147.60, - conf => '0 0', - field => 'totalgste' - } - ); - }; - - subtest 'Configuration 1: 1 1' => sub { - plan tests => 7; - $bookseller_module->mock( - 'fetch', - sub { - return { listincgst => 1, invoiceincgst => 1 }; - } - ); - - my $biblionumber_1_1 = 43; - my $order_1_1 = { - biblionumber => $biblionumber_1_1, - quantity => 2, - listprice => 82.000000, - unitprice => 73.800000, - quantityreceived => 2, - basketno => $basketno_1_1, - invoiceid => $invoiceid_1_1, - rrp => 82.00, - ecost => 73.80, - gstrate => 0.0500, - discount => 10.0000, - datereceived => $today - }; - - $order_1_1 = C4::Acquisition::populate_order_with_prices( - { - order => $order_1_1, - booksellerid => 'just_something', - ordering => 1, - } - ); - - # Note that this configuration is *not* correct - # gstvalue should be 7.03 instead of 7.02 - compare( - { - got => $order_1_1->{rrpgsti}, - expected => 82.00, - conf => '1 1', - field => 'rrpgsti' - } - ); - compare( - { - got => $order_1_1->{rrpgste}, - expected => 78.10, - conf => '1 1', - field => 'rrpgste' - } - ); - compare( - { - got => $order_1_1->{ecostgsti}, - expected => 73.80, - conf => '1 1', - field => 'ecostgsti' - } - ); - compare( - { - got => $order_1_1->{ecostgste}, - expected => 70.29, - conf => '1 1', - field => 'ecostgste' - } - ); - compare( - { - got => $order_1_1->{gstvalue}, - expected => 7.02, - conf => '1 1', - field => 'gstvalue' - } - ); - compare( - { - got => $order_1_1->{totalgsti}, - expected => 147.60, - conf => '1 1', - field => 'totalgsti' - } - ); - compare( - { - got => $order_1_1->{totalgste}, - expected => 140.58, - conf => '1 1', - field => 'totalgste' - } - ); - }; - - subtest 'Configuration 1: 1 0' => sub { - plan tests => 7; - $bookseller_module->mock( - 'fetch', - sub { - return { listincgst => 1, invoiceincgst => 0 }; - } - ); - - my $biblionumber_1_0 = 44; - my $order_1_0 = { - biblionumber => $biblionumber_1_0, - quantity => 2, - listprice => 82.000000, - unitprice => 73.804500, - quantityreceived => 2, - basketno => $basketno_1_1, - invoiceid => $invoiceid_1_1, - rrp => 82.01, - ecost => 73.80, - gstrate => 0.0500, - discount => 10.0000, - datereceived => $today - }; - - $order_1_0 = C4::Acquisition::populate_order_with_prices( - { - order => $order_1_0, - booksellerid => 'just_something', - ordering => 1, - } - ); - - # Note that this configuration is *not* correct! - # rrp gsti should be 82 (what we inserted!) - # gstvalue should be 7.03 instead of 7.02 - - compare( - { - got => $order_1_0->{rrpgsti}, - expected => 82.01, - conf => '1 0', - field => 'rrpgsti' - } - ); - compare( - { - got => $order_1_0->{rrpgste}, - expected => 78.10, - conf => '1 0', - field => 'rrpgste' - } - ); - compare( - { - got => $order_1_0->{ecostgsti}, - expected => 73.80, - conf => '1 0', - field => 'ecostgsti' - } - ); - compare( - { - got => $order_1_0->{ecostgste}, - expected => 70.29, - conf => '1 0', - field => 'ecostgste' - } - ); - compare( - { - got => $order_1_0->{gstvalue}, - expected => 7.02, - conf => '1 0', - field => 'gstvalue' - } - ); - compare( - { - got => $order_1_0->{totalgsti}, - expected => 147.60, - conf => '1 0', - field => 'totalgsti' - } - ); - compare( - { - got => $order_1_0->{totalgste}, - expected => 140.58, - conf => '1 0', - field => 'totalgste' - } - ); - }; - - subtest 'Configuration 1: 0 1' => sub { - plan tests => 7; - $bookseller_module->mock( - 'fetch', - sub { - return { listincgst => 0, invoiceincgst => 1 }; - } - ); - - my $biblionumber_0_1 = 45; - my $order_0_1 = { - biblionumber => $biblionumber_0_1, - quantity => 2, - listprice => 82.000000, - unitprice => 73.800000, - quantityreceived => 2, - basketno => $basketno_1_1, - invoiceid => $invoiceid_1_1, - rrp => 82.00, - ecost => 73.80, - gstrate => 0.0500, - discount => 10.0000, - datereceived => $today - }; - - $order_0_1 = C4::Acquisition::populate_order_with_prices( - { - order => $order_0_1, - booksellerid => 'just_something', - ordering => 1, - } - ); - - # Note that this configuration is correct \o/ - compare( - { - got => $order_0_1->{rrpgsti}, - expected => 86.10, - conf => '1 0', - field => 'rrpgsti' - } - ); - compare( - { - got => $order_0_1->{rrpgste}, - expected => 82.00, - conf => '1 0', - field => 'rrpgste' - } - ); - compare( - { - got => $order_0_1->{ecostgsti}, - expected => 77.49, - conf => '1 0', - field => 'ecostgsti' - } - ); - compare( - { - got => $order_0_1->{ecostgste}, - expected => 73.80, - conf => '1 0', - field => 'ecostgste' - } - ); - compare( - { - got => $order_0_1->{gstvalue}, - expected => 7.38, - conf => '1 0', - field => 'gstvalue' - } - ); - compare( - { - got => $order_0_1->{totalgsti}, - expected => 154.98, - conf => '1 0', - field => 'totalgsti' - } - ); - compare( - { - got => $order_0_1->{totalgste}, - expected => 147.60, - conf => '1 0', - field => 'totalgste' - } - ); - }; -} - -sub compare { - my ($params) = @_; - is( - Koha::Number::Price->new( $params->{got} )->format, - Koha::Number::Price->new( $params->{expected} )->format, -"configuration $params->{conf}: $params->{field} should be correctly calculated" - ); -} - -# format_for_editing -for my $currency_format ( qw( US FR ) ) { - t::lib::Mocks::mock_preference( 'CurrencyFormat', $currency_format ); - is( Koha::Number::Price->new( 1234567 )->format_for_editing, '1234567.00', 'format_for_editing should return unformated integer part with 2 decimals' ); - is( Koha::Number::Price->new( 1234567.89 )->format_for_editing, '1234567.89', 'format_for_editing should return unformated integer part with 2 decimals' ); -} -- 2.39.5