Bug 36075 - Add API route to list recalls
Summary: Add API route to list recalls
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 35967
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-12 23:10 UTC by Aleisha Amohia
Modified: 2024-03-11 13:58 UTC (History)
2 users (show)

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


Attachments
Bug 36075: Add REST API endpoint to list recalls (9.30 KB, patch)
2024-02-13 01:21 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 36075: Add REST API endpoint to list recalls (9.26 KB, patch)
2024-02-13 08:52 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 36075: Add REST API endpoint to list recalls (9.32 KB, patch)
2024-02-20 10:23 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36075: QA Follow-up: Fix tests (1.26 KB, patch)
2024-02-20 10:23 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36075: (QA follow-up): Fix tests (1.90 KB, patch)
2024-02-20 10:27 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2024-02-12 23:10:34 UTC

    
Comment 1 Aleisha Amohia 2024-02-13 01:21:55 UTC
Created attachment 162075 [details] [review]
Bug 36075: Add REST API endpoint to list recalls

This enhancement adds a REST API endpoint to list recalls and run search queries on recalls

/api/v1/recalls

/api/v1/recalls?patron_id=X

To test:

1. Log in to the staff interface as your superlibrarian self (Patron A)
2. Go to Koha Administration -> Global system preferences. Enable the UseRecalls system preference
3. Set the relevant recalls circulation and fines rules
4. Search for an item (Item A)
5. Check out Item A to yourself (Patron A)
6. Log in to the OPAC as Patron B
7. Search for Item A and request a recall
8. Test the API returns the recall: https://your-opac-url/api/v1/recalls
9. Test other search parameters: https://your-opac-url/api/v1/recalls?patron_id=PATRON_B_NUMBER
10. Confirm you are able to view a list of Patron B's recalls
11. Confirm tests pass: t/db_dependent/api/v1/recalls.t

Sponsored-by: Auckland University of Technology
Comment 2 Aleisha Amohia 2024-02-13 08:52:07 UTC
Created attachment 162087 [details] [review]
Bug 36075: Add REST API endpoint to list recalls

This enhancement adds a REST API endpoint to list recalls and run search queries on recalls

/api/v1/recalls

/api/v1/recalls?patron_id=X

To test:

1. Log in to the staff interface as your superlibrarian self (Patron A)
2. Go to Koha Administration -> Global system preferences. Enable the UseRecalls system preference
3. Set the relevant recalls circulation and fines rules
4. Search for an item (Item A)
5. Check out Item A to yourself (Patron A)
6. Log in to the OPAC as Patron B
7. Search for Item A and request a recall
8. Test the API returns the recall: https://your-opac-url/api/v1/recalls
9. Test other search parameters: https://your-opac-url/api/v1/recalls?patron_id=PATRON_B_NUMBER
10. Confirm you are able to view a list of Patron B's recalls
11. Confirm tests pass: t/db_dependent/api/v1/recalls.t

Sponsored-by: Auckland University of Technology
Comment 3 David Nind 2024-02-19 14:58:14 UTC
The tests failed for me 8-(:

prove -v t/db_dependent/api/v1/recalls.t
t/db_dependent/api/v1/recalls.t .. 
1..1
# Subtest: list() tests
    1..9
    ok 1 - GET //u47rPv2aNmke8fdORvXFwavpqwMprAZRXAGEIbO3a8:thePassword123@/api/v1/recalls
    ok 2 - SWAGGER3.2.2
    not ok 3 - exact match for JSON Pointer ""

    #   Failed test 'exact match for JSON Pointer ""'
    #   at t/db_dependent/api/v1/recalls.t line 51.
    #     Structures begin differing at:
    #          $got->[0] = HASH(0x5599d5bbb7f8)
    #     $expected->[0] = Does not exist
    ok 4 - GET //u47rPv2aNmke8fdORvXFwavpqwMprAZRXAGEIbO3a8:thePassword123@/api/v1/recalls?patron_id=55
    ok 5 - SWAGGER3.2.2
    ok 6 - Recalls for patron retrieved
    ok 7 - GET //u47rPv2aNmke8fdORvXFwavpqwMprAZRXAGEIbO3a8:thePassword123@/api/v1/recalls?completed=false
    ok 8 - SWAGGER3.2.2
    not ok 9 - Current recalls retrieved

    #   Failed test 'Current recalls retrieved'
    #   at t/db_dependent/api/v1/recalls.t line 60.
    #     Structures begin differing at:
    #          $got->[0]{waiting_date} = undef
    #     $expected->[0]{waiting_date} = '2024-02-19T14:53:59+00:00'
    # Looks like you failed 2 tests of 9.
not ok 1 - list() tests

#   Failed test 'list() tests'
#   at t/db_dependent/api/v1/recalls.t line 64.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests 

Test Summary Report
-------------------
t/db_dependent/api/v1/recalls.t (Wstat: 256 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 1
Files=1, Tests=1,  6 wallclock secs ( 0.01 usr  0.00 sys +  4.86 cusr  0.46 csys =  5.33 CPU)
Result: FAIL
Comment 4 Pedro Amorim 2024-02-20 10:23:56 UTC
Created attachment 162290 [details] [review]
Bug 36075: Add REST API endpoint to list recalls

This enhancement adds a REST API endpoint to list recalls and run search queries on recalls

/api/v1/recalls

/api/v1/recalls?patron_id=X

To test:

1. Log in to the staff interface as your superlibrarian self (Patron A)
2. Go to Koha Administration -> Global system preferences. Enable the UseRecalls system preference
3. Set the relevant recalls circulation and fines rules
4. Search for an item (Item A)
5. Check out Item A to yourself (Patron A)
6. Log in to the OPAC as Patron B
7. Search for Item A and request a recall
8. Test the API returns the recall: https://your-opac-url/api/v1/recalls
9. Test other search parameters: https://your-opac-url/api/v1/recalls?patron_id=PATRON_B_NUMBER
10. Confirm you are able to view a list of Patron B's recalls
11. Confirm tests pass: t/db_dependent/api/v1/recalls.t

Sponsored-by: Auckland University of Technology
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 5 Pedro Amorim 2024-02-20 10:23:58 UTC
Created attachment 162291 [details] [review]
Bug 36075: QA Follow-up: Fix tests

recall_1 needs to specifically be completed => 0 or else it'll sometimes be 1 and other times be 0 when running tests, causing tests to sometimes fails and other times succeed

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 6 Pedro Amorim 2024-02-20 10:27:43 UTC
Created attachment 162292 [details] [review]
Bug 36075: (QA follow-up): Fix tests

recall_1 needs to specifically be completed => 0 or else it'll sometimes be 1 and other times be 0 when running tests, causing tests to sometimes fails and other times succeed

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 7 Aleisha Amohia 2024-02-20 20:04:57 UTC
(In reply to Pedro Amorim from comment #6)
> Created attachment 162292 [details] [review] [review]
> Bug 36075: (QA follow-up): Fix tests
> 
> recall_1 needs to specifically be completed => 0 or else it'll sometimes be
> 1 and other times be 0 when running tests, causing tests to sometimes fails
> and other times succeed
> 
> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

That makes sense! Thank you for the follow up :)
Comment 8 Tomás Cohen Arazi 2024-03-11 13:50:11 UTC
Aleisha, do you really need those query parameters you are adding here? We are not adding them lately.
Comment 9 Tomás Cohen Arazi 2024-03-11 13:58:22 UTC
Please:

* Remove (inline) the extra query parameters. I think keeping 'completed' makes sense, though.
* Review the documentation in the spec, as it (at least) mentions "patron's recalls" in one place.
* The json_is( '' => [...], 'Current recalls....' ) is not something we use. In such cases you don't need to add the test description as it is clear that you are testing the response structure matches something. Not a blocker.

the rest of it is perfect :-D