Attachment 41489 Details for Bug 14680 – Bug 14680 - when doing acquisitions from a staged file, MarcFieldsToOrder-syspref discounts are instead added.

[patch] Bug 14680 - when doing acquisitions from a staged file, MarcFieldsToOrder-syspref discounts are instead added.

Bug-14680---when-doing-acquisitions-from-a-staged-.patch (text/plain), 4.35 KB, created by Olli-Antti Kivilahti on 2015-08-14 08:34:56 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Olli-Antti Kivilahti

Created: 2015-08-14 08:34:56 UTC

Size: 4.35 KB

patch

obsolete

>From fa44e4c791dda32e1de3cce81886280feb3a0758 Mon Sep 17 00:00:00 2001
>From: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
>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 d1dd292..94a626f 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.
>@@ -175,6 +175,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') || '';
>@@ -233,19 +238,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

Actions: View | Diff | Splinter Review

Attachments on bug 14680: 41489 | 41551 | 137649 | 137650 | 137753