Bug 29719 - onloan dates are cleared from items when importing and overlaying
Summary: onloan dates are cleared from items when importing and overlaying
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-16 16:09 UTC by Nick Clemens (kidclamp)
Modified: 2023-06-08 22:28 UTC (History)
4 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:
22.05.00
Circulation function:


Attachments
Bug 29719: Unit tests (1.59 KB, patch)
2021-12-16 16:23 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 29719: Do not clear onloan value when not passed in MARC (1.12 KB, patch)
2021-12-16 16:23 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 29719: Unit tests (1.88 KB, patch)
2022-01-14 20:01 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 29719: Do not clear onloan value when not passed in MARC (1.71 KB, patch)
2022-01-14 20:01 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 29719: Unit tests (1.93 KB, patch)
2022-05-05 20:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 29719: Do not clear onloan value when not passed in MARC (1.76 KB, patch)
2022-05-05 20:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 29719: Unit tests (1.99 KB, patch)
2022-05-14 07:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 29719: Do not clear onloan value when not passed in MARC (1.82 KB, patch)
2022-05-14 07:57 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2021-12-16 16:09:46 UTC
To recreate:
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
Comment 1 Nick Clemens (kidclamp) 2021-12-16 16:23:52 UTC
Created attachment 128635 [details] [review]
Bug 29719: Unit tests
Comment 2 Nick Clemens (kidclamp) 2021-12-16 16:23:55 UTC
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
Comment 3 Katrin Fischer 2022-01-09 00:40:38 UTC
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.
Comment 4 Nick Clemens (kidclamp) 2022-01-14 20:01:25 UTC
Created attachment 129515 [details] [review]
Bug 29719: Unit tests
Comment 5 Nick Clemens (kidclamp) 2022-01-14 20:01:29 UTC
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
Comment 6 David Nind 2022-05-05 20:47:00 UTC
Created attachment 134662 [details] [review]
Bug 29719: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2022-05-05 20:47:05 UTC
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>
Comment 8 David Nind 2022-05-05 20:52:10 UTC
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;
Comment 9 Martin Renvoize (ashimema) 2022-05-14 07:57:50 UTC
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>
Comment 10 Martin Renvoize (ashimema) 2022-05-14 07:57:54 UTC
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>
Comment 11 Martin Renvoize (ashimema) 2022-05-14 07:58:43 UTC
Good call on moving the fix Katrin :)

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

Passing QA
Comment 12 Fridolin Somers 2022-05-16 21:08:24 UTC
Good catch.

Are there other columns to protect by default ?
Like : 
issues, renewals, reserves ?
booksellerid ?
datelastseen ?
Comment 13 Fridolin Somers 2022-05-16 21:20:50 UTC
Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑
Comment 14 Lucas Gass (lukeg) 2022-10-05 17:50:50 UTC
Can this be backported to 21.11?