Bug 35070 - Koha plugins implementing "background_jobs" hook can't provide view template
Summary: Koha plugins implementing "background_jobs" hook can't provide view template
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 30410
Blocks:
  Show dependency treegraph
 
Reported: 2023-10-17 01:32 UTC by David Cook
Modified: 2024-02-15 08:05 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.02,23.05.08


Attachments
Bug 35070: Proof of concept (1.27 KB, patch)
2023-10-18 14:28 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 35070: Add plugin hook template_include_paths (4.97 KB, patch)
2023-11-23 11:11 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 35070: Add plugin hook template_include_paths (5.01 KB, patch)
2023-11-25 00:00 UTC, David Nind
Details | Diff | Splinter Review
Bug 35070: Add plugin hook template_include_paths (5.07 KB, patch)
2023-12-15 20:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35070: Tidy (3.23 KB, patch)
2023-12-15 20:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35070: Add test verifying plugins cannot override core templates (2.15 KB, patch)
2023-12-18 07:38 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 35070: Add test verifying plugins cannot override core templates (2.21 KB, patch)
2023-12-29 19:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 35070: Fix get_enabled_plugins when database is not created yet (1.51 KB, patch)
2024-01-05 08:07 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2023-10-17 01:32:33 UTC
At the moment, Koha plugins implementing the "background_jobs" hook can't provide a template to "View" the job. 

Instead, you get an error like:

Template process failed: file error - background_jobs/plugin_TYPE_ACTION.inc: not found at /kohadevbox/koha/C4/Templates.pm line 127

This should be a pretty easy fix. Basically, Koha::BackgroundJob should have a method that returns "background_jobs/{job type}" and a Koha plugin can override this method to provide its own template. 

We use the ABSOLUTE option for Template::Toolkit, so you can provide an absolute filepath to a template within the plugin and it'll get processed.
Comment 1 Julian Maurice 2023-10-18 14:09:46 UTC
Another solution would be to let plugins add paths to Template::Toolkit's INCLUDE_PATH option. It would also allow plugin's templates to INCLUDE their own .inc files
Comment 2 Julian Maurice 2023-10-18 14:28:33 UTC
Created attachment 157323 [details] [review]
Bug 35070: Proof of concept

Add the following methods to your plugin

    sub template_include_paths {
        my ($self) = @_;

        return [
            $self->mbf_path('inc'),
        ]
    }

Then create a file
Koha/Plugin/YourPlugin/inc/background_jobs/plugin_NAMESPACE_JOBTYPE.inc
with 3 TT BLOCKs: "process", "report" and "js"
Comment 3 David Cook 2023-10-23 03:00:59 UTC
(In reply to Julian Maurice from comment #1)
> Another solution would be to let plugins add paths to Template::Toolkit's
> INCLUDE_PATH option. It would also allow plugin's templates to INCLUDE their
> own .inc files

I was thinking about that the other day. Sounds good to me I think.
Comment 4 Julian Maurice 2023-11-23 11:11:36 UTC
Created attachment 159205 [details] [review]
Bug 35070: Add plugin hook template_include_paths

It allows to add paths to Template::Toolkit's INCLUDE_PATH option

http://template-toolkit.org/docs/manual/Config.html#section_INCLUDE_PATH

Test plan:
1. Install the modified kitchen sink plugin:

    git clone --branch template-include-paths \
    https://github.com/jajm/dev-koha-plugin-kitchen-sink.git

2. Run misc/devel/install_plugins.pl
3. Restart memcached and koha
4. Go to Administration -> Manage Plugins
5. Run the KitchenSink plugin's tool
6. Click on "Schedule greeting"
7. Go to Administration -> Manage jobs
8. If you don't see any jobs, uncheck "Current jobs only"
9. You should see a job of type "Unknown job type
   'plugin_kitchensink_greeter". Click on the "View" button
10. Under the Report section you should see "This is the report block"
11. Under the Detailed messages section you should see "This is the
    detail block"
12. Open the browser console, you should see a message "This is the js
    block"
Comment 5 Julian Maurice 2023-11-23 11:12:26 UTC
Added a test, and forked the kitchen sink plugin so that it's easier to test.
Comment 6 David Nind 2023-11-25 00:00:15 UTC
Created attachment 159248 [details] [review]
Bug 35070: Add plugin hook template_include_paths

It allows to add paths to Template::Toolkit's INCLUDE_PATH option

http://template-toolkit.org/docs/manual/Config.html#section_INCLUDE_PATH

Test plan:
1. Install the modified kitchen sink plugin:

    git clone --branch template-include-paths \
    https://github.com/jajm/dev-koha-plugin-kitchen-sink.git

2. Run misc/devel/install_plugins.pl
3. Restart memcached and koha
4. Go to Administration -> Manage Plugins
5. Run the KitchenSink plugin's tool
6. Click on "Schedule greeting"
7. Go to Administration -> Manage jobs
8. If you don't see any jobs, uncheck "Current jobs only"
9. You should see a job of type "Unknown job type
   'plugin_kitchensink_greeter". Click on the "View" button
10. Under the Report section you should see "This is the report block"
11. Under the Detailed messages section you should see "This is the
    detail block"
12. Open the browser console, you should see a message "This is the js
    block"

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2023-11-25 00:03:52 UTC
Testing notes (using KTD):

1. The first step in the test plan didn't work for me. Instead, I cloned the repository and moved the dev-koha-plugin-kitchen-sink/Koha directory to /var/lib/koha/kohadev/plugins/.

2. Plugins are already enabled in the kohadev/koha-conf.xml file, with the plugins directory set as /var/lib/koha/kohadev/plugins/
Comment 8 Tomás Cohen Arazi 2023-12-15 20:01:04 UTC
Created attachment 159901 [details] [review]
Bug 35070: Add plugin hook template_include_paths

It allows to add paths to Template::Toolkit's INCLUDE_PATH option

http://template-toolkit.org/docs/manual/Config.html#section_INCLUDE_PATH

Test plan:
1. Install the modified kitchen sink plugin:

    git clone --branch template-include-paths \
    https://github.com/jajm/dev-koha-plugin-kitchen-sink.git

2. Run misc/devel/install_plugins.pl
3. Restart memcached and koha
4. Go to Administration -> Manage Plugins
5. Run the KitchenSink plugin's tool
6. Click on "Schedule greeting"
7. Go to Administration -> Manage jobs
8. If you don't see any jobs, uncheck "Current jobs only"
9. You should see a job of type "Unknown job type
   'plugin_kitchensink_greeter". Click on the "View" button
10. Under the Report section you should see "This is the report block"
11. Under the Detailed messages section you should see "This is the
    detail block"
12. Open the browser console, you should see a message "This is the js
    block"

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2023-12-15 20:01:09 UTC
Created attachment 159902 [details] [review]
Bug 35070: Tidy

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Tomás Cohen Arazi 2023-12-15 20:04:06 UTC
Julian, can you add a test proving plugin authors cannot override (maliciously) core templates?
Comment 11 Julian Maurice 2023-12-18 07:38:19 UTC
Created attachment 159938 [details] [review]
Bug 35070: Add test verifying plugins cannot override core templates
Comment 12 Julian Maurice 2023-12-18 07:41:16 UTC
The added test is not very robust, as it depends on the existence of a specific core template. If this core template is renamed or removed, the test will fail.
That's the best I can do right now.
Comment 13 Katrin Fischer 2023-12-19 12:37:23 UTC
(In reply to Julian Maurice from comment #12)
> The added test is not very robust, as it depends on the existence of a
> specific core template. If this core template is renamed or removed, the
> test will fail.
> That's the best I can do right now.

Tomas, can you please check if Julian's follow-up does what you had in mind?
Comment 14 Tomás Cohen Arazi 2023-12-29 19:14:43 UTC
Created attachment 160378 [details] [review]
Bug 35070: Add test verifying plugins cannot override core templates

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2023-12-29 19:37:49 UTC
This should be backported to stables, IMHO.
Comment 16 Katrin Fischer 2024-01-04 10:43:55 UTC
 FAIL	t/lib/plugins/Koha/Plugin/Test/inc/csrf-token.inc
   FAIL	  filters
		missing_filter at line 2 ([%~ 'OVERRIDE' ~%])

Will fix.
Comment 17 Katrin Fischer 2024-01-04 15:53:40 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 18 Jonathan Druart 2024-01-05 07:09:51 UTC
t/db_dependent/selenium/00-onboarding.t is failing

I am seeing this in the logs:
                                                                DBIx::Class::Storage::DBI::_dbh_execute(): Table 'koha_kohadev.plugin_data' doesn't exist at /kohadevbox/koha/Koha/Plugins.pm line 122                                                                               
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77                                                                                                                                                                 
        DBIx::Class::Exception::throw('DBIx::Class::Exception', 'Table \'koha_kohadev.plugin_data\' doesn\'t exist', 0) called at /usr/share/perl5/DBIx/Class/Schema.pm line 1118                                            DBIx::Class::Schema::throw_exception('Koha::Schema=HASH(0x55fc8aec4730)', 'Table \'koha_kohadev.plugin_data\' doesn\'t exist') called at /usr/share/perl5/DBIx/Class/Storage.pm line 113
Comment 19 Julian Maurice 2024-01-05 07:40:23 UTC
Should we add a followup here or on a new bug report ?
Comment 20 Julian Maurice 2024-01-05 08:07:16 UTC
Created attachment 160565 [details] [review]
Bug 35070: Fix get_enabled_plugins when database is not created yet

Koha::Plugins::get_enabled_plugins is called by Koha::Plugins::call
which is called by C4::Templates->new, which is used in the web
installer when the database is not created yet
Comment 21 Julian Maurice 2024-01-05 08:09:08 UTC
I didn't try t/db_dependent/selenium/00-onboarding.t but I was able to reproduce the error by running the web installer. After the patch I was able to complete the installation and the onboarding process.
Comment 22 Katrin Fischer 2024-01-05 14:00:33 UTC
Pushed the follow-up to master. Thanks for taking care of this so quickly!
Comment 23 Fridolin Somers 2024-01-08 14:16:59 UTC
Hook moved in wiki page :
https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks#Implemented_hooks
Comment 24 Fridolin Somers 2024-01-08 14:28:38 UTC
Pushed to 23.11.x for 23.11.02
Comment 25 Lucas Gass 2024-01-12 22:17:55 UTC
Backported to 23.05.x for upcoming 23.05.08