Bug 27021

Summary: Chaining on Koha::Objects->empty should always return an empty resultset
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: RESOLVED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: andrew, david, jonathan.druart, kyle, martin.renvoize, victor
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07
Bug Depends on:    
Bug Blocks: 27002    
Attachments: Bug 27021: Regression tests
Bug 27021: Make chaining ->empty always return an empty resultset
Bug 27021: Regression tests
Bug 27021: Make chaining ->empty always return an empty resultset
Bug 27021: Regression tests
Bug 27021: Make chaining ->empty always return an empty resultset

Description Tomás Cohen Arazi 2020-11-13 18:46:27 UTC
Steps to reproduce:
- Run the provided regression tests
Comment 1 Tomás Cohen Arazi 2020-11-13 18:51:16 UTC
Created attachment 113626 [details] [review]
Bug 27021: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2020-11-13 19:01:34 UTC
Created attachment 113627 [details] [review]
Bug 27021: Make chaining ->empty always return an empty resultset

This patch borrows the implementation Martin did on bug 27002, to
woraround this issue, and implements it properly on the
Koha::Objects->empty method.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Objects.t
=> FAIL: Tests fail! The resultset should be empty and it isn't!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2020-11-13 19:37:30 UTC
The only place in the code were we used ->empty is in Koha::Acquisition::Order->current_item_level_holds so worth checking this as well:

    $ kshell
   k$ prove t/db_dependent/Koha/Acquisition/Order.t
Comment 4 David Nind 2020-11-14 07:45:13 UTC
Created attachment 113636 [details] [review]
Bug 27021: Regression tests

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2020-11-14 07:45:16 UTC
Created attachment 113637 [details] [review]
Bug 27021: Make chaining ->empty always return an empty resultset

This patch borrows the implementation Martin did on bug 27002, to
woraround this issue, and implements it properly on the
Koha::Objects->empty method.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Objects.t
=> FAIL: Tests fail! The resultset should be empty and it isn't!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off!

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

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Martin Renvoize 2020-11-14 07:58:30 UTC
Created attachment 113638 [details] [review]
Bug 27021: Regression tests

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 7 Martin Renvoize 2020-11-14 07:58:33 UTC
Created attachment 113639 [details] [review]
Bug 27021: Make chaining ->empty always return an empty resultset

This patch borrows the implementation Martin did on bug 27002, to
woraround this issue, and implements it properly on the
Koha::Objects->empty method.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/Objects.t
=> FAIL: Tests fail! The resultset should be empty and it isn't!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off!

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 8 Martin Renvoize 2020-11-14 07:59:50 UTC
I approve of moving the 'always empty search' code into this method, it's much cleaner long term.

Works as expected and no regressions found.

Passing QA
Comment 9 Jonathan Druart 2020-11-16 12:50:02 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 10 Andrew Fuerste-Henry 2020-12-09 20:06:41 UTC
Pushed to 20.05 for 20.05.07
Comment 11 Victor Grousset/tuxayo 2020-12-09 20:36:46 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.