Bugzilla – Attachment 192092 Details for
Bug 41696
Error handling in additem.pl is too specific and insufficient
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 41696: Improve additem.pl/tt's error handling
9619416.patch (text/plain), 7.25 KB, created by
HKS3 Tadeusz Sośnierz
on 2026-01-27 10:02:52 UTC
(
hide
)
Description:
Bug 41696: Improve additem.pl/tt's error handling
Filename:
MIME Type:
Creator:
HKS3 Tadeusz Sośnierz
Created:
2026-01-27 10:02:52 UTC
Size:
7.25 KB
patch
obsolete
>From 96194169616645aa54bfa325c47f4231ec6b3225 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Tadeusz=20=E2=80=9Etadzik=E2=80=9D=20So=C5=9Bnierz?= > <tadeusz@sosnierz.com> >Date: Fri, 23 Jan 2026 15:07:17 +0100 >Subject: [PATCH] Bug 41696: Improve additem.pl/tt's error handling >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This prevents us from losing track of errors and failing quietly. >Also gets rid of hardcoded error messages for Koha::Object exceptions, >defering to full_message which provides more details. > >Sponsored-by: Universitätsbibliothek der Andrássy Universität >--- > Koha/Exceptions/Object.pm | 2 + > Koha/Object.pm | 10 +---- > cataloguing/additem.pl | 6 +-- > .../prog/en/modules/cataloguing/additem.tt | 37 +++++++++++++------ > 4 files changed, 32 insertions(+), 23 deletions(-) > >diff --git a/Koha/Exceptions/Object.pm b/Koha/Exceptions/Object.pm >index 57aa4c8bc3..2f8d30bb85 100644 >--- a/Koha/Exceptions/Object.pm >+++ b/Koha/Exceptions/Object.pm >@@ -89,6 +89,8 @@ sub full_message { > } elsif ( $self->isa('Koha::Exceptions::Object::NotInstantiated') ) { > $msg = > sprintf( "Tried to access the '%s' method, but %s is not instantiated", $self->method, $self->class ); >+ } elsif ( $self->isa('Koha::Exceptions::Object::DuplicateID') ) { >+ $msg = sprintf( "Duplicate ID for key '%s'", $self->duplicate_id ); > } > } > >diff --git a/Koha/Object.pm b/Koha/Object.pm >index 55c7e489a0..f432fbc9bc 100644 >--- a/Koha/Object.pm >+++ b/Koha/Object.pm >@@ -182,16 +182,10 @@ sub store { > # FK constraints > # FIXME: MySQL error, if we support more DB engines we should implement this for each > if ( $_->{msg} =~ /FOREIGN KEY \(`(?<column>.*?)`\)/ ) { >- Koha::Exceptions::Object::FKConstraint->throw( >- error => 'Broken FK constraint', >- broken_fk => $+{column} >- ); >+ Koha::Exceptions::Object::FKConstraint->throw( broken_fk => $+{column} ); > } > } elsif ( $_->{msg} =~ /Duplicate entry '(.*?)' for key '(?<key>.*?)'/ ) { >- Koha::Exceptions::Object::DuplicateID->throw( >- error => 'Duplicate ID', >- duplicate_id => $+{key} >- ); >+ Koha::Exceptions::Object::DuplicateID->throw( duplicate_id => $+{key} ); > } elsif ( $_->{msg} =~ /Incorrect (?<type>\w+) value: '(?<value>.*)' for column \W?(?<property>\S+)/ ) > { # The optional \W in the regex might be a quote or backtick > my $type = $+{type}; >diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl >index 5c3e200512..4d4ff5dc06 100755 >--- a/cataloguing/additem.pl >+++ b/cataloguing/additem.pl >@@ -646,7 +646,7 @@ if ( $op eq "cud-additem" ) { > try { > $item->store; > } catch { >- push @errors, $_->error; >+ push @errors, $_; > } > } > >@@ -838,7 +838,5 @@ if ( $frameworkcode eq 'FA' ) { > ); > } > >-foreach my $error (@errors) { >- $template->param( $error => 1 ); >-} >+$template->param( errors => \@errors ); > output_html_with_http_headers $input, $cookie, $template->output; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >index 76758d3deb..5d8dc06bf2 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >@@ -89,18 +89,33 @@ > <h1>Items for [% biblio.title | html %] [% IF ( biblio.author ) %]by [% biblio.author | html %][% END %] (Record #[% biblio.biblionumber | html %])</h1> > <a id="newitem_jump" href="#f"><i class="fa fa-arrow-down"></i> Jump to form</a> > >- [% IF ( onloan_cannot_withdraw ) %]<div class="alert alert-warning"><strong>Error saving item</strong>: Onloan item cannot be withdrawn.</div>[% END %] >- [% IF ( intransit_cannot_withdraw ) %]<div class="alert alert-warning"><strong>Error saving item</strong>: In transit item cannot be withdrawn.</div>[% END %] >- [% IF ( barcode_not_unique ) %]<div class="alert alert-warning"><strong>Error saving item</strong>: Barcode must be unique.</div>[% END %] >- [% IF ( no_next_barcode ) %]<div class="alert alert-warning"><strong>Error saving items</strong>: Unable to automatically determine values for barcodes. No item has been inserted.</div>[% END %] >- [% IF ( book_on_loan ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: item is checked out.</div>[% END %] >- [% IF ( book_reserved ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: item has a waiting hold.</div>[% END %] >- [% IF ( not_same_branch ) %]<div class="alert alert-warning"><strong>Cannot delete</strong>: The items do not belong to your library.</div>[% END %] >- [% IF ( linked_analytics ) %] >- <div class="alert alert-warning"><strong>Cannot delete</strong>: item has linked <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber | uri %]&analyze=1">analytics.</a>.</div> >+ [% FOREACH error IN errors %] >+ <div class="alert alert-warning"> >+ [% IF error == 'onloan_cannot_withdraw' %] >+ <strong>Error saving item</strong>: Onloan item cannot be withdrawn. >+ [% ELSIF error == 'intransit_cannot_withdraw' %] >+ <strong>Error saving item</strong>: In transit item cannot be withdrawn. >+ [% ELSIF error == 'barcode_not_unique' %] >+ <strong>Error saving item</strong>: Barcode must be unique. >+ [% ELSIF error == 'no_next_barcode' %] >+ <strong>Error saving items</strong>: Unable to automatically determine values for barcodes. No item has been inserted. >+ [% ELSIF error == 'book_on_loan' %] >+ <strong>Cannot delete</strong>: item is checked out. >+ [% ELSIF error == 'book_reserved' %] >+ <strong>Cannot delete</strong>: item has a waiting hold. >+ [% ELSIF error == 'not_same_branch' %] >+ <strong>Cannot delete</strong>: The items do not belong to your library. >+ [% ELSIF error == 'linked_analytics' %] >+ <strong>Cannot delete</strong>: item has linked <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblio.biblionumber | uri %]&analyze=1">analytics.</a>. >+ [% ELSIF error == 'last_item_for_hold' %] >+ <strong>Cannot delete</strong>: Last item for bibliographic record with biblio-level hold on it. >+ [% ELSIF error == 'item_not_found' %] >+ <strong>Cannot delete</strong>: Item not found. >+ [% ELSE %] >+ <strong>Error</strong>: [% error %]. >+ [% END %] >+ </div> > [% END %] >- [% IF last_item_for_hold %]<div class="alert alert-warning"><strong>Cannot delete</strong>: Last item for bibliographic record with biblio-level hold on it.</div>[% END %] >- [% IF item_not_found %]<div class="alert alert-warning"><strong>Cannot delete</strong>: Item not found.</div>[% END %] > > <div id="cataloguing_additem_itemlist"> > [% IF items %] >-- >2.51.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 41696
:
191940
| 192092 |
192317