For this configuration in OpacHiddenItems wthdrawn and notforloan work, but restricted does not: notforloan: [2] wthdrawn: [2] restricted: [2] Looking at the code I didn't see a reason for this. If someone could test and confirm/invalidate this bug it would be appreciated.
Still puzzling, still valid. Nothing helpful in the logs.
I can verify this. on 3.22.07 opachiddenitems set with restricted: [1] does not hide items (In reply to Katrin Fischer from comment #2) > Still puzzling, still valid. Nothing helpful in the logs.
Tested on 17.11 again - still not working. Looking at the code I don't see a reason for it not working. lost and damaged are also numerical and work without issues, but restricted does not. It doesn't appear to be a reserved word in SQL... out of ideas.
Katrin, I've just tested this on 18.06.00.028 and it is working. Restricted items are being hidden in the OPAC when a value of restricted: [1] is placed in the OPACHiddenItems syspref. (In reply to Katrin Fischer from comment #4) > Tested on 17.11 again - still not working. Looking at the code I don't see a > reason for it not working. lost and damaged are also numerical and work > without issues, but restricted does not. > > It doesn't appear to be a reserved word in SQL... out of ideas.
Thank you, Joy, that's good news!
Hi Joy, I tested again today on master - it still doesn't work for me. Here is what I did: OpacHiddenItems: restricted: [1] Changed 1 item out of 4 to be restricted. Opac now shows it as: Available (Restricted Access) Not hidden...
After some debugging I think I have found the reason. It won't work with the numeric value of the authorised value used, it needs the description: restricted: ['Restricted Access'] For other values like Damaged it will works with numerics. The reason is some code in GetItemsInfo: # get notforloan complete status if applicable $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $data->{frameworkcode}, kohafield => 'items.notforloan', authorised_value => $data->{itemnotforloan} }); $data->{notforloanvalue} = $descriptions->{lib} // ''; $data->{notforloanvalueopac} = $descriptions->{opac_description} // ''; # get restricted status and description if applicable $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => $data->{frameworkcode}, kohafield => 'items.restricted', authorised_value => $data->{restricted} }); $data->{restricted} = $descriptions->{lib} // ''; $data->{restrictedopac} = $descriptions->{opac_description} // ''; While it uses a new variable "*value*" for notforloan, it overwrites the restricted numerical value with the description. I think the clean solution is to change it so that OpacHiddenItems works like for the other item fields with authorised values, using the codes.
Created attachment 83630 [details] [review] Bug 10676: Fix behaviour of OpacHiddenItems for items.restricted The numerical restricted value was overwritten in GetItemsInfo with the description. So OpacHiddenItems would only work for restricted using the description, not the numerical value of the authorised value like for other similar status fields. This changes GetItemsInfo to use a separate variable for the description, as it's already done for notforloan and changes the template files where the variable is used accordingly. To test - Before applying patch - Configure OpacHiddenItems with: restricted: [1] - Find a record with more than one item and mark one of the items as "Restricted Access" (assume standard sample data - check authorised value if status doesn't exist) - Verify the item is still shown in OPAC. - Apply patch - Verify that the item is now hidden - Verify that the status still displays nicely in the staff detail page - Delete OpacHiddenItems - Verify that the status is showing nicely in the OPAC detail page
Created attachment 83631 [details] [review] Bug 10676: Add unit tests - GetItemsInfo To test: prove t/db_dependent/Items.t
All seems allright, except the last step where the item doesn't show up when OpacHiddenItems is empty in OPAC. Can you please verify if it's the same for you?
My testing of the original issue (on 18.06) could not replicate it. I was using the code for the Restricted value, not the description. The items were hidden on the opac when Restricted: [1] was in the opachiddenitems.
Hi Joy, is it possible you used a framework that was not linked to the authorised value correctly? Looking at the code it can't work with the number, unless the number is in the description or the authorised value description cannot be found.
(In reply to Pierre-Marc Thibault from comment #11) > All seems allright, except the last step where the item doesn't show up when > OpacHiddenItems is empty in OPAC. Can you please verify if it's the same for > you? Yes it did work - when the pref is empty and read correctly, there is no more reason to hide it for Koha. Can you please check that the item didn't fit another criteria for being hidden? Is your system preference OpacHiddenItems really empty? Is your system set up with memcached and Plack?
My framework is the Default framework. 952$5 is tied to RESTRICTED authorized value list. The RESTRICTED auth value list has code of 1 and description of "Restricted Access". OpacHiddenItems contains (with a blank line at end) itype: [GAMES,EBOOK] ccode: [REFERENCE,MUSIC] location: [HOMESCHOOL,PARENTING] restricted: [1] My item was none of the values listed below and appeared in OPAC searches. When i edit the item to RESTRICTED (items.restricted=1), it dissappears from searches. Although number of bibs still show the count of 5, where search results only show 4. (In reply to Katrin Fischer from comment #13) > Hi Joy, is it possible you used a framework that was not linked to the > authorised value correctly? Looking at the code it can't work with the > number, unless the number is in the description or the authorised value > description cannot be found.
Testing with Nick shows that this is dependent on the number of items. My test were for a bib with 1 item and that item was restricted. It does hide the title from the OPAC because the one item is 'hidden'. However when another item is added, the hidden item does not appear in search results (as available), but does appear in the detail page when the title is clicked on. (In reply to Joy Nelson from comment #15) > My framework is the Default framework. 952$5 is tied to RESTRICTED > authorized value list. The RESTRICTED auth value list has code of 1 and > description of "Restricted Access". > > OpacHiddenItems contains (with a blank line at end) > itype: [GAMES,EBOOK] > ccode: [REFERENCE,MUSIC] > location: [HOMESCHOOL,PARENTING] > restricted: [1] > > My item was none of the values listed below and appeared in OPAC searches. > When i edit the item to RESTRICTED (items.restricted=1), it dissappears from > searches. Although number of bibs still show the count of 5, where search > results only show 4. > > (In reply to Katrin Fischer from comment #13) > > Hi Joy, is it possible you used a framework that was not linked to the > > authorised value correctly? Looking at the code it can't work with the > > number, unless the number is in the description or the authorised value > > description cannot be found.
(In reply to Katrin Fischer from comment #14) > (In reply to Pierre-Marc Thibault from comment #11) > > All seems allright, except the last step where the item doesn't show up when > > OpacHiddenItems is empty in OPAC. Can you please verify if it's the same for > > you? > > Yes it did work - when the pref is empty and read correctly, there is no > more reason to hide it for Koha. > > Can you please check that the item didn't fit another criteria for being > hidden? > Is your system preference OpacHiddenItems really empty? > Is your system set up with memcached and Plack? Sorry, it was likely a mistake on the refresh of the content.
Hi Joy, thx for checking again! I'd need to investigate a bit more, but it's possible the code for hiding the record is using a slightly different logic. Koha is always fun ;) Pierre, is it working ok now for you?
I didn't complete the test since there is a current conversation involving Joy which let me think there is a problem that is not in the test plan and that would make my sign off meaningless. I am wrong?
If I understood correctly, you can still sign off. This deals with the case, that not all items on a record are hidden and the OPAC detail pages. You could extend the test plan a bit: - Verify that if the restricted item is the only item, the record cannot be found in the OPAC - Verfy you can't access the record directly by URL - Verify the item information in the results is correct From what Joy said those already work, but should not be broken by the patch.
Created attachment 83668 [details] [review] Bug 10676: Fix behaviour of OpacHiddenItems for items.restricted The numerical restricted value was overwritten in GetItemsInfo with the description. So OpacHiddenItems would only work for restricted using the description, not the numerical value of the authorised value like for other similar status fields. This changes GetItemsInfo to use a separate variable for the description, as it's already done for notforloan and changes the template files where the variable is used accordingly. To test - Before applying patch - Configure OpacHiddenItems with: restricted: [1] - Find a record with more than one item and mark one of the items as "Restricted Access" (assume standard sample data - check authorised value if status doesn't exist) - Verify the item is still shown in OPAC. - Apply patch - Verify that the item is now hidden - Verify that the status still displays nicely in the staff detail page - Delete OpacHiddenItems - Verify that the status is showing nicely in the OPAC detail page Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com> https://bugs.koha-community.org/show_bug.cgi?id=22053
Created attachment 83669 [details] [review] Bug 10676: Add unit tests - GetItemsInfo To test: prove t/db_dependent/Items.t Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com> https://bugs.koha-community.org/show_bug.cgi?id=22053
Created attachment 83670 [details] [review] Bug 10676: Fix behaviour of OpacHiddenItems for items.restricted The numerical restricted value was overwritten in GetItemsInfo with the description. So OpacHiddenItems would only work for restricted using the description, not the numerical value of the authorised value like for other similar status fields. This changes GetItemsInfo to use a separate variable for the description, as it's already done for notforloan and changes the template files where the variable is used accordingly. To test - Before applying patch - Configure OpacHiddenItems with: restricted: [1] - Find a record with more than one item and mark one of the items as "Restricted Access" (assume standard sample data - check authorised value if status doesn't exist) - Verify the item is still shown in OPAC. - Apply patch - Verify that the item is now hidden - Verify that the status still displays nicely in the staff detail page - Delete OpacHiddenItems - Verify that the status is showing nicely in the OPAC detail page Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
Created attachment 83671 [details] [review] Bug 10676: Add unit tests - GetItemsInfo To test: prove t/db_dependent/Items.t Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
(In reply to Pierre-Marc Thibault from comment #22) > Created attachment 83669 [details] [review] [review] > Bug 10676: Add unit tests - GetItemsInfo > > To test: > prove t/db_dependent/Items.t > > Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com> > > https://bugs.koha-community.org/show_bug.cgi?id=22053
Your test plan works and it doesn't seem there's a problem with the extended steps either. Sorry, for the mistakes I made while editing the page, I will try to be more focus next time. :)
Created attachment 85122 [details] [review] Bug 10676: Fix behaviour of OpacHiddenItems for items.restricted The numerical restricted value was overwritten in GetItemsInfo with the description. So OpacHiddenItems would only work for restricted using the description, not the numerical value of the authorised value like for other similar status fields. This changes GetItemsInfo to use a separate variable for the description, as it's already done for notforloan and changes the template files where the variable is used accordingly. To test - Before applying patch - Configure OpacHiddenItems with: restricted: [1] - Find a record with more than one item and mark one of the items as "Restricted Access" (assume standard sample data - check authorised value if status doesn't exist) - Verify the item is still shown in OPAC. - Apply patch - Verify that the item is now hidden - Verify that the status still displays nicely in the staff detail page - Delete OpacHiddenItems - Verify that the status is showing nicely in the OPAC detail page Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 85123 [details] [review] Bug 10676: Add unit tests - GetItemsInfo To test: prove t/db_dependent/Items.t Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Well done for getting to the bottom of this challenging one Katrin. All works for me, no regressions found and the QA scripts all pass.. Passing QA
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.04
backported to 18.05.x for 18.05.11