Bug 34266 - Item type should not default to biblio itemtype if it's not a valid itemtype
Summary: Item type should not default to biblio itemtype if it's not a valid itemtype
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Julian Maurice
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-13 07:21 UTC by Julian Maurice
Modified: 2023-12-28 20:46 UTC (History)
5 users (show)

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


Attachments
Bug 34266: Don't default to biblio itemtype if item-level_types is set (1.58 KB, patch)
2023-07-13 07:28 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.58 KB, patch)
2023-07-13 08:19 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.15 KB, patch)
2023-07-13 08:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.62 KB, patch)
2023-08-17 07:52 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.19 KB, patch)
2023-08-17 07:52 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.62 KB, patch)
2023-08-17 07:55 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.19 KB, patch)
2023-08-17 07:55 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.62 KB, patch)
2023-08-17 08:02 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.19 KB, patch)
2023-08-17 08:02 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.66 KB, patch)
2023-08-17 08:04 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.22 KB, patch)
2023-08-17 08:04 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.72 KB, patch)
2023-08-18 09:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.44 KB, patch)
2023-08-18 09:56 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 34266: Default to biblioitems.itemtype only if it's a valid itemtype (2.81 KB, patch)
2023-09-01 09:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34266: Add unit tests (3.61 KB, patch)
2023-09-01 09:10 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2023-07-13 07:21:55 UTC
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
Comment 1 Julian Maurice 2023-07-13 07:28:56 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.
Comment 2 Marcel de Rooy 2023-07-13 07:42:09 UTC
(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.
Comment 3 Marcel de Rooy 2023-07-13 07:47:36 UTC
Special AV category itemtypes is bound to Koha::ItemTypes
Comment 4 Julian Maurice 2023-07-13 07:49:59 UTC
(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.
Comment 5 Marcel de Rooy 2023-07-13 07:55:03 UTC
-            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??
Comment 6 Katrin Fischer 2023-07-13 08:01:37 UTC
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.
Comment 7 Julian Maurice 2023-07-13 08:04:53 UTC
Ok, thanks for the detailed answers. I will write a different patch.
Comment 8 Katrin Fischer 2023-07-13 08:09:07 UTC
(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?
Comment 9 Julian Maurice 2023-07-13 08:19:03 UTC
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
Comment 10 Julian Maurice 2023-07-13 08:22:02 UTC
(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
Comment 11 Julian Maurice 2023-07-13 08:59:10 UTC
Created attachment 153393 [details] [review]
Bug 34266: Add unit tests
Comment 12 Mathieu Saby 2023-07-19 12:02:52 UTC
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.
Comment 13 Biblibre Sandboxes 2023-08-17 07:52:17 UTC
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>
Comment 14 Biblibre Sandboxes 2023-08-17 07:52:19 UTC
Created attachment 154486 [details] [review]
Bug 34266: Add unit tests

Signed-off-by: joubu <xxx@example.org>
Comment 15 Biblibre Sandboxes 2023-08-17 07:55:39 UTC
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>
Comment 16 Biblibre Sandboxes 2023-08-17 07:55:41 UTC
Created attachment 154488 [details] [review]
Bug 34266: Add unit tests

Signed-off-by: joubu <xxx@example.org>
Comment 17 Biblibre Sandboxes 2023-08-17 08:02:09 UTC
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>
Comment 18 Biblibre Sandboxes 2023-08-17 08:02:12 UTC
Created attachment 154490 [details] [review]
Bug 34266: Add unit tests

Signed-off-by: joubu <xxx@example.org>
Comment 19 Biblibre Sandboxes 2023-08-17 08:04:23 UTC
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>
Comment 20 Biblibre Sandboxes 2023-08-17 08:04:26 UTC
Created attachment 154492 [details] [review]
Bug 34266: Add unit tests

Signed-off-by: joubu <xxx@example.org>
Signed-off-by: tuxayo <victor@tuxayo.net>
Comment 21 Chris Cormack 2023-08-18 09:56:21 UTC
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>
Comment 22 Chris Cormack 2023-08-18 09:56:43 UTC
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>
Comment 23 Katrin Fischer 2023-08-23 20:03:24 UTC
Joubu, do you want to use your sign-off as QA here? But please fix it then :) 

Signed-off-by: joubu <xxx@example.org>
Comment 24 Marcel de Rooy 2023-09-01 09:10:08 UTC
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>
Comment 25 Marcel de Rooy 2023-09-01 09:10:11 UTC
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 :)
Comment 26 Tomás Cohen Arazi (tcohen) 2023-09-01 15:09:08 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 27 Fridolin Somers 2023-09-08 06:11:16 UTC
Pushed to 23.05.x for 23.05.04
Comment 28 Matt Blenkinsop 2023-09-15 09:33:34 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 29 Jonathan Druart 2023-09-22 09:46:20 UTC
(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.
Comment 30 Katrin Fischer 2023-09-22 11:48:50 UTC
(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.