Bugzilla – Attachment 78887 Details for
Bug 20351
Implement blocking errors for serials scripts
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20351: Shortcut serials scripts if a blocking error appeared
Bug-20351-Shortcut-serials-scripts-if-a-blocking-e.patch (text/plain), 12.30 KB, created by
Biblibre Sandboxes
on 2018-09-15 20:28:38 UTC
(
hide
)
Description:
Bug 20351: Shortcut serials scripts if a blocking error appeared
Filename:
MIME Type:
Creator:
Biblibre Sandboxes
Created:
2018-09-15 20:28:38 UTC
Size:
12.30 KB
patch
obsolete
>From 0f4cdaef0746b3dd7c5120760e25aab264d6db3c 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 >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >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 <severine.queune@bulac.fr> >Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> >--- > 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 | 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 0eb699f..91498ee 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 c91c252..e803427 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 | 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 90af448..e28b63d 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' %] > >-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> › <a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid | html %]"><i>[% title | html %]</i></a> › [% IF ( op ) %]Create routing list[% ELSE %]Edit routing list[% END %]</div> >+<div id="breadcrumbs"> >+ <a href="/cgi-bin/koha/mainpage.pl">Home</a> >+ › <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> >+ [% UNLESS blocking_error %] >+ › <a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid | uri %]"><i>[% title | html %]</i></a> >+ › [% 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> >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 7f1586c..edab9b3 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' %] > >-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> › [% IF ( modify ) %]<a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid | html %]"><i>[% bibliotitle | html %]</i></a> › Modify subscription[% ELSE %]New subscription[% END %]</div> >+<div id="breadcrumbs"> >+ <a href="/cgi-bin/koha/mainpage.pl">Home</a> >+ › <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> >+ [% UNLESS blocking_error %] >+ › [% IF ( modify ) %]<a href="/cgi-bin/koha/serials/subscription-detail.pl?subscriptionid=[% subscriptionid | uri %]"><i>[% bibliotitle | html %]</i></a> › 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 e8e8364..3135c07 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 >@@ -16,7 +16,11 @@ > [% INCLUDE 'header.inc' %] > [% INCLUDE 'serials-search.inc' %] > >-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> › <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> › Details for subscription #[% subscriptionid | html %]</div> >+<div id="breadcrumbs"> >+ <a href="/cgi-bin/koha/mainpage.pl">Home</a> >+ › <a href="/cgi-bin/koha/serials/serials-home.pl">Serials</a> >+ [% UNLESS blocking_error %]› Details for subscription #[% subscriptionid | html %][% 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 a976dac..fcfbd26 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' OR op =='multi_renew' %] > [% IF op == 'renew' %] > <span>Subscription renewed.<span> >diff --git a/serials/routing.pl b/serials/routing.pl >index c14205a..8d893f6 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 a4fd91d..e8735b3 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 928ff47..33e11ad 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 bb9e8cf..fb7e797 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 cc7a5d2..4eefe18 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 836e612..ccfbad2 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.7.4
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=78887">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20351
:
75178
|
75185
|
75274
|
78344
|
78887
|
80371