Bug 32167 - When adding an order from a a staged file without item fields we only add price if there is a vendor discount
Summary: When adding an order from a a staged file without item fields we only add pri...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 14680
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-10 15:24 UTC by Nick Clemens
Modified: 2022-11-23 17:32 UTC (History)
3 users (show)

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


Attachments
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor (3.39 KB, patch)
2022-11-10 19:30 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor (3.48 KB, patch)
2022-11-10 19:47 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor (3.54 KB, patch)
2022-11-18 19:22 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor (3.55 KB, patch)
2022-11-22 15:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32167: [22.05.x] (bug 14860 follow-up) Populate price fields even if no discount on vendor (3.66 KB, patch)
2022-11-23 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32167: [21.11.x] (bug 14860 follow-up) Populate price fields even if no discount on vendor (3.67 KB, patch)
2022-11-23 17:32 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-11-10 15:24:22 UTC
In addorderis02907.pl:
                my $c = $c_discount ? $c_discount : $bookseller->discount;
                $orderinfo{discount} = $c;
                if ( $c ) {
                    $orderinfo{ecost} = $c_price * ( 1 - $c / 100 );
                    $orderinfo{rrp}   = $c_price;
                }
                $orderinfo{listprice} = $orderinfo{rrp} / $active_currency->rate;


IF there is no discount, we don't set the price fields!

This combines with 32166 to end up importing blank order records
Comment 1 Nick Clemens 2022-11-10 19:30:00 UTC
Created attachment 143703 [details] [review]
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor

On bug 14860 an 'else' was removed, assuming the price fields were set before discount,
this was incorrect. This patch renames the variable and always set the price, only discounting
ecost if a discount, but setting the values even if not

To test:
 0 - Have a vendor with a discount of 0% specified (no discount)
 1 - Export a record from your Koha
 2 - Stage the record for import and match on biblionumber
 3 - Add to a basket in acq from the staged file
 4 - Select the title, and set order price to $10 and do not fill the discount field
 5 - Add the order - note $0 order line
 6 - Repeat with another vendor with a 10% discount and confirm that is correct
 7 - Apply patch
 8 - Repeat 2-4
 9 - Add the order and confirm $10 price
10 - Repeat with discounted vendor, confirm prices are set discounted correctly

https://bugs.koha-community.org/show_bug.cgi?id=14680
Comment 2 Nick Clemens 2022-11-10 19:47:51 UTC
Created attachment 143704 [details] [review]
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor

On bug 14860 an 'else' was removed, assuming the price fields were set before discount,
this was incorrect. This patch renames the variable and always set the price, only discounting
ecost if a discount, but setting the values even if not

To test:
 0 - Have a vendor with a discount of 0% specified (no discount)
 1 - Export a record from your Koha
 2 - Stage the record for import and match on biblionumber
 3 - Add to a basket in acq from the staged file
 4 - Select the title, and set order price to $10 and do not fill the discount field
 5 - Add the order - note $0 order line
 6 - Repeat with another vendor with a 10% discount and confirm that is correct
 7 - Apply patch
 8 - Repeat 2-4
 9 - Add the order and confirm $10 price
10 - Repeat with discounted vendor, confirm prices are set discounted correctly

https://bugs.koha-community.org/show_bug.cgi?id=14680
Comment 3 ByWater Sandboxes 2022-11-18 19:22:33 UTC
Created attachment 144077 [details] [review]
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor

On bug 14860 an 'else' was removed, assuming the price fields were set before discount,
this was incorrect. This patch renames the variable and always set the price, only discounting
ecost if a discount, but setting the values even if not

To test:
 0 - Have a vendor with a discount of 0% specified (no discount)
 1 - Export a record from your Koha
 2 - Stage the record for import and match on biblionumber
 3 - Add to a basket in acq from the staged file
 4 - Select the title, and set order price to $10 and do not fill the discount field
 5 - Add the order - note $0 order line
 6 - Repeat with another vendor with a 10% discount and confirm that is correct
 7 - Apply patch
 8 - Repeat 2-4
 9 - Add the order and confirm $10 price
10 - Repeat with discounted vendor, confirm prices are set discounted correctly

https://bugs.koha-community.org/show_bug.cgi?id=14680

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 4 Martin Renvoize 2022-11-22 15:55:51 UTC
Created attachment 144162 [details] [review]
Bug 32167: (bug 14860 follow-up) Populate price fields even if no discount on vendor

On bug 14860 an 'else' was removed, assuming the price fields were set before discount,
this was incorrect. This patch renames the variable and always set the price, only discounting
ecost if a discount, but setting the values even if not

To test:
 0 - Have a vendor with a discount of 0% specified (no discount)
 1 - Export a record from your Koha
 2 - Stage the record for import and match on biblionumber
 3 - Add to a basket in acq from the staged file
 4 - Select the title, and set order price to $10 and do not fill the discount field
 5 - Add the order - note $0 order line
 6 - Repeat with another vendor with a 10% discount and confirm that is correct
 7 - Apply patch
 8 - Repeat 2-4
 9 - Add the order and confirm $10 price
10 - Repeat with discounted vendor, confirm prices are set discounted correctly

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2022-11-22 15:56:25 UTC
Works as expected, code is clear.  Passing QA
Comment 6 Tomás Cohen Arazi 2022-11-22 19:40:19 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 7 Lucas Gass 2022-11-23 16:01:50 UTC
Doesn't apply cleanly to 22.05.x, no backport
Comment 8 Nick Clemens 2022-11-23 16:08:04 UTC
Created attachment 144208 [details] [review]
Bug 32167: [22.05.x] (bug 14860 follow-up) Populate price fields even if no discount on vendor

On bug 14860 an 'else' was removed, assuming the price fields were set before discount,
this was incorrect. This patch renames the variable and always set the price, only discounting
ecost if a discount, but setting the values even if not

To test:
 0 - Have a vendor with a discount of 0% specified (no discount)
 1 - Export a record from your Koha
 2 - Stage the record for import and match on biblionumber
 3 - Add to a basket in acq from the staged file
 4 - Select the title, and set order price to $10 and do not fill the discount field
 5 - Add the order - note $0 order line
 6 - Repeat with another vendor with a 10% discount and confirm that is correct
 7 - Apply patch
 8 - Repeat 2-4
 9 - Add the order and confirm $10 price
10 - Repeat with discounted vendor, confirm prices are set discounted correctly

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Lucas Gass 2022-11-23 16:25:38 UTC
Thanks Nick! This fix is backported to 22.05.x for upcoming 22.05.07 release!
Comment 10 Nick Clemens 2022-11-23 17:32:36 UTC
Created attachment 144212 [details] [review]
Bug 32167: [21.11.x] (bug 14860 follow-up) Populate price fields even if no discount on vendor

On bug 14860 an 'else' was removed, assuming the price fields were set before discount,
this was incorrect. This patch renames the variable and always set the price, only discounting
ecost if a discount, but setting the values even if not

To test:
 0 - Have a vendor with a discount of 0% specified (no discount)
 1 - Export a record from your Koha
 2 - Stage the record for import and match on biblionumber
 3 - Add to a basket in acq from the staged file
 4 - Select the title, and set order price to $10 and do not fill the discount field
 5 - Add the order - note $0 order line
 6 - Repeat with another vendor with a 10% discount and confirm that is correct
 7 - Apply patch
 8 - Repeat 2-4
 9 - Add the order and confirm $10 price
10 - Repeat with discounted vendor, confirm prices are set discounted correctly

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>