Attachment 75178 Details for Bug 20351 – Bug 20351: Shortcut serials scripts if a blocking error appeared

[patch] Bug 20351: Shortcut serials scripts if a blocking error appeared

Bug-20351-Shortcut-serials-scripts-if-a-blocking-e.patch (text/plain), 10.42 KB, created by Jonathan Druart on 2018-05-08 17:41:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2018-05-08 17:41:03 UTC

Size: 10.42 KB

patch

obsolete

>From e4a543b30553d65dacc8dfe3408eed7d89901187 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>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 ++
> .../intranet-tmpl/prog/en/modules/serials/subscription-add.tt    | 9 ++++++++-
> .../intranet-tmpl/prog/en/modules/serials/subscription-detail.tt | 7 ++++++-
> .../intranet-tmpl/prog/en/modules/serials/subscription-renew.tt  | 2 ++
> serials/routing.pl                                               | 4 ++++
> 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 ++++
> 11 files changed, 41 insertions(+), 2 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 @@
>         <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 'unknown_subscription' %]
>+        <div class="dialog message">This subscription does not exist.</div>
>     [% CASE %][% blocking_error %]
>     [% END %]
> 
>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' %]
> 
>-<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 %]"><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">
>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' %]
> 
>-<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 %][% END %]
>+
>+</div>
> 
> <div id="doc3" class="yui-t2">
>    
>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 @@
> <body id="ser_subscription-renew" class="ser">
>     <div class="container-fluid">
> 
>+[% INCLUDE 'blocking_errors.inc' %]
>+
> [% IF op == 'renew' %]
>   Subscription renewed.
>   
>diff --git a/serials/routing.pl b/serials/routing.pl
>index c14205aaaf..c6bd150731 100755
>--- a/serials/routing.pl
>+++ b/serials/routing.pl
>@@ -77,6 +77,10 @@ if($op eq 'save'){
> 
> my @routinglist = getroutinglist($subscriptionid);
> my $subs = GetSubscription($subscriptionid);
>+
>+output_and_exit( $query, $cookie, $template, 'unknown_subscription')
>+    unless $subs;
>+
> 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/\<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'});
>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

Actions: View | Diff | Splinter Review

Attachments on bug 20351: 75178 | 75185 | 75274 | 78344 | 78887 | 80371