Bug 27114

Created attachment 114054 [details] [review]
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab'

Koha plugins hook 'intranet_catalog_biblio_tab' is for a template,
it will be better it uses Template Toolkit plugin like intranet_js, ...
It will allow using it in other places like MARC details page for example.

Test plan uses a plugin from git.biblibre.com because this hook is not
yet in KitchenSink.

Test plan :
1) Enable Koha plugins
2) Download and install the latest version of this plugin
https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook
3) Browse to catalogue/detail.pl for a record
4) Note you see two new tabs with content

Can you explain why it would be better to do this in a Template::Toolkit plugin rather than doing it in the Perl script?

(In reply to David Cook from comment #2)
> Can you explain why it would be better to do this in a Template::Toolkit
> plugin rather than doing it in the Perl script?


It will be easier to add it in other places, like MARC details page for example.
Hooks intranet/opac_js/header are already in this TT plugin.
Maybe easier to include in test suite ?

Hmm ok. I think I was opposed to this idea yesterday, but for some reason I seem in favour of it today heh.

Since it's not modifying any existing data in detail.pl, I suppose there's no reason why it shouldn't just be handled in detail.tt, since it's really just updating the view.

And then Bug 27120 makes sense too. 

In fact... I recently did a local customization that probably could've been done as a plugin, if Bug 27120 were pushed. (Actually, it was an OPAC one... so not relevant here, so having an OPAC equivalent would be good after all...)

Created attachment 117995 [details] [review]
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab'

Koha plugins hook 'intranet_catalog_biblio_tab' is for a template,
it will be better it uses Template Toolkit plugin like intranet_js, ...
It will allow using it in other places like MARC details page for example.

Test plan uses a plugin from git.biblibre.com because this hook is not
yet in KitchenSink.

Test plan :
1) Enable Koha plugins
2) Download and install the latest version of this plugin
https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook
3) Browse to catalogue/detail.pl for a record
4) Note you see two new tabs with content

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 117996 [details] [review]
Bug 27114: (QA follow-up) Add missing POD

This works well and makes allot of sense to me..
Signing off

Comment on attachment 117995 [details] [review]
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab'

Review of attachment 117995 [details] [review]:
-----------------------------------------------------------------

This is a breaking change.. I think..  We'll need to advertise that in the release notes to prompt plugin authors to update their plugins.

::: Koha/Template/Plugin/KohaPlugins.pm
@@ +166,5 @@
> +        try {
> +            my @newtabs = $plugin->intranet_catalog_biblio_tab();
> +            foreach my $newtab (@newtabs) {
> +                # Add a unique HTML id
> +                my $html_id = 'tab-'. $plugin->{class} . '-' . $newtab->title;

The addition of $plugin->{class} here could cause problems for existing plugins.. we will need to make sure any in the wild are updated.

Indeed, but are they known use of this hook ?
We should add plugins usage (and there hooks) to HEA :D

Created attachment 119429 [details] [review]
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab'

Koha plugins hook 'intranet_catalog_biblio_tab' is for a template,
it will be better it uses Template Toolkit plugin like intranet_js, ...
It will allow using it in other places like MARC details page for example.

Test plan uses a plugin from git.biblibre.com because this hook is not
yet in KitchenSink.

Test plan :
1) Enable Koha plugins
2) Download and install the latest version of this plugin
https://git.biblibre.com/biblibre/koha-plugin-intranet-detail-hook
3) Browse to catalogue/detail.pl for a record
4) Note you see two new tabs with content

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 119430 [details] [review]
Bug 27114: (QA follow-up) Add missing POD

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

(In reply to Martin Renvoize from comment #8)
> Comment on attachment 117995 [details] [review] [review]
> Bug 27114: Use Template Toolkit plugin for Koha plugins hook
> 'intranet_catalog_biblio_tab'
> 
> Review of attachment 117995 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> This is a breaking change.. I think..  We'll need to advertise that in the
> release notes to prompt plugin authors to update their plugins.
> 
> ::: Koha/Template/Plugin/KohaPlugins.pm
> @@ +166,5 @@
> > +        try {
> > +            my @newtabs = $plugin->intranet_catalog_biblio_tab();
> > +            foreach my $newtab (@newtabs) {
> > +                # Add a unique HTML id
> > +                my $html_id = 'tab-'. $plugin->{class} . '-' . $newtab->title;
> 
> The addition of $plugin->{class} here could cause problems for existing
> plugins.. we will need to make sure any in the wild are updated.

Why do we add this change exactly? To prevent collision?

(In reply to Jonathan Druart from comment #12)

> > The addition of $plugin->{class} here could cause problems for existing
> > plugins.. we will need to make sure any in the wild are updated.
> 
> Why do we add this change exactly? To prevent collision?
Indeed.
Two plugins may add a tab with same title.
But this may be very rare.
We can remove this if it causes problems.

Release notes needed.

Pushed to master for 21.05, thanks to everybody involved!

Pushed to 20.11.x for 20.11.06

Pushed to 20.05.x for 20.05.12

Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.