From 1014f1580746ef2e7a94f58423fa9d64632e6e81 Mon Sep 17 00:00:00 2001 From: Olli-Antti Kivilahti Date: Fri, 14 Aug 2015 11:32:17 +0300 Subject: [PATCH] Bug 14680 - when doing acquisitions from a staged file, MarcFieldsToOrder-syspref discounts are instead added. Using the syspref 'MarcFieldsToOrder', getting the discount directly from the MARC causes the price to actually increase by the discount percentage, not decrease, if the Vendor/Bookseller is configured to include the VAT/GST in the list price. REPLICATE ISSUE: -1. Define 'MarcFieldsToOrder'-syspref to match the discount and price columns of your staged MARC Records. 0. Stage a MARC batch. 1. Make and acquisition from the staged file using /acqui/addorderiso2709.pl (Save the big staging list, don't click "Add order" on an individual Biblio.) 2. Observe that the Replacement price (rrp) is actually the (price + discount) instead of (price - discount). And the estimated cost is just the price in the MARC Record without any discount. AFTER THIS PATCH: Replay step 1. 2. rrp is now the price in the MARC Record incl. VAT/GST. ecost is now the VAT/GST included discounted price. NOTE! Because the subroutine I modified doesn't make much sense, it is presumed that there can be regression in other parts for other kinds of vendor configurations. Because the acquisitions module is in urgent need of complete back-end refactoring + PageObject/Cucumber regression test coverage, no further investigation is made to resolve those. --- acqui/addorderiso2709.pl | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/acqui/addorderiso2709.pl b/acqui/addorderiso2709.pl index b809fb9..971c9aa 100755 --- a/acqui/addorderiso2709.pl +++ b/acqui/addorderiso2709.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl +#!/usr/bin/perl -d #A script that lets the user populate a basket from an iso2709 file #the script first displays a list of import batches, then when a batch is selected displays all the biblios in it. @@ -178,6 +178,11 @@ if ($op eq ""){ my $c_quantity = shift( @quantities ) || GetMarcQuantity($marcrecord, C4::Context->preference('marcflavour') ) || 1; my $c_budget_id = shift( @budgets_id ) || $input->param('all_budget_id') || $budget_id; my $c_discount = shift ( @discount); + if ($c_discount) { + $c_discount =~ s/%//g; #This screws up number conversion badly + $c_discount =~ s/,/./g; #Make this an actual digit for Perl + $c_discount = $c_discount / 100 if $c_discount >= 1; + } $c_discount = $c_discount / 100 if $c_discount > 1; my $c_sort1 = shift( @sort1 ) || $input->param('all_sort1') || ''; my $c_sort2 = shift( @sort2 ) || $input->param('all_sort2') || ''; @@ -237,19 +242,19 @@ if ($op eq ""){ my $c = $c_discount ? $c_discount : $bookseller->{discount} / 100; if ( $bookseller->{listincgst} ) { if ( $c_discount ) { - $orderinfo{ecost} = $price; - $orderinfo{rrp} = $orderinfo{ecost} / ( 1 - $c ); + $orderinfo{ecost} = $price * ( 1 - $c ); #Get the VAT included discounted price + $orderinfo{rrp} = $price; #Replacement price is the non-discounted price. Otherwise our patrons can start making profit by stealing books. } else { - $orderinfo{ecost} = $price * ( 1 - $c ); + $orderinfo{ecost} = $price; $orderinfo{rrp} = $price; } } else { if ( $c_discount ) { - $orderinfo{ecost} = $price / ( 1 + $orderinfo{gstrate} ); - $orderinfo{rrp} = $orderinfo{ecost} / ( 1 - $c ); + $orderinfo{ecost} = $price / ( 1 + $orderinfo{gstrate} ) * ( 1 - $c ); #Add VAT/GST and the discount + $orderinfo{rrp} = $price / ( 1 + $orderinfo{gstrate} ); #Add the VAT } else { $orderinfo{rrp} = $price / ( 1 + $orderinfo{gstrate} ); - $orderinfo{ecost} = $orderinfo{rrp} * ( 1 - $c ); + $orderinfo{ecost} = $price / ( 1 + $orderinfo{gstrate} ); } } $orderinfo{listprice} = $orderinfo{rrp} / $cur->{rate}; -- 1.9.1