Bugzilla – Attachment 69755 Details for
Bug 19784
Adapt /v1/patrons to new naming guidelines
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19784: Adapt /v1/patrons to new naming guidelines
Bug-19784-Adapt-v1patrons-to-new-naming-guidelines.patch (text/plain), 13.40 KB, created by
Tomás Cohen Arazi (tcohen)
on 2017-12-12 22:15:46 UTC
(
hide
)
Description:
Bug 19784: Adapt /v1/patrons to new naming guidelines
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2017-12-12 22:15:46 UTC
Size:
13.40 KB
patch
obsolete
>From b7fb10757b5ca876e8636a209adabbfcd551799e Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Fri, 8 Dec 2017 09:35:45 -0300 >Subject: [PATCH] Bug 19784: Adapt /v1/patrons to new naming guidelines > >This patch introduces two functions to the patrons endpoint: >- _to_api >- _to_model > >This are in charge of field mappings in order to comply with the >guidelines. > >Koha::REST::V1:Auth is adjusted to handle 'patron_id' as well. > >To test: >- Apply the patches >- Run: > $ kshell > k$ prove t/db_dependent/api/v1/patrons.t >=> SUCCESS: Tests pass! >- Sign off :-D >--- > Koha/REST/V1/Auth.pm | 8 ++-- > Koha/REST/V1/Patrons.pm | 76 +++++++++++++++++++++++++--------- > api/v1/swagger/definitions/patron.json | 14 +++---- > api/v1/swagger/parameters.json | 8 ++-- > api/v1/swagger/parameters/patron.json | 10 ++--- > api/v1/swagger/paths.json | 4 +- > api/v1/swagger/paths/patrons.json | 20 ++++----- > api/v1/swagger/x-primitives.json | 4 +- > 8 files changed, 91 insertions(+), 53 deletions(-) > >diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm >index 77864eb26d..e859dc0f73 100644 >--- a/Koha/REST/V1/Auth.pm >+++ b/Koha/REST/V1/Auth.pm >@@ -254,7 +254,7 @@ sub check_object_ownership { > > my $parameters = { > accountlines_id => \&_object_ownership_by_accountlines_id, >- borrowernumber => \&_object_ownership_by_borrowernumber, >+ patron_id => \&_object_ownership_by_patron_id, > checkout_id => \&_object_ownership_by_checkout_id, > reserve_id => \&_object_ownership_by_reserve_id, > }; >@@ -296,10 +296,10 @@ Compares C<$borrowernumber> to currently logged in C<$user>. > > =cut > >-sub _object_ownership_by_borrowernumber { >- my ($c, $user, $borrowernumber) = @_; >+sub _object_ownership_by_patron_id { >+ my ($c, $user, $patron_id) = @_; > >- return $user->borrowernumber == $borrowernumber; >+ return $user->borrowernumber == $patron_id; > } > > =head3 _object_ownership_by_checkout_id >diff --git a/Koha/REST/V1/Patrons.pm b/Koha/REST/V1/Patrons.pm >index 7868e8d148..7cd05b8b0a 100644 >--- a/Koha/REST/V1/Patrons.pm >+++ b/Koha/REST/V1/Patrons.pm >@@ -44,7 +44,8 @@ sub list { > > return try { > my $patrons_set = Koha::Patrons->new; >- my @patrons = $c->objects->search( $patrons_set )->as_list; >+ my @patrons = $c->objects->search( $patrons_set, \&_to_model )->as_list; >+ @patrons = map { _to_api($_->TO_JSON) } @patrons; > return $c->render( status => 200, openapi => \@patrons ); > } > catch { >@@ -70,14 +71,14 @@ Controller function that handles retrieving a single Koha::Patron object > sub get { > my $c = shift->openapi->valid_input or return; > >- my $borrowernumber = $c->validation->param('borrowernumber'); >- my $patron = Koha::Patrons->find($borrowernumber); >+ my $patron_id = $c->validation->param('patron_id'); >+ my $patron = Koha::Patrons->find($patron_id); > > unless ($patron) { >- return $c->render(status => 404, openapi => { error => "Patron not found." }); >+ return $c->render( status => 404, openapi => { error => "Patron not found." } ); > } > >- return $c->render(status => 200, openapi => $patron); >+ return $c->render( status => 200, openapi => _to_api($patron->TO_JSON) ); > } > > =head3 add >@@ -92,11 +93,11 @@ sub add { > return try { > my $body = $c->validation->param('body'); > >- Koha::Patron->new($body)->_validate; >+ Koha::Patron->new(_to_model($body))->_validate; > > # TODO: Use AddMember until it has been moved to Koha-namespace >- my $borrowernumber = AddMember(%$body); >- my $patron = Koha::Patrons->find($borrowernumber); >+ my $patron_id = AddMember( %{ _to_model($body) } ); >+ my $patron = _to_api(Koha::Patrons->find( $patron_id )->TO_JSON); > > return $c->render( status => 201, openapi => $patron ); > } >@@ -118,13 +119,13 @@ sub add { > elsif ( $_->isa('Koha::Exceptions::Library::BranchcodeNotFound') ) { > return $c->render( > status => 400, >- openapi => { error => "Given branchcode does not exist" } >+ openapi => { error => "Given library_id does not exist" } > ); > } > elsif ( $_->isa('Koha::Exceptions::Category::CategorycodeNotFound') ) { > return $c->render( > status => 400, >- openapi => { error => "Given categorycode does not exist" } >+ openapi => { error => "Given category_id does not exist" } > ); > } > else { >@@ -147,7 +148,7 @@ Controller function that handles updating a Koha::Patron object > sub update { > my $c = shift->openapi->valid_input or return; > >- my $patron = Koha::Patrons->find( $c->validation->param('borrowernumber') ); >+ my $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); > > return try { > my $body = $c->validation->param('body'); >@@ -158,7 +159,7 @@ sub update { > # Add borrowernumber to $body, as required by ModMember > $body->{borrowernumber} = $patron->borrowernumber; > >- if ( ModMember(%$body) ) { >+ if ( ModMember( %{ _to_model($body) } ) ) { > return $c->render( status => 200, openapi => $patron ); > } > else { >@@ -194,13 +195,13 @@ sub update { > elsif ( $_->isa('Koha::Exceptions::Library::BranchcodeNotFound') ) { > return $c->render( > status => 400, >- openapi => { error => "Given branchcode does not exist" } >+ openapi => { error => "Given library_id does not exist" } > ); > } > elsif ( $_->isa('Koha::Exceptions::Category::CategorycodeNotFound') ) { > return $c->render( > status => 400, >- openapi => { error => "Given categorycode does not exist" } >+ openapi => { error => "Given category_id does not exist" } > ); > } > elsif ( $_->isa('Koha::Exceptions::MissingParameter') ) { >@@ -251,7 +252,7 @@ sub delete { > my $patron; > > return try { >- $patron = Koha::Patrons->find( $c->validation->param('borrowernumber') ); >+ $patron = Koha::Patrons->find( $c->validation->param('patron_id') ); > > # check if loans, reservations, debarrment, etc. before deletion! > my $res = $patron->delete; >@@ -276,6 +277,31 @@ sub delete { > }; > } > >+=head3 _to_api >+ >+Helper function that maps Koha::Patron objects into REST api >+attribute names. >+ >+=cut >+ >+sub _to_api { >+ my $patron = shift; >+ >+ my $mapping = { >+ borrowernumber => 'patron_id', >+ branchcode => 'library_id', >+ categorycode => 'category_id' >+ }; >+ >+ foreach my $key ( keys %{ $mapping } ) { >+ if ( exists $patron->{ $key } ) { >+ $patron->{ $mapping->{$key} } = delete $patron->{ $key }; >+ } >+ } >+ >+ return $patron; >+} >+ > =head3 _to_model > > Helper function that maps REST api objects into Koha::Patron >@@ -284,12 +310,24 @@ attribute names. > =cut > > sub _to_model { >- my $params = shift; >+ my $patron = shift; >+ >+ my $mapping = { >+ category_id => 'categorycode', >+ library_id => 'branchcode', >+ patron_id => 'borrowernumber' >+ }; >+ >+ foreach my $key ( keys %{ $mapping } ) { >+ if ( exists $patron->{ $key } ) { >+ $patron->{ $mapping->{$key} } = delete $patron->{ $key }; >+ } >+ } > >- $params->{lost} = ($params->{lost}) ? 1 : 0; >- $params->{gonenoaddress} = ($params->{gonenoaddress}) ? 1 : 0; >+ $patron->{lost} = ($patron->{lost}) ? 1 : 0; >+ $patron->{gonenoaddress} = ($patron->{gonenoaddress}) ? 1 : 0; > >- return $params; >+ return $patron; > } > > 1; >diff --git a/api/v1/swagger/definitions/patron.json b/api/v1/swagger/definitions/patron.json >index 40b0e51f20..48ebaf5d32 100644 >--- a/api/v1/swagger/definitions/patron.json >+++ b/api/v1/swagger/definitions/patron.json >@@ -1,8 +1,8 @@ > { > "type": "object", > "properties": { >- "borrowernumber": { >- "$ref": "../x-primitives.json#/borrowernumber" >+ "patron_id": { >+ "$ref": "../x-primitives.json#/patron_id" > }, > "cardnumber": { > "$ref": "../x-primitives.json#/cardnumber" >@@ -124,13 +124,13 @@ > "format": "date", > "description": "patron's date of birth" > }, >- "branchcode": { >+ "library_id": { > "type": "string", >- "description": "code of patron's home branch" >+ "description": "Internal identifier for the patron's home library" > }, >- "categorycode": { >+ "category_id": { > "type": "string", >- "description": "code of patron's category" >+ "description": "Internal identifier for the patron's category" > }, > "dateenrolled": { > "type": ["string", "null"], >@@ -299,5 +299,5 @@ > } > }, > "additionalProperties": false, >- "required": ["surname", "address", "city", "branchcode", "categorycode"] >+ "required": ["surname", "address", "city", "library_id", "category_id"] > } >diff --git a/api/v1/swagger/parameters.json b/api/v1/swagger/parameters.json >index 1da158c3f4..0f2b5893af 100644 >--- a/api/v1/swagger/parameters.json >+++ b/api/v1/swagger/parameters.json >@@ -1,9 +1,9 @@ > { >- "borrowernumberPathParam": { >- "$ref": "parameters/patron.json#/borrowernumberPathParam" >+ "patron_id_pp": { >+ "$ref": "parameters/patron.json#/patron_id_pp" > }, >- "borrowernumberQueryParam": { >- "$ref": "parameters/patron.json#/borrowernumberQueryParam" >+ "patron_id_qp": { >+ "$ref": "parameters/patron.json#/patron_id_qp" > }, > "cityidPathParam": { > "$ref": "parameters/city.json#/cityidPathParam" >diff --git a/api/v1/swagger/parameters/patron.json b/api/v1/swagger/parameters/patron.json >index 409d0b2c35..d2114ba36f 100644 >--- a/api/v1/swagger/parameters/patron.json >+++ b/api/v1/swagger/parameters/patron.json >@@ -1,15 +1,15 @@ > { >- "borrowernumberPathParam": { >- "name": "borrowernumber", >+ "patron_id_pp": { >+ "name": "patron_id", > "in": "path", > "description": "Internal patron identifier", > "required": true, > "type": "integer" > }, >- "borrowernumberQueryParam": { >- "name": "borrowernumber", >+ "patron_id_qp": { >+ "name": "patron_id", > "in": "query", >- "description": "Internal borrower identifier", >+ "description": "Internal patron identifier", > "type": "integer" > } > } >diff --git a/api/v1/swagger/paths.json b/api/v1/swagger/paths.json >index 9c3a6d2efd..9fd947f02d 100644 >--- a/api/v1/swagger/paths.json >+++ b/api/v1/swagger/paths.json >@@ -20,8 +20,8 @@ > "/patrons": { > "$ref": "paths/patrons.json#/~1patrons" > }, >- "/patrons/{borrowernumber}": { >- "$ref": "paths/patrons.json#/~1patrons~1{borrowernumber}" >+ "/patrons/{patron_id}": { >+ "$ref": "paths/patrons.json#/~1patrons~1{patron_id}" > }, > "/illrequests": { > "$ref": "paths/illrequests.json#/~1illrequests" >diff --git a/api/v1/swagger/paths/patrons.json b/api/v1/swagger/paths/patrons.json >index 9790fdd080..c113b80c84 100644 >--- a/api/v1/swagger/paths/patrons.json >+++ b/api/v1/swagger/paths/patrons.json >@@ -8,9 +8,9 @@ > "application/json" > ], > "parameters": [{ >- "name": "borrowernumber", >+ "name": "patron_id", > "in": "query", >- "description": "Case insensitive search on borrowernumber", >+ "description": "Search on patron_id", > "required": false, > "type": "string" > }, { >@@ -200,15 +200,15 @@ > "required": false, > "type": "string" > }, { >- "name": "branchcode", >+ "name": "library_id", > "in": "query", >- "description": "Case insensitive search on branchcode", >+ "description": "Case insensitive search on library_id", > "required": false, > "type": "string" > }, { >- "name": "categorycode", >+ "name": "category_id", > "in": "query", >- "description": "Case insensitive search on categorycode", >+ "description": "Case insensitive search on category_id", > "required": false, > "type": "string" > }, { >@@ -563,13 +563,13 @@ > } > } > }, >- "/patrons/{borrowernumber}": { >+ "/patrons/{patron_id}": { > "get": { > "x-mojo-to": "Patrons#get", > "operationId": "getPatron", > "tags": ["patrons"], > "parameters": [{ >- "$ref": "../parameters.json#/borrowernumberPathParam" >+ "$ref": "../parameters.json#/patron_id_pp" > }], > "produces": [ > "application/json" >@@ -626,7 +626,7 @@ > "tags": ["patrons"], > "parameters": [ > { >- "$ref": "../parameters.json#/borrowernumberPathParam" >+ "$ref": "../parameters.json#/patron_id_pp" > }, > { > "name": "body", >@@ -703,7 +703,7 @@ > "operationId": "deletePatron", > "tags": ["patrons"], > "parameters": [{ >- "$ref": "../parameters.json#/borrowernumberPathParam" >+ "$ref": "../parameters.json#/patron_id_pp" > }], > "produces": ["application/json"], > "responses": { >diff --git a/api/v1/swagger/x-primitives.json b/api/v1/swagger/x-primitives.json >index 12e5e9de39..94961cb0c9 100644 >--- a/api/v1/swagger/x-primitives.json >+++ b/api/v1/swagger/x-primitives.json >@@ -3,9 +3,9 @@ > "type": "integer", > "description": "internally assigned biblio identifier" > }, >- "borrowernumber": { >+ "patron_id": { > "type": "integer", >- "description": "internally assigned user identifier" >+ "description": "Internal patron identifier" > }, > "cardnumber": { > "type": ["string", "null"], >-- >2.15.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 19784
:
69643
|
69644
|
69645
|
69753
|
69754
|
69755
|
70609
|
70610
|
70611
|
70612
|
71768
|
71769
|
71770
|
71959
|
71960
|
71961
|
71967
|
71968
|
71969
|
72026
|
72027
|
72028
|
72171
|
72172
|
72173