Bug 23463: Use new method Koha::Object->set_or_blank

This patch fixes an issue when editing items.

* The issue
Cannot blank a subfield when editing an item.
If you have an item with itemcallnumber=42, then edit it, blank it and
save. The itemcallnumber is still 42

* Why? (line numbers from https://gitlab.com/joubu/Koha/-/tree/bug_23463)
additem (and other item's edition forms) receives a list of tags,
subfields and values, generates a MARC::Record::XML then calls
ModItemFromMarc:

717     my $itemtosave=MARC::Record::new_from_xml($xml, 'UTF-8');
727         my $newitem = ModItemFromMarc($itemtosave, $biblionumber,
$itemnumber);

And ModItemFromMarc:
 282     my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
 283     $item->{cn_source} = delete $item->{'items.cn_source'}; #
Because of C4::Biblio::_disambiguate
 284     $item_object->set($item);

ModItemFromMarc never knows that the field has been blank.
Prior to bug 23463 we had a map of default values, and ModItemFromMarc
was doing:
 426     my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
 427     my $default_values = _build_default_values_for_mod_marc();
 428     foreach my $item_field ( keys %$default_values ) {
 429         $item->{$item_field} = $default_values->{$item_field}
 430           unless exists $item->{$item_field};
 431     }

I do not want to reinsert that list of default values.

Here I wrote a generic method in Koha::Object to set the value passed in parameter,
or "blank" if not passed.
It's nulled if can be set to null in DB, or the default value is retrieved from
the schema info.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

C4/Items.pm

@@ -281,7 +281,9 @@ sub ModItemFromMarc {
     my $item_object = Koha::Items->find($itemnumber);
     my $item = TransformMarcToKoha( $localitemmarc, $frameworkcode, 'items' );
     $item->{cn_source} = delete $item->{'items.cn_source'}; # Because of C4::Biblio::_disambiguate
-    $item_object->set($item);
+    $item->{itemnumber} = $itemnumber;
+    $item->{biblionumber} = $biblionumber;
+    $item_object = $item_object->set_or_blank($item);
     my $unlinked_item_subfields = _get_unlinked_item_subfields( $localitemmarc, $frameworkcode );
     $item_object->more_subfields_xml(_get_unlinked_subfields_xml($unlinked_item_subfields))->store;
     $item_object->store;

Koha/Object.pm

@@ -269,6 +269,41 @@ sub set {
     return $self->_result()->set_columns($properties) ? $self : undef;
 }
 
+=head3 $object->set_or_blank( $properties_hashref )
+
+$object->set_or_blank(
+    {
+        property1 => $property1,
+        property2 => $property2,
+        property3 => $propery3,
+    }
+);
+
+If not listed in $properties_hashref, the property will be set to the default
+value defined at DB level, or nulled.
+
+=cut
+
+
+sub set_or_blank {
+    my ( $self, $properties ) = @_;
+
+    my $columns_info = $self->_result->result_source->columns_info;
+
+    foreach my $col ( keys %{$columns_info} ) {
+
+        next if exists $properties->{$col};
+
+        if ( $columns_info->{$col}->{is_nullable} ) {
+            $properties->{$col} = undef;
+        } else {
+            $properties->{$col} = $columns_info->{$col}->{default_value};
+        }
+    }
+
+    return $self->set($properties);
+}
+
 =head3 $object->unblessed();
 
 Returns an unblessed representation of object.

t/db_dependent/Items.t

@@ -33,7 +33,7 @@ use Koha::AuthorisedValues;
 use t::lib::Mocks;
 use t::lib::TestBuilder;
 
-use Test::More tests => 14;
+use Test::More tests => 15;
 
 use Test::Warn;
 
@@ -975,3 +975,43 @@ subtest 'Split subfields in Item2Marc (Bug 21774)' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'ModItemFromMarc' => sub {
+    plan tests => 2;
+    $schema->storage->txn_begin;
+
+    my $builder = t::lib::TestBuilder->new;
+    my ($itemfield) = GetMarcFromKohaField( 'items.itemnumber' );
+    my $itemtype = $builder->build_object({ class => 'Koha::ItemTypes' });
+    my $biblio = $builder->build_sample_biblio;
+    my ( $lost_tag, $lost_sf ) = GetMarcFromKohaField( 'items.itemlost' );
+    my $item_record = new MARC::Record;
+    $item_record->append_fields(
+        MARC::Field->new(
+            $itemfield, '', '',
+            'y' => $itemtype->itemtype,
+        ),
+        MARC::Field->new(
+            $itemfield, '', '',
+            $lost_sf => '1',
+        ),
+    );
+    my (undef, undef, $itemnumber) = AddItemFromMarc($item_record,
+        $biblio->biblionumber);
+
+    my $item = Koha::Items->find($itemnumber);
+    is( $item->itemlost, 1, 'itemlost picked from the item marc');
+
+    my $updated_item_record = new MARC::Record;
+    $updated_item_record->append_fields(
+        MARC::Field->new(
+            $itemfield, '', '',
+            'y' => $itemtype->itemtype,
+        )
+    );
+
+    my $updated_item = ModItemFromMarc($updated_item_record, $biblio->biblionumber, $itemnumber);
+    is( $updated_item->{itemlost}, 0, 'itemlost should have been reset to the default value in DB' );
+
+    $schema->storage->txn_rollback;
+};

t/db_dependent/Koha/Object.t

@@ -17,7 +17,7 @@
 
 use Modern::Perl;
 
-use Test::More tests => 19;
+use Test::More tests => 20;
 use Test::Exception;
 use Test::Warn;
 use DateTime;
@@ -818,3 +818,37 @@ subtest 'prefetch_whitelist() tests' => sub {
 
     $schema->storage->txn_rollback;
 };
+
+subtest 'set_or_blank' => sub {
+
+    plan tests => 5;
+
+    $schema->storage->txn_begin;
+
+    my $item = $builder->build_sample_item;
+    my $item_info = $item->unblessed;
+    $item = $item->set_or_blank($item_info);
+    is_deeply($item->unblessed, $item_info, 'set_or_blank assign the correct value if unchanged');
+
+    # int not null
+    delete $item_info->{itemlost};
+    $item = $item->set_or_blank($item_info);
+    is($item->itemlost, 0, 'set_or_blank should have set itemlost to 0, default value defined in DB');
+
+    # int nullable
+    delete $item_info->{restricted};
+    $item = $item->set_or_blank($item_info);
+    is($item->restricted, undef, 'set_or_blank should have set restristed to null' );
+
+    # datetime nullable
+    delete $item_info->{dateaccessioned};
+    $item = $item->set_or_blank($item_info);
+    is($item->dateaccessioned, undef, 'set_or_blank should have set dateaccessioned to null');
+
+    # timestamp not null
+    delete $item_info->{timestamp};
+    $item = $item->set_or_blank($item_info);
+    isnt($item->timestamp, undef, 'set_or_blank should have set timestamp to a correct value');
+
+    $schema->storage->txn_rollback;
+};