https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146421 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146421&action=edit Bug 32923: Fix new occurrences of x-koha-embed at top level -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146422 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146422&action=edit Bug 32923: Some fixes in ERM specs -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146422|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146425 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146425&action=edit Bug 32923: Some fixes in ERM specs -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146443&action=edit Bug 32923: needed? I think we need that, see later. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|x-koha-embed must be an |x-koha-embed must a header |enum |of collectionFormat csv -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 146633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146633&action=edit Bug 32923: Fix x-koha-embed from EHoldings/*/* We were validating twice the input and it broke the x-koha-embed header. I don't really know what's happening, but here is how to recreate the problem: 1. Edit Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm 2. Add the following lines in Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm, in the list method warn $c->req->headers->header('x-koha-embed'); 3. curl -v -s -u koha:koha --request GET http://kohadev-intra.mydnsname.org:8081/api/v1/erm/eholdings/ebsco/packages --header "x-koha-embed: resources+count,vendor" => There is no vendor in the response => The warn displays "resources+count" If you apply this patch you will notice that "vendor" is part of the response and the warn displays "resources+count, vendor" -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #11 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #10)

Created attachment 146633 [details] [review] Bug 32923: Fix x-koha-embed from EHoldings/*/*

We were validating twice the input and it broke the x-koha-embed header. I don't really know what's happening, but here is how to recreate the problem: 1. Edit Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm 2. Add the following lines in Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm, in the list method warn $c->req->headers->header('x-koha-embed'); 3. curl -v -s -u koha:koha --request GET http://kohadev-intra.mydnsname.org:8081/api/v1/erm/eholdings/ebsco/packages --header "x-koha-embed: resources+count,vendor"

=> There is no vendor in the response => The warn displays "resources+count"

If you apply this patch you will notice that "vendor" is part of the response and the warn displays "resources+count, vendor"

Did a lot of testing with this but openapi->valid_input in EBSCO.pm is getting rid of all arguments in the x-koha-embed header except the first one. Lot of trial and error in erm_eholdings_packages.yaml also came up with no breakthrough. Found this fix related to default collectionFormat in version 5.06 (we're using 5.05) so upgraded to Mojolicious::Plugin::OpenAPI to 5.08 but result is the same: https://github.com/jhthorsen/mojolicious-plugin-openapi/commit/c972fdd02a05d... This weird behaviour seems to be caused by mojo. The code implemented to work around this is working as intended. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #12 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 146748 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146748&action=edit Bug 32898: Remove unecessary async/await myFetch awaits already Signed-off-by: Pedro Amorim <pedro.amorim@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=32923 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146749|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=32923 Pedro Amorim <pedro.amorim@ptfs-europe.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=32923 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146425|0 |1 is obsolete| | --- Comment #15 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 146837 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146837&action=edit Bug 32923: Some fixes in ERM specs Signed-off-by: Pedro Amorim <pedro.amorim@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=32923 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146633|0 |1 is obsolete| | --- Comment #17 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 146839 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=146839&action=edit Bug 32923: Fix x-koha-embed from EHoldings/*/* We were validating twice the input and it broke the x-koha-embed header. I don't really know what's happening, but here is how to recreate the problem: 1. Edit Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm 2. Add the following lines in Koha/REST/V1/ERM/EHoldings/Packages/EBSCO.pm, in the list method warn $c->req->headers->header('x-koha-embed'); 3. curl -v -s -u koha:koha --request GET http://kohadev-intra.mydnsname.org:8081/api/v1/erm/eholdings/ebsco/packages --header "x-koha-embed: resources+count,vendor" => There is no vendor in the response => The warn displays "resources+count" If you apply this patch you will notice that "vendor" is part of the response and the warn displays "resources+count, vendor" Signed-off-by: Pedro Amorim <pedro.amorim@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=32923 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #146836|0 |1 is obsolete| | Attachment #146837|0 |1 is obsolete| | Attachment #146838|0 |1 is obsolete| | Attachment #146839|0 |1 is obsolete| | --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 147292 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147292&action=edit Bug 32923: Fix new occurrences of x-koha-embed at top level Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 147294 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147294&action=edit Bug 32923: Add 'vendor' for local packages Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #5)

I would not backport these patches, they can introduce hard to catch side-effects.

Having reviewed it, I think this needs to be backported. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Passed QA --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- After an exchange with the Mojolicious::Plugin::OpenAPI maintainer and exchanging pull requests, the conclusion is: - Validation is not supposed to happen twice - The validation step is expected to normalize the ->params and ->headers in a specific way, and we shouldn't be relying on the $c->validation->output structure as it subject to change and at some point they thought of removing it. Once validation happens we need to use ->params and ->headers. - When valid_input is called, the header effectively gets converted into an array and expecting it to still be a CSv string will lead to problems as we've seen. - When collectionFormat is used, we should be using multi, the maintainer really dislikes the 'csv' implementation. - I would've expected the plugin to provide us a consistent and usable way to access parameters through ->validation->output and leave the params untouched, but this is not the case, by design. And is not going to change. Having said all that, I agree with this patchset QA note: I have doubts about the real need for this API spec that points to an intermediate controller that later calls the *real controller*. I would've just duplicated the endpoint spec, hardcoding 'ebsco' and 'local' and calling the appropriate method. The only reason I would accept to keep this, is because you (ERM devs) might be thinking of extending this with plugins that implement provider-specific implementations, along the lines of this mocked code: ```perl sub get { my $c = shift->openapi->valid_input or return; my @plugins = GetPlugins({ method => 'erm_packages' }); my $provider = $c->validation->param('provider'); if ( $provider eq 'ebsco' ) { return Koha::REST::V1::ERM::EHoldings::Packages::EBSCO::get($c, $validated_output); } elsif (@plugins) { foreach my $plugin (@plugins) { if ( $provide eq $plugin->erm_provider ) { return $plugin->erm_package_controller::get($c); } } } return Koha::REST::V1::ERM::EHoldings::Packages::Local::get($c, $validated_output); } ``` -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #25 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|23.05.00 |23.05.00,22.11.04 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #27 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to stable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |lucas@bywatersolutions.com Status|Pushed to stable |RESOLVED --- Comment #28 from Lucas Gass <lucas@bywatersolutions.com> --- Not backporting to 22.05.x -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.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=32923 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matt.blenkinsop@ptfs-europe | |.com --- Comment #31 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Follow-up noted, added to queue for backporting to stable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 --- Comment #33 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 148413 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148413&action=edit Bug 32923: Fix licenses spec Bug 32713 is now validating embed and brought this flaw to light. Before this patch, listing licenses on ERM would result in a 400 bad request -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_11_candidate | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 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=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Is this still in time for 22.11.04? Or should I open a new bug? The agreement list table is not displayed correctly without this last patch. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate --- Comment #39 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #36)

Created attachment 148507 [details] [review] Bug 32923: Allow to embed user_roles for agreements

Patch pushed to master for 23.05. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32923 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed, | |rel_22_11_candidate | --- Comment #41 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Final follow-up pushed to 22.11.x for 22.11.04 -- You are receiving this mail because: You are watching all bug changes.