Bug 19130 - K::A::Booksellers->search broken for attribute 'name'
Summary: K::A::Booksellers->search broken for attribute 'name'
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: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 13726
Blocks: 18120
  Show dependency treegraph
 
Reported: 2017-08-16 21:46 UTC by Tomás Cohen Arazi
Modified: 2017-10-01 22:19 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19130: Remove ->search() overloading and adjust tests (3.05 KB, patch)
2017-08-22 15:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19130: (followup) Add t/db_dependent/Koha/Acquisition/Booksellers.t (7.47 KB, patch)
2017-08-22 15:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19130: (followup) Add POD (3.23 KB, patch)
2017-08-22 15:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19130: (followup) Controller scripts should preserve behaviour (3.98 KB, patch)
2017-08-22 16:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19130: Remove ->search() overloading and adjust tests (3.10 KB, patch)
2017-08-22 18:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19130: (followup) Add t/db_dependent/Koha/Acquisition/Booksellers.t (7.52 KB, patch)
2017-08-22 18:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19130: (followup) Add POD (3.27 KB, patch)
2017-08-22 18:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19130: (followup) Controller scripts should preserve behaviour (4.00 KB, patch)
2017-08-22 18:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19130: Remove ->search() overloading and adjust tests (3.18 KB, patch)
2017-08-25 14:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19130: (followup) Add t/db_dependent/Koha/Acquisition/Booksellers.t (7.60 KB, patch)
2017-08-25 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19130: (followup) Add POD (3.36 KB, patch)
2017-08-25 14:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19130: (followup) Controller scripts should preserve behaviour (4.10 KB, patch)
2017-08-25 14:58 UTC, Jonathan Druart
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 2017-08-16 21:46:29 UTC
The current overloaded implementation doesn't allow to search for stuff that is suffix of a bigger word. For example, if you have

- name: Name2
- name: Name21

the query expression { name => 'Name%2' } should only return 'Name2'. The current implementation returns both.

This was implemented this way to ease the transition into the new code without changing too many things on the controller scripts. It is time to fix it.
Comment 1 Tomás Cohen Arazi 2017-08-22 15:17:36 UTC
Created attachment 66348 [details] [review]
Bug 19130: Remove ->search() overloading and adjust tests

This patch removes the custom ->search() function. Tests are adjusted
so the results from ->search() calls are not expected to return in the
previously hardcoded order.

To test:
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Bookseller.t
=> SUCCESS: Tests pass
- Sign off :-D
Comment 2 Tomás Cohen Arazi 2017-08-22 15:17:42 UTC
Created attachment 66349 [details] [review]
Bug 19130: (followup) Add t/db_dependent/Koha/Acquisition/Booksellers.t

This followup patch adds a proper file in which add tests for
Koha::Acquisition::Bookseller(s) methods.

All current methods are covered.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Acquisition/Booksellers.t
 => SUCCESS: Test pass!
 - Sign off :-D
Comment 3 Tomás Cohen Arazi 2017-08-22 15:17:47 UTC
Created attachment 66350 [details] [review]
Bug 19130: (followup) Add POD

This patch adds POD to the following files:

- Koha/Acquisition/Bookseller.pm
- Koha/Acquisition/Booksellers.pm
Comment 4 Tomás Cohen Arazi 2017-08-22 16:15:44 UTC
Created attachment 66352 [details] [review]
Bug 19130: (followup) Controller scripts should preserve behaviour

This patch is a followup to making
Koha::Acquisition::Booksellers->search work as any other Koha::Objects
(DBIC) query instead of having a different behaviour hardcoded.

To achieve it, this patch makes the controller scripts add
wildcard/truncation chars as prefix and sufix for searches, and make the
default sorting for results be by 'name', ascending.

To test:
- Just verify the behaviour remains unchanged by this patchset on the
controller scripts (re. searching).
Comment 5 Kyle M Hall 2017-08-22 18:19:46 UTC
Created attachment 66365 [details] [review]
Bug 19130: Remove ->search() overloading and adjust tests

This patch removes the custom ->search() function. Tests are adjusted
so the results from ->search() calls are not expected to return in the
previously hardcoded order.

To test:
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Bookseller.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2017-08-22 18:19:53 UTC
Created attachment 66366 [details] [review]
Bug 19130: (followup) Add t/db_dependent/Koha/Acquisition/Booksellers.t

This followup patch adds a proper file in which add tests for
Koha::Acquisition::Bookseller(s) methods.

All current methods are covered.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Acquisition/Booksellers.t
 => SUCCESS: Test pass!
 - Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2017-08-22 18:19:56 UTC
Created attachment 66367 [details] [review]
Bug 19130: (followup) Add POD

This patch adds POD to the following files:

- Koha/Acquisition/Bookseller.pm
- Koha/Acquisition/Booksellers.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2017-08-22 18:19:58 UTC
Created attachment 66368 [details] [review]
Bug 19130: (followup) Controller scripts should preserve behaviour

This patch is a followup to making
Koha::Acquisition::Booksellers->search work as any other Koha::Objects
(DBIC) query instead of having a different behaviour hardcoded.

To achieve it, this patch makes the controller scripts add
wildcard/truncation chars as prefix and sufix for searches, and make the
default sorting for results be by 'name', ascending.

To test:
- Just verify the behaviour remains unchanged by this patchset on the
controller scripts (re. searching).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Jonathan Druart 2017-08-25 14:57:59 UTC
Created attachment 66480 [details] [review]
Bug 19130: Remove ->search() overloading and adjust tests

This patch removes the custom ->search() function. Tests are adjusted
so the results from ->search() calls are not expected to return in the
previously hardcoded order.

To test:
- Apply this patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Bookseller.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2017-08-25 14:58:04 UTC
Created attachment 66481 [details] [review]
Bug 19130: (followup) Add t/db_dependent/Koha/Acquisition/Booksellers.t

This followup patch adds a proper file in which add tests for
Koha::Acquisition::Bookseller(s) methods.

All current methods are covered.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Koha/Acquisition/Booksellers.t
 => SUCCESS: Test pass!
 - Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2017-08-25 14:58:08 UTC
Created attachment 66482 [details] [review]
Bug 19130: (followup) Add POD

This patch adds POD to the following files:

- Koha/Acquisition/Bookseller.pm
- Koha/Acquisition/Booksellers.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2017-08-25 14:58:12 UTC
Created attachment 66483 [details] [review]
Bug 19130: (followup) Controller scripts should preserve behaviour

This patch is a followup to making
Koha::Acquisition::Booksellers->search work as any other Koha::Objects
(DBIC) query instead of having a different behaviour hardcoded.

To achieve it, this patch makes the controller scripts add
wildcard/truncation chars as prefix and sufix for searches, and make the
default sorting for results be by 'name', ascending.

To test:
- Just verify the behaviour remains unchanged by this patchset on the
controller scripts (re. searching).

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2017-08-25 14:59:05 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 14 Fridolin SOMERS 2017-09-29 13:25:06 UTC
Pushed to 17.05.x, will be in 17.05.05.
Comment 15 Katrin Fischer 2017-10-01 22:19:14 UTC
Dependent bug 13726 is not in 16.11.x.