Bug 39031

Summary: ILL plugins should provide themselves the template path
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: ILLAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Needs Signoff --- QA Contact: Pedro Amorim <pedro.amorim>
Severity: enhancement    
Priority: P5 - low CC: pedro.amorim, tomascohen
Version: MainKeywords: rel_24_05_candidate, rel_24_11_candidate
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 19605    
Bug Blocks:    
Attachments: Bug 39031: Use the backend's bundle_path() method

Description Tomás Cohen Arazi (tcohen) 2025-02-04 19:46:35 UTC
Right now, Koha assumes (in `Koha::ILL::Request->expand_template`) that `$plugin->new_ill_backend` returns a Koha::Plugin object that implements the required methods for acting like a proper ILL backend.

This is implied by the internal use of `bundle_path`:

```perl
    my $backend_plugin = $self->get_backend_plugin( $self->_backend->name );
    if ($backend_plugin) {

        # New way of loading backends: Through plugins
        $backend_dir  = $backend_plugin->bundle_path;
        $backend_tmpl = $backend_dir;
```

I'm writing a backend plugin that has a convenient `templates/` dir to better organize the many pieces. My `new_ill_backend()` method returns a new object that is designed to eventually inherit from `Koha::ILL::Backend` when time comes.

The current implementation assumes that everything is contained in the plugin's bundle path in a very specific way.

I propose we either allow the backend to tell where to look for templates:

```diff
-        $backend_dir  = $backend_plugin->bundle_path;
+        $backend_dir  = $self->_backend->template_path;
```

or implement a plugin method for the job:

```diff
-        $backend_dir  = $backend_plugin->bundle_path;
+        $backend_dir  = $backend_plugin->ill_template_path;
```

i.e. just implement a plugin method, that will return the path for querying the backend templates

I'd go with the first option, leaving it to the author's creativity to organize things the way they want.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-02-04 19:51:06 UTC
I feel like just changing it like this would be flexible enough, while keeping the current behavior for plugin-as-a-backend implementations:

```diff
-        $backend_dir  = $backend_plugin->bundle_path;
+        $backend_dir  = $self->_backend->bundle_path;
```

The only weird thing that will be left in this scenario, is that both `$backend_plugin` and `$self->_backend` are instances of the same plugin... just with some extra internal variables set by `new_ill_backend()`.

But in this context we already have an instance of the plugin, loaded 'as a backend' by `new_ill_backend()`. So it is redundant to instantiate the plugin, it seems.
Comment 2 Tomás Cohen Arazi (tcohen) 2025-02-05 13:14:19 UTC
Created attachment 177533 [details] [review]
Bug 39031: Use the backend's bundle_path() method

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi (tcohen) 2025-02-05 13:15:43 UTC
Assigning Pedro for QA to get his review.
Comment 4 Tomás Cohen Arazi (tcohen) 2025-02-05 13:59:23 UTC
And please backport :-D