From bfe26433b5aa3e8bbf587292837a65161e08d38b 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 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 --- C4/Serials.pm | 2 ++ koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc | 2 ++ koha-tmpl/intranet-tmpl/prog/en/modules/serials/routing.tt | 10 +++++++++- .../intranet-tmpl/prog/en/modules/serials/subscription-add.tt | 9 ++++++++- .../prog/en/modules/serials/subscription-detail.tt | 7 ++++++- .../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, 52 insertions(+), 4 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index ac21dad49b..eb82d4ca10 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -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" 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 0ab6738260..ce761bd087 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 %] [% 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 f42656a264..2788fb67b4 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 8f95b49d27..14f76218a9 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 @@ -17,7 +17,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 ac4b8f8a83..999fad2f4b 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 @@ -12,7 +12,12 @@ [% 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 3639e57961..6ae8b8cf8c 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' %] 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 e5f69e8192..94dc5913ed 100755 --- a/serials/serials-edit.pl +++ b/serials/serials-edit.pl @@ -112,6 +112,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 b7e7c05bd0..aba95a1fee 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.11.0