Bug 28010

Summary: add plugin to modify record in "opac-detail.pl"
Product: Koha Reporter: Mark Hofstetter <koha>
Component: Plugin architectureAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, kyle, m.de.rooy, martin.renvoize, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28026
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: patch to opac-detail.pl

Description Mark Hofstetter 2021-03-20 21:21:35 UTC
Created attachment 118560 [details] [review]
patch to opac-detail.pl

I (guess I) need the abiltiy to change how records are beeing display in in opac-detail.pl

Specifically I want to change the content of a record depending if a user is logged in or not for ebook renting

So it would be great to have the ability to have a plugin hook there, something like

  1 diff --git a/opac-detail.pl.orig b/opac-detail.pl
  2 index e74cf4b..b46eb90 100755
  3 --- a/opac-detail.pl.orig
  4 +++ b/opac-detail.pl
  5 @@ -217,6 +217,16 @@ if ( $xslfile ) {
  6          $variables = { %$variables, %$plugin_variables };
  7      }
  8
  9 +    $record = Koha::Plugins->call_apply(
 10 +        'munge_record',
 11 +        $record,
 12 +        {
 13 +            patron      => $patron,
 14 +            interface   => 'opac',
 15 +            caller      => 'opac-detail',
 16 +        }
 17 +    );
 18 +


additionally I have a proposal to extend/modify the plugin system a little bit:

I don't like the idea very much that the caller of a plugin has to take care of merging the results so the first step would be to add the ability that a plugin works recursively on a variable


diff --git a/Plugins.pm.orig b/Plugins.pm
index 11d44dc..71461de 100644
--- a/Plugins.pm.orig
+++ b/Plugins.pm
@@ -82,6 +82,35 @@ sub call {
     return @responses;
 }

+=head2 call_apply
+
+Calls a plugin method for all enabled plugins, reworking on the same input and returning it.
+ideally the caller of a plugin must not know how to merge the results
+so maybe also implement a "merge" method in the plugin
+
+    @responses = Koha::Plugins->call_apply($method, $work_var, @args)
+
+=cut
+
+sub call_apply {
+    my ($class, $method, $work_var, @args) = @_;
+
+    if (C4::Context->config('enable_plugins')) {
+        my @plugins = $class->new({ enable_plugins => 1 })->GetPlugins({ method => $method });
+        @plugins = grep { $_->can($method) } @plugins;
+        foreach my $plugin (@plugins) {
+            my $work_var = eval { $plugin->$method($work_var, @args) };
+            if ($@) {
+                warn sprintf("Plugin error (%s): %s", $plugin->get_metadata->{name}, $@);
+                next;
+            }
+        }
+
+    }
+    return $work_var;
+}
+
+

 =head2 GetPlugins


In another my idea would be to have an optional "merge" method for a certain type of plugins to avoid code like

 206
 207     my @plugin_responses = Koha::Plugins->call(
 208         'opac_detail_xslt_variables',
 209         {
 210             biblio_id => $biblionumber,
 211             lang      => $lang,
 212             patron_id => $borrowernumber
 213
 214         }
 215     );
 216     for my $plugin_variables ( @plugin_responses ) {
 217         $variables = { %$variables, %$plugin_variables };
 218     }
 219

and be able to write something along the lines of

 206
 207     my $variables = Koha::Plugins->call(
 208         'opac_detail_xslt_variables',
+            $variables, 
 209         {
 210             biblio_id => $biblionumber,
 211             lang      => $lang,
 212             patron_id => $borrowernumber
 213
 214         }
 215     );


I hope I haven't packed to much into one issue, thanks a lot
Comment 1 Martin Renvoize 2021-03-23 07:55:02 UTC
Hi Mark, 

Lots of interesting idea's there..

Have you seen bug 26351: In the final patch on that bug it introduces a new plugin call structure - 'call_recursive'.  This seems pretty similar to your call_apply idea.  I had a few comments on that bug am will chase Kyle for a rebase.
Comment 2 Mark Hofstetter 2021-03-23 08:59:32 UTC
I actually thought about name it recursive/iterate, the implementation (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115180) does more than mine

(it changes the input, but with the name containing "recursive" that's ok)

so I think we are settled on this
Comment 3 Katrin Fischer 2023-07-01 06:54:12 UTC
(In reply to Mark Hofstetter from comment #2)
> I actually thought about name it recursive/iterate, the implementation
> (https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=115180) does
> more than mine
> 
> (it changes the input, but with the name containing "recursive" that's ok)
> 
> so I think we are settled on this

Can this be closed with bug 26351 in Koha?