diff --git a/C4/Context.pm b/C4/Context.pm index db69cad9c8..ef91190a7b 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -983,6 +983,26 @@ sub is_internal_PSGI_request { return $is_internal; } +=head3 is_opac_suppressed + + return if C4::Context->is_opac_suppressed; + +is_opac_suppressed returns true if and only if OPAC suppression is enabled for +the current request, based on the sysprefs OpacSuppression and +OpacSuppressionByIPRange. + +=cut + +sub is_opac_suppressed { + return 0 unless C4::Context->preference('OpacSuppression'); + + my $IPAddress = $ENV{'REMOTE_ADDR'}; + my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); + return 1 unless $IPRange; + + return $IPAddress !~ /^$IPRange/; +} + __END__ =head1 ENVIRONMENT diff --git a/C4/Output.pm b/C4/Output.pm index 3b87674872..fa4943fd18 100644 --- a/C4/Output.pm +++ b/C4/Output.pm @@ -438,8 +438,7 @@ sub parametrized_url { =item redirect_if_opac_suppressed - redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); + redirect_if_opac_suppressed( $query, $biblio ); For a given I object, it handles redirection if it is suppressed from the OPAC. @@ -456,20 +455,9 @@ sub redirect_if_opac_suppressed { } else { $redirect_url = "/cgi-bin/koha/errors/404.pl"; } - if ( $biblio->opac_suppressed() ) { - - # if OPAC suppression by IP address - if ( C4::Context->preference('OpacSuppressionByIPRange') ) { - my $IPAddress = $ENV{'REMOTE_ADDR'}; - my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); - if ( $IPAddress !~ /^$IPRange/ ) { - print $query->redirect($redirect_url); - C4::Auth::safe_exit(); - } - } else { - print $query->redirect($redirect_url); - C4::Auth::safe_exit(); - } + if ( C4::Context->is_opac_suppressed && $biblio->opac_suppressed() ) { + print $query->redirect($redirect_url); + C4::Auth::safe_exit(); } } diff --git a/Koha/OAI/Server/Repository.pm b/Koha/OAI/Server/Repository.pm index 24a2cdb8c5..0df697310f 100644 --- a/Koha/OAI/Server/Repository.pm +++ b/Koha/OAI/Server/Repository.pm @@ -216,10 +216,7 @@ sub get_biblio_marcxml { } if ($record) { - # Check if the bibliographic record is suppressed in OPAC - if ( C4::Context->preference('OpacSuppression') && $biblio->opac_suppressed ) { - return; - } + return if C4::Context->is_opac_suppressed && $biblio->opac_suppressed; my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {}; if ( $biblio->hidden_in_opac( { rules => $rules } ) ) { diff --git a/Koha/REST/V1/Biblios.pm b/Koha/REST/V1/Biblios.pm index f8e7713da9..5a95b5c94a 100644 --- a/Koha/REST/V1/Biblios.pm +++ b/Koha/REST/V1/Biblios.pm @@ -163,7 +163,7 @@ sub get_public { my $patron = $c->stash('koha.user'); # Check if the bibliographic record is suppressed in OPAC - if ( C4::Context->preference('OpacSuppression') && $biblio->opac_suppressed ) { + if ( C4::Context->is_opac_suppressed && $biblio->opac_suppressed ) { return $c->render_resource_not_found("Bibliographic record"); } diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl index 34e32ec1d6..86cf48b390 100755 --- a/opac/opac-ISBDdetail.pl +++ b/opac/opac-ISBDdetail.pl @@ -73,8 +73,7 @@ if ( !$biblio ) { } # If record should be suppressed, handle it early -redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); +redirect_if_opac_suppressed( $query, $biblio ); #open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl index 6cd85287b2..a53e4053df 100755 --- a/opac/opac-MARCdetail.pl +++ b/opac/opac-MARCdetail.pl @@ -97,8 +97,7 @@ if ( !$biblio ) { } # If record should be suppressed, handle it early -redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); +redirect_if_opac_suppressed( $query, $biblio ); unless ( $patron and $patron->category->override_hidden_items ) { diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl index ec9f2b7818..064eab3205 100755 --- a/opac/opac-detail.pl +++ b/opac/opac-detail.pl @@ -108,8 +108,7 @@ unless ( $biblio && $record ) { } # If record should be suppressed, handle it early -redirect_if_opac_suppressed( $query, $biblio ) - if C4::Context->preference('OpacSuppression'); +redirect_if_opac_suppressed( $query, $biblio ); my $items = $biblio->items->search_ordered; if ($specific_item) { diff --git a/opac/opac-search.pl b/opac/opac-search.pl index 69dafb2d4a..78cec11103 100755 --- a/opac/opac-search.pl +++ b/opac/opac-search.pl @@ -502,18 +502,7 @@ my $weight_search = $cgi->param('weight_search_submitted') # Define some global variables my ( $error, $query, $simple_query, $query_cgi, $query_desc, $limit, $limit_cgi, $limit_desc, $query_type ); -my $suppress = 0; -if ( C4::Context->preference('OpacSuppression') ) { - - # OPAC suppression by IP address - if ( C4::Context->preference('OpacSuppressionByIPRange') ) { - my $IPAddress = $ENV{'REMOTE_ADDR'}; - my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); - $suppress = ( $IPAddress !~ /^$IPRange/ ); - } else { - $suppress = 1; - } -} +my $suppress = C4::Context->is_opac_suppressed; ## I. BUILD THE QUERY ( $error, $query, $simple_query, $query_cgi, $query_desc, $limit, $limit_cgi, $limit_desc, $query_type ) = diff --git a/t/Context.t b/t/Context.t index 31b8a3104f..c2d2ff1fe6 100755 --- a/t/Context.t +++ b/t/Context.t @@ -19,7 +19,7 @@ use Modern::Perl; use DBI; use Test::NoWarnings; -use Test::More tests => 36; +use Test::More tests => 37; use Test::MockModule; use Test::Warn; use YAML::XS; @@ -271,3 +271,33 @@ subtest 'psgi_env and is_internal_PSGI_request' => sub { delete $ENV{'PSGI.UPPERCASE'}; ok( !C4::Context->is_internal_PSGI_request, 'api but no longer PSGI' ); }; + +subtest 'is_opac_suppressed' => sub { + plan tests => 6; + + $ENV{REMOTE_ADDR} = '200.16.23.1'; + + t::lib::Mocks::mock_preference( 'OpacSuppression', 0 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '' ); + ok( !C4::Context->is_opac_suppressed, 'suppression disabled' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 0 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '127.' ); + ok( !C4::Context->is_opac_suppressed, 'suppression disabled with non-matching ip range' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '200.' ); + ok( !C4::Context->is_opac_suppressed, 'suppression enabled with matching ip range' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '' ); + ok( C4::Context->is_opac_suppressed, 'suppression enabled' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '127.' ); + ok( C4::Context->is_opac_suppressed, 'suppression enabled with non-matching ip range' ); + + t::lib::Mocks::mock_preference( 'OpacSuppression', 1 ); + t::lib::Mocks::mock_preference( 'OpacSuppressionByIPRange', '200.' ); + ok( !C4::Context->is_opac_suppressed, 'suppression enabled with matching ip range' ); + }