| Summary: | Data inconsistencies - Item types and biblio level | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Command-line Utilities | Assignee: | Nick Clemens (kidclamp) <nick> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | aleisha, david, jonathan.druart, lucas, robin |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
When an item does not have an itemtype and the record (biblioitem) also has no itemtype defined the data inconsistencies script would error rather than reporting.
We add a test for missing record level itemtypes so that we can correctly report these problems too.
|
Version(s) released in: |
20.11.00, 20.05.01
|
| Circulation function: | |||
| Bug Depends on: | 21150 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 21591: Check for record level item type issues too
Bug 21591: Additionally check for blank strings in itypes Bug 21591: Check for record level item type issues too Bug 21591: Check for record level item type issues too Bug 21591: Check for record level item type issues too |
||
|
Description
Nick Clemens (kidclamp)
2018-10-17 19:56:10 UTC
Created attachment 80757 [details] [review] Bug 21591: Check for record level item type issues too To test: 1 - sudo koha-mysql kohadev 2 - UPDATE biblioitems SET itemtype = NULL where biblionumber = 1 3 - UPDATE items SET itype = NULL where biblionumber = 1 4 - perl misc/maintenance/search_for_data_inconsistencies.pl 5 - Notice warnings 6 - Apply patch 7 - Undefined itemtype on bibliolevel is now warned What about NULL vs ''? I think we should alert if '', do not you think? Could we make the warn on missing biblio-level optional? For us it's normal that there is only an itemtype on item level. We are aware it can be an issue, but it's currently not possible to change it easily because of our union catalog model. IMO we should not hide/skip something in this script. We display possible issues, if you are aware of them you can just ignore it. (In reply to Jonathan Druart from comment #2) > What about NULL vs ''? > > I think we should alert if '', do not you think? Failing QA to get an answer ;) Created attachment 88377 [details] [review] Bug 21591: Additionally check for blank strings in itypes Comment on attachment 88377 [details] [review] Bug 21591: Additionally check for blank strings in itypes Review of attachment 88377 [details] [review]: ----------------------------------------------------------------- ::: C4/Reserves.pm @@ +330,4 @@ > > # Check for item on shelves and OnShelfHoldsAllowed > return { status => 'onShelfHoldsNotAllowed' } > + unless ( IsAvailableForItemLevelRequest($item->unblessed,$borrower) ); Is that change correct? Patch no longer applies: root@333442e28e76:koha(21591)$ git bz apply 21591 Bug 21591 - Data inconsistencies - Item types and biblio level 80757 - Bug 21591: Check for record level item type issues too 88377 - Bug 21591: Additionally check for blank strings in itypes Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 21591: Check for record level item type issues too Applying: Bug 21591: Additionally check for blank strings in itypes error: sha1 information is lacking or useless (C4/Reserves.pm). error: could not build fake ancestor Patch failed at 0001 Bug 21591: Additionally check for blank strings in itypes Created attachment 99920 [details] [review] Bug 21591: Check for record level item type issues too To test: 1 - sudo koha-mysql kohadev 2 - UPDATE biblioitems SET itemtype = NULL where biblionumber = 1 3 - UPDATE items SET itype = NULL where biblionumber = 1 4 - perl misc/maintenance/search_for_data_inconsistencies.pl 5 - Notice warnings 6 - Apply patch 7 - Undefined itemtype on bibliolevel is now warned 7 - Test also with itype="" Created attachment 99980 [details] [review] Bug 21591: Check for record level item type issues too To test: 1 - sudo koha-mysql kohadev 2 - UPDATE biblioitems SET itemtype = NULL where biblionumber = 1 3 - UPDATE items SET itype = NULL where biblionumber = 1 4 - perl misc/maintenance/search_for_data_inconsistencies.pl 5 - Notice warnings 6 - Apply patch 7 - Undefined itemtype on bibliolevel is now warned 7 - Test also with itype="" Signed-off-by: David Nind <david@davidnind.com> Notes from testing: Warnings before patch applied ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ == Items do not have itype defined == Use of uninitialized value in sprintf at misc/maintenancesearch_for_data_inconsistencies.pl line 61. * Item with itemnumber=1 does not have a itype value, biblio's item type will be used () => The system preference item-level_itypes expects item types to be defined at item level Warning after patch applied (also tested itype=""): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ == Items do not have itype defined == * Item with itemnumber=1 does not have a itype value, additionally no item type defined for biblionumber=1 => The system preference item-level_itypes expects item types to be defined at item level Comment on attachment 99980 [details] [review] Bug 21591: Check for record level item type issues too Review of attachment 99980 [details] [review]: ----------------------------------------------------------------- ::: misc/maintenance/search_for_data_inconsistencies.pl @@ +89,4 @@ > > my @itemtypes = Koha::ItemTypes->search->get_column('itemtype'); > if ( C4::Context->preference('item-level_itypes') ) { > + my $items_with_invalid_itype = Koha::Items->search( { -and => [itype => { not_in => \@itemtypes }, itype => { '!=' => '' }] } ); I do not think we need that change. @itemtypes contains the values from the itemtypes table. (In reply to Jonathan Druart from comment #12) > Comment on attachment 99980 [details] [review] [review] > Bug 21591: Check for record level item type issues too > > Review of attachment 99980 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: misc/maintenance/search_for_data_inconsistencies.pl > @@ +89,4 @@ > > > > my @itemtypes = Koha::ItemTypes->search->get_column('itemtype'); > > if ( C4::Context->preference('item-level_itypes') ) { > > + my $items_with_invalid_itype = Koha::Items->search( { -and => [itype => { not_in => \@itemtypes }, itype => { '!=' => '' }] } ); > > I do not think we need that change. @itemtypes contains the values from the > itemtypes table. I see what I did here, I add the check for "" to the check for 'Items do not have itype defined' I considered a blank string to be 'unset' and so report in that section rather than in 'invalid' section Created attachment 105180 [details] [review] Bug 21591: Check for record level item type issues too To test: 1 - sudo koha-mysql kohadev 2 - UPDATE biblioitems SET itemtype = NULL where biblionumber = 1 3 - UPDATE items SET itype = NULL where biblionumber = 1 4 - perl misc/maintenance/search_for_data_inconsistencies.pl 5 - Notice warnings 6 - Apply patch 7 - Undefined itemtype on bibliolevel is now warned 7 - Test also with itype="" Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 20.11, thanks to everybody involved! backported to 20.05.x for 20.05.01 enhancement, not backported to 19.11.x |