Bugzilla – Attachment 134068 Details for
Bug 29697
Replace GetMarcBiblio occurrences with $biblio->metadata->record
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 29697: Remove GetHiddenItemnumbers
Bug-29697-Remove-GetHiddenItemnumbers.patch (text/plain), 17.29 KB, created by
Jonathan Druart
on 2022-04-27 14:07:00 UTC
(
hide
)
Description:
Bug 29697: Remove GetHiddenItemnumbers
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2022-04-27 14:07:00 UTC
Size:
17.29 KB
patch
obsolete
>From b6b0dd1396234876d1ad07d739e4fe6d25d17474 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 21 Jan 2022 15:06:54 +0100 >Subject: [PATCH] Bug 29697: Remove GetHiddenItemnumbers > >--- > C4/Biblio.pm | 8 ++-- > C4/Items.pm | 60 ------------------------ > C4/Search.pm | 15 ++---- > opac/opac-basket.pl | 32 ++++--------- > opac/opac-detail.pl | 29 ++++++------ > opac/opac-search.pl | 2 +- > opac/opac-tags.pl | 19 ++++---- > t/db_dependent/Items.t | 101 +---------------------------------------- > 8 files changed, 40 insertions(+), 226 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index e384eda8f40..44321e8dbca 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -100,7 +100,7 @@ use C4::Charset qw( > ); > use C4::Linker; > use C4::OAI::Sets; >-use C4::Items qw( GetHiddenItemnumbers GetMarcItem ); >+use C4::Items qw( GetMarcItem ); > > use Koha::Logger; > use Koha::Caches; >@@ -2516,15 +2516,13 @@ sub EmbedItemsInMarcBiblio { > } > push @items, { itemnumber => $itemnumber, item => $item }; > } >- 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; >+ my @itemnumbers = Koha::Items->search( { itemnumber => $itemnumbers } ) >+ ->filter_by_visible_in_opac({ patron => })->get_column('itemnumber'); > foreach my $itemnumber ( map { $_->{itemnumber} } @items ) { > next if $hiddenitems{$itemnumber}; > my $item_marc = C4::Items::GetMarcItem( $biblionumber, $itemnumber ); >diff --git a/C4/Items.pm b/C4/Items.pm >index 7aa072fff89..70b96e8eb0c 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -38,7 +38,6 @@ BEGIN { > GetItemsLocationInfo > GetHostItemsInfo > get_hostitemnumbers_of >- GetHiddenItemnumbers > GetMarcItem > CartToShelf > GetAnalyticsCount >@@ -975,65 +974,6 @@ sub get_hostitemnumbers_of { > return @returnhostitemnumbers; > } > >-=head2 GetHiddenItemnumbers >- >- 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. Optionally takes a borcat parameter for certain borrower types >-to be excluded >- >-=cut >- >-sub GetHiddenItemnumbers { >- 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 don't hide anything for this borrower category >- } >- } >- } >- my @resultitems; >- >- my $hidingrules = C4::Context->yaml_preference('OpacHiddenItems'); >- >- return >- unless $hidingrules; >- >- my $dbh = C4::Context->dbh; >- >- # For each item >- foreach my $item (@$items) { >- >- # We check each rule >- foreach my $field (keys %$hidingrules) { >- my $val; >- if (exists $item->{$field}) { >- $val = $item->{$field}; >- } >- else { >- my $query = "SELECT $field from items where itemnumber = ?"; >- $val = $dbh->selectrow_array($query, undef, $item->{'itemnumber'}); >- } >- $val = '' unless defined $val; >- >- # If the results matches the values in the yaml file >- if (any { $val eq $_ } @{$hidingrules->{$field}}) { >- >- # We add the itemnumber to the list >- push @resultitems, $item->{'itemnumber'}; >- >- # If at least one rule matched for an item, no need to test the others >- last; >- } >- } >- } >- return @resultitems; >-} >- > =head1 LIMITED USE FUNCTIONS > > The following functions, while part of the public API, >diff --git a/C4/Search.pm b/C4/Search.pm >index 85a02bf5a26..afe0c2d4581 100644 >--- a/C4/Search.pm >+++ b/C4/Search.pm >@@ -1802,21 +1802,16 @@ sub searchResults { > } > $item->{description} = $itemtypes{ $item->{itype} }{translated_description} if $item->{itype}; > >- # OPAC hidden items >+ # OPAC hidden items > if ($is_opac) { >- # hidden because lost >- if ($hidelostitems && $item->{itemlost}) { >+ # hidden based on OpacHiddenItems syspref or because lost >+ my $hi = Koha::Items->search( { itemnumber => $item->{itemnumber} } ) >+ ->filter_by_visible_in_opac({ patron => $search_context->{patron} }); >+ unless ( $hi->count ) { > push @hiddenitems, $item->{itemnumber}; > $hideatopac_count++; > next; > } >- # hidden based on OpacHiddenItems syspref >- my @hi = C4::Items::GetHiddenItemnumbers({ items=> [ $item ], borcat => $search_context->{category} }); >- if (scalar @hi) { >- push @hiddenitems, @hi; >- $hideatopac_count++; >- next; >- } > } > > my $hbranch = C4::Context->preference('StaffSearchResultsDisplayBranch'); >diff --git a/opac/opac-basket.pl b/opac/opac-basket.pl >index d22106089cc..978bd727bf9 100755 >--- a/opac/opac-basket.pl >+++ b/opac/opac-basket.pl >@@ -26,7 +26,7 @@ use C4::Biblio qw( > GetMarcSubjects > GetMarcUrls > ); >-use C4::Items qw( GetHiddenItemnumbers GetItemsInfo ); >+use C4::Items qw( GetItemsInfo ); > use C4::Circulation qw( GetTransfers ); > use C4::Auth qw( get_template_and_user ); > use C4::Output qw( output_html_with_http_headers ); >@@ -63,12 +63,7 @@ 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 $patron = Koha::Patrons->find($borrowernumber); >- $borcat = $patron ? $patron->categorycode : $borcat; >-} >+my $logged_in_user = Koha::Patrons->find($borrowernumber); > > my $record_processor = Koha::RecordProcessor->new({ filters => 'ViewPolicy' }); > my $rules = C4::Context->yaml_preference('OpacHiddenItems'); >@@ -95,27 +90,16 @@ foreach my $biblionumber ( @bibs ) { > my $marcseriesarray = GetMarcSeries ($record,$marcflavour); > my $marcurlsarray = GetMarcUrls ($record,$marcflavour); > >- # 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 $biblio->hidden_in_opac({ rules => $rules }); > >- # copy the visible ones into the items array. >- my @items; >- foreach my $item (@all_items) { >- >- # next if item is hidden >- next if grep { $item->{itemnumber} eq $_ } @hidden_items ; >- >- my $reserve_status = C4::Reserves::GetReserveStatus($item->{itemnumber}); >- if( $reserve_status eq "Waiting"){ $item->{'waiting'} = 1; } >- if( $reserve_status eq "Reserved"){ $item->{'onhold'} = 1; } >- push @items, $item; >+ # grab all the items... >+ my @items = $biblio->items->filter_by_visible_in_opac({ patron => $logged_in_user })->unblessed; >+ foreach my $item (@items) { >+ my $reserve_status = C4::Reserves::GetReserveStatus($item->{itemnumber}); >+ if( $reserve_status eq "Waiting"){ $item->{'waiting'} = 1; } >+ if( $reserve_status eq "Reserved"){ $item->{'onhold'} = 1; } > } > > my $hasauthors = 0; >diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl >index 286e68b9cd1..a99dd3f7ac5 100755 >--- a/opac/opac-detail.pl >+++ b/opac/opac-detail.pl >@@ -45,7 +45,7 @@ use C4::Biblio qw( > GetMarcSubjects > GetMarcUrls > ); >-use C4::Items qw( GetHiddenItemnumbers GetItemsInfo ); >+use C4::Items qw( GetItemsInfo ); > use C4::Circulation qw( GetTransfers ); > use C4::Tags qw( get_tags ); > use C4::XISBN qw( get_xisbns ); >@@ -108,7 +108,7 @@ if( $specific_item ) { > @all_items = grep { $_->{itemnumber} == $query->param('itemnumber') } @all_items; > $template->param( specific_item => 1 ); > } >-my @hiddenitems; >+my @items_to_show; > my $patron = Koha::Patrons->find( $borrowernumber ); > > my $biblio = Koha::Biblios->find( $biblionumber ); >@@ -126,8 +126,8 @@ unless ( $patron and $patron->category->override_hidden_items ) { > exit; > } > if ( scalar @all_items >= 1 ) { >- push @hiddenitems, >- GetHiddenItemnumbers( { items => \@all_items, borcat => $patron ? $patron->categorycode : undef } ); >+ @items_to_show = Koha::Items->search( { itemnumbers => [ map { $_->{itemnumber} } @all_items ] } ) >+ ->filter_by_visible_in_opac( { patron => $patron } ); > } > } > >@@ -249,8 +249,8 @@ if ($session->param('busc')) { > my $hits; > my @newresults; > my $search_context = { >- 'interface' => 'opac', >- 'category' => ($patron) ? $patron->categorycode : q{} >+ interface => 'opac', >+ patron => $patron, > }; > for (my $i=0;$i<@servers;$i++) { > my $server = $servers[$i]; >@@ -513,18 +513,17 @@ if ( C4::Context->preference('EasyAnalyticalRecords') ) { > my @items; > > # Are there items to hide? >-my $hideitems; >-$hideitems = 1 if C4::Context->preference('hidelostitems') or scalar(@hiddenitems) > 0; >- > # Hide items >-if ($hideitems) { >+if ( @items_to_show != @all_items ) { > for my $itm (@all_items) { >- if ( C4::Context->preference('hidelostitems') ) { >- push @items, $itm unless $itm->{itemlost} or any { $itm->{'itemnumber'} eq $_ } @hiddenitems; >- } else { >- push @items, $itm unless any { $itm->{'itemnumber'} eq $_ } @hiddenitems; >+ next unless any { $itm->{itemnumber} eq $_ } @items_to_show; >+ if ( C4::Context->preference('hidelostitems') ) { >+ push @items, $itm unless $itm->{itemlost}; >+ } >+ else { >+ push @items, $itm; >+ } > } >-} > } else { > # Or not > @items = @all_items; >diff --git a/opac/opac-search.pl b/opac/opac-search.pl >index 15fd6e52d25..e2fc5d2246f 100755 >--- a/opac/opac-search.pl >+++ b/opac/opac-search.pl >@@ -587,7 +587,7 @@ my @sup_results_array; > my $search_context = {}; > $search_context->{'interface'} = 'opac'; > if (C4::Context->preference('OpacHiddenItemsExceptions')){ >- $search_context->{'category'} = $patron ? $patron->categorycode : q{}; >+ $search_context->{patron} = $patron; > } > > my $variables = { anonymous_session => ($borrowernumber) ? 0 : 1 }; >diff --git a/opac/opac-tags.pl b/opac/opac-tags.pl >index 515f4ccd2e4..4e16f8c72a2 100755 >--- a/opac/opac-tags.pl >+++ b/opac/opac-tags.pl >@@ -34,13 +34,14 @@ use Modern::Perl; > > use CGI qw ( -utf8 ); > use CGI::Cookie; # need to check cookies before having CGI parse the POST request >+use Array::Utils qw( array_minus ); > > use C4::Auth qw( check_cookie_auth get_template_and_user ); > use C4::Context; > use C4::Output qw( output_with_http_headers is_ajax output_html_with_http_headers ); > use C4::Scrubber; > use C4::Biblio qw( GetMarcBiblio ); >-use C4::Items qw( GetHiddenItemnumbers GetItemsInfo ); >+use C4::Items qw( GetItemsInfo ); > use C4::Tags qw( > add_tag > get_approval_rows >@@ -226,7 +227,6 @@ if ($is_ajax) { > > my $results = []; > my $my_tags = []; >-my $borcat = q{}; > > if ($loggedinuser) { > my $patron = Koha::Patrons->find( { borrowernumber => $loggedinuser } ); >@@ -258,15 +258,12 @@ if ($loggedinuser) { > opac => 1, > borcat => $borcat }); > next unless $record; >- my $hidden_items = undef; >- my @hidden_itemnumbers; >- my @all_items; >+ my @hidden_items; > if ($should_hide) { >- @all_items = GetItemsInfo( $tag->{biblionumber} ); >- @hidden_itemnumbers = GetHiddenItemnumbers({ >- items => \@all_items, >- borcat => $borcat }); >- $hidden_items = \@hidden_itemnumbers; >+ my $items = $biblio->items; >+ my @all_itemnumbers = $items->get_column('itemnumber'); >+ my @items_to_show = $items->filter_by_visible_in_opac({ opac => 1, patron => $patron }); >+ @hidden_items = array_minus( @all_itemnumbers, @items_to_show ); > } > next > if ( >@@ -294,7 +291,7 @@ if ($loggedinuser) { > record => $record, > xsl_syspref => 'OPACXSLTResultsDisplay', > fix_amps => 1, >- hidden_items => $hidden_items, >+ hidden_items => \@hidden_items, > xslt_variables => $variables > } > ); >diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t >index ab0fa8e66e6..7dbf838b895 100755 >--- a/t/db_dependent/Items.t >+++ b/t/db_dependent/Items.t >@@ -19,7 +19,7 @@ use Modern::Perl; > use Data::Dumper; > > use MARC::Record; >-use C4::Items qw( ModItemTransfer GetHiddenItemnumbers GetItemsInfo SearchItems AddItemFromMarc ModItemFromMarc get_hostitemnumbers_of Item2Marc ); >+use C4::Items qw( ModItemTransfer GetItemsInfo SearchItems AddItemFromMarc ModItemFromMarc get_hostitemnumbers_of Item2Marc ); > use C4::Biblio qw( GetMarcFromKohaField EmbedItemsInMarcBiblio GetMarcBiblio AddBiblio ); > use C4::Circulation qw( AddIssue ); > use Koha::Items; >@@ -190,105 +190,6 @@ subtest 'ModItemTransfer tests' => sub { > $schema->storage->txn_rollback; > }; > >-subtest 'GetHiddenItemnumbers tests' => sub { >- >- plan tests => 11; >- >- # This sub is controlled by the OpacHiddenItems system preference. >- >- $schema->storage->txn_begin; >- >- my $builder = t::lib::TestBuilder->new; >- my $library1 = $builder->build({ >- source => 'Branch', >- }); >- >- my $library2 = $builder->build({ >- source => 'Branch', >- }); >- my $itemtype = $builder->build({ >- source => 'Itemtype', >- }); >- >- # Create a new biblio >- t::lib::Mocks::mock_preference('marcflavour', 'MARC21'); >- my $biblio = $builder->build_sample_biblio(); >- >- # Add two items >- my $item1_itemnumber = $builder->build_sample_item( >- { >- biblionumber => $biblio->biblionumber, >- library => $library1->{branchcode}, >- withdrawn => 1, >- itype => $itemtype->{itemtype} >- } >- )->itemnumber; >- my $item2_itemnumber = $builder->build_sample_item( >- { >- biblionumber => $biblio->biblionumber, >- library => $library2->{branchcode}, >- withdrawn => 0, >- itype => $itemtype->{itemtype} >- } >- )->itemnumber; >- my $opachiddenitems; >- my @itemnumbers = ($item1_itemnumber,$item2_itemnumber); >- my @hidden; >- my @items; >- push @items, Koha::Items->find( $item1_itemnumber )->unblessed; >- push @items, Koha::Items->find( $item2_itemnumber )->unblessed; >- >- # Empty OpacHiddenItems >- t::lib::Mocks::mock_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 => \@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 => \@items } ); >- ok( scalar @hidden == 1, "Only one hidden item"); >- is( $hidden[0], $item1_itemnumber, "withdrawn=1 is hidden"); >- >- # One variable, two values >- $opachiddenitems = " >- withdrawn: [1,0]"; >- t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); >- @hidden = GetHiddenItemnumbers( { items => \@items } ); >- ok( scalar @hidden == 2, "Two items hidden"); >- is_deeply( \@hidden, \@itemnumbers, "withdrawn=1 and withdrawn=0 hidden"); >- >- # Two variables, a value each >- $opachiddenitems = " >- withdrawn: [1] >- homebranch: [$library2->{branchcode}] >- "; >- t::lib::Mocks::mock_preference( 'OpacHiddenItems', $opachiddenitems ); >- @hidden = GetHiddenItemnumbers( { items => \@items } ); >- 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 } ); >- ok( scalar @hidden == 0, "Empty items list, no item hidden"); >- >- $schema->storage->txn_rollback; >-}; >- > subtest 'GetItemsInfo tests' => sub { > > plan tests => 9; >-- >2.25.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 29697
:
129757
|
129758
|
129759
|
129760
|
129761
|
131868
|
131869
|
131870
|
131871
|
131872
|
131873
|
134066
|
134067
|
134068
|
134069
|
134070
|
134071
|
135574
|
135575
|
135576
|
135577
|
135578
|
135579
|
135580
|
135662
|
135663
|
135664
|
135665
|
135666
|
135667
|
135668
|
135669
|
135670
|
135671
|
135672
|
135673
|
135797
|
135798
|
135799
|
135800
|
135801
|
135802
|
135978
|
135979
|
135980
|
135981
|
135982
|
137835
|
137836
|
137837
|
137838
|
137839
|
137840
|
137894
|
137918
|
137919
|
137920
|
137921
|
137922
|
137923
|
137924
|
137948
|
138057
|
138076
|
138077
|
138078
|
138079
|
138080
|
138081
|
138089
|
138090
|
138804
|
139127
|
140007
|
140008