|
Description
Philip Orr
2024-11-07 09:56:55 UTC
We had discussed that at the hackfest and it has caused great trouble for some plugin users. We should also consider adding a warning to documentation, even the advent calendar. Created attachment 174150 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha I think this may solve the issue. I don't have a specific test plan to move this forward though. Is this ready for testing, Kyle? And thanks! The problem is that this fix creates a circular dependency that just breaks. We actually need a way to move the logic within 'get_enabled_plugins' into the BEGIN block of C4::Context w/o depending on Koha::Plugins at all. To test (preliminary): 1) Get the koha-plugin-fancyplugin from KohaAdvent. 2) Install it 3) Use test_transaction.pl and observe that due to the reinitialization, the transaction breaks and the biblionumber is returned(!) but after that we can't find the corresponding biblio. Then: 4) Find a way to implement comment #5 5) Apply your patch 6) See that the biblio is found by Koha::Biblios->find Created attachment 174264 [details]
koha-plugin-fancyplugin from KohaAdvent
Created attachment 174265 [details]
Test case (script) to reproduce the breaking database transaction
Using Koha::Cache::Memory::Lite in C4::Context's BEGIN should be find, though. Created attachment 174280 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block Not too sure on that latest patch. I essentially only translated the call to Koha::Plugins->get_enabled_plugins to an inlined version without dependencies on other local modules. This is however very ugly, I am just curious what others think. Thoughts? Created attachment 174282 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first I don't think we should be using XML::Simple here, even for a simple use case. Worth exploring other fast alternatives though... Btw, you should be able to use Koha::Database instead of DBI directly there, since it's already lightweight (and uses the same modules, so it's same-same but with less duplicated code). (In reply to David Cook from comment #13) > Btw, you should be able to use Koha::Database instead of DBI directly there, > since it's already lightweight (and uses the same modules, so it's same-same > but with less duplicated code). For instance, look at bug 37657 where C4::Context->dbh is replaced by Koha::Database->dbh for big speed gain. We have "TableExists" in C4::Installer - it doesn't need a different syntax per engine, could we do the same here? Select all form the table and check if any warning (In reply to Nick Clemens (kidclamp) from comment #15) > We have "TableExists" in C4::Installer - it doesn't need a different syntax > per engine, could we do the same here? Select all form the table and check > if any warning Did you comment on the right bug here, Nick? @David I think Nick means the "TableExists" here: https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=174282&action=diff&headers=0#a/C4/Context.pm_sec3 (In reply to Philip Orr from comment #17) > @David I think Nick means the "TableExists" here: > https://bugs.koha-community.org/bugzilla3/attachment. > cgi?id=174282&action=diff&headers=0#a/C4/Context.pm_sec3 Yeah, just suggesting we could use existing code in C4/Installer: 719 sub TableExists { # Could be renamed table_exists for consistency 720 my $table = shift; 721 eval { 722 my $dbh = C4::Context->dbh; 723 local $dbh->{PrintError} = 0; 724 local $dbh->{RaiseError} = 1; 725 $dbh->do(qq{SELECT * FROM $table WHERE 1 = 0 }); 726 }; 727 return 1 unless $@; 728 return 0; Created attachment 189636 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Created attachment 189637 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Created attachment 189638 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Created attachment 189639 [details] [review] Bug 38384: Refactor plugin loading to use lightweight Koha::Plugins::Loader This patch addresses the circular dependency issues and code quality concerns identified in QA by introducing a clean architectural solution. Changes: 1. Created Koha::Plugins::Loader - A lightweight plugin loader with minimal dependencies that can be safely called during early initialization. Uses DBIx::Class when available for proper transaction support, falls back to raw DBI for early init. 2. Added Koha::Database::table_exists() - Consolidated table checking logic into a reusable method that can be called as both class and instance method. 3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to ~13 lines (91% reduction). Removed XML::Simple dependency and manual database connection code. 4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to the Loader module for consistency. 5. Updated C4::Installer::TableExists - Delegates to the new Koha::Database::table_exists method. This addresses all QA concerns: - Eliminates XML::Simple usage (David Cook) - Uses Koha::Database instead of manual DBI (David Cook) - Reuses table checking logic (Nick Clemens) - Eliminates circular dependencies (Paul Derscheid) - Much cleaner and maintainable code Test plan: 1. Run tests: prove t/db_dependent/Koha/Database.t prove t/db_dependent/Koha/Plugins/Loader.t prove t/db_dependent/Koha/Plugins/Plugins.t 2. All tests should pass 3. Install and enable a plugin via the UI 4. Verify plugin hooks are called correctly 5. Verify no regressions in plugin functionality Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> OK.. So this bit me hard today in edi_cron.pl. It gave me the impetus to look carefully at all the QA comments here and come up with my own follow-up that hopefully addresses them all. Created attachment 189640 [details] [review] Bug 38384: Refactor plugin loading to use lightweight Koha::Plugins::Loader This patch addresses the circular dependency issues and code quality concerns identified in QA by introducing a clean architectural solution. Changes: 1. Created Koha::Plugins::Loader - A lightweight plugin loader with minimal dependencies that can be safely called during early initialization. Uses DBIx::Class when available for proper transaction support, falls back to raw DBI for early init. 2. Added Koha::Database::table_exists() - Consolidated table checking logic into a reusable method that can be called as both class and instance method. 3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to ~13 lines (91% reduction). Removed XML::Simple dependency and manual database connection code. 4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to the Loader module for consistency. 5. Updated C4::Installer::TableExists - Delegates to the new Koha::Database::table_exists method. This addresses all QA concerns: - Eliminates XML::Simple usage (David Cook) - Uses Koha::Database instead of manual DBI (David Cook) - Reuses table checking logic (Nick Clemens) - Eliminates circular dependencies (Paul Derscheid) - Much cleaner and maintainable code Test plan: 1. Run tests: prove t/db_dependent/Koha/Database.t prove t/db_dependent/Koha/Plugins/Loader.t prove t/db_dependent/Koha/Plugins/Plugins.t 2. All tests should pass 3. Install and enable a plugin via the UI 4. Verify plugin hooks are called correctly 5. Verify no regressions in plugin functionality Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Looking here Looks good to me. But reading the referred 36736 I would appreciate any proof of testing with a plugin that adds schema files from a plugin directory like Koha::Plugin::FancyPlugin seems to do. Since the last patch rearranges quite a lot, another signoff doing so would be nice. Wondering if we really break current transactions btw, or do we create connections that do not include all plugin table schemas and fail for that reason?
> Wondering if we really break current transactions btw, or do we create connections that do not include all plugin table schemas and fail for that reason?
I can say from what we observed that this breaks unrelated transactions.
It does indeed break unrelated transactions. By killing the connection on first plugin load we nuke whatever transaction may be in progress a d as such a rollback happens. Created attachment 190514 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: John Doe <you@example.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 190515 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: John Doe <you@example.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 190516 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: John Doe <you@example.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 190517 [details] [review] Bug 38384: Refactor plugin loading to use lightweight Koha::Plugins::Loader This patch addresses the circular dependency issues and code quality concerns identified in QA by introducing a clean architectural solution. Changes: 1. Created Koha::Plugins::Loader - A lightweight plugin loader with minimal dependencies that can be safely called during early initialization. Uses DBIx::Class when available for proper transaction support, falls back to raw DBI for early init. 2. Added Koha::Database::table_exists() - Consolidated table checking logic into a reusable method that can be called as both class and instance method. 3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to ~13 lines (91% reduction). Removed XML::Simple dependency and manual database connection code. 4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to the Loader module for consistency. 5. Updated C4::Installer::TableExists - Delegates to the new Koha::Database::table_exists method. This addresses all QA concerns: - Eliminates XML::Simple usage (David Cook) - Uses Koha::Database instead of manual DBI (David Cook) - Reuses table checking logic (Nick Clemens) - Eliminates circular dependencies (Paul Derscheid) - Much cleaner and maintainable code Test plan: 1. Run tests: prove t/db_dependent/Koha/Database.t prove t/db_dependent/Koha/Plugins/Loader.t prove t/db_dependent/Koha/Plugins/Plugins.t 2. All tests should pass 3. Install and enable a plugin via the UI 4. Verify plugin hooks are called correctly 5. Verify no regressions in plugin functionality Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> I haven't read the code, but I confirm this patches DON'T FIX something similar I found and reported on bug 41603. This looks good, however, if I install the fancy plugin and run the test script I get "No biblio" It is there after the transaction. When I disable fancy plugin, then I get the biblio - so I think we haven't solved the issue Doing some debug - it seems the first pass through we don't successfully cache the plugins because they cannot be loaded, because they can't be found I tried add the addition of pluginsdir to @INC into Koha::Plugins::Loader, but then I get failures when the plugins try to call C4::Context->config - but we haven't finished loading C4::Context yet That's as far as I could get, I don't know how to resolve that Created attachment 191860 [details] [review] Bug 38384: (follow-up) Move loading into Koha::Plugins BEGIN block Thinking about this today, we really just need the plugins initialized anytime we are using Koha::Plugins Moving the loading into the BEGIN there seems to resolve the issue That's even better. (In reply to Nick Clemens (kidclamp) from comment #37) > Created attachment 191860 [details] [review] [review] > Bug 38384: (follow-up) Move loading into Koha::Plugins BEGIN block > > Thinking about this today, we really just need the plugins initialized > anytime we are using Koha::Plugins > > Moving the loading into the BEGIN there seems to resolve the issue I think in addition, we should update all "require Koha::Plugins" to "use Koha::Plugins" otherwise the BEGIN block is evaluated at runtime, right? It shouldn't hurt anything at least, and we only have a handful of instances! Created attachment 192119 [details] [review] Bug 38384: (follow-up) move require Koha::Plugins to use remaning ocurrences in test files seem okay, this covers the others Created attachment 192628 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Created attachment 192629 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Created attachment 192630 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Created attachment 192631 [details] [review] Bug 38384: Refactor plugin loading to use lightweight Koha::Plugins::Loader This patch addresses the circular dependency issues and code quality concerns identified in QA by introducing a clean architectural solution. Changes: 1. Created Koha::Plugins::Loader - A lightweight plugin loader with minimal dependencies that can be safely called during early initialization. Uses DBIx::Class when available for proper transaction support, falls back to raw DBI for early init. 2. Added Koha::Database::table_exists() - Consolidated table checking logic into a reusable method that can be called as both class and instance method. 3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to ~13 lines (91% reduction). Removed XML::Simple dependency and manual database connection code. 4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to the Loader module for consistency. 5. Updated C4::Installer::TableExists - Delegates to the new Koha::Database::table_exists method. This addresses all QA concerns: - Eliminates XML::Simple usage (David Cook) - Uses Koha::Database instead of manual DBI (David Cook) - Reuses table checking logic (Nick Clemens) - Eliminates circular dependencies (Paul Derscheid) - Much cleaner and maintainable code Test plan: 1. Run tests: prove t/db_dependent/Koha/Database.t prove t/db_dependent/Koha/Plugins/Loader.t prove t/db_dependent/Koha/Plugins/Plugins.t 2. All tests should pass 3. Install and enable a plugin via the UI 4. Verify plugin hooks are called correctly 5. Verify no regressions in plugin functionality Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Created attachment 192632 [details] [review] Bug 38384: (follow-up) Move loading into Koha::Plugins BEGIN block Thinking about this today, we really just need the plugins initialized anytime we are using Koha::Plugins Moving the loading into the BEGIN there seems to resolve the issue Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Created attachment 192633 [details] [review] Bug 38384: (follow-up) move require Koha::Plugins to use remaning ocurrences in test files seem okay, this covers the others Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Created attachment 192662 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Created attachment 192663 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Created attachment 192664 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Created attachment 192665 [details] [review] Bug 38384: Refactor plugin loading to use lightweight Koha::Plugins::Loader This patch addresses the circular dependency issues and code quality concerns identified in QA by introducing a clean architectural solution. Changes: 1. Created Koha::Plugins::Loader - A lightweight plugin loader with minimal dependencies that can be safely called during early initialization. Uses DBIx::Class when available for proper transaction support, falls back to raw DBI for early init. 2. Added Koha::Database::table_exists() - Consolidated table checking logic into a reusable method that can be called as both class and instance method. 3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to ~13 lines (91% reduction). Removed XML::Simple dependency and manual database connection code. 4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to the Loader module for consistency. 5. Updated C4::Installer::TableExists - Delegates to the new Koha::Database::table_exists method. This addresses all QA concerns: - Eliminates XML::Simple usage (David Cook) - Uses Koha::Database instead of manual DBI (David Cook) - Reuses table checking logic (Nick Clemens) - Eliminates circular dependencies (Paul Derscheid) - Much cleaner and maintainable code Test plan: 1. Run tests: prove t/db_dependent/Koha/Database.t prove t/db_dependent/Koha/Plugins/Loader.t prove t/db_dependent/Koha/Plugins/Plugins.t 2. All tests should pass 3. Install and enable a plugin via the UI 4. Verify plugin hooks are called correctly 5. Verify no regressions in plugin functionality Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Created attachment 192666 [details] [review] Bug 38384: (follow-up) Move loading into Koha::Plugins BEGIN block Thinking about this today, we really just need the plugins initialized anytime we are using Koha::Plugins Moving the loading into the BEGIN there seems to resolve the issue Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Created attachment 192667 [details] [review] Bug 38384: (follow-up) move require Koha::Plugins to use remaning ocurrences in test files seem okay, this covers the others Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Marcel, are you available for QA for this? We're having a couple of plugins impacted by this. Looking here now Looks good to me. Just a few minor questions:
- my $verbose = $params->{verbose} // $class->_verbose;
Removed. Verbose test still passes. So seems to be OK.
+ my $cache_key = 'enabled_plugins';
Constant?
+ # Test caching behavior
+ my @plugins_cached = Koha::Plugins::Loader->get_enabled_plugins();
+ is( scalar @plugins_cached, 0, 'Cached empty result works correctly' );
When paranoid, you could still doubt if the cache was really used?
And what about this line: Koha::Cache::Memory::Lite->set_in_cache( $cache_key, \@plugins ) if @plugins;
This shows that an empty result is not even cached ! So this test is misleading..
Should we remove the if @plugins or adjust the test?
t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t ... 1/7 # Looks like you planned 6 tests but ran 4.
# Failed test 'after_biblio_action() and after_item_action() hooks tests'
# at t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t line 92.
[Foreign key constraint broken on deleting resource]# Looks like your test exited with 255 just after 5.
t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t ... Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 3/7 subtests
=> Fails for me too without this patch set.. So ignoring.
Although it seems bit out of scope, C4::Context was initially modified here. It does call Koha::Database but it does not include a use or require of that module. That looks wrong. (It comes with Koha::Config::Sysprefs -> Koha::Objects.)
Should we formally add it here too? And perhaps reorder the modules?
Changing status for feedback.
Created attachment 193142 [details] [review] Bug 38384: (QA follow-up) Restore verbose option Created attachment 193143 [details] [review] Bug 38384: (follow-up) Improve tests for caching Created attachment 193144 [details] [review] Bug 38384: (follow-up) use Koha::Database in C4::Context Created attachment 193171 [details] [review] Bug 38384: Trigger plugin modules loading as soon as possible in Koha Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193172 [details] [review] Bug 38384: Inline plugin loading into C4::Context's BEGIN block Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193173 [details] [review] Bug 38384: Prevent initialization failure by checking for existence of plugin_data first Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193174 [details] [review] Bug 38384: Refactor plugin loading to use lightweight Koha::Plugins::Loader This patch addresses the circular dependency issues and code quality concerns identified in QA by introducing a clean architectural solution. Changes: 1. Created Koha::Plugins::Loader - A lightweight plugin loader with minimal dependencies that can be safely called during early initialization. Uses DBIx::Class when available for proper transaction support, falls back to raw DBI for early init. 2. Added Koha::Database::table_exists() - Consolidated table checking logic into a reusable method that can be called as both class and instance method. 3. Simplified C4::Context BEGIN block - Reduced from ~120 lines to ~13 lines (91% reduction). Removed XML::Simple dependency and manual database connection code. 4. Updated Koha::Plugins::get_enabled_plugins() - Now delegates to the Loader module for consistency. 5. Updated C4::Installer::TableExists - Delegates to the new Koha::Database::table_exists method. This addresses all QA concerns: - Eliminates XML::Simple usage (David Cook) - Uses Koha::Database instead of manual DBI (David Cook) - Reuses table checking logic (Nick Clemens) - Eliminates circular dependencies (Paul Derscheid) - Much cleaner and maintainable code Test plan: 1. Run tests: prove t/db_dependent/Koha/Database.t prove t/db_dependent/Koha/Plugins/Loader.t prove t/db_dependent/Koha/Plugins/Plugins.t 2. All tests should pass 3. Install and enable a plugin via the UI 4. Verify plugin hooks are called correctly 5. Verify no regressions in plugin functionality Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193175 [details] [review] Bug 38384: (follow-up) Move loading into Koha::Plugins BEGIN block Thinking about this today, we really just need the plugins initialized anytime we are using Koha::Plugins Moving the loading into the BEGIN there seems to resolve the issue Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193176 [details] [review] Bug 38384: (follow-up) move require Koha::Plugins to use remaning ocurrences in test files seem okay, this covers the others Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193177 [details] [review] Bug 38384: (QA follow-up) Restore verbose option Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193178 [details] [review] Bug 38384: (follow-up) Improve tests for caching Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 193179 [details] [review] Bug 38384: (follow-up) use Koha::Database in C4::Context Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Nice work everyone! Pushed to main for 26.05 Awesome that this is fixed! Thanks for all the work. Any chance we can backport this to 25.11.x? |