Bug 29290 - Add routes to fetch checkouts for a given biblio
Summary: Add routes to fetch checkouts for a given biblio
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 29288
Blocks: 29275 29380
  Show dependency treegraph
 
Reported: 2021-10-20 21:05 UTC by Tomás Cohen Arazi
Modified: 2022-06-06 20:25 UTC (History)
6 users (show)

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


Attachments
Bug 29290: Add spec changes (4.94 KB, patch)
2021-10-20 21:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29290: Add GET /biblios/:biblio_id/checkouts (1.59 KB, patch)
2021-10-20 21:13 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29290: Rename relationships borrower => patron (2.30 KB, patch)
2021-10-20 21:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29290: Add GET /biblios/:biblio_id/checkouts (1.56 KB, patch)
2021-10-20 21:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29290: Add spec changes (5.00 KB, patch)
2021-10-21 02:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 29290: Rename relationships borrower => patron (2.35 KB, patch)
2021-10-21 02:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 29290: Add GET /biblios/:biblio_id/checkouts (1.61 KB, patch)
2021-10-21 02:50 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 29290: Add spec changes (5.06 KB, patch)
2021-10-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29290: Rename relationships borrower => patron (2.42 KB, patch)
2021-10-27 08:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29290: Add GET /biblios/:biblio_id/checkouts (1.68 KB, patch)
2021-10-27 08:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29290: Unit tests (3.40 KB, patch)
2021-10-27 12:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29290: Unit tests (3.46 KB, patch)
2021-10-27 12:13 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-10-20 21:05:35 UTC
I propose we add

GET /biblios/:biblio_id/checkouts

it will have the same behavior as /checkouts, but filtering by the specified biblio.
Comment 1 Tomás Cohen Arazi 2021-10-20 21:13:38 UTC
Created attachment 126617 [details] [review]
Bug 29290: Add spec changes

This patch adds the new route. It also tweaks the checkout object
definition to allow embedding the required related objects (for bug 29275).

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2021-10-20 21:13:44 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2021-10-20 21:29:03 UTC
Created attachment 126622 [details] [review]
Bug 29290: Rename relationships borrower => patron

This is a trivial change that is required to be able to embed patron
objects in the (old) checkout object.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi 2021-10-20 21:33:19 UTC
Created attachment 126625 [details] [review]
Bug 29290: Add GET /biblios/:biblio_id/checkouts

This patch adds the required controller method.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Victor Grousset/tuxayo 2021-10-21 02:50:32 UTC
Created attachment 126637 [details] [review]
Bug 29290: Add spec changes

This patch adds the new route. It also tweaks the checkout object
definition to allow embedding the required related objects (for bug 29275).

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2021-10-21 02:50:36 UTC
Created attachment 126638 [details] [review]
Bug 29290: Rename relationships borrower => patron

This is a trivial change that is required to be able to embed patron
objects in the (old) checkout object.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Victor Grousset/tuxayo 2021-10-21 02:50:40 UTC
Created attachment 126639 [details] [review]
Bug 29290: Add GET /biblios/:biblio_id/checkouts

This patch adds the required controller method.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 8 Victor Grousset/tuxayo 2021-10-21 02:52:21 UTC
I tested bug 29275 and checked the network requests to confirm it uses the new route.
Comment 9 Martin Renvoize 2021-10-27 08:31:35 UTC
The only thing I wonder about is the 'checked_in' parameter.  I'm wondering if there's a use case for 'all' checkouts.. i.e both historic and current?

Perhaps a 'state' param instead with 'out, in, both' ?

Just a thought, let me know what you think?
Comment 10 Martin Renvoize 2021-10-27 08:37:52 UTC
Created attachment 126957 [details] [review]
Bug 29290: Add spec changes

This patch adds the new route. It also tweaks the checkout object
definition to allow embedding the required related objects (for bug 29275).

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2021-10-27 08:37:56 UTC
Created attachment 126958 [details] [review]
Bug 29290: Rename relationships borrower => patron

This is a trivial change that is required to be able to embed patron
objects in the (old) checkout object.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-10-27 08:38:00 UTC
Created attachment 126959 [details] [review]
Bug 29290: Add GET /biblios/:biblio_id/checkouts

This patch adds the required controller method.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2021-10-27 08:38:52 UTC
Of course.. the normal checkouts route already uses a 'checked_in' parameter so this is consistent.

All works as expected then, QA scripts happy. Passing QA
Comment 14 Martin Renvoize 2021-10-27 08:49:15 UTC
Hold off.. what's our current requirements regards API route testing?  I'm not seeing any tasts here.
Comment 15 Tomás Cohen Arazi 2021-10-27 12:08:08 UTC
Created attachment 126980 [details] [review]
Bug 29290: Unit tests

This patch adds unit tests for the new route. All behaviors are tested:

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2021-10-27 12:08:57 UTC
(In reply to Martin Renvoize from comment #14)
> Hold off.. what's our current requirements regards API route testing?  I'm
> not seeing any tasts here.

Good catch, I forgot!

Done!
Comment 17 Martin Renvoize 2021-10-27 12:13:54 UTC
Created attachment 126982 [details] [review]
Bug 29290: Unit tests

This patch adds unit tests for the new route. All behaviors are tested:

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2021-10-27 12:14:15 UTC
Nice one.. tests added and passing.. PQA proper.
Comment 19 Jonathan Druart 2021-10-28 15:45:25 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 20 Joonas Kylmälä 2021-10-30 19:17:32 UTC
This broke a few scripts, please review bug 29380 which contains fixes for those.
Comment 21 Jonathan Druart 2021-11-02 12:12:48 UTC
(In reply to Joonas Kylmälä from comment #20)
> This broke a few scripts, please review bug 29380 which contains fixes for
> those.

I completely overlooked that patch.

It was definitely a candidate for its own bug, it needs to be advertised in the release notes as well (may break plugins, notice templates, custom scripts, etc.). Also it would have been nice to rename other ->borrower methods then.
Comment 22 Tomás Cohen Arazi 2021-11-02 12:43:10 UTC
(In reply to Jonathan Druart from comment #21)
> (In reply to Joonas Kylmälä from comment #20)
> > This broke a few scripts, please review bug 29380 which contains fixes for
> > those.
> 
> I completely overlooked that patch.
> 
> It was definitely a candidate for its own bug, it needs to be advertised in
> the release notes as well (may break plugins, notice templates, custom
> scripts, etc.). Also it would have been nice to rename other ->borrower
> methods then.

I usually add new relationships, I have overlooked it myself.