Bug 29106

sub effective_itemtype {
    my ( $self ) = @_;

    my $pref = C4::Context->preference('item-level_itypes');
    if ( $pref && $self->itype() ) {
        return $self->itype();
    } else {
        warn "item-level_itypes set but no itemtype set for item (".$self->itemnumber.")"
          if $pref;
        return $self->biblioitemnumber()->itemtype();
    }
}

This sub c/should be moved out of the DBIx schema file to Koha::Item ?
There is no need to warn each time imo.

(In reply to Marcel de Rooy from comment #1)
> sub effective_itemtype {
>     my ( $self ) = @_;
> 
>     my $pref = C4::Context->preference('item-level_itypes');
>     if ( $pref && $self->itype() ) {
>         return $self->itype();
>     } else {
>         warn "item-level_itypes set but no itemtype set for item
> (".$self->itemnumber.")"
>           if $pref;
>         return $self->biblioitemnumber()->itemtype();
>     }
> }
> 
> This sub c/should be moved out of the DBIx schema file to Koha::Item ?
> There is no need to warn each time imo.

I think it should be moved to the object. That method was added to the DBIC class before Koha::Object(s) existed!

When I said we send a warning to the logs, I meant we are telling the users their data is wrong. It implies that the user needs to fix that.
Implicitly, we are telling people they need to set a value there.

Can't we force that somehow? And set the 'effective item type' on a DBRev instead? It feels like it was a trasitional step, for a transition that never took place.

(In reply to Tomás Cohen Arazi from comment #3)
> When I said we send a warning to the logs, I meant we are telling the users
> their data is wrong. It implies that the user needs to fix that.
> Implicitly, we are telling people they need to set a value there.
> 
> Can't we force that somehow? And set the 'effective item type' on a DBRev
> instead? It feels like it was a trasitional step, for a transition that
> never took place.

It would be better to have a Koha data linter tool for sure. Maybe we should move this check to the about page, considering we have other checks of this nature there.

The reason the warning is there is because itemtype is required for Koha to operate correctly. It can be set at either the item or the bib level. That's why that warning ended up where it is. However, unless somebody is routinely combing through the logs that warning will go unnoticed. It seems more sensible to remove the warning there and add a check to the about page.

IIRC items without itemtype come from Acquisition

(In reply to Marcel de Rooy from comment #5)
> IIRC items without itemtype come from Acquisition

Yes, they often do. And they won't cause an issue until you try to check them out. We have moved to setting itemtype mandatory for items in our libraries to avoid this from happening - at least on the non-ACQ frameworks.

I am not sure about the consequences of removing the warn - so not going to comment on that. But having a way to tell users that their data needs fixing is probably a good thing, given that Koha explodes badly when those items slip through in some areas.

(In reply to Katrin Fischer from comment #6)
> I am not sure about the consequences of removing the warn - so not going to
> comment on that. But having a way to tell users that their data needs fixing
> is probably a good thing, given that Koha explodes badly when those items
> slip through in some areas.

misc/maintenance/search_for_data_inconsistencies.pl is catching that.

If you want to get rid of this method you should get rid of item-level_itypes first :D

Long standing discussion
https://lists.koha-community.org/pipermail/koha-devel/2015-December/042114.html

(In reply to Jonathan Druart from comment #8)
> If you want to get rid of this method you should get rid of
> item-level_itypes first :D
> 
> Long standing discussion
> https://lists.koha-community.org/pipermail/koha-devel/2015-December/042114.
> html

Yes, long standing :) I am for killing the system preference, but we do require teh item type on record level for a number of features by now, so not sure we could get rid of all the effective itemtype handling.

The idea was to have biblioitems.itemtype always populated and accurate (done when upgrading and when a new item is created). No need to check anything else.

I am not sure we can have it always accurate - I know libraries use this a lot for records without items (articles, electronic resources, series records, set records, etc.), so manual interaction will always be required there to make it accurate.

We could add an automatism to update the record level itemtype when all items have the same itemtype - but this might not be what some libraries want (it would fit ours). But if the itemtypes for the items are different, this would not work. 
Use case in academics: one item has a not for loan item type to always remain in the library for reference, while additional copies can be checked out

I think especially in some French/UNIMARC libraries, there were different authorised valued used for bibioitems (document type) and items - something we would need to clarify is not an intended workflow and maybe help provide workarounds for.

Hm adding to that: as long we can't "save" 942$c from being overwritten by the import, enforcing the itemtype on record level is quite an unsolvable issue for us.

> misc/maintenance/search_for_data_inconsistencies.pl is catching that.

I know, but I think we need something more low level, that doesn't require server side interaction for libraries.

HEA figures for item-level_itypes show that there is a very convincing number of libraries to remove the pref.

28 NULL
16 (empty?)
239 0
11386 1

Btw the discussion for an default itemtype might be a topic on itself?

Bug 10385 ?