Bug 29955 - Move C4::Acquisition::populate_order_with_prices to Koha::Acquisition::Order
Summary: Move C4::Acquisition::populate_order_with_prices to Koha::Acquisition::Order
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Nick Clemens
URL:
Keywords:
: 30875 (view as bug list)
Depends on:
Blocks: 21204 29959 15348 32171
  Show dependency treegraph
 
Reported: 2022-01-27 10:10 UTC by Julian Maurice
Modified: 2023-12-28 20:43 UTC (History)
9 users (show)

See Also:
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


Attachments
Bug 29955: Move populate_order_with_prices to Koha namespace (71.94 KB, patch)
2022-01-27 10:11 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 29955: Move populate_order_with_prices to Koha namespace (72.08 KB, patch)
2022-05-28 10:27 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29955: Move populate_order_with_prices to Koha namespace (72.07 KB, patch)
2022-07-16 22:06 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 29955: Move populate_order_with_prices to Koha namespace (72.11 KB, patch)
2022-08-19 12:23 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29955: (QA follow-up) Tidy code block (2.97 KB, patch)
2022-08-19 18:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29955: Fix method import issues (2.13 KB, patch)
2022-08-22 16:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.