Summary: | Validate ISBN when cataloguing bibliographic records | ||
---|---|---|---|
Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
Component: | Cataloging | Assignee: | Aleisha Amohia <aleisha> |
Status: | Failed QA --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | david, m.de.rooy, shi-yao.wang |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
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: | Circulation function: | ||
Attachments: |
Bug 29980: Add ValidateISBN system preference
Bug 29980: Validate ISBNs when saving biblio records Bug 29980: Add ValidateISBN system preference Bug 29980: Validate ISBNs when saving biblio records Screenshot - placement of error messages when other errors |
Description
Aleisha Amohia
2022-01-31 03:27:29 UTC
Created attachment 129984 [details] [review] Bug 29980: Add ValidateISBN system preference 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 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. 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. (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! 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. 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. 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. No worries I'll take a look. 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! Created attachment 134432 [details] [review] Bug 29980: Add ValidateISBN system preference Signed-off-by: David Nind <david@davidnind.com> 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> 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. 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 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? (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? (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 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. 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. 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. |