Bug 29980 - Validate ISBN when cataloguing bibliographic records
Summary: Validate ISBN when cataloguing bibliographic records
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Aleisha Amohia
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-31 03:27 UTC by Aleisha Amohia
Modified: 2025-10-10 06:26 UTC (History)
6 users (show)

See Also:
GIT URL:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a new system preference ValidateISBN. When enabled, Koha will apply a valid checksum algorithm to ISBN fields (020$a for MARC21 and 010$a for UNIMARC) to validate the ISBN when cataloguing bibliographic records. The user will be prompted to fix their invalid ISBN, or have the option to bypass the ISBN check and save anyway. This will work for both 10-digit and 13-digit ISBNs. This patch also cleans up some bad indentation in cataloguing/addbiblio.pl.
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 29980: Add ValidateISBN system preference (3.35 KB, patch)
2022-01-31 04:20 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 29980: Validate ISBNs when saving biblio records (10.52 KB, patch)
2022-01-31 04:20 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 29980: Add ValidateISBN system preference (3.31 KB, patch)
2022-04-30 04:09 UTC, David Nind
Details | Diff | Splinter Review
Bug 29980: Validate ISBNs when saving biblio records (10.56 KB, patch)
2022-04-30 04:09 UTC, David Nind
Details | Diff | Splinter Review
Screenshot - placement of error messages when other errors (48.48 KB, image/png)
2022-04-30 04:32 UTC, David Nind
Details
Bug 29980: [alternative patch] Add a cataloguing plugin to check ISBN (2.78 KB, patch)
2025-09-08 15:24 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 29980: Add a cataloguing plugin to check ISBN (4.71 KB, patch)
2025-09-17 08:18 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 29980: Add a cataloguing plugin to check ISBN (4.71 KB, patch)
2025-09-17 08:22 UTC, Arthur Suzuki
Details | Diff | Splinter Review
Bug 29980: Add a cataloguing plugin to check ISBN (4.75 KB, patch)
2025-10-08 14:53 UTC, Johannes Reuter
Details | Diff | Splinter Review
Bug 29980: Add a cataloguing plugin to check ISBN (4.80 KB, patch)
2025-10-08 15:19 UTC, Thomas Klausner
Details | Diff | Splinter Review
Bug 29980: (follow-up) Fix GNU license link (875 bytes, patch)
2025-10-09 22:31 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2022-01-31 03:27:29 UTC
https://metacpan.org/pod/Business::ISBN

Business::ISBN has a valid_isbn_checksum method that implements an ISBN check digit algorithm for both 10-digit and 13-digit ISBNs. Koha could adopt this to validate ISBNs when cataloguing biblios.
Comment 1 Aleisha Amohia 2022-01-31 04:20:02 UTC
Created attachment 129984 [details] [review]
Bug 29980: Add ValidateISBN system preference
Comment 2 Aleisha Amohia 2022-01-31 04:20:06 UTC
Created attachment 129985 [details] [review]
Bug 29980: Validate ISBNs when saving biblio records

This enhancement adds a new system preference ValidateISBN. When
enabled, Koha will apply a valid checksum algorithm to ISBN fields
(020$a for MARC21 and 010$a for UNIMARC) to validate the ISBN when
cataloguing bibliographic records. The user will be prompted to fix
their invalid ISBN, or have the option to bypass the ISBN check and
save anyway. This will work for both 10-digit and 13-digit ISBNs.

This patch also cleans up some bad indentation in
cataloguing/addbiblio.pl.

To test:
1) Apply patch, update database.
2) Go to Koha administration -> global system preferences -> cataloguing
tab -> Record structure heading. Note the new ValidateISBN system
preference. It should be disabled by default. Leave it disabled.
3) Set marcflavour system preference to MARC21.
4) In a new tab, go to add a new biblio record, or edit an existing one.
5) Find the 020$a field and input an incorrect ISBN. Save the record and
confirm the ISBN is not validated and does not block saving the record.
6) Go back to your system preferences tab. Set the ValidateISBN tab to
"Validate" to enable it.
7) Go back to your cataloguing tab. Edit the record. Save the record
without making any changes.
8) Confirm an Invalid ISBN error pops up, blocking you from saving.
Click Continue to bypass the ISBN check and save the record with the
invalid ISBN.
9) Edit the record again. Save the record without making any changes.
10) When the Invalid ISBN error pops up, this time fix the invalid ISBN,
then save the record again. Confirm you are able to save the record.
11) Go back to the system preferences tab and change the marcflavour
system preference to UNIMARC. Repeat the above steps, but this time use
the 010$a field instead of 020$a for ISBN.

Sponsored-by: Education Services Australia SCIS
Comment 3 Shi Yao Wang 2022-03-10 21:47:15 UTC
Everything works until I switch marcflavour from MARC21 to UNIMARC. Error 500 appears when I search the catalog to continue the steps. Did the switch on origin/master and the same happens.
Comment 4 Katrin Fischer 2022-03-10 21:59:36 UTC
If you system hasn't been set up for UNIMARC from the start, it#s to be expected that the search does not work. What will probably work: Keep your record open in cataloguing and refresh the page after switching the marcflavor. This is just a workaround so you can test this on a MARC21 system, but it should be ok.
Comment 5 Aleisha Amohia 2022-03-10 22:03:30 UTC
(In reply to Katrin Fischer from comment #4)
> If you system hasn't been set up for UNIMARC from the start, it#s to be
> expected that the search does not work. What will probably work: Keep your
> record open in cataloguing and refresh the page after switching the
> marcflavor. This is just a workaround so you can test this on a MARC21
> system, but it should be ok.

Thanks for this suggestion Katrin!
Comment 6 Shi Yao Wang 2022-03-10 22:14:04 UTC
I kept one tab with a record open and another tab with system preferences open. Then I switched marcflavour to UNIMARC, went to the other tab, clicked edit and then error 500 appears once again.
Comment 7 Aleisha Amohia 2022-03-10 22:15:37 UTC
Maybe try clicking Edit and then changing the marcflavour, so you're refreshing the page already in the editor?

If that still doesn't work I'll take another look.
Comment 8 Shi Yao Wang 2022-03-10 22:24:24 UTC
I tried it and modified 010$a field but when I clicked save the same error 500 pops up. Maybe my system HAS to support UNIMARC for this to work but I don't really know. Sorry I couldn't test it.
Comment 9 Aleisha Amohia 2022-03-10 22:25:45 UTC
No worries I'll take a look.
Comment 10 Aleisha Amohia 2022-03-10 23:07:14 UTC
Looks like that error occurs because you don't have any UNIMARC records or frameworks in your Koha system.

When testing for UNIMARC, try to import a UNIMARC record and test with that!
Comment 11 David Nind 2022-04-30 04:09:35 UTC
Created attachment 134432 [details] [review]
Bug 29980: Add ValidateISBN system preference

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2022-04-30 04:09:40 UTC
Created attachment 134433 [details] [review]
Bug 29980: Validate ISBNs when saving biblio records

This enhancement adds a new system preference ValidateISBN. When
enabled, Koha will apply a valid checksum algorithm to ISBN fields
(020$a for MARC21 and 010$a for UNIMARC) to validate the ISBN when
cataloguing bibliographic records. The user will be prompted to fix
their invalid ISBN, or have the option to bypass the ISBN check and
save anyway. This will work for both 10-digit and 13-digit ISBNs.

This patch also cleans up some bad indentation in
cataloguing/addbiblio.pl.

To test:
1) Apply patch, update database.
2) Go to Koha administration -> global system preferences -> cataloguing
tab -> Record structure heading. Note the new ValidateISBN system
preference. It should be disabled by default. Leave it disabled.
3) Set marcflavour system preference to MARC21.
4) In a new tab, go to add a new biblio record, or edit an existing one.
5) Find the 020$a field and input an incorrect ISBN. Save the record and
confirm the ISBN is not validated and does not block saving the record.
6) Go back to your system preferences tab. Set the ValidateISBN tab to
"Validate" to enable it.
7) Go back to your cataloguing tab. Edit the record. Save the record
without making any changes.
8) Confirm an Invalid ISBN error pops up, blocking you from saving.
Click Continue to bypass the ISBN check and save the record with the
invalid ISBN.
9) Edit the record again. Save the record without making any changes.
10) When the Invalid ISBN error pops up, this time fix the invalid ISBN,
then save the record again. Confirm you are able to save the record.
11) Go back to the system preferences tab and change the marcflavour
system preference to UNIMARC. Repeat the above steps, but this time use
the 010$a field instead of 020$a for ISBN.

Sponsored-by: Education Services Australia SCIS

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2022-04-30 04:32:21 UTC
Created attachment 134434 [details]
Screenshot - placement of error messages when other errors

Testing notes (using koha-testing-docker):

- I have tested in both MARC21 and UNIMARC environments.

- When using koha-testing-docker you can't just change the MARC flavour. Before starting it up you need to change KOHA_MARC_FLAVOUR=marc21 in your .env file to KOHA_MARC_FLAVOUR=unimarc ; to change back to a MARC21 environment shut everything down (CTRL-C and then kd), and change it back to marc21. It is also possible to get a UNIMARC installation using the sandboxes - change the MARC flavour when you create the sandbox (see https://wiki.koha-community.org/wiki/Sandboxes). Basically, I found that I had to test in a MARC21 environment, then set up fresh in a UNIMARC one.

- One thing I noted is that if there are other errors, they display ABOVE the title - see the screenshot. It may pay to check with Owen about this. To me the errors should be below the title, like happens with the invalid ISBN error message. So I don't think that is an issue for this bug.

- The other thing I noted was that if a record is saved with an invalid ISBN and you then 1) edit it again 2) introduce other errors (such as deleting the 942$c Koha item type) 3) save, the invalid ISBN error doesn't display until after the other errors are resolved.
Comment 14 Marcel de Rooy 2022-05-03 12:18:15 UTC
QA:
Please remove indentation corrections out of your regular patches. They obscure the review. Put them separately (always). Thanks :)

Why dont you use Koha::NormalizeISBN and friends ?

FQA
Comment 15 Marcel de Rooy 2022-05-03 12:21:16 UTC
Just another thought, but you may leave it for another report (since it may be adjusting your scope):
What about checking if this ISBN is already used in another bib record?
Comment 16 Katrin Fischer 2022-05-03 16:03:32 UTC
(In reply to Marcel de Rooy from comment #15)
> Just another thought, but you may leave it for another report (since it may
> be adjusting your scope):
> What about checking if this ISBN is already used in another bib record?

I think it would not necessarily be a duplicate, if you treat different editions. I think mixing in duplicate checking would be out of scope here.

We already use Business::ISBN in other places - it seems ok to me. Where would you use NormalizeIBSN?
Comment 17 Marcel de Rooy 2022-05-04 11:14:41 UTC
(In reply to Katrin Fischer from comment #16)
> (In reply to Marcel de Rooy from comment #15)
> > Just another thought, but you may leave it for another report (since it may
> > be adjusting your scope):
> > What about checking if this ISBN is already used in another bib record?
> 
> I think it would not necessarily be a duplicate, if you treat different
> editions. I think mixing in duplicate checking would be out of scope here.

Fine
Comment 18 Marcel de Rooy 2022-05-04 11:22:46 UTC
Found another thing too:

Look at this piece of code:
my $r = MARC::Record->new;
my $f1 = MARC::Field->new('020','','', a => 'isbn1' );
my $f2 = MARC::Field->new('020','','', a => 'isbn2' );
$r->append_fields( $f1, $f2 );
my $scal = $r->subfield( '020', 'a' );
my @list = $r->subfield( '020', 'a' );
print Dumper( $scal, '-', @list );

It prints:
$VAR1 = 'isbn1';
$VAR2 = '-';
$VAR3 = 'isbn1';

Which means that the following code does not work:
+    my @input_isbns = ( C4::Context->preference('marcflavour') eq "UNIMARC" ) ? $record->subfield('010','a') : $record->subfield('020','a');

The @input_isbns will just contain the first ISBN only.
Comment 19 Marcel de Rooy 2022-05-04 11:30:58 UTC
The previous comment illustrates why we need code that can be tested easily. We do not achieve with putting all code in scripts but by using modules.

(In reply to Katrin Fischer from comment #16)
> We already use Business::ISBN in other places - it seems ok to me. Where
> would you use NormalizeIBSN?

The intention of this remark is: let's not call Business::ISBN everywhere in scripts. Use a module where we already have that functionality.
The above remark serves to illustrate my point.
Comment 20 Marcel de Rooy 2022-05-04 11:34:34 UTC
Another thing while revisiting this bug:
We are building tabs again, passing an invalid_isbn flag, etc. It would be better imo to put this code in a (default) cataloguing plugin with a very small ajax call (or just javascript, but then we cannot use the perl module).

Earlier validation, not just at time of saving biblio record.
Comment 21 Arthur Suzuki 2025-09-03 08:19:22 UTC
(In reply to Marcel de Rooy from comment #20)
> Another thing while revisiting this bug:
> We are building tabs again, passing an invalid_isbn flag, etc. It would be
> better imo to put this code in a (default) cataloguing plugin with a very
> small ajax call (or just javascript, but then we cannot use the perl module).
> 
> Earlier validation, not just at time of saving biblio record.

I agree with Marcel, also I think having a systempref for this is a bit "heavy lifting" for the task. I will try to provide a new patch with a cataloguing plugin asap.
One question though, do you think the cataloguing plugin should be enabled by default?
Or should just be available to use but have to be enabled/used in the cataloguing framework in use (and attached to the proper field/subfield according to your framework).
Comment 22 Marcel de Rooy 2025-09-04 09:47:49 UTC
(In reply to Arthur Suzuki from comment #21)
> I agree with Marcel, also I think having a systempref for this is a bit
> "heavy lifting" for the task. I will try to provide a new patch with a
> cataloguing plugin asap.
> One question though, do you think the cataloguing plugin should be enabled
> by default?
> Or should just be available to use but have to be enabled/used in the
> cataloguing framework in use (and attached to the proper field/subfield
> according to your framework).

I would not mind having it as default in a new system. Would not recommend to change it on existing ones. Just advertize in the release notes that they can set it.
Comment 23 Arthur Suzuki 2025-09-08 15:24:21 UTC
Created attachment 186261 [details] [review]
Bug 29980: [alternative patch] Add a cataloguing plugin to check ISBN
Comment 24 Arthur Suzuki 2025-09-08 15:29:59 UTC
Hello,
This is an alternative patch.
It's a cataloguing plugin (value builder) which you can attach to any subfield to check ISBN.
To test, edit the cataloguing framework and attach the value builder to any field containing an ISBN.
Then when you edit the field, it will alert and say "Valid" or "Invalid" ISBN when the field looses focus.
However, I don't know if it's possible for value builders to block the form submission (maybe improve the JS?).
Also I've seen there are methods in Business::ISBN to fix the ISBN if only the checksum part is invalid.
That could be very useful when you want to change from isbn10 to isbn13 but don't know / want to compute the checksum by hand.
This is a draft patch, any ideas for improvement are welcome.
(I have to submit a test plan as well...).
Comment 25 Arthur Suzuki 2025-09-17 08:18:30 UTC
Created attachment 186509 [details] [review]
Bug 29980: Add a cataloguing plugin to check ISBN

Test plan:
- without patch login to Koha
- edit a biblio and put a wrong ISBN in 020a (010a for unimarc).
- leave the input field (looses focus), no warning
- apply patch and do "reset_all" to make a full re-install.
- edit the biblio again and put an invalid ISBN in 020a (010a for unimarc).
- when you leave the input field you should have a warning saying "invalid ISBN".
- the input field should turn yellow
- you can still save the biblio with a wrong isbn, it is just a warning.
- if you apply the patch on an existing install, you must manually link the value builder to your existing frameworks to get it working.
Comment 26 Arthur Suzuki 2025-09-17 08:22:48 UTC
Created attachment 186510 [details] [review]
Bug 29980: Add a cataloguing plugin to check ISBN

Test plan:
- without patch login to Koha
- edit a biblio and put a wrong ISBN in 020a (010a for unimarc).
- leave the input field (looses focus), no warning
- apply patch and do "reset_all" to make a full re-install.
- edit the biblio again and put an invalid ISBN in 020a (010a for unimarc).
- when you leave the input field you should have a warning saying "invalid ISBN".
- the input field should turn yellow
- you can still save the biblio with a wrong isbn, it is just a warning.
- if you apply the patch on an existing install, you must manually link the value builder to your existing frameworks to get it working.
Comment 27 Arthur Suzuki 2025-09-17 08:27:30 UTC
(In reply to Marcel de Rooy from comment #22)
> I would not mind having it as default in a new system. Would not recommend
> to change it on existing ones. Just advertize in the release notes that they
> can set it.

It is ready for testing, I've added a test plan.
It will be installed on a new system, if you apply the patch before starting ktd or if you "reset_all" after applying patch the value builder will be linked with the proper marc subfield.
Existing system will not get the value_builder attached, it has to be done manually in your marc framework configuration.
QA scripts are not complaining, I've made the previous patches obsolete hope that's ok.
Best,
Arthur
Comment 28 Johannes Reuter 2025-10-08 14:53:04 UTC
Created attachment 187583 [details] [review]
Bug 29980: Add a cataloguing plugin to check ISBN

Test plan:
- without patch login to Koha
- edit a biblio and put a wrong ISBN in 020a (010a for unimarc).
- leave the input field (looses focus), no warning
- apply patch and do "reset_all" to make a full re-install.
- edit the biblio again and put an invalid ISBN in 020a (010a for unimarc).
- when you leave the input field you should have a warning saying "invalid ISBN".
- the input field should turn yellow
- you can still save the biblio with a wrong isbn, it is just a warning.
- if you apply the patch on an existing install, you must manually link the value builder to your existing frameworks to get it working.

Signed-off-by: Johannes Reuter <reuter@die-bonn.de>
Comment 29 Thomas Klausner 2025-10-08 15:19:17 UTC
Created attachment 187586 [details] [review]
Bug 29980: Add a cataloguing plugin to check ISBN

Test plan:
- without patch login to Koha
- edit a biblio and put a wrong ISBN in 020a (010a for unimarc).
- leave the input field (looses focus), no warning
- apply patch and do "reset_all" to make a full re-install.
- edit the biblio again and put an invalid ISBN in 020a (010a for unimarc).
- when you leave the input field you should have a warning saying "invalid ISBN".
- the input field should turn yellow
- you can still save the biblio with a wrong isbn, it is just a warning.
- if you apply the patch on an existing install, you must manually link the value builder to your existing frameworks to get it working.

Signed-off-by: Johannes Reuter <reuter@die-bonn.de>
Signed-off-by: Thomas Klausner <domm@plix.at>
Comment 30 Lucas Gass (lukeg) 2025-10-09 21:01:17 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 31 Lucas Gass (lukeg) 2025-10-09 22:31:43 UTC
Created attachment 187709 [details] [review]
Bug 29980: (follow-up) Fix GNU license link
Comment 32 Lucas Gass (lukeg) 2025-10-09 22:48:09 UTC
follow-up pushed to main
Comment 33 Marcel de Rooy 2025-10-10 06:26:21 UTC
Great to have that! Forgive my boldness, but could we expect a counterpart for ISSN too btw?