Bug 11755 - Argument 'booksellerid' not properly handled in orderreceive.pl
Summary: Argument 'booksellerid' not properly handled in orderreceive.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 10613
  Show dependency treegraph
 
Reported: 2014-02-13 12:40 UTC by Jacek Ablewicz
Modified: 2019-06-27 09:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11755: SearchOrders does not return booksellerid (1.83 KB, patch)
2014-03-05 11:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11755: SearchOrders does not return booksellerid (1.94 KB, patch)
2014-03-06 08:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 11755: SearchOrders does not return booksellerid (2.13 KB, patch)
2014-03-06 13:53 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 11755: SearchOrders does not return booksellerid (1.69 KB, patch)
2014-03-11 11:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11755: SearchOrders does not return booksellerid (1.75 KB, patch)
2014-03-11 15:35 UTC, Brendan Gallagher
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Ablewicz 2014-02-13 12:40:11 UTC
In orderreceive.pl, argument 'booksellerid' passed to the template is always empty (= because we don't have booksellerid field in order records); subsequently, finishreceive.pl script is also not getting this parameter (and it relies on it to do some possibly important work). This has the following consequences:

1) link to vendor in the breadcrumbs on the receiving page is not working
2) this code block (introduced by Bug 5335 - More granular VAT)

 if ( $bookseller->{listincgst} ) {
        if ( not $bookseller->{invoiceincgst} ) {
            $order->{rrp} = $order->{rrp} * ( 1 + $order->{gstrate} );
            $order->{ecost} = $order->{ecost} * ( 1 + $order->{gstrate} );
            $order->{unitprice} = $order->{unitprice} * ( 1 + $order->{gstrate} );
        }
    } else {
        if ( $bookseller->{invoiceincgst} ) {
            $order->{rrp} = $order->{rrp} / ( 1 + $order->{gstrate} );
            $order->{ecost} = $order->{ecost} / ( 1 + $order->{gstrate} );
            $order->{unitprice} = $order->{unitprice} / ( 1 + $order->{gstrate} );
        }
    }
 }

in finishreceive.pl never runs

3) in the received item records, booksellerid (952 $e) field is not updated with the correct value by ModItem(), it always becomes '' after receiving.

Regarding 2), I'm not quite sure it is neceserilly the bad thing ;). But it (partially) explains to me why tax calculations in Koha are occasionally working seemingly weird (at least for some less usual invoiceincgst & listincgst settings in vendor record).
Comment 1 Jonathan Druart 2014-03-05 11:10:59 UTC Comment hidden (obsolete)
Comment 2 Jacek Ablewicz 2014-03-05 14:04:19 UTC
(In reply to Jonathan Druart from comment #1)
> Created attachment 25834 [details] [review] [review]
> Bug 11755: SearchOrders does not return booksellerid
> 
> The SearchOrders routine should return the booksellerid.
> 
> Booksellerid was returned before bug 10723.
> 

Patch does resolve all the issues mentioned; I'm a little worried about an impact it will have on issue 2), though.. Situation regarding tax recalculation code in finishreceive.pl seems to be somehow like that:

- in 3.10 & 3.12 branches, this code block was, and still is working like intended,
- it got broken in 3.14 and up (since bug 10723) resulting in price values in all newly received orders being inconsistently saved in the database (at least for those cases where listincgst xor invoiceincgst != 1) ?

As a side effect, some bug reports currently sitting in the queue which regard various problems with tax calculations in Koha will probably need to be retested / reevaluated. Still, IMO pushing this patch first (and ASAP) will be a huge change in the right direction.

I wonder if there may be some way to retroactively correct wrongly calculated price values which may already got stored in aqorders table, for those people using 3.14 right now in production environment?
Comment 3 Jonathan Druart 2014-03-06 08:43:02 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2014-03-06 13:53:54 UTC Comment hidden (obsolete)
Comment 5 Brendan Gallagher 2014-03-11 10:19:11 UTC
Jonathan - 

tests failed.  There are a lot of new additions to the Acquisitions.t 

Besides that - the patch does do what it advertize - so once tests past :)
Comment 6 Jonathan Druart 2014-03-11 11:12:16 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2014-03-11 11:12:44 UTC
Last patch rebased.
Comment 8 Brendan Gallagher 2014-03-11 15:35:07 UTC
Created attachment 26121 [details] [review]
Bug 11755: SearchOrders does not return booksellerid

The SearchOrders routine should return the booksellerid.

Booksellerid was returned before bug 10723.

Quick test plan:
Go on orderreceive.pl and verify that the vendor link is correct.

Followed test plan. Vendor link is now correct.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 9 Galen Charlton 2014-03-11 16:21:58 UTC
Pushed to master.  Thanks, Jonathan!
Comment 10 Fridolin Somers 2014-05-27 15:53:32 UTC
Pushed to 3.14.x, will be in 3.14.07