Bug 35353 - Add API endpoint to fetch patron's previous holds
Summary: Add API endpoint to fetch patron's previous holds
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on: 35610
Blocks: 35560
  Show dependency treegraph
 
Reported: 2023-11-16 15:14 UTC by Kyle M Hall
Modified: 2024-04-02 18:46 UTC (History)
5 users (show)

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


Attachments
Bug 35353: Add api endpoint to fetch patron's previous holds (7.07 KB, patch)
2023-11-16 15:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35353: Add api endpoint to fetch patron's previous holds (7.12 KB, patch)
2023-11-16 21:52 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35353: DBIC specific (1.46 KB, patch)
2024-03-25 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35353: Add REST API endpoint to retrieve old holds (11.78 KB, patch)
2024-03-25 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 35353: DBIC specific (1.54 KB, patch)
2024-03-25 16:31 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35353: Add REST API endpoint to retrieve old holds (11.84 KB, patch)
2024-03-25 16:31 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 35353: DBIC specific (1.58 KB, patch)
2024-04-02 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 35353: Add REST API endpoint to retrieve old holds (11.90 KB, patch)
2024-04-02 18:46 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2023-11-16 15:14:49 UTC
We have an endpoint to fetch a patron's current holds ( /api/va/patrons/$id/holds ) but some external services need to get previous holds to see if they were completed/canceled/etc.
Comment 1 Kyle M Hall 2023-11-16 15:17:11 UTC
Created attachment 159055 [details] [review]
Bug 35353: Add api endpoint to fetch patron's previous holds

We have an endpoint to fetch a patron's current holds ( /api/va/patrons/$id/holds ) but some external services need to get previous holds to see if they were completed/canceled/etc.

Test Plan:
1) Create some holds for a patron
2) Complete or cancel those holds
3) Browse to /api/va/patrons/$id/holds/previous where $id is the
   borrowernumber
4) You should get a list of previous holds!
Comment 2 ByWater Sandboxes 2023-11-16 21:52:12 UTC
Created attachment 159067 [details] [review]
Bug 35353: Add api endpoint to fetch patron's previous holds

We have an endpoint to fetch a patron's current holds ( /api/va/patrons/$id/holds ) but some external services need to get previous holds to see if they were completed/canceled/etc.

Test Plan:
1) Create some holds for a patron
2) Complete or cancel those holds
3) Browse to /api/va/patrons/$id/holds/previous where $id is the
   borrowernumber
4) You should get a list of previous holds!

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 3 Andrew Fuerste-Henry 2023-11-16 21:52:53 UTC
> 3) Browse to /api/va/patrons/$id/holds/previous where $id is the
>    borrowernumber

Note this should be "/api/v1/patrons..."
Comment 4 Tomás Cohen Arazi 2023-12-15 21:17:27 UTC
On the original implementation for bug 20936, we implemented this on the API... it is sad how it become a rabbit hole and we dropped that dev entirely! (we were requested to 'first solve how Koha handles timezones in dates on the API'!).

Anyway, in that context I remembered the discussion...

As opposed to what happens with biblios, items and patrons, which when deleted are really just put in a vault until a maintenance tool decides to clear them for good, checkouts, holds, etc are transactions which change their statuses, and that status can just be 'finished' or 'completed'.

As 'reserves' and 'old_reserves' are not yet merged (maybe this cycle?) we don't have a clean way to implement it. But on bug 20936 we overcome this by adding an 'old=1' query parameter on the current */holds endpoint. The advantage is that, if the tables are merged, we can just translate old=1 into $rs->filter_by_complete or similar, keeping the behavior.

TL;DR

1. The ideal would be to implement this on top of bug 25260, with 'completed' being just another (set of?) status value which we can filter on. Adding a special query parameter to make it return completed holds seems fairly straight-forward.
2. Implementing this in the spirit of bug 20936 (see [1]) could work without needing bug 25260, and keeping forward compatibility, behavior and query parameters wise.

I feel that's the way to go.

My 2 cents.

[1] https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100473
Comment 5 Jonathan Druart 2023-12-19 08:31:01 UTC
I have started this as well (on bug 35560), and just copied what we are doing for checkouts: we are passing a checked_in flag to /checkouts

Why not doing the same for holds?
Comment 6 Martin Renvoize 2024-01-26 09:54:47 UTC
I agree with the two above comments.. lets use &old=1 as per checkouts and it leaves the door open to the table merging bug.

I'm going to go ahead and mark this one as a duplicate.

*** This bug has been marked as a duplicate of bug 35560 ***
Comment 7 Jonathan Druart 2024-03-25 16:21:29 UTC
Created attachment 163828 [details] [review]
Bug 35353: DBIC specific
Comment 8 Jonathan Druart 2024-03-25 16:21:31 UTC
Created attachment 163829 [details] [review]
Bug 35353: Add REST API endpoint to retrieve old holds

Same as checkout but for holds, we need to provide a way to retrieve old
holds for a patron.

Test plan:
Create some holds for a patron, cancel and fulfill some, then use the
REST API endpoint with the new 'old' flag set to 1
  /api/v1/patrons/42/holds?old=1
Comment 9 Matt Blenkinsop 2024-03-25 16:31:14 UTC
Created attachment 163833 [details] [review]
Bug 35353: DBIC specific

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 10 Matt Blenkinsop 2024-03-25 16:31:16 UTC
Created attachment 163834 [details] [review]
Bug 35353: Add REST API endpoint to retrieve old holds

Same as checkout but for holds, we need to provide a way to retrieve old
holds for a patron.

Test plan:
Create some holds for a patron, cancel and fulfill some, then use the
REST API endpoint with the new 'old' flag set to 1
  /api/v1/patrons/42/holds?old=1

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 11 Nick Clemens 2024-04-02 18:46:35 UTC
Created attachment 164298 [details] [review]
Bug 35353: DBIC specific

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Nick Clemens 2024-04-02 18:46:37 UTC
Created attachment 164299 [details] [review]
Bug 35353: Add REST API endpoint to retrieve old holds

Same as checkout but for holds, we need to provide a way to retrieve old
holds for a patron.

Test plan:
Create some holds for a patron, cancel and fulfill some, then use the
REST API endpoint with the new 'old' flag set to 1
  /api/v1/patrons/42/holds?old=1

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>