Bugzilla – Attachment 193998 Details for
Bug 35104
We should warn when attempting to save MARC records that contain characters invalid in XML
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35104: Persist nonxml_stripped on biblio_metadata, show on detail pages
b65286b.patch (text/plain), 11.15 KB, created by
Martin Renvoize (ashimema)
on 2026-02-26 13:45:46 UTC
(
hide
)
Description:
Bug 35104: Persist nonxml_stripped on biblio_metadata, show on detail pages
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-26 13:45:46 UTC
Size:
11.15 KB
patch
obsolete
>From b65286bdf55cae8384da2387b08c2c56d9c29560 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Thu, 26 Feb 2026 12:08:28 +0000 >Subject: [PATCH] Bug 35104: Persist nonxml_stripped on biblio_metadata, show > on detail pages > >Add a nonxml_stripped column to biblio_metadata so that the automatic >stripping of non-XML characters during store() survives beyond the >current request. The column is set to the XML parse error message when >stripping was needed, and cleared to NULL when the record is saved >cleanly, so the warning disappears once the cataloguer has reviewed and >re-saved the record. > >Show a warning alert on the biblio detail page and MARC detail page >whenever nonxml_stripped is set, with a direct link to the edit form. > >Fix Koha::Import::Record::get_marc_record() to fall back to >StripNonXmlChars when the raw staged MARCXML cannot be parsed, so that >the import preview (showmarc.pl?importid=...) no longer returns a 404 >for records that contain non-XML characters. > >Sponsored-by: OpenFifth >--- > C4/Biblio.pm | 4 +-- > Koha/Biblio/Metadata.pm | 26 +++++-------------- > Koha/Import/Record.pm | 6 ++++- > Koha/Schema/Result/BiblioMetadata.pm | 9 +++++++ > catalogue/MARCdetail.pl | 3 ++- > catalogue/detail.pl | 3 ++- > .../data/mysql/atomicupdate/bug_35104.pl | 23 ++++++++++++++++ > .../prog/en/modules/catalogue/MARCdetail.tt | 10 +++++++ > .../prog/en/modules/catalogue/detail.tt | 10 +++++++ > t/db_dependent/Koha/Biblio/Metadata.t | 4 +-- > 10 files changed, 72 insertions(+), 26 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_35104.pl > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index 629ed885c4a..1e33fa722b8 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -3004,9 +3004,9 @@ sub ModBiblioMarc { > } > )->store; > >- if ( $m_rs->stripped_nonxml && ref $options->{warnings} eq 'ARRAY' ) { >+ if ( $m_rs->nonxml_stripped && ref $options->{warnings} eq 'ARRAY' ) { > push @{ $options->{warnings} }, >- { type => 'nonxml_stripped', message => $m_rs->stripped_nonxml }; >+ { type => 'nonxml_stripped', message => $m_rs->nonxml_stripped }; > } > > unless ($skip_record_index) { >diff --git a/Koha/Biblio/Metadata.pm b/Koha/Biblio/Metadata.pm >index 2dca4d7641c..8195553cb3f 100644 >--- a/Koha/Biblio/Metadata.pm >+++ b/Koha/Biblio/Metadata.pm >@@ -48,7 +48,7 @@ to committing to the database. > > If the MARCXML cannot be parsed but can be recovered by stripping non-XML > characters (C<StripNonXmlChars>), the stripped version is saved and >-C<stripped_nonxml> is set on the object so callers can notify the user. >+C<nonxml_stripped> is persisted on the row so callers and the UI can notify the user. > > If the MARCXML cannot be recovered at all, a > I<Koha::Exceptions::Metadata::Invalid> exception is thrown. >@@ -69,7 +69,11 @@ sub store { > }; > my $marcxml_error = $@; > chomp $marcxml_error; >- unless ($marcxml) { >+ if ($marcxml) { >+ >+ # Clean parse รข clear any previously recorded stripping >+ $self->nonxml_stripped(undef); >+ } else { > > # Attempt recovery by stripping non-XML characters > my $stripped_metadata = StripNonXmlChars( $self->metadata ); >@@ -85,7 +89,7 @@ sub store { > ) > ); > $self->metadata($stripped_metadata); >- $self->{_stripped_nonxml} = $marcxml_error; >+ $self->nonxml_stripped($marcxml_error); > } else { > > # Truly unrecoverable >@@ -104,22 +108,6 @@ sub store { > return $self->SUPER::store; > } > >-=head3 stripped_nonxml >- >- if ( my $error = $metadata->stripped_nonxml ) { >- # warn user that non-XML characters were removed on last store() >- } >- >-Returns the original decode error string if non-XML characters were stripped >-during the most recent call to C<store()>, or C<undef> otherwise. >- >-=cut >- >-sub stripped_nonxml { >- my $self = shift; >- return $self->{_stripped_nonxml}; >-} >- > =head3 record > > my $record = $metadata->record; >diff --git a/Koha/Import/Record.pm b/Koha/Import/Record.pm >index 9b1e1c3f675..a492c5e0702 100644 >--- a/Koha/Import/Record.pm >+++ b/Koha/Import/Record.pm >@@ -20,6 +20,7 @@ use Modern::Perl; > use Carp; > use MARC::Record; > >+use C4::Charset qw( StripNonXmlChars ); > use C4::Context; > use C4::Biblio qw(ModBiblio); > use C4::AuthoritiesMarc qw(GuessAuthTypeCode ModAuthority); >@@ -56,7 +57,10 @@ sub get_marc_record { > $format = 'UNIMARCAUTH'; > } > >- my $record = MARC::Record->new_from_xml( $self->marcxml, 'UTF-8', $format ); >+ my $record = eval { MARC::Record->new_from_xml( $self->marcxml, 'UTF-8', $format ) }; >+ unless ($record) { >+ $record = eval { MARC::Record->new_from_xml( StripNonXmlChars( $self->marcxml ), 'UTF-8', $format ) }; >+ } > > return $record; > } >diff --git a/Koha/Schema/Result/BiblioMetadata.pm b/Koha/Schema/Result/BiblioMetadata.pm >index 8970c1384a9..2c38814aaaa 100644 >--- a/Koha/Schema/Result/BiblioMetadata.pm >+++ b/Koha/Schema/Result/BiblioMetadata.pm >@@ -52,6 +52,13 @@ __PACKAGE__->table("biblio_metadata"); > data_type: 'longtext' > is_nullable: 0 > >+=head2 nonxml_stripped >+ >+ data_type: 'mediumtext' >+ is_nullable: 1 >+ >+Parse error message recorded when non-XML characters were stripped on last store(). >+ > =head2 timestamp > > data_type: 'timestamp' >@@ -80,6 +87,8 @@ __PACKAGE__->add_columns( > { data_type => "varchar", is_nullable => 0, size => 16 }, > "metadata", > { data_type => "longtext", is_nullable => 0 }, >+ "nonxml_stripped", >+ { data_type => "mediumtext", is_nullable => 1 }, > "timestamp", > { > data_type => "timestamp", >diff --git a/catalogue/MARCdetail.pl b/catalogue/MARCdetail.pl >index 72cf4807c4a..3b71fbd90fd 100755 >--- a/catalogue/MARCdetail.pl >+++ b/catalogue/MARCdetail.pl >@@ -120,7 +120,8 @@ if ( $query->cookie("searchToOrder") ) { > ); > } > >-$template->param( ocoins => $biblio_object->get_coins ); >+$template->param( ocoins => $biblio_object->get_coins ); >+$template->param( nonxml_stripped => $biblio_object->metadata->nonxml_stripped ); > > #count of item linked > my $itemcount = $biblio_object->items->count; >diff --git a/catalogue/detail.pl b/catalogue/detail.pl >index d6ff6220315..99464f0de20 100755 >--- a/catalogue/detail.pl >+++ b/catalogue/detail.pl >@@ -176,7 +176,8 @@ my $marcflavour = C4::Context->preference("marcflavour"); > > $template->param( 'SpineLabelShowPrintOnBibDetails' => C4::Context->preference("SpineLabelShowPrintOnBibDetails") ); > >-$template->param( ocoins => !$invalid_marc_record ? $biblio->get_coins : undef ); >+$template->param( ocoins => !$invalid_marc_record ? $biblio->get_coins : undef ); >+$template->param( nonxml_stripped => $biblio->metadata->nonxml_stripped ); > > # some useful variables for enhanced content; > # in each case, we're grabbing the first value we find in >diff --git a/installer/data/mysql/atomicupdate/bug_35104.pl b/installer/data/mysql/atomicupdate/bug_35104.pl >new file mode 100644 >index 00000000000..eafc9eef483 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_35104.pl >@@ -0,0 +1,23 @@ >+use Modern::Perl; >+use Koha::Installer::Output qw(say_warning say_success say_info); >+ >+return { >+ bug_number => "35104", >+ description => "Add nonxml_stripped column to biblio_metadata", >+ up => sub { >+ my ($args) = @_; >+ my ( $dbh, $out ) = @$args{qw(dbh out)}; >+ >+ unless ( column_exists( 'biblio_metadata', 'nonxml_stripped' ) ) { >+ $dbh->do( >+ q{ >+ ALTER TABLE biblio_metadata >+ ADD COLUMN `nonxml_stripped` mediumtext NULL DEFAULT NULL >+ COMMENT 'parse error message recorded when non-XML characters were stripped on last store()' >+ AFTER metadata >+ } >+ ); >+ say $out "Added new column 'biblio_metadata.nonxml_stripped'"; >+ } >+ }, >+}; >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt >index 66a5774709d..dcf6fd84ae5 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/MARCdetail.tt >@@ -68,6 +68,16 @@ > </select> > </strong></p > > >+ [% IF nonxml_stripped %] >+ <div class="alert alert-warning"> >+ <h1>Record was automatically modified</h1> >+ <p >+ >Non-XML characters were stripped from this record when it was last saved. Please <a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=[% biblionumber | uri %]">review and re-save the record</a> to confirm the data >+ is correct.</p >+ > >+ <pre class="error">[% nonxml_stripped | html %]</pre> >+ </div> >+ [% END %] > [% IF ( ocoins ) %] > <!-- COinS / OpenURL --> > <span class="Z3988" title="[% ocoins | html %]"></span> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt >index 763cda06baf..6c1d78ad720 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt >@@ -102,6 +102,16 @@ > <!-- /.row --> > <div class="row"> > <div id="catalogue_detail_biblio" class="col"> >+ [% IF nonxml_stripped %] >+ <div class="alert alert-warning"> >+ <h1>Record was automatically modified</h1> >+ <p >+ >Non-XML characters were stripped from this record when it was last saved. Please <a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=[% biblio.biblionumber | uri %]">review and re-save the record</a> to >+ confirm the data is correct.</p >+ > >+ <pre class="error">[% nonxml_stripped | html %]</pre> >+ </div> >+ [% END %] > [% IF decoding_error || analytics_error %] > <div class="alert alert-warning"> > <h1>Errors found</h1> >diff --git a/t/db_dependent/Koha/Biblio/Metadata.t b/t/db_dependent/Koha/Biblio/Metadata.t >index ad3138df690..177bf94d482 100755 >--- a/t/db_dependent/Koha/Biblio/Metadata.t >+++ b/t/db_dependent/Koha/Biblio/Metadata.t >@@ -75,7 +75,7 @@ EOX > > lives_ok { $record->store } > 'Valid MARCXML record stores successfully'; >- is( $record->stripped_nonxml, undef, 'stripped_nonxml is undef for clean record' ); >+ is( $record->nonxml_stripped, undef, 'nonxml_stripped is undef for clean record' ); > > $schema->storage->txn_rollback; > }; >@@ -104,7 +104,7 @@ EOX > > lives_ok { $record->store } 'MARCXML with strippable characters stores successfully'; > >- ok( $record->stripped_nonxml, 'stripped_nonxml is set after stripping' ); >+ ok( $record->nonxml_stripped, 'nonxml_stripped is set after stripping' ); > > my $stored = Koha::Biblio::Metadatas->find( { biblionumber => $biblio->{biblionumber}, format => 'marcxml' } ); > is( >-- >2.53.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 35104
:
157625
|
157627
|
157628
|
157642
|
159615
|
159620
|
159621
|
159630
|
159810
|
159811
|
159812
|
159813
|
166431
|
173145
|
173146
|
176820
|
176821
|
176850
|
176851
|
176852
|
176860
|
193985
|
193986
|
193987
|
193988
|
193989
|
193990
|
193991
|
193992
|
193993
|
193994
|
193995
|
193996
|
193997
|
193998
|
193999
|
194000
|
194001
|
194004
|
194005
|
194006
|
194007
|
194008
|
194009
|
194010
|
194011
|
194012
|
194013