From 695957a2c62c8fb448aa94037878e103db43661d Mon Sep 17 00:00:00 2001 From: Ere Maijala Date: Thu, 25 Apr 2019 11:31:02 +0300 Subject: [PATCH] Bug 20447: Use blessed objects and fix issues found during review --- C4/Biblio.pm | 1 - C4/Items.pm | 2 +- C4/Search.pm | 2 +- Koha/Holding.pm | 39 +- Koha/Template/Plugin/Holdings.pm | 42 +- catalogue/detail.pl | 2 +- .../prog/en/includes/cat-toolbar.inc | 2 +- .../prog/en/modules/catalogue/detail.tt | 10 +- .../prog/en/modules/cataloguing/addholding.tt | 760 +++++++++--------- .../marc21_field_008_holdings.tt | 39 +- .../value_builder/marc21_leader_holdings.tt | 32 +- opac/opac-detail.pl | 2 +- t/db_dependent/Koha/Holding.t | 8 +- t/db_dependent/Koha/Holdings.t | 2 - 14 files changed, 480 insertions(+), 463 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index baa4d6c2d5..c0939df774 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -2831,7 +2831,6 @@ sub EmbedItemsInMarcBiblio { $itemnumbers = [] unless defined $itemnumbers; if ( !$skip_holdings && C4::Context->preference('SummaryHoldings') && !@$itemnumbers ) { - require Koha::Holdings; my $holdings_fields = Koha::Holdings->get_embeddable_marc_fields({ biblionumber => $biblionumber }); $marc->append_fields(@$holdings_fields) if ( @$holdings_fields ); } diff --git a/C4/Items.pm b/C4/Items.pm index 141dbd873e..a42b882194 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -1694,7 +1694,7 @@ sub MoveItemFromBiblio { |, undef, $tobiblioitem, $tobiblio, $itemnumber, $frombiblio ); if ($return == 1) { # Check if the moved item is attached to a holdings record - my $item = my $item = Koha::Items->find($itemnumber); + my $item = Koha::Items->find($itemnumber); if ($item->holding_id()) { my $oldHolding = Koha::Holdings->find($item->holding_id()); if ($oldHolding) { diff --git a/C4/Search.pm b/C4/Search.pm index bbffc055dc..0446e6475d 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -2259,7 +2259,7 @@ sub searchResults { # Fetch summary holdings if (C4::Context->preference('SummaryHoldings')) { - $summary_holdings = Koha::Holdings->search({ biblionumber => $oldbiblio->{biblionumber}, deleted_on => undef })->unblessed; + $summary_holdings = Koha::Holdings->search({ biblionumber => $oldbiblio->{biblionumber}, deleted_on => undef }); } # XSLT processing of some stuff diff --git a/Koha/Holding.pm b/Koha/Holding.pm index c0053610c4..a58c39f8fc 100644 --- a/Koha/Holding.pm +++ b/Koha/Holding.pm @@ -28,6 +28,7 @@ use C4::Log; use Koha::Database; use Koha::DateUtils qw(dt_from_string); use Koha::Holdings::Metadatas; +use Koha::Items; use base qw(Koha::Object); @@ -41,6 +42,21 @@ Koha::Holding - Koha Holding Object class =cut +=head3 holding_branch + +my $branch = $hold->holding_branch(); + +Returns the holding branch for this record. + +=cut + +sub holding_branch { + my ($self) = @_; + + my $branch = $self->_result->holdingbranch(); + return Koha::Library->_new_from_dbic($branch); +} + =head3 metadata my $metadata = $holding->metadata(); @@ -52,9 +68,9 @@ Returns a Koha::Holding::Metadata object sub metadata { my ($self) = @_; - $self->{_metadata} ||= Koha::Holdings::Metadatas->find({ holding_id => $self->id }); - - return $self->{_metadata}; + my $metadata = $self->_result()->metadata(); + return unless $metadata; + return Koha::Holdings::Metadata->_new_from_dbic($metadata); } =head3 set_marc @@ -92,7 +108,15 @@ sub set_marc { $self->{_marcxml} = $record->as_xml_record($encoding); my $fields = $self->marc_to_koha_fields({ record => $record }); delete $fields->{holding_id}; - $self->set($fields); + # Filter the columns since we have e.g. public_note that's not stored in the database + my $columns = [$self->_result()->result_source()->columns()]; + my $db_fields = {}; + foreach my $key (keys %{$fields}) { + if (grep {/^$key$/} @{$columns}) { + $db_fields->{$key} = $fields->{$key}; + } + } + $self->set($db_fields); return $self; } @@ -110,9 +134,12 @@ or list of Koha::Item objects in list context. sub items { my ($self) = @_; - $self->{_items} ||= Koha::Items->search( { holding_id => $self->holding_id() } ); + my $items_rs = $self->_result->items; - return wantarray ? $self->{_items}->as_list : $self->{_items}; + return + wantarray + ? Koha::Items->_new_from_dbic($items_rs)->as_list + : Koha::Items->_new_from_dbic($items_rs); } =head3 store diff --git a/Koha/Template/Plugin/Holdings.pm b/Koha/Template/Plugin/Holdings.pm index 6370630eb1..d4948b2f36 100644 --- a/Koha/Template/Plugin/Holdings.pm +++ b/Koha/Template/Plugin/Holdings.pm @@ -56,8 +56,8 @@ sub GetLocation { return ''; } - if (ref($holding) ne 'HASH') { - $holding = Koha::Holdings->find($holding)->unblessed; + if (ref($holding) ne 'Koha::Holding') { + $holding = Koha::Holdings->find($holding); if (!$holding) { return ''; } @@ -66,27 +66,22 @@ sub GetLocation { my @parts; if ($opac) { - if ($holding->{'holdingbranch'}) { - my $query = "SELECT branchname FROM branches WHERE branchcode = ?"; - my $sth = C4::Context->dbh->prepare($query); - $sth->execute($holding->{'holdingbranch'}); - my $b = $sth->fetchrow_hashref(); - push @parts, $b->{'branchname'} if $b; - $sth->finish(); + if (my $branch = $holding->holding_branch()) { + push @parts, $branch->branchname(); } - if ($holding->{'location'}) { - my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $holding->{'location'} }); - push @parts, $av->next->opac_description if $av->count; + if ($holding->location()) { + my $av = Koha::AuthorisedValues->search({ category => 'LOC', authorised_value => $holding->location() }); + push @parts, $av->next()->opac_description() if $av->count; } - push @parts, $holding->{'callnumber'} if $holding->{'callnumber'}; + push @parts, $holding->callnumber() if $holding->callnumber(); return join(' - ', @parts); } - push @parts, $holding->{'holding_id'}; - push @parts, $holding->{'holdingbranch'} if $holding->{'holdingbranch'}; - push @parts, $holding->{'location'} if $holding->{'location'}; - push @parts, $holding->{'ccode'} if $holding->{'ccode'}; - push @parts, $holding->{'callnumber'} if $holding->{'callnumber'}; + push @parts, $holding->holding_id(); + push @parts, $holding->holdingbranch() if $holding->holdingbranch(); + push @parts, $holding->location() if $holding->location(); + push @parts, $holding->ccode() if $holding->ccode(); + push @parts, $holding->callnumber() if $holding->callnumber(); return join(' ', @parts); } @@ -106,13 +101,16 @@ sub GetDetails { return ''; } - if (ref($holding) eq 'HASH') { - $holding = $holding->{'holding_id'}; + if (ref($holding) ne 'Koha::Holding') { + $holding = Koha::Holdings->find($holding); + if (!$holding) { + return ''; + } } - my $holding_marc = Koha::Holdings->find($holding)->metadata()->record(); + my $holding_marc = $holding->metadata()->record(); - return Koha::Holding::marc_to_koha_fields({ record => $holding_marc }); + return Koha::Holding->marc_to_koha_fields({ record => $holding_marc }); } 1; diff --git a/catalogue/detail.pl b/catalogue/detail.pl index f5485160a0..cd7b6c90ef 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -195,7 +195,7 @@ foreach my $subscription (@subscriptions) { # Summary holdings my $summary_holdings; if (C4::Context->preference('SummaryHoldings')) { - $summary_holdings = Koha::Holdings->search({ biblionumber => $biblionumber, deleted_on => undef })->unblessed; + $summary_holdings = Koha::Holdings->search({ biblionumber => $biblionumber, deleted_on => undef }); } # Get acquisition details diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc index 0415d4ba7f..7e5adc308d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/cat-toolbar.inc @@ -11,7 +11,7 @@ CAN_user_serials_create_subscription ) %] [% END %] [% IF ( show_summary_holdings && CAN_user_editcatalogue_edit_items ) %] -
  • New holdings record
    • +
  • New holdings record
    • [% END %] [% IF ( CAN_user_editcatalogue_edit_items ) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt index 4e55a694d0..39841fb7b7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt @@ -145,7 +145,7 @@