Summary: | transferstoreceive.pl returns internal server error | ||
---|---|---|---|
Product: | Koha | Reporter: | lulihua |
Component: | Reports | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED WONTFIX | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P2 | CC: | alexis.ripetti, jonathan.druart, katrin.fischer, paul |
Version: | Main | ||
Hardware: | All | ||
OS: | Linux | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21150 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: | Don't invoke _description_ on an undef itemtype |
Description
lulihua
2018-06-01 12:06:53 UTC
Could you give us a step by step plan to recreate the problem? Can you have a look at the Koha log file, you will find a more meaningful error. Personal note: 98 $getransf{'itemtype'} = $itemtype->description; # FIXME Should not it be translated_description? may cause problem if the item type is not defined in the itemtypes table. Many thanks to Jonathan Based on your hint, we found that quite a few of items in our branches with 'itype.items' as null. Eliminating all the nulls, /cgi-bin/koha/circ/transferstoreceive.pl works fine for all branches. Created attachment 77499 [details] [review] Don't invoke _description_ on an undef itemtype Sorry that my comment on the attachment was rather cryptic; this is only my first or second bug comment. I've upped the priority because the bug makes it impossible to view any list of holds (using waitingreserves.pl) that includes an item with NULL itype. Paul, Do you agree that it's a data inconsistency you should fix on your side? See bug 21150. I don't consider it a case of bad data -- as much as I would like to -- because itype (952 $y) is optional in the default framework. See installer/data/mysql/en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.sql, which sets marc_subfield_structure.mandatory to 0: 89 INSERT IGNORE INTO `marc_subfield_structure` (`tagfield`, `tagsubfield`, `liblibrarian`, `libopac`, `repeatable`, `mandatory`, `kohafield`, `tab`, `authorised_valu e`, `authtypecode`, `value_builder`, `isurl`, `hidden`, `frameworkcode`, `seealso`, `link`, `defaultvalue`) VALUES [...] 123 ('952', 'y', 'Koha item type', 'Koha item type', 0, 0, 'items.itype', 10, 'itemtypes', '', '', NULL, 0, '', '', '', NULL), Hi Paul, Yes I know, that's the problem. There are several places in the code where we assumed items.itype or biblioitems.itemtype are set to an itemtype defined in the Koha administration module. But nothing validate that (no foreign key, no mandatory field, etc.) What I am suggesting with bug 21150 is to provide a script to catch the different situations where it can causes problems. Marking items.itype mandatory is wrong if item-level_itypes is set to biblio. Well, bug 21150 presents a way to clean up bad data, but I consider it a bug that the bad data is allowed to exist in the first place. And, worse, that the bad data (while it exists) breaks waitingreserves.pl and (apprently) other scripts. An easy solution is for us is to update our frameworks so that they all require 952 $y -- we have item-level_itypes set to 1 ("Use the item type of the <specific item> as the authoritative item type"). But that seems like a band-aid solution. Frankly, my patch also strikes me as a band-aid solution, but at least it prevents waitingreserves.pl et al. from breaking. Here are some possible solutions that come to mind, with the pros and cons as I see them: (1) Check item-level_itypes at item creation time; if it is 1, require the subfield that maps to items.itype even if the framework's marc_subfield_structure.mandatory is 0. Pro: Prevents bad data from being created. Con: What happens when item-level_itypes is changed to 0 (biblio)? And then back again? Which it shouldn't, I know, but... (2) When item-level_itypes is changed, update marc_subfield_structure.mandatory to 1 for all frameworks. Pro: Prevents waitingreserves.pl et al. from breaking. Con: Not reversible. (3) At item creation time, allow NULL items.itype if the framework allows it. But when displaying items don't assume that they have an itype. That's what my patch does (or attempts to do!). Pro: Prevents waitingreserves.pl et al. from breaking. Con: Allows for bad data. But run search_for_data_inconsistencies.pl to fix that. (4) Something else that's a lot more complicated. :-( Pro: Presumably we can find The Right Way to fix this. Con: My head hurts trying to imagine how. Paul. Hi Paul, I think making items.itype manatory in your configuration would be a good first step. We also plan to do this for our libraries, but there is currently bug 14662. Should we mark this WONTFIX as we have agreed that the data inconsistency should be fixed instead? (In reply to Katrin Fischer from comment #11) > Should we mark this WONTFIX as we have agreed that the data inconsistency > should be fixed instead? Yes, done. Hi, I reopen this bug because one of our client also encountered this problem and I think this needs to be patched because data inconsistency should not get a software error. This is not the first time that this probleme is reported and this will not be the last. Alexis, this has been discussed a lot of time already. We provided maintenance scripts to deal with such inconsistencies, you should tell your support team to use them. This patch won't help much anyway, there are ton of other places where the error will appear. |