Bug 36902 - Blank year fields should not be indexed.
Summary: Blank year fields should not be indexed.
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial
Assignee: Andreas Jonsson
QA Contact:
URL:
Keywords:
Depends on: 24807
Blocks:
  Show dependency treegraph
 
Reported: 2024-05-21 07:00 UTC by Andreas Jonsson
Modified: 2024-10-16 10:16 UTC (History)
4 users (show)

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


Attachments
Bug 36902: Filter year fields containing blanks (1.81 KB, patch)
2024-05-21 07:32 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 36902: Filter year fields containing blanks (1.85 KB, patch)
2024-05-23 06:51 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jonsson 2024-05-21 07:00:13 UTC
The comment says 'Only accept years containing digits and "u"' but proceeds to also accept fields with blanks:

        # Only accept years containing digits and "u"
        push @{$default_options->{value_callbacks}}, sub {
            my ($value) = @_;
            # Replace "u" with "0" for sorting
            return map { s/[u\s]/0/gr } ( $value =~ /[0-9u\s]{4}/g );
        };

If the field contains blanks it is either because it is invalid or because the year is missing, so the field should not be indexed.
Comment 1 Andreas Jonsson 2024-05-21 07:32:37 UTC
Created attachment 166951 [details] [review]
Bug 36902: Filter year fields containing blanks

Test plan:

* If you are using koha-testing-docker, make sure that you have an elasticsearch container
  running (e.g., ku-es7) and that the searchengine system preference is set to
  ElasticSearch, and that the indexer is running (koha-es-indexer --start kohadev).
* Under Koha administration -> Search engine configuration, make sure that the
  type of the field date-of-publication is set to 'year'.
* If you needed to changed the type above, rebuild the index:
  koha-elasticsearch --delete --rebuild kohadev
* Edit any record and change control field 008 position 7-10 (Date 1) to all blanks.
* Save the record.
* Show the elasticsearch record (Klick on "Show" in Elasticsearch record: Show")
* Make sure there are no value "0000" for date-of-publication.
* Edit the record again and change control field 008 position 7-10 (Date 1) to valid year.
* Save the record.
* Show the elasticsearch record.
* Make sure there set year is present in the field date-of-publication.
Comment 2 Janusz Kaczmarek 2024-05-21 12:11:04 UTC
I would agree that a blank in 008/7-10 is usually an invalid value, but not for 008/6 = 'b' (a very rare case).

The current behavior is more forgiving which may a be good or may be bad think.

My doubt is: how this patch would impact the sorting feature, i.e. when sorting according to the date of publication, where these invalid records will be placed not having 008/7-10 indexed?
Comment 3 Andreas Jonsson 2024-05-21 21:13:18 UTC
The default is to sort documents with missing sort field last, which seems reasonable to me.

My client wants to have this fixed for this very reason, i.e., to not have the search results cluttered by records with invalid year in 008 when sorting on date of publication.
Comment 4 David Nind 2024-05-23 06:51:24 UTC
Created attachment 167065 [details] [review]
Bug 36902: Filter year fields containing blanks

Test plan:

* If you are using koha-testing-docker, make sure that you have an elasticsearch container
  running (e.g., ku-es7) and that the searchengine system preference is set to
  ElasticSearch, and that the indexer is running (koha-es-indexer --start kohadev).
* Under Koha administration -> Search engine configuration, make sure that the
  type of the field date-of-publication is set to 'year'.
* If you needed to changed the type above, rebuild the index:
  koha-elasticsearch --delete --rebuild kohadev
* Edit any record and change control field 008 position 7-10 (Date 1) to all blanks.
* Save the record.
* Show the elasticsearch record (Klick on "Show" in Elasticsearch record: Show")
* Make sure there are no value "0000" for date-of-publication.
* Edit the record again and change control field 008 position 7-10 (Date 1) to valid year.
* Save the record.
* Show the elasticsearch record.
* Make sure there set year is present in the field date-of-publication.

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Nick Clemens (kidclamp) 2024-06-24 23:04:36 UTC
See bug 24807 comment 29:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24807#c29

3 digit years and UNIMARC need to be considered here.

YASP?
Comment 6 Magnus Enger 2024-10-15 13:03:30 UTC
Sorry if this is not relevant here. 

We are seeing searches ordered by publication date, where the first few records are sorted as expected, and then the rest seem to be in random order. When we check the "Elasticsearch record" for records that fall in the "random" category, the data for date-of-publication often looks something like this: 

"date-of-publication": [
				"    ",
				"2019"
			],

My hunch is that the first, blank string is from 008, and the second string is from 260c. And that only the first string is indexed, so records with a blank year in 008 are returned at the end of the list, in seemingly random order. 

Would it be possible to skip the first string here, if it is only four blank spaces, and consider the second string for indexing, if there is a second string?
Comment 7 Janusz Kaczmarek 2024-10-15 13:19:47 UTC
I sounds strange--by default only 008/7-10 should be indexed with date-of-publication (for MARC 21).  (In 260c there are other things like 'cop.' etc. that would make sorting and searching by date unpredictable and therefor 260c shouldn't be indexed with date-of-publication, IMO).  I would check the local mapping.  Or maybe there are two 008 fields in the record in question...?
Comment 8 Magnus Enger 2024-10-16 10:16:44 UTC
Ah, indexing 260c as date-of-publication was a local thing. Sorry for the noise. (The solution for us is probably to run a script to copy dates from 260c to 008/7-10)