Bug 21786

Summary: Routes for credits should include library_id
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Alex Arnaud <alex.arnaud>
Severity: normal    
Priority: P5 - low CC: alex.arnaud, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 19066, 20942, 20944    
Bug Blocks:    
Attachments: Bug 21786: Add library_id to the spec definitions
Bug 21786: Unit tests
Bug 21786: Make balance and credits endpoints handle library_id
Bug 21786: Add library_id to the spec definitions
Bug 21786: Unit tests
Bug 21786: Make balance and credits endpoints handle library_id
Bug 21786: (QA follow-up) library_id can be null
Bug 21786: Add library_id to the spec definitions
Bug 21786: Unit tests
Bug 21786: Make balance and credits endpoints handle library_id
Bug 21786: (QA follow-up) library_id can be null

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.