Bug 37535 - Adding a debit via API will show the patron as the librarian that caused the debit
Summary: Adding a debit via API will show the patron as the librarian that caused the ...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Jan Kissig
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-07-31 09:37 UTC by Jan Kissig
Modified: 2024-08-01 16:27 UTC (History)
1 user (show)

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


Attachments
Bug 37535: Adding a debit via API will show the belated patron as the librarian that caused the debit (3.74 KB, patch)
2024-07-31 13:15 UTC, Jan Kissig
Details | Diff | Splinter Review
Bug 37535: Regression tests (5.81 KB, patch)
2024-07-31 16:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 37535: Adding a debit via API will show the belated patron as the librarian that caused the debit (3.80 KB, patch)
2024-07-31 16:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Kissig 2024-07-31 09:37:05 UTC
When POSTing 

{
  "amount": 1.17,
  "description": "some description",
  "internal_note": "internal_noteo",
  "type": "MANUAL",
  "user_id": 17
}

to http://localhost:8081/api/v1/patrons/5/account/debits

I would expect patron 5 to be debited by user_id 17. At least that is what the definition for debit says. But instead the belated patron is also shown as librarian in
http://localhost:8081/cgi-bin/koha/members/accountline-details.pl?accountlines_id=<ACCOUNTLINE FROM RESPONSE> 

When POSTing without user_id I would expect that the user_id is gained from the api user, but also the belated patron is shown as librarian.
Comment 1 Jan Kissig 2024-07-31 13:15:07 UTC
Created attachment 169887 [details] [review]
Bug 37535: Adding a debit via API will show the belated patron as the librarian that caused the debit

This patch fixes the addPatronDebit route so that the librarian that caused the debit is taken from either the requests payload user_id or if not set from the api user.

Test plan:

 a) enable system preference RESTBasicAuth
 b) use a REST client to send a POST request with the following JSON body to http://localhost:8081/api/v1/patrons/5/account/debits

{
  "amount": 1.23,
  "description": "some description",
  "internal_note": "internal_note",
  "type": "MANUAL"
}

 Authentication username and password is "koha"
 c) verify that "user_id" is the same as patron_id in response.
 d) send a different request including user_id to the same endpoint
{
  "amount": 1.23,
  "description": "some description",
  "internal_note": "internal_note",
  "type": "MANUAL",
  "user_id": 19
}

 e) verify that "user_id" is the same as patron_id in response.
 f) apply patch and repeat step b) and d)
 e) verify that user_id in b) is now 51 (which is the borrowernumber of koha user)
 f) verify that user_id in d) is now 19 as defined in request
 g) recheck on http://localhost:8081/cgi-bin/koha/members/accountline-details.pl?accountlines_id=<account_line_id> (from response) that column Librarian now says the user from user_id

 h) sign off :)
Comment 2 Tomás Cohen Arazi 2024-07-31 16:38:12 UTC
Created attachment 169905 [details] [review]
Bug 37535: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2024-07-31 16:38:15 UTC
Created attachment 169906 [details] [review]
Bug 37535: Adding a debit via API will show the belated patron as the librarian that caused the debit

This patch fixes the addPatronDebit route so that the librarian that caused the debit is taken from either the requests payload user_id or if not set from the api user.

Test plan:

 a) enable system preference RESTBasicAuth
 b) use a REST client to send a POST request with the following JSON body to http://localhost:8081/api/v1/patrons/5/account/debits

{
  "amount": 1.23,
  "description": "some description",
  "internal_note": "internal_note",
  "type": "MANUAL"
}

 Authentication username and password is "koha"
 c) verify that "user_id" is the same as patron_id in response.
 d) send a different request including user_id to the same endpoint
{
  "amount": 1.23,
  "description": "some description",
  "internal_note": "internal_note",
  "type": "MANUAL",
  "user_id": 19
}

 e) verify that "user_id" is the same as patron_id in response.
 f) apply patch and repeat step b) and d)
 e) verify that user_id in b) is now 51 (which is the borrowernumber of koha user)
 f) verify that user_id in d) is now 19 as defined in request
 g) recheck on http://localhost:8081/cgi-bin/koha/members/accountline-details.pl?accountlines_id=<account_line_id> (from response) that column Librarian now says the user from user_id

 h) sign off :)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi 2024-07-31 16:40:46 UTC
I wrote tests once I so you filed this bug. I confirm your patch fixes the issue.

A revisited test plan (using the tests) would be:

1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: Tests fail.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass :-D
Comment 5 Katrin Fischer 2024-08-01 16:27:43 UTC
Pushed for 24.11!

Well done everyone, thank you!