Bug 29719

Created attachment 128635 [details] [review]
Bug 29719: Unit tests

Created attachment 128636 [details] [review]
Bug 29719: Do not clear onloan value when not passed in MARC

To test:
 1 - Check an item out to a paron
 2 - Export the item using Tools->Export data
 3 - Stage the record for import
 4 - Match on 999c and replace items
 5 - Import the batch
 6 - View the record and note item is checked out and Available
 7 - In the DB note the onloan value is now null
 8 - Check in the item
 9 - Apply patch
10 - Repeat 1-5
11 - View the record and note item is checked out
12 - In the DB note the onloan value matches the due date

It might get too late and I might be misunderstanding it all, but I am not sure if this fix is in the right spot.

Instead of introducing the onloan special case to the general method in Object.pm, would the solution not be to add it to the $properties_hashref? 

321     $item->{cn_source} = delete $item->{'items.cn_source'}; # Because of C4::Biblio::_disambiguate
 322     delete $item->{'items.cn_sort'};   # Because of C4::Biblio::_disambiguate
 323     $item->{itemnumber} = $itemnumber;
 324     $item->{biblionumber} = $biblionumber;
 325 
 326     my $existing_cn_sort = $item_object->cn_sort; # set_or_blank will reset cn_sort to undef as we are not passing it
 327                                                   # We rely on Koha::Item->store to modify it if itemcallnumber or cn_source is modified
 328     $item_object = $item_object->set_or_blank($item);



279 =head3 $object->set_or_blank( $properties_hashref )
280 
281 $object->set_or_blank(
282     {
283         property1 => $property1,
284         property2 => $property2,
285         property3 => $propery3,
286     }
287 );
288 
289 If not listed in $properties_hashref, the property will be set to the default
290 value defined at DB level, or nulled.

Created attachment 129515 [details] [review]
Bug 29719: Unit tests

Created attachment 129516 [details] [review]
Bug 29719: Do not clear onloan value when not passed in MARC

We blank the field to prevent users from setting it during import, but this has the
affect of blanking it in the DB.
This patch replaces the onloan field when not passed in to 'ModItemFromMARC' to preserve
the value

To test:
 1 - Check an item out to a paron
 2 - Export the item using Tools->Export data
 3 - Stage the record for import
 4 - Match on 999c and replace items
 5 - Import the batch
 6 - View the record and note item is checked out and Available
 7 - In the DB note the onloan value is now null
 8 - Check in the item
 9 - Apply patch
10 - Repeat 1-5
11 - View the record and note item is checked out
12 - In the DB note the onloan value matches the due date

Created attachment 134662 [details] [review]
Bug 29719: Unit tests

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 134663 [details] [review]
Bug 29719: Do not clear onloan value when not passed in MARC

We blank the field to prevent users from setting it during import, but this has the
affect of blanking it in the DB.
This patch replaces the onloan field when not passed in to 'ModItemFromMARC' to preserve
the value

To test:
 1 - Check an item out to a paron
 2 - Export the item using Tools->Export data
 3 - Stage the record for import
 4 - Match on 999c and replace items
 5 - Import the batch
 6 - View the record and note item is checked out and Available
 7 - In the DB note the onloan value is now null
 8 - Check in the item
 9 - Apply patch
10 - Repeat 1-5
11 - View the record and note item is checked out
12 - In the DB note the onloan value matches the due date

Signed-off-by: David Nind <david@davidnind.com>

Testing notes (koha-testing-docker):

- Note for me: remember to choose framework when importing staged records, otherwise it will fail

- Query for step 7 where XX is the item you checked out: 
  select * from items where itemnumber = XX;

Created attachment 134996 [details] [review]
Bug 29719: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 134997 [details] [review]
Bug 29719: Do not clear onloan value when not passed in MARC

We blank the field to prevent users from setting it during import, but this has the
affect of blanking it in the DB.
This patch replaces the onloan field when not passed in to 'ModItemFromMARC' to preserve
the value

To test:
 1 - Check an item out to a paron
 2 - Export the item using Tools->Export data
 3 - Stage the record for import
 4 - Match on 999c and replace items
 5 - Import the batch
 6 - View the record and note item is checked out and Available
 7 - In the DB note the onloan value is now null
 8 - Check in the item
 9 - Apply patch
10 - Repeat 1-5
11 - View the record and note item is checked out
12 - In the DB note the onloan value matches the due date

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Good call on moving the fix Katrin :)

All works as expected and Unit tests included. QA Script happy.

Passing QA

Good catch.

Are there other columns to protect by default ?
Like : 
issues, renewals, reserves ?
booksellerid ?
datelastseen ?

Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑

Can this be backported to 21.11?