Bug 27114 - Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab'
Summary: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab'
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 23050
Blocks: 27120
  Show dependency treegraph
 
Reported: 2020-11-30 07:28 UTC by Fridolin Somers
Modified: 2021-12-13 21:10 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Koha plugins hook 'intranet_catalog_biblio_tab' now uses Template Toolkit plugin (like hook 'intranet_js', ...). It makes it easy to use it in other places (like MARC details page for example).
Version(s) released in:
21.05.00,20.11.06,20.05.12


Attachments
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab' (5.38 KB, patch)
2020-11-30 07:43 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab' (5.44 KB, patch)
2021-03-09 16:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27114: (QA follow-up) Add missing POD (1.23 KB, patch)
2021-03-09 16:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27114: Use Template Toolkit plugin for Koha plugins hook 'intranet_catalog_biblio_tab' (5.51 KB, patch)
2021-04-09 18:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 27114: (QA follow-up) Add missing POD (1.30 KB, patch)
2021-04-09 18:24 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2020-11-30 07:28:42 UTC
Koha plugins hook 'intranet_catalog_biblio_tab' is for a template, it will be better it uses Template Toolkit plugin like intranet_js, ...
Comment 1 Fridolin Somers 2020-11-30 07:43:56 UTC
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
Comment 2 David Cook 2020-12-02 01:46:21 UTC
Can you explain why it would be better to do this in a Template::Toolkit plugin rather than doing it in the Perl script?
Comment 3 Fridolin Somers 2020-12-02 15:27:26 UTC
(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 ?
Comment 4 David Cook 2020-12-03 00:46:56 UTC
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...)
Comment 5 Martin Renvoize 2021-03-09 16:55:49 UTC
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>
Comment 6 Martin Renvoize 2021-03-09 16:55:52 UTC
Created attachment 117996 [details] [review]
Bug 27114: (QA follow-up) Add missing POD
Comment 7 Martin Renvoize 2021-03-09 16:56:29 UTC
This works well and makes allot of sense to me..
Signing off
Comment 8 Martin Renvoize 2021-03-09 16:57:56 UTC
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.
Comment 9 Fridolin Somers 2021-03-11 08:08:07 UTC
Indeed, but are they known use of this hook ?
We should add plugins usage (and there hooks) to HEA :D
Comment 10 Kyle M Hall 2021-04-09 18:24:30 UTC
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>
Comment 11 Kyle M Hall 2021-04-09 18:24:40 UTC
Created attachment 119430 [details] [review]
Bug 27114: (QA follow-up) Add missing POD

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Jonathan Druart 2021-04-12 11:42:45 UTC
(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?
Comment 13 Fridolin Somers 2021-04-12 12:43:32 UTC
(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.
Comment 14 Jonathan Druart 2021-04-21 08:15:37 UTC
Release notes needed.
Comment 15 Jonathan Druart 2021-04-22 12:39:39 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 16 Fridolin Somers 2021-04-29 13:48:12 UTC
Pushed to 20.11.x for 20.11.06
Comment 17 Andrew Fuerste-Henry 2021-05-24 16:38:10 UTC
Pushed to 20.05.x for 20.05.12
Comment 18 Victor Grousset/tuxayo 2021-05-24 17:02:06 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.