Bugzilla – Attachment 91871 Details for
Bug 23395
batch operations on biblios/items should be able to pass a list of plugins
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 23395: Add a $plugins parameter to *Biblio and *Item methods
Bug-23395-Add-a-plugins-parameter-to-Biblio-and-It.patch (text/plain), 12.12 KB, created by
Tomás Cohen Arazi
on 2019-07-30 12:53:56 UTC
(
hide
)
Description:
Bug 23395: Add a $plugins parameter to *Biblio and *Item methods
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi
Created:
2019-07-30 12:53:56 UTC
Size:
12.12 KB
patch
obsolete
>From 2c6cb9ebd4152b42d0b243ff6edaacf5f1a95c03 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >Date: Mon, 29 Jul 2019 16:35:39 -0300 >Subject: [PATCH] Bug 23395: Add a $plugins parameter to *Biblio and *Item > methods > >This patch adds a $plugin parameter (the way those methods already >expect parameters) that is expected to contain an arrayref of plugin >object instances. > >The target scenario are batch operations, in which we really don't want >to fetch the available plugins on each iteration. > >This patch doesn't make batch-related scripts use this new option. It >will be done on a follow-up patch. > >To test: >- Apply this patches >- Run: > $ kshell > k$ prove t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t >=> SUCCESS: All tests pass, new tests as well >- Sign off :-D >--- > C4/Biblio.pm | 67 ++++++++++++++++++++++++++++++++++++---------------- > C4/Items.pm | 45 +++++++++++++++++++++++++---------- > 2 files changed, 80 insertions(+), 32 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index a0af92453e..bf2361f9ef 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -183,15 +183,19 @@ bib to add, while the second argument is the desired MARC > framework code. > > This function also accepts a third, optional argument: a hashref >-to additional options. The only defined option is C<defer_marc_save>, >-which if present and mapped to a true value, causes C<AddBiblio> >-to omit the call to save the MARC in C<biblio_metadata.metadata> >-This option is provided B<only> >-for the use of scripts such as C<bulkmarcimport.pl> that may need >-to do some manipulation of the MARC record for item parsing before >-saving it and which cannot afford the performance hit of saving >-the MARC record twice. Consequently, do not use that option >-unless you can guarantee that C<ModBiblioMarc> will be called. >+to additional options. >+ >+The C<defer_marc_save> option, if present and mapped to a true value, >+causes C<AddBiblio> to omit the call to save the MARC in C<biblio_metadata.metadata> >+This option is provided B<only> for the use of scripts such as >+C<bulkmarcimport.pl> that may need to do some manipulation of the >+MARC record for item parsing before saving it and which cannot afford >+the performance hit of saving the MARC record twice. Consequently, do >+not use that option unless you can guarantee that C<ModBiblioMarc> will be called. >+ >+The C<plugins> option, expects an arrayref of plugin objects, to be >+passed to the plugin hooks to avoid unnecessary queries in the context >+of batch operations (e.g. bulkmarcimport.pl) > > =cut > >@@ -200,12 +204,17 @@ sub AddBiblio { > my $frameworkcode = shift; > my $options = @_ ? shift : undef; > my $defer_marc_save = 0; >+ my $plugins; >+ > if (!$record) { > carp('AddBiblio called with undefined record'); > return; > } >- if ( defined $options and exists $options->{'defer_marc_save'} and $options->{'defer_marc_save'} ) { >- $defer_marc_save = 1; >+ if ( defined $options ) { >+ $defer_marc_save = 1 >+ if exists $options->{defer_marc_save} and $options->{defer_marc_save}; >+ $plugins = $options->{plugins} >+ if exists $options->{plugins} and $options->{plugins}; > } > > if (C4::Context->preference('BiblioAddsAuthorities')) { >@@ -235,7 +244,7 @@ sub AddBiblio { > C4::OAI::Sets::UpdateOAISetsBiblio($biblionumber, $record); > } > >- _after_biblio_action_hooks({ action => 'create', biblio_id => $biblionumber }); >+ _after_biblio_action_hooks({ action => 'create', biblio_id => $biblionumber, plugins => $plugins }); > > logaction( "CATALOGUING", "ADD", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); > return ( $biblionumber, $biblioitemnumber ); >@@ -243,7 +252,7 @@ sub AddBiblio { > > =head2 ModBiblio > >- ModBiblio( $record,$biblionumber,$frameworkcode, $disable_autolink); >+ ModBiblio( $record,$biblionumber,$frameworkcode, $disable_autolink, $plugins); > > Replace an existing bib record identified by C<$biblionumber> > with one supplied by the MARC::Record object C<$record>. The embedded >@@ -263,12 +272,16 @@ Unless C<$disable_autolink> is passed ModBiblio will relink record headings > to authorities based on settings in the system preferences. This flag allows > us to not relink records when the authority linker is saving modifications. > >+The C<plugins> parameter, expects an arrayref of plugin objects, to be >+passed to the plugin hooks to avoid unnecessary queries in the context >+of batch operations (e.g. bulkmarcimport.pl) >+ > Returns 1 on success 0 on failure > > =cut > > sub ModBiblio { >- my ( $record, $biblionumber, $frameworkcode, $disable_autolink ) = @_; >+ my ( $record, $biblionumber, $frameworkcode, $disable_autolink, $plugins ) = @_; > if (!$record) { > carp 'No record passed to ModBiblio'; > return 0; >@@ -322,7 +335,7 @@ sub ModBiblio { > _koha_modify_biblio( $dbh, $oldbiblio, $frameworkcode ); > _koha_modify_biblioitem_nonmarc( $dbh, $oldbiblio ); > >- _after_biblio_action_hooks({ action => 'modify', biblio_id => $biblionumber }); >+ _after_biblio_action_hooks({ action => 'modify', biblio_id => $biblionumber, plugins => $plugins }); > > # update OAI-PMH sets > if(C4::Context->preference("OAI-PMH:AutoUpdateSets")) { >@@ -366,10 +379,14 @@ Checks to make sure that the biblio has no items attached. > return: > C<$error> : undef unless an error occurs > >+The C<plugins> option, expects an arrayref of plugin objects, to be >+passed to the plugin hooks to avoid unnecessary queries in the context >+of batch operations. >+ > =cut > > sub DelBiblio { >- my ($biblionumber) = @_; >+ my ($biblionumber, $plugins) = @_; > > my $biblio = Koha::Biblios->find( $biblionumber ); > return unless $biblio; # Should we throw an exception instead? >@@ -424,7 +441,7 @@ sub DelBiblio { > # from being generated by _koha_delete_biblioitems > $error = _koha_delete_biblio( $dbh, $biblionumber ); > >- _after_biblio_action_hooks({ action => 'delete', biblio_id => $biblionumber }); >+ _after_biblio_action_hooks({ action => 'delete', biblio_id => $biblionumber, plugins => $plugins }); > > logaction( "CATALOGUING", "DELETE", $biblionumber, "biblio" ) if C4::Context->preference("CataloguingLog"); > >@@ -3460,12 +3477,22 @@ sub _after_biblio_action_hooks { > > my $biblio_id = $args->{biblio_id}; > my $action = $args->{action}; >+ my $plugins = $args->{plugins}; > > if ( C4::Context->preference('UseKohaPlugins') && C4::Context->config("enable_plugins") ) { > >- my @plugins = Koha::Plugins->new->GetPlugins({ >- method => 'after_biblio_action', >- }); >+ my @plugins; >+ >+ if ( $plugins and >+ ref($plugins) eq 'ARRAY' ) { >+ @plugins = grep { $_->can('after_biblio_action') } @{ $plugins }; >+ } >+ else { >+ # We haven't been passed a plugins list, get it ourselves >+ @plugins = Koha::Plugins->new->GetPlugins({ >+ method => 'after_biblio_action', >+ }); >+ } > > if (@plugins) { > >diff --git a/C4/Items.pm b/C4/Items.pm >index 797bda6244..b5e7da94c1 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -168,8 +168,8 @@ sub AddItemFromMarc { > > =head2 AddItem > >- my ($biblionumber, $biblioitemnumber, $itemnumber) >- = AddItem($item, $biblionumber[, $dbh, $frameworkcode, $unlinked_item_subfields]); >+ my ( $biblionumber, $biblioitemnumber, $itemnumber ) >+ = AddItem( $item, $biblionumber[ , $dbh, $frameworkcode, $unlinked_item_subfields, $plugins ] ); > > Given a hash containing item column names as keys, > create a new Koha item record. >@@ -178,13 +178,17 @@ The first two optional parameters (C<$dbh> and C<$frameworkcode>) > do not need to be supplied for general use; they exist > simply to allow them to be picked up from AddItemFromMarc. > >-The final optional parameter, C<$unlinked_item_subfields>, contains >+The optional C<$unlinked_item_subfields> parameter contains > an arrayref containing subfields present in the original MARC > representation of the item (e.g., from the item editor) that are > not mapped to C<items> columns directly but should instead > be stored in C<items.more_subfields_xml> and included in > the biblio items tag for display and indexing. > >+The optional C<$plugins> parameter contains an arrayref containing >+a list of plugin objects. If it is not passed they will get queried >+on the plugin hooks. >+ > =cut > > sub AddItem { >@@ -197,6 +201,7 @@ sub AddItem { > if (@_) { > $unlinked_item_subfields = shift; > } >+ my $plugins = shift; > > # needs old biblionumber and biblioitemnumber > $item->{'biblionumber'} = $biblionumber; >@@ -225,7 +230,7 @@ sub AddItem { > logaction( "CATALOGUING", "ADD", $itemnumber, "item" ) > if C4::Context->preference("CataloguingLog"); > >- _after_item_action_hooks({ action => 'create', item_id => $itemnumber }); >+ _after_item_action_hooks({ action => 'create', item_id => $itemnumber, plugins => $plugins }); > > return ( $item->{biblionumber}, $item->{biblioitemnumber}, $itemnumber ); > } >@@ -443,8 +448,9 @@ ModItem( > $biblionumber, > $itemnumber, > { >- [ unlinked_item_subfields => $unlinked_item_subfields, ] >- [ log_action => 1, ] >+ [ unlinked_item_subfields => $unlinked_item_subfields, ], >+ [ log_action => 1, ], >+ [ plugins => [ $plugin_1, ... ] ] > } > ); > >@@ -471,12 +477,16 @@ and set the value. > If log_action is set to false, the action will not be logged. > If log_action is true or undefined, the action will be logged. > >+If plugins is passed an arrayref, then it is passed to the plugin >+hooks so no plugin query needs to take place. >+ > =cut > > sub ModItem { > my ( $item, $biblionumber, $itemnumber, $additional_params ) = @_; > my $log_action = $additional_params->{log_action} // 1; > my $unlinked_item_subfields = $additional_params->{unlinked_item_subfields}; >+ my $plugins = $additional_params->{plugins}; > > return unless %$item; > $item->{'itemnumber'} = $itemnumber or return; >@@ -531,7 +541,7 @@ sub ModItem { > # item status is possible > ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); > >- _after_item_action_hooks({ action => 'modify', item_id => $itemnumber }); >+ _after_item_action_hooks({ action => 'modify', item_id => $itemnumber, plugins => $plugins }); > > logaction( "CATALOGUING", "MODIFY", $itemnumber, "item " . Dumper($item) ) > if $log_action && C4::Context->preference("CataloguingLog"); >@@ -592,7 +602,7 @@ sub ModDateLastSeen { > > =head2 DelItem > >- DelItem({ itemnumber => $itemnumber, [ biblionumber => $biblionumber ] } ); >+ DelItem({ itemnumber => $itemnumber, [ biblionumber => $biblionumber, plugins => [ $plugin_1, ... ] ] } ); > > Exported function (core API) for deleting an item record in Koha. > >@@ -603,6 +613,7 @@ sub DelItem { > > my $itemnumber = $params->{itemnumber}; > my $biblionumber = $params->{biblionumber}; >+ my $plugins = $params->{plugins}; > > unless ($biblionumber) { > my $item = Koha::Items->find( $itemnumber ); >@@ -617,7 +628,7 @@ sub DelItem { > > ModZebra( $biblionumber, "specialUpdate", "biblioserver" ); > >- _after_item_action_hooks({ action => 'delete', item_id => $itemnumber }); >+ _after_item_action_hooks({ action => 'delete', item_id => $itemnumber, plugins => $plugins }); > > #search item field code > logaction("CATALOGUING", "DELETE", $itemnumber, "item") if C4::Context->preference("CataloguingLog"); >@@ -2653,12 +2664,22 @@ sub _after_item_action_hooks { > > my $item_id = $args->{item_id}; > my $action = $args->{action}; >+ my $plugins = $args->{plugins}; > > if ( C4::Context->preference('UseKohaPlugins') && C4::Context->config("enable_plugins") ) { > >- my @plugins = Koha::Plugins->new->GetPlugins({ >- method => 'after_item_action', >- }); >+ my @plugins; >+ >+ if ( $plugins and >+ ref($plugins) eq 'ARRAY' ) { >+ @plugins = grep { $_->can('after_item_action') } @{ $plugins }; >+ } >+ else { >+ # We haven't been passed a plugins list, get it ourselves >+ @plugins = Koha::Plugins->new->GetPlugins({ >+ method => 'after_item_action', >+ }); >+ } > > if (@plugins) { > >-- >2.22.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 23395
:
91870
| 91871 |
91872
|
91874