[Bug 17927] New: REST API: /holds and /patrons attributes have wrong types
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Bug ID: 17927 Summary: REST API: /holds and /patrons attributes have wrong types Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Web services Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org -- 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=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com 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=17927 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This bug tries to resurrect the fixes to data types from bug 17008. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 59386 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59386&action=edit Bug 17927: Fix /holds and /patrons data types This patch changes current Swagger definitions for patrons and holds to have data types corresponding to column data types in their database tables. To test: 1. GET http://yourlibrary/api/v1/patrons/YYY where YYY is existing borrowernumber 2. Observe that numbers / integers are in string data type. 3. Apply this patch 4. Repeat step 1. 5. Observe that numbers / integers are now actually numbers / integers. Signed-off-by: Josef Moravec <josef.moravec@gmail.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=17927 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 59387 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59387&action=edit Bug 17927: (followup) Fix /cities cityid type 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=17927 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 59388 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59388&action=edit Bug 17927: (QA followup) Fix boolean types 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=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com, | |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=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Tomás Cohen Arazi <tomascohen@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=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- This patches are taken from bug 17008, which got splitted into other bugs that take care of smaller problems. They where signed by Josef, and I'm passing them along with a small followup for bookeans. Note for the QA team: the current API tests are failing. And this patch is not touching them. I'm working on a solution for the tests and the API as a whole on another bugs. Notably on bug 17932 which will allow us to actually have the API return the data types this patches explicitly define on the swagger files. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |17932 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17932 [Bug 17932] Koha::Object should provide a TO_JSON method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Status|Passed QA |Failed QA --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- One test is failing after this patch: # Failed test 'Columns is nullable in DB, not in swagger file for hold: timestamp' # at t/db_dependent/api/v1/swagger/definitions.t line 71. I also think that we should add tests to check data types, don't you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #6)
One test is failing after this patch:
# Failed test 'Columns is nullable in DB, not in swagger file for hold: timestamp' # at t/db_dependent/api/v1/swagger/definitions.t line 71.
I also think that we should add tests to check data types, don't you think?
I think we might better get rid of definitions.t as there's no obvious reason why the swagger definition should match the DB. I can take care of adding 'null' as a valid type for 'timestamp' but I think we should better move that to another bug as the tests are failing without this patches too. See comment #5 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #6)
One test is failing after this patch:
# Failed test 'Columns is nullable in DB, not in swagger file for hold: timestamp' # at t/db_dependent/api/v1/swagger/definitions.t line 71.
I also think that we should add tests to check data types, don't you think?
Also, are you sure you tested this along with bug 17932? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #7)
I think we might better get rid of definitions.t as there's no obvious reason why the swagger definition should match the DB. I can take care of adding 'null' as a valid type for 'timestamp' but I think we should better move that to another bug as the tests are failing without this patches too.
No, they pass on master. (In reply to Tomás Cohen Arazi from comment #8)
Also, are you sure you tested this along with bug 17932?
Yes I did. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 59899 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59899&action=edit Bug 17927: (QA followup) Fix timestamp nullable in hold.json 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=17927 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Lari, Josef, Tomas! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17927 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This appears to break /holds on 16.11.x - so not backporting it. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org