Bug 30779 - Do not need to remove items from import biblios marc
Summary: Do not need to remove items from import biblios marc
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 38279
  Show dependency treegraph
 
Reported: 2022-05-16 19:55 UTC by Nick Clemens (kidclamp)
Modified: 2024-10-28 13:25 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,22.05.06,21.11.12
Circulation function:


Attachments
Bug 30779: Do not update import marc after importing items (1.45 KB, patch)
2022-05-16 20:01 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30779: Do not update import marc after importing items (1.49 KB, patch)
2022-08-01 14:39 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 30779: Remove _update_import_record_marc and update tests (4.66 KB, patch)
2022-08-17 12:03 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30779: Remove _update_import_record_marc and update tests (5.81 KB, patch)
2022-08-17 12:49 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 30779: Do not update import marc after importing items (1.57 KB, patch)
2022-08-17 13:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30779: Remove _update_import_record_marc and update tests (5.89 KB, patch)
2022-08-17 13:08 UTC, Jonathan Druart
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) 2022-05-16 19:55:59 UTC
When staging biblios we save the marc, then, if we are looking for items and find them, we save the marc without the 952

While there is a space savings here, I think the extra write is costing us with no benefit.

While we could remove the items before the initial save, I think there is actually value in seeing the items in the imported marc, so one can compare what was imported to what was created
Comment 1 Nick Clemens (kidclamp) 2022-05-16 20:01:15 UTC
Created attachment 135051 [details] [review]
Bug 30779: Do not update import marc after importing items

We are stripping the marc item tags at import - we save them when not importing items, but
strip when importing items

I think we can save on writes by leaving them in the record. This also allows comparison to what was staged
versus items created

To test:
 1 - Stage a marc record with items, but do not look for items
 2 - Confirm the item tags remain in staged marc
 3 - Import the record
 4 - Confirm items are nto in imported marc record
 5 - Stage the record again, but look for items
 6 - Confirm the item tags are stipped from imported record
 7 - Import and confirm imported record has no item tags
 8 - Apply patch and repeat 1-5
 9 - Confirm item tags remain in record
10 - Import and confirm item tags not in imported marc
Comment 2 ByWater Sandboxes 2022-08-01 14:39:17 UTC
Created attachment 138475 [details] [review]
Bug 30779: Do not update import marc after importing items

We are stripping the marc item tags at import - we save them when not importing items, but
strip when importing items

I think we can save on writes by leaving them in the record. This also allows comparison to what was staged
versus items created

To test:
 1 - Stage a marc record with items, but do not look for items
 2 - Confirm the item tags remain in staged marc
 3 - Import the record
 4 - Confirm items are nto in imported marc record
 5 - Stage the record again, but look for items
 6 - Confirm the item tags are stipped from imported record
 7 - Import and confirm imported record has no item tags
 8 - Apply patch and repeat 1-5
 9 - Confirm item tags remain in record
10 - Import and confirm item tags not in imported marc

Signed-off-by: Andrew <andrewfh@dubcolib.org>
Comment 3 Jonathan Druart 2022-08-09 09:08:16 UTC
* space vs time? How do you decide which one is best? :D

* _update_import_record_marc is no longer used

* Missing tests.
Comment 4 Nick Clemens (kidclamp) 2022-08-17 12:03:09 UTC
Created attachment 139275 [details] [review]
Bug 30779: Remove _update_import_record_marc and update tests

This patch removes the now unused: _update_import_record_marc

Additionally, as items are already present in import biblio we no longer need to embed
them, so that parameter is removed

In all cases, we were embedding, so we don't need a way to get without

Tests updated
Comment 5 Nick Clemens (kidclamp) 2022-08-17 12:04:01 UTC
(In reply to Jonathan Druart from comment #3)
> * space vs time? How do you decide which one is best? :D

Storage can be expanded, and space recovered. Time is not a renewable resource

> * _update_import_record_marc is no longer used

removed

> * Missing tests.

tests updated
Comment 6 Nick Clemens (kidclamp) 2022-08-17 12:49:16 UTC
Created attachment 139285 [details] [review]
Bug 30779: Remove _update_import_record_marc and update tests

This patch removes the now unused: _update_import_record_marc

Additionally, as items are already present in import biblio we no longer need to embed
them, so that parameter is removed and the option removed from the sub and pod and everywhere
it was used

In all cases, we were embedding, so we don't need a way to get without

Tests updated
Comment 7 Jonathan Druart 2022-08-17 13:08:51 UTC
Created attachment 139287 [details] [review]
Bug 30779: Do not update import marc after importing items

We are stripping the marc item tags at import - we save them when not importing items, but
strip when importing items

I think we can save on writes by leaving them in the record. This also allows comparison to what was staged
versus items created

To test:
 1 - Stage a marc record with items, but do not look for items
 2 - Confirm the item tags remain in staged marc
 3 - Import the record
 4 - Confirm items are nto in imported marc record
 5 - Stage the record again, but look for items
 6 - Confirm the item tags are stipped from imported record
 7 - Import and confirm imported record has no item tags
 8 - Apply patch and repeat 1-5
 9 - Confirm item tags remain in record
10 - Import and confirm item tags not in imported marc

Signed-off-by: Andrew <andrewfh@dubcolib.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2022-08-17 13:08:56 UTC
Created attachment 139288 [details] [review]
Bug 30779: Remove _update_import_record_marc and update tests

This patch removes the now unused: _update_import_record_marc

Additionally, as items are already present in import biblio we no longer need to embed
them, so that parameter is removed and the option removed from the sub and pod and everywhere
it was used

In all cases, we were embedding, so we don't need a way to get without

Tests updated

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Tomás Cohen Arazi (tcohen) 2022-08-18 12:05:37 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 10 Lucas Gass (lukeg) 2022-10-03 22:50:39 UTC
Backported to 22.05.x for 22.05.06
Comment 11 Victor Grousset/tuxayo 2022-10-17 00:27:46 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.
Comment 12 Arthur Suzuki 2022-10-22 22:08:09 UTC
Thanks!

Pushed to 21.11 for 21.11.12