Bug 23676 - Elasticsearch - 0 is not a valid boolean for suppress
Summary: Elasticsearch - 0 is not a valid boolean for suppress
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact:
URL:
Keywords:
: 23470 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-09-25 19:02 UTC by Nick Clemens
Modified: 2020-02-06 18:38 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03


Attachments
Bug 23676: Use 'false' for opac suppression (1.39 KB, patch)
2019-09-25 19:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23676: Use 'false' for opac suppression (1.49 KB, patch)
2019-11-19 15:50 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23676: Use 'false' for opac suppression (1.56 KB, patch)
2019-12-31 01:59 UTC, Myka Kennedy Stephens
Details | Diff | Splinter Review
Bug 23676: Use 'false' for opac suppression (1.63 KB, patch)
2020-01-10 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-09-25 19:02:26 UTC
To recreate:
1 - Enable ES
2 - Enable OpacSuppression
3 - Suppress a bib in staff client
    Edit 942n to be 1
4 - Search the opac for anything
5 - Tail the plack logs and note a deprecation warning

In a production system these add up fast and can fill the disk
Comment 1 Nick Clemens 2019-09-25 19:06:00 UTC
Created attachment 93161 [details] [review]
Bug 23676: Use 'false' for opac suppression

To test:
 1 - Enable ES
 2 - Enable OpacSuppression
 3 - Suppress a bib in staff client
     Edit 942n to be 1
 4 - Search the opac for anything
 5 - Tail the plack logs and note a deprecation warning
 6 - Apply patch
 7 - Restart all the things
 8 - Repeat search on opac
 9 - No error/warn in logs
10 - Record is correctly suppressed
Comment 2 Michal Denar 2019-10-02 19:51:11 UTC
Hello Nick,
can you rebase this patch on master?

CONFLICT (content): Merge conflict in Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 23676: Use 'false' for opac suppression


Thank you.
Comment 3 Joonas Kylmälä 2019-11-11 14:41:07 UTC
What log is where the warning goes and which version of elasticsearch do you use?
Comment 4 Séverine Queune 2019-11-19 15:18:36 UTC
(In reply to Michal Denar from comment #2)
> Hello Nick,
> can you rebase this patch on master?
> 
> CONFLICT (content): Merge conflict in
> Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
> error: Failed to merge in the changes.
> Patch failed at 0001 Bug 23676: Use 'false' for opac suppression
> 
> 
> Thank you.

Same as Michal :(
Comment 5 Nick Clemens 2019-11-19 15:50:58 UTC
Created attachment 95572 [details] [review]
Bug 23676: Use 'false' for opac suppression

To test:
 1 - Enable ES
 2 - Enable OpacSuppression
 3 - Suppress a bib in staff client
     Edit 942n to be 1
 4 - Search the opac for anything
 5 - Tail the plack logs and note a deprecation warning
 6 - Apply patch
 7 - Restart all the things
 8 - Repeat search on opac
 9 - No error/warn in logs
10 - Record is correctly suppressed
Comment 6 Nick Clemens 2019-11-19 15:51:57 UTC
(In reply to Joonas Kylmälä from comment #3)
> What log is where the warning goes and which version of elasticsearch do you
> use?

plack-error.log

This is in ES 5.x, I think it breaks in 6
Comment 7 Myka Kennedy Stephens 2019-12-31 01:59:29 UTC
Created attachment 96695 [details] [review]
Bug 23676: Use 'false' for opac suppression

To test:
 1 - Enable ES
 2 - Enable OpacSuppression
 3 - Suppress a bib in staff client
     Edit 942n to be 1
 4 - Search the opac for anything
 5 - Tail the plack logs and note a deprecation warning
 6 - Apply patch
 7 - Restart all the things
 8 - Repeat search on opac
 9 - No error/warn in logs
10 - Record is correctly suppressed

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Comment 8 Myka Kennedy Stephens 2019-12-31 02:04:24 UTC
Patch seemed to work as expected. Test results:

 1 - Enabled ES
 2 - Enabled OpacSuppression
 3 - Suppressed a bib in staff client
     Edit 942n to be 1
 4 - Searched the opac for keywords related to suppressed record
 5 - Tail the plack logs and note a deprecation warning
    >> entered command koha-plack-err and watched for warnings; noted deprecation warnings when searching
 6 - Applied patch
 7 - Restart all the things
    >> restart_all, updatedatabase, reindex ES
 8 - Repeated search on opac
 9 - No error/warn in logs
    >> watched koha-plack-err again and no new deprecation warnings appeared after multiple searches
10 - Record is correctly suppressed
    >> repeated search in staff client and verified opac suppression
Comment 9 Jonathan Druart 2020-01-10 13:46:33 UTC
I am using 6.8.3 and OpacSuppression breaks the OPAC search, nothing in the log. This patch fixes the problem!

Also it correctly hides the record.
Comment 10 Jonathan Druart 2020-01-10 13:47:04 UTC
Created attachment 97220 [details] [review]
Bug 23676: Use 'false' for opac suppression

To test:
 1 - Enable ES
 2 - Enable OpacSuppression
 3 - Suppress a bib in staff client
     Edit 942n to be 1
 4 - Search the opac for anything
 5 - Tail the plack logs and note a deprecation warning
 6 - Apply patch
 7 - Restart all the things
 8 - Repeat search on opac
 9 - No error/warn in logs
10 - Record is correctly suppressed

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2020-01-10 13:48:21 UTC
*** Bug 23470 has been marked as a duplicate of this bug. ***
Comment 12 Martin Renvoize 2020-01-10 16:23:32 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 13 Joy Nelson 2020-01-31 00:20:30 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 14 Lucas Gass 2020-02-06 17:14:24 UTC
backported to 19.05.x for 19.05.08
Comment 15 Lucas Gass 2020-02-06 18:38:38 UTC
this patchset has been reverted from 19.05.x as it has been problematic.