Bug 21786 - Routes for credits should include library_id
Summary: Routes for credits should include library_id
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Alex Arnaud
URL:
Keywords:
Depends on: 19066 20942 20944
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-08 14:00 UTC by Tomás Cohen Arazi
Modified: 2020-01-06 20:15 UTC (History)
3 users (show)

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


Attachments
Bug 21786: Add library_id to the spec definitions (1.53 KB, patch)
2018-11-09 19:36 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21786: Unit tests (3.33 KB, patch)
2018-11-09 19:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21786: Make balance and credits endpoints handle library_id (2.34 KB, patch)
2018-11-09 19:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21786: Add library_id to the spec definitions (1.60 KB, patch)
2018-11-13 14:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21786: Unit tests (3.40 KB, patch)
2018-11-13 14:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21786: Make balance and credits endpoints handle library_id (2.41 KB, patch)
2018-11-13 14:55 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 21786: (QA follow-up) library_id can be null (1.58 KB, patch)
2018-12-27 14:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21786: Add library_id to the spec definitions (1.62 KB, patch)
2018-12-27 15:45 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 21786: Unit tests (3.43 KB, patch)
2018-12-27 15:45 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 21786: Make balance and credits endpoints handle library_id (2.44 KB, patch)
2018-12-27 15:45 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 21786: (QA follow-up) library_id can be null (1.62 KB, patch)
2018-12-27 15:45 UTC, Alex Arnaud
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 2018-11-08 14:00:54 UTC

    
Comment 1 Tomás Cohen Arazi 2018-11-09 19:36:56 UTC
Created attachment 82198 [details] [review]
Bug 21786: Add library_id to the spec definitions
Comment 2 Tomás Cohen Arazi 2018-11-09 19:37:06 UTC
Created attachment 82199 [details] [review]
Bug 21786: Unit tests
Comment 3 Tomás Cohen Arazi 2018-11-09 19:37:12 UTC
Created attachment 82200 [details] [review]
Bug 21786: Make balance and credits endpoints handle library_id

This patch:
- Adds the branchcode <-> library_id mapping in the controller
- Adds 'library_id' to the credit and account_line definitions
- Makes add_credit handle the library_id param, passing it to the
  underlying $account->add_credit method.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: Tests fail, because library_id is not mapped
(balance) and because it is not handled when adding a credit.
- Apply this patch
- Run:
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 4 Kyle M Hall 2018-11-13 14:55:02 UTC
Created attachment 82279 [details] [review]
Bug 21786: Add library_id to the spec definitions

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2018-11-13 14:55:14 UTC
Created attachment 82280 [details] [review]
Bug 21786: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2018-11-13 14:55:17 UTC
Created attachment 82281 [details] [review]
Bug 21786: Make balance and credits endpoints handle library_id

This patch:
- Adds the branchcode <-> library_id mapping in the controller
- Adds 'library_id' to the credit and account_line definitions
- Makes add_credit handle the library_id param, passing it to the
  underlying $account->add_credit method.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: Tests fail, because library_id is not mapped
(balance) and because it is not handled when adding a credit.
- Apply this patch
- Run:
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi 2018-12-27 14:45:18 UTC
Created attachment 83529 [details] [review]
Bug 21786: (QA follow-up) library_id can be null

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Alex Arnaud 2018-12-27 14:49:24 UTC
got this error:
{"errors":[{"message":"Expected string - got null.","path":"\/outstanding_debits\/lines\/0\/library_id"}],"status":500}

While retrieving an account with debits made before applying this patch.
Comment 9 Tomás Cohen Arazi 2018-12-27 14:51:22 UTC
(In reply to Alex Arnaud from comment #8)
> got this error:
> {"errors":[{"message":"Expected string - got
> null.","path":"\/outstanding_debits\/lines\/0\/library_id"}],"status":500}
> 
> While retrieving an account with debits made before applying this patch.

Are you sure you restarted Plack? (if you tested with the follow-up)
Comment 10 Alex Arnaud 2018-12-27 14:54:01 UTC
(In reply to Tomás Cohen Arazi from comment #9)
> (In reply to Alex Arnaud from comment #8)
> > got this error:
> > {"errors":[{"message":"Expected string - got
> > null.","path":"\/outstanding_debits\/lines\/0\/library_id"}],"status":500}
> > 
> > While retrieving an account with debits made before applying this patch.
> 
> Are you sure you restarted Plack? (if you tested with the follow-up)

The last patch arrived just before my comment. What a quick fix Thomas! :-)
Comment 11 Alex Arnaud 2018-12-27 15:45:25 UTC
Created attachment 83530 [details] [review]
Bug 21786: Add library_id to the spec definitions

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 12 Alex Arnaud 2018-12-27 15:45:32 UTC
Created attachment 83531 [details] [review]
Bug 21786: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 13 Alex Arnaud 2018-12-27 15:45:38 UTC
Created attachment 83532 [details] [review]
Bug 21786: Make balance and credits endpoints handle library_id

This patch:
- Adds the branchcode <-> library_id mapping in the controller
- Adds 'library_id' to the credit and account_line definitions
- Makes add_credit handle the library_id param, passing it to the
  underlying $account->add_credit method.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: Tests fail, because library_id is not mapped
(balance) and because it is not handled when adding a credit.
- Apply this patch
- Run:
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 14 Alex Arnaud 2018-12-27 15:45:46 UTC
Created attachment 83533 [details] [review]
Bug 21786: (QA follow-up) library_id can be null

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Comment 15 Nick Clemens 2019-01-08 15:55:37 UTC
Awesome work all!

Pushed to master for 19.05
Comment 16 Martin Renvoize 2019-01-10 14:12:43 UTC
Depends on enhancement in bug 19066, will not be backported to 18.11.x series.