From f71124764b204aedfdeb5fa0289c1d1f47da16db Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 8 Apr 2011 13:18:54 -0300 Subject: [PATCH] [Bug 5791] Robust handling of deleted/non-existent biblios and authority records Content-Type: text/plain; charset="utf-8" This patch is a refactoring of the original patches, as those don't apply anymore. The wording was suggested by oleonard. Basically what I do is check in authorities/detail.pl catalogue/ISBDdetail.pl catalogue/MARCdetail.pl catalogue/detail.pl if the record retreived from GetMarcBiblio, GetAuthority and GetISBDView exists and if not set several variables for the templates and call exit just after output_html_with_http_headers is called. In the corresponding *.tmpl files I handle those 'error variables' properly to show the user a consistent error message. Notes: - I changed one line in C4/Biblio.pm. I was worried about it. But it's so simple it won't hurt :-) - In authorities/detail.pl I deleted a few variables that wheren't actually used, plus changing the order of some variable definition/calculation so the dropdown menus for auth search remain pretty and consistent. - I'm not sure this will catch an error in BuildUnimarcHierachies. I didn't want to touch anything there, but there is an unused varible, and zero error catchig there. As I don't know anything from UNIMARC, if someone helps me, I can work on it. At least provinding me some UNIMARC hierachies for my dev install. - Some code refactoring in ISBDdetail.pl is a mistake I made. I noticed late. --- C4/Biblio.pm | 1 + authorities/detail.pl | 48 ++++++++++--------- catalogue/ISBDdetail.pl | 20 +++++--- catalogue/MARCdetail.pl | 18 ++++++-- catalogue/detail.pl | 25 +++++----- catalogue/labeledMARCdetail.pl | 16 +++++- .../prog/en/modules/authorities/detail.tmpl | 29 ++++++------ .../prog/en/modules/catalogue/ISBDdetail.tmpl | 37 ++++++++------- .../prog/en/modules/catalogue/MARCdetail.tmpl | 9 +++- .../prog/en/modules/catalogue/detail.tmpl | 10 +++- .../en/modules/catalogue/labeledMARCdetail.tmpl | 19 +++++--- 11 files changed, 139 insertions(+), 93 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 135dd91..aa072fd 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -759,6 +759,7 @@ Return the ISBD view which can be included in opac and intranet sub GetISBDView { my ( $biblionumber, $template ) = @_; my $record = GetMarcBiblio($biblionumber); + return undef unless defined $record; my $itemtype = &GetFrameworkCode($biblionumber); my ( $holdingbrtagf, $holdingbrtagsubf ) = &GetMarcFromKohaField( "items.holdingbranch", $itemtype ); my $tagslib = &GetMarcStructure( 1, $itemtype ); diff --git a/authorities/detail.pl b/authorities/detail.pl index 59273c1..57e0737 100755 --- a/authorities/detail.pl +++ b/authorities/detail.pl @@ -172,13 +172,33 @@ my ($template, $loggedinuser, $cookie) my $authid = $query->param('authid'); - - # Using default authtypecode, so all fields are seen my $authtypecode = ''; $tagslib = &GetTagsLabels(1,$authtypecode); -my $record; +# Build list of authtypes for showing them +my $authtypes = getauthtypes; +my @authtypesloop; + +foreach my $thisauthtype (sort { $authtypes->{$b} cmp $authtypes->{$a} } keys %$authtypes) { + my %row =(value => $thisauthtype, + selected => $thisauthtype eq $authtypecode, + authtypetext => $authtypes->{$thisauthtype}{'authtypetext'}, + ); + push @authtypesloop, \%row; +} + +my $record=GetAuthority($authid); + +if (not defined $record) { + # authid invalid + $template->param ( errauthid => $authid, + unknownauthid => 1, + authtypesloop => \@authtypesloop ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} + if (C4::Context->preference("AuthDisplayHierarchy")){ my $trees=BuildUnimarcHierarchies($authid); my @trees = split /;/,$trees ; @@ -201,9 +221,8 @@ if (C4::Context->preference("AuthDisplayHierarchy")){ 'displayhierarchy' =>C4::Context->preference("AuthDisplayHierarchy"), 'loophierarchies' =>\@loophierarchies, ); -} else { - $record=GetAuthority($authid); } + my $count = CountUsage($authid); # find the marc field/subfield used in biblio by this authority @@ -215,24 +234,7 @@ while (my ($tagfield) = $sth->fetchrow) { } chop $biblio_fields; - -# fill arrays -my @loop_data =(); -my $tag; -# loop through each tab 0 through 9 -# for (my $tabloop = 0; $tabloop<=10;$tabloop++) { -# loop through each tag - build_tabs ($template, $record, $dbh,"",$query); - -my $authtypes = getauthtypes; -my @authtypesloop; -foreach my $thisauthtype (sort { $authtypes->{$b} cmp $authtypes->{$a} } keys %$authtypes) { - my %row =(value => $thisauthtype, - selected => $thisauthtype eq $authtypecode, - authtypetext => $authtypes->{$thisauthtype}{'authtypetext'}, - ); - push @authtypesloop, \%row; -} +build_tabs ($template, $record, $dbh,"",$query); $template->param(authid => $authid, count => $count, diff --git a/catalogue/ISBDdetail.pl b/catalogue/ISBDdetail.pl index df84a3a..58a8d90 100755 --- a/catalogue/ISBDdetail.pl +++ b/catalogue/ISBDdetail.pl @@ -67,6 +67,16 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); +my $res = GetISBDView($biblionumber, "intranet"); +if ( not defined $res ) { + # biblionumber invalid -> report and exit + $template->param( unknownbiblionumber => 1, + biblionumber => $biblionumber + ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} + if($query->cookie("holdfor")){ my $holdfor_patron = GetMember('borrowernumber' => $query->cookie("holdfor")); $template->param( @@ -77,10 +87,6 @@ if($query->cookie("holdfor")){ ); } -# my @blocs = split /\@/,$ISBD; -# my @fields = $record->fields(); -my $res = GetISBDView($biblionumber, "intranet"); - # count of item linked with biblio my $itemcount = GetItemsCount($biblionumber); $template->param( count => $itemcount); @@ -98,9 +104,9 @@ if ($subscriptionsnumber) { $template->param ( ISBD => $res, biblionumber => $biblionumber, - isbdview => 1, - z3950_search_params => C4::Search::z3950_search_args(GetBiblioData($biblionumber)), - C4::Search::enabled_staff_search_views, + isbdview => 1, + z3950_search_params => C4::Search::z3950_search_args(GetBiblioData($biblionumber)), + C4::Search::enabled_staff_search_views ); output_html_with_http_headers $query, $cookie, $template->output; diff --git a/catalogue/MARCdetail.pl b/catalogue/MARCdetail.pl index c3cc5d4..0a2974b 100755 --- a/catalogue/MARCdetail.pl +++ b/catalogue/MARCdetail.pl @@ -70,10 +70,6 @@ my $popup = ; # if set to 1, then don't insert links, it's just to show the biblio my $subscriptionid = $query->param('subscriptionid'); -my $tagslib = &GetMarcStructure(1,$frameworkcode); - -my $record = GetMarcBiblio($biblionumber); -my $biblio = GetBiblioData($biblionumber); # open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -86,6 +82,20 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); +my $record = GetMarcBiblio($biblionumber); + +if ( not defined $record ) { + # biblionumber invalid -> report and exit + $template->param( unknownbiblionumber => 1, + biblionumber => $biblionumber + ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} + +my $tagslib = &GetMarcStructure(1,$frameworkcode); +my $biblio = GetBiblioData($biblionumber); + if($query->cookie("holdfor")){ my $holdfor_patron = GetMember('borrowernumber' => $query->cookie("holdfor")); $template->param( diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 21fcb1f..03271b4 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -51,6 +51,17 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( } ); +my $biblionumber = $query->param('biblionumber'); +my $record = GetMarcBiblio($biblionumber); + +if ( not defined $record ) { + # biblionumber invalid -> report and exit + $template->param( unknownbiblionumber => 1, + biblionumber => $biblionumber ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} + if($query->cookie("holdfor")){ my $holdfor_patron = GetMember('borrowernumber' => $query->cookie("holdfor")); $template->param( @@ -61,14 +72,9 @@ if($query->cookie("holdfor")){ ); } -my $biblionumber = $query->param('biblionumber'); -my $fw = GetFrameworkCode($biblionumber); - +my $fw = GetFrameworkCode($biblionumber); my $showallitems = $query->param('showallitems'); - -## get notes and subjects from MARC record -my $marcflavour = C4::Context->preference("marcflavour"); -my $record = GetMarcBiblio($biblionumber); +my $marcflavour = C4::Context->preference("marcflavour"); # XSLT processing of some stuff if (C4::Context->preference("XSLTDetailsDisplay") ) { @@ -93,11 +99,6 @@ $template->param( normalized_isbn => $isbn, ); -unless (defined($record)) { - print $query->redirect("/cgi-bin/koha/errors/404.pl"); - exit; -} - my $marcnotesarray = GetMarcNotes( $record, $marcflavour ); my $marcisbnsarray = GetMarcISBN( $record, $marcflavour ); my $marcauthorsarray = GetMarcAuthors( $record, $marcflavour ); diff --git a/catalogue/labeledMARCdetail.pl b/catalogue/labeledMARCdetail.pl index 8faecfc..b8c7982 100755 --- a/catalogue/labeledMARCdetail.pl +++ b/catalogue/labeledMARCdetail.pl @@ -38,9 +38,6 @@ my $popup = $query->param('popup') ; # if set to 1, then don't insert links, it's just to show the biblio -my $tagslib = GetMarcStructure(1,$frameworkcode); -my $record = GetMarcBiblio($biblionumber); -my $biblio = GetBiblioData($biblionumber); # open template my ( $template, $loggedinuser, $cookie ) = get_template_and_user( { @@ -53,6 +50,19 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( } ); +my $record = GetMarcBiblio($biblionumber); +if ( not defined $record ) { + # biblionumber invalid -> report and exit + $template->param( unknownbiblionumber => 1, + biblionumber => $biblionumber + ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} + +my $tagslib = GetMarcStructure(1,$frameworkcode); +my $biblio = GetBiblioData($biblionumber); + if($query->cookie("holdfor")){ my $holdfor_patron = GetMember('borrowernumber' => $query->cookie("holdfor")); $template->param( diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tmpl index 417581c..f16298d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tmpl +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tmpl @@ -1,5 +1,5 @@ -Koha › Authorities › Details for Authority #<!-- TMPL_VAR NAME="authid" --> (<!-- TMPL_VAR name="authtypetext" -->) +Koha › Authorities › <!-- TMPL_IF NAME="unknownauthid" --> Unknown authority record<!-- TMPL_ELSE -->Details for Authority #<!-- TMPL_VAR NAME="authid" --> (<!-- TMPL_VAR name="authtypetext" -->)<!-- /TMPL_IF -->