In the Acquisitions module : - Create a new vendor with a discount (Or select one that has a discount). Here the discount will be 25% - Create a new basket or select an existing one - Select "Add to basket" => From a staged file - Add an order from one of the file ==> Here, you can use a specific discount for the order, to override the vendor discount. The result should be quite the same if you do, but with this discount - The order is created ==> Modify the order - The discount is 0.25% instead of 25% Note that the prices are initially correctly calculated, but the discount value will appear as false. Plus, if you modify the order and change a price, the wrong discount will be used. In database, the discount is indeed stored as a value between 0 and 1, but only in that case (for the vendor, it is the actual percentage, and when you create an order from anoher source, it is stored correctly) This was reported on Koha 20.11, but is still relevant on master. In the code, this happens because the discount value is first divided by 100 to calculate the price, but it is that divided value that is stored in database, instead of the whole percentage value. On the master branch, in the file Koha/acqui/addorderiso2709.pl, the line where it's done are lines 166, 279 and 340 What I would suggest is : - Remove the division per 100 on those lines (166, 279 and 340) - Divide per 100 only when calculating the price (lines 283, 285, 344, 346) ==> ( 1 - $c/100 ) instead of ( 1 - $c ) - In the template (for importing from file), add a "%" just to be clear that the discount is a percentage, and not a value between 0 and 1 I know there was a kind of security (divide by 100 if the input is > 1), but it could be the other way around (multiply by 100 if the input is < 1). But as it is based on the field that is in the form, with a simple "%" indication it should not be needed.
Hello Christophe, that looks correct, can you provide a patch please?
Created attachment 128127 [details] [review] Patch for the bug 29607 Here is the patch I suggest for the resolution of the bug. With this, the discount is properly stored. I oversaw this but the '%' is already present in the template.
I get this message when trying to apply the patch (using koha-testing-docker) - I think that this means the patch isn't correctly formatted, but I could be wrong: git bz apply 29607 Bug 29607 - addorderiso2709: The stored discount when importing an order from a file is invalid 128127 - Patch for the bug 29607 Apply? [(y)es, (n)o, (i)nteractive] y Traceback (most recent call last): File "/usr/bin/git-bz", line 2716, in <module> applied = do_apply(bug_ref) File "/usr/bin/git-bz", line 1827, in do_apply users = bug.server.get_xmlrpc_proxy().User.get({ 'names': [patch.attacher] })['users'] File "/usr/lib/python2.7/xmlrpclib.py", line 1243, in __call__ return self.__send(self.__name, args) File "/usr/lib/python2.7/xmlrpclib.py", line 1602, in __request verbose=self.__verbose File "/usr/lib/python2.7/xmlrpclib.py", line 1283, in request return self.single_request(host, handler, request_body, verbose) File "/usr/lib/python2.7/xmlrpclib.py", line 1331, in single_request response.msg, xmlrpclib.ProtocolError: <ProtocolError for bugs.koha-community.org/xmlrpc.cgi: 404 Not Found> Testing notes ~~~~~~~~~~~~~ The QA Team will probably want the commit message formatted according to https://wiki.koha-community.org/wiki/Commit_messages I have drafted a test plan, based on the comments in the bug description. Bug 29607: Store the discount amount correctly when importing and order from a file This bug fixes how the discount amount for an order is stored, when an order is added to a basket using "From staged MARC records". Test plan: 1. Export a record (without the items) (Tools > Catalog > Export data). 2. Delete the record from the catalog (including any items). 3. Stage the record for import (Tools > Catalog > Stage MARC records for import). 4. Create a new vendor with a discount of 25%. (Or select and existing vendor that has a discount). (Acquisitions > New vendor) 5. Create a new basket for the vendor. (Acquisitions > Manage orders > search for vendors > New > Basket). 6. Add to the basket using "From a staged file": . select "Add orders" next to the staged file . select the record to add to the order . enter a price . leave the discount field blank . select a fund . select "Save" ==> The order is created! 7. Modify the order - note that the discount is showing on the form as .25% instead of 25%, also note that the discount amount is calculated correctly. 8. Check what is recorded in the database: . koha-mysql kohadev . select * from aqorders; ==> discount field for the basket item shows as 0.2500 9. Change the discount to 25%. Run step 8 again - discount amount will be correctly shown as 25.0000 10. Apply the patch. 11. Repeat steps 1-9 - discount amount is shown and calculated correctly. 12. Test modifying the discount amount - should be calculated and shown correctly. 13. Sign off! See additional comments in the bug description.
Created attachment 135538 [details] [review] Bug 29607: Store the discount amount correctly when importing and order from a file This bug fixes how the discount amount for an order is stored, when an order is added to a basket using "From staged MARC records". Test plan: 1. Export a record (without the items) (Tools > Catalog > Export data). 2. Delete the record from the catalog (including any items). 3. Stage the record for import (Tools > Catalog > Stage MARC records for import). 4. Create a new vendor with a discount of 25%. (Or select and existing vendor that has a discount). (Acquisitions > New vendor) 5. Create a new basket for the vendor. (Acquisitions > Manage orders > search for vendors > New > Basket). 6. Add to the basket using "From a staged file": . select "Add orders" next to the staged file . select the record to add to the order . enter a price . leave the discount field blank . select a fund . select "Save" ==> The order is created! 7. Modify the order - note that the discount is showing on the form as .25% instead of 25%, also note that the discount amount is calculated correctly. 8. Check what is recorded in the database: . koha-mysql kohadev . select * from aqorders; ==> discount field for the basket item shows as 0.2500 9. Change the discount to 25%. Run step 8 again - discount amount will be correctly shown as 25.0000 10. Apply the patch. 11. Repeat steps 1-9 - discount amount is shown and calculated correctly. 12. Test modifying the discount amount - should be calculated and shown correctly. 13. Sign off! See additional comments in the bug description.
Created attachment 135561 [details] [review] Bug 29607: Store the discount amount correctly when importing and order from a file This bug fixes how the discount amount for an order is stored, when an order is added to a basket using "From staged MARC records". Test plan: 1. Export a record (without the items) (Tools > Catalog > Export data). 2. Delete the record from the catalog (including any items). 3. Stage the record for import (Tools > Catalog > Stage MARC records for import). 4. Create a new vendor with a discount of 25%. (Or select and existing vendor that has a discount). (Acquisitions > New vendor) 5. Create a new basket for the vendor. (Acquisitions > Manage orders > search for vendors > New > Basket). 6. Add to the basket using "From a staged file": . select "Add orders" next to the staged file . select the record to add to the order . enter a price . leave the discount field blank . select a fund . select "Save" ==> The order is created! 7. Modify the order - note that the discount is showing on the form as .25% instead of 25%, also note that the discount amount is calculated correctly. 8. Check what is recorded in the database: . koha-mysql kohadev . select * from aqorders; ==> discount field for the basket item shows as 0.2500 9. Change the discount to 25%. Run step 8 again - discount amount will be correctly shown as 25.0000 10. Apply the patch. 11. Repeat steps 1-9 - discount amount is shown and calculated correctly. 12. Test modifying the discount amount - should be calculated and shown correctly. 13. Sign off! See additional comments in the bug description. Signed-off-by: David Nind <david@davidnind.com>
Thanks for fixing and updating the patch Jonathan!
Hi Christophe, thank you for your patch! Please add yourself as Assignee when you start working on a bug. I've fixed it here.
Created attachment 135728 [details] [review] Bug 29607: Store the discount amount correctly when importing and order from a file This bug fixes how the discount amount for an order is stored, when an order is added to a basket using "From staged MARC records". Test plan: 1. Export a record (without the items) (Tools > Catalog > Export data). 2. Delete the record from the catalog (including any items). 3. Stage the record for import (Tools > Catalog > Stage MARC records for import). 4. Create a new vendor with a discount of 25%. (Or select and existing vendor that has a discount). (Acquisitions > New vendor) 5. Create a new basket for the vendor. (Acquisitions > Manage orders > search for vendors > New > Basket). 6. Add to the basket using "From a staged file": . select "Add orders" next to the staged file . select the record to add to the order . enter a price . leave the discount field blank . select a fund . select "Save" ==> The order is created! 7. Modify the order - note that the discount is showing on the form as .25% instead of 25%, also note that the discount amount is calculated correctly. 8. Check what is recorded in the database: . koha-mysql kohadev . select * from aqorders; ==> discount field for the basket item shows as 0.2500 9. Change the discount to 25%. Run step 8 again - discount amount will be correctly shown as 25.0000 10. Apply the patch. 11. Repeat steps 1-9 - discount amount is shown and calculated correctly. 12. Test modifying the discount amount - should be calculated and shown correctly. 13. Sign off! See additional comments in the bug description. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.03
applied to 21.11 for 21.11.19
Not backported to 21.05.x