Bug 20351: Shortcut serials scripts if a blocking error appeared

The idea of output_and_exit_if_error (added by bug 18403) is to make sure
parameters are valid before executing the script.
If not (old or broken URLs), we shortcut everything coming next to display a
generic error ("object does not exist", "you do not have permission to do that", etc.)

This bug report fixes the scripts under serials/*.

Test plan:
Hit the script under the serials directory with an invalid subscriptionid parameter
and confirm you get an error instead of the normal view with empty values.

The goal is not to be exhaustive during the first iteration, but at least to fix
the most common views.

For instance:
/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=XXX
/cgi-bin/koha/serials/serials-collection.pl?subscriptionid=XXX
/cgi-bin/koha/serials/routing.pl?subscriptionid=XXX&op=new
/cgi-bin/koha/serials/subscription-add.pl?op=modify&subscriptionid=XXx
/cgi-bin/koha/serials/subscription-add.pl?op=dup&subscriptionid=XXX

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(cherry picked from commit 62285d2de4)
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Jonathan Druart 2018-05-08 14:25:57 -03:00 committed by Martin Renvoize
parent 81ceef6b84
commit 8969eccaae
No known key found for this signature in database
GPG Key ID: 422B469130441A0F
12 changed files with 54 additions and 5 deletions

View File

@ -291,6 +291,8 @@ sub GetSubscription {
$sth->execute($subscriptionid);
my $subscription = $sth->fetchrow_hashref;
return unless $subscription;
$subscription->{cannotedit} = not can_edit_subscription( $subscription );
# Add additional fields to the subscription into a new key "additional_fields"

View File

@ -7,7 +7,9 @@
<div class="dialog message">This bibliographic record does not exist.</div>
[% CASE 'unknown_item' %]
<div class="dialog message">This item does not exist.</div>
[% CASE %][% blocking_error %]
[% CASE 'unknown_subscription' %]
<div class="dialog message">This subscription does not exist.</div>
[% CASE %][% blocking_error | html %]
[% END %]
[% INCLUDE 'intranet-bottom.inc' %]

View File

@ -7,7 +7,14 @@
[% INCLUDE 'header.inc' %]
[% INCLUDE 'serials-search.inc' %]
<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> &rsaquo; <a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid %]"><i>[% title |html %]</i></a> &rsaquo; [% IF ( op ) %]Create routing list[% ELSE %]Edit routing list[% END %]</div>
<div id="breadcrumbs">
<a href="/cgi-bin/koha/mainpage.pl">Home</a>
&rsaquo; <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a>
[% UNLESS blocking_error %]
&rsaquo; <a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid | uri %]"><i>[% title | html %]</i></a>
&rsaquo; [% IF ( op ) %]Create routing list[% ELSE %]Edit routing list[% END %]
[% END %]
</div>
<div id="doc3" class="yui-t2">
@ -15,6 +22,7 @@
<div id="yui-main">
<div class="yui-b">
[% INCLUDE 'blocking_errors.inc' %]
[% IF ( op ) %]
<h1>Create routing list for <i>[% title |html %]</i></h1>

View File

@ -17,7 +17,15 @@ fieldset.rows li.radio { width: 100%; } /* override staff-global.css */
[% INCLUDE 'header.inc' %]
[% INCLUDE 'serials-search.inc' %]
<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> &rsaquo; [% IF ( modify ) %]<a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid %]"><i>[% bibliotitle |html %]</i></a> &rsaquo; Modify subscription[% ELSE %]New subscription[% END %]</div>
<div id="breadcrumbs">
<a href="/cgi-bin/koha/mainpage.pl">Home</a>
&rsaquo; <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a>
[% UNLESS blocking_error %]
&rsaquo; [% IF ( modify ) %]<a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid | uri %]"><i>[% bibliotitle | html %]</i></a>
&rsaquo; Modify subscription[% ELSE %]New subscription[% END %]
[% END %]
</div>
[% INCLUDE 'blocking_errors.inc' %]
<div class="main container-fluid">
<div class="row">

View File

@ -12,7 +12,12 @@
[% INCLUDE 'header.inc' %]
[% INCLUDE 'serials-search.inc' %]
<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> &rsaquo; Details for subscription #[% subscriptionid %]</div>
<div id="breadcrumbs">
<a href="/cgi-bin/koha/mainpage.pl">Home</a>
&rsaquo; <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a>
[% UNLESS blocking_error %]
&rsaquo; Details for subscription #[% subscriptionid | html %]</div>
[% END %]
<div id="doc3" class="yui-t2">

View File

@ -11,6 +11,8 @@
<body id="ser_subscription-renew" class="ser">
<div class="container-fluid">
[% INCLUDE 'blocking_errors.inc' %]
[% IF op == 'renew' OR op =='multi_renew' %]
[% IF op == 'renew' %]
<span>Subscription renewed.<span>

View File

@ -61,6 +61,11 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
}
);
my $subs = GetSubscription($subscriptionid);
output_and_exit( $query, $cookie, $template, 'unknown_subscription')
unless $subs;
if($op eq 'delete'){
delroutingmember($routingid,$subscriptionid);
}
@ -76,7 +81,7 @@ if($op eq 'save'){
}
my @routinglist = getroutinglist($subscriptionid);
my $subs = GetSubscription($subscriptionid);
my ($count,@serials) = GetSerials($subscriptionid);
my $serialdates = GetLatestSerials($subscriptionid,$count);

View File

@ -104,6 +104,7 @@ if($op eq 'gennext' && @subscriptionid){
last if HasSubscriptionExpired($subscriptionid) > 0;
}
print $query->redirect('/cgi-bin/koha/serials/serials-collection.pl?subscriptionid='.$subscriptionid);
exit;
}
my $subscriptioncount;
@ -113,6 +114,7 @@ if (@subscriptionid){
my $closed = 0;
foreach my $subscriptionid (@subscriptionid){
my $subs= GetSubscription($subscriptionid);
next unless $subs;
$closed = 1 if $subs->{closed};
$subs->{opacnote} =~ s/\n/\<br\/\>/g;
@ -134,6 +136,9 @@ if (@subscriptionid){
my $tmpsubscription= GetFullSubscription($subscriptionid);
@subscriptioninformation=(@$tmpsubscription,@subscriptioninformation);
}
output_and_exit( $query, $cookie, $template, 'unknown_subscription') unless @subscriptioninformation;
$template->param(closed => $closed);
$subscriptions=PrepareSerialsData(\@subscriptioninformation);
$subscriptioncount = CountSubscriptionFromBiblionumber($subscriptiondescs->[0]{'biblionumber'});

View File

@ -112,6 +112,7 @@ unless ( @serialids ) {
$string =~ s/,$//;
print $query->redirect($string);
exit;
}
my ( $template, $loggedinuser, $cookie ) = get_template_and_user(

View File

@ -69,6 +69,9 @@ if ($op eq 'modify' || $op eq 'dup' || $op eq 'modsubscription') {
my $subscriptionid = $query->param('subscriptionid');
$subs = GetSubscription($subscriptionid);
output_and_exit( $query, $cookie, $template, 'unknown_subscription')
unless $subs;
## FIXME : Check rights to edit if mod. Could/Should display an error message.
if ($subs->{'cannotedit'} && $op eq 'modify'){
carp "Attempt to modify subscription $subscriptionid by ".C4::Context->userenv->{'id'}." not allowed";

View File

@ -62,6 +62,10 @@ my ($template, $loggedinuser, $cookie)
my $subs = GetSubscription($subscriptionid);
output_and_exit( $query, $cookie, $template, 'unknown_subscription')
unless $subs;
$subs->{enddate} ||= GetExpirationDate($subscriptionid);
my ($totalissues,@serialslist) = GetSerials($subscriptionid);

View File

@ -75,6 +75,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
if ( $op eq "renew" ) {
# Do not use this script with op=renew and @subscriptionids > 1!
my $subscriptionid = $subscriptionids[0];
# Make sure the subscription exists
my $subscription = GetSubscription( $subscriptionid );
output_and_exit( $query, $cookie, $template, 'unknown_subscription') unless $subscription;
my $startdate = output_pref( { str => scalar $query->param('startdate'), dateonly => 1, dateformat => 'iso' } );
ReNewSubscription(
$subscriptionid, $loggedinuser,
@ -95,6 +98,7 @@ if ( $op eq "renew" ) {
} else {
my $subscriptionid = $subscriptionids[0];
my $subscription = GetSubscription($subscriptionid);
output_and_exit( $query, $cookie, $template, 'unknown_subscription') unless $subscription;
if ($subscription->{'cannotedit'}){
carp "Attempt to renew subscription $subscriptionid by ".C4::Context->userenv->{'id'}." not allowed";
print $query->redirect("/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=$subscriptionid");