Bugzilla – Attachment 194225 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: Add 'Mark resolved' button for nonxml_stripped errors in edit form
078addf.patch (text/plain), 12.59 KB, created by
Martin Renvoize (ashimema)
on 2026-03-01 16:23:53 UTC
(
hide
)
Description:
Bug 35104: Add 'Mark resolved' button for nonxml_stripped errors in edit form
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-03-01 16:23:53 UTC
Size:
12.59 KB
patch
obsolete
>From 078addfabd6a64af7514fdbdb27b3f08f2a2fee7 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Sat, 28 Feb 2026 21:50:45 +0000 >Subject: [PATCH] Bug 35104: Add 'Mark resolved' button for nonxml_stripped > errors in edit form > >Extend the data quality warnings banner in the MARC editor with a >per-error 'Mark resolved' button. Clicking it calls a new REST endpoint >(DELETE /api/v1/biblios/{biblio_id}/metadata/errors/{error_id}) which >removes the corresponding biblio_metadata_errors row. The button is then >hidden client-side to give immediate feedback. > >The new Koha::REST::V1::Biblios::MetadataErrors controller handles the >delete, verifying that the requested error belongs to the requested biblio >before removing it. > >Sponsored-by: OpenFifth >--- > Koha/REST/V1/Biblios/MetadataErrors.pm | 58 +++++++++ > .../paths/biblios_metadata_errors.yaml | 48 +++++++ > api/v1/swagger/swagger.yaml | 2 + > .../prog/en/modules/cataloguing/addbiblio.tt | 25 +++- > .../api/v1/biblios_metadata_errors.t | 123 ++++++++++++++++++ > 5 files changed, 254 insertions(+), 2 deletions(-) > create mode 100644 Koha/REST/V1/Biblios/MetadataErrors.pm > create mode 100644 api/v1/swagger/paths/biblios_metadata_errors.yaml > create mode 100644 t/db_dependent/api/v1/biblios_metadata_errors.t > >diff --git a/Koha/REST/V1/Biblios/MetadataErrors.pm b/Koha/REST/V1/Biblios/MetadataErrors.pm >new file mode 100644 >index 00000000000..2e784225a32 >--- /dev/null >+++ b/Koha/REST/V1/Biblios/MetadataErrors.pm >@@ -0,0 +1,58 @@ >+package Koha::REST::V1::Biblios::MetadataErrors; >+ >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <https://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+ >+use Mojo::Base 'Mojolicious::Controller'; >+ >+use Koha::Biblio::Metadata::Errors; >+use Koha::Biblios; >+ >+use Try::Tiny; >+ >+=head1 NAME >+ >+Koha::REST::V1::Biblios::MetadataErrors - REST controller for biblio metadata errors >+ >+=head1 API >+ >+=head2 Methods >+ >+=head3 delete >+ >+Delete (resolve) a single biblio metadata error row. >+ >+=cut >+ >+sub delete { >+ my $c = shift->openapi->valid_input or return; >+ >+ my $biblio = Koha::Biblios->find( $c->param('biblio_id') ); >+ return $c->render_resource_not_found("Biblio") unless $biblio; >+ >+ my $error = $biblio->metadata->metadata_errors->find( $c->param('error_id') ); >+ return $c->render_resource_not_found("Metadata error") unless $error; >+ >+ return try { >+ $error->delete; >+ return $c->render_resource_deleted; >+ } catch { >+ $c->unhandled_exception($_); >+ }; >+} >+ >+1; >diff --git a/api/v1/swagger/paths/biblios_metadata_errors.yaml b/api/v1/swagger/paths/biblios_metadata_errors.yaml >new file mode 100644 >index 00000000000..a8252831b70 >--- /dev/null >+++ b/api/v1/swagger/paths/biblios_metadata_errors.yaml >@@ -0,0 +1,48 @@ >+--- >+"/biblios/{biblio_id}/metadata/errors/{error_id}": >+ delete: >+ x-mojo-to: Biblios::MetadataErrors#delete >+ operationId: deleteBiblioMetadataError >+ tags: >+ - biblios >+ summary: Delete a biblio metadata error >+ description: Deletes a specific metadata error record, marking it as resolved. >+ parameters: >+ - name: biblio_id >+ in: path >+ description: Internal identifier for the bibliographic record >+ required: true >+ type: integer >+ - name: error_id >+ in: path >+ description: Internal identifier for the metadata error >+ required: true >+ type: integer >+ produces: >+ - application/json >+ responses: >+ '204': >+ description: Metadata error deleted >+ '401': >+ description: Authentication required >+ schema: >+ "$ref": "../swagger.yaml#/definitions/error" >+ '403': >+ description: Access forbidden >+ schema: >+ "$ref": "../swagger.yaml#/definitions/error" >+ '404': >+ description: Metadata error not found >+ schema: >+ "$ref": "../swagger.yaml#/definitions/error" >+ '500': >+ description: Internal error >+ schema: >+ "$ref": "../swagger.yaml#/definitions/error" >+ '503': >+ description: Under maintenance >+ schema: >+ "$ref": "../swagger.yaml#/definitions/error" >+ x-koha-authorization: >+ permissions: >+ editcatalogue: edit_catalogue >diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml >index 9a9b7af7565..7b6cfcf00d9 100644 >--- a/api/v1/swagger/swagger.yaml >+++ b/api/v1/swagger/swagger.yaml >@@ -299,6 +299,8 @@ paths: > $ref: "./paths/biblios_item_groups.yaml#/~1biblios~1{biblio_id}~1item_groups~1{item_group_id}~1items" > "/biblios/{biblio_id}/item_groups/{item_group_id}/items/{item_id}": > $ref: "./paths/biblios_item_groups.yaml#/~1biblios~1{biblio_id}~1item_groups~1{item_group_id}~1items~1{item_id}" >+ "/biblios/{biblio_id}/metadata/errors/{error_id}": >+ $ref: "./paths/biblios_metadata_errors.yaml#/~1biblios~1{biblio_id}~1metadata~1errors~1{error_id}" > "/biblios/{biblio_id}/merge": > $ref: "./paths/biblios_merge.yaml#/~1biblios~1{biblio_id}~1merge" > /cash_registers: >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt >index dea2d4911ad..9614bf5ff3f 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt >@@ -208,6 +208,26 @@ > window.scrollTo(0, getScrollto($li.attr("id"), "toolbar")); > }); > >+ $("body").on("click", ".nonxml-resolve", function(e){ >+ e.preventDefault(); >+ var errorId = $(this).data("error-id"); >+ var biblioId = $(this).data("biblio-id"); >+ var $item = $("#nonxml-error-" + errorId); >+ $.ajax({ >+ url: "/api/v1/biblios/" + biblioId + "/metadata/errors/" + errorId, >+ method: "DELETE", >+ success: function(){ >+ $item.remove(); >+ if ( $("#nonxml-error-list li").length === 0 ) { >+ $("#nonxml-errors-notice").remove(); >+ } >+ }, >+ error: function(){ >+ alert( _("Failed to mark error as resolved. Please try again.") ); >+ } >+ }); >+ }); >+ > }); > > function selectTab( tablink ){ >@@ -817,9 +837,9 @@ > <div class="alert alert-warning" id="nonxml-errors-notice"> > <h2>Data quality warnings</h2> > <p><strong>Non-XML characters were automatically stripped from this record when it was last saved. Please review the affected fields and correct or confirm the data.</strong></p> >- <ul> >+ <ul id="nonxml-error-list"> > [% FOREACH err IN existing_nonxml_errors %] >- <li> >+ <li id="nonxml-error-[% err.id | html %]"> > [% IF err.tag %] > <a class="nonxml-linkfield btn btn-link" href="#" data-tag="[% err.tag | html %]" [% IF err.subfield %]data-subfield="[% err.subfield | html %]"[% END %]> > <i class="fa fa-arrow-right" aria-hidden="true"></i> >@@ -827,6 +847,7 @@ > </a> > [% END %] > <code>[% err.message | html %]</code> >+ <button class="nonxml-resolve btn btn-default btn-xs" data-error-id="[% err.id | html %]" data-biblio-id="[% biblionumber | html %]"> Mark resolved </button> > </li> > [% END %] > </ul> >diff --git a/t/db_dependent/api/v1/biblios_metadata_errors.t b/t/db_dependent/api/v1/biblios_metadata_errors.t >new file mode 100644 >index 00000000000..1946f6cd9a0 >--- /dev/null >+++ b/t/db_dependent/api/v1/biblios_metadata_errors.t >@@ -0,0 +1,123 @@ >+#!/usr/bin/env perl >+ >+# This file is part of Koha. >+# >+# Koha is free software; you can redistribute it and/or modify it >+# under the terms of the GNU General Public License as published by >+# the Free Software Foundation; either version 3 of the License, or >+# (at your option) any later version. >+# >+# Koha is distributed in the hope that it will be useful, but >+# WITHOUT ANY WARRANTY; without even the implied warranty of >+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >+# GNU General Public License for more details. >+# >+# You should have received a copy of the GNU General Public License >+# along with Koha; if not, see <https://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+ >+use Test::More tests => 2; >+use Test::NoWarnings; >+use Test::Mojo; >+ >+use t::lib::Mocks; >+use t::lib::TestBuilder; >+ >+use Koha::Biblio::Metadata::Error; >+use Koha::Database; >+ >+my $schema = Koha::Database->new->schema; >+my $builder = t::lib::TestBuilder->new; >+ >+t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); >+ >+my $t = Test::Mojo->new('Koha::REST::V1'); >+ >+subtest 'delete() tests' => sub { >+ >+ plan tests => 13; >+ >+ $schema->storage->txn_begin; >+ >+ my $password = 'thePassword123'; >+ >+ my $unauthorized_patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 0 } >+ } >+ ); >+ $unauthorized_patron->set_password( { password => $password, skip_validation => 1 } ); >+ my $unauth_userid = $unauthorized_patron->userid; >+ >+ my $authorized_patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 0 } >+ } >+ ); >+ $authorized_patron->set_password( { password => $password, skip_validation => 1 } ); >+ my $auth_userid = $authorized_patron->userid; >+ >+ $builder->build( >+ { >+ source => 'UserPermission', >+ value => { >+ borrowernumber => $authorized_patron->borrowernumber, >+ module_bit => 9, >+ code => 'edit_catalogue', >+ }, >+ } >+ ); >+ >+ my $biblio = $builder->build_sample_biblio; >+ my $metadata_id = $biblio->metadata->id; >+ >+ my $error = Koha::Biblio::Metadata::Error->new( >+ { >+ metadata_id => $metadata_id, >+ error_type => 'nonxml_stripped', >+ message => '245$a: invalid char value 31 (U+001F) at position 5', >+ } >+ )->store; >+ my $error_id = $error->id; >+ >+ # Unauthenticated >+ $t->delete_ok( "/api/v1/biblios/" . $biblio->biblionumber . "/metadata/errors/$error_id" ) >+ ->status_is( 401, 'Unauthenticated request returns 401' ); >+ >+ # Unauthorized (no permission) >+ $t->delete_ok( >+ "//$unauth_userid:$password@/api/v1/biblios/" . $biblio->biblionumber . "/metadata/errors/$error_id" ) >+ ->status_is( 403, 'Missing permission returns 403' ); >+ >+ # Biblio not found >+ $t->delete_ok("//$auth_userid:$password@/api/v1/biblios/0/metadata/errors/$error_id") >+ ->status_is( 404, 'Non-existent biblio returns 404' ); >+ >+ # Error not found (wrong error_id for this biblio) >+ my $other_biblio = $builder->build_sample_biblio; >+ my $other_error = Koha::Biblio::Metadata::Error->new( >+ { >+ metadata_id => $other_biblio->metadata->id, >+ error_type => 'nonxml_stripped', >+ message => '245$a: invalid char value 31 (U+001F) at position 5', >+ } >+ )->store; >+ >+ $t->delete_ok( >+ "//$auth_userid:$password@/api/v1/biblios/" . $biblio->biblionumber . "/metadata/errors/" . $other_error->id ) >+ ->status_is( 404, 'Error belonging to a different biblio returns 404' ); >+ >+ # Successful delete >+ $t->delete_ok( "//$auth_userid:$password@/api/v1/biblios/" . $biblio->biblionumber . "/metadata/errors/$error_id" ) >+ ->status_is( 204, 'Authorized delete of existing error returns 204' ) >+ ->content_is( '', 'No response body on successful delete' ); >+ >+ # Already deleted / not found >+ $t->delete_ok( "//$auth_userid:$password@/api/v1/biblios/" . $biblio->biblionumber . "/metadata/errors/$error_id" ) >+ ->status_is( 404, 'Deleting an already-deleted error returns 404' ); >+ >+ $schema->storage->txn_rollback; >+}; >-- >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
|
194152
|
194153
|
194154
|
194155
|
194156
|
194157
|
194158
|
194159
|
194160
|
194161
|
194162
|
194163
|
194164
|
194165
|
194166
|
194167
|
194168
|
194169
|
194170
|
194171
|
194172
|
194213
|
194214
|
194215
|
194216
|
194217
|
194218
|
194219
|
194220
|
194221
|
194222
|
194223
|
194224
| 194225 |
194226
|
194227