21591 – Data inconsistencies - Item types and biblio level

Bug 21591 - Data inconsistencies - Item types and biblio level

Summary: Data inconsistencies - Item types and biblio level

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Command-line Utilities (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Nick Clemens (kidclamp)
QA Contact:	Testopia

URL:
Keywords:

Depends on:	21150
Blocks:
	Show dependency tree / graph

Reported:	2018-10-17 19:56 UTC by Nick Clemens (kidclamp)
Modified:	2021-06-14 21:31 UTC (History)
CC List:	5 users (show)

See Also:
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:

Attachments
Bug 21591: Check for record level item type issues too (2.11 KB, patch) 2018-10-17 19:58 UTC, Nick Clemens (kidclamp)	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 (kidclamp)	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 (kidclamp)	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
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Nick Clemens (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (lukeg) 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