Created attachment 46100 [details] Suppressed items shown When a patron places a hold on a specific item in the copiesrow table, suppressed items show in the OPAC. System preference for OPACSuppression is set to Hide. System preference for OPAC Hidden Items is set to hide is suppressed items.
Can you share a bit more about your configuration? What's the exact entry in opachiddenitems? Does it work on the detail page?
I could reproduce this by: 1. Allowing item-level holds for an itemtype in Circulation and Fine rules. 2. Setting the OpacHiddenItems preference to "withdrawn: [1]." 3. Edit an item and set the withdrawn status to 1. 4. Search for the title in the OPAC. The withdrawn item isn't shown. 5. View the detail page for the title. The withdrawn item isn't shown. 6. Place a hold on the item: -- Click "Show more options" -- Select "A specific item" -- Observe that the withdrawn item appears in the list of items. (along with item which are lost, by the way, even though hidelostitems is set to "don't show") I don't think the OPACSuppression preference comes into play here.
In 16.05.05 when a rule to hide specific items in OPACHiddenItems is set, the item does not show in the holdings table, but does appear when the user attempts to place the hold. 1. Circulation and Fine rules for item-level holds are set to allow. 2. Setting the OpacHiddenItems preference to "ccode: [SUPPRESS]." 3. An item with collection Code Suppress is set. 4. Search for the title in the OPAC. The Suppress ccode item isn't shown. 5. View the detail page for the title. The suppress ccode item isn't shown. 6. Place a hold on the item: -- Click "Show more options" -- Select "A specific item" -- the ccode with the opachiddenitems rule has an item that appear in the list to place a hold. Jessie
Confirmed on current master. Updating to normal.
Maybe hidden items should be removed directly in C4::Items::GetItemsInfo, called by opac-reserve.pl
As hidden items are hidden for reason... bumping up severity a little bit to get more attention.
(In reply to Fridolin SOMERS from comment #5) > Maybe hidden items should be removed directly in C4::Items::GetItemsInfo, > called by opac-reserve.pl Can you explain this a bit more? Not sure how it works right now, did you mean move or remove?
Old and mean bug with quite some votes... just giving it another bump.
(In reply to Katrin Fischer from comment #7) > (In reply to Fridolin SOMERS from comment #5) > > Maybe hidden items should be removed directly in C4::Items::GetItemsInfo, > > called by opac-reserve.pl > > Can you explain this a bit more? Not sure how it works right now, did you > mean move or remove? I mean remove, so they are never given to the OPAC. One should then add a new arg to the method to say opac or staff interface.
*** Bug 18167 has been marked as a duplicate of this bug. ***
I would like to add another vote to seeing this bug fixed.
(In reply to Chris Kirby from comment #11) > I would like to add another vote to seeing this bug fixed. +1
+1 on this!
Created attachment 113875 [details] [review] Bug 15448: Clean existing code in opac-detail This patch groups the code related to OpacHiddenItems and hidelostitems. It certainly fixed few issues (we are now filtering after EasyAnalyticalRecords), maybe not... At least now the code is at the same place.
Created attachment 113876 [details] [review] Bug 15448: Copy code to opac-reserve.pl So yes, this code must be moved to a module, and use in the different places when the items are not hidden at the OPAC (See bug 24403). Call this a first step.
Those 2 patches should fix the reported issue. But they are not ready for integration IMO. However it would be good to have early feedback from testers and QA.
The code should somehow be using Koha::Item->hidden_in_opac.
(In reply to Tomás Cohen Arazi from comment #17) > The code should somehow be using Koha::Item->hidden_in_opac. That's for hidelostitems, not directly related. There are other bug reports for that. But yes, everything should end up there at some point.
I want to propose an alternate solution based on bug 24254. Writing the alternate patch. I would love your feedback on that bug and if it makes sense to use here.
(In reply to Tomás Cohen Arazi from comment #19) > I want to propose an alternate solution based on bug 24254. Writing the > alternate patch. I would love your feedback on that bug and if it makes > sense to use here. ^^ this last comment is because the other bug introduces a new method and it could make things harder for bakporting to the stable branches (don't really think so, but just in case)
Created attachment 114188 [details] [review] Bug 15448: Filter out items hidden in OPAC on placing hold This patch leverages on bug 24254's Koha::Items->filter_by_visible_in_opac to filter out items that shouldn't be presented to the end user in the OPAC interface. To test: 1. Circulation and Fine rules for item-level holds are set to allow. 2. Setting the OpacHiddenItems preference to "ccode: [SUPPRESS]" 3. An item with collection Code Suppress is set. 4. Search for the title in the OPAC. => SUCCESS: The Suppress ccode item isn't shown. 5. View the detail page for the title. => SUCCESS: The suppress ccode item isn't shown. 6. Place a hold on the item: -- Click "Show more options" -- Select "A specific item" => FAIL: the ccode with the opachiddenitems rule has an item that appear in the list to place a hold 7. Apply this patch and repeat 6 => SUCCESS: The item doesn't show! 8. Sign off :-D
Created attachment 114189 [details] [review] Bug 15448: (follow-up) Fetch the patron category once
Created attachment 114191 [details] [review] Bug 15448: (QA follow-up) Move fetching the rules outside the loop
Created attachment 114193 [details] [review] Bug 15448: Filter out items hidden in OPAC on placing hold This patch leverages on bug 24254's Koha::Items->filter_by_visible_in_opac to filter out items that shouldn't be presented to the end user in the OPAC interface. To test: 1. Circulation and Fine rules for item-level holds are set to allow. 2. Setting the OpacHiddenItems preference to "ccode: [SUPPRESS]" 3. An item with collection Code Suppress is set. 4. Search for the title in the OPAC. => SUCCESS: The Suppress ccode item isn't shown. 5. View the detail page for the title. => SUCCESS: The suppress ccode item isn't shown. 6. Place a hold on the item: -- Click "Show more options" -- Select "A specific item" => FAIL: the ccode with the opachiddenitems rule has an item that appear in the list to place a hold 7. Apply this patch and repeat 6 => SUCCESS: The item doesn't show! 8. Sign off :-D
Created attachment 114194 [details] [review] Bug 15448: (follow-up) Fetch the patron category once
Created attachment 114208 [details] [review] Bug 15448: Filter out items hidden in OPAC on placing hold This patch leverages on bug 24254's Koha::Items->filter_by_visible_in_opac to filter out items that shouldn't be presented to the end user in the OPAC interface. To test: 1. Circulation and Fine rules for item-level holds are set to allow. 2. Setting the OpacHiddenItems preference to "ccode: [SUPPRESS]" 3. An item with collection Code Suppress is set. 4. Search for the title in the OPAC. => SUCCESS: The Suppress ccode item isn't shown. 5. View the detail page for the title. => SUCCESS: The suppress ccode item isn't shown. 6. Place a hold on the item: -- Click "Show more options" -- Select "A specific item" => FAIL: the ccode with the opachiddenitems rule has an item that appear in the list to place a hold 7. Apply this patch and repeat 6 => SUCCESS: The item doesn't show! 8. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 114209 [details] [review] Bug 15448: (follow-up) Fetch the patron category once Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 114263 [details] [review] Bug 15448: Filter out items hidden in OPAC on placing hold This patch leverages on bug 24254's Koha::Items->filter_by_visible_in_opac to filter out items that shouldn't be presented to the end user in the OPAC interface. To test: 1. Circulation and Fine rules for item-level holds are set to allow. 2. Setting the OpacHiddenItems preference to "ccode: [SUPPRESS]" 3. An item with collection Code Suppress is set. 4. Search for the title in the OPAC. => SUCCESS: The Suppress ccode item isn't shown. 5. View the detail page for the title. => SUCCESS: The suppress ccode item isn't shown. 6. Place a hold on the item: -- Click "Show more options" -- Select "A specific item" => FAIL: the ccode with the opachiddenitems rule has an item that appear in the list to place a hold 7. Apply this patch and repeat 6 => SUCCESS: The item doesn't show! 8. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 114264 [details] [review] Bug 15448: (follow-up) Fetch the patron category once Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Works a expect, QA script ok, code looks ok.
Glad to see this one moving!
Bug 24254 is not PQA yet.
Now it is, thanks Martin! :)
Created attachment 114578 [details] [review] Bug 15448: (follow-up) Adapt to changes on bug 24254 Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
The method Koha::Items->itemnumber is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 592 Koha::Objects::AUTOLOAD('Koha::Items=HASH(0x55981fd94790)') called at /kohadevbox/koha/opac/opac-reserve.pl line 465 Hum?
Created attachment 115133 [details] [review] Bug 15448: Fix ISE The method Koha::Items->itemnumber is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 592 Koha::Objects::AUTOLOAD('Koha::Items=HASH(0x55981fd94790)') called at /kohadevbox/koha/opac/opac-reserve.pl line 465
Created attachment 115134 [details] [review] Bug 15448: Remove items fetch We fetch them already too many times.
Pushed to master for 21.05, thanks to everybody involved!
Depends on Bug 24254 not in 20.11.x
(In reply to Fridolin Somers from comment #39) > Depends on Bug 24254 not in 20.11.x Hi Frido, could you consider backporting bug 24254? 20.11.x is still young and it's the base for fixing quit evil bugs like this one.
(In reply to Katrin Fischer from comment #40) > (In reply to Fridolin Somers from comment #39) > > Depends on Bug 24254 not in 20.11.x > > Hi Frido, could you consider backporting bug 24254? 20.11.x is still young > and it's the base for fixing quit evil bugs like this one. Oh yes indeed, I had mistaken this one for an enhancement. I'm on it
Pushed to 20.11.x for 20.11.02 Major so I backported Bug 24254 to be able to backport this one.
(In reply to Fridolin Somers from comment #42) > Pushed to 20.11.x for 20.11.02 > > Major so I backported Bug 24254 to be able to backport this one. Thank you :)
Bug 24254, on which this depends, does not apply cleanly to 20.05. Please clear that if this is needed in 20.05. Thanks!
(In reply to Andrew Fuerste-Henry from comment #44) > Bug 24254, on which this depends, does not apply cleanly to 20.05. Please > clear that if this is needed in 20.05. Thanks! Done!
This doesn't work as intended and I'm not clear on whether the issue is here or on bug 24254. This blocks holds on any item that doesn't have a value that doesn't match what's in OPACHiddenItems. So, if I set OPACHiddenItems to "copynumber: [2]," then I'm blocked from placing holds on any item with copynubmer=2 AND any item with copynumber is NULL. This applies to both 20.05 and master.
(In reply to Andrew Fuerste-Henry from comment #46) > This doesn't work as intended and I'm not clear on whether the issue is here > or on bug 24254. This blocks holds on any item that doesn't have a value > that doesn't match what's in OPACHiddenItems. So, if I set OPACHiddenItems > to "copynumber: [2]," then I'm blocked from placing holds on any item with > copynubmer=2 AND any item with copynumber is NULL. > > This applies to both 20.05 and master. Good catch, can you file a separate bug and I'll take a look now?
Pushed to 20.05.x for 20.05.09 Works as intended so long as it doesn't encounter any NULLs. That issue comes from bug 24254 and is corrected in bug 27580.
Backported: Pushed to 19.11.x branch for 19.11.15