Bug 36902

Summary: Blank year fields should not be indexed.
Product: Koha Reporter: Andreas Jonsson <andreas.jonsson>
Component: Searching - ElasticsearchAssignee: Andreas Jonsson <andreas.jonsson>
Status: Failed QA --- QA Contact:
Severity: trivial    
Priority: P5 - low CC: david, januszop, magnus, nick
Version: unspecified   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 24807    
Bug Blocks:    
Attachments: Bug 36902: Filter year fields containing blanks
Bug 36902: Filter year fields containing blanks

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)