Bug 17787

Summary: Hidden items included in count of search results
Product: Koha Reporter: Viktor Sarge <viktor.sarge>
Component: SearchingAssignee: Tomás Cohen Arazi <tomascohen>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, fridolin.somers, janet.mcgowan, jonathan.druart, jrobb, katrin.fischer, lucas, marjorie.barry-vila, martin.renvoize, noemie.labine, r.delahunty, rogan.hamby, severine.queune, simon.voyer, tomascohen, victor
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18989
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31114
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 24403    
Attachments: Wrong number of results when opachiddenitems is used
Bug 17787: Hidden items included in count of search results
Bug 17787: Hidden items included in count of search results

Description Viktor Sarge 2016-12-16 16:12:34 UTC
We have a use case where like 99% of the collection is hidden and only a few selected items are displayed in the opac (by showing their covers up front on the main page). When going to the detail page and clicking the link for an author that has a long list of records (but only one visible in the opac) I might get indication of 63 hits in the search results and a the links to pages 1,2,3 etc for search results. Only problem is that there is actually only one result since the rest are suppressed. 

My guess is that the code calculating the number of hits and creating links to the pages of search results is not talking to the suppression system. I could not find any open bugs about this. Perhaps because our use of suppression is a bit of a edge case(?) 

I ended up "solving" the problem by removing the links on author etc. since we don't use search anyhow. I'll likely not research the issue further, but I felt I should at least report it and keep an eye out. All input is welcome.
Comment 1 Séverine Queune 2018-09-21 14:11:20 UTC
Still valid.
Same behaviour when OpacHiddenItems is used.
See screenshot attached.
Comment 2 Séverine Queune 2018-09-21 14:12:04 UTC
Created attachment 79234 [details]
Wrong number of results when opachiddenitems is used
Comment 3 Katrin Fischer 2019-12-07 10:23:32 UTC
*** Bug 24142 has been marked as a duplicate of this bug. ***
Comment 4 Gérard Simon Voyer 2021-04-21 18:29:40 UTC
Bonjour everyone!

I've had my shot at this and broke my logic circuits hard.
At first, I tried complex matching rules to try while opening loops done over @newresults (from opac/opac-search.pl) to get something. That's when I realized nothing ever matched. Then I realized that @newresults was already filtered, which is why I had no match.

So in the end :

my $results_count = scalar @newresults;
...
...
         if ($results_hashref->{$server}->{"hits"}){
            $total = $results_count;
         }
...
...
$template->param(total => $total);

gave me the right count...

I'll have to go document myself on how to propose a fix on this issue, but I think the explanation above can give anyone looking for an answer the missing piece.
Comment 5 Jonathan Druart 2021-05-07 07:50:02 UTC
Seems correct with OpacHiddenItems now on master.
Comment 6 Jonathan Druart 2021-05-07 07:52:38 UTC
(In reply to Jonathan Druart from comment #5)
> Seems correct with OpacHiddenItems now on master.

staff detail: https://snipboard.io/B1JPT9.jpg
opac search : https://snipboard.io/zMv1qo.jpg

 OpacHiddenItems: itype: [VM]
Comment 7 Gérard Simon Voyer 2021-05-07 12:53:35 UTC
Hi Jonathan!

Is there anything I have to do to follow-up and possibly help you close the bug? I'm new and joining IRC today so I can start seasoning myself as a Koha dev. 

I realize that the 40+ Koha boxes we maintain and host at Collecto are using a codebase that is shifting towards an important an uncomfortable diff with the master branch.

I worked around using $results_count = scalar @newresults;

if ($hits > results_per_page) { $total = $results_count } else { $total = hits }

But this seems to base itself off the fact that our users will not notice biased results counts if the results span over more than a page.

I'd rather use official patches from veteran Koha devs!
Comment 8 Tomás Cohen Arazi 2021-05-07 13:59:40 UTC
My opinion, is that we should add indexes for hidden things, and implicitly use them so records are not included in search results. As opposed to doing funky arithmetic calculations afterwards.

This would solve many other issues like hidden records being included in facets, etc.

I can commit to work on this after the 21.05 release.
And would love to have help with testing, from people participating in this bug.
Comment 9 Gérard Simon Voyer 2021-05-07 15:27:50 UTC
Currently setting up a fresh docker-testing box so I can sign-off on a few bugs I've commented on.
Comment 10 Gérard Simon Voyer 2021-05-07 15:41:35 UTC
(In reply to Tomás Cohen Arazi from comment #8)

Also the smartest thing I've heard today, thanks!
Comment 11 Gérard Simon Voyer 2021-05-07 19:08:15 UTC
Sorry for the wait, it's a beautiful day outside in Québec.

Tests made with a fresh koha-testing-docker setup on "master" branch of the GitLab repo (https://gitlab.com/koha-community/Koha)

1 - Search for "dance" in OPAC
2 - "Your search has returned 3 results" (https://snipboard.io/TalPXj.jpg) -- good
3 - add "itype: [MU]" to the OpacHiddenItems preference
4 - "Your search has returned 3 results" (https://snipboard.io/oFKXML.jpg) -- bug in action

Am I still allowed to propose my glitchy bugfix? It would be good practice for me to test how to submit a first fix, even if it's still not going to be pushed anywhere else.
Comment 12 Gérard Simon Voyer 2021-05-07 19:59:35 UTC
Created attachment 120721 [details] [review]
Bug 17787: Hidden items included in count of search results

opac-search.pl does not consider items hidden by OpacHiddenItems in the search count.
This attachment aims at fixing the count for visual harmony, since the bug lives on but
is only reported in cases where the results count is greater than the value of results_per_page.
Comment 13 Lucas Gass 2022-04-20 16:34:41 UTC
This is still a problem in master and this bugfix seems to fix the problem, I am setting it to NSO.
Comment 14 David Nind 2022-05-31 04:05:43 UTC
Created attachment 135460 [details] [review]
Bug 17787: Hidden items included in count of search results

opac-search.pl does not consider items hidden by OpacHiddenItems in the search count.
This attachment aims at fixing the count for visual harmony, since the bug lives on but
is only reported in cases where the results count is greater than the value of results_per_page.

Signed-off-by: David Nind <david@davidnind.com>
Comment 15 David Nind 2022-05-31 04:15:26 UTC
I tested (using koha-testing-docker) by:

1. Searching in the OPAC for 'doctor who' - "Your search returned 4 results", four results listed (1x book, 3 x visual resources)
2. Changed OpacHiddenItems- itype: [BK]
3. Searched again as per step 1 - shows "Your search returned 4 results", three results listed (3 x visual resources).
4. Applied patch.
5. Now number of search results matches.
Comment 16 Victor Grousset/tuxayo 2022-06-03 03:01:24 UTC
After testing many search terms to find a case that is closer to the original report (multiple pages of results), here is one and it doesn't work.

== Test plan ==
1. Use sample data
2. Change OpacHiddenItems to «itype: [BK]»
3. Search in the OPAC for 'man'
4. 29 or 28 results depending on SearchEngine being Zebra of ElasticSearch
   Actually there are a handful of results on every page.
   Which is wrong because it shouldn't say 29 but around 10 and they should be on one page
5. Apply patch and restart service
6. Redo the search
7. Number of hits should match the actual number and be on one page.

But last step doesn't happen with the current patch.

I found the reason:

comment 7:
> I worked around using [...]
> But this seems to base itself off the fact that our users will not notice biased results counts if the results span over more than a page.

comment 11:
> my glitchy bugfix

So patch isn't ready, it was just a workaround solving part of the issue.