I fill this bug to start a conversation about this issue. It seems the codebase has several ways of determining the itemtype. For example, C4::Circulation::AddReturn used the biblio-level itemtype (until bug 14598) and C4::Circulation::CanBookBeIssued just sticks to item-level itemtype. At first sight, it seems clear to me that we should start moving into using Koha::Item objects, that even provide an ->effective_itemtype method! Having talked about this on IRC there seems to be a consensus that if the item doesn't have an itemtype, then there should be a fallback to biblio-level itemtype. This means we should add that check to the ->effective_itemtype method. But it would mean including Koha::Biblios. Thoughts?
Created attachment 41635 [details] [review] Bug 14651: (regression test) fallback to bib-level if itype is undef Koha::Item->effective_itemtype should fallback to biblio-level itemtype even if item-level item types are set, in the case the item has no itemtype set (bad migration, bad old code). To test: - Run $ prove t/db_dependent/Items.t => FAIL: Koha::Item->effective_itemtype doesn't work properly
Created attachment 41636 [details] [review] Bug 14651: Koha::Item->effective_itemtype fallback to bib-level In some situations (bad migrations, old bugs that introduced bad data, people having bib-level itypes for ages and switching to item level...) the user ends with undex itype values for items. The current code, if the user has item_level-itype set, just returns undef. It should fallback to bib-level. To test: - Run the regression tets: $ prove t/db_dependent/Items.t => FAIL: tests fail. - Apply the patch - Run the tests again => SUCCESS: The tests now pass. - Sign off :-D
(In reply to Tomás Cohen Arazi from comment #0) > At first sight, it seems clear to me that we should start moving into using > Koha::Item objects, that even provide an ->effective_itemtype method! > > Having talked about this on IRC there seems to be a consensus that if the > item doesn't have an itemtype, then there should be a fallback to > biblio-level itemtype. > > This means we should add that check to the ->effective_itemtype method. But > it would mean including Koha::Biblios. The patches work fine, but aren't we masquerading a problem that should be addressed instead of hidden ?
Created attachment 42083 [details] [review] Bug 14651: (regression test) fallback to bib-level if itype is undef Koha::Item->effective_itemtype should fallback to biblio-level itemtype even if item-level item types are set, in the case the item has no itemtype set (bad migration, bad old code). To test: - Run $ prove t/db_dependent/Items.t => FAIL: Koha::Item->effective_itemtype doesn't work properly Edit: Added a test for a warning when falling back as per QA request and because it made a lot of sense :-D
Created attachment 42084 [details] [review] Bug 14651: Koha::Item->effective_itemtype fallback to bib-level In some situations (bad migrations, old bugs that introduced bad data, people having bib-level itypes for ages and switching to item level...) the user ends with undex itype values for items. The current code, if the user has item_level-itype set, just returns undef. It should fallback to bib-level. In order to avoid hiding the problem a warning is raised. To test: - Run the regression tets: $ prove t/db_dependent/Items.t => FAIL: tests fail. - Apply the patch - Run the tests again => SUCCESS: The tests now pass. - Sign off :-D
Created attachment 42264 [details] [review] Bug 14651: (regression test) fallback to bib-level if itype is undef Koha::Item->effective_itemtype should fallback to biblio-level itemtype even if item-level item types are set, in the case the item has no itemtype set (bad migration, bad old code). To test: - Run $ prove t/db_dependent/Items.t => FAIL: Koha::Item->effective_itemtype doesn't work properly Edit: Added a test for a warning when falling back as per QA request and because it made a lot of sense :-D Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 42265 [details] [review] Bug 14651: Koha::Item->effective_itemtype fallback to bib-level In some situations (bad migrations, old bugs that introduced bad data, people having bib-level itypes for ages and switching to item level...) the user ends with undex itype values for items. The current code, if the user has item_level-itype set, just returns undef. It should fallback to bib-level. In order to avoid hiding the problem a warning is raised. To test: - Run the regression tets: $ prove t/db_dependent/Items.t => FAIL: tests fail. - Apply the patch - Run the tests again => SUCCESS: The tests now pass. - Sign off :-D Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Created attachment 42270 [details] [review] Bug 14651: (regression test) fallback to bib-level if itype is undef Koha::Item->effective_itemtype should fallback to biblio-level itemtype even if item-level item types are set, in the case the item has no itemtype set (bad migration, bad old code). To test: - Run $ prove t/db_dependent/Items.t => FAIL: Koha::Item->effective_itemtype doesn't work properly Edit: Added a test for a warning when falling back as per QA request and because it made a lot of sense :-D Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 42271 [details] [review] Bug 14651: Koha::Item->effective_itemtype fallback to bib-level In some situations (bad migrations, old bugs that introduced bad data, people having bib-level itypes for ages and switching to item level...) the user ends with undex itype values for items. The current code, if the user has item_level-itype set, just returns undef. It should fallback to bib-level. In order to avoid hiding the problem a warning is raised. To test: - Run the regression tets: $ prove t/db_dependent/Items.t => FAIL: tests fail. - Apply the patch - Run the tests again => SUCCESS: The tests now pass. - Sign off :-D Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Patches pushed to master.