https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28272 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 120630 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120630&action=edit Bug 28272: Prevent api spec to accept extra parameters Some of our definition files are missing { additionalProperties: false } We must have it to make sure all properties are defined and prevent the spec to accept extra parameters (being more strict but also allowing to catch errors more easily) grep -rL additionalProperties api/v1/swagger/definitions/*.json -- 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=28272 Owen Leonard <oleonard@myacpl.org> 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=28272 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |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=28272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120673|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=28272 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=28272 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com 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=28272 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Still failing: * t/db_dependent/api/v1/checkouts.t because of unseen_renewals but it's defined (?) * t/db_dependent/api/v1/acquisitions_orders.t about "conflict" that we are adding in the response. We do that in other places as well, what would be best to deal with that? And maybe others. I need a bit of help here. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28272 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 120812 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120812&action=edit Bug 28272: Fix many things... This patch could've been splitted into several. But, overall, adding additionalAttributes: false made the API fail on requests that send extra info (i.e. cases in which a dev added an attribute to the underlaying class/table and forgot to deal with it on the API (either adding it on the spec, or removing it from the response using Koha::Class::to_api_mapping). - Koha::Account::Line was missing: credit_type, interface, status, register_id and credit_number. I decided to call cash_register_id, and to remove credit_number from the response. FIXME: We need consensus on a name for the credit_number attribute, and add it to the response on the API. It deserves a separate bug. Too opinionated for a last-minute fix. - Koha::Club::Hold::add was returning bad auto-calculated values on field that (also) wasn't specified on the spec. Needs a test. - import_batch_profile had a typo: id_profile vs. profile_id. - error.json: In this case I reverted the change. This is because some routes are adding more 'info' with the error message, and I consider this should be done in a more generic approach. Time is required for us to think about this. So don't break the API in the meantime. FIXME: Implement a generic way to add a payload to error messages on the API. Maybe something to work on while on bug 28020. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/ => FAIL: Lots of tests fail 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests pass! 4. 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=28272 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 120819 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120819&action=edit Bug 28272: Add unseen_renewals for checkouts t/db_dependent/api/v1/checkouts.t ...................... 7/93 # Failed test '200 OK' # at t/db_dependent/api/v1/checkouts.t line 198. # got: '500' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/checkouts.t line 198. # Structures begin differing at: # $got->{unseen_renewals} = Does not exist # $expected->{unseen_renewals} = '0' t/db_dependent/api/v1/checkouts.t ...................... 88/93 # Failed test '200 OK' # at t/db_dependent/api/v1/checkouts.t line 218. # got: '500' # expected: '200' # Failed test 'exact match for JSON Pointer ""' # at t/db_dependent/api/v1/checkouts.t line 218. # Structures begin differing at: # $got->{max_renewals} = Does not exist # $expected->{max_renewals} = '1' # Looks like you failed 4 tests of 93. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28272 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 120833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120833&action=edit Bug 28272: issues.notseen needs to be marked as a boolean 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=28272 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Last patch pushed to master. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |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=28272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |When the REST API spec was release notes| |first written we assumed | |additionalProperties: false | |was the default behavior. | |It probably was by then. | | | |We recently found the need | |to explicitly add it to all | |the relevant places (i.e. | |those that require | |strictness). | | | |This bug adds | |it, and fixes the tests | |that fail or the error | |conditions that were hidden | |due to this being absent in | |the spec. Keywords|release-notes-needed | Assignee|tomascohen@gmail.com |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=28272 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28272 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 121184 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121184&action=edit Bug 28272: (QA follow-up) Add 'manager' to cashups definition The embeddable 'manager' relation field was missing from the specification file and as such calls using this embed would result in a 500 error since we introduced 'additionalProperties: false'. Test plan 1/ Enable 'UseCashRegisters' 2/ Navigate to the register details page 3/ Inspect the network traffic and note a 500 error on the cashups api call 4/ Apply the patch 5/ The 500 should have gone away and been replaced with an empty resultset 200 page. 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=28272 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #16 from Fridolin Somers <fridolin.somers@biblibre.com> --- Can this be backported to stable branches ? -- You are receiving this mail because: You are watching all bug changes.