|
Description
Julian Maurice
2023-07-13 07:21:55 UTC
Created attachment 153389 [details] [review] Bug 34266: Don't default to biblio itemtype if item-level_types is set biblioitems.itemtype and items.itype can use differents sets of values so biblioitems.itemtype can be innappropriate as a default for items.itype. This patch removes this behaviour when item-level_itypes is set Test plan: 1. Disable item-level_itypes 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Enable item-level_itypes 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. (In reply to Julian Maurice from comment #0) > When cataloging a new item, a subfield linked to the authorized value > "itemtypes" will default to its biblio itemtype. > But when the system preference item-level_itypes is set we don't want that. > We often use a different authorized value category for biblioitems.itemtype > so the values stored there are not appropriate as a default for items.itype. > > I suggest that Koha uses biblioitems.itemtype as a default only if > item-level_itypes is disabled I am not sure about this. Please describe the use case where 942c should differ from values used in 952y. Speaking MARC21 here. Special AV category itemtypes is bound to Koha::ItemTypes (In reply to Marcel de Rooy from comment #3) > Special AV category itemtypes is bound to Koha::ItemTypes Yes, but you can configure 942$c to use something else than this special AV category. - if (!$value && $biblionumber) {
+ if (!$value && $biblionumber && !C4::Context->preference('item-level_itypes')) {
my $itype_sth = $dbh->prepare(
"SELECT itemtype FROM biblioitems WHERE biblionumber = ?");
$itype_sth->execute($biblionumber);
As long as it would be bound to 'itemtypes' I definitely think that you should not do this. 942c is for me like default itype.
You could add a check if you are using another AV category or so for 942c??
I think we need to make a decision here. The MARC21 and UNIMARC installations drifted apart with a re-configuration decision made at some point (I believe at Biblibre?). In MARC21 installations biblioitems.itemtype and items.itype are always mapped to itemtypes. I also believe that the MARC framework check checks for it. In UNIMARC the beahavior was altered. The standard frameworks even reflect this decision now, which is, I think, unfortunate. It creates a lot of issues and makes some features not work as intended for UNIMARC libraries: * Circulation rules for article requests on records without items are not configurable. * Hold max values for record level holds won't be checked correctly. * Item type search for records without items won't work. ... I personally believe Koha should not adapt to the UNIMARC change, but we should instead implement a separate document type column on record level if there is a good use case for it. And migrate the UNIMARC libraries to use it, freeing up the biblioitems.itemtype again. I believe this change works more accidentally than actually at the moment. For this patch: We DO want this behaviour with item-level itypes - it allows to save time when adding multiple items to a record. So the pref check as suggested does not work, it would need to check for a different mapping instead. Ok, thanks for the detailed answers. I will write a different patch. (In reply to Julian Maurice from comment #7) > Ok, thanks for the detailed answers. I will write a different patch. We'll be bumping heads again and again about this stuff... instead of complicating code, how about solving it once and for all with a new column? Created attachment 153390 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value (In reply to Katrin Fischer from comment #8) > (In reply to Julian Maurice from comment #7) > > Ok, thanks for the detailed answers. I will write a different patch. > > We'll be bumping heads again and again about this stuff... instead of > complicating code, how about solving it once and for all with a new column? That's probably the way to go... but right now I can't work on that. Please have a look at the new patch. It's really simple as it only ensures that the default value is one of the values shown in the dropdown list Created attachment 153393 [details] [review] Bug 34266: Add unit tests Hi For your information, item creation in our 22.11 UNIMARC Koha (with item-level_itypes = specific item ) was unusable (invalid value in the list of itemtypes to pickup). Biblibre applied this bz and the problem was fixed. Created attachment 154485 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value Signed-off-by: joubu <xxx@example.org> Created attachment 154486 [details] [review] Bug 34266: Add unit tests Signed-off-by: joubu <xxx@example.org> Created attachment 154487 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value Signed-off-by: joubu <xxx@example.org> Created attachment 154488 [details] [review] Bug 34266: Add unit tests Signed-off-by: joubu <xxx@example.org> Created attachment 154489 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value Signed-off-by: joubu <xxx@example.org> Created attachment 154490 [details] [review] Bug 34266: Add unit tests Signed-off-by: joubu <xxx@example.org> Created attachment 154491 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value Signed-off-by: joubu <xxx@example.org> Signed-off-by: tuxayo <victor@tuxayo.net> Created attachment 154492 [details] [review] Bug 34266: Add unit tests Signed-off-by: joubu <xxx@example.org> Signed-off-by: tuxayo <victor@tuxayo.net> Created attachment 154630 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value Signed-off-by: joubu <xxx@example.org> Signed-off-by: tuxayo <victor@tuxayo.net> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Created attachment 154631 [details] [review] Bug 34266: Add unit tests Signed-off-by: joubu <xxx@example.org> Signed-off-by: tuxayo <victor@tuxayo.net> Took the opportunity to perltidy this file as I was signing off Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Joubu, do you want to use your sign-off as QA here? But please fix it then :) Signed-off-by: joubu <xxx@example.org> Created attachment 155072 [details] [review] Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype biblioitems.itemtype can contain values that are not itemtypes (for instance if the MARC field mapped to it uses another authorised value category) This patch checks that before using it as a default value for the 'itemtypes' item subfield. Test plan: 0. Do not apply the patch yet 1. Change your MARC framework so that the field linked to biblioitems.itemtype uses an authorised value category different from 'itemtypes' 2. Make sure that you have an item subfield linked to the authorised value 'itemtypes' in your MARC framework 3. Create a biblio with an itemtype (using a value that is NOT an item type, but a value from the authorised value category you selected in step 1) 4. Create a new item for this biblio. Notice that the biblio itemtype is automatically selected for the 'itemtypes' subfield 5. Apply the patch 6. Create a new item for the same biblio. Now the 'itemtypes' subfield should be empty. 7. Revert the change at step 1 so that your biblioitems.itemtype MARC field uses the 'itemtypes' AV category 8. Modify your biblio to use a valid item type. 9. Create a new item. The biblio itemtype should be used as a default value Signed-off-by: joubu <xxx@example.org> Signed-off-by: tuxayo <victor@tuxayo.net> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 155073 [details] [review] Bug 34266: Add unit tests Signed-off-by: joubu <xxx@example.org> Signed-off-by: tuxayo <victor@tuxayo.net> Took the opportunity to perltidy this file as I was signing off Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Added a rollback. Shouldnt harm :) Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.04 Nice work everyone! Pushed to oldstable for 22.11.x (In reply to Katrin Fischer from comment #23) > Joubu, do you want to use your sign-off as QA here? But please fix it then > :) > > Signed-off-by: joubu <xxx@example.org> Could have been adjusted before push really... Stuffs I put when creating the sandbox, then it has been used for signoff. (In reply to Jonathan Druart from comment #29) > (In reply to Katrin Fischer from comment #23) > > Joubu, do you want to use your sign-off as QA here? But please fix it then > > :) > > > > Signed-off-by: joubu <xxx@example.org> > > Could have been adjusted before push really... > > Stuffs I put when creating the sandbox, then it has been used for signoff. The problem is, that the sandbox still uses the setup information, not what you enter when using the sign-off form. long standing bug. |