Bug 38932 - Adding debits and credits should return the correct Location header
Summary: Adding debits and credits should return the correct Location header
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Matt Blenkinsop
URL:
Keywords:
Depends on: 38931
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-20 20:01 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-01-24 15:15 UTC (History)
5 users (show)

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


Attachments
Bug 38932: Regression tests (2.16 KB, patch)
2025-01-20 20:08 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38932: Make adding a credit return the Location header (1.33 KB, patch)
2025-01-20 20:08 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38932: Regression tests (2.20 KB, patch)
2025-01-22 10:26 UTC, David Nind
Details | Diff | Splinter Review
Bug 38932: Make adding a credit return the Location header (1.38 KB, patch)
2025-01-22 10:26 UTC, David Nind
Details | Diff | Splinter Review
Bug 38932: Regression tests (2.27 KB, patch)
2025-01-24 12:22 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38932: Make adding a credit return the Location header (1.44 KB, patch)
2025-01-24 12:22 UTC, Matt Blenkinsop
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 (tcohen) 2025-01-20 20:01:09 UTC
Adding credits is not setting the Location header for the response. Adding debits does, but before bug 38931 it pointed to a non-existent endpoint.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-01-20 20:08:22 UTC
Created attachment 176842 [details] [review]
Bug 38932: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2025-01-20 20:08:24 UTC
Created attachment 176843 [details] [review]
Bug 38932: Make adding a credit return the Location header

This patch makes the `POST /patrons/:patron_id/account/credits` endpoint
return a valid `Location` header. Tests are added.

Bonus: the debits counterpart already returned the header, but pointed
to a non-existent endpoint (now added by bug 38931) and it gets tests
added.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: The behavior is not implemented for credits.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 David Nind 2025-01-22 10:26:54 UTC
Created attachment 176896 [details] [review]
Bug 38932: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2025-01-22 10:26:56 UTC
Created attachment 176897 [details] [review]
Bug 38932: Make adding a credit return the Location header

This patch makes the `POST /patrons/:patron_id/account/credits` endpoint
return a valid `Location` header. Tests are added.

Bonus: the debits counterpart already returned the header, but pointed
to a non-existent endpoint (now added by bug 38931) and it gets tests
added.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: The behavior is not implemented for credits.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2025-01-22 10:28:18 UTC
Testing notes (using KTD):

1. Apply 38931
2. Run yarn api:bundle
3. Follow the test plan for this bug (38932)
Comment 6 Matt Blenkinsop 2025-01-24 12:22:04 UTC
Created attachment 177111 [details] [review]
Bug 38932: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 7 Matt Blenkinsop 2025-01-24 12:22:07 UTC
Created attachment 177112 [details] [review]
Bug 38932: Make adding a credit return the Location header

This patch makes the `POST /patrons/:patron_id/account/credits` endpoint
return a valid `Location` header. Tests are added.

Bonus: the debits counterpart already returned the header, but pointed
to a non-existent endpoint (now added by bug 38931) and it gets tests
added.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: The behavior is not implemented for credits.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 8 Katrin Fischer 2025-01-24 15:15:27 UTC
Pushed for 25.05!

Well done everyone, thank you!