gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories 

Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This commit is contained in:
Chris Cormack 2014-11-06 10:34:18 +13:00 committed by Nick Clemens
parent 12ed04eabb
commit 5e4e10c4ca
11 changed files with 83 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
C4/Items.pm
View File

@ -1278,16 +1278,25 @@ sub get_hostitemnumbers_of {
=head2 GetHiddenItemnumbers
my @itemnumbers_to_hide = GetHiddenItemnumbers(@items);
my @itemnumbers_to_hide = GetHiddenItemnumbers({ items => \@items, borcat => $category });
Given a list of items it checks which should be hidden from the OPAC given
the current configuration. Returns a list of itemnumbers corresponding to
those that should be hidden.
those that should be hidden. Optionally takes a borcat parameter for certain borrower types
to be excluded
=cut
sub GetHiddenItemnumbers {
my (@items) = @_;
my $params = shift;
my $items = $params->{items};
if (my $exceptions = C4::Context->preference('OpacHiddenItemsExceptions') and $params->{'borcat'}){
foreach my $except (split(/\|/, $exceptions)){
if ($params->{'borcat'} eq $except){
return; # we dont hide anything for this borrower category
}
}
}
my @resultitems;
my $yaml = C4::Context->preference('OpacHiddenItems');
@ -1304,7 +1313,7 @@ sub GetHiddenItemnumbers {
my $dbh = C4::Context->dbh;
# For each item
foreach my $item (@items) {
foreach my $item (@$items) {
# We check each rule
foreach my $field (keys %$hidingrules) {

13
C4/Search.pm
View File

@ -1856,9 +1856,9 @@ sub searchResults {
require C4::Items;
$search_context = 'opac' if !$search_context || $search_context ne 'intranet';
$search_context->{'interface'} = 'opac' if !$search_context->{'interface'} || $search_context->{'interface'} ne 'intranet';
my ($is_opac, $hidelostitems);
if ($search_context eq 'opac') {
if ($search_context->{'interface'} eq 'opac') {
$hidelostitems = C4::Context->preference('hidelostitems');
$is_opac = 1;
}
@ -1948,6 +1948,7 @@ sub searchResults {
# add imageurl to itemtype if there is one
$oldbiblio->{imageurl} = getitemtypeimagelocation( $search_context, $itemtypes{ $oldbiblio->{itemtype} }->{imageurl} );
$oldbiblio->{'authorised_value_images'} = ($search_context->{'interface'} eq 'opac' && C4::Context->preference('AuthorisedValueImages')) || ($search_context->{'interface'} eq 'intranet' && C4::Context->preference('StaffAuthorisedValueImages')) ? C4::Items::get_authorised_value_images( C4::Biblio::get_biblio_authorised_values( $oldbiblio->{'biblionumber'}, $marcrecord ) ) : [];
$oldbiblio->{normalized_upc} = GetNormalizedUPC( $marcrecord,$marcflavour);
$oldbiblio->{normalized_ean} = GetNormalizedEAN( $marcrecord,$marcflavour);
$oldbiblio->{normalized_oclc} = GetNormalizedOCLCNumber($marcrecord,$marcflavour);
@ -2079,7 +2080,7 @@ sub searchResults {
next;
}
# hidden based on OpacHiddenItems syspref
my @hi = C4::Items::GetHiddenItemnumbers($item);
my @hi = C4::Items::GetHiddenItemnumbers({ items=> [ $item ], borcat => $search_context->{category} });
if (scalar @hi) {
push @hiddenitems, @hi;
$hideatopac_count++;
@ -2208,7 +2209,7 @@ sub searchResults {
$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, $itemtypes{ $item->{itype} }->{imageurl} );
$other_items->{$key}->{imageurl} = getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype} }->{imageurl} );
}
# item is available
else {
@ -2219,7 +2220,7 @@ sub searchResults {
$available_items->{$prefix}->{$_} = $item->{$_};
}
$available_items->{$prefix}->{location} = $shelflocations->{ $item->{location} };
$available_items->{$prefix}->{imageurl} = getitemtypeimagelocation( $search_context, $itemtypes{ $item->{itype} }->{imageurl} );
$available_items->{$prefix}->{imageurl} = getitemtypeimagelocation( $search_context->{'interface'}, $itemtypes{ $item->{itype} }->{imageurl} );
}
}
} # notforloan, item level and biblioitem level
@ -2245,9 +2246,9 @@ sub searchResults {
# XSLT processing of some stuff
# we fetched the sysprefs already before the loop through all retrieved record!
my $interface = $search_context->{'interface'} eq 'opac' ? 'OPAC' : '';
if (!$scan && $xslfile) {
$oldbiblio->{XSLTResultsRecord} = XSLTParse4Display($oldbiblio->{biblionumber}, $marcrecord, $xslsyspref, 1, \@hiddenitems, $sysxml, $xslfile, $lang);
# the last parameter tells Koha to clean up the problematic ampersand entities that Zebra outputs
}
# if biblio level itypes are used and itemtype is notforloan, it can't be reserved either

2
catalogue/search.pl
View File

@ -541,7 +541,7 @@ for (my $i=0;$i<@servers;$i++) {
if ($server =~/biblioserver/) { # this is the local bibliographic server
my $hits = $results_hashref->{$server}->{"hits"} // 0;
my $page = $cgi->param('page') || 0;
my @newresults = searchResults('intranet', $query_desc, $hits, $results_per_page, $offset, $scan,
my @newresults = searchResults({ 'interface' => 'intranet' }, $query_desc, $hits, $results_per_page, $offset, $scan,
$results_hashref->{$server}->{"RECORDS"});
$total = $total + $hits;

5
cataloguing/addbooks.pl
View File

@ -93,7 +93,10 @@ if ($query) {
# format output
# SimpleSearch() give the results per page we want, so 0 offet here
my $total = @{$marcresults};
my @newresults = searchResults( 'intranet', $query, $total, $results_per_page, 0, 0, $marcresults );
my @newresults = searchResults( {'interface' => 'intranet'}, $query, $total, $results_per_page, 0, 0, $marcresults );
foreach my $line (@newresults) {
if ( not exists $line->{'size'} ) { $line->{'size'} = "" }
}
$template->param(
total => $total_hits,
query => $query,

3
koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
View File

@ -589,6 +589,9 @@ OPAC:
type: textarea
class: code
- Define custom rules to hide specific items from search and view on the OPAC. How to write these rules is documented on the <a href="http://wiki.koha-community.org/wiki/OpacHiddenItems" target="_blank">Koha wiki</a>.
-
- pref: OpacHiddenItemsExceptions
- List of borrower categories, separated by |, that can see items otherwise hidden by <tt>OpacHiddenItems</tt>
-
- pref: OpacAllowPublicListCreation
default: 1

10
opac/opac-ISBDdetail.pl
View File

@ -85,7 +85,15 @@ my $marcflavour = C4::Context->preference("marcflavour");
my @items = GetItemsInfo($biblionumber);
if (scalar @items >= 1) {
my @hiddenitems = GetHiddenItemnumbers(@items);
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

9
opac/opac-MARCdetail.pl
View File

@ -85,7 +85,14 @@ if ( ! $record ) {
my @all_items = GetItemsInfo($biblionumber);
my @items2hide;
if (scalar @all_items >= 1) {
push @items2hide, GetHiddenItemnumbers(@all_items);
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");

15
opac/opac-detail.pl
View File

@ -83,8 +83,17 @@ $biblionumber = int($biblionumber);
my @all_items = GetItemsInfo($biblionumber);
my @hiddenitems;
if (scalar @all_items >= 1) {
push @hiddenitems, GetHiddenItemnumbers(@all_items);
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 @hiddenitems,
GetHiddenItemnumbers( { items => \@all_items, borcat => $borcat } );
if (scalar @hiddenitems == scalar @all_items ) {
print $query->redirect("/cgi-bin/koha/errors/404.pl"); # escape early
@ -233,7 +242,7 @@ if ($session->param('busc')) {
for (my $i=0;$i<@servers;$i++) {
my $server = $servers[$i];
$hits = $results_hashref->{$server}->{"hits"};
@newresults = searchResults('opac', '', $hits, $results_per_page, $offset, $arrParamsBusc->{'scan'}, $results_hashref->{$server}->{"RECORDS"});
@newresults = searchResults({ 'interface' => 'opac' }, '', $hits, $results_per_page, $offset, $arrParamsBusc->{'scan'}, $results_hashref->{$server}->{"RECORDS"});
}
return \@newresults;
}#searchAgain

13
opac/opac-search.pl
View File

@ -51,6 +51,7 @@ use C4::Koha;
use C4::Tags qw(get_tags);
use C4::SocialData;
use C4::External::OverDrive;
use C4::Borrowers qw(GetMember);
use Koha::ItemTypes;
use Koha::Ratings;
@ -621,6 +622,14 @@ if (not $tag and ( $@ || $error)) {
# At this point, each server has given us a result set
# now we build that set for template display
my @sup_results_array;
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'};
}
for (my $i=0;$i<@servers;$i++) {
my $server = $servers[$i];
if ($server && $server =~/biblioserver/) { # this is the local bibliographic server
@ -632,12 +641,12 @@ for (my $i=0;$i<@servers;$i++) {
# because pazGetRecords handles retieving only the records
# we want as specified by $offset and $results_per_page,
# we need to set the offset parameter of searchResults to 0
my @group_results = searchResults( 'opac', $query_desc, $group->{'group_count'},$results_per_page, 0, $scan,
my @group_results = searchResults( $search_context, $query_desc, $group->{'group_count'},$results_per_page, 0, $scan,
$group->{"RECORDS"});
push @newresults, { group_label => $group->{'group_label'}, GROUP_RESULTS => \@group_results };
}
} else {
@newresults = searchResults('opac', $query_desc, $hits, $results_per_page, $offset, $scan,
@newresults = searchResults( $search_context, $query_desc, $hits, $results_per_page, $offset, $scan,
$results_hashref->{$server}->{"RECORDS"});
}
$hits = 0 unless @newresults;

17
t/db_dependent/Items.t
View File

@ -198,20 +198,20 @@ subtest 'GetHiddenItemnumbers tests' => sub {
push @items, GetItem( $item2_itemnumber );
# Empty OpacHiddenItems
t::lib::Mocks::mock_preference('OpacHiddenItems','');
ok( !defined( GetHiddenItemnumbers( @items ) ),
C4::Context->set_preference('OpacHiddenItems','');
ok( !defined( GetHiddenItemnumbers( { items => \@items } ) ),
"Hidden items list undef if OpacHiddenItems empty");
# Blank spaces
t::lib::Mocks::mock_preference('OpacHiddenItems',' ');
ok( scalar GetHiddenItemnumbers( @items ) == 0,
C4::Context->set_preference('OpacHiddenItems',' ');
ok( scalar GetHiddenItemnumbers( { items => \@items } ) == 0,
"Hidden items list empty if OpacHiddenItems only contains blanks");
# One variable / value
$opachiddenitems = "
withdrawn: [1]";
t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems );
@hidden = GetHiddenItemnumbers( @items );
@hidden = GetHiddenItemnumbers( { items => \@items } );
ok( scalar @hidden == 1, "Only one hidden item");
is( $hidden[0], $item1_itemnumber, "withdrawn=1 is hidden");
@ -219,7 +219,8 @@ subtest 'GetHiddenItemnumbers tests' => sub {
$opachiddenitems = "
withdrawn: [1,0]";
t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems );
@hidden = GetHiddenItemnumbers( @items );
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");
@ -229,13 +230,13 @@ subtest 'GetHiddenItemnumbers tests' => sub {
homebranch: [$library2->{branchcode}]
";
t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems );
@hidden = GetHiddenItemnumbers( @items );
@hidden = GetHiddenItemnumbers( { items => \@items } );
ok( scalar @hidden == 2, "Two items hidden");
is_deeply( \@hidden, \@itemnumbers, "withdrawn=1 and homebranch library2 hidden");
# Valid OpacHiddenItems, empty list
@items = ();
@hidden = GetHiddenItemnumbers( @items );
@hidden = GetHiddenItemnumbers( { items => \@items } );
ok( scalar @hidden == 0, "Empty items list, no item hidden");
$schema->storage->txn_rollback;

12
t/db_dependent/Search.t
View File

@ -373,7 +373,7 @@ ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'
($error, $results_hashref, $facets_loop) = getRecords($query,$simple_query,[ ], [ 'biblioserver' ],20,0,undef,\%branches,\%itemtypes,$query_type,0);
is($results_hashref->{biblioserver}->{hits}, 19, "getRecords generated keyword search for 'salud' matched right number of records");
my @newresults = searchResults('opac', $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 18, 0, 0,
my @newresults = searchResults({'interface' => 'opac'}, $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 18, 0, 0,
$results_hashref->{'biblioserver'}->{"RECORDS"});
is(scalar @newresults,18, "searchResults returns requested number of hits");
@ -455,7 +455,7 @@ ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'
($error, $results_hashref, $facets_loop) = getRecords($query,$simple_query,[ ], [ 'biblioserver' ],20,0,undef,\%branches,\%itemtypes,$query_type,0);
is($results_hashref->{biblioserver}->{hits}, 26, "getRecords generated availability-limited search matched right number of records");
@newresults = searchResults('opac', $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 17, 0, 0,
@newresults = searchResults({'interface'=>'opac'}, $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 17, 0, 0,
$results_hashref->{'biblioserver'}->{"RECORDS"});
my $allavailable = 'true';
foreach my $result (@newresults) {
@ -593,7 +593,7 @@ ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'
# Let's just test a few other bits and bobs, just for fun
($error, $results_hashref, $facets_loop) = getRecords("Godzina pąsowej róży","Godzina pąsowej róży",[ ], [ 'biblioserver' ],20,0,undef,\%branches,\%itemtypes,$query_type,0);
@newresults = searchResults('intranet', $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 17, 0, 0,
@newresults = searchResults({'interface'=>'intranet'}, $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 17, 0, 0,
$results_hashref->{'biblioserver'}->{"RECORDS"});
is($newresults[0]->{'alternateholdings_count'}, 1, 'Alternate holdings filled in correctly');
@ -612,7 +612,7 @@ ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'
});
($error, $results_hashref, $facets_loop) = getRecords("TEST12121212","TEST12121212",[ ], [ 'biblioserver' ],20,0,undef,\%branches,\%itemtypes,$query_type,0);
@newresults = searchResults('intranet', $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 17, 0, 0,
@newresults = searchResults({'interface'=>'intranet'}, $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 17, 0, 0,
$results_hashref->{'biblioserver'}->{"RECORDS"});
ok(!exists($newresults[0]->{norequests}), 'presence of a transit does not block hold request action (bug 10741)');
@ -620,7 +620,7 @@ ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'
( undef, $results_hashref, $facets_loop ) =
getRecords('ti:punctuation', 'punctuation', [], [ 'biblioserver' ], '19', 0, undef, \%branches, \%itemtypes, 'ccl', undef);
is($results_hashref->{biblioserver}->{hits}, 1, "search for ti:punctuation returned expected number of records");
warning_like { @newresults = searchResults('intranet', $query_desc,
warning_like { @newresults = searchResults({'intranet' => 'intranet'}, $query_desc,
$results_hashref->{'biblioserver'}->{'hits'}, 20, 0, 0,
$results_hashref->{'biblioserver'}->{"RECORDS"}) }
qr/^ERROR DECODING RECORD - Tag "50%" is not a valid tag/,
@ -763,7 +763,7 @@ ok(MARC::Record::new_from_xml($results_hashref->{biblioserver}->{RECORDS}->[0],'
( undef, $results_hashref, $facets_loop ) =
getRecords('ti:marc the large record', '', [], [ 'biblioserver' ], '20', 0, undef, \%branches, \%itemtypes, 'ccl', undef);
is($results_hashref->{biblioserver}->{hits}, 1, "Can do a search that retrieves an over-large bib record (bug 11096)");
@newresults = searchResults('opac', $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 10, 0, 0,
@newresults = searchResults({'interface' =>'opac'}, $query_desc, $results_hashref->{'biblioserver'}->{'hits'}, 10, 0, 0,
$results_hashref->{'biblioserver'}->{"RECORDS"});
is($newresults[0]->{title}, 'Marc the Large Record', 'Able to render the title for over-large bib record (bug 11096)');
is($newresults[0]->{biblionumber}, '300', 'Over-large bib record has the correct biblionumber (bug 11096)');