@@ -, +, @@ --- C4/Biblio.pm | 224 +++++++++++++++++++--------- C4/Items.pm | 68 ++++++--- Koha/BiblioUtils.pm | 80 +++++++--- Koha/Items.pm | 66 ++++++++ misc/search_tools/rebuild_elastic_search.pl | 2 +- t/db_dependent/Items.t | 84 ++++++++++- 6 files changed, 404 insertions(+), 120 deletions(-) --- a/C4/Biblio.pm +++ a/C4/Biblio.pm @@ -1108,6 +1108,26 @@ sub GetMarcSubfieldStructureFromKohaField { return wantarray ? @{$mss->{$kohafield}} : $mss->{$kohafield}->[0]; } + +sub GetMarcBiblio { + my ($params) = @_; + + if (not defined $params) { + carp 'GetMarcBiblio called without parameters'; + return; + } + my $biblionumber = $params->{biblionumber}; + + if (not defined $biblionumber) { + carp 'GetMarcBiblio called with undefined biblionumber'; + return; + } + delete $params->{biblionumber}; + $params->{biblionumbers} = [$biblionumber]; + my ($biblio) = GetMarcBiblios($params); + return $biblio; +} + =head2 GetMarcBiblio my $record = GetMarcBiblio({ @@ -1148,57 +1168,68 @@ It only makes sense in combination both embed_items and opac values true. =cut -sub GetMarcBiblio { +sub GetMarcBiblios { my ($params) = @_; if (not defined $params) { - carp 'GetMarcBiblio called without parameters'; + carp 'GetMarcBiblios called without parameters'; return; } - my $biblionumber = $params->{biblionumber}; + my $biblionumbers = $params->{biblionumbers}; my $embeditems = $params->{embed_items} || 0; my $opac = $params->{opac} || 0; my $borcat = $params->{borcat} // q{}; - if (not defined $biblionumber) { - carp 'GetMarcBiblio called with undefined biblionumber'; + ## START HERE ## + + if (not defined $biblionumbers) { + carp 'GetMarcBiblio called with undefined biblionumbers'; return; } - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber=? "); - $sth->execute($biblionumber); - my $row = $sth->fetchrow_hashref; - my $biblioitemnumber = $row->{'biblioitemnumber'}; - my $marcxml = GetXmlBiblio( $biblionumber ); - $marcxml = StripNonXmlChars( $marcxml ); - my $frameworkcode = GetFrameworkCode($biblionumber); - MARC::File::XML->default_record_format( C4::Context->preference('marcflavour') ); - my $record = MARC::Record->new(); + my $in_placeholders = join ', ', (('?') x @{$biblionumbers}); + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT biblionumber, biblioitemnumber FROM biblioitems WHERE biblionumber IN ($in_placeholders)"); + $sth->execute(@{$biblionumbers}); + + my %marc_records; + my ($biblionumber, $biblioitemnumber); + my $marc_flavour = C4::Context->preference('marcflavour'); + while (my $biblio_keys = $sth->fetchrow_arrayref) { + ($biblionumber, $biblioitemnumber) = @{$biblio_keys}; + + my $marcxml = GetXmlBiblio( $biblionumber ); + $marcxml = StripNonXmlChars( $marcxml ); + my $frameworkcode = GetFrameworkCode($biblionumber); + MARC::File::XML->default_record_format( $marc_flavour ); + my $record = MARC::Record->new(); + + if ($marcxml) { + $record = eval { + MARC::Record::new_from_xml( $marcxml, "utf8", $marc_flavour); + }; + if ($@) { warn " problem with :$biblionumber : $@ \n$marcxml"; } + next unless $record; + + C4::Biblio::_koha_marc_update_bib_ids( + $record, + $frameworkcode, + $biblionumber, + $biblioitemnumber + ); + $marc_records{$biblionumber} = $record; + } + } - if ($marcxml) { - $record = eval { - MARC::Record::new_from_xml( $marcxml, "utf8", - C4::Context->preference('marcflavour') ); - }; - if ($@) { warn " problem with :$biblionumber : $@ \n$marcxml"; } - return unless $record; - - C4::Biblio::_koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, - $biblioitemnumber ); - C4::Biblio::EmbedItemsInMarcBiblio({ - marc_record => $record, - biblionumber => $biblionumber, + if ($embeditems) { + C4::Biblio::EmbedItemsInMarcBiblios({ + marc_records => \%marc_records, opac => $opac, - borcat => $borcat }) - if ($embeditems); - - return $record; - } - else { - return; + borcat => $borcat + }); } + return wantarray ? values %marc_records : \%marc_records; } =head2 GetXmlBiblio @@ -2790,57 +2821,104 @@ override if necessary. sub EmbedItemsInMarcBiblio { my ($params) = @_; - my ($marc, $biblionumber, $itemnumbers, $opac, $borcat); - $marc = $params->{marc_record}; - if ( !$marc ) { + + if ( !$params->{marc_record} ) { carp 'EmbedItemsInMarcBiblio: No MARC record passed'; return; } - $biblionumber = $params->{biblionumber}; + if ( !$params->{biblionumber} ) { + carp 'EmbedItemsInMarcBiblio: No biblionumber passed'; + return; + } + + $params->{marc_records} = { $params->{biblionumber} => $params->{marc_record} }; + delete $params->{marc_record}; + delete $params->{biblionumber}; + + my ($marc_with_items) = EmbedItemsInMarcBiblios($params); + return $marc_with_items; +} + +sub EmbedItemsInMarcBiblios { + my ($params) = @_; + my ($marc_records, $biblionumber, $itemnumbers, $opac, $borcat); + $marc_records = $params->{marc_records}; + $itemnumbers = $params->{item_numbers}; $opac = $params->{opac}; $borcat = $params->{borcat} // q{}; $itemnumbers = [] unless defined $itemnumbers; - my $frameworkcode = GetFrameworkCode($biblionumber); - _strip_item_fields($marc, $frameworkcode); - # ... and embed the current items my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber = ?"); - $sth->execute($biblionumber); - my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField( "items.itemnumber", $frameworkcode ); - my @item_fields; # Array holding the actual MARC data for items to be included. - my @items; # Array holding items which are both in the list (sitenumbers) - # and on this biblionumber + my @biblionumbers = keys %{$marc_records}; + my $in_placeholders = join ', ', (('?') x @biblionumbers); - # Flag indicating if there is potential hiding. - my $opachiddenitems = $opac - && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); + my $biblio_itemnumbers = $dbh->selectcol_arrayref( + "SELECT itemnumber FROM items WHERE biblionumber IN ($in_placeholders)", + undef, + @biblionumbers + ); + + # Remove invalid item numbers by intersecting with all valid item numbers + # if $itemnumbers argument was provided + # TODO: MAKE SURE THERE IS A TEST FOR THIS + if(@{$itemnumbers}) { + my %h = map { $_ => undef } @{$itemnumbers}; + $itemnumbers = [grep { exists $h{$_} } @{$biblio_itemnumbers}]; + } + else { + $itemnumbers = $biblio_itemnumbers; + } + # If no item numbers then no point in continuing + return unless (@{$itemnumbers}); + + + my $opachiddenitems = $opac + && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); require C4::Items; - while ( my ($itemnumber) = $sth->fetchrow_array ) { - next if @$itemnumbers and not grep { $_ == $itemnumber } @$itemnumbers; - my $i = $opachiddenitems ? C4::Items::GetItem($itemnumber) : undef; - push @items, { itemnumber => $itemnumber, item => $i }; - } - my @items2pass = map { $_->{item} } @items; - my @hiddenitems = - $opachiddenitems - ? C4::Items::GetHiddenItemnumbers({ - items => \@items2pass, - borcat => $borcat }) - : (); - # Convert to a hash for quick searching - my %hiddenitems = map { $_ => 1 } @hiddenitems; - foreach my $itemnumber ( map { $_->{itemnumber} } @items ) { - next if $hiddenitems{$itemnumber}; - my $item_marc = C4::Items::GetMarcItem( $biblionumber, $itemnumber ); - push @item_fields, $item_marc->field($itemtag); - } - $marc->append_fields(@item_fields); + + my $items = C4::Items::GetItems($itemnumbers); + # We have now fetched all items, time to partion by biblionumber + my %items_by_biblionumber = (map { ($_, []) } @biblionumbers); + foreach my $item (@{$items}) { + push @{$items_by_biblionumber{$item->{biblionumber}}}, $item; + } + + foreach my $biblionumber (@biblionumbers) { + my $marc_record = $marc_records->{$biblionumber}; + + # Array holding items which are both in itemnumbers + # (if provided) and in biblionumbers itemnumbers + my $items = $items_by_biblionumber{$biblionumber}; + + # Could this be moved lower down and run only when items + # exists for improved performance? Probably.. + my $frameworkcode = GetFrameworkCode($biblionumber); + _strip_item_fields($marc_record, $frameworkcode); + + # Flag indicating if there is potential hiding + if ($opachiddenitems) { + my %hidden_items = map { $_ => undef } C4::Items::GetHiddenItemnumbers({ + items => @{$items}, + borcat => $borcat + }); + # Reduce items to non hidden items + $items = [grep { !(exists $hidden_items{$_->{itemnumber}}) } @{$items}]; + } + + my ($itemtag) = GetMarcFromKohaField("items.itemnumber", $frameworkcode); + my @item_fields; # Array holding the actual MARC data for items to be included. + foreach my $item (@{$items}) { + my $item_marc = C4::Items::GetMarcItem($biblionumber, $item); + push @item_fields, $item_marc->field($itemtag); + } + $marc_record->append_fields(@item_fields); + } + return $marc_records; } =head1 INTERNAL FUNCTIONS @@ -3292,7 +3370,7 @@ sub ModBiblioMarc { # deal with UNIMARC field 100 (encoding) : create it if needed & set encoding to unicode if ( $encoding eq "UNIMARC" ) { - my $defaultlanguage = C4::Context->preference("UNIMARCField100Language"); + my $defaultlanguage = C4::Context->preference("UNIMARCField100Language"); $defaultlanguage = "fre" if (!$defaultlanguage || length($defaultlanguage) != 3); my $string = $record->subfield( 100, "a" ); if ( ($string) && ( length( $record->subfield( 100, "a" ) ) == 36 ) ) { @@ -3302,7 +3380,7 @@ sub ModBiblioMarc { $string = POSIX::strftime( "%Y%m%d", localtime ); $string =~ s/\-//g; $string = sprintf( "%-*s", 35, $string ); - substr ( $string, 22, 3, $defaultlanguage); + substr ( $string, 22, 3, $defaultlanguage); } substr( $string, 25, 3, "y50" ); unless ( $record->subfield( 100, "a" ) ) { --- a/C4/Items.pm +++ a/C4/Items.pm @@ -129,34 +129,60 @@ of C Return item information, for a given itemnumber or barcode. The return value is a hashref mapping item column -names to values. If C<$serial> is true, include serial publication data. +names to values, or undef if item was not found. If C<$serial> is true, include serial publication data. =cut sub GetItem { - my ($itemnumber,$barcode, $serial) = @_; - my $dbh = C4::Context->dbh; + my ($itemnumber, $barcode, $serial) = @_; + my $items = GetItems($itemnumber ? [$itemnumber] : undef, $barcode ? [$barcode] : undef, $serial); + return @{$items} ? $items->[0] : undef; +} - my $item; - if ($itemnumber) { - $item = Koha::Items->find( $itemnumber ); - } else { - $item = Koha::Items->find( { barcode => $barcode } ); - } +=head2 GetItems - return unless ( $item ); + $item = GetItems($itemnumber,$barcode,$serial); - my $data = $item->unblessed(); - $data->{itype} = $item->effective_itemtype(); # set the correct itype +Return list of item information, for given itemnumbers or barcodes. +The return value is an arrayref with hashrefs mapping item column +names to values, or an empty arrayref if no items were found. +If C<$serial> is true, include serial publication data. +=cut + +sub GetItems { + my ($itemnumbers, $barcodes, $serial) = @_; + my $dbh = C4::Context->dbh; + my $items_data = []; + + # Important: If 'item-level_itypes' preference is not set Koha::Item::effective_itemtype() + # will be equal to $item->{itype} and it is possible to fetch the data directly from items table. + # This will have a much smaller footprint then unblessing the item objects fetched through + # find/search. If more calculated attributes are added in the future, this code will need + # to account for that. + if (C4::Context->preference('item-level_itypes')) { + $items_data = Koha::Items->search_unblessed($itemnumbers ? $itemnumbers : { barcode => $barcodes }); + } + else { + my @items = Koha::Items->search( + $itemnumbers ? { itemnumber => { IN => $itemnumbers } } : { barcode => { IN => $barcodes } } + ); + foreach my $item (@items) { + my $data = $item->unblessed(); + # Set the correct itype + $data->{itype} = $item->effective_itemtype(); + push @{$items_data}, $data; + } + } if ($serial) { - my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=?"); - $ssth->execute( $data->{'itemnumber'} ); - ( $data->{'serialseq'}, $data->{'publisheddate'} ) = $ssth->fetchrow_array(); + foreach my $data (@{$items_data}) { + my $ssth = $dbh->prepare("SELECT serialseq,publisheddate from serialitems left join serial on serialitems.serialid=serial.serialid where serialitems.itemnumber=?"); + $ssth->execute( $data->{'itemnumber'} ); + ( $data->{'serialseq'}, $data->{'publisheddate'} ) = $ssth->fetchrow_array(); + } } - - return $data; -} # sub GetItem + return $items_data; +} =head2 CartToShelf @@ -1331,13 +1357,13 @@ sub GetMarcItem { # while the other treats the MARC representation as authoritative # under certain circumstances. - my $itemrecord = GetItem($itemnumber); + my $itemrecord = ref($itemnumber) ? $itemnumber : GetItem($itemnumber); # Tack on 'items.' prefix to column names so that C4::Biblio::TransformKohaToMarc will work. # Also, don't emit a subfield if the underlying field is blank. - - return Item2Marc($itemrecord,$biblionumber); + + return Item2Marc($itemrecord, $biblionumber); } sub Item2Marc { --- a/Koha/BiblioUtils.pm +++ a/Koha/BiblioUtils.pm @@ -111,29 +111,69 @@ The iterator is a Koha::MetadataIterator object. =cut sub get_all_biblios_iterator { + my ($self, $params) = @_; + my $batched; + my $batch_size; + if ($params) { + $batched //= $params->{batched}; + $batch_size = $params->{batch_size} || 2000; + } + my $database = Koha::Database->new(); my $schema = $database->schema(); - my $rs = - $schema->resultset('Biblio')->search( {}, - { columns => [qw/ biblionumber /] } ); - my $next_func = sub { - # Warn and skip bad records, otherwise we break the loop - while (1) { - my $row = $rs->next(); - return if !$row; - my $marc = C4::Biblio::GetMarcBiblio({ - biblionumber => $row->biblionumber, - embed_items => 1 }); - my $next = eval { - __PACKAGE__->new($marc, $row->biblionumber); - }; - if ($@) { - warn "Something went wrong reading record for biblio $row->biblionumber: $@\n"; - next; + my $rs = $schema->resultset('Biblio')->search( + {}, + { columns => [qw/ biblionumber /] } + ); + + my $next_func; + if ($batched) { + my @marc_records_batch; + $next_func = sub { + unless (@marc_records_batch) { + #Batch empty, nead to buffer up more records + my $limit = $batch_size; + my @biblionumbers; + while ($limit--) { + my $row = $rs->next(); + last if !$row; + push @biblionumbers, $row->biblionumber; + } + if (@biblionumbers) { + my $marc_records = C4::Biblio::GetMarcBiblios({ + biblionumbers => \@biblionumbers, + embed_items => 1 + }); + while (my ($biblionumber, $marc_record) = each %{$marc_records}) { + my $next = __PACKAGE__->new($marc_record, $biblionumber); + push @marc_records_batch, $next; + } + } } - return $next; - } - }; + return pop @marc_records_batch; + }; + } + else { + $next_func = sub { + # Warn and skip bad records, otherwise we break the loop + while (1) { + my $row = $rs->next(); + return if !$row; + my $marc = C4::Biblio::GetMarcBiblio({ + biblionumber => $row->biblionumber, + embed_items => 1 + }); + my $next = eval { + __PACKAGE__->new($marc, $row->biblionumber); + }; + if ($@) { + warn "Something went wrong reading record for biblio $row->biblionumber: $@\n"; + next; + } + return $next; + } + }; + } return Koha::MetadataIterator->new($next_func); } --- a/Koha/Items.pm +++ a/Koha/Items.pm @@ -53,6 +53,72 @@ sub object_class { return 'Koha::Item'; } +=head2 search_unblessed + + $items = Koha::Items->search_unblessed($conditions); + +In scalar context, returns an array reference with hashes containing item data, in list context returns an array. +Calling search_unblessed should produce the same result as calling Koha::Items->search and iterating +though the result unblessing each item. In cases where you only need the item data using this optimized +method is much faster. The arguments accepted are a subset and approximation of those supported by +Koha::items->search, with only the "barcode" and "itemnumber" attributes supported as conditions. +If called with a non-hash conditions argument a search will be performed as if "itemnumber" was the +provided condition. Only one condition is accepted and if more are provided the method will die with +an error. + +=cut + +sub search_unblessed { + my ($self, $conditions) = @_; + my $items = []; + my $field_name; + if (ref($conditions) eq 'HASH') { + my %valid_condition_fields = ( + 'barcode' => undef, + 'itemnumber' => undef, + ); + my @fields = keys %{$conditions}; + if (@fields == 1) { + foreach my $field (@fields) { + die("Invalid condition: \"$field\"") unless exists $valid_condition_fields{$field}; + } + } + elsif(@fields > 1) { + die("Multiple conditions are not supported"); + } + else { + die("No conditions provided"); + } + } + else { + $conditions = { + 'itemnumber' => $conditions, + }; + } + # Only accepts one condition + my ($field, $values) = (%{$conditions}); + + if (ref($values) eq 'ARRAY' && @{$values} == 1) { + ($values) = @{$values}; + } + if (!ref($values)) { + my $item = C4::Context->dbh->selectrow_hashref(qq{SELECT * FROM items WHERE $field = ?}, undef, $values); + push @{$items}, $item; + } + elsif (ref($values) eq 'ARRAY') { + my $in_placeholders = join ', ', (('?') x @{$values}); + my $sth = C4::Context->dbh->prepare(qq{SELECT * FROM items WHERE $field IN($in_placeholders)}); + $sth->execute(@{$values}); + while (my $item = $sth->fetchrow_hashref) { + push @{$items}, $item; + } + } + else { + die("Invalid value for field: \"$field\""); + } + return wantarray ? @{$items} : $items; +} + =head1 AUTHOR Kyle M Hall --- a/misc/search_tools/rebuild_elastic_search.pl +++ a/misc/search_tools/rebuild_elastic_search.pl @@ -131,7 +131,7 @@ if ($index_biblios) { return ($r, Koha::BiblioUtils->get_from_biblionumber($r, item_data => 1 )); }; } else { - my $records = Koha::BiblioUtils->get_all_biblios_iterator(); + my $records = Koha::BiblioUtils->get_all_biblios_iterator({ batched => 1 }); $next = sub { $records->next(); } --- a/t/db_dependent/Items.t +++ a/t/db_dependent/Items.t @@ -42,7 +42,9 @@ my $location = 'My Location'; subtest 'General Add, Get and Del tests' => sub { - plan tests => 16; + plan tests => 25; + + my $item_barcode = 123; $schema->storage->txn_begin; @@ -59,13 +61,21 @@ subtest 'General Add, Get and Del tests' => sub { my ($bibnum, $bibitemnum) = get_biblio(); # Add an item. - my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, itype => $itemtype->{itemtype} } , $bibnum); + my ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + location => $location, + itype => $itemtype->{itemtype}, + barcode => $item_barcode + }, $bibnum + ); cmp_ok($item_bibnum, '==', $bibnum, "New item is linked to correct biblionumber."); cmp_ok($item_bibitemnum, '==', $bibitemnum, "New item is linked to correct biblioitemnumber."); # Get item. my $getitem = GetItem($itemnumber); cmp_ok($getitem->{'itemnumber'}, '==', $itemnumber, "Retrieved item has correct itemnumber."); + cmp_ok($getitem->{'barcode'}, '==', $item_barcode, "Retrieved item has correct barcode."); cmp_ok($getitem->{'biblioitemnumber'}, '==', $item_bibitemnum, "Retrieved item has correct biblioitemnumber."); is( $getitem->{location}, $location, "The location should not have been modified" ); is( $getitem->{permanent_location}, $location, "The permanent_location should have been set to the location value" ); @@ -75,18 +85,77 @@ subtest 'General Add, Get and Del tests' => sub { my $dbh = C4::Context->dbh; local $dbh->{RaiseError} = 1; ModItem({}, $bibnum, $itemnumber); + $getitem = GetItem(undef, $item_barcode); + cmp_ok($getitem->{'itemnumber'}, '==', $itemnumber, "Item retrieved by barcode has correct itemnumber."); # Modify item; setting barcode. - ModItem({ barcode => '987654321' }, $bibnum, $itemnumber); + $item_barcode = '987654321'; + ModItem({ barcode => $item_barcode }, $bibnum, $itemnumber); my $moditem = GetItem($itemnumber); - cmp_ok($moditem->{'barcode'}, '==', '987654321', 'Modified item barcode successfully to: '.$moditem->{'barcode'} . '.'); + cmp_ok($moditem->{'barcode'}, '==', $item_barcode, 'Modified item barcode successfully to: ' . $moditem->{'barcode'} . '.'); + + # Update some more properties to get larger coverage + my $item_properties = { + 'dateaccessioned' => '2012-12-12', + 'datelastborrowed' => '2013-12-12', + 'datelastseen' => '2013-12-12', + 'notforloan' => '0', + 'damaged' => '0', + 'itemlost' => '0', + 'itemcallnumber' => '1234', + 'restricted' => '0', + }; + + # Clone item propreties since ModItem modifies this argument + # causing later tests to fail + ModItem({%{$item_properties}}, $bibnum, $itemnumber); + + # Search unblessed tests + my $search_conditions = { + 'itemnumber' => $itemnumber, + }; + my ($item_object) = Koha::Items->search($search_conditions); + my $item_object_data = $item_object->unblessed; + cmp_ok($item_object_data->{'itemnumber'}, '==', $itemnumber, "Item object retrieved by Koha::Items->search using \"itemnumber\" condition has correct itemnumber."); + + # Intersect with updated properties + my $updated_item_properties = { map { ($_ => $item_object_data->{$_}) } keys %{$item_properties} }; + + is_deeply($updated_item_properties, $item_properties, "Updated item properties have correct values."); + + my ($item_data) = Koha::Items->search_unblessed($search_conditions); + cmp_ok( + $item_data->{'itemnumber'}, + '==', + $itemnumber, + "Item data retrieved by Koha::Items->search_unblessed using \"itemnumber\" condition has correct itemnumber." + ); + + ($item_data) = Koha::Items->search_unblessed({ 'barcode' => $item_barcode }); + cmp_ok( + $item_data->{'itemnumber'}, + '==', + $itemnumber, + "Item data retrieved by Koha::Items->search_unblessed using \"barcode\" condition has correct itemnumber." + ); + + is_deeply($item_object_data, $item_data, "Item data retrieved by unblessing item object is identical to item data from Koha::Items->search_unblessed."); # Delete item. DelItem({ biblionumber => $bibnum, itemnumber => $itemnumber }); my $getdeleted = GetItem($itemnumber); is($getdeleted->{'itemnumber'}, undef, "Item deleted as expected."); - ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ homebranch => $library->{branchcode}, holdingbranch => $library->{branchcode}, location => $location, permanent_location => 'my permanent location', itype => $itemtype->{itemtype} } , $bibnum); + ($item_bibnum, $item_bibitemnum, $itemnumber) = AddItem({ + homebranch => $library->{branchcode}, + holdingbranch => $library->{branchcode}, + location => $location, + permanent_location => 'my permanent location', + itype => $itemtype->{itemtype}, + barcode => $item_barcode + }, + $bibnum + ); $getitem = GetItem($itemnumber); is( $getitem->{location}, $location, "The location should not have been modified" ); is( $getitem->{permanent_location}, 'my permanent location', "The permanent_location should not have modified" ); @@ -104,6 +173,11 @@ subtest 'General Add, Get and Del tests' => sub { t::lib::Mocks::mock_preference('item-level_itypes', '1'); $getitem = GetItem($itemnumber); is( $getitem->{itype}, $itemtype->{itemtype}, "Itemtype set correctly when using item-level_itypes" ); + # Good to test this since different code for retrieving items is run in GetItems() depending on item-level_itypes preference + is( $getitem->{itemnumber}, $itemnumber, "Item successfully retrieved by itemnumber when using item-level_itypes" ); + $getitem = GetItem(undef, $item_barcode); + is( $getitem->{itemnumber}, $itemnumber, "Item successfully retrieved by barcode when using item-level_itypes" ); + t::lib::Mocks::mock_preference('item-level_itypes', '0'); $getitem = GetItem($itemnumber); is( $getitem->{itype}, undef, "Itemtype set correctly when not using item-level_itypes" ); --