Bug 14456 - EmbedSeeFromHeadings record filter shouldn't process MARC holding fields
Summary: EmbedSeeFromHeadings record filter shouldn't process MARC holding fields
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jacek Ablewicz
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-24 17:41 UTC by Jacek Ablewicz
Modified: 2016-12-05 21:23 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14456 - EmbedSeeFromHeadings record filter shouldn't process MARC holding fields (2.89 KB, patch)
2015-06-24 17:49 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
[SIGNED OFF] Bug 14456: (regression test) Holdings fields should be skipped by EmbedSeeFromHeadings (2.95 KB, patch)
2015-07-22 14:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[SIGNED OFF] Bug 14456: EmbedSeeFromHeadings record filter shouldn't process MARC holding fields (3.20 KB, patch)
2015-07-22 14:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[PASSED QA] Bug 14456: (regression test) Holdings fields should be skipped by EmbedSeeFromHeadings (3.01 KB, patch)
2015-07-23 22:28 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 14456: EmbedSeeFromHeadings record filter shouldn't process MARC holding fields (3.25 KB, patch)
2015-07-23 22:29 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Ablewicz 2015-06-24 17:41:57 UTC
If the system preference IncludeSeeFromInSearches is enabled, records exported for zebra indexing are being additionally processed by EmbedSeeFromHeadings record filter (right now used only in rebuild_zebra.pl script). This filter embeds 'see from' fields (extracted from authority records linked with the given biblio via $9 subfields) into target MARC record, which is then subsequently indexed in zebra.

Currently all fields containing $9 are getting the same exact treatment by this filter. But on the export stage when the filter is applied, MARC record being processed already does have holdings data fields added in the previous stage (usually 952 / 995, depending on the MARC format). Problem is that holdings data fields use to have $9 subfields in them as well (mapped to item.itemnumber by default). As a consequence, some (great many in the typical setup) records exported for zebra indexing may have surplus "see from" fields added erroneously in semi-random fashion, so biblio searches would often return some completely unexpected additional results.

EmbedSeeFromHeadings record filter should not process holdings fields when dealing with MARC records intended for zebra indexing.

To reproduce:

1) database with as many sample or real-world biblio, item and authority records as possible is recommended for testing purposes
2) enable IncludeSeeFromInSearches
3) export a bunch of biblio records for zebra (e.g.: misc/migration_tools/rebuild_zebra.pl -I -b -x -k -length=1000), inspect the result xml records in /tmp/<whatever> file; observe that at the end of many records, here and there some extra "see from" (= 1st indicator: 'z') fields tend to appear, which shouldn't be there ;)
Comment 1 Jacek Ablewicz 2015-06-24 17:49:18 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2015-07-22 14:28:44 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2015-07-22 14:28:55 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2015-07-23 22:28:45 UTC
Created attachment 41141 [details] [review]
[PASSED QA] Bug 14456: (regression test) Holdings fields should be skipped by EmbedSeeFromHeadings

This patch introduces a regression test for the situation in which
an itemnumber on a record matches the authid of an authority record
and the current implementation of the EmbedSeeFromHeadings filter
wrongly includes new holding fields with authority data...

To test:
- Apply the patch
- Run:
  $ prove t/db_dependent/RecordProcessor_EmbedSeeFromHeadings.t
=> FAIL: The test 'Holdings fields not processed to introduce See-from heading'
   fails.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2015-07-23 22:29:03 UTC
Created attachment 41142 [details] [review]
[PASSED QA] Bug 14456: EmbedSeeFromHeadings record filter shouldn't process MARC holding fields

If the system preference IncludeSeeFromInSearches is enabled, records
exported for zebra indexing are being additionally processed by
EmbedSeeFromHeadings record filter (right now used only in rebuild_zebra.pl
script). This filter embeds 'see from' fields (extracted from authority
records linked with the given biblio via $9 subfields) into target MARC
record, which is then subsequently indexed in zebra.

Currently all fields containing $9 are getting the same exact treatment
by this filter. But on the export stage when the filter is applied, MARC
record being processed already does have holdings data fields added in
the previous stage (usually 952 / 995, depending on the MARC format).
Problem is that holdings data fields use to have $9 subfields in them
as well (mapped to item.itemnumber by default). As a consequence, some
(great many in the typical setup) records exported for zebra indexing
may have surplus "see from" fields added erroneously in semi-random
fashion, so biblio searches would often return some completely
unexpected additional results.

EmbedSeeFromHeadings record filter should not process holdings fields
when dealing with MARC records intended for zebra indexing.

To reproduce:

1) database with as many sample or real-world biblio, item and authority
records as possible is recommended for testing purposes
2) enable IncludeSeeFromInSearches
3) export a bunch of biblio records for zebra (e.g.:
misc/migration_tools/rebuild_zebra.pl -I -b -x -k -length=1000),
inspect the result xml records in /tmp/<whatever> file; observe that at
the end of many records, here and there some extra "see from" (= 1st
indicator: 'z') fields tend to appear, which shouldn't be there ;)

To test:

4) apply patch
5) redo 3)
6) compare results from 3) and 5) with diff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I introduced a regression test for this. You should run the tests
without/with the patch and verify that the patch actually fixes the problem.
Good job Jacek! I'm sure writing the regression test would take less time
than such a detailed commit message!

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Tomás Cohen Arazi 2015-07-24 17:10:21 UTC
Important fix pushed to master.

thanks Jacek!
Comment 7 Chris Cormack 2015-08-05 03:06:08 UTC
Pushed to 3.20.x will be in 3.20.3
Comment 8 Liz Rea 2015-08-11 02:01:31 UTC
Pushed to 3.18.x, will be in 3.18.10.
Comment 9 Mason James 2015-08-28 05:45:07 UTC
Pushed to 3.16.x, will be in 3.16.14