Bug 23050

Summary: Add hook to add a tab in intranet biblio details page
Product: Koha Reporter: Matthias Meusburger <matthias.meusburger>
Component: Plugin architectureAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: new feature    
Priority: P5 - low CC: arthur.suzuki, julian.maurice, katrin.fischer, kyle, magnus, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This new feature allows plugin authors to add additional tabs to the intranet biblio details page. The new `intranet_catalog_biblio_tab` method which should return an array of `Koha::Plugins::Tab` objects is introduced. **Warning**: Care should be taken when installing any plugins and only plugins you trust should be used.
Version(s) released in:
19.11.00
Bug Depends on:    
Bug Blocks: 27114    
Attachments: Bug 23050: Plugin hook to add tabs in intranet biblio details page
Bug 23050: Plugin hook to add tabs in intranet biblio details page
Bug 23050: Plugin hook to add tabs in intranet biblio details page
Bug 23050: Add missing template filters
Bug 23050: Fix tab's id to avoid compatibility issues
Bug 23050: Fix creation of biblio tab's id
Bug 23050: Plugin hook to add tabs in intranet biblio details page
Bug 23050: Add missing template filters
Bug 23050: Fix tab's id to avoid compatibility issues
Bug 23050: Fix creation of biblio tab's id
Bug 23050: (QA follow-up) Add Koha::Plugins::Tab class
Bug 23050: (RM follow-up) Add Try::Tiny to controller

Description Matthias Meusburger 2019-06-05 12:30:58 UTC
Add a plugin hook that will allow plugins to add a new tab in intranet biblio details page (catalogue/detail.pl)
Comment 1 Julian Maurice 2019-06-05 14:29:14 UTC
Created attachment 90337 [details] [review]
Bug 23050: Plugin hook to add tabs in intranet biblio details page

Test Plan:
1) Enable 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 Claire Gravely 2019-06-06 08:02:59 UTC
Created attachment 90348 [details] [review]
Bug 23050: Plugin hook to add tabs in intranet biblio details page

Test Plan:
1) Enable 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: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 3 Katrin Fischer 2019-07-11 06:07:39 UTC
Hi Julian,

nice patch!

1) Some complaints form the QA script:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
   FAIL	  filters
		wrong_html_filter at line 173 (    <li><a href="#[% tab.id %]">[% tab.title %]</a></li>)
		missing_filter at line 173 (    <li><a href="#[% tab.id %]">[% tab.title %]</a></li>)
		missing_filter at line 173 (    <li><a href="#[% tab.id %]">[% tab.title %]</a></li>)

2) Wondering: What happens if the tab name consists non-latin characters, diacritics etc. Will this still work ok as id?

+            $tab->{id} = $tab->{title};
+            $tab->{id} =~ s/[^\w]+/-/g;

I've found this looking for valid characters in ids:

he value must not contain any space characters. HTML 4: ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".").

3) Can you please document the new hooks on the wiki (once we have this in PQA)?
https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks
Comment 4 Julian Maurice 2019-07-15 12:52:04 UTC
Created attachment 91523 [details] [review]
Bug 23050: Plugin hook to add tabs in intranet biblio details page

Test Plan:
1) Enable 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: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 5 Julian Maurice 2019-07-15 12:52:07 UTC
Created attachment 91524 [details] [review]
Bug 23050: Add missing template filters
Comment 6 Julian Maurice 2019-07-15 12:52:11 UTC
Created attachment 91525 [details] [review]
Bug 23050: Fix tab's id to avoid compatibility issues

MDN says:

  Using characters except ASCII letters, digits, '_', '-' and '.' may
  cause compatibility problems, as they weren't allowed in HTML 4.
  Though this restriction has been lifted in HTML5, an ID should start
  with a letter for compatibility.

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id
Comment 7 Julian Maurice 2019-07-15 12:54:51 UTC
All issues mentioned in comment 3 should be fixed.

I also modified the https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks page to include the bug number and the first Koha version that includes the hooks.
Comment 8 Tomás Cohen Arazi 2019-09-06 19:21:17 UTC
Julian, I think this is a pretty cool idea.
I don't like it that you generate content on the .pl (tab id, title, etc). It feels like all that could be done on the templates if we pass the plugins like:

Why not just:

    my @tab_plugins = Koha::Plugins->new()->GetPlugins({
        method => 'intranet_catalog_biblio_tab',
    });
     $template->param(
        plugins     => \@plugins,
        tab_plugins => \@tab_pluginss,
     );

Also, (for a later enhancement) there should be a way to define the order in which the tabs will be placed. Not sure how/where.
Comment 9 Julian Maurice 2019-09-30 08:08:26 UTC
(In reply to Tomás Cohen Arazi from comment #8)
> Julian, I think this is a pretty cool idea.
> I don't like it that you generate content on the .pl (tab id, title, etc).
> It feels like all that could be done on the templates

It could be done in templates, but, as we need the tab id in two different places, it means it would duplicate the code that creates that id, right ?
Comment 10 Julian Maurice 2019-09-30 08:11:53 UTC
Created attachment 93222 [details] [review]
Bug 23050: Fix creation of biblio tab's id

(Too much javascript in my head...)
Comment 11 Tomás Cohen Arazi 2019-11-04 13:29:59 UTC
Created attachment 95004 [details] [review]
Bug 23050: Plugin hook to add tabs in intranet biblio details page

Test Plan:
1) Enable 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: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2019-11-04 13:30:04 UTC
Created attachment 95005 [details] [review]
Bug 23050: Add missing template filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2019-11-04 13:30:08 UTC
Created attachment 95006 [details] [review]
Bug 23050: Fix tab's id to avoid compatibility issues

MDN says:

  Using characters except ASCII letters, digits, '_', '-' and '.' may
  cause compatibility problems, as they weren't allowed in HTML 4.
  Though this restriction has been lifted in HTML5, an ID should start
  with a letter for compatibility.

https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2019-11-04 13:30:13 UTC
Created attachment 95007 [details] [review]
Bug 23050: Fix creation of biblio tab's id

(Too much javascript in my head...)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2019-11-04 15:03:25 UTC
Created attachment 95013 [details] [review]
Bug 23050: (QA follow-up) Add Koha::Plugins::Tab class

This minimal class encapsulates the tabs to be passed around to the
templates, so error checking on missing bits is done in a single place.

It throws exceptions on errors

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Katrin Fischer 2019-11-04 21:38:20 UTC
Thx, Tomas!
Comment 17 Martin Renvoize 2019-11-05 08:14:22 UTC
Nice work!

Pushed to master for 19.11.00
Comment 18 Matthias Meusburger 2019-11-05 10:13:40 UTC
Tomás, didn't you forgot to "use Try::Tiny;" in catalogue/detail.pl for your last patch? (attachment 95013 [details] [review])

Without the use statement, I get the following error when trying to display content:

Can't locate object method "catch" via package "1" (perhaps you forgot to load "1"?) at /home/koha/src/catalogue/detail.pl line 89.

With the use statement, it works properly.
Comment 19 Tomás Cohen Arazi 2019-11-05 10:19:54 UTC
(In reply to Matthias Meusburger from comment #18)
> Tomás, didn't you forgot to "use Try::Tiny;" in catalogue/detail.pl for your
> last patch? (attachment 95013 [details] [review] [review])
> 
> Without the use statement, I get the following error when trying to display
> content:
> 
> Can't locate object method "catch" via package "1" (perhaps you forgot to
> load "1"?) at /home/koha/src/catalogue/detail.pl line 89.
> 
> With the use statement, it works properly.

I did
Comment 20 Martin Renvoize 2019-11-13 16:20:10 UTC
Created attachment 95406 [details] [review]
Bug 23050: (RM follow-up) Add Try::Tiny to controller

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