[Bug 37872] New: System information tab on About Koha page crashes with plugins disabled and ILLModule enabled
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Bug ID: 37872 Summary: System information tab on About Koha page crashes with plugins disabled and ILLModule enabled Change sponsored?: --- Product: Koha Version: 24.05 Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Plugin architecture Assignee: koha-bugs@lists.koha-community.org Reporter: andreas.jonsson@kreablo.se QA Contact: testopia@bugs.koha-community.org This is similar to bug 35930. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Andreas Jonsson <andreas.jonsson@kreablo.se> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #1 from Andreas Jonsson <andreas.jonsson@kreablo.se> --- Created attachment 171227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171227&action=edit Bug 37872: Add guards for enable_plugins There are ILL-backends that are not implemented as Koha plugins 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. Testplan for koha-testing-docker: - Edit /etc/koha/sites/kohadev/koha-conf.xml and disable plugins by setting <enable_plugins>0</enable_plugins> - sudo koha-mysql kohadev -e "UPDATE systempreferences SET value='1' WHERE variable='ILLModule';" - Run restart_all. - In staff interface, go to About koha -> System information. The page should load normally (as opposed to returning 500 internal server error). -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171227|0 |1 is obsolete| | --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 171237 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=171237&action=edit Bug 37872: Add guards for enable_plugins There are ILL-backends that are not implemented as Koha plugins 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. Testplan for koha-testing-docker: - Edit /etc/koha/sites/kohadev/koha-conf.xml and disable plugins by setting <enable_plugins>0</enable_plugins> - sudo koha-mysql kohadev -e "UPDATE systempreferences SET value='1' WHERE variable='ILLModule';" - Run restart_all. - In staff interface, go to About koha -> System information. The page should load normally (as opposed to returning 500 internal server error). Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Text to go in the| |This fixes an issue when release notes| |plugins are not enabled and | |the ILL module is enabled. | |This caused an error on the | |About Koha > System | |information section. Assignee|koha-bugs@lists.koha-commun |andreas.jonsson@kreablo.se |ity.org | --- Comment #3 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. I tested on main. 2. I updated the assignee -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- git grepped to avoid yet another bug and found: Koha/ILL/Request.pm 408 sub get_backend_plugin { 409 my ( $self, $backend_id ) = @_; 410 411 my @backend_plugins = Koha::Plugins->new()->GetPlugins( Not relevant as well? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com, | |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |pedro.amorim@ptfs-europe.co |y.org |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Looking here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173617&action=edit Bug 37872: Add tests $ prove t/db_dependent/Koha/ILL/Request.t $ prove t/db_dependent/Koha/ILL/Request/Config.t 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=37872 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #171237|0 |1 is obsolete| | --- Comment #7 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173618 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173618&action=edit Bug 37872: Add guards for enable_plugins There are ILL-backends that are not implemented as Koha plugins 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. Testplan for koha-testing-docker: - Edit /etc/koha/sites/kohadev/koha-conf.xml and disable plugins by setting <enable_plugins>0</enable_plugins> - sudo koha-mysql kohadev -e "UPDATE systempreferences SET value='1' WHERE variable='ILLModule';" - Run restart_all. - In staff interface, go to About koha -> System information. The page should load normally (as opposed to returning 500 internal server error). Signed-off-by: David Nind <david@davidnind.com> 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=37872 --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 173619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173619&action=edit Bug 37872: Consider enable_plugins = 0 when fetching plugin backends This patch builds on top of Andreas patch checking for Koha::Plugins->new() instead as the enable_plugins is already done there. It also adds a check in reply to Joubu's observation, which is indeed an additional bug. Test plan: 1) Apply tests patch. Run the tests. Notice it fails 2) Apply the remaining patches. Run the tests. Notice they pass. Additional (and better) tests can be written, but for now I think this is good enough. Additional tests should be written on top of bug 36197 (which adds a dummy ILL backend plugin for testing), at: https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=36197&attachment=171371 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=37872 --- Comment #9 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi guys, I followed up on this a bit. @Andreas, @David are you available to take a look and let me know if you agree / sign-off? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This fixes an issue when |This fixes an issue when release notes|plugins are not enabled and |plugins are not enabled and |the ILL module is enabled. |the ILL module is enabled. |This caused an error on the |This caused an error on the |About Koha > System |About Koha > System |information section. |information section. | |This | |also fixes a page error | |shown when accessing the | |ILL module with | |enable_plugins = 0 in | |koha-conf.xml. Priority|P5 - low |P3 Summary|System information tab on |ILL module has issues when |About Koha page crashes |plugins are disabled |with plugins disabled and |(enable_plugins = 0) |ILLModule enabled | Severity|normal |major --- Comment #10 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Updating this to major as this may lead to a halt in service in the certain circumstances. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 David Nind <david@davidnind.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=37872 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173617|0 |1 is obsolete| | --- Comment #11 from David Nind <david@davidnind.com> --- Created attachment 173727 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173727&action=edit Bug 37872: Add tests $ prove t/db_dependent/Koha/ILL/Request.t $ prove t/db_dependent/Koha/ILL/Request/Config.t Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173618|0 |1 is obsolete| | --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 173728 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173728&action=edit Bug 37872: Add guards for enable_plugins There are ILL-backends that are not implemented as Koha plugins 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. Testplan for koha-testing-docker: - Edit /etc/koha/sites/kohadev/koha-conf.xml and disable plugins by setting <enable_plugins>0</enable_plugins> - sudo koha-mysql kohadev -e "UPDATE systempreferences SET value='1' WHERE variable='ILLModule';" - Run restart_all. - In staff interface, go to About koha -> System information. The page should load normally (as opposed to returning 500 internal server error). Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173619|0 |1 is obsolete| | --- Comment #13 from David Nind <david@davidnind.com> --- Created attachment 173729 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173729&action=edit Bug 37872: Consider enable_plugins = 0 when fetching plugin backends This patch builds on top of Andreas patch checking for Koha::Plugins->new() instead as the enable_plugins is already done there. It also adds a check in reply to Joubu's observation, which is indeed an additional bug. Test plan: 1) Apply tests patch. Run the tests. Notice it fails 2) Apply the remaining patches. Run the tests. Notice they pass. Additional (and better) tests can be written, but for now I think this is good enough. Additional tests should be written on top of bug 36197 (which adds a dummy ILL backend plugin for testing), at: https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=36197&attachment=171371 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173727|0 |1 is obsolete| | --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173771 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173771&action=edit Bug 37872: Add tests $ prove t/db_dependent/Koha/ILL/Request.t $ prove t/db_dependent/Koha/ILL/Request/Config.t Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=37872 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173728|0 |1 is obsolete| | --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173772 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173772&action=edit Bug 37872: Add guards for enable_plugins There are ILL-backends that are not implemented as Koha plugins 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. Testplan for koha-testing-docker: - Edit /etc/koha/sites/kohadev/koha-conf.xml and disable plugins by setting <enable_plugins>0</enable_plugins> - sudo koha-mysql kohadev -e "UPDATE systempreferences SET value='1' WHERE variable='ILLModule';" - Run restart_all. - In staff interface, go to About koha -> System information. The page should load normally (as opposed to returning 500 internal server error). Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=37872 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173729|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173773 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173773&action=edit Bug 37872: Consider enable_plugins = 0 when fetching plugin backends This patch builds on top of Andreas patch checking for Koha::Plugins->new() instead as the enable_plugins is already done there. It also adds a check in reply to Joubu's observation, which is indeed an additional bug. Test plan: 1) Apply tests patch. Run the tests. Notice it fails 2) Apply the remaining patches. Run the tests. Notice they pass. Additional (and better) tests can be written, but for now I think this is good enough. Additional tests should be written on top of bug 36197 (which adds a dummy ILL backend plugin for testing), at: https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=36197&attachment=171371 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@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=37872 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #17 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- I did a final run-through on this to get it over the line.. happy to call it PQA given all the eyes on it and my own testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #18 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Unit tests, updated POD and release notes! :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |24.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.11! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Pushed to main |Pushed to stable Version(s)|24.11.00 |24.11.00,24.05.06 released in| | --- Comment #20 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Backported to 24.05.x for upcoming 24.05.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #21 from Fridolin Somers <fridolin.somers@biblibre.com> --- Does not apply easy on 23.11.x Looks like depends on Bug 35581 for test suite Can I backport only main patch ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #22 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Fridolin Somers from comment #21)
Does not apply easy on 23.11.x Looks like depends on Bug 35581 for test suite Can I backport only main patch ?
Hi Frido, I'm fairly confident this is only required after bug 19605, and that was only pushed to 24.05. So no further backport should be required here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #23 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Pedro Amorim from comment #22)
Hi Frido, I'm fairly confident this is only required after bug 19605, and that was only pushed to 24.05. So no further backport should be required here.
Upon second read, I may be wrong here, it may have been originally introduced by bug 30719 (23.11). Let me test this on 23.11.x, confirm the bug is there and see if I can provide a rebased version of main patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 --- Comment #24 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Pedro Amorim from comment #23)
Upon second read, I may be wrong here, it may have been originally introduced by bug 30719 (23.11). Let me test this on 23.11.x, confirm the bug is there and see if I can provide a rebased version of main patch.
Tested this on 23.11.x, there's no issue. This was indeed caused by bug 19605 not in 23.11.x. No backport needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Needs documenting --- Comment #25 from Fridolin Somers <fridolin.somers@biblibre.com> --- Thanks a lot for your tests Pedro -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37872 Aude Charillon <aude.charillon@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |aude.charillon@ptfs-europe. | |com Status|Needs documenting |RESOLVED --- Comment #26 from Aude Charillon <aude.charillon@ptfs-europe.com> --- No need for any updates to the Koha Manual. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org