Bug 27684

Summary: Serial add is confused by authorized values with a code of '0'
Product: Koha Reporter: Didier Gautheron <didier.gautheron>
Component: SerialsAssignee: Didier Gautheron <didier.gautheron>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: black23, david, jonathan.druart, magnus, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 8976    
Bug Blocks:    
Attachments: Bug 27684: Serial 0 is valid default value.
Bug 27684: Serial 0 is valid default value.

Description Didier Gautheron 2021-02-11 14:52:02 UTC
It doesn't make difference between undefined and zero and it uses default value.


To test:
diff --git a/C4/Items.pm b/C4/Items.pm
index 5d0495b8..01f81bdf 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -2390,7 +2390,7 @@ sub PrepareItemrecordDisplay {
                 if ($itemrecord) {
                     ( $x, $defaultvalue ) = _find_value( $tag, $subfield, $itemrecord );
                 }
-                $defaultvalue = $tagslib->{$tag}->{$subfield}->{defaultvalue} unless $defaultvalue;
+                $defaultvalue //= $tagslib->{$tag}->{$subfield}->{defaultvalue};
                 if ( !defined $defaultvalue ) {
                     $defaultvalue = q||;
                 } else {
Comment 1 Jonathan Druart 2021-02-12 12:29:51 UTC
Can you provide a patch?
Comment 2 Didier Gautheron 2021-08-27 10:23:54 UTC Comment hidden (obsolete)
Comment 3 Martin Renvoize 2021-11-02 14:51:52 UTC
Does it make sense to leave in the unless.. can you explain?
Comment 4 Didier Gautheron 2021-11-02 15:32:40 UTC
(In reply to Martin Renvoize from comment #3)
> Does it make sense to leave in the unless.. can you explain?

No it doesn't.

Regards
Didier
Comment 5 Didier Gautheron 2021-11-02 15:41:24 UTC
Created attachment 127221 [details] [review]
Bug 27684: Serial 0 is valid default value.

To test:
1- Create a NOT_LOAN VA: code = 0 lib = available
2- Add a serial with one item
3- Check that in items modification 952$7 is on 'available'
4- Receive this serial.
5- Add item
6- check that not for loan is empty
6- Apply patch
7- redo 4,5
8- check that not for loan is on 'available'
Comment 6 Jonathan Druart 2021-11-02 16:00:52 UTC
You need a test and certainly to modify this other place:

Koha/UI/Form/Builder/Item.pm
122         $value = $subfield->{defaultvalue};
Comment 7 David Nind 2022-01-19 23:20:08 UTC
Changed status to failed QA - see comment #6.
Comment 8 Didier Gautheron 2022-07-28 10:03:27 UTC
(In reply to Jonathan Druart from comment #6)
> You need a test and certainly to modify this other place:
> 
> Koha/UI/Form/Builder/Item.pm
> 122         $value = $subfield->{defaultvalue};

No, in my understanding it's already done there :
  
 if ( $prefill_with_default_values && ( !defined($value) || $value eq '' ) ) {
        $value = $subfield->{defaultvalue} if !$item->{itemnumber}; # apply defaultvalue only to new items


it's only set $value if it's undefined or an empty string.

Unlike 0 empty string for a VA code is an issue in a lot of place.
Comment 9 David Nind 2022-07-28 20:25:04 UTC
I'm not that familiar with serials, so could you clarify steps 1-6 in a bit more detail?

I've:
1. Added an authorized value for NOT_LOAN:
   . Authorized value = 0
   . Description = Available
   . Library limitations = All libraries
2. Created a serial, set so that it creates an item record when received.
3. Received an edition: 952$7 has no value selected by default and I set it to Available (using the authorized value setup in step 1).
4. After applying the patch and receiving another edition, the default still has no value selected by default.

Am I supposed to manually create an item when I first set up the serial? (Do subsequent editions received inherit the values set up for this first item?)
Comment 10 Katrin Fischer 2022-07-30 19:56:37 UTC
I think this is about the different behaviour of the add item form (cataloguing) vs. the add item form (serials). If I read the test plan right, they should preselect the 0 value in both when you open the forms.
Comment 11 Magnus Enger 2023-03-27 09:49:40 UTC
Sounds to me like this is "Failed QA" until the questions in Comment 9 have been answered.