Bug 26579 - Remove unused C4::Acquisition::DelOrder function
Summary: Remove unused C4::Acquisition::DelOrder function
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
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 26577
Blocks: 21204
  Show dependency treegraph
 
Reported: 2020-09-30 14:45 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2021-06-14 21:33 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Circulation function:


Attachments
Bug 26579: Remove unused C4::Acquisition::DelOrder function (9.06 KB, patch)
2020-09-30 14:57 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 26579: Remove unused C4::Acquisition::DelOrder function (9.14 KB, patch)
2020-10-01 13:11 UTC, David Nind
Details | Diff | Splinter Review
Bug 26579: Remove unused C4::Acquisition::DelOrder function (9.17 KB, patch)
2020-10-02 12:02 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 26579: Remove unused C4::Acquisition::DelOrder function (9.18 KB, patch)
2020-10-15 12:53 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2020-09-30 14:45:57 UTC
As of bug 26577, it is only used in a bunch of test files.
Comment 1 Tomás Cohen Arazi (tcohen) 2020-09-30 14:57:26 UTC
Created attachment 110996 [details] [review]
Bug 26579: Remove unused C4::Acquisition::DelOrder function

This patch removes an unused function, its tests, and adjusts other test
files that relied on it by replacing it with $order->cancel calls.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Acquisition.t \
           t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t \
           t/db_dependent/Acquisition/close_reopen_basket.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat (1)
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 David Nind 2020-10-01 13:11:06 UTC
Created attachment 111040 [details] [review]
Bug 26579: Remove unused C4::Acquisition::DelOrder function

This patch removes an unused function, its tests, and adjusts other test
files that relied on it by replacing it with $order->cancel calls.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Acquisition.t \
           t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t \
           t/db_dependent/Acquisition/close_reopen_basket.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat (1)
=> SUCCESS: Tests pass!
4. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Martin Renvoize (ashimema) 2020-10-02 12:02:25 UTC
Created attachment 111138 [details] [review]
Bug 26579: Remove unused C4::Acquisition::DelOrder function

This patch removes an unused function, its tests, and adjusts other test
files that relied on it by replacing it with $order->cancel calls.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Acquisition.t \
           t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t \
           t/db_dependent/Acquisition/close_reopen_basket.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat (1)
=> SUCCESS: Tests pass!
4. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize (ashimema) 2020-10-02 12:02:57 UTC
I concur DelOrder is no longer required.

Passing QA
Comment 5 Martin Renvoize (ashimema) 2020-10-02 12:03:18 UTC
Blocked by dependancies which need QA
Comment 6 Jonathan Druart 2020-10-08 12:09:58 UTC
If I am reading the code correctly there is a behaviour changed:

before: the order is cancelled and the items that can be deleted are deleted
after : the order is cancelled but if at least 1 item cannot be deleted, none are deleted
Comment 7 Jonathan Druart 2020-10-15 09:19:07 UTC
(In reply to Jonathan Druart from comment #6)
> If I am reading the code correctly there is a behaviour changed:
> 
> before: the order is cancelled and the items that can be deleted are deleted
> after : the order is cancelled but if at least 1 item cannot be deleted,
> none are deleted

To clarify the situation: the whole tree is blocked by this.
Comment 8 Tomás Cohen Arazi (tcohen) 2020-10-15 09:50:53 UTC
(In reply to Jonathan Druart from comment #7)
> (In reply to Jonathan Druart from comment #6)
> > If I am reading the code correctly there is a behaviour changed:
> > 
> > before: the order is cancelled and the items that can be deleted are deleted
> > after : the order is cancelled but if at least 1 item cannot be deleted,
> > none are deleted
> 
> To clarify the situation: the whole tree is blocked by this.

Oh, I forgot about it. Will fix it!
Comment 9 Tomás Cohen Arazi (tcohen) 2020-10-15 12:46:23 UTC
Solved, with follow-ups on bug 26515 and bug 26555.
Comment 10 Tomás Cohen Arazi (tcohen) 2020-10-15 12:53:43 UTC
Created attachment 111726 [details] [review]
Bug 26579: Remove unused C4::Acquisition::DelOrder function

This patch removes an unused function, its tests, and adjusts other test
files that relied on it by replacing it with $order->cancel calls.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Acquisition.t \
           t/db_dependent/Acquisition/GetBasketsInfosByBookseller.t \
           t/db_dependent/Acquisition/close_reopen_basket.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat (1)
=> SUCCESS: Tests pass!
4. Sign off :-D

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

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Tomás Cohen Arazi (tcohen) 2020-10-15 12:54:12 UTC
Rebased against master, test count error.
Comment 12 Jonathan Druart 2020-10-15 13:25:26 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 13 Lucas Gass (lukeg) 2020-11-13 15:51:51 UTC
enhancement will not be backported to 20.05.x