Bug 28384 - Add 'no_items' option to TransformMarcToKoha
Summary: Add 'no_items' option to TransformMarcToKoha
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-19 10:19 UTC by Nick Clemens
Modified: 2022-12-12 21:23 UTC (History)
3 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:
21.11.00,21.05.02


Attachments
Bug 28384: Add 'no_items' option to TransformMarcToKoha (4.66 KB, patch)
2021-05-19 10:25 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28384: Add 'no_items' option to TransformMarcToKoha (4.71 KB, patch)
2021-05-19 11:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 28384: Add 'no_items' option to TransformMarcToKoha (4.85 KB, patch)
2021-06-25 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28384: (QA follow-up) Rearranging unit tests (3.78 KB, patch)
2021-06-25 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28384: (follow-up) TransformMarcToKohaOneField should allow zeroes (1.57 KB, patch)
2021-06-25 08:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-05-19 10:19:29 UTC
The routine has an 'items' option if you only want item info, we should have the opposite, ignoring items if we don't need them.

In C4::Search::searchResults - we ignore the item info returned, skipping processing item lines can save us time
Comment 1 Nick Clemens 2021-05-19 10:25:17 UTC
Created attachment 121146 [details] [review]
Bug 28384: Add 'no_items' option to TransformMarcToKoha

This patch adds a new option 'no_items' and uses it in the C4::Search::searchResults
routine. We don't use the item info fetched here, so skipping those lines saves us time.

Additionally, I fix an incorrect FIXME comment, and pass the ISBN returned by the routine above
into GetNormalizedISBN to save another lookup

TO test:
1 - Enable AmazonCoverImages system preference
2 - Search staff client with a term that returns books with covers
3 - Apply patch
4 - prove -v t/db_dependent/Biblio/TransformMarcToKoha.t
5 - Confirm searching works
6 - Confirm Amazon images display (normalized_isbn is used for these)
Comment 2 David Nind 2021-05-19 11:02:51 UTC
Created attachment 121150 [details] [review]
Bug 28384: Add 'no_items' option to TransformMarcToKoha

This patch adds a new option 'no_items' and uses it in the C4::Search::searchResults
routine. We don't use the item info fetched here, so skipping those lines saves us time.

Additionally, I fix an incorrect FIXME comment, and pass the ISBN returned by the routine above
into GetNormalizedISBN to save another lookup

TO test:
1 - Enable AmazonCoverImages system preference
2 - Search staff client with a term that returns books with covers
3 - Apply patch
4 - prove -v t/db_dependent/Biblio/TransformMarcToKoha.t
5 - Confirm searching works
6 - Confirm Amazon images display (normalized_isbn is used for these)

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Marcel de Rooy 2021-06-25 07:44:20 UTC
QAing
Comment 4 Marcel de Rooy 2021-06-25 08:47:25 UTC
Created attachment 122418 [details] [review]
Bug 28384: Add 'no_items' option to TransformMarcToKoha

This patch adds a new option 'no_items' and uses it in the C4::Search::searchResults
routine. We don't use the item info fetched here, so skipping those lines saves us time.

Additionally, I fix an incorrect FIXME comment, and pass the ISBN returned by the routine above
into GetNormalizedISBN to save another lookup

TO test:
1 - Enable AmazonCoverImages system preference
2 - Search staff client with a term that returns books with covers
3 - Apply patch
4 - prove -v t/db_dependent/Biblio/TransformMarcToKoha.t
5 - Confirm searching works
6 - Confirm Amazon images display (normalized_isbn is used for these)

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Amended, replacing a tab character.
Comment 5 Marcel de Rooy 2021-06-25 08:47:28 UTC
Created attachment 122419 [details] [review]
Bug 28384: (QA follow-up) Rearranging unit tests

This does two things:
[1] Rearranging makes clearing the cache twice no longer needed.
[2] Preparing the ground for an additional refinement in the
    TransformMarcToKoha routine.

Test plan:
Run TransformMarcToKoha.t.
Should fail now for:
    not ok 1 - Biblio field 510a
    not ok 5 - Biblio field returned when "no_items" passed

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2021-06-25 08:47:32 UTC
Created attachment 122420 [details] [review]
Bug 28384: (follow-up) TransformMarcToKohaOneField should allow zeroes

Until now, the test did not pass false values like '0' or ''.
But we should pass '0' and not ''.

Note that the test for control fields is a bit theoretical since
we dont have control fields bound to Koha fields currently.

The adjusted unit test in the former patch should now pass.

Test plan:
Run TransformMarcToKoha.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Since this change theoretically has a larger impact, I also
tested the cataloguing editor, item editor. And run quite a
few tests referring to AddBiblio or ModBiblio.
Comment 7 Marcel de Rooy 2021-06-25 08:51:37 UTC
(In reply to Nick Clemens from comment #0)
> In C4::Search::searchResults - we ignore the item info returned, skipping
> processing item lines can save us time

The time savings will be absolutely minimal since this whole process runs in memory using cached values etc.
Comment 8 Marcel de Rooy 2021-06-25 09:20:30 UTC
Removed the follow-ups.
Comment 9 Jonathan Druart 2021-07-12 13:49:34 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 10 Kyle M Hall 2021-07-16 13:32:56 UTC
Performance patch, backported to 21.05.x for 21.05.02
Comment 11 Fridolin Somers 2021-08-02 21:41:48 UTC
Enhancement not pushed to 20.11.x