Bug 29955

Summary: Move C4::Acquisition::populate_order_with_prices to Koha::Acquisition::Order
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Architecture, internals, and plumbingAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Nick Clemens <nick>
Severity: enhancement    
Priority: P5 - low CC: arthur.suzuki, jonathan.druart, joonas.kylmala, lucas, m.de.rooy, nick, philippe.blouin, tomascohen, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32080
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.06, 21.11.14
Bug Depends on:    
Bug Blocks: 21204, 29959, 15348, 32171    
Attachments: Bug 29955: Move populate_order_with_prices to Koha namespace
Bug 29955: Move populate_order_with_prices to Koha namespace
Bug 29955: Move populate_order_with_prices to Koha namespace
Bug 29955: Move populate_order_with_prices to Koha namespace
Bug 29955: (QA follow-up) Tidy code block
Bug 29955: Fix method import issues

Description Julian Maurice 2022-01-27 10:10:44 UTC

    
Comment 1 Julian Maurice 2022-01-27 10:11:39 UTC
Created attachment 129856 [details] [review]
Bug 29955: Move populate_order_with_prices to Koha namespace

and split the subroutine into 2 smaller subroutines (one for ordering,
the other for receiving)

Test plan:
1. Create a vendor and an acquisition basket
2. In this basket, create new orders using all the different methods
   (from an existing record, from a suggestion, from a new record, ...)
   then close the basket and receive these orders.
   Make sure it works the same with and without the patch
3. Run tests in t/Prices.t,
   t/db_dependent/Acquisition/populate_order_with_prices.t, and
   t/db_dependent/Budgets.t
Comment 2 Joonas Kylmälä 2022-05-28 10:27:03 UTC
Created attachment 135436 [details] [review]
Bug 29955: Move populate_order_with_prices to Koha namespace

and split the subroutine into 2 smaller subroutines (one for ordering,
the other for receiving)

Test plan:
1. Create a vendor and an acquisition basket
2. In this basket, create new orders using all the different methods
   (from an existing record, from a suggestion, from a new record, ...)
   then close the basket and receive these orders.
   Make sure it works the same with and without the patch
3. Run tests in t/Prices.t,
   t/db_dependent/Acquisition/populate_order_with_prices.t, and
   t/db_dependent/Budgets.t

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 3 Victor Grousset/tuxayo 2022-07-16 22:06:58 UTC
Created attachment 137797 [details] [review]
Bug 29955: Move populate_order_with_prices to Koha namespace

and split the subroutine into 2 smaller subroutines (one for ordering,
the other for receiving)

Test plan:
1. Create a vendor and an acquisition basket
2. In this basket, create new orders using all the different methods
   (from an existing record, from a suggestion, from a new record, ...)
   then close the basket and receive these orders.
   Make sure it works the same with and without the patch
3. Run tests in t/Prices.t,
   t/db_dependent/Acquisition/populate_order_with_prices.t, and
   t/db_dependent/Budgets.t

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 4 Victor Grousset/tuxayo 2022-07-16 22:07:16 UTC
Rebased
Comment 5 Nick Clemens 2022-08-19 12:23:15 UTC
Created attachment 139487 [details] [review]
Bug 29955: Move populate_order_with_prices to Koha namespace

and split the subroutine into 2 smaller subroutines (one for ordering,
the other for receiving)

Test plan:
1. Create a vendor and an acquisition basket
2. In this basket, create new orders using all the different methods
   (from an existing record, from a suggestion, from a new record, ...)
   then close the basket and receive these orders.
   Make sure it works the same with and without the patch
3. Run tests in t/Prices.t,
   t/db_dependent/Acquisition/populate_order_with_prices.t, and
   t/db_dependent/Budgets.t

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Tomás Cohen Arazi 2022-08-19 18:47:16 UTC
*** Bug 30875 has been marked as a duplicate of this bug. ***
Comment 7 Tomás Cohen Arazi 2022-08-19 18:51:49 UTC
Created attachment 139518 [details] [review]
Bug 29955: (QA follow-up) Tidy code block

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2022-08-19 19:04:59 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 9 Tomás Cohen Arazi 2022-08-22 16:14:45 UTC
Created attachment 139613 [details] [review]
Bug 29955: Fix method import issues

This patch fixes this:

$ prove t/db_dependent/Utils/Datatables_Virtualshelves.t
t/db_dependent/Utils/Datatables_Virtualshelves.t .. 2/13 Use of inherited AUTOLOAD for non-method Koha::Virtualshelf::haspermission() is no longer allowed at /kohadevbox/koha/Koha/Virtualshelf.pm line 248.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Lucas Gass 2022-10-14 19:28:04 UTC
I suppose this is an enhancement? I will not backport to 22.05.x unless requested.
Comment 11 Blou 2022-11-01 18:05:05 UTC
The last patch "import issue" fixes a crash in 22.05.06 (at least).

I haven't managed to apply the whole set on Koha/VirtualShelf.pm, because of Bug 30933.  It's a rabbit hole, but without it we get a 500 on public lists when logged in the OPAC.
Comment 12 Marcel de Rooy 2022-11-03 07:21:17 UTC
(In reply to Blou from comment #11)
> The last patch "import issue" fixes a crash in 22.05.06 (at least).
> 
> I haven't managed to apply the whole set on Koha/VirtualShelf.pm, because of
> Bug 30933.  It's a rabbit hole, but without it we get a 500 on public lists
> when logged in the OPAC.

See also bug 32080.
Comment 13 Arthur Suzuki 2022-11-15 17:17:20 UTC
applied to 21.11 for 21.11.14 (backported from 22.05, lucas probably just forgot to update the bz)
Comment 14 Arthur Suzuki 2022-11-15 17:18:11 UTC
applied to 21.11 for 21.11.14 (backported from 22.05, lucas probably just forgot to update the bz)
Comment 15 Lucas Gass 2022-11-22 21:05:14 UTC
To clarify, this was backported for 22.05.x and will be in the upcoming 22.05.07 release
Comment 16 Nick Clemens 2022-11-23 12:36:04 UTC
(In reply to Lucas Gass from comment #15)
> To clarify, this was backported for 22.05.x and will be in the upcoming
> 22.05.07 release

It looks like only the last patch was picked? That should be okay, just verifiying
Comment 17 Victor Grousset/tuxayo 2022-11-27 00:16:57 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 18 Jonathan Druart 2023-01-16 20:45:24 UTC
Hum, is this really working? It feels like there is a store call missing on $order_obj.
Comment 19 Jonathan Druart 2023-01-16 21:04:05 UTC
(In reply to Jonathan Druart from comment #18)
> Hum, is this really working? It feels like there is a store call missing on
> $order_obj.

This was for finishreceive. I think it's actually ok, the store is done in ModReceiveOrder.