Bug 26579

Summary: Remove unused C4::Acquisition::DelOrder function
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: Pushed to master --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, kyle, lucas, martin.renvoize
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26580
Change sponsored?: --- Patch complexity: ---
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on: 26577    
Bug Blocks: 21204    
Attachments: Bug 26579: Remove unused C4::Acquisition::DelOrder function
Bug 26579: Remove unused C4::Acquisition::DelOrder function
Bug 26579: Remove unused C4::Acquisition::DelOrder function
Bug 26579: Remove unused C4::Acquisition::DelOrder function

Description Tomás Cohen Arazi 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 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 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 2020-10-02 12:02:57 UTC
I concur DelOrder is no longer required.

Passing QA
Comment 5 Martin Renvoize 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 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 2020-10-15 12:46:23 UTC
Solved, with follow-ups on bug 26515 and bug 26555.
Comment 10 Tomás Cohen Arazi 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 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 2020-11-13 15:51:51 UTC
enhancement will not be backported to 20.05.x