Bug 15685 - Allow creation of items (AcqCreateItem) to be customizable per-basket
Summary: Allow creation of items (AcqCreateItem) to be customizable per-basket
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 19468
  Show dependency treegraph
 
Reported: 2016-01-27 22:53 UTC by Jesse Weaver
Modified: 2017-10-15 08:37 UTC (History)
9 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 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (29.01 KB, patch)
2016-02-08 23:01 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.03 KB, patch)
2016-09-01 15:19 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.17 KB, patch)
2016-09-01 17:39 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.51 KB, patch)
2016-11-09 13:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (29.83 KB, patch)
2016-11-09 16:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.01 KB, patch)
2016-11-09 16:52 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.13 KB, patch)
2016-11-10 13:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.13 KB, patch)
2016-11-10 14:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order (2.51 KB, patch)
2016-11-10 14:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Bug qa script errors (1010 bytes, patch)
2016-11-10 14:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 15685 - QA Followup (5.52 KB, patch)
2017-07-07 14:55 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.37 KB, patch)
2017-07-07 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order (2.51 KB, patch)
2017-07-07 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Bug qa script errors (1010 bytes, patch)
2017-07-07 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 - QA Followup (5.52 KB, patch)
2017-07-07 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.23 KB, patch)
2017-07-19 17:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order (2.51 KB, patch)
2017-07-19 17:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Bug qa script errors (1010 bytes, patch)
2017-07-19 17:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 - QA Followup (5.52 KB, patch)
2017-07-19 17:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.23 KB, patch)
2017-08-11 12:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order (2.51 KB, patch)
2017-08-11 12:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 [QA Followup] - Bug qa script errors (1010 bytes, patch)
2017-08-11 12:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 - QA Followup (5.52 KB, patch)
2017-08-11 12:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 - Schema changes (1.70 KB, patch)
2017-08-11 12:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685 - QA followup - fix typo (988 bytes, patch)
2017-08-11 12:01 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED OFF] Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.29 KB, patch)
2017-08-16 15:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order (2.58 KB, patch)
2017-08-16 15:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 15685 [QA Followup] - Bug qa script errors (1.05 KB, patch)
2017-08-16 15:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 15685 - QA Followup (5.59 KB, patch)
2017-08-16 15:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 15685 - Schema changes (1.77 KB, patch)
2017-08-16 15:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED OFF] Bug 15685 - QA followup - fix typo (1.03 KB, patch)
2017-08-16 15:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.34 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: Implement needed changes in Koha::Edifact::Order (2.61 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: (QA followup) Fix qa script errors (1.10 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: QA Followup (5.79 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: Schema changes (1.82 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: (QA followup) Fix typo (1.07 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: (followup) K:A:O->find and ->fetch are no longer used (2.85 KB, patch)
2017-10-05 15:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: (QA followup) Tidy tests (4.76 KB, patch)
2017-10-05 15:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: POD (1.94 KB, patch)
2017-10-05 15:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: Add tests for Koha::Acquisition::Order->basket (2.75 KB, patch)
2017-10-05 15:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (30.21 KB, patch)
2017-10-06 10:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Implement needed changes in Koha::Edifact::Order (2.60 KB, patch)
2017-10-06 10:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: (QA followup) Fix qa script errors (1.09 KB, patch)
2017-10-06 10:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: QA Followup (5.76 KB, patch)
2017-10-06 10:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Schema changes (1.81 KB, patch)
2017-10-06 10:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: (QA followup) Fix typo (1.06 KB, patch)
2017-10-06 10:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: (followup) K:A:O->find and ->fetch are no longer used (2.89 KB, patch)
2017-10-06 10:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: (QA followup) Tidy tests (4.80 KB, patch)
2017-10-06 10:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: POD (1.98 KB, patch)
2017-10-06 10:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Add tests for Koha::Acquisition::Order->basket (2.80 KB, patch)
2017-10-06 10:57 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Followup - address QA issues (9.93 KB, patch)
2017-10-10 15:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket (28.01 KB, patch)
2017-10-10 20:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: Implement needed changes in Koha::Edifact::Order (2.61 KB, patch)
2017-10-10 20:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: (QA followup) Fix qa script errors (1.10 KB, patch)
2017-10-10 20:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: QA Followup (5.79 KB, patch)
2017-10-10 20:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: (QA followup) Fix typo (1.07 KB, patch)
2017-10-10 20:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: (followup) K:A:O->find and ->fetch are no longer used (2.91 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: (QA followup) Tidy tests (4.81 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: POD (1.84 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: Add tests for Koha::Acquisition::Order->basket (2.81 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: Followup - address QA issues (9.98 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: Fix DB columns order (2.05 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: DBIC Schema changes (1.68 KB, patch)
2017-10-10 20:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: Avoid "Use of uninitialized value" warning in logs (800 bytes, patch)
2017-10-10 21:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15685: Send the basket to _cancel_items_receipt (2.77 KB, patch)
2017-10-10 21:17 UTC, Jonathan Druart
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-27 22:53:55 UTC

    
Comment 1 Jesse Weaver 2016-02-08 23:01:14 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2016-09-01 15:19:48 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2016-09-01 17:39:54 UTC
Created attachment 55096 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.
Comment 4 J Schmidt 2016-09-02 18:59:36 UTC
I used PTFS test set up to create test budget; test Ebsco vendor and baskets. The System Preference by default for AcqCreateItem is set to create an item when "placing an order"...mainpage says updgrade to 16.05 done....

Test 1. 2016_0902 basket had the basket's Create items when set to "when placing an order". Results: Success in terms of when creating a new item is prompted correctly upon adding a title to a new basket. This is the same as the default system preference, however....

Test 2. Basket's 'create items when' option is set to make an item when "receiving an order"... Results: Fail. I tried to skip adding an item record at the point of adding a new title to a test basket. I went to Accounting details to put in order record information. However, I was blocked on the QUANTITY field. It stayed at zero and didn't allow for a numeral (e.g., 1, 2). Once I click Add item record above that area, then the Quantity field gets filled in as '1' and I can save the newly ordered title. 
I closed the 2016_0902Test2 basket and hit receive shipment. I receive one title in the basket.... Software error screen appears....Can't locate object method "find" via package "Koha::Acquisition::Order" at /home/koha/src/acqui/orderreceive.pl line 113.
I hit F5, refresh, then try to click text link to receive other title. Same thing.

Test 3. New test basket has the 'create items when' option set to 'cataloging the record'... Results: same thing happened as in test 2. I next close that basket. Receive shipment on 89.00 dollar book. Software error. Can't locate object method "find" via package "Koha::Acquisition::Order" at /home/koha/src/acqui/orderreceive.pl line 113. 

Seems to fail but I'll try another sandbox soon. Hesistant to change the status yet in case my test plan was off. jls.
Comment 5 J Schmidt 2016-09-02 19:01:33 UTC
(In reply to J Schmidt from comment #4)
> I used PTFS test set up to create test budget; test Ebsco vendor and
> baskets. The System Preference by default for AcqCreateItem is set to create
> an item when "placing an order"...mainpage says updgrade to 16.05 done....
> 
> Test 1. 2016_0902 basket had the basket's Create items when set to "when
> placing an order". Results: Success in terms of when creating a new item is
> prompted correctly upon adding a title to a new basket. This is the same as
> the default system preference, however....
> 
> Test 2. Basket's 'create items when' option is set to make an item when
> "receiving an order"... Results: Fail. I tried to skip adding an item record
> at the point of adding a new title to a test basket. I went to Accounting
> details to put in order record information. However, I was blocked on the
> QUANTITY field. It stayed at zero and didn't allow for a numeral (e.g., 1,
> 2). Once I click Add item record above that area, then the Quantity field
> gets filled in as '1' and I can save the newly ordered title. 
> I closed the 2016_0902Test2 basket and hit receive shipment. I receive one
> title in the basket.... Software error screen appears....Can't locate object
> method "find" via package "Koha::Acquisition::Order" at
> /home/koha/src/acqui/orderreceive.pl line 113.
> I hit F5, refresh, then try to click text link to receive other title. Same
> thing.
> 
> Test 3. New test basket has the 'create items when' option set to
> 'cataloging the record'... Results: same thing happened as in test 2. I next
> close that basket. Receive shipment on 89.00 dollar book. Software error.
> Can't locate object method "find" via package "Koha::Acquisition::Order" at
> /home/koha/src/acqui/orderreceive.pl line 113. 
> 
> Seems to fail but I'll try another sandbox soon. Hesistant to change the
> status yet in case my test plan was off. jls.

In each case I had to create a brief, new bib record. couldn't locate a biblionumber from test database. That would be a better thing to use upon adding new titles to a basket....
Comment 6 Nick Clemens 2016-09-14 19:50:59 UTC
Hi Jenny,

I think the problem may have been that the schema needs to be updated in order to fully test this patch. 

I had similar results to yours, but after updating the schema (not possible on a sandbox) things worked correctly

-Nick
Comment 7 J Schmidt 2016-09-15 18:23:41 UTC
Okay. I'll work on another bug then based on what you are sharing here.. Thanks.
Comment 8 delaye 2016-10-11 10:18:41 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2016-10-16 11:45:34 UTC
Would love to see this happen.
Comment 10 Nick Clemens 2016-11-09 13:58:58 UTC
Created attachment 57390 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.
Comment 11 Nick Clemens 2016-11-09 16:22:00 UTC
Created attachment 57392 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.
Comment 12 Nick Clemens 2016-11-09 16:52:09 UTC
Created attachment 57395 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.
Comment 13 Kyle M Hall 2016-11-10 13:48:31 UTC
Created attachment 57424 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>
Comment 14 Kyle M Hall 2016-11-10 13:50:00 UTC
Patch set doesn't take Edifact ordering into account.
Comment 15 Kyle M Hall 2016-11-10 14:07:54 UTC
Created attachment 57425 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>
Comment 16 Kyle M Hall 2016-11-10 14:08:07 UTC
Created attachment 57426 [details] [review]
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order
Comment 17 Kyle M Hall 2016-11-10 14:08:10 UTC
Created attachment 57427 [details] [review]
Bug 15685 [QA Followup] - Bug qa script errors
Comment 18 Jonathan Druart 2016-11-30 09:33:37 UTC
Tests are missing.
Comment 19 Kyle M Hall 2017-04-14 11:14:00 UTC
Comment on attachment 57425 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

Review of attachment 57425 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/kohastructure.sql
@@ +2964,4 @@
>    `basketgroupid` int(11), -- links this basket to its group (aqbasketgroups.id)
>    `deliveryplace` varchar(10) default NULL, -- basket delivery place
>    `billingplace` varchar(10) default NULL, -- basket billing place
> +  create_items ENUM('ordering', 'receiving', 'cataloguing') default NULL; -- when items should be created for orders in this basket

The semicolon needs to be a comma, and an atomicupdate needs to be added.
Comment 20 Nick Clemens 2017-07-07 14:55:34 UTC
Created attachment 64918 [details] [review]
Bug 15685 - QA Followup

DB revision fixes
Unit tests
Comment 21 Nick Clemens 2017-07-07 14:56:09 UTC
Created attachment 64919 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>
Comment 22 Nick Clemens 2017-07-07 14:56:14 UTC
Created attachment 64920 [details] [review]
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order
Comment 23 Nick Clemens 2017-07-07 14:56:18 UTC
Created attachment 64921 [details] [review]
Bug 15685 [QA Followup] - Bug qa script errors
Comment 24 Nick Clemens 2017-07-07 14:56:21 UTC
Created attachment 64922 [details] [review]
Bug 15685 - QA Followup

DB revision fixes
Unit tests
Comment 25 Katrin Fischer 2017-07-10 06:22:14 UTC
Hi, very interested to see this one make it in. Took a look at the code and noticed some merge markers were left in:

b/acqui/finishreceive.pl
+<<<<<<< HEAD
 $unitprice = Koha::Number::Price->new( $unitprice )->unformat();
 
 #need old receivedate if we update the order, parcel.pl only shows the right parcel this way FIXME
+=======
Comment 26 Nick Clemens 2017-07-19 17:59:00 UTC
Created attachment 65119 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>
Comment 27 Nick Clemens 2017-07-19 17:59:04 UTC
Created attachment 65120 [details] [review]
Bug 15685 [QA Followup] - Implement needed changes in Koha::Edifact::Order
Comment 28 Nick Clemens 2017-07-19 17:59:08 UTC
Created attachment 65121 [details] [review]
Bug 15685 [QA Followup] - Bug qa script errors
Comment 29 Nick Clemens 2017-07-19 17:59:18 UTC
Created attachment 65122 [details] [review]
Bug 15685 - QA Followup

DB revision fixes
Unit tests
Comment 30 Katrin Fischer 2017-08-11 09:23:43 UTC
Still trying to give this a second sign-off as I think this will make life a lot easier. Sadly, I ran into a big problem:

AcqCreateItem is set to: on order

1) The method create_items is not covered by tests!
- Create a new basket, choose: on receive
- Add to basket by searching for an existing record in your catalog
- Click order on one of the results
=> Internal server error, translates to: The method create_items is not covered by tests!

Can't test further from here :(

2) The test file should not be named Basket.pm, but Basket.t :)

3) Patch reintroduced a typo (not really blocking)
-#need old receivedate if we update the order, parcel.pl only shows the right parcel this way FIXME
+#need old recievedate if we update the order, parcel.pl only shows the right parcel this way FIXME


Tiny stuff (definitely non-blocker, maybe for later):
- Baskets created before the introduction of the feature don't show the information about item creation on the basket summary page
- Editing a basket after inititally creating it, doesn't offer the option to change. This makes a lot of sense, but maybe it would be good to display the selection as information (not changeable).
Comment 31 Nick Clemens 2017-08-11 12:01:06 UTC Comment hidden (obsolete)
Comment 32 Nick Clemens 2017-08-11 12:01:11 UTC Comment hidden (obsolete)
Comment 33 Nick Clemens 2017-08-11 12:01:16 UTC Comment hidden (obsolete)
Comment 34 Nick Clemens 2017-08-11 12:01:20 UTC Comment hidden (obsolete)
Comment 35 Nick Clemens 2017-08-11 12:01:24 UTC Comment hidden (obsolete)
Comment 36 Nick Clemens 2017-08-11 12:01:28 UTC Comment hidden (obsolete)
Comment 37 Katrin Fischer 2017-08-16 15:22:03 UTC Comment hidden (obsolete)
Comment 38 Katrin Fischer 2017-08-16 15:22:07 UTC Comment hidden (obsolete)
Comment 39 Katrin Fischer 2017-08-16 15:22:11 UTC Comment hidden (obsolete)
Comment 40 Katrin Fischer 2017-08-16 15:22:16 UTC Comment hidden (obsolete)
Comment 41 Katrin Fischer 2017-08-16 15:22:20 UTC Comment hidden (obsolete)
Comment 42 Katrin Fischer 2017-08-16 15:22:24 UTC Comment hidden (obsolete)
Comment 43 Katrin Fischer 2017-08-16 15:24:51 UTC
Absolutely love the feauture!
This makes acquisitions so much more flexible especially for all the cases where you don't want to create items, like for electronic resources. Can't want to get my hands on it.
Comment 44 Tomás Cohen Arazi 2017-10-05 15:25:16 UTC
Created attachment 67638 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 45 Tomás Cohen Arazi 2017-10-05 15:25:23 UTC
Created attachment 67639 [details] [review]
Bug 15685: Implement needed changes in Koha::Edifact::Order

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 46 Tomás Cohen Arazi 2017-10-05 15:25:29 UTC
Created attachment 67640 [details] [review]
Bug 15685: (QA followup) Fix qa script errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 47 Tomás Cohen Arazi 2017-10-05 15:25:35 UTC
Created attachment 67641 [details] [review]
Bug 15685: QA Followup

DB revision fixes
Unit tests

Edit: fixed the update step description (tcohen)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 48 Tomás Cohen Arazi 2017-10-05 15:25:41 UTC
Created attachment 67642 [details] [review]
Bug 15685: Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 49 Tomás Cohen Arazi 2017-10-05 15:25:47 UTC
Created attachment 67643 [details] [review]
Bug 15685: (QA followup) Fix typo

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 50 Tomás Cohen Arazi 2017-10-05 15:25:54 UTC
Created attachment 67644 [details] [review]
Bug 15685: (followup) K:A:O->find and ->fetch are no longer used

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 51 Tomás Cohen Arazi 2017-10-05 15:26:01 UTC
Created attachment 67645 [details] [review]
Bug 15685: (QA followup) Tidy tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 52 Tomás Cohen Arazi 2017-10-05 15:26:08 UTC
Created attachment 67646 [details] [review]
Bug 15685: POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 53 Tomás Cohen Arazi 2017-10-05 15:26:15 UTC
Created attachment 67647 [details] [review]
Bug 15685: Add tests for Koha::Acquisition::Order->basket

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 54 Tomás Cohen Arazi 2017-10-05 15:29:50 UTC
I've rebased the patches and checked everyting is ok regarding the changes introduced by bug 19256 in Koha::Acquisition::Order(s).

Added tests for Koha::Acquisition::Order::basket (found a bug in the meantime).

Please re-test.
Comment 55 Nick Clemens 2017-10-06 10:56:39 UTC
Created attachment 67689 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 56 Nick Clemens 2017-10-06 10:56:43 UTC
Created attachment 67690 [details] [review]
Bug 15685: Implement needed changes in Koha::Edifact::Order

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 57 Nick Clemens 2017-10-06 10:56:47 UTC
Created attachment 67691 [details] [review]
Bug 15685: (QA followup) Fix qa script errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 58 Nick Clemens 2017-10-06 10:56:52 UTC
Created attachment 67692 [details] [review]
Bug 15685: QA Followup

DB revision fixes
Unit tests

Edit: fixed the update step description (tcohen)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 59 Nick Clemens 2017-10-06 10:56:56 UTC
Created attachment 67693 [details] [review]
Bug 15685: Schema changes

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 60 Nick Clemens 2017-10-06 10:56:59 UTC
Created attachment 67694 [details] [review]
Bug 15685: (QA followup) Fix typo

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 61 Nick Clemens 2017-10-06 10:57:04 UTC
Created attachment 67695 [details] [review]
Bug 15685: (followup) K:A:O->find and ->fetch are no longer used

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 62 Nick Clemens 2017-10-06 10:57:09 UTC
Created attachment 67696 [details] [review]
Bug 15685: (QA followup) Tidy tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 63 Nick Clemens 2017-10-06 10:57:12 UTC
Created attachment 67697 [details] [review]
Bug 15685: POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 64 Nick Clemens 2017-10-06 10:57:16 UTC
Created attachment 67698 [details] [review]
Bug 15685: Add tests for Koha::Acquisition::Order->basket

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 65 Jonathan Druart 2017-10-09 20:49:35 UTC
Some adjustments are required:
1. Update DB entry must use column_exists()
2. t/db_dependent/Acquisition/CancelReceipt.t and t/db_dependent/Acquisition/TransferOrder.t are failing with
  Can't use string ("Koha::Acquisition::Order") as a HASH ref while "strict refs" in use at /home/vagrant/kohaclone/Koha/Object.pm line 275.
3. Koha::EDI and circ/reserveratios.pl are still using AcqCreateItem from the pref
4. On basketheader.pl?booksellerid=1&op=add_form, the values for the dropdown "Create items when:" ended with a '.'
I'd say it should be removed or moved out the options.
5. This looks like a typo (basketheader.tt):
-                        <input type="hidden" name="is_an_edit" value="1" />
+                        <input type="hidden" name="rs_an_edit" value="1" />
6. Koha::Acquisition::Order->basket must use DBIx::Class rs
7. Double check the code in addorderiso2709, it is buggy (GetBasket and Koha::Baskets->find calls)
Can't call method "effective_create_items" on unblessed reference at /home/vagrant/kohaclone/acqui/addorderiso2709.pl line 107.
8. Change in acqui/basket.pl is not needed (basket passed twice to the template)
Comment 66 Nick Clemens 2017-10-10 15:50:03 UTC
Created attachment 67884 [details] [review]
Bug 15685: Followup - address QA issues
Comment 67 Jonathan Druart 2017-10-10 20:47:17 UTC
Created attachment 67898 [details] [review]
Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket

This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 68 Jonathan Druart 2017-10-10 20:47:25 UTC
Created attachment 67899 [details] [review]
Bug 15685: Implement needed changes in Koha::Edifact::Order

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 69 Jonathan Druart 2017-10-10 20:47:32 UTC
Created attachment 67900 [details] [review]
Bug 15685: (QA followup) Fix qa script errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 70 Jonathan Druart 2017-10-10 20:47:43 UTC
Created attachment 67901 [details] [review]
Bug 15685: QA Followup

DB revision fixes
Unit tests

Edit: fixed the update step description (tcohen)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 71 Jonathan Druart 2017-10-10 20:47:52 UTC
Created attachment 67902 [details] [review]
Bug 15685: (QA followup) Fix typo

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 72 Jonathan Druart 2017-10-10 20:48:06 UTC
Created attachment 67903 [details] [review]
Bug 15685: (followup) K:A:O->find and ->fetch are no longer used

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 73 Jonathan Druart 2017-10-10 20:48:14 UTC
Created attachment 67904 [details] [review]
Bug 15685: (QA followup) Tidy tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 74 Jonathan Druart 2017-10-10 20:48:21 UTC
Created attachment 67905 [details] [review]
Bug 15685: POD

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 75 Jonathan Druart 2017-10-10 20:48:27 UTC
Created attachment 67906 [details] [review]
Bug 15685: Add tests for Koha::Acquisition::Order->basket

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 76 Jonathan Druart 2017-10-10 20:48:36 UTC
Created attachment 67907 [details] [review]
Bug 15685: Followup - address QA issues
Comment 77 Jonathan Druart 2017-10-10 20:48:42 UTC
Created attachment 67908 [details] [review]
Bug 15685: Fix DB columns order
Comment 78 Jonathan Druart 2017-10-10 20:48:48 UTC
Created attachment 67909 [details] [review]
Bug 15685: DBIC Schema changes
Comment 79 Jonathan Druart 2017-10-10 20:51:41 UTC
Comment on attachment 67907 [details] [review]
Bug 15685: Followup - address QA issues

Review of attachment 67907 [details] [review]:
-----------------------------------------------------------------

-    my $basket = Koha::Acquisition::Order->find( $order->{ordernumber} )->basket;
+    my $basket = Koha::Acquisition::Orders->find({ordernumber => $ordernumber})->basket;

Why?
Comment 80 Jonathan Druart 2017-10-10 20:52:18 UTC
my $basket = Koha::Acquisition::Orders->find($ordernumber)->basket;

should be enough.
Comment 81 Jonathan Druart 2017-10-10 21:17:07 UTC
Created attachment 67910 [details] [review]
Bug 15685: Avoid "Use of uninitialized value" warning in logs
Comment 82 Jonathan Druart 2017-10-10 21:17:15 UTC
Created attachment 67911 [details] [review]
Bug 15685: Send the basket to _cancel_items_receipt
Comment 83 Jonathan Druart 2017-10-10 21:17:39 UTC
(In reply to Jonathan Druart from comment #82)
> Created attachment 67911 [details] [review] [review]
> Bug 15685: Send the basket to _cancel_items_receipt

Nick, does this last patch makes sense to you?
Comment 84 Nick Clemens 2017-10-10 22:02:05 UTC
(In reply to Jonathan Druart from comment #83)
> Nick, does this last patch makes sense to you?

Yes, there is more we could switch to use of objects, but I didn't want to get sidetracked
Comment 85 Nick Clemens 2017-10-10 22:03:14 UTC
(In reply to Jonathan Druart from comment #80)
> my $basket = Koha::Acquisition::Orders->find($ordernumber)->basket;
> 
> should be enough.
Indeed - I was debugging a typo so was more specific to be sure error was not there
Comment 86 Jonathan Druart 2017-10-11 16:53:06 UTC
Pushed to master for 17.11, thanks to everybody involved!