Bug 21591 - Data inconsistencies - Item types and biblio level
Summary: Data inconsistencies - Item types and biblio level
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 21150
Blocks:
  Show dependency treegraph
 
Reported: 2018-10-17 19:56 UTC by Nick Clemens
Modified: 2020-11-20 10:46 UTC (History)
6 users (show)

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


Attachments
Bug 21591: Check for record level item type issues too (2.11 KB, patch)
2018-10-17 19:58 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21591: Additionally check for blank strings in itypes (2.58 KB, patch)
2019-04-22 13:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21591: Check for record level item type issues too (3.00 KB, patch)
2020-03-02 14:37 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21591: Check for record level item type issues too (3.04 KB, patch)
2020-03-03 09:55 UTC, David Nind
Details | Diff | Splinter Review
Bug 21591: Check for record level item type issues too (3.10 KB, patch)
2020-05-21 12:09 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-10-17 19:56:10 UTC
bug 21150 added a script to check for missing item level itemtypes - it doesn't like when both the item and the record are missing itemtype
Comment 1 Nick Clemens 2018-10-17 19:58:49 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
Comment 2 Jonathan Druart 2018-10-17 20:23:04 UTC
What about NULL vs ''?

I think we should alert if '', do not you think?
Comment 3 Katrin Fischer 2018-10-28 18:37:19 UTC
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.
Comment 4 Jonathan Druart 2019-03-05 19:14:44 UTC
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.
Comment 5 Jonathan Druart 2019-03-05 19:15:20 UTC
(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 ;)
Comment 6 Nick Clemens 2019-04-22 13:42:15 UTC
Created attachment 88377 [details] [review]
Bug 21591: Additionally check for blank strings in itypes
Comment 7 Jonathan Druart 2019-04-24 20:23:55 UTC
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?
Comment 8 David Nind 2020-02-22 17:56:48 UTC
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
Comment 9 Nick Clemens 2020-03-02 14:37:56 UTC
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=""
Comment 10 David Nind 2020-03-03 09:55:46 UTC
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>
Comment 11 David Nind 2020-03-03 09:58:52 UTC
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 12 Jonathan Druart 2020-03-03 11:59:55 UTC
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.
Comment 13 Nick Clemens 2020-05-21 09:54:08 UTC
(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
Comment 14 Jonathan Druart 2020-05-21 12:09:46 UTC
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>
Comment 15 Jonathan Druart 2020-06-15 09:59:08 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 16 Lucas Gass 2020-06-17 23:44:07 UTC
backported to 20.05.x for 20.05.01
Comment 17 Aleisha Amohia 2020-06-22 04:42:12 UTC
enhancement, not backported to 19.11.x