Bug 25964 - Data loss possible when items are modified
Summary: Data loss possible when items are modified
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 23463
Blocks:
  Show dependency treegraph
 
Reported: 2020-07-09 09:30 UTC by Jonathan Druart
Modified: 2021-12-13 21:08 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.03


Attachments
Bug 25964: Prevent data loss when editing items from a MARC record (3.70 KB, patch)
2020-07-09 09:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25964: Prevent data loss when editing items from a MARC record (3.76 KB, patch)
2020-07-09 17:39 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 25964: Prevent data loss when editing items from a MARC record (3.82 KB, patch)
2020-07-10 09:51 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-07-09 09:30:53 UTC
Coming from:
  Bug 23463: Use new method Koha::Object->set_or_blank

We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.

In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record.
Comment 1 Jonathan Druart 2020-07-09 09:33:06 UTC
Created attachment 106695 [details] [review]
Bug 25964: Prevent data loss when editing items from a MARC record

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

We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.

In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record
Comment 2 Jonathan Druart 2020-07-09 09:34:23 UTC
I don't have much time to investigate the consequence of this bug but it sounds quite bad. Could you have a look?
Comment 3 David Cook 2020-07-09 10:02:39 UTC
Sounds bad. I haven't looked either. Trying to remember where ModItemFromMarc gets called...
Comment 4 Victor Grousset/tuxayo 2020-07-09 17:39:02 UTC
Created attachment 106752 [details] [review]
Bug 25964: Prevent data loss when editing items from a MARC record

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

We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.

In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2020-07-09 17:40:29 UTC
My test plan was to apply this patch on top of bug 20271 and run:
prove t/db_dependent/Items.t
to confirm that it's fixed. See bug 20271 comment 243.

And then apply it to master, run the same test to confirm that it still passes.
And attach the signed off patch here (the on-top-of-master one)


Is that a good test? If so we can mark this ticket as signed off
Comment 6 Martin Renvoize 2020-07-10 09:51:44 UTC
Created attachment 106765 [details] [review]
Bug 25964: Prevent data loss when editing items from a MARC record

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

We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.

In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2020-07-10 09:53:21 UTC
I'm comfortable that this patch fixes the issue and I'm not seeing any obvious regressions... Passing QA
Comment 8 Tomás Cohen Arazi 2020-07-13 14:42:59 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Lucas Gass 2020-07-24 20:34:56 UTC
backported to 20.05.x for 20.05.03
Comment 10 Aleisha Amohia 2020-08-03 20:22:38 UTC
missing dependencies, not backported to 19.11.x