In the perl script we test whether the vendor invoice prices include tax: if ( $bookseller->invoiceincgst ) { $rrp = $order->{rrp_tax_included}; $ecost = $order->{ecost_tax_included}; unless ( $unitprice != 0 and defined $unitprice) { $unitprice = $order->{ecost_tax_included}; } } else { $rrp = $order->{rrp_tax_excluded}; $ecost = $order->{ecost_tax_excluded}; unless ( $unitprice != 0 and defined $unitprice) { $unitprice = $order->{ecost_tax_excluded}; } } In the template we test whether the vendor list prices include tax: <li><label for="rrp">Retail price: </label>[% rrp | $Price %] <span class="hint">(adjusted for [% cur_active | html %], [% IF (listincgst == 1) %]tax inclusive[% ELSE %]tax exclusive[% END %])</span></li> <li> <label for="replacementprice">Replacement price:</label> <input type="text" size="20" name="replacementprice" id="replacementprice" value="[% replacementprice | $Price %]" /> </li> <li><label for="ecost">Budgeted cost: </label>[% ecost | $Price %] <span class="hint">[% IF (listincgst == 1) %](tax inclusive)[% ELSE %](tax exclusive)[% END %]</span></li> <li> <label for="unitprice">Actual cost:</label> <input type="text" size="20" name="unitprice" id="unitprice" value="[% unitprice | $Price on_editing => 1 %]" /> <span class="hint">[% IF (invoiceincgst == 1) %](tax inclusive)[% ELSE %](tax exclusive)[% END %]</span> </li>
Created attachment 84971 [details] [review] Bug 22225: Correctly use invoiceincgst over listincgst Mistakenly, we were looking at listincgst in the templates but using invoiceincgst in the scripts. This patch rectifies that by switching to use invoiceincgst in the templates too.
Can you please provide a test plan so we can easily confirm it works? Thank you
Test Plan, 1) Navigate to a vendor page (acquisitions, search for vendor, click vendor name) 2) Click 'Edit vendor' 3) Check the values of 'List prices - include/don't include tax' and 'Invoice prices - include/don't include tax' 4) Find an invoice from that vendor, check that the tax hints displayed to the right of the prices match the settings above. 5) Change the settings in step 3 and repeat step 4.
Hi, I tested this and the labels didn't seem to be correct per the vendor settings. This works correctly on current master near as I can tell. Sorry :( Liz
The issue appears during order receipt, not on the invoice view. To test: 1 - Set a vendor to: List prices - include tax Invoive prices - don't include tax 2 - Create a basket for this vendor and order an item 3 - Ensure the order has tax applied - the price you enter should include that tax 4 - Close the basket 5 - Receive shipment 6 - Select the order to receive 7 - On the receipt page note 'Retail price' and 'Budgeted cost' fields claim to be tax inclusive, but display the price adjusted to be tax exclusive 8 - In another tab edit the vendor to: List prices - don't include tax 9 - Reload receipt page, prices display correctly 10 - Edit vendor in other tab to set Invoice prices - include tax 11 - Reload receipt page - now prices include tax but hints claim they exclude tax
Created attachment 88550 [details] [review] Bug 22225: Correctly use invoiceincgst over listincgst Mistakenly, we were looking at listincgst in the templates but using invoiceincgst in the scripts. This patch rectifies that by switching to use invoiceincgst in the templates too. Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Ack, thanks both, nick for spotting it and correcting my test plan and Liz for having the patience to test it.. too work, thanks allot.
Actually... I am not sure if this is correct. If Retail price is entered (in Germany it would even be printed on the book and including tax) that should always be used and not displayed differently on different pages. How I think it should work: The setting list price should always be used for list price, budgeted cost etc. that are calculated when entering the order. The setting inovice price should be used for the actual price entered on receive. So the settings are not about the pages, but the kind of prices?
I personally feel that this patch should go through as is right now to provoke thought and allow us to clarify and resolve bugs later. This patch merely corrects the display hints to conform with the actual data that is being displayed.. to me it's a worse case left as is as it's misleading as to what data is being displayed on screen without this patch. This may provoke further thought and lead to bugfixes.
I am ok with that.
Created attachment 89614 [details] [review] Bug 22225: Correctly use invoiceincgst over listincgst Mistakenly, we were looking at listincgst in the templates but using invoiceincgst in the scripts. This patch rectifies that by switching to use invoiceincgst in the templates too. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.06
Patch does not apply on 18.05, skipping and marking resolved. :)