Bug 30326 - Cronjob that checks biblios for MARC errors
Summary: Cronjob that checks biblios for MARC errors
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-22 02:27 UTC by Aleisha Amohia
Modified: 2022-05-02 01:09 UTC (History)
3 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
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:


Attachments
Bug 30326: check_marc_errors.pl script which prints MARC errors (6.16 KB, patch)
2022-03-22 03:34 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30326: check_marc_errors.pl script which prints MARC errors (6.16 KB, patch)
2022-03-22 20:36 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30326: check_marc_errors.pl script which prints MARC errors (6.22 KB, patch)
2022-03-23 12:49 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30326: (follow-up) Add output parameter and handle bad biblionumbers (3.78 KB, patch)
2022-04-21 02:43 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30326: (follow-up) Add output parameter and handle bad biblionumbers (3.83 KB, patch)
2022-04-22 12:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL (2.21 KB, patch)
2022-04-29 03:01 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL (2.69 KB, patch)
2022-05-02 01:09 UTC, Aleisha Amohia
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-03-22 02:27:58 UTC
It would be useful to have a script that could be run regularly which reported on biblios with MARC errors.
Comment 1 Aleisha Amohia 2022-03-22 03:34:19 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
Comment 2 Owen Leonard 2022-03-22 12:40:27 UTC
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.
Comment 3 Aleisha Amohia 2022-03-22 20:36:41 UTC
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
Comment 4 Owen Leonard 2022-03-23 12:49:41 UTC
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>
Comment 5 Tomás Cohen Arazi 2022-04-20 13:52:46 UTC
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.
Comment 6 Aleisha Amohia 2022-04-21 02:39:02 UTC
(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.
Comment 7 Aleisha Amohia 2022-04-21 02:43:23 UTC
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.
Comment 8 Owen Leonard 2022-04-22 12:33:38 UTC
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>
Comment 9 Katrin Fischer 2022-04-28 16:38:53 UTC
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!
Comment 10 David Cook 2022-04-28 23:38:56 UTC
(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?
Comment 11 Aleisha Amohia 2022-04-29 03:01:48 UTC
Created attachment 134345 [details] [review]
Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL
Comment 12 Katrin Fischer 2022-04-29 08:58:53 UTC
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!
Comment 13 Aleisha Amohia 2022-05-02 01:09:24 UTC
Created attachment 134447 [details] [review]
Bug 30326: (follow-up) Tidy up output and use staffClientBaseURL