[Bug 31061] New: OPACSuppress does not work properly with Elasticsearch
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Bug ID: 31061 Summary: OPACSuppress does not work properly with Elasticsearch Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Searching - Elasticsearch Assignee: koha-bugs@lists.koha-community.org Reporter: fridolin.somers@biblibre.com OPACSuppress is system preference to hide records in OPAC search by adding a limit in search query. With Zebra, this limit is : not Suppress=1 https://git.koha-community.org/Koha-community/Koha/src/commit/244b847a08bf61... With Elasticsearch this limit is currently quite different : suppress:false https://git.koha-community.org/Koha-community/Koha/src/commit/244b847a08bf61... Main problem is that in default framework we define Suppress search field on 942$n with authorized values YES_NO. This generates records with 1 for YES and 0 for NO. But a boolean for Elasticsearch is not a perl boolean. So only missing subfield means false : https://www.elastic.co/guide/en/elasticsearch/reference/6.8/boolean.html Records with 0 are hidden ! Another side-effect : We whould like to have several MARC fields with 'suppress' search field. To distinguish several causes of hidden at OPAC. Any of those fields having 1 must hide at OPAC. So syntax 'NOT(suppress:1)' is more correct. I propose we change to have the same behavior as Zebra. Change 'suppress' search field to not remove boolean type. Change QueryBuilder to use 'NOT(suppress:1)'. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |23676 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23676 [Bug 23676] Elasticsearch - 0 is not a valid boolean for suppress -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |fridolin.somers@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 --- Comment #1 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 136695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136695&action=edit Bug 31061: Fix test suite -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 --- Comment #2 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 136696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136696&action=edit Bug 31061: Change type of suppress in Elasticsearch mappings -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 --- Comment #3 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 136697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136697&action=edit Bug 31061: OPACSuppress does not work properly with Elasticsearch OPACSuppress is system preference to hide records in OPAC search by adding a limit in search query. With Zebra, this limit is : not Suppress=1 https://git.koha-community.org/Koha-community/Koha/src/commit/244b847a08bf61... With Elasticsearch this limit is currently quite different : suppress:false https://git.koha-community.org/Koha-community/Koha/src/commit/244b847a08bf61... Main problem is that in default framework we define Suppress search field on 942$n with authorized values YES_NO. This generates records with 1 for YES and 0 for NO. But a boolean for Elasticsearch is not a perl boolean. So only missing subfield means false : https://www.elastic.co/guide/en/elasticsearch/reference/6.8/boolean.html Records with 0 are hidden ! Another side-effect : We whould like to have several MARC fields with 'suppress' search field. To distinguish several causes of hidden at OPAC. Any of those fields having 1 must hide at OPAC. So syntax 'NOT(suppress:1)' is more correct. I propose we change to have the same behavior as Zebra. Change 'suppress' search field to not remove boolean type. Change QueryBuilder to use 'NOT(suppress:1)'. Test plan : 1) Use search engine Elasticsearch 2) Disable system preference OpacSuppression 3) Search in OPAC for all records : '*:*' => Note how many results 4) Enable system preference OpacSuppression 5) Edit a biblio record with 942$n set to YES 6) Search in OPAC for all records : '*:*' => Note how many results, you should have 1 less than 3) 7) Edit a biblio record with 942$n set to NO 8) Search in OPAC for all records : '*:*' => Note how many results, you should have same number as 3) 9) Edit a biblio record to remove 942$n 10) Search in OPAC for all records : '*:*' => Note how many results, you should have same number as 3) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #4 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- I must not be completely awake, but I cannot reproduce the original "problematic" behaviour. Without applying the patch, using ES, the number of records fluctuates according to 942$n being Yes (1) or No (0) and OpacSuppression being on or off. There are 3 records with 1 in 942$n With OpacSuppression = 405 Without OpacSuppression = 408 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 --- Comment #5 from Fridolin Somers <fridolin.somers@biblibre.com> --- Ah indeed seems to be a code that convert 'perl boolean' value into true/false : https://git.koha-community.org/Koha-community/Koha/src/commit/6c315c164dbcf3... I need to test again -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 --- Comment #6 from Fridolin Somers <fridolin.somers@biblibre.com> --- Ahhhhh now that I've retested with KTD, indeed it works. Maybe my search was too faster after unhiding record with 942$n=0.
But a boolean for Elasticsearch is not a perl boolean. Ouf its is thanks to a trick in the code.
I will change patches in order to only change to : NOT(suppress:true) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136695|0 |1 is obsolete| | Attachment #136696|0 |1 is obsolete| | Attachment #136697|0 |1 is obsolete| | --- Comment #7 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 136799 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136799&action=edit Bug 31061: OPACSuppress behavior with Elasticsearch OPACSuppress is system preference to hide records in OPAC search by adding a limit in search query. With Zebra, this limit is : not Suppress=1 https://git.koha-community.org/Koha-community/Koha/src/commit/244b847a08bf61... With Elasticsearch this limit is currently quite different : suppress:false https://git.koha-community.org/Koha-community/Koha/src/commit/244b847a08bf61... We whould like to have several MARC fields with 'suppress' search field. To distinguish several causes of hidden at OPAC. Any of those fields having 1 must hide at OPAC. Syntax 'NOT(suppress:true)' does that, like with Zebra. Test plan : 1) Use search engine Elasticsearch 2) Disable system preference OpacSuppression 3) Search in OPAC for all records : '*:*' => Note how many results 4) Enable system preference OpacSuppression 5) Edit a biblio record with 942$n set to YES 6) Search in OPAC for all records : '*:*' => Note how many results, you should have 1 less than 3) 7) Edit a biblio record with 942$n set to NO 8) Search in OPAC for all records : '*:*' => Note how many results, you should have same number as 3) 9) Edit a biblio record to remove 942$n 10) Search in OPAC for all records : '*:*' => Note how many results, you should have same number as 3) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|OPACSuppress does not work |OPACSuppress behavior with |properly with Elasticsearch |Elasticsearch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #8 from David Nind <david@davidnind.com> --- I had a go at testing using KTD and the sample data. I couldn't replicate the issue before the patch was applied, and there were no differences in the expected results with 943$n set to Yes, No and removed. I've set out what I did below. Before the patch was applied ============================ Using Elasticsearch, index rebuilt (koha-elasticsearch --rebuild -d -b -a kohadev) OpacSuppression = Don't hide: 435 records OpacSuppression = Hide: 433 records Records that are suppressed already in the sample data = 2: biblionumber 232 and 234 (used report to find https://wiki.koha-community.org/wiki/SQL_Reports_Library#Bibs_Suppressed_in_...) Recorded edited with OpacSuppression = Hide: 126 (Intermediate Perl) - 942$n = Yes: records = 432 (as expected) (435 - 2 records already hidden - 1 new record) - 942$n = No: records = 433 (as expected) - 942$n = removed: records = 433 (as expected) Tests pass: prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t After the patch is applied ========================== Using Elasticsearch, index rebuilt (koha-elasticsearch --rebuild -d -b -a kohadev) OpacSuppression = Don't hide: 435 records OpacSuppression = Hide: 433 records Records that are suppressed already in the sample data = 2: biblionumber 232 and 234 (used report to find https://wiki.koha-community.org/wiki/SQL_Reports_Library#Bibs_Suppressed_in_...) Recorded edited with OpacSuppression = Hide: 126 (Intermediate Perl) - 942$n = Yes: records = 432 (as expected) (435 - 2 records already hidden - 1 new record) - 942$n = No: records = 433 (as expected) - 942$n = removed: records = 433 (as expected) Tests pass: prove t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31061 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |RESOLVED Resolution|--- |INVALID --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> ---
We whould like to have several MARC fields with 'suppress' search field. We abandonned this idea Lets keep the actual code ;)
-- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org