From 3addac4def484a2c02c4b8063e591725865363b9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Mon, 16 Mar 2020 14:51:06 +0100 Subject: [PATCH] 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: Signed-off-by: Nick Clemens --- C4/Items.pm | 4 +++- Koha/Object.pm | 35 ++++++++++++++++++++++++++++++ t/db_dependent/Items.t | 42 +++++++++++++++++++++++++++++++++++- t/db_dependent/Koha/Object.t | 36 ++++++++++++++++++++++++++++++- 4 files changed, 114 insertions(+), 3 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 96bccf2af5..8045cd57e0 100644 --- a/C4/Items.pm +++ b/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; diff --git a/Koha/Object.pm b/Koha/Object.pm index fcb1e802ff..5b64e6a58d 100644 --- a/Koha/Object.pm +++ b/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. diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index d885dcce77..2397a5dff3 100755 --- a/t/db_dependent/Items.t +++ b/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; +}; diff --git a/t/db_dependent/Koha/Object.t b/t/db_dependent/Koha/Object.t index 40f117007e..73e4e10762 100755 --- a/t/db_dependent/Koha/Object.t +++ b/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; +}; -- 2.20.1