Bug 15198 - Make OpacSuppression work even if there are no records suppressed
Summary: Make OpacSuppression work even if there are no records suppressed
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-17 00:05 UTC by David Cook
Modified: 2017-06-14 22:10 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15198 - Make OpacSuppression work even if there are no records suppressed (2.53 KB, patch)
2015-11-17 03:22 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 15198: Make OpacSuppression work even if there are no records suppressed (2.75 KB, patch)
2015-11-17 05:19 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 15198 - Change wording of OpacSuppression system preference (1.27 KB, patch)
2015-11-17 06:18 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 15198: Change wording of OpacSuppression system preference (1.37 KB, patch)
2015-11-17 14:26 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 15198: Make OpacSuppression work even if there are no records suppressed (2.80 KB, patch)
2015-11-18 22:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 15198: Change wording of OpacSuppression system preference (1.42 KB, patch)
2015-11-18 22:42 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2015-11-17 00:05:04 UTC
At the moment, if you turn on OpacSuppression and try to search for records without having at least one record with 942$n = 1 indexed, you'll get zero results back.

However, the special Zebra attribute 14 allows you to get around this problem. Attribute 14: 

"Specifies whether un-indexed fields should be ignored. A zero value (default) throws a diagnostic when an un-indexed field is specified. A non-zero value makes it return 0 hits." (http://www.indexdata.com/zebra/doc/querymodel-zebra.html)

Now the description is a bit vague... you can't try something like "@attr 14=1 @attr 1=9999 1" to work. You'll still get a diagnostic error. However, you can try something like "@attr 14=1 @attr 1=9011 1" to work as 9011 is a valid use attribute in bib1.att.

Anyway, by adding 14=1 to the Suppress qualifier in ccl.properties, we can make it so that OpacSuppression queries still work even when there are no suppressed records in Zebra.

Cool, n'est-ce pas? 

No... it's not cool, but it's handy.
Comment 1 David Cook 2015-11-17 00:33:55 UTC
Unfortunately, I can't currently install the dependencies for master, namely Crypt::GCrypt which despite being marked as not required is required to even visit mainpage.pl, so I'll just be posting something here for someone else to do:

--

ccl.properties:

Replace:

Suppress 1=9011

With:

Suppress 1=9011 14=1

--

opac-search.pl:

Replace:

$query = '@not '.$query.' @attr 1=9011 1';
and
$query = '@not '.$query.' @attr 1=9011 1'; #PQF syntax

With:

$query = '@not '.$query.' @attr 14=1 @attr 1=9011 1';
and
$query = '@not '.$query.' @attr 14=1 @attr 1=9011 1'; #PQF syntax

--

You'll need to test the opac-search.pl stuff, but I've already tested the ccl.properties change and it works a treat.
Comment 2 David Cook 2015-11-17 03:22:25 UTC Comment hidden (obsolete)
Comment 3 Héctor Eduardo Castro Avalos 2015-11-17 05:19:53 UTC Comment hidden (obsolete)
Comment 4 Héctor Eduardo Castro Avalos 2015-11-17 05:28:32 UTC
I think, IMHO, you need to send another patch rewording the syspref  OpacSuppression  "Hide/Don't hide items marked as suppressed from OPAC search results. Note that you must have the Suppress index set up in Zebra and at least one suppressed biblio record, or your searches will be broken."

I also add Documentation Manager
Comment 5 David Cook 2015-11-17 06:14:31 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #4)
> I think, IMHO, you need to send another patch rewording the syspref 
> OpacSuppression  "Hide/Don't hide items marked as suppressed from OPAC
> search results. Note that you must have the Suppress index set up in Zebra
> and at least one suppressed biblio record, or your searches will be broken."
> 
> I also add Documentation Manager

Yes, excellent point! I was thinking of that earlier today, but I totally forgot by the time I was writing the patch.
Comment 6 David Cook 2015-11-17 06:17:15 UTC
I suppose some of the wording might be accurate... "att 9011    Suppress" does need to exist in bib1.att... but that's fairly obvious and it's a default.
Comment 7 David Cook 2015-11-17 06:17:37 UTC
I suppose you also have to have Suppress in ccl.properties... but again that's a default.
Comment 8 David Cook 2015-11-17 06:18:55 UTC Comment hidden (obsolete)
Comment 9 David Cook 2015-11-17 06:19:13 UTC
Marking this back to "Needs Signoff" for the second patch...
Comment 10 Héctor Eduardo Castro Avalos 2015-11-17 14:26:55 UTC Comment hidden (obsolete)
Comment 11 Fridolin SOMERS 2015-11-18 16:09:36 UTC
Very nice catch.
I had seen this attribute type and had the same idea once (but then I forgot).
Comment 12 Katrin Fischer 2015-11-18 22:03:03 UTC
I am sorry, but this isn't working for me.
I checked that no 942$n != "" existed in my database.
I did a full reindex (-b -v -r).
I checked that "de" returned results on master, then turned on OpacSuppression.
As expected, the search did not return any results after.
Applied patch.
Search is still not returning any results.

What could be wrong? something I can check? 
Running my git installation in Ubuntu 14.04 LTS.
Comment 13 Katrin Fischer 2015-11-18 22:05:35 UTC
... ok, I know what happened... ccl.properties... gah.
Comment 14 David Cook 2015-11-18 22:36:45 UTC
(In reply to Katrin Fischer from comment #13)
> ... ok, I know what happened... ccl.properties... gah.

It'll get you every time. Testing Zebra changes are a bit of a pain...
Comment 15 Katrin Fischer 2015-11-18 22:42:03 UTC
Created attachment 44971 [details] [review]
[PASSED QA] Bug 15198: Make OpacSuppression work even if there are no records suppressed

This patch adds the Zebra special attribute 14 to ccl.properties and
opac-search.pl, so that we can turn on OpacSuppression and still return
results even if there are no records in Zebra for the Suppress index.

_TEST PLAN_

Before applying:

1) Make sure that you have no suppressed records indexed in Zebra
2) Turn on OpacSuppression system preference

3) Search using a keyword which should bring up records
4) Note that no records are returned in the results
5) Change UseQueryParser system preference to "Try"
6) Repeat steps 3-4

Apply the patch.

After applying:

7) Repeat step 3 (ie search using a keyword which should bring up records)
8) Confirm that records are appearing in the results!
9) Change UseQueryParser system preference to "Do not try"
10) Repeat step 3
11) Confirm that records are appearing in the results!

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised. No more, won't need to have at least one record with the
value "1" in the field mapped with this index. All records in OPAC returned.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Katrin Fischer 2015-11-18 22:42:16 UTC
Created attachment 44972 [details] [review]
[PASSED QA] Bug 15198: Change wording of OpacSuppression system preference

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Paragraph rewording successfully

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Tomás Cohen Arazi 2015-11-23 14:33:13 UTC
Patches pushed to master.

Thanks David!
Comment 18 Liz Rea 2015-12-11 01:50:48 UTC
Does this also require an update to the system preference text, now that we don't require there to be one record marked suppressed?
Comment 19 Liz Rea 2015-12-11 01:51:08 UTC
oh my gosh. Reading failure.

ignore me.
Comment 20 Liz Rea 2016-01-06 21:22:19 UTC
Pushed to 3.18.13, and released.