From 8969eccaaeddfffbdc30a2f36e6dfc64595d607e 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 (cherry picked from commit 62285d2de463811380262e4cc65c82937145d477) Signed-off-by: Martin Renvoize --- C4/Serials.pm | 2 ++ .../intranet-tmpl/prog/en/includes/blocking_errors.inc | 4 +++- .../intranet-tmpl/prog/en/modules/serials/routing.tt | 10 +++++++++- .../prog/en/modules/serials/subscription-add.tt | 10 +++++++++- .../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, 54 insertions(+), 5 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 14598410b2..f7cff313f2 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..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,7 +7,9 @@
This bibliographic record does not exist.
[% CASE 'unknown_item' %]
This item does not exist.
- [% CASE %][% blocking_error %] + [% CASE 'unknown_subscription' %] +
This subscription does not exist.
+ [% CASE %][% blocking_error | html %] [% END %] [% INCLUDE 'intranet-bottom.inc' %] 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..e06cc53962 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 786aac82cf..7bb9580d17 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,15 @@ 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..15fbe03f10 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' %] - + + [% END %]
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 4a5fd2c285..8b83605a0f 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 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.39.5