Summary: | Cronjob that checks biblios for MARC errors | ||
---|---|---|---|
Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
Component: | Command-line Utilities | Assignee: | Aleisha Amohia <aleisha> |
Status: | In Discussion --- | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | dcook, elizabethjohn6654, jonathan.druart, robin, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | Sponsored | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This new check_marc_errors.pl script uses the MARC::Lint package to check bibliographic records for MARC errors, and print any errors found. The script can check all biblios, or specified biblios.
|
Version(s) released in: | |
Circulation function: | |||
Attachments: |
Bug 30326: check_marc_errors.pl script which prints MARC errors
Bug 30326: check_marc_errors.pl script which prints MARC errors Bug 30326: check_marc_errors.pl script which prints MARC errors Bug 30326: (follow-up) Add output parameter and handle bad biblionumbers Bug 30326: (follow-up) Add output parameter and handle bad biblionumbers Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL |
Description
Aleisha Amohia
2022-03-22 02:27:58 UTC
Created attachment 131994 [details] [review] Bug 30326: check_marc_errors.pl script which prints MARC errors This new check_marc_errors.pl script uses the MARC::Lint package to check bibliographic records for MARC errors, and print any errors found. The script can check all biblios, or specified biblios. To test: 1. Run the script from your commandline. This should not be run from within your koha shell. Replace the --intranet parameter with your staff client URL. sudo koha-shell kohadev -c "misc/cronjobs/check_marc_errors.pl --intranet http://localhost:8081/" > /tmp/marc_errors.html 2. The above command will run the script for all biblios in your database and print the output to a file /tmp/marc_errors.html. View the file and confirm it has been populated as expected, with errors! cat /tmp/marc_errors.html 3. Run the script again, this time specify one or more biblionumbers. sudo koha-shell kohadev -c "misc/cronjobs/check_marc_errors.pl --bibnum 1 --bibnum 2 --bibnum 3 --intranet http://localhost:8081/" > /tmp/marc_errors.html 4. Confirm the file is populated as expected cat /tmp/marc_errors.html Sponsored-by: Catalyst IT This works well but there is a warning from the QA tool: FAIL misc/cronjobs/check_marc_errors.pl FAIL critic Subroutine "new" called using indirect syntax at line 99, column 18. See page 349 of PBP. Created attachment 132038 [details] [review] Bug 30326: check_marc_errors.pl script which prints MARC errors This new check_marc_errors.pl script uses the MARC::Lint package to check bibliographic records for MARC errors, and print any errors found. The script can check all biblios, or specified biblios. To test: 1. Run the script from your commandline. This should not be run from within your koha shell. Replace the --intranet parameter with your staff client URL. sudo koha-shell kohadev -c "misc/cronjobs/check_marc_errors.pl --intranet http://localhost:8081/" > /tmp/marc_errors.html 2. The above command will run the script for all biblios in your database and print the output to a file /tmp/marc_errors.html. View the file and confirm it has been populated as expected, with errors! cat /tmp/marc_errors.html 3. Run the script again, this time specify one or more biblionumbers. sudo koha-shell kohadev -c "misc/cronjobs/check_marc_errors.pl --bibnum 1 --bibnum 2 --bibnum 3 --intranet http://localhost:8081/" > /tmp/marc_errors.html 4. Confirm the file is populated as expected cat /tmp/marc_errors.html Sponsored-by: Catalyst IT Created attachment 132053 [details] [review] Bug 30326: check_marc_errors.pl script which prints MARC errors This new check_marc_errors.pl script uses the MARC::Lint package to check bibliographic records for MARC errors, and print any errors found. The script can check all biblios, or specified biblios. To test: 1. Run the script from your commandline. This should not be run from within your koha shell. Replace the --intranet parameter with your staff client URL. sudo koha-shell kohadev -c "misc/cronjobs/check_marc_errors.pl --intranet http://localhost:8081/" > /tmp/marc_errors.html 2. The above command will run the script for all biblios in your database and print the output to a file /tmp/marc_errors.html. View the file and confirm it has been populated as expected, with errors! cat /tmp/marc_errors.html 3. Run the script again, this time specify one or more biblionumbers. sudo koha-shell kohadev -c "misc/cronjobs/check_marc_errors.pl --bibnum 1 --bibnum 2 --bibnum 3 --intranet http://localhost:8081/" > /tmp/marc_errors.html 4. Confirm the file is populated as expected cat /tmp/marc_errors.html Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Hi, a couple remarks: 1. I'm not sure about the output, meaning that maybe as other scripts do, it could have an option switch for different formats. Not a blocker 2. In the --bibnum use case, it is not handling non-existing records correctly. It should report it correctly (right now it tells 'Must pass a MARC::Record object to check_record' which might be misleading for the end user). 3. I suggest you add a --where parameter, that is passed as a literal query to Koha::Biblios->search( $query, { prefetch => ['biblio_metadata'] } ); and use $biblio->metadata->record instead of GetMarcBiblio. Failing because of 2, primarily. (In reply to Tomás Cohen Arazi from comment #5) > 3. I suggest you add a --where parameter, that is passed as a literal query > to > > Koha::Biblios->search( $query, { prefetch => ['biblio_metadata'] } ); > > and use $biblio->metadata->record instead of GetMarcBiblio. I thought about doing it this way but it's significantly slower, not ideal when there are lots of biblios to check through. I can implement the other suggestions. Created attachment 133511 [details] [review] Bug 30326: (follow-up) Add output parameter and handle bad biblionumbers When the script is run without the --html parameter, the output will be simply text. When the script is run with the --html parameter, the output will display in an HTML table. Additional test plan: 5. Test running script with and without --html parameter and confirm output is correct. 6. Test running script with a --bibnum that does not exist and confirm the output acknowledges this. Created attachment 133674 [details] [review] Bug 30326: (follow-up) Add output parameter and handle bad biblionumbers When the script is run without the --html parameter, the output will be simply text. When the script is run with the --html parameter, the output will display in an HTML table. Additional test plan: 5. Test running script with and without --html parameter and confirm output is correct. 6. Test running script with a --bibnum that does not exist and confirm the output acknowledges this. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Hi Aleisha, this works, but some questions: 1) Could we not use staffClientBaseURL instead of or as a default/fallback for the --intranet option? This way it would work for multiple instances without having differentiate the crontab entries. 2) The HTML output is a little hard to read. 2 suggestions: a) Add a header row b) Add <br> instead of line breaks. The output on the command line is something like: <td>100: Indicator 1 must be 0, 1 or 3 but it's " " 245: Indicator 1 must be 0 or 1 but it's " " 245: Indicator 2 must be 0 thru 9 but it's " " 245: Must end with . (period). 245: Non-filing indicator is non-numeric 245: First word, clouded, does not appear to be an article, check 2nd indicator ( ).</td> But then the lines all get "mushed together" into a long block of text without any separators making it hard to read/take apart: 100: Indicator 1 must be 0, 1 or 3 but it's " " 245: Indicator 1 must be 0 or 1 but it's " " 245: Indicator 2 must be 0 thru 9 but it's " " 245: Must end with . (period). 245: Non-filing indicator is non-numeric 245: First word, clouded, does not appear to be an article, check 2nd indicator ( ). Could you take a look and give me some feedback? Thx! (In reply to Katrin Fischer from comment #9) > But then the lines all get "mushed together" into a long block of text > without any separators making it hard to read/take apart: Perhaps wrap it in "<pre></pre>" tags that preserve the original formatting from MARC::Lint? Created attachment 134345 [details] [review] Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL I'll be happy to treat the changes as a QA follow-up (no new sign-off required), but could you update POD for the changed intranet option behaviour? Thx! Created attachment 134447 [details] [review] Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL Isn't it the same as misc/maintenance/search_for_data_inconsistencies.pl? (In reply to Elizabeth John from comment #15) Spam |