Bug 15531 - Add support for standing orders
Summary: Add support for standing orders
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jesse Weaver
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
: 15183 (view as bug list)
Depends on:
Blocks: 17667 19340 16984
  Show dependency treegraph
 
Reported: 2016-01-08 17:43 UTC by Jesse Weaver
Modified: 2017-10-09 17:18 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15531: Add support for standing orders (27.64 KB, patch)
2016-01-27 22:57 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: (followup) Use a quantity of 1, not null, for standing orders (11.07 KB, patch)
2016-02-02 21:50 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: Add support for standing orders (27.50 KB, patch)
2016-04-18 22:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: (followup) Use a quantity of 1, not null, for standing orders (11.07 KB, patch)
2016-04-18 22:18 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: Add support for standing orders (27.58 KB, patch)
2016-04-25 14:21 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 15531: (followup) Use a quantity of 1, not null, for standing orders (11.14 KB, patch)
2016-04-25 14:21 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 15531: (QA followup) Fix several small issues (5.12 KB, patch)
2016-04-28 21:52 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: Add support for standing orders (27.50 KB, patch)
2016-04-28 21:53 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: (followup) Use a quantity of 1, not null, for standing orders (11.07 KB, patch)
2016-04-28 21:53 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15531: (QA followup) Fix several small issues (5.12 KB, patch)
2016-04-28 21:53 UTC, Jesse Weaver
Details | Diff | Splinter Review
[PASSED QA] Bug 15531: Add support for standing orders (27.57 KB, patch)
2016-04-28 22:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15531: (followup) Use a quantity of 1, not null, for standing orders (11.14 KB, patch)
2016-04-28 22:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15531: (QA followup) Fix several small issues (5.19 KB, patch)
2016-04-28 22:11 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Weaver 2016-01-08 17:43:31 UTC
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
Comment 1 Jesse Weaver 2016-01-27 22:54:21 UTC
Last item (Creation of items upon receive customizable per-basket) split off into bug 15685.
Comment 2 Jesse Weaver 2016-01-27 22:57:23 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2016-01-28 18:56:22 UTC
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
Comment 4 Jesse Weaver 2016-02-02 21:50:46 UTC Comment hidden (obsolete)
Comment 5 Aleisha Amohia 2016-02-05 00:56:15 UTC
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 :)
Comment 6 Jesse Weaver 2016-02-08 21:48:20 UTC
(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)?
Comment 7 Liz Rea 2016-02-18 21:36:06 UTC
Aleisha,

I think I know what your problem may be here, come find me next time you're in. :)

Liz
Comment 8 Jesse Weaver 2016-04-18 22:18:31 UTC Comment hidden (obsolete)
Comment 9 Jesse Weaver 2016-04-18 22:18:43 UTC Comment hidden (obsolete)
Comment 10 sandboxes@biblibre.com 2016-04-25 14:21:18 UTC
Patch tested with a sandbox, by Jonathan Field <jonathan.field@ptfs-europe.com>
Comment 11 sandboxes@biblibre.com 2016-04-25 14:21:50 UTC Comment hidden (obsolete)
Comment 12 sandboxes@biblibre.com 2016-04-25 14:21:53 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-04-25 16:28:54 UTC
(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
Comment 14 Katrin Fischer 2016-04-27 22:10:17 UTC
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!
Comment 15 Jesse Weaver 2016-04-28 21:52:50 UTC Comment hidden (obsolete)
Comment 16 Jesse Weaver 2016-04-28 21:53:29 UTC Comment hidden (obsolete)
Comment 17 Jesse Weaver 2016-04-28 21:53:33 UTC Comment hidden (obsolete)
Comment 18 Jesse Weaver 2016-04-28 21:53:38 UTC Comment hidden (obsolete)
Comment 19 Jesse Weaver 2016-04-28 21:57:15 UTC
> 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?
Comment 20 Katrin Fischer 2016-04-28 22:01:11 UTC
(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
Comment 21 Katrin Fischer 2016-04-28 22:11:20 UTC
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>
Comment 22 Katrin Fischer 2016-04-28 22:11:23 UTC
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>
Comment 23 Katrin Fischer 2016-04-28 22:11:27 UTC
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>
Comment 24 Katrin Fischer 2016-04-28 23:09:35 UTC
*** Bug 15183 has been marked as a duplicate of this bug. ***
Comment 25 Kyle M Hall 2016-04-29 13:15:39 UTC
Pushed to master for the May 2016 release! Thanks Jesse!
Comment 26 Jonathan Druart 2016-11-15 13:45:20 UTC
This patchset breaks the acquisition process if the basket is marked as standing.
See bug 16984, please fix ASAP!
Comment 27 Jonathan Druart 2016-11-22 09:29:58 UTC
(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.
Comment 28 Jonathan Druart 2016-11-22 09:31:59 UTC
When cancelling a receipt, the quantity of the original order is increased.
That does not sound correct.