[Bug 35052] New: OpacHiddenItemsHidesRecord system preference should be considered on index time instead
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 Bug ID: 35052 Summary: OpacHiddenItemsHidesRecord system preference should be considered on index time instead Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Searching Assignee: koha-bugs@lists.koha-community.org Reporter: pedro.amorim@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org CC: alexander.blanchard@ptfs-europe.com, dcook@prosentient.com.au, jacob.omara@ptfs-europe.com, jonathan.field@ptfs-europe.com, martin.renvoize@ptfs-europe.com, matt.blenkinsop@ptfs-europe.com Currently, results are being skipped after the search has been processed in C4/Search.pm. This may cause several unintended scenarios: 1) The first page has all the results skipped, even thought there are more results in 2nd page and so on, this will will result in a "No search results" page 2) The total search results number still shows 80 (for example), even if it has skipped 70 and is only showing 10. To reproduce we need to use both OpacHiddenItems and OpacHiddenItemsHidesRecord Use OpacHiddenItems and put some rules that hide some items Use OpacHiddenItemsHidesRecord = 'hide' -- 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=35052 Chris Rowlands <chris.rowlands6@nhs.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris.rowlands6@nhs.net -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I actually see two ways to achieve this.. a) we add a field to the index to denote hidden by these options and fill it during index runs. or b) we adapt the queries at runtime with each search to take into consideration the preferences -- 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=35052 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- for b) I think we need to begin indexing the items individually, right now we take the values from each, but the connections are broken. i.e. homebranch index gets the list of homebranches on the item holdingbranch the same We cannot search for records with items with matching homebranch and holdingbranch, only records that have some items in a homebranch and some items in a holdingbranch -- 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=35052 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24403 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- I recall Frido and I talking about this on a separate bug but now I can't remember which one it was... One problem with indexing whether or not to hide based on OpacHiddenItems and OpacHiddenItemsHidesRecord is that you'd need to do a full reindex for any change to OpacHiddenItems or OpacHiddenItemsHidesRecord. (Of course, if we note that caveat in the syspref descriptions, then it's not necessarily a huge problem.) Unless we re-use the index used by OpacSuppression, then we'll need to adapt the search queries to take into account a new OpacHiddenItems index as well. -- As Nick says, I don't think just updating the search query is enough, since I don't think there's any way to know if all the items in a record should be hidden in Zebra. In bug 19704, I'm able to generate a SQL query using OpacHiddenItems, but it's able to deal with individual item records. That said, I don't think we can index items separately in Zebra and there wouldn't be any way to join the bibs and items together anyway. So I think that really just leaves the indexing option? -- 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=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |42835 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42835 [Bug 42835] ElasticSearch should have an items index -- 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=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|42835 |43151 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42835 [Bug 42835] ElasticSearch should have an items index https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43151 [Bug 43151] ES should apply search filters on each item discretely -- 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=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|OpacHiddenItemsHidesRecord |OpacHiddenItemsHidesRecord |system preference should be |system preference should be |considered on index time |considered in search |instead |indexes instead of post | |filtering results -- 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=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43355 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43355 [Bug 43355] Add a /search API endpoint for biblios -- 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=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk -- 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=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Sponsorship status|--- |Unsponsored Status|NEW |Needs Signoff Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #4 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203925 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203925&action=edit Bug 35052: Extract shared items-index composite aggregation helper -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203926&action=edit Bug 35052: Add _get_item_counts_by_biblio -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203927&action=edit Bug 35052: Add _resolve_opac_hidden_items_rules -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203928&action=edit Bug 35052: Attach resolved OpacHiddenItems rules to the built query Wire _resolve_opac_hidden_items_rules into build_query_compat by: 1. Calling the method to resolve the rules (returns undef if not applicable) 2. Attaching the result to $query->{_opac_hidden_items_rules} when non-undef This allows Task 5's _apply_opac_hidden_items_filter to read and delete these rules from the query object when filtering search results. Adds 2 new assertions testing that: - OPAC searches with is_opac=1 carry the resolved hidden-items rules - Staff searches with is_opac=0 do not carry the rules -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203929&action=edit Bug 35052: Add _apply_opac_hidden_items_filter -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203930 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203930&action=edit Bug 35052: Apply the opac-hidden-items filter in search_compat -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203931 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203931&action=edit Bug 35052: Pass the searching patron into build_query_compat for OpacHiddenItemsExceptions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203932 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203932&action=edit Bug 35052: Add end-to-end test for the query-time OpacHiddenItems filter -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203933 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203933&action=edit Bug 35052: Fix test warning in OpacHiddenItems end-to-end test Changed build_query_compat call to use empty arrays [] instead of [undef] for operators and indexes parameters, which prevents uninitialized value warnings. This makes the test output pristine while maintaining the same semantics (both represent 'no operators/indexes specified'). Removed the Test::Warn wrapper since the warning is no longer generated, and reverted the subtest assertion count from 4 back to 3. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203934 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203934&action=edit Bug 35052: Add defensive null check for patron category Prevent potential null pointer dereference if $patron->category unexpectedly returns undef. While this shouldn't happen in normal Koha operation (category is a required foreign key), add explicit check to close the defensive-coding gap. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 --- Comment #14 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 203935 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203935&action=edit Bug 35052: (QA follow-up) Fix perlcritic and POD coverage findings - QueryBuilder.pm: replace explicit "return undef" with bare "return" in _resolve_opac_hidden_items_rules (perlcritic severity 5, PBP p.199) - Search.pm: add missing POD for _composite_item_aggregation and _get_item_counts_by_biblio; correct the POD block that had drifted onto _composite_item_aggregation when it actually documented _get_matching_biblionumbers' public contract (Task 1's refactor moved the sub the POD was written for without moving the POD itself) Test plan: 1) ktd --name "${KTD_INSTANCE:-kohadev}" --shell --run '/kohadevbox/qa-test-tools/koha-qa.pl -v 2 --more-tests' 2) Only remaining flagged item is a pre-existing critic finding on MockESSearchClient's package-vs-filename mismatch in t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t, inherited unchanged from Bug 43151 - not introduced or touched by this bug 3) ktd --name "${KTD_INSTANCE:-kohadev}" --shell --run 'prove t/Koha/SearchEngine/Elasticsearch/QueryBuilder.t t/db_dependent/Koha/SearchEngine/Elasticsearch/QueryBuilder.t t/db_dependent/Koha/SearchEngine/Elasticsearch/Search.t' 4) All tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35052 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |26.11 Initiative type|--- |Feature -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org