When upgrading from 23.05 to 23.11.01 our ILL module (https://github.com/Libriotech/koha-illbackend-libris) stopped working until <enable_plugins> variable in koha-conf.xml was set to 1. Plack-error.log showed message Can't call method "GetPlugins" on an undefined value at /usr/share/koha/intranet/cgi-bin/ill/ill-requests.pl line 534. Since the ILL-module is not a plugin this behaviour is unwanted.
This is caused by the changes introduced in bug 30719: # Get available metadata enrichment plugins sub get_metadata_enrichment { my @candidates = Koha::Plugins->new()->GetPlugins({ method => 'provides_api' }); my @services = (); foreach my $plugin(@candidates) { my $supported = $plugin->provides_api(); if ($supported->{type} eq 'search') { push @services, $supported; } } return \@services; }
Created attachment 162018 [details] [review] Bug 35930: Add guards for plugins_enabled There are ILL-backends that are not implemented as Koha plubins and does not require plugins to be enabled. The 'new' method in Koha::Plugins returns undefined if plugins are disabled. Therefore, calls to this method must be guarded by a check that plugins actually are enabled. Test plan: * Code inspection of patch, alternatively * Activate the ill system by installing a backend such as koha-illbackend-libris: https://github.com/Libriotech/koha-illbackend-libris * Make sure plugins are disabled in koha-conf.xml * In the staff interface, go to ILL requests. * The page should load without getting an error 500.
Created attachment 162019 [details] [review] Bug 35930: Add guards for plugins_enabled The 'new' method in Koha::Plugins returns undefined if plugins are disabled. Therefore, calls to this method must be guarded by a check that plugins actually are enabled. Test plan: * Code inspection of patch, alternatively * Activate the ill system by installing a backend such as koha-illbackend-libris: https://github.com/Libriotech/koha-illbackend-libris * Make sure plugins are disabled in koha-conf.xml * In the staff interface, go to ILL requests. * The page should load without getting an error 500. PA amended commit message: This is not related to ILL backends being plugins or not This is about ILL batches, where checking for metadata enrichment plugins was missing 'enable_plugins' guard Additionally, unrelated to batches, it's also about ILLAvailability, where checking for ILL availabililty plugins was missing enable_plugins guard Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Thanks @Andreas. This makes sense, and I've tested to also make sure there are no regressions: ILLCheckAvailability -> enabled, installed an availability plugin and tested with enable_plugins 0 and 1, all working as expected: https://github.com/PTFS-Europe/koha-plugin-ill-avail-eds Installed a metadata enrichment plugin, tested with ILL batches, with enable_plugins 0 and 1, all working as expected: https://github.com/PTFS-Europe/koha-plugin-api-pubmed "As expected" above means these features are not available if enable_plugins=0 and no errors occur. If someone else SOs this, please convert my SO to PQA and update accordingly.
Upping this to "major" as it will cause an ugly 500 error and block ILL basic functionality for installations using ILLModule with enable_plugins=0.
Tested the patch and it works. I mark this as PQA per request, if that´s wrong I'll take the blame. :)
Created attachment 162038 [details] [review] Bug 35930: Add guards for plugins_enabled The 'new' method in Koha::Plugins returns undefined if plugins are disabled. Therefore, calls to this method must be guarded by a check that plugins actually are enabled. Test plan: * Code inspection of patch, alternatively * Activate the ill system by installing a backend such as koha-illbackend-libris: https://github.com/Libriotech/koha-illbackend-libris * Make sure plugins are disabled in koha-conf.xml * In the staff interface, go to ILL requests. * The page should load without getting an error 500. PA amended commit message: This is not related to ILL backends being plugins or not This is about ILL batches, where checking for metadata enrichment plugins was missing 'enable_plugins' guard Additionally, unrelated to batches, it's also about ILLAvailability, where checking for ILL availabililty plugins was missing enable_plugins guard Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Hans Pålsson from comment #6) > Tested the patch and it works. I mark this as PQA per request, if that´s > wrong I'll take the blame. :) It never hurts to have more testers :-D Don't forget to stamp your signature on the patch, though. ``` git commit --amend -s --no-edit ``` Best regards
I added Hans Pålsson signoff line inline and have pushed to master for 24.05.00 to allow rmaints to backport for the next releases. Thanks for your work everyone.
Pushed to 23.11.x for 23.11.03