Bug 24458 - Search results don't use Koha::Filter::MARC::ViewPolicy
Summary: Search results don't use Koha::Filter::MARC::ViewPolicy
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
: 17527 (view as bug list)
Depends on: 25416 15870
Blocks: 25538
  Show dependency treegraph
 
Reported: 2020-01-20 06:11 UTC by David Cook
Modified: 2020-05-20 21:02 UTC (History)
7 users (show)

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


Attachments
Bug 24458: Regression tests (2.89 KB, patch)
2020-05-11 13:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24458: Use the ViewPolicy filter on search results (2.24 KB, patch)
2020-05-11 13:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 24458: Regression tests (2.95 KB, patch)
2020-05-11 16:38 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 24458: Use the ViewPolicy filter on search results (2.29 KB, patch)
2020-05-11 16:39 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 24458: Regression tests (3.01 KB, patch)
2020-05-12 12:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24458: Use the ViewPolicy filter on search results (2.35 KB, patch)
2020-05-12 12:33 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-01-20 06:11:35 UTC
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.
Comment 1 Tomás Cohen Arazi 2020-05-11 12:12:36 UTC
*** Bug 17527 has been marked as a duplicate of this bug. ***
Comment 2 Tomás Cohen Arazi 2020-05-11 13:22:15 UTC
Created attachment 104696 [details] [review]
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
Comment 3 Tomás Cohen Arazi 2020-05-11 13:22:20 UTC
Created attachment 104697 [details] [review]
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
Comment 4 Victor Grousset/tuxayo 2020-05-11 16:38:58 UTC
Created attachment 104707 [details] [review]
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>
Comment 5 Victor Grousset/tuxayo 2020-05-11 16:39:02 UTC
Created attachment 104708 [details] [review]
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>
Comment 6 Victor Grousset/tuxayo 2020-05-11 16:41:43 UTC
> 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 ^^"
Comment 7 Tomás Cohen Arazi 2020-05-11 16:44:33 UTC
(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.
Comment 8 David Cook 2020-05-11 23:27:30 UTC
(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
Comment 9 Nick Clemens 2020-05-12 12:33:44 UTC
Created attachment 104765 [details] [review]
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>
Comment 10 Nick Clemens 2020-05-12 12:33:47 UTC
Created attachment 104766 [details] [review]
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>
Comment 11 Martin Renvoize 2020-05-15 08:58:22 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Joy Nelson 2020-05-19 21:56:19 UTC
missing dependencies - not backported to 19.11.x
Comment 13 Joy Nelson 2020-05-20 21:02:30 UTC
thanks to tomas' help on 25416 i was able to push this to 19.11.x.  Will be in 19.11.06