Bug 17428 - REST API: CRUD endpoint for cities
Summary: REST API: CRUD endpoint for cities
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 17425 17445
Blocks: 14974
  Show dependency treegraph
 
Reported: 2016-10-11 13:43 UTC by Tomás Cohen Arazi
Modified: 2017-06-14 22:18 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
This patch introduces full CRUD for Koha::Cit(ies|y) classes through the REST api. To test, point your browser to /api/v1/cities to use HTTPRequester/Postman (or the like). It is a full example about the creation of an REST API in Koha. Read and copy it for others REST API.


Attachments
Bug 17428: [REST] Cities endpoint unit tests (11.47 KB, patch)
2016-10-11 13:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: [REST] Cities swagger specification (9.15 KB, patch)
2016-10-11 13:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: [REST] Koha::REST::V1::Cities introduced (4.48 KB, patch)
2016-10-11 13:45 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: [REST] Cities endpoint unit tests (11.52 KB, patch)
2016-10-12 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17428: [REST] Cities swagger specification (9.20 KB, patch)
2016-10-12 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17428: [REST] Koha::REST::V1::Cities introduced (4.54 KB, patch)
2016-10-12 07:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17428: [REST] Koha::REST::V1::Cities introduced (4.60 KB, patch)
2016-10-12 12:25 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 174828: Followup - More RESTfull handling of endpoints (15.16 KB, patch)
2016-10-13 09:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 174828: Followup - More RESTfull handling of endpoints (15.16 KB, patch)
2016-10-14 10:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: Just some refactoring (2.70 KB, patch)
2016-10-14 14:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: Move the params check after the authentication check (2.49 KB, patch)
2016-10-14 14:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: Remove debug statement (663 bytes, patch)
2016-10-14 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 174828: QA Followup - Add readOnly marker to cityid (4.63 KB, patch)
2016-10-14 15:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: [REST] Cities endpoint unit tests (11.52 KB, patch)
2016-10-14 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: [REST] Cities swagger specification (9.20 KB, patch)
2016-10-14 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: [REST] Koha::REST::V1::Cities introduced (4.60 KB, patch)
2016-10-14 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: Followup - More RESTfull handling of endpoints (15.11 KB, patch)
2016-10-14 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: Remove debug statement (663 bytes, patch)
2016-10-14 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: QA Followup - Add readOnly marker to cityid (4.58 KB, patch)
2016-10-14 15:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
QA Followup - Fix failing test (1.21 KB, patch)
2016-10-21 15:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: [REST best practice Followup] (17.95 KB, patch)
2016-10-24 08:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17428: [REST] Cities endpoint unit tests (11.58 KB, patch)
2016-10-24 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: [REST] Cities swagger specification (9.26 KB, patch)
2016-10-24 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: [REST] Koha::REST::V1::Cities introduced (4.66 KB, patch)
2016-10-24 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: [REST best practice Followup] (18.01 KB, patch)
2016-10-24 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: Remove warning (782 bytes, patch)
2016-10-24 15:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17428: [REST] Cities endpoint unit tests (11.63 KB, patch)
2016-10-24 16:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: [REST] Cities swagger specification (9.31 KB, patch)
2016-10-24 16:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: [REST] Koha::REST::V1::Cities introduced (4.71 KB, patch)
2016-10-24 16:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: [REST] best practice followup (18.07 KB, patch)
2016-10-24 16:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: (QA followup) Remove warning (851 bytes, patch)
2016-10-24 16:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17428: (QA followup) Fix community site in swagger file (846 bytes, patch)
2016-10-24 16:08 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 Tomás Cohen Arazi 2016-10-11 13:43:12 UTC
Add an endpoint for handling cities.
Comment 1 Tomás Cohen Arazi 2016-10-11 13:45:11 UTC
Created attachment 56189 [details] [review]
Bug 17428: [REST] Cities endpoint unit tests

This patch introduces unit tests for the cities endpoint.
Comment 2 Tomás Cohen Arazi 2016-10-11 13:45:28 UTC
Created attachment 56190 [details] [review]
Bug 17428: [REST] Cities swagger specification

This patch adds the swagger definitions for the /cities endpoint
Comment 3 Tomás Cohen Arazi 2016-10-11 13:45:40 UTC
Created attachment 56191 [details] [review]
Bug 17428: [REST] Koha::REST::V1::Cities introduced

This patch introduces full CRUD for Koha::Cit(ies|y) classes through
the REST api. To test, point your browser to /api/v1/cities to use
HTTPRequester/Postman (or the like).

And of course, run the unit tests:
- Apply the patches
- Update your minified swagger file:
  $ cd kohaclone/
  $ misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json \
                                -d api/v1/swagger/swagger.min.json
- Run:
  $ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests should return green, and no warnings.
- Sign off :-D
Comment 4 Claire Gravely 2016-10-12 07:41:42 UTC
I'm getting raptor not found when pointing my browser at it!
Comment 5 Claire Gravely 2016-10-12 07:42:45 UTC
(In reply to Claire Gravely from comment #4)
> I'm getting raptor not found when pointing my browser at it!

ignore me!
Comment 6 Josef Moravec 2016-10-12 07:51:30 UTC
Created attachment 56226 [details] [review]
Bug 17428: [REST] Cities endpoint unit tests

This patch introduces unit tests for the cities endpoint.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Josef Moravec 2016-10-12 07:51:36 UTC
Created attachment 56227 [details] [review]
Bug 17428: [REST] Cities swagger specification

This patch adds the swagger definitions for the /cities endpoint

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Josef Moravec 2016-10-12 07:51:43 UTC
Created attachment 56228 [details] [review]
Bug 17428: [REST] Koha::REST::V1::Cities introduced

This patch introduces full CRUD for Koha::Cit(ies|y) classes through
the REST api. To test, point your browser to /api/v1/cities to use
HTTPRequester/Postman (or the like).

And of course, run the unit tests:
- Apply the patches
- Update your minified swagger file:
  $ cd kohaclone/
  $ misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json \
                                -d api/v1/swagger/swagger.min.json
- Run:
  $ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests should return green, and no warnings.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Claire Gravely 2016-10-12 12:25:36 UTC
Created attachment 56261 [details] [review]
Bug 17428: [REST] Koha::REST::V1::Cities introduced

This patch introduces full CRUD for Koha::Cit(ies|y) classes through
the REST api. To test, point your browser to /api/v1/cities to use
HTTPRequester/Postman (or the like).

And of course, run the unit tests:
- Apply the patches
- Update your minified swagger file:
  $ cd kohaclone/
  $ misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json \
                                -d api/v1/swagger/swagger.min.json
- Run:
  $ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests should return green, and no warnings.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Comment 10 Martin Renvoize 2016-10-13 09:17:02 UTC
Created attachment 56314 [details] [review]
Bug 174828: Followup - More RESTfull handling of endpoints

* PUT should always be full objects, and not partial updates (use PATCH
  for partials)
* Validate query parameters instead of blindly passing them to the model
* Update tests to check for swagger validation errors instead of koha exceptions
Comment 11 Martin Renvoize 2016-10-13 09:24:50 UTC
I have added a followup with some minor alterations to encourage Restful best practice a little more.  The biggest change is to always expect full object for PUT requests.

We have some options to discus should we wish to implement partial updates, but I believe that should be left for it's own bug.

Just as a quick overview, we have two options for partial updates:

1) Individual endpoints (hopefully an automated process to create them could be developed) for each field in the object.
2) Defining a HTTP PATCH method for each overall object endpoint using somthing along the lines of JSON-Patch (http://jsonpatch.com/) or JSONT (JSON Transforms) for delivering patch instructions
Comment 12 Jonathan Druart 2016-10-13 14:37:44 UTC
Comment on attachment 56314 [details] [review]
Bug 174828: Followup - More RESTfull handling of endpoints

Review of attachment 56314 [details] [review]:
-----------------------------------------------------------------

::: t/db_dependent/api/v1/cities.t
@@ +96,5 @@
>      $tx = $t->ua->build_tx( GET => '/api/v1/cities?city_blah=blah' );
>      $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
>      $tx->req->env( { REMOTE_ADDR => $remote_address } );
> +    $t->request_ok($tx)->status_is(200)
> +      ->json_is( [ $city, $another_city, $city_with_another_country ] );

I'd like to make sure this is the expected behaviour. It will make debug less easier.
Could you find resources/best practices about that please?
Comment 13 Jonathan Druart 2016-10-13 15:15:54 UTC
POST with id return html with "<head><title>Server error</title></head>"

DBD::mysql::st execute failed: Duplicate entry '1' for key 'PRIMARY'

Not sure it's expected ;)
Comment 14 Jonathan Druart 2016-10-13 15:18:31 UTC
POST with non-existing id allows me to choose the id.
Comment 15 Martin Renvoize 2016-10-14 08:02:35 UTC
(In reply to Jonathan Druart from comment #12)
> Comment on attachment 56314 [details] [review] [review]
> Bug 174828: Followup - More RESTfull handling of endpoints
> 
> Review of attachment 56314 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: t/db_dependent/api/v1/cities.t
> @@ +96,5 @@
> >      $tx = $t->ua->build_tx( GET => '/api/v1/cities?city_blah=blah' );
> >      $tx->req->cookies( { name => 'CGISESSID', value => $session_id } );
> >      $tx->req->env( { REMOTE_ADDR => $remote_address } );
> > +    $t->request_ok($tx)->status_is(200)
> > +      ->json_is( [ $city, $another_city, $city_with_another_country ] );
> 
> I'd like to make sure this is the expected behaviour. It will make debug
> less easier.
> Could you find resources/best practices about that please?

From an api consumers stand point I totally agree it's nicer to receive an error as apposed to silently throwing away the incorrect parameters.

I'm going to implement a standard error response for such a case in it's own bug and make this patch depend upon it.
Comment 16 Martin Renvoize 2016-10-14 10:07:13 UTC
Created attachment 56501 [details] [review]
Bug 174828: Followup - More RESTfull handling of endpoints

* PUT should always be full objects, and not partial updates (use PATCH
  for partials)
* Validate query parameters instead of blindly passing them to the model
* Update tests to check for swagger validation errors instead of koha exceptions
Comment 17 Jonathan Druart 2016-10-14 14:21:09 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2016-10-14 14:21:13 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2016-10-14 14:27:19 UTC
Created attachment 56548 [details] [review]
Bug 17428: Remove debug statement
Comment 20 Martin Renvoize 2016-10-14 15:26:13 UTC
Created attachment 56553 [details] [review]
Bug 174828: QA Followup - Add readOnly marker to cityid

This should prevent api consumers from specifying an id on PUT or
POST requests and thus restrict them from creating new cities with
id's of their choosing, or worse updating cities via the POST method
Comment 21 Martin Renvoize 2016-10-14 15:31:11 UTC
Created attachment 56554 [details] [review]
Bug 17428: [REST] Cities endpoint unit tests

This patch introduces unit tests for the cities endpoint.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 22 Martin Renvoize 2016-10-14 15:31:32 UTC
Created attachment 56555 [details] [review]
Bug 17428: [REST] Cities swagger specification

This patch adds the swagger definitions for the /cities endpoint

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 23 Martin Renvoize 2016-10-14 15:31:38 UTC
Created attachment 56556 [details] [review]
Bug 17428: [REST] Koha::REST::V1::Cities introduced

This patch introduces full CRUD for Koha::Cit(ies|y) classes through
the REST api. To test, point your browser to /api/v1/cities to use
HTTPRequester/Postman (or the like).

And of course, run the unit tests:
- Apply the patches
- Update your minified swagger file:
  $ cd kohaclone/
  $ misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json \
                                -d api/v1/swagger/swagger.min.json
- Run:
  $ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests should return green, and no warnings.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Comment 24 Martin Renvoize 2016-10-14 15:31:44 UTC
Created attachment 56557 [details] [review]
Bug 17428: Followup - More RESTfull handling of endpoints

* PUT should always be full objects, and not partial updates (use PATCH
  for partials)
* Validate query parameters instead of blindly passing them to the model
* Update tests to check for swagger validation errors instead of koha exceptions
Comment 25 Martin Renvoize 2016-10-14 15:31:49 UTC
Created attachment 56558 [details] [review]
Bug 17428: Remove debug statement
Comment 26 Martin Renvoize 2016-10-14 15:31:54 UTC
Created attachment 56559 [details] [review]
Bug 17428: QA Followup - Add readOnly marker to cityid

This should prevent api consumers from specifying an id on PUT or
POST requests and thus restrict them from creating new cities with
id's of their choosing, or worse updating cities via the POST method
Comment 27 Martin Renvoize 2016-10-14 15:34:22 UTC
Please note, the last followup include a currently failing test case.. The failure is in the upstream library (::Swagger2), and I intend to attempt to fix it there.

In short, the readOnly marker doesn't appear to handle the difference between existence and definedness correctly yet.
Comment 28 Martin Renvoize 2016-10-21 15:47:06 UTC
Created attachment 56724 [details] [review]
QA Followup - Fix failing test

Failing test case is due to random array order, so the best we can do is
check for the presence of the errors key.
Comment 29 Martin Renvoize 2016-10-21 15:49:41 UTC
Final followup, I was wrong in my last comment.. Swagger Validator is in fact doing the right thing it's just that we can't easily test against it. 

We are given an array or validation errors. Those errors are in the order in which the hash structure is validated.. so due to Perl randomisation the error order varies and thus we cannot easily test the actual error message when there is more than one.

I have amended the test to simply check for the presence of the /errors key which should suffice.
Comment 30 Martin Renvoize 2016-10-24 08:52:00 UTC
Created attachment 56741 [details] [review]
Bug 17428: [REST best practice Followup]

This followup alters a few area's to be aligned more closely with
RESTfull best practices:

* PUT should always be full objects, and not partial updates (use PATCH
  for partials)
* Validate query parameters instead of blindly passing them to the model
  * Functional Change: Convert filter params from 'equality' to 'starts with'
    matching
* Update tests to check for swagger validation errors instead of koha exceptions
* Mark 'id' as readOnly so swagger may prevent, via validation, id
  changes.
Comment 31 Martin Renvoize 2016-10-24 08:53:01 UTC
Only the final [REST best practice Followup] needs signing off, would love to get some movement on this again before feature freeze ;)
Comment 32 Martin Renvoize 2016-10-24 09:08:49 UTC
Comment on attachment 56555 [details] [review]
Bug 17428: [REST] Cities swagger specification

Review of attachment 56555 [details] [review]:
-----------------------------------------------------------------

Minor query regarding returned serialised bodies

::: api/v1/swagger/definitions/city.json
@@ +3,5 @@
> +  "properties": {
> +      "cityid": {
> +        "$ref": "../x-primitives.json#/cityid"
> +      },
> +      "city_name": {

Does this not break our own rule of following the UI term.. I feel this should simply be 'name' and not expose the db field names to the end user (the api consumer in this case).
Comment 33 Jonathan Druart 2016-10-24 14:30:06 UTC
Please try to separate indentation fixes and other changes.
Comment 34 Jonathan Druart 2016-10-24 15:36:57 UTC
Created attachment 56800 [details] [review]
Bug 17428: [REST] Cities endpoint unit tests

This patch introduces unit tests for the cities endpoint.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 35 Jonathan Druart 2016-10-24 15:37:01 UTC
Created attachment 56801 [details] [review]
Bug 17428: [REST] Cities swagger specification

This patch adds the swagger definitions for the /cities endpoint

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 36 Jonathan Druart 2016-10-24 15:37:04 UTC
Created attachment 56802 [details] [review]
Bug 17428: [REST] Koha::REST::V1::Cities introduced

This patch introduces full CRUD for Koha::Cit(ies|y) classes through
the REST api. To test, point your browser to /api/v1/cities to use
HTTPRequester/Postman (or the like).

And of course, run the unit tests:
- Apply the patches
- Update your minified swagger file:
  $ cd kohaclone/
  $ misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json \
                                -d api/v1/swagger/swagger.min.json
- Run:
  $ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests should return green, and no warnings.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 37 Jonathan Druart 2016-10-24 15:37:08 UTC
Created attachment 56803 [details] [review]
Bug 17428: [REST best practice Followup]

This followup alters a few area's to be aligned more closely with
RESTfull best practices:

* PUT should always be full objects, and not partial updates (use PATCH
  for partials)
* Validate query parameters instead of blindly passing them to the model
  * Functional Change: Convert filter params from 'equality' to 'starts with'
    matching
* Update tests to check for swagger validation errors instead of koha exceptions
* Mark 'id' as readOnly so swagger may prevent, via validation, id
  changes.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 38 Jonathan Druart 2016-10-24 15:37:11 UTC
Created attachment 56804 [details] [review]
Bug 17428: Remove warning

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 39 Jonathan Druart 2016-10-24 15:38:32 UTC
Everything works perfectly!
Comment 40 Tomás Cohen Arazi 2016-10-24 16:08:12 UTC
Created attachment 56807 [details] [review]
Bug 17428: [REST] Cities endpoint unit tests

This patch introduces unit tests for the cities endpoint.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 41 Tomás Cohen Arazi 2016-10-24 16:08:19 UTC
Created attachment 56808 [details] [review]
Bug 17428: [REST] Cities swagger specification

This patch adds the swagger definitions for the /cities endpoint

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 42 Tomás Cohen Arazi 2016-10-24 16:08:26 UTC
Created attachment 56809 [details] [review]
Bug 17428: [REST] Koha::REST::V1::Cities introduced

This patch introduces full CRUD for Koha::Cit(ies|y) classes through
the REST api. To test, point your browser to /api/v1/cities to use
HTTPRequester/Postman (or the like).

And of course, run the unit tests:
- Apply the patches
- Update your minified swagger file:
  $ cd kohaclone/
  $ misc/devel/minifySwagger.pl -s api/v1/swagger/swagger.json \
                                -d api/v1/swagger/swagger.min.json
- Run:
  $ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests should return green, and no warnings.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 43 Tomás Cohen Arazi 2016-10-24 16:08:37 UTC
Created attachment 56810 [details] [review]
Bug 17428: [REST] best practice followup

This followup alters a few area's to be aligned more closely with
RESTfull best practices:

* PUT should always be full objects, and not partial updates (use PATCH
  for partials)
* Validate query parameters instead of blindly passing them to the model
  * Functional Change: Convert filter params from 'equality' to 'starts with'
    matching
* Update tests to check for swagger validation errors instead of koha exceptions
* Mark 'id' as readOnly so swagger may prevent, via validation, id
  changes.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 44 Tomás Cohen Arazi 2016-10-24 16:08:46 UTC
Created attachment 56811 [details] [review]
Bug 17428: (QA followup) Remove warning

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 45 Tomás Cohen Arazi 2016-10-24 16:08:55 UTC
Created attachment 56812 [details] [review]
Bug 17428: (QA followup) Fix community site in swagger file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 46 Martin Renvoize 2016-10-25 07:51:25 UTC
(In reply to Jonathan Druart from comment #33)
> Please try to separate indentation fixes and other changes.

Oops, Sorry about that.. been working on another project too long and forgot to comment out the 'tidy before commit' line in my git hooks.
Comment 47 Martin Renvoize 2016-10-25 07:53:01 UTC
Woop, woop!
Comment 48 Kyle M Hall 2016-10-28 14:44:40 UTC
Pushed to master for 16.11, thanks Tomas, Martin, Jonathan!