Bug 33166 - Item type database fields should not be nullable
Summary: Item type database fields should not be nullable
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-08 09:07 UTC by Julian Maurice
Modified: 2024-09-17 12:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2023-03-08 09:07:11 UTC
Steps to reproduce:
1. Create a biblio with no item type, and an item with no item type
2. Try to place a hold for this biblio

You should get an "internal server error" message, and in the logs the following line:

Can't call method "notforloan" on an undefined value at [...]/C4/Reserves.pm line 1446.
Comment 1 Katrin Fischer 2023-03-08 09:29:15 UTC
Hi Julian,

we expect all items to have itemtypes now, so this is an INVALID I am afraid. You need to fix the data using the script for finding inconsistencies.
Comment 2 Julian Maurice 2023-03-08 09:42:40 UTC
Shouldn't Koha prevent creating biblio/item without itemtype then ?

From a user perspective, it looks like it's possible but then you get hit by this internal server error without a clue about what's wrong
Comment 3 Katrin Fischer 2023-03-08 09:46:23 UTC
We have made the itemtype mandatory in the frameworks for MARC21 at least, not sure if the same applies to UNIMARC (I haven't checked), but of course this will be only the case for new installations.

I think it makes sense to say it should be mandatory, but I am also not super happy about the errors it causes as we have a lot of libraries that are still missing itemtypes (from the past)

We will probably try to clean this up with 22.11 as the problems it causes are getting more and more sever (like OPAC detail page exploding).
Comment 4 Julian Maurice 2023-03-08 10:01:29 UTC
(In reply to Katrin Fischer from comment #3)
> We have made the itemtype mandatory in the frameworks for MARC21 at least,
> not sure if the same applies to UNIMARC (I haven't checked), but of course
> this will be only the case for new installations.

Ok, but MARC frameworks are modifiable...
And even if they weren't, we can import biblio from other sources and mandatory fields are not checked in that case.

> I think it makes sense to say it should be mandatory, but I am also not
> super happy about the errors it causes as we have a lot of libraries that
> are still missing itemtypes (from the past)

I think that as long as biblioitems.itemtype/items.itype is NULL-able, we cannot expect to always have a value there.
Comment 5 Jonathan Druart 2023-03-08 10:40:51 UTC
Julian, there are several places where we are not checking if the item type exists, and we decided to not fix them all but assume the data are correct instead.

The script misc/maintenance/search_for_data_inconsistencies.pl is catching such inconsistencies.

> Ok, but MARC frameworks are modifiable...

Yes, but you have hundreds of ways to make Koha explodes when you modify them.

(In reply to Julian Maurice from comment #4)
> I think that as long as biblioitems.itemtype/items.itype is NULL-able, we
> cannot expect to always have a value there.

Yes, you are right. That's is something we should fix.
Comment 6 David Cook 2023-09-18 06:56:33 UTC
(In reply to Jonathan Druart from comment #5)
> (In reply to Julian Maurice from comment #4)
> > I think that as long as biblioitems.itemtype/items.itype is NULL-able, we
> > cannot expect to always have a value there.
> 
> Yes, you are right. That's is something we should fix.

In that case, do we want to change the title of this bug to reflex that, or close this one and open a new one?
Comment 7 Jonathan Druart 2023-09-18 14:19:16 UTC
(In reply to David Cook from comment #6)
> (In reply to Jonathan Druart from comment #5)
> > (In reply to Julian Maurice from comment #4)
> > > I think that as long as biblioitems.itemtype/items.itype is NULL-able, we
> > > cannot expect to always have a value there.
> > 
> > Yes, you are right. That's is something we should fix.
> 
> In that case, do we want to change the title of this bug to reflex that, or
> close this one and open a new one?

Whatever you prefer.
Comment 8 Emily Lamancusa (emlam) 2023-10-24 21:04:08 UTC
Is there anything else that needs discussing here? Or should we set the bug back to NEW with the agreement that itemtype fields shouldn't be nullable?
Comment 9 Katrin Fischer 2023-12-09 11:25:47 UTC
I would be happy if we could avoid making the biblio-level itype mandatory. 
We don't set it usually, because of our union catalog situation. You cannot really predict the item type reliably from the bibliographic data and in the past it was not able to protect the field.