Bug 25961 - Add hooks for plugins to inject variables to XSLT
Summary: Add hooks for plugins to inject variables to XSLT
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL: https://github.com/bywatersolutions/k...
Keywords:
Depends on:
Blocks: 26063
  Show dependency treegraph
 
Reported: 2020-07-08 19:15 UTC by Tomás Cohen Arazi
Modified: 2020-08-05 21:14 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03


Attachments
[DO NOT PUSH] Bug 25961: Sample usage of the OPAC XSLT variables hooks (2.24 KB, patch)
2020-07-08 20:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT (3.78 KB, patch)
2020-07-08 20:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25961: Sample usage of the OPAC XSLT variables hooks (2.29 KB, patch)
2020-07-13 14:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT (3.85 KB, patch)
2020-07-13 14:09 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25961: Sample usage of the OPAC XSLT variables hooks (2.34 KB, patch)
2020-07-16 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT (3.90 KB, patch)
2020-07-16 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25961: [DO NOT PUSH] Sample usage of the OPAC XSLT variables hooks (2.35 KB, patch)
2020-07-29 08:10 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2020-07-08 19:15:04 UTC

    
Comment 1 Tomás Cohen Arazi 2020-07-08 20:20:20 UTC
Created attachment 106666 [details] [review]
[DO NOT PUSH] Bug 25961: Sample usage of the OPAC XSLT variables hooks

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2020-07-08 20:20:25 UTC
Created attachment 106667 [details] [review]
Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT

This patch adds the following plugin hooks:
- opac_results_xslt_variables
- opac_detail_xslt_variables

This hooks will inject variables returned by the plugin in the form of a
hashref, into the ones that are passed to the XSLT processing code.

To test:
1. Apply the 'DO NOT PUSH' commit
2. Install the Kitchensink plugin
3. Restart all
4. Search biblios in the OPAC
=> SUCCESS: A text is injected in front of the biblio title
5. Enter the detail page of any of the results
=> SUCCESS: A text is injected in front of the biblio title
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2020-07-08 20:31:30 UTC
Only to clarify the test plan: you need to apply both patches.
Comment 4 Kyle M Hall 2020-07-13 14:09:04 UTC
Created attachment 106832 [details] [review]
Bug 25961: Sample usage of the OPAC XSLT variables hooks

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2020-07-13 14:09:15 UTC
Created attachment 106833 [details] [review]
Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT

This patch adds the following plugin hooks:
- opac_results_xslt_variables
- opac_detail_xslt_variables

This hooks will inject variables returned by the plugin in the form of a
hashref, into the ones that are passed to the XSLT processing code.

To test:
1. Apply the 'DO NOT PUSH' commit
2. Install the Kitchensink plugin
3. Restart all
4. Search biblios in the OPAC
=> SUCCESS: A text is injected in front of the biblio title
5. Enter the detail page of any of the results
=> SUCCESS: A text is injected in front of the biblio title
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Martin Renvoize 2020-07-16 14:50:42 UTC
Created attachment 106954 [details] [review]
Bug 25961: Sample usage of the OPAC XSLT variables hooks

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2020-07-16 14:50:45 UTC
Created attachment 106955 [details] [review]
Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT

This patch adds the following plugin hooks:
- opac_results_xslt_variables
- opac_detail_xslt_variables

This hooks will inject variables returned by the plugin in the form of a
hashref, into the ones that are passed to the XSLT processing code.

To test:
1. Apply the 'DO NOT PUSH' commit
2. Install the Kitchensink plugin
3. Restart all
4. Search biblios in the OPAC
=> SUCCESS: A text is injected in front of the biblio title
5. Enter the detail page of any of the results
=> SUCCESS: A text is injected in front of the biblio title
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2020-07-16 14:51:37 UTC
Works as expected, not regressions, no qa script failures..

Passing QA.

Please make sure the new hook is well documented, both by adding it to the wiki and with an example use case somewhere perhaps.
Comment 9 Tomás Cohen Arazi 2020-07-16 15:12:45 UTC
I already documented it upon submission, but it is true that we would need a good example usage on the wiki.
Comment 10 Jonathan Druart 2020-07-23 10:33:35 UTC
(In reply to Tomás Cohen Arazi from comment #9)
> I already documented it upon submission, but it is true that we would need a
> good example usage on the wiki.

Yes, please.
I also created https://tree.taiga.io/project/koha-ils/task/75
Comment 11 Tomás Cohen Arazi 2020-07-24 13:21:26 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Tomás Cohen Arazi from comment #9)
> > I already documented it upon submission, but it is true that we would need a
> > good example usage on the wiki.
> 
> Yes, please.
> I also created https://tree.taiga.io/project/koha-ils/task/75

The fact that the KitchenSink plugin has a sample implementation, and the pointed bug supplies a sample XSLT making use of the variables, looks to me like a good source for documentation on the feature.
Comment 12 Marcel de Rooy 2020-07-24 13:35:30 UTC
More or less same code in two scripts ?
Comment 13 Tomás Cohen Arazi 2020-07-24 13:49:34 UTC
(In reply to Marcel de Rooy from comment #12)
> More or less same code in two scripts ?

I don't know what that means.
Comment 14 Kyle M Hall 2020-07-24 14:17:55 UTC
(In reply to Marcel de Rooy from comment #12)
> More or less same code in two scripts ?

This is in line with the standard convention we have at this time. If you look at Biblio.pm, Item.pm, Patron.pm, Availability.pm, SuggestionEngineMean, didyoumean.pl, process_message_queue.pl, etc you'll see the same style code.

Bug 24031 will introduce Koha::Plugins::call which we can use to simplify all these loops, but that should be done on a separate bug report to rework all those loops.
Comment 15 Jonathan Druart 2020-07-24 14:35:40 UTC
(In reply to Kyle M Hall from comment #14)
> (In reply to Marcel de Rooy from comment #12)
> > More or less same code in two scripts ?
> 
> This is in line with the standard convention we have at this time. If you
> look at Biblio.pm, Item.pm, Patron.pm, Availability.pm,
> SuggestionEngineMean, didyoumean.pl, process_message_queue.pl, etc you'll
> see the same style code.
> 
> Bug 24031 will introduce Koha::Plugins::call which we can use to simplify
> all these loops, but that should be done on a separate bug report to rework
> all those loops.

Could we imagine to have it done now, on top of all the bug reports that are PQA?
Comment 16 Tomás Cohen Arazi 2020-07-27 20:27:30 UTC
(In reply to Jonathan Druart from comment #15)
> (In reply to Kyle M Hall from comment #14)
> > (In reply to Marcel de Rooy from comment #12)
> > > More or less same code in two scripts ?
> > 
> > This is in line with the standard convention we have at this time. If you
> > look at Biblio.pm, Item.pm, Patron.pm, Availability.pm,
> > SuggestionEngineMean, didyoumean.pl, process_message_queue.pl, etc you'll
> > see the same style code.
> > 
> > Bug 24031 will introduce Koha::Plugins::call which we can use to simplify
> > all these loops, but that should be done on a separate bug report to rework
> > all those loops.
> 
> Could we imagine to have it done now, on top of all the bug reports that are
> PQA?

It is done, on bug 26063. I marked it as depending on 24031 so it might have been missed.
Comment 17 Jonathan Druart 2020-07-29 08:10:58 UTC
Created attachment 107477 [details] [review]
Bug 25961: [DO NOT PUSH] Sample usage of the OPAC XSLT variables hooks

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Jonathan Druart 2020-07-30 15:47:15 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 19 Lucas Gass 2020-07-31 21:30:58 UTC
backported to 20.05.x for 20.05.03
Comment 20 Aleisha Amohia 2020-08-04 00:34:21 UTC
enhancement, not backported to 19.11.x
Comment 21 Tomás Cohen Arazi 2020-08-04 00:57:12 UTC
(In reply to Aleisha Amohia from comment #20)
> enhancement, not backported to 19.11.x

This is a handy one :-)
Comment 22 Aleisha Amohia 2020-08-05 21:14:03 UTC
(In reply to Tomás Cohen Arazi from comment #21)
> (In reply to Aleisha Amohia from comment #20)
> > enhancement, not backported to 19.11.x
> 
> This is a handy one :-)

It may be handy but it's still very much an enhancement and not fixing any bugs. I did discuss with Hayley and Chris and they agree that enhancements like these are just worth upgrading for. :)