Bug 27021 - Chaining on Koha::Objects->empty should always return an empty resultset
Summary: Chaining on Koha::Objects->empty should always return an empty resultset
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks: 27002
  Show dependency treegraph
 
Reported: 2020-11-13 18:46 UTC by Tomás Cohen Arazi
Modified: 2020-12-09 20:36 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07


Attachments
Bug 27021: Regression tests (1.59 KB, patch)
2020-11-13 18:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27021: Make chaining ->empty always return an empty resultset (1.05 KB, patch)
2020-11-13 19:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27021: Regression tests (1.64 KB, patch)
2020-11-14 07:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 27021: Make chaining ->empty always return an empty resultset (1.09 KB, patch)
2020-11-14 07:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 27021: Regression tests (1.70 KB, patch)
2020-11-14 07:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27021: Make chaining ->empty always return an empty resultset (1.15 KB, patch)
2020-11-14 07:58 UTC, Martin Renvoize
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 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.