A new kind of standing order basket, with the following behavior: * Items can be received without the basket being closed. * Items can be received without their price being known in advance. * Creation of items upon receive customizable per-basket
Last item (Creation of items upon receive customizable per-basket) split off into bug 15685.
Created attachment 47367 [details] [review] Bug 15531: Add support for standing orders This allows creation of special baskets that include standing orders. These orders do not have a known quantity (and may not have a known price in advance). Upon receipt, the received items are split into a new completed order. Test plan: 1) Run updatedatabase.pl. 2) Run prove t/db_dependent/Acquisition/StandingOrders.t . (and the other Acquisition tests). 3) Create a new basket, mark it as a standing order basket. 4) Add an order to this basket, and notice that the quantity field is missing (and thus not required). 5) Receive items for this order, and notice that the original order is unchanged. The new child order line should have the correct price and quantity information. (Note: the QA tools output what seems to be a spurious spelling error for Test::More's "isnt" in StandingOrders.t.)
Couple issues right off the bat: - When adding an order to a basket I get the form to add an item, but when I click 'Add item' the form disappears and no item is added - If I put a cost in, I get NaN for total since there is no quantity
Created attachment 47569 [details] [review] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders This seems to cause fewer problems with the existing acquisitions code. Test plan is unchanged.
Tests failed: StandingOrders.t CancelReceipt.t OrderUsers.t Also I can still see the quantity field there, just can't change it Otherwise everything else seems to work well and will be happy to sign off once those few things are fixed up :)
(In reply to Aleisha Amohia from comment #5) > Tests failed: > StandingOrders.t > CancelReceipt.t > OrderUsers.t > > Also I can still see the quantity field there, just can't change it > > Otherwise everything else seems to work well and will be happy to sign off > once those few things are fixed up :) Could you please post the full test output (using prove -v)?
Aleisha, I think I know what your problem may be here, come find me next time you're in. :) Liz
Created attachment 50365 [details] [review] Bug 15531: Add support for standing orders This allows creation of special baskets that include standing orders. These orders do not have a known quantity (and may not have a known price in advance). Upon receipt, the received items are split into a new completed order. Test plan: 1) Run updatedatabase.pl. 2) Run prove t/db_dependent/Acquisition/StandingOrders.t . (and the other Acquisition tests). 3) Create a new basket, mark it as a standing order basket. 4) Add an order to this basket, and notice that the quantity field is missing (and thus not required). 5) Receive items for this order, and notice that the original order is unchanged. The new child order line should have the correct price and quantity information. (Note: the QA tools output what seems to be a spurious spelling error for Test::More's "isnt" in StandingOrders.t.)
Created attachment 50366 [details] [review] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders This seems to cause fewer problems with the existing acquisitions code. Rebased, please test.
Patch tested with a sandbox, by Jonathan Field <jonathan.field@ptfs-europe.com>
Created attachment 50636 [details] [review] Bug 15531: Add support for standing orders This allows creation of special baskets that include standing orders. These orders do not have a known quantity (and may not have a known price in advance). Upon receipt, the received items are split into a new completed order. Test plan: 1) Run updatedatabase.pl. 2) Run prove t/db_dependent/Acquisition/StandingOrders.t . (and the other Acquisition tests). 3) Create a new basket, mark it as a standing order basket. 4) Add an order to this basket, and notice that the quantity field is missing (and thus not required). 5) Receive items for this order, and notice that the original order is unchanged. The new child order line should have the correct price and quantity information. (Note: the QA tools output what seems to be a spurious spelling error for Test::More's "isnt" in StandingOrders.t.) Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
Created attachment 50637 [details] [review] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders This seems to cause fewer problems with the existing acquisitions code. Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com>
(In reply to Jesse Weaver from comment #2) > (Note: the QA tools output what seems to be a spurious spelling error > for Test::More's "isnt" in StandingOrders.t.) Update it :) See bug 16003
1) Database update: can you please include an AFTER there to ensure that the column always ends up in the same spot? 2)Add order: "Orders are standing:" basket summary: "Is standing order basket" Non-native speaker :), but wondering if 'Standing order:' would work too? Might cause less confusion in translation. 3) When ordering from a staged file using the checkboxes, it seems that an item is generated, while this doesn't happen when using one of the other order options. This might be something to look into - what's the intended behaviour when AcqCreateItem is set to 'placing an order'? I think to not overcomplicate things for now I think it would be ok to say items for standing orders have to be created separately. Later on, optionally creating items on order/receive would be nice. 4) If you edit the order after creating it - the checkbox is unchecked even if your basket 'is_standing'. 5) The order status of the order lines will remain 'new' (usually means: not send to the vendor, unfinished) as the basket is never closed, which would set it to 'ordered'. Should we use a new order status here? I think it would be good for searching and to help avoid confusion. Maybe 'standing'? Otherwise this seems to work pretty well - with serial subscription orders too! Please take a look asap!
Created attachment 50955 [details] [review] Bug 15531: (QA followup) Fix several small issues * Add `AFTER` to DB update * Change "Is standing order basket:" to "Orders are standing:" * Disable item creation when adding from basket * Correctly show is_standing for existing baskets
Created attachment 50956 [details] [review] Bug 15531: Add support for standing orders This allows creation of special baskets that include standing orders. These orders do not have a known quantity (and may not have a known price in advance). Upon receipt, the received items are split into a new completed order. Test plan: 1) Run updatedatabase.pl. 2) Run prove t/db_dependent/Acquisition/StandingOrders.t . (and the other Acquisition tests). 3) Create a new basket, mark it as a standing order basket. 4) Add an order to this basket, and notice that the quantity field is missing (and thus not required). 5) Receive items for this order, and notice that the original order is unchanged. The new child order line should have the correct price and quantity information. (Note: the QA tools output what seems to be a spurious spelling error for Test::More's "isnt" in StandingOrders.t.)
Created attachment 50957 [details] [review] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders This seems to cause fewer problems with the existing acquisitions code.
Created attachment 50958 [details] [review] Bug 15531: (QA followup) Fix several small issues * Add `AFTER` to DB update * Change "Is standing order basket:" to "Orders are standing:" * Disable item creation when adding from basket * Correctly show is_standing for existing baskets
> 5) The order status of the order lines will remain 'new' (usually means: not > send to the vendor, unfinished) as the basket is never closed, which would > set it to 'ordered'. Should we use a new order status here? I think it would > be good for searching and to help avoid confusion. Maybe 'standing'? > I think this is a legitimate possible point of confusion, but the current approach shouldn't cause any issues with workflow, and changing this patch and the rest of acq to work with a new order status could be quite tricky. Something to approach as a future enhancement?
(In reply to Jesse Weaver from comment #19) > > 5) The order status of the order lines will remain 'new' (usually means: not > > send to the vendor, unfinished) as the basket is never closed, which would > > set it to 'ordered'. Should we use a new order status here? I think it would > > be good for searching and to help avoid confusion. Maybe 'standing'? > > > > I think this is a legitimate possible point of confusion, but the current > approach shouldn't cause any issues with workflow, and changing this patch > and the rest of acq to work with a new order status could be quite tricky. > Something to approach as a future enhancement? OK
Created attachment 50960 [details] [review] [PASSED QA] Bug 15531: Add support for standing orders This allows creation of special baskets that include standing orders. These orders do not have a known quantity (and may not have a known price in advance). Upon receipt, the received items are split into a new completed order. Test plan: 1) Run updatedatabase.pl. 2) Run prove t/db_dependent/Acquisition/StandingOrders.t . (and the other Acquisition tests). 3) Create a new basket, mark it as a standing order basket. 4) Add an order to this basket, and notice that the quantity field is missing (and thus not required). 5) Receive items for this order, and notice that the original order is unchanged. The new child order line should have the correct price and quantity information. (Note: the QA tools output what seems to be a spurious spelling error for Test::More's "isnt" in StandingOrders.t.) Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 50961 [details] [review] [PASSED QA] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders This seems to cause fewer problems with the existing acquisitions code. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 50962 [details] [review] [PASSED QA] Bug 15531: (QA followup) Fix several small issues * Add `AFTER` to DB update * Change "Is standing order basket:" to "Orders are standing:" * Disable item creation when adding from a staged file * Correctly show is_standing for existing baskets Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
*** Bug 15183 has been marked as a duplicate of this bug. ***
Pushed to master for the May 2016 release! Thanks Jesse!
This patchset breaks the acquisition process if the basket is marked as standing. See bug 16984, please fix ASAP!
(In reply to Jonathan Druart from comment #26) > This patchset breaks the acquisition process if the basket is marked as > standing. > See bug 16984, please fix ASAP! This does not break acquisition process but just raise the JS error. Does not seem critical.
When cancelling a receipt, the quantity of the original order is increased. That does not sound correct.