[Bug 28370] New: Routes still missing additionalProperties in spec
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Bug ID: 28370 Summary: Routes still missing additionalProperties in spec Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com There are still some routes that have inline object definitions, missing additionalProperties. -- 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=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Depends on| |28272 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28272 [Bug 28272] Definition files are missing additionalProperties: false -- 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=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |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=28370 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121110 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121110&action=edit Bug 28370: Add missing additionalProperties in spec This patch adds missing attribute to the object definitions in the spec. We wrongly supposed it was the default, but it proved not. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1 => SUCCESS: Tests pass, nothing breaks! 3. Sign off :-D 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=28370 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major Keywords| |rel_21_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 David Nind <david@davidnind.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=28370 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121110|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 121134 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121134&action=edit Bug 28370: Add missing additionalProperties in spec This patch adds missing attribute to the object definitions in the spec. We wrongly supposed it was the default, but it proved not. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1 => SUCCESS: Tests pass, nothing breaks! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121134|0 |1 is obsolete| | --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 121232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121232&action=edit Bug 28370: Add missing additionalProperties in spec This patch adds missing attribute to the object definitions in the spec. We wrongly supposed it was the default, but it proved not. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1 => SUCCESS: Tests pass, nothing breaks! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <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=28370 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Good Call, Should we also add an explicit coding guideline for this perhaps.. and a QA script catch to spot removals of the line in api/swagger/*.json files or lack of it in new files in that directory? Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 121270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121270&action=edit Bug 28370: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Those tests are failing, can you have a look please? 1. They caught 3 missing additionalProperties in paths/* 2. It seems that we should ignore the error.json and open a separate bug report -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 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=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121232|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=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #121270|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=28370 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121284 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121284&action=edit Bug 28370: Add missing additionalProperties in spec This patch adds missing attribute to the object definitions in the spec. We wrongly supposed it was the default, but it proved not. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1 => SUCCESS: Tests pass, nothing breaks! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=28370 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121285 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121285&action=edit Bug 28370: Add tests 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=28370 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121286 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121286&action=edit Bug 28370: Rewrite tests This patch rewrites the tests, and also doesn't enforce additionalProperties to be false. We could need it, and there are routes that need to be reviewed that would break otherwise now: error.json, for example, is used everywhere, and some routes add some payload to it. The main change this patch introduces is using the resolved spec instead of finding each .json file, which wasn't even correct as we have .yaml files already. Parameters and responses are tested. To test: 1. Apply this patch 2. Run: $ kshell k$ prove xt/api.t => SUCCESS: Tests fail! A nice report on the failures is printed 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=28370 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121287 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121287&action=edit Bug 28370: (follow-up) More cases 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=28370 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121289 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121289&action=edit Bug 28370: Fix invalid responses in /patrons This response codes are not actually used. 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=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28424 See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28424 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28424 [Bug 28424] POST /patrons/:patron_id/account/credits return value wrong -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Bug 28424 fixes the remaining issue. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Great teamwork, the test looks a solid improvement. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- t/db_dependent/api/v1/holds.t tests are failing. Didn't we decide to fix error.json separately, because of the "conflict" param that was not in the spec? bug 28020 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #14)
t/db_dependent/api/v1/holds.t tests are failing.
Didn't we decide to fix error.json separately, because of the "conflict" param that was not in the spec? bug 28020
You are missing bug 28369 on your tree. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |28369 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28369 [Bug 28369] additionalProperties missing in holds routes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 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=28370 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #14)
Didn't we decide to fix error.json separately, because of the "conflict" param that was not in the spec? bug 28020
Do you think we should rush about it? It felt like some coordinated teamwork is needed to think about it and propose something sound. Specially when it comes to a generic payload schema. I allowed the extra attributes in error.json as a transitional step as we are a few days away from the release. Can happily work on a better solution on that bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #16)
(In reply to Jonathan Druart from comment #14)
Didn't we decide to fix error.json separately, because of the "conflict" param that was not in the spec? bug 28020
Do you think we should rush about it?
No, definitely not. I saw you changed it and so that the tests were failing because of that. Wrong assumption! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28369 Depends on|28369 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28369 [Bug 28369] additionalProperties missing in holds routes -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 28272 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28370 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_21_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org