Bug 36930 - Item search gives irrelevant results when using 2+ added filter criteria
Summary: Item search gives irrelevant results when using 2+ added filter criteria
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low trivial (vote)
Assignee: Janusz Kaczmarek
QA Contact: Martin Renvoize
URL:
Keywords: RM_priority
Depends on: 36563
Blocks:
  Show dependency treegraph
 
Reported: 2024-05-22 21:05 UTC by Janusz Kaczmarek
Modified: 2024-07-25 11:11 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.02,23.11.07


Attachments
Bug 36930: Item search gives irrelevant results when using 2+ added filter criteria (1.92 KB, patch)
2024-05-22 21:18 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 36930: Item search gives irrelevant results when using 2+ added filter criteria (1.98 KB, patch)
2024-05-24 15:08 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36930: Item search gives irrelevant results when using 2+ added filter criteria (2.05 KB, patch)
2024-06-03 11:06 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Janusz Kaczmarek 2024-05-22 21:05:03 UTC
In the Item  search a librarian is allowed, in the first step, to define additional filters like Title, Author, Publisher, Publication date etc. (the third fieldset).  This works fine but only for one criterion.  If one adds two or more criteria, the filter does not apply at all.
Comment 1 Janusz Kaczmarek 2024-05-22 21:18:56 UTC
Created attachment 167052 [details] [review]
Bug 36930: Item search gives irrelevant results when using 2+ added filter criteria

In the Item search the librarian is allowed, in the first step, to define
additional filters like Title, Author, Publisher, Publication date etc.
(in the third fieldset).  This works fine but only for one criterion.
If one adds two or more criteria, the filter does not apply at all.

Test plan
=========
1. Make an Item search with the Pulblisher filter. Put
   %University of California% as the value.
   You should get 5 rows (with standard ktd test data set), three
   from 1982, and two from 1988.
2. Edit search -> add the second criterion: AND Publication date is 1982.
   You would expect three rows but you get 900+ rows.
3. Apply the patch; restart_all.
4. Repeat p. 2. You should get the expected three rows.
Comment 2 Janusz Kaczmarek 2024-05-22 21:22:20 UTC
BTW, I have no idea why the CGI parameters change their names from q into q[], f into f[], etc., depending on the number of the rows in the form--but they do.  Certainly there is an explanation--any hint would be appreciated.
Comment 3 Katrin Fischer 2024-05-23 07:47:11 UTC
This is needed in addition to bug 36563?
Comment 4 Janusz Kaczmarek 2024-05-23 08:04:57 UTC
(In reply to Katrin Fischer from comment #3)
> This is needed in addition to bug 36563?

Oh, that's strange indeed.  The bug description seems similar.  To be sincere, I didn't see it prior to submitting my patch, but I worked on fresh main of yesterday.  So, it seems to be still a problem.  The bug 36563 would work only for the two first fieldsetd, if I guess right.  And the mine--for the third.
Comment 5 Pedro Amorim 2024-05-23 11:37:50 UTC
Hey, I intend to take a look here when possible but adding other possibly interested parties to the mix in the meantime.
Comment 6 Pedro Amorim 2024-05-24 15:06:46 UTC
(In reply to Janusz Kaczmarek from comment #2)
> Certainly there is an explanation--any hint would be appreciated.

This patch (I believe a follow-up to the datatables upgrade from 1.10.18 to 1.13.6):
https://github.com/Koha-Community/Koha/commit/63183059dd975c2f5265fa4c7d2bc59b6ad1a252#diff-20d8383af8b7699632bc26d50203158fc58acbfad77efdbe6134017a1a1d1ec7R482

(link goes directly to the line we're addressing here)
This changed the payload data from an array of JSON objects to a single JSON object.
This caused a problem, because a JSON object can only have one instance of a property and this scenario was not considered for more than 1 arg for the same filter.
Using Janusz's use-case, this is what the request payload looked like:
[
  ...,
  {name: 'f', value: 'publishercode'},
  {name: 'op', value: 'like'},
  {name: 'q', value: '%University of California%'},
  {name: 'c', value: 'and'},
  {name: 'f', value: 'publicationyear'},
  {name: 'op', value: 'like'},
  {name: 'q', value: '1988'},
  ...
]

But after that patch, this is what we had (same use-case - notice the absence of the first query parameters set):
{
  ...
  c: 'and',
  f: 'publicationyear',
  op: 'like',
  q: '1988',
  ...
}

This is because the following line:
d[params[i].name] = params[i].value;
Was effectivelly doing:
d['f'] = 'publishercode';
d['f'] = 'publicationyear';

The end result is you only get the last property value in the final object.

You can test this yourself:
Rest your branch to the commit directly before the datatables upgrade:
git reset --hard b54da05c73c9ea5b04d3552b34fcc636b0a71851
reset_all
And console.log(aoData) in itemsearch.tt

Then, reset your branch to the commit directly before bug 36563:
git reset --hard 903a8685c661ddde61335a08e126506120345abc
reset_all
And console.log(d) after the for loop
Comment 7 Pedro Amorim 2024-05-24 15:08:57 UTC
Created attachment 167169 [details] [review]
Bug 36930: Item search gives irrelevant results when using 2+ added filter criteria

In the Item search the librarian is allowed, in the first step, to define
additional filters like Title, Author, Publisher, Publication date etc.
(in the third fieldset).  This works fine but only for one criterion.
If one adds two or more criteria, the filter does not apply at all.

Test plan
=========
1. Make an Item search with the Pulblisher filter. Put
   %University of California% as the value.
   You should get 5 rows (with standard ktd test data set), three
   from 1982, and two from 1988.
2. Edit search -> add the second criterion: AND Publication date is 1982.
   You would expect three rows but you get 900+ rows.
3. Apply the patch; restart_all.
4. Repeat p. 2. You should get the expected three rows.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 8 Pedro Amorim 2024-05-24 15:11:47 UTC
I think the patch is good, it's basically a follow-up of bug 36563 as it's addressing use cases we missed (i.e. forgot to test) at the time.
Comment 9 Paulina 2024-05-29 05:57:43 UTC
Works fine, even with +3 filter criteria (one with MARC field 260$a)
Comment 10 Martin Renvoize 2024-06-03 11:06:09 UTC
Created attachment 167340 [details] [review]
Bug 36930: Item search gives irrelevant results when using 2+ added filter criteria

In the Item search the librarian is allowed, in the first step, to define
additional filters like Title, Author, Publisher, Publication date etc.
(in the third fieldset).  This works fine but only for one criterion.
If one adds two or more criteria, the filter does not apply at all.

Test plan
=========
1. Make an Item search with the Pulblisher filter. Put
   %University of California% as the value.
   You should get 5 rows (with standard ktd test data set), three
   from 1982, and two from 1988.
2. Edit search -> add the second criterion: AND Publication date is 1982.
   You would expect three rows but you get 900+ rows.
3. Apply the patch; restart_all.
4. Repeat p. 2. You should get the expected three rows.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2024-06-03 11:06:55 UTC
Thanks for the fix here Janusz, and the clear reasoning and testing Pedro.

All looking solid to me, 

Passing QA
Comment 12 Martin Renvoize 2024-06-13 13:33:03 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 13 Lucas Gass 2024-07-18 17:51:18 UTC
Backported to 24.05.x for upcoming 24.05.02
Comment 14 Fridolin Somers 2024-07-19 07:54:36 UTC
Pushed to 23.11.x for 23.11.07