Bug 36620 - Broken order management for suggestions with quantity
Summary: Broken order management for suggestions with quantity
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P3 normal (vote)
Assignee: Janusz Kaczmarek
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 34708
Blocks:
  Show dependency treegraph
 
Reported: 2024-04-17 11:12 UTC by Janusz Kaczmarek
Modified: 2024-05-30 19:41 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06


Attachments
Bug 36620: Broken order management for suggestions with quantity (4.52 KB, patch)
2024-04-17 12:36 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36620: Broken order management for suggestions with quantity (4.57 KB, patch)
2024-04-17 19:00 UTC, Roman Dolny
Details | Diff | Splinter Review
Bug 36620: Broken order management for suggestions with quantity (4.64 KB, patch)
2024-05-15 08:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36620: (QA follow-up) Add missing filters (1.57 KB, patch)
2024-05-15 08:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36620: (QA follow-up) Tidy controller (8.28 KB, patch)
2024-05-15 08:45 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2024-04-17 11:12:56 UTC
After applying bug 34708 the management of orders created from suggestions with defined quantity is broken.
Comment 1 Janusz Kaczmarek 2024-04-17 12:36:42 UTC
Created attachment 165004 [details] [review]
Bug 36620: Broken order management for suggestions with quantity

Test plan
=========
Scenario A (the bug)
--------------------
1. In OPAC, create a purchase suggestion with defined quantity
   ("Copies" in staff interface).
2. Accept the suggestion.
2. Have a budget, fund, vendor etc. to use the Acquisitions module.
   Ensure that AcqCreateItem syspref is set to 'placing an order'.
3. Create a new basket and add to basket an order line from accepted
   suggestion.  Note the initial Quantity set to the quantity entered
   in suggestion.  Note increasing this number while you add items
   to the order with the 'Add item' button. Save the order, close the
   basket.
4. Start receiving shipments for this order (create invoice etc.).
   Receive items you created by 'Add item' until there is nothing more
   to receive.  Note the number of unreecived items in the order (eqal
   to the initial quantity from the suggestion) that cannot be regulary
   received (with the items table on the left).  This is the *bug*.
   [This is due to the ambiguity of $data->{quantity} in the neworderempty.pl
   -- it normally comes from the order: GetOrder, but in the case of a yet
   empty order created from a suggestion, it comes from the suggestions table:
   GetSuggestion].
5. Apply the patch, restart plack etc.
6. Repeat step 2 and 3. While adding items in order note that you start
   from quantity 0, and that the suggested quantity is shown in brackets.
7. Continue with p. 4.  You should be able to receive the order in the
   regular way (with the items created).

Scenario B (bonus)
------------------
Repeat the above with AcqCreateItem syspref is set to 'receiving the order',
or 'cataloging the record'.  These cases do not create a bug as it does
the case with 'placing an order'.  Note however, after applying the patch,
the initial quantity in the order is set to 0 and there is the bonus infomation
about the suggested number of items (from the suggestion).

Sponsored-by: Ignatianum University in Cracow
Comment 2 Roman Dolny 2024-04-17 19:00:07 UTC
Created attachment 165032 [details] [review]
Bug 36620: Broken order management for suggestions with quantity

Test plan
=========
Scenario A (the bug)
--------------------
1. In OPAC, create a purchase suggestion with defined quantity
   ("Copies" in staff interface).
2. Accept the suggestion.
2. Have a budget, fund, vendor etc. to use the Acquisitions module.
   Ensure that AcqCreateItem syspref is set to 'placing an order'.
3. Create a new basket and add to basket an order line from accepted
   suggestion.  Note the initial Quantity set to the quantity entered
   in suggestion.  Note increasing this number while you add items
   to the order with the 'Add item' button. Save the order, close the
   basket.
4. Start receiving shipments for this order (create invoice etc.).
   Receive items you created by 'Add item' until there is nothing more
   to receive.  Note the number of unreecived items in the order (eqal
   to the initial quantity from the suggestion) that cannot be regulary
   received (with the items table on the left).  This is the *bug*.
   [This is due to the ambiguity of $data->{quantity} in the neworderempty.pl
   -- it normally comes from the order: GetOrder, but in the case of a yet
   empty order created from a suggestion, it comes from the suggestions table:
   GetSuggestion].
5. Apply the patch, restart plack etc.
6. Repeat step 2 and 3. While adding items in order note that you start
   from quantity 0, and that the suggested quantity is shown in brackets.
7. Continue with p. 4.  You should be able to receive the order in the
   regular way (with the items created).

Scenario B (bonus)
------------------
Repeat the above with AcqCreateItem syspref is set to 'receiving the order',
or 'cataloging the record'.  These cases do not create a bug as it does
the case with 'placing an order'.  Note however, after applying the patch,
the initial quantity in the order is set to 0 and there is the bonus infomation
about the suggested number of items (from the suggestion).

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Comment 3 Martin Renvoize 2024-05-15 08:44:55 UTC
Created attachment 166757 [details] [review]
Bug 36620: Broken order management for suggestions with quantity

Test plan
=========
Scenario A (the bug)
--------------------
1. In OPAC, create a purchase suggestion with defined quantity
   ("Copies" in staff interface).
2. Accept the suggestion.
2. Have a budget, fund, vendor etc. to use the Acquisitions module.
   Ensure that AcqCreateItem syspref is set to 'placing an order'.
3. Create a new basket and add to basket an order line from accepted
   suggestion.  Note the initial Quantity set to the quantity entered
   in suggestion.  Note increasing this number while you add items
   to the order with the 'Add item' button. Save the order, close the
   basket.
4. Start receiving shipments for this order (create invoice etc.).
   Receive items you created by 'Add item' until there is nothing more
   to receive.  Note the number of unreecived items in the order (eqal
   to the initial quantity from the suggestion) that cannot be regulary
   received (with the items table on the left).  This is the *bug*.
   [This is due to the ambiguity of $data->{quantity} in the neworderempty.pl
   -- it normally comes from the order: GetOrder, but in the case of a yet
   empty order created from a suggestion, it comes from the suggestions table:
   GetSuggestion].
5. Apply the patch, restart plack etc.
6. Repeat step 2 and 3. While adding items in order note that you start
   from quantity 0, and that the suggested quantity is shown in brackets.
7. Continue with p. 4.  You should be able to receive the order in the
   regular way (with the items created).

Scenario B (bonus)
------------------
Repeat the above with AcqCreateItem syspref is set to 'receiving the order',
or 'cataloging the record'.  These cases do not create a bug as it does
the case with 'placing an order'.  Note however, after applying the patch,
the initial quantity in the order is set to 0 and there is the bonus infomation
about the suggested number of items (from the suggestion).

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2024-05-15 08:44:58 UTC
Created attachment 166758 [details] [review]
Bug 36620: (QA follow-up) Add missing filters
Comment 5 Martin Renvoize 2024-05-15 08:45:00 UTC
Created attachment 166759 [details] [review]
Bug 36620: (QA follow-up) Tidy controller
Comment 6 Martin Renvoize 2024-05-15 08:45:34 UTC
Passing QA here, a couple of minor follow-ups were needed to make the QA scripts happy.
Comment 7 Katrin Fischer 2024-05-17 08:37:41 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 8 Jennifer Weston 2024-05-22 21:55:44 UTC
Great fix - thanks!
Would it be possible to backport this to 23.11.x for upcoming 23.11.06?
Comment 9 David Cook 2024-05-23 04:01:51 UTC
(In reply to Jennifer Weston from comment #8)
> Great fix - thanks!
> Would it be possible to backport this to 23.11.x for upcoming 23.11.06?

That would be great, because I am getting reports of this problem from people using 23.11.x.

(I'll be backporting it in a moment for my clients, and I'll report here if there's any issues with that.)
Comment 10 David Cook 2024-05-23 04:02:54 UTC
Comment on attachment 166757 [details] [review]
Bug 36620: Broken order management for suggestions with quantity

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

::: acqui/neworderempty.pl
@@ +206,5 @@
>      # otherwise, retrieve suggestion information.
>      elsif ($suggestionid) {
>          $data = GetSuggestion($suggestionid);
> +        $data->{quantitysugg} = $data->{quantity};
> +        undef $data->{quantity};

It's too late now, but this probably could've been "$data->{quantitysugg} = delete $data->{quantity}" instead.

Not a big deal. Just sharing for next time.
Comment 11 David Cook 2024-05-23 04:17:29 UTC
The "Bug 36620: (QA follow-up) Tidy controller" patch doesn't apply cleanly on 23.11.x because "main" has bug 35927 applied first.

So I'll add that as a dependency and try again. Looks like a small change...
Comment 12 David Cook 2024-05-23 04:25:35 UTC
Blarg... "Bug 36620: (QA follow-up) Tidy controller" relies on bug 34478 which isn't going to be backported. (Rather than add it as a dependency, I'll remove bug 35927 as a dependency, because they're just dependencies of that tidy patch...)

I'm just going to skip that patch rather than try to do a 23.11.x version of it, but that has some risks for future backporting.

I'll leave it up to the RMaint whether they want to skip that last patch, or if they want to do a 23.11 version...
Comment 13 Jennifer Weston 2024-05-23 15:43:31 UTC
(In reply to David Cook from comment #12)
> Blarg... "Bug 36620: (QA follow-up) Tidy controller" relies on bug 34478
> which isn't going to be backported. (Rather than add it as a dependency,
> I'll remove bug 35927 as a dependency, because they're just dependencies of
> that tidy patch...)
> 
> I'm just going to skip that patch rather than try to do a 23.11.x version of
> it, but that has some risks for future backporting.
> 
> I'll leave it up to the RMaint whether they want to skip that last patch, or
> if they want to do a 23.11 version...

David - thanks for testing out the backporting options and dependency effects.
Comment 14 Fridolin Somers 2024-05-29 13:03:46 UTC
Pushed to 23.11.x for 23.11.06
Comment 15 Lucas Gass 2024-05-30 19:41:53 UTC
Not needed in 23.05.x, no backport.