Bug 16276 changed the Patron attributes (adding lastseen) but failed to update the swagger/definitions/patron.json file accordingly. To test: - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => FAIL: Definitions are not complete compared to DB schema
Created attachment 57423 [details] [review] Bug 17607: Fix patron definition in Swagger This patch makes the patron swagger definition match the DB schema. To test: - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => FAIL: 'lastseen' field is not declared in the swagger definition - Apply the patch - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => SUCCESS: Tests pass! - Sigh off :-D Sponsored-by: ByWater Solutions
Created attachment 57432 [details] [review] [SIGNED-OFF]Bug 17607: Fix patron definition in Swagger This patch makes the patron swagger definition match the DB schema. To test: - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => FAIL: 'lastseen' field is not declared in the swagger definition - Apply the patch - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => SUCCESS: Tests pass! - Sigh off :-D Sponsored-by: ByWater Solutions Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfuly
Created attachment 57433 [details] [review] [SIGNED-OFF]Bug 17607: Fix patron definition in Swagger This patch makes the patron swagger definition match the DB schema. To test: - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => FAIL: 'lastseen' field is not declared in the swagger definition - Apply the patch - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => SUCCESS: Tests pass! - Sigh off :-D Sponsored-by: ByWater Solutions Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfuly Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Created attachment 57435 [details] [review] [SIGNED-OFF]Bug 17607: Fix patron definition in Swagger This patch makes the patron swagger definition match the DB schema. To test: - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => FAIL: 'lastseen' field is not declared in the swagger definition - Apply the patch - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => SUCCESS: Tests pass! - Sigh off :-D Sponsored-by: ByWater Solutions Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfuly Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Fixed a typo: seed -> seen
(In reply to Lari Taskula from comment #4) > Fixed a typo: seed -> seen :) +1
QA Remark: We should probably be moving towards stricter validation application wide, and this may be a good one to start with. datelastseen looks like a date for me ;) So we should probably return it in an internationally recognised format. Swagger allows for this with data type format modifiers: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#data-types { type: string, format: date-time } or { type: string, format: date } should suffice depending upon whether this is a date-time or just a short date. What do we think? API consumers like explicit specification and consistency for this sort of thing ;)
Yes, I agree with Martin. We need to standardise our output to make sure we always return the same date type. On the other hand this is a follow-up for bug 16276, and I guess we need it before the release.
(In reply to Jonathan Druart from comment #7) > Yes, I agree with Martin. We need to standardise our output to make sure we > always return the same date type. > On the other hand this is a follow-up for bug 16276, and I guess we need it > before the release. Exactly, what Martin proposes is an enhancement, which I agree with, but is out of the scope of this one, that is needed for the next release in less than two weeks!
If it were as simple as adding "format: date" to each of the date fields then I'd have said lets do it here.. but the update-on field is a date-time (and one that's not in standard format yet at that).. So lets just go with this for now and add a followup to specify formats in another bug.
Created attachment 57467 [details] [review] [PASSED QA] Bug 17607: Fix patron definition in Swagger This patch makes the patron swagger definition match the DB schema. To test: - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => FAIL: 'lastseen' field is not declared in the swagger definition - Apply the patch - Run: $ prove t/db_dependent/api/v1/swagger/definitions.t => SUCCESS: Tests pass! - Sigh off :-D Sponsored-by: ByWater Solutions Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfuly Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Fixed a typo: seed -> seen Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Pushed to master for 16.11, thanks Tomas!
(In reply to Kyle M Hall from comment #11) > Pushed to master for 16.11, thanks Tomas! blocked by enhancement, passing for 16.05