From 0b33b61caac7025f6c503e2111ee338f0a1afb9a Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Mon, 5 Mar 2018 20:11:52 +0000 Subject: [PATCH] Bug 14385: Squash of a lot of patches rebased Content-Type: text/plain; charset=utf-8 - Added missing GetHiddenItems parameter change case Without this prove t had a failure. - Always use mocks, not set_preference - Tweaks so t/db_dependent/00-strict.t passes There was a typo botcat vs borcat and borrowernumber was never defined. Grabbing from userenv, like other code does. - Tweak t/db_dependent/Items.t to fully test changes This will test all the if structures fully in GetHiddenItemnumbers. prove t/db_dependent/Items.t - Tweak borrower category code $borrower->{categorycode} on a Koha::Patron is not the same as $borrower->categorycode. Fixed error. - Search was returning URLS for wrong interface There was one search context place wrong. Changed it to $is_opac as the logic for setting $is_opac was modified correctly. - Corrected issues with category code. When a user isn't logged in, $borrower is undef and causes error when determining category code. Added conditional check. - Properly trigger all changes in C4/Search.pm - Fix QA Test tool failures C4/Search.pm had some tabs. - Add some commenting to make sense of logic - Refactor EmbedItemsInMarcBiblio parameters to hashref - Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call. prove t/db_dependent/Items.t - Add missing test to trigger Koha/BiblioUtils/Iterator change - Add borrower category overrides These files generally add borcat parameter to GetMarcBiblio. Others might include correction of filtering of items (opac-basket), or a comment as to why no changes were done (opac-search). In the case of opac-search, correcting the first FIXME will likely correct the OpacHiddenItems issues on tags. As such, that is beyond this bugs scope. Some code had loop optimizations and fixes made, like a 'next unless $record' when the biblio shouldn't even be in the list. - Modify opac-ISBDdetail and opac-MARCdetail Both files had similar logic. They were rearranged and optimized, so that both files would have practically identical initial blocks of code. Optimizations were possible, because GetMarcBiblio returns a filtered record, so that there is no double call (once in the opac-### file and once in GetMarcBiblio) to GetHiddenItemnumbers. - Fix hiding in opac-tags opac/opac-tags.pl was not properly hiding. There is currently one known bug associated with tags left. If you have two biblios tagged by different people with the same tag, the opac-search will show the one you tagged that is supposed to be hidden, because tag searches work differently than regular searches. This is beyond the scope of this bug. See the FIXME's in opac/opac-search.pl - Trigger the C4::ILSDI::Services changes prove t/db_dependent/ILSDI_Services.t - Added missing 'my' - Test C4/Labels/Label.pm changes - Improve C4::Record::marcrecord2csv test cases - Corrected opac-details searchResult call - Fix breaking issues constraint in ITerator test - Fix ILSDI_Services test when clubs with branch exist - Rebased again! - Rebased t/db_dependent/Items.t conflict. The test plan is in comment #112 last I checked. Signed-off-by: Marcel de Rooy --- C4/Biblio.pm | 47 ++++++- C4/Record.pm | 5 +- C4/Search.pm | 14 +- Koha/BiblioUtils/Iterator.pm | 4 +- Koha/Exporter/Record.pm | 5 +- cataloguing/additem.pl | 4 +- .../opac-tmpl/bootstrap/en/modules/opac-tags.tt | 2 + misc/migration_tools/build_oai_sets.pl | 4 +- opac/opac-ISBDdetail.pl | 68 +++++----- opac/opac-MARCdetail.pl | 81 ++++++------ opac/opac-basket.pl | 28 +++- opac/opac-detail.pl | 56 ++++---- opac/opac-downloadcart.pl | 13 +- opac/opac-downloadshelf.pl | 11 +- opac/opac-export.pl | 16 ++- opac/opac-search.pl | 8 +- opac/opac-sendbasket.pl | 5 +- opac/opac-sendshelf.pl | 15 ++- opac/opac-tags.pl | 46 ++++++- opac/opac-user.pl | 8 +- t/db_dependent/ILSDI_Services.t | 42 +++++- t/db_dependent/Items.t | 52 ++++++-- t/db_dependent/Koha/BiblioUtils/Iterator.t | 76 +++++++++++ t/db_dependent/Labels/t_Label.t | 147 +++++++++++++++++++++ t/db_dependent/Record/marcrecord2csv.t | 8 +- t/db_dependent/Search.t | 38 ++++++ 26 files changed, 648 insertions(+), 155 deletions(-) create mode 100644 t/db_dependent/Koha/BiblioUtils/Iterator.t create mode 100644 t/db_dependent/Labels/t_Label.t diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 0c5a917..153241f 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1128,7 +1128,8 @@ sub GetMarcSubfieldStructureFromKohaField { my $record = GetMarcBiblio({ biblionumber => $biblionumber, embed_items => $embeditems, - opac => $opac }); + opac => $opac, + borcat => $patron_category }); Returns MARC::Record representing a biblio record, or C if the biblionumber doesn't exist. @@ -1152,6 +1153,12 @@ set to true to include item information. set to true to make the result suited for OPAC view. This causes things like OpacHiddenItems to be applied. +=item C<$borcat> + +If the OpacHiddenItemsExceptions system preference is set, this patron category +can be used to make visible OPAC items which would be normally hidden. +It only makes sense in combination both embed_items and opac values true. + =back =cut @@ -1167,6 +1174,7 @@ sub GetMarcBiblio { my $biblionumber = $params->{biblionumber}; 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'; @@ -1194,7 +1202,11 @@ sub GetMarcBiblio { C4::Biblio::_koha_marc_update_bib_ids( $record, $frameworkcode, $biblionumber, $biblioitemnumber ); - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, undef, $opac ) + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber, + opac => $opac, + borcat => $borcat }) if ($embeditems); return $record; @@ -2772,7 +2784,11 @@ sub ModZebra { =head2 EmbedItemsInMarcBiblio - EmbedItemsInMarcBiblio($marc, $biblionumber, $itemnumbers, $opac); + EmbedItemsInMarcBiblio({ + marc_record => $marc, + biblionumber => $biblionumber, + item_numbers => $itemnumbers, + opac => $opac }); Given a MARC::Record object containing a bib record, modify it to include the items attached to it as 9XX @@ -2781,14 +2797,23 @@ if $itemnumbers is defined, only specified itemnumbers are embedded. If $opac is true, then opac-relevant suppressions are included. +If opac filtering will be done, borcat should be passed to properly +override if necessary. + =cut sub EmbedItemsInMarcBiblio { - my ($marc, $biblionumber, $itemnumbers, $opac) = @_; + my ($params) = @_; + my ($marc, $biblionumber, $itemnumbers, $opac, $borcat); + $marc = $params->{marc_record}; if ( !$marc ) { carp 'EmbedItemsInMarcBiblio: No MARC record passed'; return; } + $biblionumber = $params->{biblionumber}; + $itemnumbers = $params->{item_numbers}; + $opac = $params->{opac}; + $borcat = $params->{borcat} // q{}; $itemnumbers = [] unless defined $itemnumbers; @@ -2799,20 +2824,28 @@ sub EmbedItemsInMarcBiblio { my $dbh = C4::Context->dbh; my $sth = $dbh->prepare("SELECT itemnumber FROM items WHERE biblionumber = ?"); $sth->execute($biblionumber); - my @item_fields; my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField( "items.itemnumber", $frameworkcode ); - my @items; + + 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 + + # Flag indicating if there is potential hiding. 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( map { $_->{item} } @items ) + ? C4::Items::GetHiddenItemnumbers({ + items => \@items2pass, + borcat => $borcat }) : (); # Convert to a hash for quick searching my %hiddenitems = map { $_ => 1 } @hiddenitems; diff --git a/C4/Record.pm b/C4/Record.pm index 174731d..cde523f 100644 --- a/C4/Record.pm +++ b/C4/Record.pm @@ -472,7 +472,10 @@ sub marcrecord2csv { # Getting the record my $record = GetMarcBiblio({ biblionumber => $biblio }); return unless $record; - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblio, $itemnumbers ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblio, + item_numbers => $itemnumbers }); # Getting the framework my $frameworkcode = GetFrameworkCode($biblio); diff --git a/C4/Search.pm b/C4/Search.pm index 4009c18..1e4ae38 100644 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1908,7 +1908,7 @@ sub searchResults { my ($bibliotag,$bibliosubf)=GetMarcFromKohaField('biblio.biblionumber',''); # set stuff for XSLT processing here once, not later again for every record we retrieved - my $interface = $search_context eq 'opac' ? 'OPAC' : ''; + my $interface = $is_opac ? 'OPAC' : ''; my $xslsyspref = $interface . "XSLTResultsDisplay"; my $xslfile = C4::Context->preference($xslsyspref); my $lang = $xslfile ? C4::Languages::getlanguage() : undef; @@ -2206,9 +2206,9 @@ sub searchResults { $other_items->{$key}->{intransit} = ( $transfertwhen ne '' ) ? 1 : 0; $other_items->{$key}->{onhold} = ($reservestatus) ? 1 : 0; $other_items->{$key}->{notforloan} = GetAuthorisedValueDesc('','',$item->{notforloan},'','',$notforloan_authorised_value) if $notforloan_authorised_value and $item->{notforloan}; - $other_items->{$key}->{count}++ if $item->{$hbranch}; - $other_items->{$key}->{location} = $shelflocations->{ $item->{location} }; - $other_items->{$key}->{description} = $item->{description}; + $other_items->{$key}->{count}++ if $item->{$hbranch}; + $other_items->{$key}->{location} = $shelflocations->{ $item->{location} }; + $other_items->{$key}->{description} = $item->{description}; $other_items->{$key}->{imageurl} = getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype} }->{imageurl} ); } # item is available @@ -2217,9 +2217,9 @@ sub searchResults { $available_count++; $available_items->{$prefix}->{count}++ if $item->{$hbranch}; foreach (qw(branchname itemcallnumber description)) { - $available_items->{$prefix}->{$_} = $item->{$_}; - } - $available_items->{$prefix}->{location} = $shelflocations->{ $item->{location} }; + $available_items->{$prefix}->{$_} = $item->{$_}; + } + $available_items->{$prefix}->{location} = $shelflocations->{ $item->{location} }; $available_items->{$prefix}->{imageurl} = getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype} }->{imageurl} ); } } diff --git a/Koha/BiblioUtils/Iterator.pm b/Koha/BiblioUtils/Iterator.pm index 0e71966..ff60871 100644 --- a/Koha/BiblioUtils/Iterator.pm +++ b/Koha/BiblioUtils/Iterator.pm @@ -108,7 +108,9 @@ sub next { if ( !defined($bibnum) ); # TODO this should really be in Koha::BiblioUtils or something similar. - C4::Biblio::EmbedItemsInMarcBiblio( $marc, $bibnum ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $marc, + biblionumber => $bibnum }); } if (wantarray) { diff --git a/Koha/Exporter/Record.pm b/Koha/Exporter/Record.pm index 311f88d..5b145f8 100644 --- a/Koha/Exporter/Record.pm +++ b/Koha/Exporter/Record.pm @@ -70,7 +70,10 @@ sub _get_biblio_for_export { return if $@ or not defined $record; if ($export_items) { - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, $itemnumbers ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber, + item_numbers => $itemnumbers }); if ($only_export_items_for_branches && @$only_export_items_for_branches) { my %export_items_for_branches = map { $_ => 1 } @$only_export_items_for_branches; my ( $homebranchfield, $homebranchsubfield ) = GetMarcFromKohaField( 'items.homebranch', '' ); # Should be GetFrameworkCode( $biblionumber )? diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index 6af351d..02d45dd 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -735,7 +735,9 @@ my @big_array; #---- finds where items.itemnumber is stored my ( $itemtagfield, $itemtagsubfield) = &GetMarcFromKohaField("items.itemnumber", $frameworkcode); my ($branchtagfield, $branchtagsubfield) = &GetMarcFromKohaField("items.homebranch", $frameworkcode); -C4::Biblio::EmbedItemsInMarcBiblio($temp, $biblionumber); +C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $temp, + biblionumber => $biblionumber }); my @fields = $temp->fields(); diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt index 5316f57..08b5528 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt @@ -130,6 +130,7 @@  TermTitleDate added [% FOREACH MY_TAG IN MY_TAGS %] + [% IF MY_TAG.visible %] @@ -167,6 +168,7 @@ + [% END %] [% END %] diff --git a/misc/migration_tools/build_oai_sets.pl b/misc/migration_tools/build_oai_sets.pl index 926b1db..50b8d1f 100755 --- a/misc/migration_tools/build_oai_sets.pl +++ b/misc/migration_tools/build_oai_sets.pl @@ -127,7 +127,9 @@ foreach my $res (@$results) { next; } if($embed_items) { - C4::Biblio::EmbedItemsInMarcBiblio($record, $biblionumber); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber }); } my @biblio_sets = CalcOAISetsBiblio($record, $mappings); diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index d6aba31..1cfbf00 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -60,6 +60,14 @@ use Koha::RecordProcessor; use Koha::Biblios; my $query = CGI->new(); +my $biblionumber = $query->param('biblionumber'); +if ( !$biblionumber ) { + print $query->redirect('/cgi-bin/koha/errors/404.pl'); + exit; +} +$biblionumber = int($biblionumber); + +#open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { template_name => "opac-ISBDdetail.tt", @@ -70,48 +78,33 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); -my $biblionumber = $query->param('biblionumber'); -$biblionumber = int($biblionumber); - -# get biblionumbers stored in the cart -if(my $cart_list = $query->cookie("bib_list")){ - my @cart_list = split(/\//, $cart_list); - if ( grep {$_ eq $biblionumber} @cart_list) { - $template->param( incart => 1 ); - } -} - -my $marcflavour = C4::Context->preference("marcflavour"); - -my @items = GetItemsInfo($biblionumber); -if (scalar @items >= 1) { - my $borcat; - if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { - - # we need to fetch the borrower info here, so we can pass the category - my $borrower = GetMember( borrowernumber => $borrowernumber ); - $borcat = $borrower->{categorycode}; - } - - my @hiddenitems = GetHiddenItemnumbers( { items => \@items, borcat => $borcat }); - - if (scalar @hiddenitems == scalar @items ) { - print $query->redirect("/cgi-bin/koha/errors/404.pl"); # escape early - exit; - } +my $borcat = q{}; +if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { + # we need to fetch the borrower info here, so we can pass the category + my $patron = Koha::Patrons->find( { borrowernumber => $loggedinuser } ); + $borcat = $patron ? $patron->categorycode : $borcat; } my $record = GetMarcBiblio({ biblionumber => $biblionumber, - embed_items => 1 }); + embed_items => 1, + opac => 1, + borcat => $borcat }); if ( ! $record ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; } +my @all_items = GetItemsInfo($biblionumber); my $biblio = Koha::Biblios->find( $biblionumber ); +my $framework = $biblio ? $biblio->frameworkcode : q{}; +my ($tag_itemnumber, $subtag_itemnumber) = &GetMarcFromKohaField('items.itemnumber',$framework); +my @nonhiddenitems = $record->field($tag_itemnumber); +if (scalar @all_items >= 1 && scalar @nonhiddenitems == 0) { + print $query->redirect('/cgi-bin/koha/errors/404.pl'); # escape early + exit; +} -my $framework = GetFrameworkCode( $biblionumber ); my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy', options => { @@ -121,6 +114,16 @@ my $record_processor = Koha::RecordProcessor->new({ }); $record_processor->process($record); +# get biblionumbers stored in the cart +if(my $cart_list = $query->cookie("bib_list")){ + my @cart_list = split(/\//, $cart_list); + if ( grep {$_ eq $biblionumber} @cart_list) { + $template->param( incart => 1 ); + } +} + +my $marcflavour = C4::Context->preference("marcflavour"); + # some useful variables for enhanced content; # in each case, we're grabbing the first value we find in # the record and normalizing it @@ -142,7 +145,6 @@ $template->param( #coping with subscriptions my $subscriptionsnumber = CountSubscriptionFromBiblionumber($biblionumber); -my $dbh = C4::Context->dbh; my $dat = TransformMarcToKoha( $record ); my @subscriptions = SearchSubscriptions({ biblionumber => $biblionumber, orderby => 'title' }); @@ -178,7 +180,7 @@ my $res = GetISBDView({ my $itemtypes = { map { $_->{itemtype} => $_ } @{ Koha::ItemTypes->search_with_localization->unblessed } }; my $patron = Koha::Patrons->find( $loggedinuser ); -for my $itm (@items) { +for my $itm (@all_items) { my $item = Koha::Items->find( $itm->{itemnumber} ); $norequests = 0 if $norequests diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index 533c9a5..4867eda 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -64,46 +64,53 @@ use Koha::ItemTypes; use Koha::Patrons; use Koha::RecordProcessor; -my $query = new CGI; - -my $dbh = C4::Context->dbh; +my $query = CGI->new(); my $biblionumber = $query->param('biblionumber'); if ( ! $biblionumber ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; } +$biblionumber = int($biblionumber); + +# open template +my ( $template, $loggedinuser, $cookie ) = get_template_and_user( + { + template_name => "opac-MARCdetail.tt", + query => $query, + type => "opac", + authnotrequired => ( C4::Context->preference("OpacPublic") ? 1 : 0 ), + debug => 1, + } +); + +my $patron = Koha::Patrons->find( $loggedinuser ); +my $borcat = q{}; +if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { + # we need to fetch the borrower info here, so we can pass the category + $borcat = $patron ? $patron->categorycode : $borcat; +} my $record = GetMarcBiblio({ biblionumber => $biblionumber, - embed_items => 1 }); + embed_items => 1, + opac => 1, + borcat => $borcat }); if ( ! $record ) { print $query->redirect("/cgi-bin/koha/errors/404.pl"); exit; } my @all_items = GetItemsInfo($biblionumber); -my @items2hide; -if (scalar @all_items >= 1) { - my $borcat; - if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { - - # we need to fetch the borrower info here, so we can pass the category - my $borrower = GetMember( borrowernumber => $borrowernumber ); - $borcat = $borrower->{categorycode}; - } - push @items2hide, GetHiddenItemnumbers({ items => \@all_items, borcat => $botcat }); - - if (scalar @items2hide == scalar @all_items ) { - print $query->redirect("/cgi-bin/koha/errors/404.pl"); - exit; - } -} - -my $framework = &GetFrameworkCode( $biblionumber ); +my $biblio = Koha::Biblios->find( $biblionumber ); +my $framework = $biblio ? $biblio->frameworkcode : q{}; my $tagslib = &GetMarcStructure( 0, $framework ); my ($tag_itemnumber,$subtag_itemnumber) = &GetMarcFromKohaField('items.itemnumber',$framework); -my $biblio = Koha::Biblios->find( $biblionumber ); +my @nonhiddenitems = $record->field($tag_itemnumber); +if (scalar @all_items >= 1 && scalar @nonhiddenitems == 0) { + print $query->redirect("/cgi-bin/koha/errors/404.pl"); + exit; +} my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy', @@ -114,23 +121,6 @@ my $record_processor = Koha::RecordProcessor->new({ }); $record_processor->process($record); -# open template -my ( $template, $loggedinuser, $cookie ) = get_template_and_user( - { - template_name => "opac-MARCdetail.tt", - query => $query, - type => "opac", - authnotrequired => ( C4::Context->preference("OpacPublic") ? 1 : 0 ), - debug => 1, - } -); - -my ($bt_tag,$bt_subtag) = GetMarcFromKohaField('biblio.title',$framework); -$template->param( - bibliotitle => $biblio->title, -) if $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} <= 0 && # <=0 OPAC visible. - $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} > -8; # except -8; - # get biblionumbers stored in the cart if(my $cart_list = $query->cookie("bib_list")){ my @cart_list = split(/\//, $cart_list); @@ -139,8 +129,13 @@ if(my $cart_list = $query->cookie("bib_list")){ } } +my ($bt_tag,$bt_subtag) = GetMarcFromKohaField('biblio.title',$framework); +$template->param( + bibliotitle => $biblio->title, +) if $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} <= 0 && # <=0 OPAC visible. + $tagslib->{$bt_tag}->{$bt_subtag}->{hidden} > -8; # except -8; + my $allow_onshelf_holds; -my $patron = Koha::Patrons->find( $loggedinuser ); for my $itm (@all_items) { my $item = Koha::Items->find( $itm->{itemnumber} ); $allow_onshelf_holds = Koha::IssuingRules->get_onshelfholds_policy( { item => $item, patron => $patron } ); @@ -279,6 +274,7 @@ for ( my $tabloop = 0 ; $tabloop <= 9 ; $tabloop++ ) { # loop through each tag # warning : we may have differents number of columns in each row. Thus, we first build a hash, complete it if necessary # then construct template. +# $record has already had all the item fields filtered above. my @fields = $record->fields(); my %witness ; #---- stores the list of subfields used at least once, with the "meaning" of the code @@ -286,9 +282,6 @@ my @item_subfield_codes; my @item_loop; foreach my $field (@fields) { next if ( $field->tag() < 10 ); - next if ( ( $field->tag() eq $tag_itemnumber ) && - ( any { $field->subfield($subtag_itemnumber) eq $_ } - @items2hide) ); my @subf = $field->subfields; my $item; diff --git a/opac/opac-basket.pl b/opac/opac-basket.pl index 87eea86..fd48dcf 100755 --- a/opac/opac-basket.pl +++ b/opac/opac-basket.pl @@ -56,6 +56,13 @@ if (C4::Context->preference('TagsEnabled')) { } } +my $borcat = q{}; +if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { + # we need to fetch the borrower info here, so we can pass the category + my $borrower = Koha::Patron->find( { borrowernumber -> $borrowernumber } ); + $borcat = $borrower ? $borrower->categorycode : $borcat; +} + my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); foreach my $biblionumber ( @bibs ) { $template->param( biblionumber => $biblionumber ); @@ -63,6 +70,8 @@ foreach my $biblionumber ( @bibs ) { my $dat = &GetBiblioData($biblionumber); next unless $dat; + # No filtering on the item records needed for the record itself + # since the only reason item information is grabbed is because of branchcodes. my $record = &GetMarcBiblio({ biblionumber => $biblionumber }); my $framework = &GetFrameworkCode( $biblionumber ); $record_processor->options({ @@ -76,7 +85,24 @@ foreach my $biblionumber ( @bibs ) { my $marcsubjctsarray = GetMarcSubjects( $record, $marcflavour ); my $marcseriesarray = GetMarcSeries ($record,$marcflavour); my $marcurlsarray = GetMarcUrls ($record,$marcflavour); - my @items = &GetItemsInfo( $biblionumber ); + + # grab all the items... + my @all_items = &GetItemsInfo( $biblionumber ); + + # determine which ones should be hidden / visible + my @hidden_items = GetHiddenItemnumbers({ items => \@all_items, borcat => $borcat }); + + # If every item is hidden, then the biblio should be hidden too. + next if (scalar @all_items >= 1 && scalar @hidden_items == scalar @all_items); + + # copy the visible ones into the items array. + my @items; + foreach my $item (@all_items) { + if ( none { $item->{itemnumber} ne $_ } @hidden_items ) { + push @items, $item; + } + } + my $subtitle = GetRecordValue('subtitle', $record, GetFrameworkCode($biblionumber)); my $hasauthors = 0; diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index 460cc39..56ebf62 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -68,7 +68,11 @@ BEGIN { } } -my $query = new CGI; +my $query = CGI->new(); + +my $biblionumber = $query->param('biblionumber') || $query->param('bib') || 0; +$biblionumber = int($biblionumber); + my ( $template, $borrowernumber, $cookie ) = get_template_and_user( { template_name => "opac-detail.tt", @@ -78,20 +82,23 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); -my $biblionumber = $query->param('biblionumber') || $query->param('bib') || 0; -$biblionumber = int($biblionumber); - my @all_items = GetItemsInfo($biblionumber); my @hiddenitems; -if ( scalar @all_items >= 1 ) { - my $borcat; - if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { +my $patron = Koha::Patrons->find( $borrowernumber ); +my $borcat= q{}; +if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { + $borcat = $patron ? $patron->categorycode : q{}; +} - # we need to fetch the borrower info here, so we can pass the category - my $borrower = GetMember( borrowernumber => $borrowernumber ); - $borcat = $borrower->{categorycode}; - } +my $record = GetMarcBiblio({ + biblionumber => $biblionumber, + opac => 1 }); +if ( ! $record ) { + print $query->redirect("/cgi-bin/koha/errors/404.pl"); # escape early + exit; +} +if ( scalar @all_items >= 1 ) { push @hiddenitems, GetHiddenItemnumbers( { items => \@all_items, borcat => $borcat } ); @@ -101,14 +108,8 @@ if ( scalar @all_items >= 1 ) { } } -my $record = GetMarcBiblio({ biblionumber => $biblionumber }); -if ( ! $record ) { - print $query->redirect("/cgi-bin/koha/errors/404.pl"); # escape early - exit; -} - my $biblio = Koha::Biblios->find( $biblionumber ); -my $framework = &GetFrameworkCode( $biblionumber ); +my $framework = $biblio ? $biblio->frameworkcode : q{}; my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy', options => { @@ -239,10 +240,14 @@ if ($session->param('busc')) { }; my $hits; my @newresults; + my $search_context = { + 'interface' => 'opac', + 'category' => $borcat + }; for (my $i=0;$i<@servers;$i++) { my $server = $servers[$i]; $hits = $results_hashref->{$server}->{"hits"}; - @newresults = searchResults({ 'interface' => 'opac' }, '', $hits, $results_per_page, $offset, $arrParamsBusc->{'scan'}, $results_hashref->{$server}->{"RECORDS"}); + @newresults = searchResults( $search_context, '', $hits, $results_per_page, $offset, $arrParamsBusc->{'scan'}, $results_hashref->{$server}->{"RECORDS"}); } return \@newresults; }#searchAgain @@ -670,7 +675,6 @@ if ( not $viewallitems and @items > $max_items_to_display ) { items_count => scalar( @items ), ); } else { - my $patron = Koha::Patrons->find( $borrowernumber ); for my $itm (@items) { my $item = Koha::Items->find( $itm->{itemnumber} ); $itm->{holds_count} = $item_reserves{ $itm->{itemnumber} }; @@ -865,16 +869,16 @@ if ( C4::Context->preference('reviewson') ) { } } for my $review (@$reviews) { - my $patron = Koha::Patrons->find( $review->{borrowernumber} ); # FIXME Should be Koha::Review->reviewer or similar + my $review_patron = Koha::Patrons->find( $review->{borrowernumber} ); # FIXME Should be Koha::Review->reviewer or similar # setting some borrower info into this hash - if ( $patron ) { - $review->{patron} = $patron; - if ( $libravatar_enabled and $patron->email ) { - $review->{avatarurl} = libravatar_url( email => $patron->email, https => $ENV{HTTPS} ); + if ( $review_patron ) { + $review->{patron} = $review_patron; + if ( $libravatar_enabled and $review_patron->email ) { + $review->{avatarurl} = libravatar_url( email => $review_patron->email, https => $ENV{HTTPS} ); } - if ( $patron->borrowernumber eq $borrowernumber ) { + if ( $review_patron->borrowernumber eq $borrowernumber ) { $loggedincommenter = 1; } } diff --git a/opac/opac-downloadcart.pl b/opac/opac-downloadcart.pl index 76fa095..ec5e2c8 100755 --- a/opac/opac-downloadcart.pl +++ b/opac/opac-downloadcart.pl @@ -32,7 +32,7 @@ use Koha::CsvProfiles; use Koha::RecordProcessor; use utf8; -my $query = new CGI; +my $query = CGI->new(); my ( $template, $borrowernumber, $cookie ) = get_template_and_user ( { @@ -49,6 +49,13 @@ my $dbh = C4::Context->dbh; if ($bib_list && $format) { + my $borcat = q{}; + if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { + # we need to fetch the borrower info here, so we can pass the category + my $borrower = Koha::Patrons->find( { borrowernumber => $borrowernumber } ); + $borcat = $borrower ? $borrower->categorycode : $borcat; + } + my @bibs = split( /\//, $bib_list ); my $marcflavour = C4::Context->preference('marcflavour'); @@ -70,7 +77,9 @@ if ($bib_list && $format) { my $record = GetMarcBiblio({ biblionumber => $biblio, - embed_items => 1 }); + embed_items => 1, + opac => 1, + borcat => $borcat }); my $framework = &GetFrameworkCode( $biblio ); $record_processor->options({ interface => 'opac', diff --git a/opac/opac-downloadshelf.pl b/opac/opac-downloadshelf.pl index 007dd5f..ac6e910 100755 --- a/opac/opac-downloadshelf.pl +++ b/opac/opac-downloadshelf.pl @@ -50,6 +50,13 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user ( } ); +my $borcat = q{}; +if ( C4::Context->preference('OpacHiddenItemsExceptions') ) { + # we need to fetch the borrower info here, so we can pass the category + my $borrower = Koha::Patrons->find( { borrowernumber => $borrowernumber } ); + $borcat = $borrower ? $borrower->categorycode : $borcat; +} + my $shelfnumber = $query->param('shelfnumber'); my $format = $query->param('format'); my $context = $query->param('context'); @@ -83,7 +90,9 @@ if ( $shelf and $shelf->can_be_viewed( $borrowernumber ) ) { my $record = GetMarcBiblio({ biblionumber => $biblionumber, - embed_items => 1 }); + embed_items => 1, + opac => 1, + borcat => $borcat }); my $framework = &GetFrameworkCode( $biblionumber ); $record_processor->options({ interface => 'opac', diff --git a/opac/opac-export.pl b/opac/opac-export.pl index 4ffe1d3..a992d46 100755 --- a/opac/opac-export.pl +++ b/opac/opac-export.pl @@ -35,11 +35,25 @@ my $biblionumber = $query->param("bib")||0; $biblionumber = int($biblionumber); my $error = q{}; +# Determine logged in user's patron category. +# Blank if not logged in. +my $userenv = C4::Context->userenv; +my $borcat = q{}; +if ($userenv) { + my $borrowernumber = $userenv->{'number'}; + if ($borrowernumber) { + my $borrower = Koha::Patrons->find( { borrowernumber => $borrowernumber } ); + $borcat = $borrower ? $borrower->categorycode : $borcat; + } +} + my $include_items = ($format =~ /bibtex/) ? 0 : 1; my $marc = $biblionumber ? GetMarcBiblio({ biblionumber => $biblionumber, - embed_items => $include_items }) + embed_items => $include_items, + opac => 1, + borcat => $borcat }) : undef; if(!$marc) { diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 5542e32..de72a8b 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -51,8 +51,8 @@ use C4::Koha; use C4::Tags qw(get_tags); use C4::SocialData; use C4::External::OverDrive; -use C4::Members qw(GetMember); +use Koha::Libraries; use Koha::ItemTypes; use Koha::Ratings; use Koha::Virtualshelves; @@ -596,6 +596,8 @@ if ($tag) { $DEBUG and printf STDERR "taglist (%s biblionumber)\nmarclist (%s records)\n", scalar(@$taglist), scalar(@marclist); $results_hashref->{biblioserver}->{RECORDS} = \@marclist; # FIXME: tag search and standard search should work together, not exclusively + # FIXME: Because search and standard search don't work together OpacHiddenItems + # displays search results which should be hidden. # FIXME: No facets for tags search. } elsif ($build_grouped_results) { eval { @@ -626,8 +628,8 @@ my $search_context = {}; $search_context->{'interface'} = 'opac'; if (C4::Context->preference('OpacHiddenItemsExceptions')){ # we need to fetch the borrower info here, so we can pass the category - my $borrower = GetMember( borrowernumber => $borrowernumber ); - $search_context->{'category'} = $borrower->{'categorycode'}; + my $patron = Koha::Patrons->find( { borrowernumber => $borrowernumber } ); + $search_context->{'category'} = $patron ? $patron->categorycode : q{}; } for (my $i=0;$i<@servers;$i++) { diff --git a/opac/opac-sendbasket.pl b/opac/opac-sendbasket.pl index 09c462c..a62711c 100755 --- a/opac/opac-sendbasket.pl +++ b/opac/opac-sendbasket.pl @@ -59,6 +59,7 @@ if ( $email_add ) { }); my $email = Koha::Email->new(); my $patron = Koha::Patrons->find( $borrowernumber ); + my $borcat = $patron ? $patron->categorycode : q{}; my $user_email = $patron->first_valid_email_address || C4::Context->preference('KohaAdminEmailAddress'); @@ -89,7 +90,9 @@ if ( $email_add ) { next unless $dat; my $record = GetMarcBiblio({ biblionumber => $biblionumber, - embed_items => 1 }); + embed_items => 1, + opac => 1, + borcat => $borcat }); my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); my $marcsubjctsarray = GetMarcSubjects( $record, $marcflavour ); diff --git a/opac/opac-sendshelf.pl b/opac/opac-sendshelf.pl index a3b6663..0fe5769 100755 --- a/opac/opac-sendshelf.pl +++ b/opac/opac-sendshelf.pl @@ -79,6 +79,9 @@ if ( $email ) { } ); + my $patron = Koha::Patrons->find( $borrowernumber ); + my $borcat = $patron ? $patron->categorycode : q{}; + my $shelf = Koha::Virtualshelves->find( $shelfid ); my $contents = $shelf->get_contents; my $marcflavour = C4::Context->preference('marcflavour'); @@ -87,11 +90,15 @@ if ( $email ) { while ( my $content = $contents->next ) { my $biblionumber = $content->biblionumber; - my $fw = GetFrameworkCode($biblionumber); - my $dat = GetBiblioData($biblionumber); my $record = GetMarcBiblio({ biblionumber => $biblionumber, - embed_items => 1 }); + embed_items => 1, + opac => 1, + borcat => $borcat }); + next unless $record; + my $fw = GetFrameworkCode($biblionumber); + my $dat = GetBiblioData($biblionumber); + my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); my $marcsubjctsarray = GetMarcSubjects( $record, $marcflavour ); my $subtitle = GetRecordValue('subtitle', $record, $fw); @@ -111,8 +118,6 @@ if ( $email ) { push( @results, $dat ); } - my $patron = Koha::Patrons->find( $borrowernumber ); - $template2->param( BIBLIO_RESULTS => \@results, comment => $comment, diff --git a/opac/opac-tags.pl b/opac/opac-tags.pl index 267c3c0..272d896 100755 --- a/opac/opac-tags.pl +++ b/opac/opac-tags.pl @@ -41,6 +41,7 @@ use C4::Debug; use C4::Output qw(:html :ajax pagination_bar); use C4::Scrubber; use C4::Biblio; +use C4::Items qw(GetItemsInfo GetHiddenItemnumbers); use C4::Tags qw(add_tag get_approval_rows get_tag_rows remove_tag stratify_tags); use C4::XSLT; @@ -226,13 +227,35 @@ if ($is_ajax) { my $results = []; my $my_tags = []; +my $borcat = q{}; if ($loggedinuser) { + my $patron = Koha::Patrons->find( { borrowernumber => $loggedinuser } ); + $borcat = $patron ? $patron->categorycode : $borcat; + my $should_hide = C4::Context->preference('OpacHiddenItems') // q{}; + $should_hide = ( $should_hide =~ /\S/ ) ? 1 : 0; $my_tags = get_tag_rows({borrowernumber=>$loggedinuser}); my $my_approved_tags = get_approval_rows({ approved => 1 }); foreach my $tag (@$my_tags) { + $tag->{visible} = 0; my $biblio = Koha::Biblios->find( $tag->{biblionumber} ); - my $record = &GetMarcBiblio({ biblionumber => $tag->{biblionumber} }); + my $record = &GetMarcBiblio({ + biblionumber => $tag->{biblionumber}, + embed_items => 1, + opac => 1, + borcat => $borcat }); + next unless $record; + my $hidden_items = undef; + my @hidden_itemnumbers; + my @all_items; + if ($should_hide) { + @all_items = GetItemsInfo( $tag->{biblionumber} ); + @hidden_itemnumbers = GetHiddenItemnumbers({ + items => \@all_items, + borcat => $borcat }); + $hidden_items = \@hidden_itemnumbers; + } + next if ( $should_hide && scalar @all_items == scalar @hidden_itemnumbers ); $tag->{subtitle} = GetRecordValue( 'subtitle', $record, GetFrameworkCode( $tag->{biblionumber} ) ); $tag->{title} = $biblio->title; $tag->{author} = $biblio->author; @@ -244,7 +267,7 @@ if ($loggedinuser) { if ( $xslfile ) { $tag->{XSLTBloc} = XSLTParse4Display( $tag->{ biblionumber }, $record, "OPACXSLTResultsDisplay", - 1, undef, $sysxml, $xslfile, $lang + 1, $hidden_items, $sysxml, $xslfile, $lang ); } @@ -252,6 +275,7 @@ if ($loggedinuser) { $date =~ /\s+(\d{2}\:\d{2}\:\d{2})/; $tag->{time_created_display} = $1; $tag->{approved} = ( grep { $_->{term} eq $tag->{term} and $_->{approved} } @$my_approved_tags ); + $tag->{visible} = 1; } } @@ -281,6 +305,24 @@ if ($add_op) { $arghash->{biblionumber} = $arg; } $results = get_approval_rows($arghash); + my @filtered_results; + foreach my $my_tag (@$my_tags) { + if (grep { $_->{term} eq $my_tag->{term} } @$results) { + if (! $my_tag->{visible} ) { + my $check_biblio = GetMarcBiblio({ + biblionumber => $my_tag->{biblionumber}, + embed_items => 1, + opac => 1, + borcat => $borcat }); + if ($check_biblio) { + push @filtered_results, $my_tag; + } + } else { + push @filtered_results, $my_tag; + } + } + } + $results = \@filtered_results; stratify_tags(10, $results); # work out the differents sizes for things my $count = scalar @$results; $template->param(TAGLOOP_COUNT => $count, mine => $mine); diff --git a/opac/opac-user.pl b/opac/opac-user.pl index 500042d..bc159da 100755 --- a/opac/opac-user.pl +++ b/opac/opac-user.pl @@ -90,6 +90,8 @@ $template->param( shibbolethAuthentication => C4::Context->config('useshibboleth # get borrower information .... my $patron = Koha::Patrons->find( $borrowernumber ); my $borr = $patron->unblessed; +# unblessed is a hash vs. object/undef. Hence the use of curly braces here. +my $borcat = $borr ? $borr->{categorycode} : q{}; my ( $today_year, $today_month, $today_day) = Today(); my ($warning_year, $warning_month, $warning_day) = split /-/, $borr->{'dateexpiry'}; @@ -215,7 +217,11 @@ if ( $pending_checkouts->count ) { # Useless test ); $issue->{rentalfines} = $rental_fines->count ? $rental_fines->next->get_column('rental_fines') : 0; - my $marcrecord = GetMarcBiblio({ biblionumber => $issue->{'biblionumber'} }); + my $marcrecord = GetMarcBiblio({ + biblionumber => $issue->{'biblionumber'}, + embed_items => 1, + opac => 1, + borcat => $borcat }); $issue->{'subtitle'} = GetRecordValue('subtitle', $marcrecord, GetFrameworkCode($issue->{'biblionumber'})); # check if item is renewable my ($status,$renewerror) = CanBookBeRenewed( $borrowernumber, $issue->{'itemnumber'} ); diff --git a/t/db_dependent/ILSDI_Services.t b/t/db_dependent/ILSDI_Services.t index 9b6908f..32115f2 100644 --- a/t/db_dependent/ILSDI_Services.t +++ b/t/db_dependent/ILSDI_Services.t @@ -19,7 +19,7 @@ use Modern::Perl; use CGI qw ( -utf8 ); -use Test::More tests => 4; +use Test::More tests => 5; use Test::MockModule; use t::lib::Mocks; use t::lib::TestBuilder; @@ -113,6 +113,7 @@ subtest 'GetPatronInfo/GetBorrowerAttributes test for extended patron attributes $schema->resultset( 'BorrowerAttributeType' )->delete_all; $schema->resultset( 'Category' )->delete_all; $schema->resultset( 'Item' )->delete_all; # 'Branch' deps. on this + $schema->resultset( 'Club' )->delete_all; $schema->resultset( 'Branch' )->delete_all; # Configure Koha to enable ILS-DI server and extended attributes: @@ -287,3 +288,42 @@ subtest 'LookupPatron test' => sub { # Cleanup $schema->storage->txn_rollback; }; + +# This is a stub, as it merely is for triggering the GetMarcBiblio call. +subtest 'GetRecords' => sub { + + plan tests => 1; + + $schema->storage->txn_begin; + + my $biblio = $builder->build({ + source => 'Biblio', + value => { + title => 'Title 1', }, + }); + + my $item = $builder->build({ + source => 'Item', + value => { + biblionumber => $biblio->{biblionumber}, + }, + }); + + my $biblioitem = $builder->build({ + source => 'Biblioitem', + value => { + biblionumber => $biblio->{biblionumber}, + itemnumber => $item->{itemnumber}, + }, + }); + + my $query = CGI->new({ + 'schema' => 'MARCXML', + 'id' => [ $biblio->{biblionumber} ] + }); + + my $result = C4::ILSDI::Services::GetRecords($query); + ok($result,'There is a result'); + + $schema->storage->txn_rollback; +} diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 9783d82..4c5c570 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -142,7 +142,7 @@ subtest 'ModItem tests' => sub { subtest 'GetHiddenItemnumbers tests' => sub { - plan tests => 9; + plan tests => 11; # This sub is controlled by the OpacHiddenItems system preference. @@ -192,12 +192,12 @@ subtest 'GetHiddenItemnumbers tests' => sub { push @items, GetItem( $item2_itemnumber ); # Empty OpacHiddenItems - C4::Context->set_preference('OpacHiddenItems',''); + t::lib::Mocks::mock_preference('OpacHiddenItems',''); ok( !defined( GetHiddenItemnumbers( { items => \@items } ) ), "Hidden items list undef if OpacHiddenItems empty"); # Blank spaces - C4::Context->set_preference('OpacHiddenItems',' '); + t::lib::Mocks::mock_preference('OpacHiddenItems',' '); ok( scalar GetHiddenItemnumbers( { items => \@items } ) == 0, "Hidden items list empty if OpacHiddenItems only contains blanks"); @@ -213,7 +213,6 @@ subtest 'GetHiddenItemnumbers tests' => sub { $opachiddenitems = " withdrawn: [1,0]"; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); - C4::Context->set_preference( 'OpacHiddenItems', $opachiddenitems ); @hidden = GetHiddenItemnumbers( { items => \@items } ); ok( scalar @hidden == 2, "Two items hidden"); is_deeply( \@hidden, \@itemnumbers, "withdrawn=1 and withdrawn=0 hidden"); @@ -228,6 +227,13 @@ subtest 'GetHiddenItemnumbers tests' => sub { ok( scalar @hidden == 2, "Two items hidden"); is_deeply( \@hidden, \@itemnumbers, "withdrawn=1 and homebranch library2 hidden"); + # Override hidden with patron category + t::lib::Mocks::mock_preference( 'OpacHiddenItemsExceptions', 'S' ); + @hidden = GetHiddenItemnumbers( { items => \@items, borcat => 'PT' } ); + ok( scalar @hidden == 2, "Two items still hidden"); + @hidden = GetHiddenItemnumbers( { items => \@items, borcat => 'S' } ); + ok( scalar @hidden == 0, "Two items not hidden"); + # Valid OpacHiddenItems, empty list @items = (); @hidden = GetHiddenItemnumbers( { items => \@items } ); @@ -548,7 +554,7 @@ subtest 'Koha::Item(s) tests' => sub { }; subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { - plan tests => 7; + plan tests => 8; $schema->storage->txn_begin(); @@ -599,18 +605,30 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { my $record = C4::Biblio::GetMarcBiblio({ biblionumber => $biblionumber }); warning_is { C4::Biblio::EmbedItemsInMarcBiblio() } { carped => 'EmbedItemsInMarcBiblio: No MARC record passed' }, - 'Should crap is no record passed.'; + 'Should carp is no record passed.'; - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber }); my @items = $record->field($itemfield); is( scalar @items, $number_of_items, 'Should return all items' ); - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, - [ $itemnumbers[1], $itemnumbers[3] ] ); + my $marc_with_items = C4::Biblio::GetMarcBiblio({ + biblionumber => $biblionumber, + embed_items => 1 }); + is_deeply( $record, $marc_with_items, 'A direct call to GetMarcBiblio with items matches'); + + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber, + item_numbers => [ $itemnumbers[1], $itemnumbers[3] ] }); @items = $record->field($itemfield); is( scalar @items, 2, 'Should return all items present in the list' ); - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, undef, 1 ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber, + opac => 1 }); @items = $record->field($itemfield); is( scalar @items, $number_of_items, 'Should return all items for opac' ); @@ -618,13 +636,18 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { homebranch: ['$library1->{branchcode}']"; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber }); @items = $record->field($itemfield); is( scalar @items, $number_of_items, 'Even with OpacHiddenItems set, all items should have been embedded' ); - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, undef, 1 ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber, + opac => 1 }); @items = $record->field($itemfield); is( scalar @items, @@ -635,7 +658,10 @@ subtest 'C4::Biblio::EmbedItemsInMarcBiblio' => sub { $opachiddenitems = " homebranch: ['$library1->{branchcode}', '$library2->{branchcode}']"; t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); - C4::Biblio::EmbedItemsInMarcBiblio( $record, $biblionumber, undef, 1 ); + C4::Biblio::EmbedItemsInMarcBiblio({ + marc_record => $record, + biblionumber => $biblionumber, + opac => 1 }); @items = $record->field($itemfield); is( scalar @items, diff --git a/t/db_dependent/Koha/BiblioUtils/Iterator.t b/t/db_dependent/Koha/BiblioUtils/Iterator.t new file mode 100644 index 0000000..ac11cf7 --- /dev/null +++ b/t/db_dependent/Koha/BiblioUtils/Iterator.t @@ -0,0 +1,76 @@ +#!/usr/bin/perl +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 3; + +use_ok('Koha::BiblioUtils'); +use_ok('Koha::BiblioUtils::Iterator'); + +use C4::Biblio; +use C4::Items; +use DBI; +use t::lib::TestBuilder; +use t::lib::Mocks; + +my $schema = Koha::Database->new()->schema(); +$schema->storage->txn_begin(); + +# Delete issues to prevent foreign constraint failures. +# blank all biblios, biblioitems, and items. +$schema->resultset('Issue')->delete(); +$schema->resultset('Biblio')->delete(); + +my $location = 'My Location'; +my $builder = t::lib::TestBuilder->new; +my $library = $builder->build({ + source => 'Branch', +}); +my $itemtype = $builder->build({ + source => 'Itemtype', +}); + +# Create a biblio instance for testing +t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); +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 $rs = $schema->resultset('Biblioitem')->search({}); +my $iterator = Koha::BiblioUtils::Iterator->new($rs, items => 1 ); +my $record = $iterator->next(); +my $expected_tags = [ 100, 245, 952, 999 ]; +my @result_tags = map { $_->tag() } $record->field('...'); +my @sorted_tags = sort @result_tags; +is_deeply(\@sorted_tags,$expected_tags, "Got the same tags as expected"); + +$schema->storage->txn_rollback(); + +# Helper method to set up a Biblio. +sub get_biblio { + my ( $frameworkcode ) = @_; + $frameworkcode //= ''; + my $bib = MARC::Record->new(); + $bib->append_fields( + MARC::Field->new('100', ' ', ' ', a => 'Moffat, Steven'), + MARC::Field->new('245', ' ', ' ', a => 'Silence in the library'), + ); + my ($bibnum, $bibitemnum) = AddBiblio($bib, $frameworkcode); + return ($bibnum, $bibitemnum); +} diff --git a/t/db_dependent/Labels/t_Label.t b/t/db_dependent/Labels/t_Label.t new file mode 100644 index 0000000..4f60b07 --- /dev/null +++ b/t/db_dependent/Labels/t_Label.t @@ -0,0 +1,147 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Copyright (C) 2017 Mark Tompsett +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 3; +use t::lib::TestBuilder; + +use MARC::Record; +use MARC::Field; +use Data::Dumper; + +use C4::Biblio; +use C4::Items; +use C4::Labels::Layout; + +use Koha::Database; + +use_ok('C4::Labels::Label'); + +my $database = Koha::Database->new(); +my $schema = $database->schema(); +$schema->storage->txn_begin(); + +my $batch_id; +my ( $llx, $lly ) = ( 0, 0 ); +my $frameworkcode = q{}; + +## Setup Test +my $builder = t::lib::TestBuilder->new; + +# Add branch +my $branch_1 = $builder->build( { source => 'Branch' } )->{branchcode}; + +# Add categories +my $category_1 = $builder->build( { source => 'Category' } )->{categorycode}; + +# Add an item type +my $itemtype = + $builder->build( { source => 'Itemtype', value => { notforloan => undef } } ) + ->{itemtype}; + +C4::Context->set_userenv( undef, undef, undef, undef, undef, undef, $branch_1 ); + +# Create a helper biblio +my $bib = MARC::Record->new(); +my $title = 'Silence in the library'; +if ( C4::Context->preference('marcflavour') eq 'UNIMARC' ) { + $bib->append_fields( + MARC::Field->new( '600', q{}, '1', a => 'Moffat, Steven' ), + MARC::Field->new( '200', q{}, q{}, a => $title ), + ); +} +else { + $bib->append_fields( + MARC::Field->new( '100', q{}, q{}, a => 'Moffat, Steven' ), + MARC::Field->new( '245', q{}, q{}, a => $title ), + ); +} +my ($bibnum) = AddBiblio( $bib, $frameworkcode ); + +# Create a helper item instance for testing +my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = AddItem( + { + homebranch => $branch_1, + holdingbranch => $branch_1, + itype => $itemtype + }, + $bibnum +); + +# Modify item; setting barcode. +my $testbarcode = '97531'; +ModItem( { barcode => $testbarcode }, $bibnum, $itemnumber ); + +my $layout = C4::Labels::Layout->new( layout_name => 'TEST' ); + +my $dummy_template_values = { + creator => 'Labels', + profile_id => 0, + template_code => 'Avery 5160 | 1 x 2-5/8', + template_desc => '3 columns, 10 rows of labels', + page_width => 8.5, + page_height => 11, + label_width => 2.63, + label_height => 1, + top_text_margin => 0.139, + left_text_margin => 0.0417, + top_margin => 0.35, + left_margin => 0.23, + cols => 3, + rows => 10, + col_gap => 0.13, + row_gap => 0, + units => 'INCH', + template_stat => 1, +}; + +my $label = C4::Labels::Label->new( + batch_id => $batch_id, + item_number => $itemnumber, + llx => $llx, + lly => $lly, + width => $dummy_template_values->{'label_width'}, + height => $dummy_template_values->{'label_height'}, + top_text_margin => $dummy_template_values->{'top_text_margin'}, + left_text_margin => $dummy_template_values->{'left_text_margin'}, + barcode_type => $layout->get_attr('barcode_type'), + printing_type => 'BIB', + guidebox => $layout->get_attr('guidebox'), + oblique_title => $layout->get_attr('oblique_title'), + font => $layout->get_attr('font'), + font_size => $layout->get_attr('font_size'), + callnum_split => $layout->get_attr('callnum_split'), + justify => $layout->get_attr('text_justify'), + format_string => $layout->get_attr('format_string'), + text_wrap_cols => $layout->get_text_wrap_cols( + label_width => $dummy_template_values->{'label_width'}, + left_text_margin => $dummy_template_values->{'left_text_margin'} + ), +); + +my $label_text = $label->create_label(); +ok( defined $label_text, 'Label Text Value defined.' ); + +my $label_csv_data = $label->csv_data(); +ok( defined $label_csv_data, 'Label CSV Data defined' ); + +$schema->storage->txn_rollback(); + +1; diff --git a/t/db_dependent/Record/marcrecord2csv.t b/t/db_dependent/Record/marcrecord2csv.t index e5cf0d3..3f2c951 100644 --- a/t/db_dependent/Record/marcrecord2csv.t +++ b/t/db_dependent/Record/marcrecord2csv.t @@ -1,7 +1,7 @@ #!/usr/bin/perl; use Modern::Perl; -use Test::More tests => 11; +use Test::More tests => 12; use Test::MockModule; use MARC::Record; use MARC::Field; @@ -29,7 +29,11 @@ my $csv_content = q(Title=245$a|Author=245$c|Subject=650$a); my $csv_profile_id_1 = insert_csv_profile({ csv_content => $csv_content }); my $csv = Text::CSV::Encoded->new(); -my $csv_output = C4::Record::marcrecord2csv( $biblionumber, $csv_profile_id_1, 1, $csv ); +# Test bad biblionumber case +my $csv_output = C4::Record::marcrecord2csv( -1, $csv_profile_id_1, 1, $csv ); +ok(! defined $csv_output, 'Bad biblionumber gives undef as expected.'); + +$csv_output = C4::Record::marcrecord2csv( $biblionumber, $csv_profile_id_1, 1, $csv ); is( $csv_output, q[Title|Author|Subject "The art of computer programming,The art of another title"|"Donald E. Knuth.,Donald E. Knuth. II"|"Computer programming.,Computer algorithms." diff --git a/t/db_dependent/Search.t b/t/db_dependent/Search.t index 93e4875..4a913e6 100644 --- a/t/db_dependent/Search.t +++ b/t/db_dependent/Search.t @@ -22,6 +22,7 @@ use utf8; use YAML; use C4::Debug; +use C4::XSLT; require C4::Context; # work around spurious wide character warnings @@ -99,6 +100,11 @@ our $QueryFuzzy = 0; our $UseQueryParser = 0; our $SearchEngine = 'Zebra'; our $marcflavour = 'MARC21'; +our $htdocs = File::Spec->rel2abs(dirname($0)); +my @htdocs = split /\//, $htdocs; +$htdocs[-2] = 'koha-tmpl'; +$htdocs[-1] = 'opac-tmpl'; +$htdocs = join '/', @htdocs; our $contextmodule = new Test::MockModule('C4::Context'); $contextmodule->mock('preference', sub { my ($self, $pref) = @_; @@ -140,6 +146,38 @@ $contextmodule->mock('preference', sub { return ''; } elsif ($pref eq 'template') { return 'prog'; + } elsif ($pref eq 'OPACXSLTResultsDisplay') { + return C4::XSLT::_get_best_default_xslt_filename($htdocs, 'bootstrap','en',$marcflavour . 'slim2OPACResults.xsl'); + } elsif ($pref eq 'BiblioDefaultView') { + return 'normal'; + } elsif ($pref eq 'IdRef') { + return '0'; + } elsif ($pref eq 'IntranetBiblioDefaultView') { + return 'normal'; + } elsif ($pref eq 'OPACBaseURL') { + return 'http://library.mydnsname.org'; + } elsif ($pref eq 'OPACResultsLibrary') { + return 'homebranch'; + } elsif ($pref eq 'OpacSuppression') { + return '0'; + } elsif ($pref eq 'OPACURLOpenInNewWindow') { + return '0'; + } elsif ($pref eq 'TraceCompleteSubfields') { + return '0'; + } elsif ($pref eq 'TraceSubjectSubdivisions') { + return '0'; + } elsif ($pref eq 'TrackClicks') { + return '0'; + } elsif ($pref eq 'URLLinkText') { + return q{}; + } elsif ($pref eq 'UseAuthoritiesForTracings') { + return '1'; + } elsif ($pref eq 'UseControlNumber') { + return '0'; + } elsif ($pref eq 'UseICU') { + return '0'; + } elsif ($pref eq 'viewISBD') { + return '1'; } else { warn "The syspref $pref was requested but I don't know what to say; this indicates that the test requires updating" unless $pref =~ m/(XSLT|item|branch|holding|image)/i; -- 2.1.4