From 62285d2de463811380262e4cc65c82937145d477 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Tue, 8 May 2018 14:25:57 -0300 Subject: [PATCH] Bug 20351: Shortcut serials scripts if a blocking error appeared MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Séverine QUEUNE Signed-off-by: Katrin Fischer Signed-off-by: Nick Clemens --- C4/Serials.pm | 2 ++ .../intranet-tmpl/prog/en/includes/blocking_errors.inc | 2 ++ .../intranet-tmpl/prog/en/modules/serials/routing.tt | 10 +++++++++- .../prog/en/modules/serials/subscription-add.tt | 9 ++++++++- .../prog/en/modules/serials/subscription-detail.tt | 6 +++++- .../prog/en/modules/serials/subscription-renew.tt | 2 ++ serials/routing.pl | 7 ++++++- serials/serials-collection.pl | 5 +++++ serials/serials-edit.pl | 1 + serials/subscription-add.pl | 3 +++ serials/subscription-detail.pl | 4 ++++ serials/subscription-renew.pl | 4 ++++ 12 files changed, 51 insertions(+), 4 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 0eb699fc45..91498ee262 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -269,6 +269,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" diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc index c91c2522f5..e803427eeb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc @@ -7,6 +7,8 @@
This bibliographic record does not exist.
[% CASE 'unknown_item' %]
This item does not exist.
+ [% CASE 'unknown_subscription' %] +
This subscription does not exist.
[% CASE %][% blocking_error | html %] [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/routing.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/routing.tt index 90af448ca2..e28b63d59b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/routing.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/routing.tt @@ -7,7 +7,14 @@ [% INCLUDE 'header.inc' %] [% INCLUDE 'serials-search.inc' %] - +
@@ -15,6 +22,7 @@
+[% INCLUDE 'blocking_errors.inc' %] [% IF ( op ) %]

Create routing list for [% title | html %]

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt index 7f1586c71c..edab9b3f0d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt @@ -18,7 +18,14 @@ fieldset.rows li.radio { width: 100%; } /* override staff-global.css */ [% INCLUDE 'header.inc' %] [% INCLUDE 'serials-search.inc' %] - + +[% INCLUDE 'blocking_errors.inc' %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt index 5818cf1740..d9f1732de0 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-detail.tt @@ -17,7 +17,11 @@ [% INCLUDE 'header.inc' %] [% INCLUDE 'serials-search.inc' %] - +
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt index a976dacf4f..fcfbd26b07 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt @@ -11,6 +11,8 @@
+[% INCLUDE 'blocking_errors.inc' %] + [% IF op == 'renew' OR op =='multi_renew' %] [% IF op == 'renew' %] Subscription renewed. diff --git a/serials/routing.pl b/serials/routing.pl index c14205aaaf..8d893f66de 100755 --- a/serials/routing.pl +++ b/serials/routing.pl @@ -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); diff --git a/serials/serials-collection.pl b/serials/serials-collection.pl index a4fd91d906..e8735b319d 100755 --- a/serials/serials-collection.pl +++ b/serials/serials-collection.pl @@ -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/\/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'}); diff --git a/serials/serials-edit.pl b/serials/serials-edit.pl index 928ff47ab3..33e11ad3bc 100755 --- a/serials/serials-edit.pl +++ b/serials/serials-edit.pl @@ -114,6 +114,7 @@ unless ( @serialids ) { $string =~ s/,$//; print $query->redirect($string); + exit; } my ( $template, $loggedinuser, $cookie ) = get_template_and_user( diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index bb9e8cfc9b..fb7e797762 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -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"; diff --git a/serials/subscription-detail.pl b/serials/subscription-detail.pl index cc7a5d2985..4eefe181f3 100755 --- a/serials/subscription-detail.pl +++ b/serials/subscription-detail.pl @@ -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); diff --git a/serials/subscription-renew.pl b/serials/subscription-renew.pl index 836e612344..ccfbad2916 100755 --- a/serials/subscription-renew.pl +++ b/serials/subscription-renew.pl @@ -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"); -- 2.39.5