Bug 27854 - Clean GET /patrons controller
Summary: Clean GET /patrons controller
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 27855
  Show dependency treegraph
 
Reported: 2021-03-03 19:35 UTC by Tomás Cohen Arazi
Modified: 2021-06-14 21:34 UTC (History)
4 users (show)

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


Attachments
Bug 27854: Clean GET /patrons controller (3.43 KB, patch)
2021-03-03 19:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27854: Clean GET /patrons controller (3.48 KB, patch)
2021-03-04 09:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 27854: Clean GET /patrons controller (3.54 KB, patch)
2021-03-04 10:40 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 2021-03-03 19:35:11 UTC
The controller has a hardcoded version of objects.search and should be cleaned up, to gain superpowers and make it extendable.
Comment 1 Tomás Cohen Arazi 2021-03-03 19:40:31 UTC
Created attachment 117624 [details] [review]
Bug 27854: Clean GET /patrons controller

The controller method was written a lot of time before the enhancements
we added to the objects.search helper, and it is now much easier to
handle the 'restricted' param use case.

No need to do it like that anymore.

This patch fetches the 'restricted' param from the query parameters and
cleans it from the validated data, so we can just pass the resultset to
$c->objects->search as in all other controllers.

And we had tests for the expected behavior, so testing this is as easy
as:

1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass! Of course!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 David Nind 2021-03-04 09:37:38 UTC
Created attachment 117655 [details] [review]
Bug 27854: Clean GET /patrons controller

The controller method was written a lot of time before the enhancements
we added to the objects.search helper, and it is now much easier to
handle the 'restricted' param use case.

No need to do it like that anymore.

This patch fetches the 'restricted' param from the query parameters and
cleans it from the validated data, so we can just pass the resultset to
$c->objects->search as in all other controllers.

And we had tests for the expected behavior, so testing this is as easy
as:

1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass! Of course!
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 2021-03-04 10:40:36 UTC
Created attachment 117675 [details] [review]
Bug 27854: Clean GET /patrons controller

The controller method was written a lot of time before the enhancements
we added to the objects.search helper, and it is now much easier to
handle the 'restricted' param use case.

No need to do it like that anymore.

This patch fetches the 'restricted' param from the query parameters and
cleans it from the validated data, so we can just pass the resultset to
$c->objects->search as in all other controllers.

And we had tests for the expected behavior, so testing this is as easy
as:

1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass! Of course!
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 2021-03-04 10:41:25 UTC
Nice bit of tidying up here.. great to see how far the API infrastructure has come.

Works as expected, tests all pass, QA script happy.

Passing QA
Comment 5 Jonathan Druart 2021-03-09 10:02:46 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 6 Fridolin Somers 2021-03-12 08:51:31 UTC
Enhancement not pushed to 20.11.x