[Bug 17428] New: REST api: CRUD endpoint for cities
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Bug ID: 17428 Summary: REST api: CRUD endpoint for cities Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Web services Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org Add an endpoint for handling cities. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56189 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56189&action=edit Bug 17428: [REST] Cities endpoint unit tests This patch introduces unit tests for the cities endpoint. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56190 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56190&action=edit Bug 17428: [REST] Cities swagger specification This patch adds the swagger definitions for the /cities endpoint -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56191 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56191&action=edit 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 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17425 Status|NEW |Needs Signoff Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17425 [Bug 17425] Koha::Object should raise exceptions -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |claire_gravely@hotmail.com --- Comment #4 from Claire Gravely <claire_gravely@hotmail.com> --- I'm getting raptor not found when pointing my browser at it! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #5 from Claire Gravely <claire_gravely@hotmail.com> --- (In reply to Claire Gravely from comment #4)
I'm getting raptor not found when pointing my browser at it!
ignore me! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56189|0 |1 is obsolete| | --- Comment #6 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56226 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56226&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56190|0 |1 is obsolete| | --- Comment #7 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56227&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56191|0 |1 is obsolete| | --- Comment #8 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 56228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56228&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Assignee|tomascohen@gmail.com |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Claire Gravely <claire_gravely@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56228|0 |1 is obsolete| | --- Comment #9 from Claire Gravely <claire_gravely@hotmail.com> --- Created attachment 56261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56261&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56314 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56314&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Comment on attachment 56314 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56314 Bug 174828: Followup - More RESTfull handling of endpoints Review of attachment 56314: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17428&attachment=56314) ----------------------------------------------------------------- ::: 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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #13 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 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 ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- POST with non-existing id allows me to choose the id. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #12)
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?
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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17445 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17445 [Bug 17445] REST API: Generic handling of malformed query parameters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|17445 | Depends on| |17445 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17445 [Bug 17445] REST API: Generic handling of malformed query parameters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lari.taskula@jns.fi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56314|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56501&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56544 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56544&action=edit Bug 17428: Just some refactoring The idea is to make the code more readable moving the code to its own subroutine -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56545 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56545&action=edit Bug 17428: Move the params check after the authentication check If the user is not authorised to call this route, we would prefer to raise a 403 instead of 400 Note that we wanted to submit tests for this change but the city code does not let use do that (we are allowed to list/show cities even without any permissions). The patrons.t is not complete enought and the holds.t tests do not pass... Tomas plans to submit tests but we reach the end of the hackfest ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56544|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56545|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56548 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56548&action=edit Bug 17428: Remove debug statement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56553 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56553&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56226|0 |1 is obsolete| | Attachment #56227|0 |1 is obsolete| | Attachment #56261|0 |1 is obsolete| | Attachment #56501|0 |1 is obsolete| | Attachment #56548|0 |1 is obsolete| | Attachment #56553|0 |1 is obsolete| | --- Comment #21 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56554 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56554&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56555&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56556&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56557&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56558&action=edit Bug 17428: Remove debug statement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56559&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #27 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Karam Qubsi <karamqubsi@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |karamqubsi@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #28 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56724 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56724&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #29 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56557|0 |1 is obsolete| | Attachment #56558|0 |1 is obsolete| | Attachment #56559|0 |1 is obsolete| | Attachment #56724|0 |1 is obsolete| | --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 56741 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56741&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #31 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Only the final [REST best practice Followup] needs signing off, would love to get some movement on this again before feature freeze ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 56555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56555 Bug 17428: [REST] Cities swagger specification Review of attachment 56555: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=17428&attachment=56555) ----------------------------------------------------------------- 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). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #33 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Please try to separate indentation fixes and other changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56554|0 |1 is obsolete| | --- Comment #34 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56800 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56800&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56555|0 |1 is obsolete| | --- Comment #35 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56801&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56556|0 |1 is obsolete| | --- Comment #36 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56802&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56741|0 |1 is obsolete| | --- Comment #37 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56803 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56803&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #38 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 56804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56804&action=edit Bug 17428: Remove warning Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #39 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Everything works perfectly! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56800|0 |1 is obsolete| | --- Comment #40 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56807 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56807&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56801|0 |1 is obsolete| | --- Comment #41 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56808 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56808&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56802|0 |1 is obsolete| | --- Comment #42 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56809 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56809&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56803|0 |1 is obsolete| | --- Comment #43 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56810 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56810&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #56804|0 |1 is obsolete| | --- Comment #44 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56811 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56811&action=edit Bug 17428: (QA followup) Remove warning Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #45 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 56812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=56812&action=edit Bug 17428: (QA followup) Fix community site in swagger file Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #46 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Woop, woop! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #48 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.11, thanks Tomas, Martin, Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|REST api: CRUD endpoint for |REST API: CRUD endpoint for |cities |cities CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |14974 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14974 [Bug 14974] Use the REST API for cities -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17428 Zeno Tajoli <z.tajoli@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This patch introduces full release notes| |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. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org