[Bug 24458] New: OPAC search results don't use Koha::Filter::MARC::ViewPolicy
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Bug ID: 24458 Summary: OPAC search results don't use Koha::Filter::MARC::ViewPolicy Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Searching Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org With https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870, we started enforcing the bibliographic framework visibility in the OPAC. However, we're not using this for search results, which means that hidden fields could come through very visibly. -- 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=24458 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |15870 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15870 [Bug 15870] Add Filter for MARC to respect visibility settings -- 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=24458 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24403 -- 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=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |eb@efdss.org --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- *** Bug 17527 has been marked as a duplicate of this bug. *** -- 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=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|minor |normal Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |tomascohen@gmail.com -- 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=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|OPAC search results don't |Search results don't use |use |Koha::Filter::MARC::ViewPol |Koha::Filter::MARC::ViewPol |icy |icy | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major Status|ASSIGNED |Needs Signoff CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 104696 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104696&action=edit Bug 24458: Regression tests This patch introduces tests for searchResults. The idea is to set a subfield as hidden in OPAC and another on the staff interface, and see if searchResults honours this on the XSLTResultsRecord contents. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not really implemented Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> https://bugs.koha-community.org/show_bug.cgi?id=25416 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 104697 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104697&action=edit Bug 24458: Use the ViewPolicy filter on search results This patch implements the use of the ViewPolicy record processor filter inside C4::Search::searchResults. The idea is that the $record_processor is instantiated once and reused inside the loop. This leaves options for further optimizations I will do on a follow-up bug. The filter is applied to the MARC data before it is passed to the XSLT processor. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not implemented, tests fail 3. Apply this patch 4. Repeat 2 5. Feel the joy in your body from a long standing bug being solved 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> https://bugs.koha-community.org/show_bug.cgi?id=25416 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |25416 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25416 [Bug 25416] Add information about anonymous session for XSLT use -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25457 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104696|0 |1 is obsolete| | --- Comment #4 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 104707 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104707&action=edit Bug 24458: Regression tests This patch introduces tests for searchResults. The idea is to set a subfield as hidden in OPAC and another on the staff interface, and see if searchResults honours this on the XSLTResultsRecord contents. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not really implemented https://bugs.koha-community.org/show_bug.cgi?id=25416 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104697|0 |1 is obsolete| | --- Comment #5 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 104708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104708&action=edit Bug 24458: Use the ViewPolicy filter on search results This patch implements the use of the ViewPolicy record processor filter inside C4::Search::searchResults. The idea is that the $record_processor is instantiated once and reused inside the loop. This leaves options for further optimizations I will do on a follow-up bug. The filter is applied to the MARC data before it is passed to the XSLT processor. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not implemented, tests fail 3. Apply this patch 4. Repeat 2 5. Feel the joy in your body from a long standing bug being solved 6. Sign off :-D https://bugs.koha-community.org/show_bug.cgi?id=25416 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Needs Signoff |Signed Off --- Comment #6 from Victor Grousset/tuxayo <victor@tuxayo.net> ---
5. Feel the joy in your body from a long standing bug being solved
And also the joy Test Driven Development in the context of a signoff :D I'll take a look at bug 25416, because I might I got the testing order wrong ^^" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #6)
5. Feel the joy in your body from a long standing bug being solved
And also the joy Test Driven Development in the context of a signoff :D
I'll take a look at bug 25416, because I might I got the testing order wrong ^^"
The order was because I wrote the tests for the other one first. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #3)
5. Feel the joy in your body from a long standing bug being solved
<3 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104707|0 |1 is obsolete| | --- Comment #9 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 104765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104765&action=edit Bug 24458: Regression tests This patch introduces tests for searchResults. The idea is to set a subfield as hidden in OPAC and another on the staff interface, and see if searchResults honours this on the XSLTResultsRecord contents. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not really implemented https://bugs.koha-community.org/show_bug.cgi?id=25416 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <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=24458 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104708|0 |1 is obsolete| | --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 104766 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104766&action=edit Bug 24458: Use the ViewPolicy filter on search results This patch implements the use of the ViewPolicy record processor filter inside C4::Search::searchResults. The idea is that the $record_processor is instantiated once and reused inside the loop. This leaves options for further optimizations I will do on a follow-up bug. The filter is applied to the MARC data before it is passed to the XSLT processor. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Search.t => FAIL: This is not implemented, tests fail 3. Apply this patch 4. Repeat 2 5. Feel the joy in your body from a long standing bug being solved 6. Sign off :-D https://bugs.koha-community.org/show_bug.cgi?id=25416 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Nick Clemens <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=24458 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25538 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25538 [Bug 25538] www/search_utf8.t is failing randomly -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #12 from Joy Nelson <joy@bywatersolutions.com> --- missing dependencies - not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|20.05.00 |20.05.00, 19.11.06 released in| | --- Comment #13 from Joy Nelson <joy@bywatersolutions.com> --- thanks to tomas' help on 25416 i was able to push this to 19.11.x. Will be in 19.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- *** Bug 25124 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #15 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Considering to backport to 19.05.X Why is bug 25416 a dependency? Trying to backport this bug without bug 25416 is the cause a few conflicts. But they can be reliably solved(independent changes). And t/db_dependent/Search.t still passes. But that's just a quick check. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- It was because I wrote one before the other, and the tests initially depended on on setup of the mocked things. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25686 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- I'm glad to see this change go in, but it appears that it caused Bug 25686, because the visibility for the 999$c was set to hide. I don't think that it's a bug or regression, but rather we should let people know somehow that they may need to update their MARC bibliographic frameworks, since they won't be automatically updated. I suppose that the release notes would've been the place for that, but perhaps it's too late for that? I think Koha should probably automatically update the "Default Framework" on upgrades. The first time we do it could be problematic, since people's changes would disappear (although we could back them up in an "Old Default Framework"), but I figure we need to have some strategy for delivering framework updates automatically. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25457 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25457 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25457 [Bug 25457] The ViewPolicy filter should cache the MARC framework structure -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #18 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- David, What about adding a check in search_for_data_inconsistencies.pl? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #19 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Tomás Cohen Arazi from comment #16)
It was because I wrote one before the other, and the tests initially depended on on setup of the mocked things.
Ok, so I'll backport only this patchset. Since it's for a major bug, the conflicts are reliable to solve and IIUC there is no strong dependency that could cause undetected regressions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.05.00, 19.11.06 |20.05.00, 19.11.06, released in| |19.05.12 Status|Pushed to oldstable |Pushed to oldoldstable --- Comment #20 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Backported to 19.05.x branch for 19.05.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #21 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #18)
David, What about adding a check in search_for_data_inconsistencies.pl?
Yeah maybe. I wasn't familiar with search_for_data_inconsistencies.pl, so I just took a look. (It actually looks like it would benefit from Bug 25661.) I suppose /cgi-bin/koha/admin/checkmarc.pl would be the best place for a user initiated check. Maybe a Koha::Check::* or Koha::Validate::* module that could be used in both places. Alternatively, I suppose we could make ViewPolicy skip critical MARC fields... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #17)
I'm glad to see this change go in, but it appears that it caused Bug 25686, because the visibility for the 999$c was set to hide.
I don't think that it's a bug or regression, but rather we should let people know somehow that they may need to update their MARC bibliographic frameworks, since they won't be automatically updated.
I suppose that the release notes would've been the place for that, but perhaps it's too late for that?
I think Koha should probably automatically update the "Default Framework" on upgrades. The first time we do it could be problematic, since people's changes would disappear (although we could back them up in an "Old Default Framework"), but I figure we need to have some strategy for delivering framework updates automatically.
I'm surprised by this, as I recall some bug excluding 999 from this, or maybe it was a comment on the tests because now I cannot find it in bugzilla. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #23 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #22)
I'm surprised by this, as I recall some bug excluding 999 from this, or maybe it was a comment on the tests because now I cannot find it in bugzilla.
I see a comment in ./Koha/Filter/MARC/ViewPolicy.pm about how hiding the LDR causes "the MARCXML->MARC::Record->MARCXML transformation blows up"? I imagine the fix would be to have a hash containing critical fields based on the MARC schema (I don't recall if Unimarc uses 999$c as well or not), and then just consult that in _filter_field()? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25686 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25686 [Bug 25686] Title in the catalogue search result doesn't open the full (normal) record display -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |25826 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25826 [Bug 25826] Hiding biblionumber in the frameworks breaks links in result list -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 --- Comment #24 from David Cook <dcook@prosentient.com.au> --- (In reply to Tomás Cohen Arazi from comment #22)
I'm surprised by this, as I recall some bug excluding 999 from this, or maybe it was a comment on the tests because now I cannot find it in bugzilla.
I think maybe Bug 19261 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25686 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Lucy Vaux-Harvey <lucy.vaux-harvey@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucy.vaux-harvey@ptfs-europ | |e.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24458 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34847 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34847 [Bug 34847] Search.t is failing if the DB has been upgraded -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org