From d5dad709acd79b4ee1f31a8ae9879b98036be19d 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" Patch reworked for master using Template::Toolkit. To+ --- 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.tt | 21 ++++++++- .../prog/en/modules/catalogue/ISBDdetail.tt | 43 ++++++++++++------ .../prog/en/modules/catalogue/MARCdetail.tt | 20 +++++++- .../prog/en/modules/catalogue/detail.tt | 22 ++++++++- .../prog/en/modules/catalogue/labeledMARCdetail.tt | 22 ++++++++- 11 files changed, 183 insertions(+), 73 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 7be1b3e..120592f 100755 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -761,6 +761,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.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tt index 24d200b..6c74a3b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/authorities/detail.tt @@ -1,5 +1,11 @@ [% INCLUDE 'doc-head-open.inc' %] -Koha › Authorities › Details for Authority #[% authid %] ([% authtypetext %]) +Koha › Authorities › + [% IF ( unknownauthid ) %] + Unknown authority record + [% ELSE %] + Details for Authority #[% authid %] ([% authtypetext %]) + [% END %] + [% INCLUDE 'doc-head-close.inc' %]