Per a discussion on and off the mailing list the opachiddenitems system preference worked in 3.4.1 (and 3.4) but does not appear to be working in master. I have not been able to test in both versions personally, so this is based on tests by myself and two others in different versions of Koha. I'm kind of hoping that it's a template issue that came about by fixing one of the TT bugs - but in case it's not I'm putting this in the admin category.
Created attachment 5312 [details] [review] patch YAML does not like files (strings) that are not ended with \n. I've added a gratuitous \n to the syspref before parsing (additional \n's are not harmful) There is an entry in the error log file for failed parsing.
Okay, we're almost there. The holdings are hidden on the detail display, but they show up as available in the search results list. See my coming attachments.
Created attachment 5314 [details] detail display is hidden My preference has: itype: [BK] homebranch: [NIC] And it's working on this image.
Created attachment 5315 [details] not hidden on search results My preference has: itype: [BK] homebranch: [NIC] and it's not working in this image.
I have no problem placing the filter on the search screen as well, but: * it was not there to start with * maybe there's a reason to be able to see the records even without being able to reach the items Nicole, can you please confirm that opachiddenitems should be applied on the search screen as well
It seems that we're all in agreement that it should be hidden all over if it's marked hidden: [12:01] <nengard> bug 6488 can anyone think of a reason that the hidden items should show on the search results and not the bib detail? [12:01] <huginn> Bug http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6488 normal, PATCH-Sent, ---, srdjan, ASSIGNED , opachiddenitems not working in master [12:01] <nengard> i don't want to make a decision alone [12:01] <nengard> i think if it's hidden it should be hidden [12:01] <wizzyrea> ^^ [12:01] <wizzyrea> what's the point of hiding it if it's not hidden [12:01] <cait> wizzyrea: here now, but have to be careful to not burn my dinner [12:01] <oleonard> Agreed. [12:01] <wizzyrea> ooo no, dinner is important [12:02] <slef> I think it should be hidden on all OPAC searches, not on intranet [12:02] <wizzyrea> I can ask you after you eat :) [12:02] <sekjal> yes, if it's hidden in the OPAC, that should be on search results, details, Lists, etc. [12:02] <sekjal> consistency [12:02] <wizzyrea> make it so, nengard ;) [12:02] <cait> nengard: I agree - should be hidden everywhere or it will only be confusing :)
Created attachment 5351 [details] [review] patch A note: result rows are removed from the page leaving holes, rather than all results being renumerated and repaged. This can only be an issue for strong filters, eg itype = ['BK']. Repaging resulset is fairly complex and goes well beyond this bug.
Also, since I've changed searchResults() which is used on both interfaces, please check the intranet app as well
Created attachment 5477 [details] [review] bug_6488: Changed searchResults() interface Take in account opachiddenitems when searching in opac Added trailing \n when parsing OpacHiddenItems to make YAML happy I sign off on this with the following caveat: results numbering will have to be revisited (or removed). I would also like one other sign off. I didn't see anything wrong, but I feel the need to request one more set of eyes. Signed-off-by: Liz Rea <lrea@nekls.org>
I'm thinking this still isn't working quite right - but I could be wrong: http://screencast.com/t/NN73y6yErzie Is this what we expect to see?
Is it possible that cd is marked as lost?
Nope ... doesn't look that way.
Those two screens are going off different things, (MARC fields vs database). I cannot get it to misbihave in that manner in my environment. Is it possible to get a database dump, please? I'm very keen to sort this out, I suspect some other problems there. Also Nicole, in that screencast, you said that you wanted to hide books from a branch. The way it works now is that it will hide all items that are books as well as all items that are homed in that branch. Can you please comment.
Srdjan, Because my BK item in that video was currently at the NIC branch but not normally owned by that branch it should show in the holdings in the OPAC. The second item on that record was CD not a BK but was also being hidden for some reason. Basically, neither of those two items should have been hidden because they didn't meet the criteria - unless the criteria is being ORed instead of ANDed. Does that make sense? Nicole
Created attachment 5767 [details] harry potter marc record this is the marc record i was using in the video - or did you need my entire database dumped?
Entire database please, some config option may be important.
Yes, criteria for hiding are ORed, not ANDed. If you think it is a bug, we need to go differently about it.
Okay if it's ORed then we need to ask the community what they want/expect. I would think that it should be ANDed - but that's just me. Community we need feedback. Nicole
I'm not sure how to give you a full DB dump since it's pretty darn large and won't attach here.
I think that we want it to be "OR" because in our case we have OpacHiddenItems set to itype: [07, 10] location: [STAFF, ISO] meaning that if itype ='07' OR itype='10' OR location='STAFF' OR location='ISO' then we want it hidden
I have tested again with the one patch on here - are there supposed to be two? Anyway, there are all kinds of things going wrong and I doubt it's a data issue. Here's a new video: http://screencast.com/t/06N5phPIarm
Created attachment 6310 [details] [review] patch * I have removed numbering from the results * On the results page, numbers are coming from the record XML, so they are not affected with our post processing. In order to change that we would need to either a) change XML on the fly, or b) introduce same Hide items logic to XSLT I am mentioning b) just for the sake of completeness, I would not even contemplate that. * I tried details screen with items of different types scenario, and it came right. That is after reindexing zebra. Can you please retest that, and if it still does not come up right then we definitely need either some screenshots of item details, or preferably the database dump.
Created attachment 6687 [details] [review] patch
Please retest. Again, if changing item records zebra needs reindexing
Created attachment 6783 [details] [review] [SIGNED-OFF] bug_6488: Take in account opachiddenitems when searching in opac Changed searchResults() interface Added trailing \n when parsing OpacHiddenItems to make YAML happy XSLTParse4Display() and buildKohaItemsNamespace() take hidden items as input param Removed numbering from the search results, looks wrong with hidden items Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com> Test Plan: * Edit OpacHiddenItems to hide one item type * search opac confirm items are hidden * Edit OpacHiddenItems to hide a different item type * search opac to confirm items are hidden * Edit OpacHiddenItems add a branch to the item type * search opac to confirm that items of the type or the branch are hidden I can find nothing wrong with the functionliaty of this bug and so I submit my signoff.
QA comment: 1 this patch adds a lot of reindentation, he's quite hard to read. But those indentations are needed, so it's OK 2 prove t/00-perlcritic.t is OK 3 prove xt/author/valid-templates.t is OK 4 the patch works if XSLT is ON. With XSLT OFF there is no difference before and after applying the patch. 5 about the removed numbering, which one is it. On my setup & test I still see a numbering after each item, for example: Availability: Copies available for loan: Médiathèque Cannet des Maures[800] (43). Copies available for reference: Médiathèque Cannet des Maures[800] (1). Checked out (2). Marking failed QA until I've some feedback for points 4 and 5
4. I tried both XSLT and "normal", and got the same result. Can we ask someone else to check this? 5. The number that was removed is one that was next to the Save tickbox, the ordinal number of the result in the result set. It was removed because it was awkward to show eg 6 results numbered 3,5,8,13,16,19
I switch back to "signed-off". I'll try to send a screenshot. Could it be a MARC21 / UNIMARC difference ? it's highly possible !
Patch no longer applies to master. Please rebase (looks to be pretty straightforward, just the one change to cataloguing/addbooks.pl)
Created attachment 7284 [details] [review] patch
Created attachment 7386 [details] [review] [SIGNED-OFF] bug_6488: Take in account opachiddenitems when searching in opac Changed searchResults() interface Added trailing \n when parsing OpacHiddenItems to make YAML happy XSLTParse4Display() and buildKohaItemsNamespace() take hidden items as input param Removed numbering from the search results, looks wrong with hidden items Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Testing this patch again, to see if my previous question: " 4 the patch works if XSLT is ON. With XSLT OFF there is no difference before and after applying the patch." still applies. It still applies, but I think/feel it's related to my test database. I think it's not related to UNIMARC, but to the fact some of my items don't have any callnumber at all. In this case, the counter is not exact, but I don't think it's related to this patch. QAind the code now
QA comment: * changes the signature of searchResults by adding a new parameter at the end and changing the type of variable for the 7th ($ instead of @) All instances of the use of this sub have been updated * in addbooks.pl, searchResults is called without opachiddenitems, that's OK, as it's the last argument. i've updated coding guidelines (PERL8 rule) passed QA
Warning like below apparently come from the latest commit on Search.pm: [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 2012] opac-search.pl: Unable to parse OpacHiddenItems syspref : YAML Error: Stream does not end with newline character, referer: http://libdevelop.rijksmuseum.nl:8008/ [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 2012] opac-search.pl: Code: YAML_PARSE_ERR_NO_FINAL_NEWLINE, referer: http://libdevelop.rijksmuseum.nl:8008/ [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 2012] opac-search.pl: Line: 0, referer: http://libdevelop.rijksmuseum.nl:8008/ [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 2012] opac-search.pl: Document: 0, referer: http://libdevelop.rijksmuseum.nl:8008/ [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 2012] opac-search.pl: at /usr/local/lib/perl5/site_perl/5.10.0/YAML.pm line 36, referer: http://libdevelop.rijksmuseum.nl:8008/ [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 2012] opac-search.pl: Unable to parse OpacHiddenItems syspref : YAML Error: Stream does not end with newline character, referer: http://libdevelop.rijksmuseum.nl:8008/
(In reply to comment #34) > Warning like below apparently come from the latest commit on Search.pm: > > [Thu Feb 02 13:33:42 2012] [error] [client 192.87.126.61] [Thu Feb 2 13:33:42 > 2012] opac-search.pl: Unable to parse OpacHiddenItems syspref : YAML Error: OK, I get the error, but only when the syspref is empty. That's strange, because in the code there is: + $yaml = "$yaml\n"; # YAML is anal on ending \n. Surplus does not hurt so yaml should never be empty... Adding 2 \n solve the problem. follow-up with a 2nd \n pushed, thanks to confirm the log is silenced now.
Yep, works fine now! Thx.
it seems that adding this has broken viewing of bibs with no items in Master? Also, a followup will be necessary that removes the documentation from the filesystem (or duplicates it in a more accessible place) and putting it somewhere more accessible to users. Asking people to consult a file on the filesystem will confuse librarians.
Additionally, search.pm reports that there are x results, and paginates them, but doesn't show them. It's a decent sized display weirdness.
(In reply to comment #37) > it seems that adding this has broken viewing of bibs with no items in Master? Could you give more details about your configuration, because I don't see this kind of problem on mine
MARC21 db nothing in the syspref or with itype: [BK] - no difference independantbranches ON and OFF (same result either way) Many bibs with no items - none can be found. Default framework on all. Any other relevant preferences you want me to check?
XSLT on or off - no change.
Its these lines here + next if $is_opac && $hideatopac_count >= $items_count; + next if $hidelostitems && $itemlost_count >= $items_count; + That are the problem. Means that if its opac, and the counts are both 0 (ie record with no items) its skipped. At least I think that is what the problem is, it is saturday morning and I havent had coffee yet so someone else looking would be good
(In reply to comment #42) > Its these lines here > > + next if $is_opac && $hideatopac_count >= $items_count; > + next if $hidelostitems && $itemlost_count >= $items_count; > + > > That are the problem. Means that if its opac, and the counts are both 0 (ie > record with no items) its skipped. > > At least I think that is what the problem is, it is saturday morning and I > havent had coffee yet so someone else looking would be good I can confirm that those two lines are the problem. What I'm not sure about is why there isn't an: if ($items_count > 0) { ... } around it.
confirm that adding the check for >0 items fixes the issue. Also discovered that the patch was nuking serials as well, as they don't have items.
Created attachment 7745 [details] [review] Bug 6488 - followup - fixes issue with bibs with no items and serials being suppressed To test: * create a bib with no items * update your index serial records and your new bib with no items should be displayed. * add something to the OPACHiddenItems syspref (I like itype: [BK] from the test data) *** test both ways, with something in there and with the syspref empty. * add an item to your new bib that would be suppressed * update your index * search for the bib The item should not show * change the item into a state where it would no longer be suppressed * update your index * search for the bib The item should show in the opac * just for fun, delete your item * update your index * Search for the bib - it should still display. I tested on MARC21 - please test UNIMARC as well. http://bugs.koha-community.org/show_bug.cgi?id=7386
Created attachment 7748 [details] [review] Bug 6488 - followup - fixes issue with bibs with no items and serials being suppressed To test: * create a bib with no items * update your index serial records and your new bib with no items should be displayed. * add something to the OPACHiddenItems syspref (I like itype: [BK] from the test data) *** test both ways, with something in there and with the syspref empty. * add an item to your new bib that would be suppressed * update your index * search for the bib The item should not show * change the item into a state where it would no longer be suppressed * update your index * search for the bib The item should show in the opac * just for fun, delete your item * update your index * Search for the bib - it should still display. I tested on MARC21 - please test UNIMARC as well. Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Note that serial and ebook records will not show up in the OPAC without the follow up. Raising importance of bug to reflect seriousness of issue.
QA Comment: Small fix for serious problem. Marked as Passed QA.
follow-up pushed to master
Created attachment 7957 [details] Picture of staff results Follows..
Comment on attachment 7957 [details] Picture of staff results Wrong report. *Sorry*
Also mailed the dev list: I personally do not favor that adding an optional feature of hiding items removes search results numbering in general. The numbering is very handy. Now everybody loses that while not even using the hidden items feature! I would suggest that possibly Srdjan (as the author) adds the search results numbering back and only removes it when opachiddenitems is active. This could be provisional while resolving the numbering issue in full.
I agree that making it dependent on opachiddenitems would be good. I also have the impression that people use the numbering for orientation.
Right, so can I assume that we all agree that I put the numbering back conditionally? And since this has been pushed to master, I take that we need a new bug report. BTW, hiding items is not the only way to make numbers skip, not showing non-available items is an example. And I'm not the author, just a willing executor :)
Great that you want to do do. Sorry for confusing you with the author, but I read your name on the commit.. You do no need a new report. Just set the status back here. If you have it there, I will be happy to sign off on it..
Created attachment 8045 [details] [review] follow-up patch
Created attachment 8054 [details] [review] [SIGNED_OFF] Followup patch restoring search results numbering
QA Comment: Small patch restoring search results numbering when opachiddenitems is not in effect. Thanks Srdjan for submitting this change on short notice. Marking as Passed QA.