In the case of vendor prices don't include gst we are removing the gst from the order price, passing it to populate_order_with_prices and calculating tax on the price with tax removed! Example: Order with price of $10 and tax rate of 10% We calculate ecost/rrp at 9.09 Then calculate tax on 9.09 We should be calculating tax on $10 In addorderiso2707.pl: 270 if ( $bookseller->listincgst ) { 271 if ( $c_discount ) { 272 $orderinfo{ecost} = $price; 273 $orderinfo{rrp} = $orderinfo{ecost} / ( 1 - $c ); 274 } else { 275 $orderinfo{ecost} = $price * ( 1 - $c ); 276 $orderinfo{rrp} = $price; 277 } 278 } else { 279 if ( $c_discount ) { 280 $orderinfo{ecost} = $price / ( 1 + $orderinfo{tax_rate} ); 281 $orderinfo{rrp} = $orderinfo{ecost} / ( 1 - $c ); 282 } else { 283 $orderinfo{rrp} = $price / ( 1 + $orderinfo{tax_rate} ); 284 $orderinfo{ecost} = $orderinfo{rrp} * ( 1 - $c ); 285 } 286 } ... 296 297 %orderinfo = %{ 298 C4::Acquisition::populate_order_with_prices( 299 { 300 order => \%orderinfo, 301 booksellerid => $booksellerid, 302 ordering => 1, 303 receiving => 1, 304 } 305 ) 306 }; Then in C4 Acquisition: 2943 if ($ordering) { 2944 $order->{tax_rate_on_ordering} //= $order->{tax_rate}; 2945 if ( $bookseller->listincgst ) { 2946 # The user entered the rrp tax included 2947 $order->{rrp_tax_included} = $order->{rrp}; 2948 2949 # rrp tax excluded = rrp tax included / ( 1 + tax rate ) 2950 $order->{rrp_tax_excluded} = $order->{rrp_tax_included} / ( 1 + $order->{tax_rate_on_ordering} ); 2951 2952 # ecost tax excluded = rrp tax excluded * ( 1 - discount ) 2953 $order->{ecost_tax_excluded} = $order->{rrp_tax_excluded} * ( 1 - $discount ); 2954 2955 # ecost tax included = rrp tax included ( 1 - discount ) 2956 $order->{ecost_tax_included} = $order->{rrp_tax_included} * ( 1 - $discount ); 2957 } 2958 else { 2959 # The user entered the rrp tax excluded 2960 $order->{rrp_tax_excluded} = $order->{rrp}; 2961 2962 # rrp tax included = rrp tax excluded * ( 1 - tax rate ) 2963 $order->{rrp_tax_included} = $order->{rrp_tax_excluded} * ( 1 + $order->{tax_rate_on_ordering} ); 2964 2965 # ecost tax excluded = rrp tax excluded * ( 1 - discount ) 2966 $order->{ecost_tax_excluded} = $order->{rrp_tax_excluded} * ( 1 - $discount ); 2967 2968 # ecost tax included = rrp tax excluded * ( 1 - tax rate ) * ( 1 - discount ) 2969 $order->{ecost_tax_included} = 2970 $order->{rrp_tax_excluded} * 2971 ( 1 + $order->{tax_rate_on_ordering} ) * 2972 ( 1 - $discount ); 2973 } 2974 2975 # tax value = quantity * ecost tax excluded * tax rate 2976 $order->{tax_value_on_ordering} = 2977 $order->{quantity} * $order->{ecost_tax_excluded} * $order->{tax_rate_on_ordering}; 2978 }
Created attachment 67110 [details] [review] Bug 19296 - Allow all tax processing to happen in C4::Acquisition::populate_order_with_prices To test: 1 - Create an order file that includes prices and items (MarcFieldsToOrder) 2 - Stage the file 3 - Set vendor to 'prices exclude tax' 4 - Open a basket and add from the file 5 - View the items in the basket 6 - Prices are reduced by the tax rate and tax is calculated to return prices to the value in the file 7 - Apply patch 8 - Repeat steps 1-6 9 - Prices should now calculate correctly 10 - Repeat with 'MarcItemFieldsToOrder'
Tested both variations: list price includes GST and list price doesn't include GST. Verified bug and made sure that price calculations for ordering from staged and ordering from new (empty) record now match and make sense.
Created attachment 67822 [details] [review] Bug 19296 - Allow all tax processing to happen in C4::Acquisition::populate_order_with_prices To test: 1 - Create an order file that includes prices and items (MarcFieldsToOrder) 2 - Stage the file 3 - Set vendor to 'prices exclude tax' 4 - Open a basket and add from the file 5 - View the items in the basket 6 - Prices are reduced by the tax rate and tax is calculated to return prices to the value in the file 7 - Apply patch 8 - Repeat steps 1-6 9 - Prices should now calculate correctly 10 - Repeat with 'MarcItemFieldsToOrder' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
QA: Looking here now
Created attachment 68045 [details] [review] Bug 19296 - Allow all tax processing to happen in C4::Acquisition::populate_order_with_prices To test: 1 - Create an order file that includes prices and items (MarcFieldsToOrder) 2 - Stage the file 3 - Set vendor to 'prices exclude tax' 4 - Open a basket and add from the file 5 - View the items in the basket 6 - Prices are reduced by the tax rate and tax is calculated to return prices to the value in the file 7 - Apply patch 8 - Repeat steps 1-6 9 - Prices should now calculate correctly 10 - Repeat with 'MarcItemFieldsToOrder' Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 68046 [details] [review] Bug 19296: (QA Follow-up) Correct comment in populate_orders Just replacing a wrong - by a correct + sign in the comment. The code itself is fine. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
listprice is going to be different as well, is it expected?
(In reply to Jonathan Druart from comment #7) > listprice is going to be different as well, is it expected? I think this is correct - if list price includes GST then it should already have it, if it doesn't it doesn't need it. Looking harder I am not sure why list price isn't just passed through the form but not adding that here.
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.06.
Dependent bug is not in 16.11.x, skipping.