|
Description
Tomás Cohen Arazi (tcohen)
2021-02-23 12:19:53 UTC
Created attachment 117308 [details] [review] Bug 27760: Add stash_overrides helper This patch adds a simple helper that reads the x-koha-override request header, and processes it to stash a hashref with the passed overrides. No check on the overrides themselves is done, as they should be validated using the OpenAPI spec. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! 3. Sign off :-D Created attachment 117309 [details] [review] Bug 27760: Make auhtenticate_api_request call stash_overrides By making this method call the helper, we make sure the overrides hashref is available to any controller to make us of it. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! The helper works as expected and the authenticate_api_request method invokes it! 3. Sign off :-D Created attachment 117326 [details] [review] Bug 27760: Add stash_overrides helper This patch adds a simple helper that reads the x-koha-override request header, and processes it to stash a hashref with the passed overrides. No check on the overrides themselves is done, as they should be validated using the OpenAPI spec. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Created attachment 117327 [details] [review] Bug 27760: Make auhtenticate_api_request call stash_overrides By making this method call the helper, we make sure the overrides hashref is available to any controller to make us of it. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! The helper works as expected and the authenticate_api_request method invokes it! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> I wanted to mention that it feels the right way to retrieve the headers in the code should use the validated data (by the OpenAPI plugin) so we could take advantage of data coercion and such. But I couldn't find a way to get the Mojolicious::Validator::Validation object ($c->validation) to give us the headers. So skipping that for now. I will follow-up on this once I get answers from the mojo/openapi devs on IRC. Created attachment 117342 [details] [review] Bug 27760: Add stash_overrides helper This patch adds a simple helper that reads the x-koha-override request header, and processes it to stash a hashref with the passed overrides. No check on the overrides themselves is done, as they should be validated using the OpenAPI spec. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 117343 [details] [review] Bug 27760: Make auhtenticate_api_request call stash_overrides By making this method call the helper, we make sure the overrides hashref is available to any controller to make us of it. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! The helper works as expected and the authenticate_api_request method invokes it! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 117344 [details] [review] Bug 27760: Make authenticate_api_request call stash_overrides By making this method call the helper, we make sure the overrides hashref is available to any controller to make us of it. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! The helper works as expected and the authenticate_api_request method invokes it! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Blimey.. I was about to SO this and by the time I'd read the code in splinter review it had been SO'd and QA'd!.. Impressive work chaps. 1. Missing POD 2. (In reply to Tomás Cohen Arazi from comment #5) > I will follow-up on this once I get answers from the mojo/openapi devs on > IRC. Any news? Created attachment 118108 [details] [review] Bug 27760: Improve readabiliity using map (In reply to Jonathan Druart from comment #10) > 1. Missing POD Well check. > 2. (In reply to Tomás Cohen Arazi from comment #5) > > I will follow-up on this once I get answers from the mojo/openapi devs on > > IRC. > > Any news? We need to upgrade our JSON::Validator. Created attachment 118110 [details] [review] Bug 27760: (QA follow-up) Improve POD This patch improves the POD for the added helper, and also adds references to the JSON::Validator issue that explains the current implementation. A FIXME is added for when we upgrade our used JSON::Validator version. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Rebased on top of bug 28157. Created attachment 120112 [details] [review] Bug 27760: Add stash_overrides helper This patch adds a simple helper that reads the x-koha-override request header, and processes it to stash a hashref with the passed overrides. No check on the overrides themselves is done, as they should be validated using the OpenAPI spec. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120113 [details] [review] Bug 27760: Make authenticate_api_request call stash_overrides By making this method call the helper, we make sure the overrides hashref is available to any controller to make us of it. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! The helper works as expected and the authenticate_api_request method invokes it! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 120114 [details] [review] Bug 27760: Improve readabiliity using map Created attachment 120115 [details] [review] Bug 27760: (QA follow-up) Improve POD This patch improves the POD for the added helper, and also adds references to the JSON::Validator issue that explains the current implementation. A FIXME is added for when we upgrade our used JSON::Validator version. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Looks like upgrading JSON::Validator would give us a few different wins across the board... (In reply to David Cook from comment #19) > Looks like upgrading JSON::Validator would give us a few different wins > across the board... Oh yeah. But this is the very latest version. Would love to, though :-) (In reply to Tomás Cohen Arazi from comment #20) > (In reply to David Cook from comment #19) > > Looks like upgrading JSON::Validator would give us a few different wins > > across the board... > > Oh yeah. But this is the very latest version. Would love to, though :-) Ahh that's harder :( Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x |