Bug 35004 - Cannot receive order lines with items created in cataloguing
Summary: Cannot receive order lines with items created in cataloguing
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Slava Shishkin
QA Contact: Katrin Fischer
URL:
Keywords: rel_23_05_candidate
: 35296 (view as bug list)
Depends on: 8179 33864 34736
Blocks:
  Show dependency treegraph
 
Reported: 2023-10-06 16:15 UTC by Slava Shishkin
Modified: 2023-11-14 15:12 UTC (History)
4 users (show)

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


Attachments
Bug 35004: Set row.quantity_received to 1 if row.quantity_received is empty and we're in 'cataloguing' mode (2.58 KB, patch)
2023-10-06 16:29 UTC, Slava Shishkin
Details | Diff | Splinter Review
Bug 35004: Set row.quantity_received to 1 if row.quantity_received is empty and we're in 'cataloguing' mode (2.63 KB, patch)
2023-11-03 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35004: Set row.quantity_received to 1 if row.quantity_received is empty and we're in 'cataloguing' mode (2.40 KB, patch)
2023-11-03 14:20 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 Slava Shishkin 2023-10-06 16:15:00 UTC
An error occurs when receiving the order. This happens if the user does not change the "Quantity received" field value, thereby not triggering the "OnChange" event.

By default, the "Quantity received" field value is set to 1 by JS,
but there is a row.quantity_received variable which serves as a shadow value
which sent in a POST Ajax request after clicking "Confirm",
yet row.quantity_received stays undefined despite the fact that we changed the form value by JS.
row.quantity_received is expected to be updated on the "onChange" event for the form field, but that event does not fired when JS changes the form field value to 1.
So if the user does not change the form field, it has a value of "1" but the form sends an empty value from the shadow value. This leads to an error from the backend which says "No quantity to receive set. No items will be created." in the popup.


To test:
1. Add a new basket.
2. Create an order line
3. Receive shipment and create an invoice
4. Start receiving your order:
    Verify:
    - Quantity ordered: X
    - Quantity received: 1 (pre-filled, don't touch or change it)
5. Click "Confirm"
6. Observe the Receive error popup: "Order ZZZ: No quantity to receive set. No items will be created."
7. Verify that "Order receive" was not happen (received orders remains empty)
Comment 1 Slava Shishkin 2023-10-06 16:29:36 UTC
Created attachment 156634 [details] [review]
Bug 35004: Set row.quantity_received to 1 if row.quantity_received is empty and we're in 'cataloguing' mode

Before the code changes only field value was set to 1,
but "row.quantity_received" still had an undefined value.
After correction row.quantity_received set to 1 if it is empty,
but only when effective_create_items == 'cataloguing',
and after that its value assigned to the form field.

To test:
1. Add a new basket.
2. Create an order line
3. Receive shipment and create an invoice
4. Start receiving your order:
    Verify:
    - Quantity ordered: X
    - Quantity received: 1 (pre-filled, don't touch or change it)
5. Click "Confirm"
6. Observe the Receive error popup: "Order ZZZ: No quantity to receive
   set. No items will be created."
7. Verify that "Order receive" was not happen (received orders remains
   empty)
8. Apply the patch and make sure your order is received correctly after
   step 5.
Comment 2 Katrin Fischer 2023-10-06 21:53:09 UTC
Hi Slava,

did you test on latest master? This reads like bug 34880.


+                    $("#creator").html(__("Account has been deleted"));
This won't work, the __ is only for .js files, not for JavaScript in general.
Comment 3 Katrin Fischer 2023-10-06 21:54:34 UTC
In your test plan, which value did you set for creating items?
Comment 4 Katrin Fischer 2023-10-06 21:55:05 UTC
(In reply to Katrin Fischer from comment #3)
> In your test plan, which value did you set for creating items?

Standing order?
Subscription order?
Comment 5 Slava Shishkin 2023-10-07 12:45:14 UTC
(In reply to Katrin Fischer from comment #2)
> Hi Slava,
> 
> did you test on latest master? This reads like bug 34880.

Hi Katrin,
Yes, I checked this on the latest master.



(In reply to Katrin Fischer from comment #4)
> > In your test plan, which value did you set for creating items?
> 
> Standing order?
> Subscription order?

The basket was closed before receiving items. It wasn't "Orders are standing" or "Subscription order".
Comment 6 Katrin Fischer 2023-11-03 12:45:24 UTC
I am sorry, to be late here.

I just confirmed this bug in the recent 23.05, possibly in master as well.
Comment 7 Nick Clemens 2023-11-03 13:12:04 UTC
Created attachment 158334 [details] [review]
Bug 35004: Set row.quantity_received to 1 if row.quantity_received is empty and we're in 'cataloguing' mode

Before the code changes only field value was set to 1,
but "row.quantity_received" still had an undefined value.
After correction row.quantity_received set to 1 if it is empty,
but only when effective_create_items == 'cataloguing',
and after that its value assigned to the form field.

To test:
1. Add a new basket.
2. Create an order line
3. Receive shipment and create an invoice
4. Start receiving your order:
    Verify:
    - Quantity ordered: X
    - Quantity received: 1 (pre-filled, don't touch or change it)
5. Click "Confirm"
6. Observe the Receive error popup: "Order ZZZ: No quantity to receive
   set. No items will be created."
7. Verify that "Order receive" was not happen (received orders remains
   empty)
8. Apply the patch and make sure your order is received correctly after
   step 5.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Katrin Fischer 2023-11-03 13:24:35 UTC
I have amended the patch removing this change:

$("#creator").html(_("Account has been deleted"));
$("#creator").html(__("Account has been deleted"));

The double undescore for translations is only used in .js files. In .inc and embedded JavaScript code it doesn't work. We still need the single underscore there.

Continuing testing.
Comment 9 Katrin Fischer 2023-11-03 14:19:30 UTC
Did a lot of testing, hopefully not missing anything this time:

Items in cataloging
* ordered 1, receive 1
* ordered 5, receive 2
* standing order, received on multiple invoices
* subscription*
* subscription + standing order*


Items on receive
* ordered 1, receive 1
* ordered 5, receive 2
* standing order, received on multiple invoices
* subscription*
* subscription + standing order* 

Items on order
* ordered 1, receive 1
* ordered 5, receive 2
* standing order, received on multiple invoices
* subscription (no items, because it's a subscription order)*
* subscription + standing order (no items, because it's a subscription order)*
Comment 10 Katrin Fischer 2023-11-03 14:20:07 UTC
Created attachment 158336 [details] [review]
Bug 35004: Set row.quantity_received to 1 if row.quantity_received is empty and we're in 'cataloguing' mode

Before the code changes only field value was set to 1,
but "row.quantity_received" still had an undefined value.
After correction row.quantity_received set to 1 if it is empty,
but only when effective_create_items == 'cataloguing',
and after that its value assigned to the form field.

To test:
1. Add a new basket.
2. Create an order line
3. Receive shipment and create an invoice
4. Start receiving your order:
    Verify:
    - Quantity ordered: X
    - Quantity received: 1 (pre-filled, don't touch or change it)
5. Click "Confirm"
6. Observe the Receive error popup: "Order ZZZ: No quantity to receive
   set. No items will be created."
7. Verify that "Order receive" was not happen (received orders remains
   empty)
8. Apply the patch and make sure your order is received correctly after
   step 5.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 11 Tomás Cohen Arazi 2023-11-03 15:06:11 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Katrin Fischer 2023-11-03 15:16:11 UTC
Thanks for the quick push!
Comment 13 Danyon Sewell 2023-11-09 03:43:26 UTC
*** Bug 35296 has been marked as a duplicate of this bug. ***
Comment 14 Fridolin Somers 2023-11-10 02:21:37 UTC
Pushed to 23.05.x for 23.05.06