|
Description
Martin Renvoize (ashimema)
2022-01-06 12:51:43 UTC
Created attachment 129063 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Created attachment 129064 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Created attachment 129067 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Created attachment 129068 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Created attachment 129069 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Created attachment 129070 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Created attachment 129071 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Created attachment 129072 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Created attachment 129073 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Grr.. looks like collectionFormat support requires a newer version of JSON::Validator so this breaks our embeds Created attachment 132131 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Created attachment 132132 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Created attachment 132133 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Created attachment 132134 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Created attachment 132135 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Created attachment 132136 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Created attachment 132137 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Rebased against bug 30194 and ready for testing again :) Created attachment 132160 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Created attachment 132161 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Created attachment 132162 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Created attachment 132163 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Created attachment 132164 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Created attachment 132165 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Created attachment 132166 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Created attachment 132167 [details] [review] Debugging Tests Looks like collectionFormat still isn't working as expected for these headers :(... I've added a slight modification to one of the failing tests in the final patch to highlight the issue.. Created attachment 132223 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132224 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132225 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132226 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132227 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132228 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132229 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 132230 [details] [review] Bug 29810: Add 'x-koha-embed' to reserved words The maintainers of Mojolicious::Plugin::OpenAPI and friends fixed the problem with using collectionFormat for Headers in the upstream library. However, this highlighted an outstanding issue in our code when I ran the unit tests. We have not been stripping x-koha-embed at the query level as it did not exist in the reserved_words list. This patch resolves that and gets the acquisitions_orders test passing again. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Woopie, this now all works! This collectionFormat issue I had with headers was quickly fixed upstream with the release of JSON::Validator 5.08 (which also bumped Mojolicious::Plugin::OpenAPI to v5.05). Could we get those packaged whilst you are in the packaging mindset for these modules Mason? With that in mind, the test plan is to read the documentation added in the patches and confirm it is current and meaningful. Once you've done that, ensure you have the latest modules at mentioned above (cpanm Mojolicous::Plugin::OpenAPI should do the trick if your testing before it's been packaged) and then run the API test suit.. prove t/db_dependant/api/v1/*. Created attachment 132255 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132256 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132257 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132258 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132259 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132260 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132261 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 132262 [details] [review] Bug 29810: Add 'x-koha-embed' to reserved words The maintainers of Mojolicious::Plugin::OpenAPI and friends fixed the problem with using collectionFormat for Headers in the upstream library. However, this highlighted an outstanding issue in our code when I ran the unit tests. We have not been stripping x-koha-embed at the query level as it did not exist in the reserved_words list. This patch resolves that and gets the acquisitions_orders test passing again. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Great work, love where we are headed to. We should have a test and make it a requirement. Created attachment 132341 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132342 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132343 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132344 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132345 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132346 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132347 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 132348 [details] [review] Bug 29810: Add 'x-koha-embed' to reserved words The maintainers of Mojolicious::Plugin::OpenAPI and friends fixed the problem with using collectionFormat for Headers in the upstream library. However, this highlighted an outstanding issue in our code when I ran the unit tests. We have not been stripping x-koha-embed at the query level as it did not exist in the reserved_words list. This patch resolves that and gets the acquisitions_orders test passing again. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Sorry does not apply anymore since Bug 30394 Rebase looks easy but i prefer it is done the author ;) Created attachment 133063 [details] [review] Bug 29810: Add summary of x-koha-embed header to api spec Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133064 [details] [review] Bug 29810: Document x-koha-embed header on patrons endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133065 [details] [review] Bug 29810: Document x-koha-embed header on checkouts endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133066 [details] [review] Bug 29810: Document x-koha-embed header on biblios endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133067 [details] [review] Bug 29810: Document x-koha-embed header on cash registers endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133068 [details] [review] Bug 29810: Document x-koha-embed header on libraries endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133069 [details] [review] Bug 29810: Document x-koha-embed header on orders endpoints Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 133070 [details] [review] Bug 29810: Add 'x-koha-embed' to reserved words The maintainers of Mojolicious::Plugin::OpenAPI and friends fixed the problem with using collectionFormat for Headers in the upstream library. However, this highlighted an outstanding issue in our code when I ran the unit tests. We have not been stripping x-koha-embed at the query level as it did not exist in the reserved_words list. This patch resolves that and gets the acquisitions_orders test passing again. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> (In reply to Fridolin Somers from comment #55) > Sorry does not apply anymore since Bug 30394 > Rebase looks easy but i prefer it is done the author ;) Done :-D Pushed to master for 22.05, thanks to everybody involved [U+1F984] Created attachment 133285 [details] [review] Bug 29810: Fix patron embed definitions Due to a race and a bad rebase, the patrons embeds added on bug 30063 didn't get copied to the parameters secition, thus breaking embedding. To test: 1. In master, open the patron search page and the inspector 2. Perform a search => FAIL: No results => FAIL: There's an error 500 in the API response, mentioning embedding is not allowed 3. Apply this patch 4. Restart all 6. Repeat 2 => SUCCESS: Results are back! => SUCCESS: No more API errors 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> *** Bug 30537 has been marked as a duplicate of this bug. *** Follow-up pushed as RM assistant to get people back up and running. :) |