|
Description
John Yorio
2025-09-09 15:53:59 UTC
Created attachment 187526 [details] [review] Bug 40777: Introduce Koha::Database::DataInconsistency Created attachment 187527 [details] [review] Bug 40777: Add 'Audit' button in the toolbar Created attachment 187528 [details] [review] Bug 40777: Add info in the alert Patches for discussion and feedback. I am suggesting to move the code from the search_for_inconsistencies.pl maintenance script to a module to make it reusable from the Koha UI. Those patch moves only the first check (no valid home or holding library) to the module, and add an "Audit" icon to the bibliographic toolbar. https://snipboard.io/1owIgH.jpg If people are willing to test and provide feedback, I will provide more work. From kohastructure `homebranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the branches table for the library that owns this item (MARC21 952$a)', `holdingbranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the branches table for the library that is currently in possession item (MARC21 952$b)', While both columns may still be NULL, we will have such issues. What about NOT NULL to ensure integrity? If we create an item somewhere or import it, we should probably know where it belongs. Just suspecting that invalid branchcodes during import may be at stake. And note that the REST API who is called to fetch the items, may need some extra check while we dont address it in the table design? (In reply to Marcel de Rooy from comment #5) > From kohastructure > `homebranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the > branches table for the library that owns this item (MARC21 952$a)', > `holdingbranch` varchar(10) DEFAULT NULL COMMENT 'foreign key from the > branches table for the library that is currently in possession item (MARC21 > 952$b)', > > While both columns may still be NULL, we will have such issues. > What about NOT NULL to ensure integrity? > If we create an item somewhere or import it, we should probably know where > it belongs. Just suspecting that invalid branchcodes during import may be at > stake. > > And note that the REST API who is called to fetch the items, may need some > extra check while we dont address it in the table design? This is outside the scope. I have implemented the first test of misc/maintenance/search_for_data_inconsistencies.pl, but there are more: * incorrect item type (yes you could argue that a FK is missing as well for this one) * invalid marcxml * item fields in MARC * wrong authorised values * record without title The goal here is to offer the output of the script from the UI. Created attachment 188222 [details] [review] Bug 40777: Move all biblio tests to the new package Created attachment 188223 [details] [review] Bug 40777: Add tests for invalid_item_library and no_item_type Created attachment 188224 [details] [review] Bug 40777: Add tests for invalid_item_type Created attachment 188225 [details] [review] Bug 40777: Add tests for errors_in_marc Created attachment 188226 [details] [review] Bug 40777: Add tests for nonexistent_AV Created attachment 188227 [details] [review] Bug 40777: Add tests for empty_title Ready for testing. The tests directly related to a bibliographic record have been moved to the new packages (which are now covered by unit tests). To test this you need to confirm that the UI shows (using the audit tool) the same errors/warnings we have with the maintenance script. I can provide a more detailed test plan if needed ;) The UI shows literally Errors found ARRAY(0xaaab121c8750) Created attachment 189598 [details] [review] Bug 40777: Flatten arrays Patch from commit e253e76 (In reply to Phil Ringnalda from comment #14) > The UI shows literally > > > Errors found > ARRAY(0xaaab121c8750) Oops sorry, it seems that I have not retested the UI after the follow-ups. Should be fixed now. |