Bug 22801 - Advance search yr uses copydate instead of date-of-publication
Summary: Advance search yr uses copydate instead of date-of-publication
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Fridolin Somers
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 22524
Blocks:
  Show dependency treegraph
 
Reported: 2019-04-29 08:37 UTC by Fridolin Somers
Modified: 2021-08-16 18:55 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
This fixes the advanced search form in the OPAC and staff interface so that the publication date (and range) uses the value(s) in 008 instead of 260$c when using Elasticsearch.
Version(s) released in:
21.11.00,21.05.03,20.11.09


Attachments
Bug 22801: Advance search yr must use date-of-publication and not copydate (2.57 KB, patch)
2021-01-22 08:46 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 22801: Advance search yr must use date-of-publication and not copydate (2.63 KB, patch)
2021-06-17 12:37 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22801: (follow-up) Use date-of-publication directly (2.61 KB, patch)
2021-06-17 12:37 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 22801: Advance search yr must use date-of-publication and not copydate (2.62 KB, patch)
2021-06-17 12:40 UTC, David Nind
Details | Diff | Splinter Review
Bug 22801: Advance search yr must use date-of-publication and not copydate (2.67 KB, patch)
2021-06-17 13:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 22801: (follow-up) Use date-of-publication directly (2.65 KB, patch)
2021-06-17 13:03 UTC, David Nind
Details | Diff | Splinter Review
Bug 22801: Advance search yr must use date-of-publication and not copydate (2.74 KB, patch)
2021-07-23 15:06 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22801: (follow-up) Use date-of-publication directly (2.72 KB, patch)
2021-07-23 15:06 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2019-04-29 08:37:12 UTC
In advanced search with Elasticsearch the limit on years range is actually using copydate : 

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm in _fix_limit_special_cases() : 
        if ( $l =~ /^yr,st-numeric,ge=/ ) {
            my ( $start, $end ) =
              ( $l =~ /^yr,st-numeric,ge=(.*) and yr,st-numeric,le=(.*)$/ );
            next unless defined($start) && defined($end);
            push @new_lim, "copydate:[$start TO $end]";
        }

With Zebra it uses date-of-publication and also in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm :

our %index_field_convert = (
(...)
    'yr' => 'date-of-publication',
Comment 1 Katrin Fischer 2019-04-29 08:48:23 UTC
Not sure if it helps, but for MARC21 the date range uses 008 and the publication year option int he pull down uses 260$c (maybe also 264$c, haven't checked).
Comment 2 Fridolin Somers 2019-04-29 10:32:14 UTC
This is actually in etc/zebradb/ccl.properties :

#Date                   30  The point of time at which      005, 008/00-05,
#                           a transaction or event          008/07-10, 260$c,
#                           takes place.                    008/11-14, 033,etc.
# interpreting this as the copyright date in 260$c (MARC21) and 210$d (UNIMARC)
copydate 1=30 r=r

#Date-publication       31  The date (usually year) in      008/07-10, 260$c
#                           which a document is published.  046, 533$d
Date-of-publication 1=31 r=r
yr Date-of-publication
pubdate Date-of-publication
Comment 3 Katrin Fischer 2019-04-29 10:41:39 UTC
Hm, so copydate = 260$c

 443   <!--record.abs line 121: melm 260$c      copydate,copydate:s-->
 444   <index_subfields tag="260" subfields="c">
 445     <target_index>copydate:w</target_index>
 446     <target_index>copydate:s</target_index>
 447   </index_subfields>


And pubdate 008:

  49   <index_control_field tag="008" offset="7" length="4">
  50     <target_index>pubdate:w</target_index>
  51   </index_control_field>

and pubdate = yr = Date-of-publication

... and we are missing proper indexing of 264 (see 15704)

It seems I missed a change to advanced search, could have sworn we had publication year in the drop downs too. :(
Comment 4 Marjorie Barry-Vila 2020-06-11 15:52:49 UTC
+1
Comment 5 Marjorie Barry-Vila 2021-01-15 11:39:57 UTC
Hi,

Does anyone work on this problem?

If not, is there a way around the problem with configuring indexes?

Regards,
Marjorie
Comment 6 Fridolin Somers 2021-01-22 08:46:52 UTC
Created attachment 115666 [details] [review]
Bug 22801: Advance search yr must use date-of-publication and not copydate

In advanced search with Elasticsearch the limit on years range is actually using copydate :

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm in _fix_limit_special_cases() :
        if ( $l =~ /^yr,st-numeric,ge=/ ) {
            my ( $start, $end ) =
              ( $l =~ /^yr,st-numeric,ge=(.*) and yr,st-numeric,le=(.*)$/ );
            next unless defined($start) && defined($end);
            push @new_lim, "copydate:[$start TO $end]";
        }

With Zebra it uses date-of-publication and also in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm :

our %index_field_convert = (
(...)
    'yr' => 'date-of-publication',

This patch uses %index_field_convert to perform 'yr' limit.

Test plan:
1) Apply patch
2) Use Elasticsearch searchengine
3) Go to advanced search with 'More options'
4) Perform a search with a year limit (value or range)
5) Check results are correct
Comment 7 Nick Clemens 2021-06-17 12:37:44 UTC
Created attachment 122064 [details] [review]
Bug 22801: Advance search yr must use date-of-publication and not copydate

In advanced search with Elasticsearch the limit on years range is actually using copydate :

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm in _fix_limit_special_cases() :
        if ( $l =~ /^yr,st-numeric,ge=/ ) {
            my ( $start, $end ) =
              ( $l =~ /^yr,st-numeric,ge=(.*) and yr,st-numeric,le=(.*)$/ );
            next unless defined($start) && defined($end);
            push @new_lim, "copydate:[$start TO $end]";
        }

With Zebra it uses date-of-publication and also in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm :

our %index_field_convert = (
(...)
    'yr' => 'date-of-publication',

This patch uses %index_field_convert to perform 'yr' limit.

Test plan:
1) Apply patch
2) Use Elasticsearch searchengine
3) Go to advanced search with 'More options'
4) Perform a search with a year limit (value or range)
5) Check results are correct

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Nick Clemens 2021-06-17 12:37:47 UTC
Created attachment 122065 [details] [review]
Bug 22801: (follow-up) Use date-of-publication directly

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Nick Clemens 2021-06-17 12:39:06 UTC
Hi Frido,

I added a followup to remove:
my $yrfield = ( exists $index_field_convert{'yr'} ) ? $index_field_convert{'yr'} : 'yr';

The index_field_convert will always be built the same, so will always resolve to date-of-publication - so we don't need the condition, right?

I also update tests to pass
Comment 10 David Nind 2021-06-17 12:40:22 UTC
Created attachment 122066 [details] [review]
Bug 22801: Advance search yr must use date-of-publication and not copydate

In advanced search with Elasticsearch the limit on years range is actually using copydate :

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm in _fix_limit_special_cases() :
        if ( $l =~ /^yr,st-numeric,ge=/ ) {
            my ( $start, $end ) =
              ( $l =~ /^yr,st-numeric,ge=(.*) and yr,st-numeric,le=(.*)$/ );
            next unless defined($start) && defined($end);
            push @new_lim, "copydate:[$start TO $end]";
        }

With Zebra it uses date-of-publication and also in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm :

our %index_field_convert = (
(...)
    'yr' => 'date-of-publication',

This patch uses %index_field_convert to perform 'yr' limit.

Test plan:
1) Apply patch
2) Use Elasticsearch searchengine
3) Go to advanced search with 'More options'
4) Perform a search with a year limit (value or range)
5) Check results are correct

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 Nick Clemens 2021-06-17 12:42:45 UTC
(In reply to David Nind from comment #10)
> Signed-off-by: David Nind <david@davidnind.com>

Looks like we overlapped, can you test with second patch as well David?
Comment 12 David Nind 2021-06-17 12:48:56 UTC
> Looks like we overlapped, can you test with second patch as well David?

Will do!
Comment 13 David Nind 2021-06-17 13:03:48 UTC
Created attachment 122069 [details] [review]
Bug 22801: Advance search yr must use date-of-publication and not copydate

In advanced search with Elasticsearch the limit on years range is actually using copydate :

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm in _fix_limit_special_cases() :
        if ( $l =~ /^yr,st-numeric,ge=/ ) {
            my ( $start, $end ) =
              ( $l =~ /^yr,st-numeric,ge=(.*) and yr,st-numeric,le=(.*)$/ );
            next unless defined($start) && defined($end);
            push @new_lim, "copydate:[$start TO $end]";
        }

With Zebra it uses date-of-publication and also in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm :

our %index_field_convert = (
(...)
    'yr' => 'date-of-publication',

This patch uses %index_field_convert to perform 'yr' limit.

Test plan:
1) Apply patch
2) Use Elasticsearch searchengine
3) Go to advanced search with 'More options'
4) Perform a search with a year limit (value or range)
5) Check results are correct

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 David Nind 2021-06-17 13:03:52 UTC
Created attachment 122070 [details] [review]
Bug 22801: (follow-up) Use date-of-publication directly

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 David Nind 2021-06-17 13:05:34 UTC
Testing notes (koha-testing-docker):
- tested first using Zebra: works as expected
- changed search engine to Elastic and reindexed
- change a record so that 008 and 260$c have different values (such as 2009 and 2011)
- use the advanced search form > more options > limits > year to search on the 008 value (such as 2009) => record will NOT appear in the search results
- search using the 260$c value (such as 2011) => record WILL appear in the search results
- apply the patch (and reindex - not sure whether this is necessary, but did anyway)
- correct behavour should now occur:
  . results for limit search should show matching record with 008 value, not 260$c value
Comment 16 Kyle M Hall 2021-07-23 15:06:17 UTC
Created attachment 123138 [details] [review]
Bug 22801: Advance search yr must use date-of-publication and not copydate

In advanced search with Elasticsearch the limit on years range is actually using copydate :

Koha/SearchEngine/Elasticsearch/QueryBuilder.pm in _fix_limit_special_cases() :
        if ( $l =~ /^yr,st-numeric,ge=/ ) {
            my ( $start, $end ) =
              ( $l =~ /^yr,st-numeric,ge=(.*) and yr,st-numeric,le=(.*)$/ );
            next unless defined($start) && defined($end);
            push @new_lim, "copydate:[$start TO $end]";
        }

With Zebra it uses date-of-publication and also in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm :

our %index_field_convert = (
(...)
    'yr' => 'date-of-publication',

This patch uses %index_field_convert to perform 'yr' limit.

Test plan:
1) Apply patch
2) Use Elasticsearch searchengine
3) Go to advanced search with 'More options'
4) Perform a search with a year limit (value or range)
5) Check results are correct

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Kyle M Hall 2021-07-23 15:06:59 UTC
Created attachment 123139 [details] [review]
Bug 22801: (follow-up) Use date-of-publication directly

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 18 Jonathan Druart 2021-07-26 09:23:43 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 19 Kyle M Hall 2021-07-30 13:40:51 UTC
Pushed to 21.05.x fro 21.05.03
Comment 20 Fridolin Somers 2021-08-03 01:45:24 UTC
Pushed to 20.11.x for 20.11.09

Viva Elasticsearch ^^
Comment 21 Victor Grousset/tuxayo 2021-08-16 18:51:03 UTC
Missing dependencies for 20.05.x, it shouldn't be affected, no backport.
Comment 22 Victor Grousset/tuxayo 2021-08-16 18:55:27 UTC
errata: no issue related to dependencies, anyway:
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.