Bug 27734

Summary: OpacHiddenItems produces a 404 from borrower holds list
Product: Koha Reporter: Martha Fuerst <mfuerst>
Component: OPACAssignee: Lari Taskula <lari.taskula>
Status: Signed Off --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: david, dcook, jonathan.druart, lari.taskula, magnus, nick, pablo.bianchi, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38936
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Replace OpacHiddenRecordMessage system preference with HTML customization
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences
Bug 27734: Replace OpacHiddenRecordMessage system preference with HTML customization
Bug 27734: Add tests
Bug 27734: Add C4::Output::redirect_if_opac_hidden()
Bug 27734: Use redirect_if_opac_hidden() in opac-*detail.pages
Bug 27734: Add OpacHiddenRecordRedirect system preference
Bug 27734: Add tests
Bug 27734: Add C4::Output::redirect_if_opac_hidden()
Bug 27734: Use redirect_if_opac_hidden() in opac-*detail.pages
Bug 27734: Add OpacHiddenRecordRedirect system preference

Description Martha Fuerst 2021-02-19 15:59:28 UTC
When a borrower clicks on a hold in their holds list they have placed on a title that was subsequently hidden with OpacHiddenItems, it results in a 404 error instead of the "This record is blocked" error page.

It seems like OpacHiddenItems should work the same as individually suppressed records and how they are handled in OpacSuppressionRedirect. This would make more sense from the user's perspective.
Comment 1 Nick Clemens (kidclamp) 2021-06-09 19:33:37 UTC Comment hidden (obsolete)
Comment 2 OpenFifth Sandboxes 2021-06-10 16:14:30 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens (kidclamp) 2021-06-15 12:03:20 UTC
Created attachment 121988 [details] [review]
Bug 27734: Add OpacHiddenRecordRedirect and OpacHiddenRecordMessage system preferences

This patch adds options analogous to OpacSuppressionRedirect and OpacSuppressionMessage for OpacHiddtenItems

Test plan:
1 - Apply patch
2 - Test OpacSupressionsRedirect and OpacSuppressionMessage prefs to ensure they work as before
3 - Enable OpacHiddenItemsHidesRecord and hide a biblio using OpacHiddenItems:
    biblionumber: [303]
3 - Test OpacHiddenRecordRedirect and OpacHiddenRecordMessage to ensure they work as expected
4 - Hit the link: http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=303
5 - When OpacHiddenRecordRedirect set to 404 you get a 404
6 - When OpacHiddenRecordRedirect set to 'an explanatory page' you get 'opac-blocked.pl'
7 - If you set a message in  OpacHiddenRecordMessage  it displays on opac-blocked.pl

Signed-off-by: marti <mfuerst@hmcpl.org>
Comment 4 Tomás Cohen Arazi (tcohen) 2021-06-15 12:13:12 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi (tcohen) 2021-06-15 12:28:02 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2021-06-21 10:41:32 UTC
Should not we stop using syspref for the messages we display on the interface? Should we use "news" instead?

See bug 26050.
Comment 7 Katrin Fischer 2021-06-21 21:05:59 UTC
(In reply to Jonathan Druart from comment #6)
> Should not we stop using syspref for the messages we display on the
> interface? Should we use "news" instead?
> 
> See bug 26050.

Actually, I think you are right. It would also be fair, since we had a similar thing just rewritten and this is a quite new patch set.
Comment 8 Magnus Enger 2021-06-29 11:18:13 UTC

*** This bug has been marked as a duplicate of bug 22286 ***
Comment 9 Magnus Enger 2021-06-29 11:21:05 UTC
Sorry, I got the duplicity backwards!
Comment 10 Magnus Enger 2021-06-29 11:21:40 UTC
*** Bug 22286 has been marked as a duplicate of this bug. ***
Comment 11 Pablo AB 2022-09-16 18:13:42 UTC
This seems to be strongly related to bug 9966. The staff should be able to let access to bibliographic info even if all items are hidden (similar as with biblios without items). Else, show an appropriate message instead of 404.
Comment 12 Lari Taskula 2024-10-22 14:44:58 UTC Comment hidden (obsolete)
Comment 13 Lari Taskula 2024-10-22 14:45:00 UTC Comment hidden (obsolete)
Comment 14 Lari Taskula 2024-10-22 14:46:54 UTC
I've rebased the original patch and added a new patch that replaces OpacHiddenRecordMessage system preference with an HTML customization entry of the same name.
Comment 15 Lari Taskula 2024-10-22 14:47:44 UTC Comment hidden (obsolete)
Comment 16 Roman Dolny 2024-12-19 22:24:44 UTC Comment hidden (obsolete)
Comment 17 Roman Dolny 2024-12-19 22:24:47 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2024-12-30 13:09:23 UTC
Tomas, you are still QA contact here, could you take another look?
Comment 19 Jonathan Druart 2025-01-23 14:22:21 UTC
Feels like opac-MARCdetail and opac-ISBDdetail will need adjustement.

We should follow what has been done on bug 38936.
Comment 20 Tomás Cohen Arazi (tcohen) 2025-02-24 14:14:37 UTC
This needs to be redone.
Comment 21 Lari Taskula 2025-10-21 17:59:32 UTC
Created attachment 188253 [details] [review]
Bug 27734: Add tests
Comment 22 Lari Taskula 2025-10-21 17:59:35 UTC
Created attachment 188254 [details] [review]
Bug 27734: Add C4::Output::redirect_if_opac_hidden()

To test:
1. prove t/db_dependent/Output.t
Comment 23 Lari Taskula 2025-10-21 17:59:37 UTC
Created attachment 188255 [details] [review]
Bug 27734: Use redirect_if_opac_hidden() in opac-*detail.pages
Comment 24 Lari Taskula 2025-10-21 17:59:39 UTC
Created attachment 188256 [details] [review]
Bug 27734: Add OpacHiddenRecordRedirect system preference

This patch adds option analogous to OpacSuppressionRedirect for OpacHiddtenItems

Test plan:
1. Apply patch
2. Create a bibiliographic record and add an item to it. Note down its biblionumber.

It will be later reffered to as <biblionumber>.

3. Set system preference OpacHiddenRecordRedirect on
4. Create HTML customization OpacHiddenRecordMessage. Add some placeholder
   content in it.
5. Enable OpacHiddenItemsHidesRecord and hide a biblio using OpacHiddenItems:

biblionumber: [<biblionumber>]

e.g.

biblionumber: [123]

6. Test OpacHiddenRecordRedirect and OpacHiddenRecordMessage to ensure they work
   as expected
7. Hit these links:

http://koha/cgi-bin/koha/opac-detail.pl?biblionumber=<biblionumber>
http://koha/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=<biblionumber>
http://koha/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=<biblionumber>

8. When OpacHiddenRecordRedirect set to 404 you get a 404
9. When OpacHiddenRecordRedirect set to 'an explanatory page' you get 'opac-blocked.pl'
10. If you set a message in OpacHiddenRecordMessage it displays on opac-blocked.pl
11. Using preference OpacHiddenItemsExceptions, exclude your patron category
12. Observe the record is no longer blocked when your patron category is excluded
Comment 25 David Nind 2026-01-01 20:22:05 UTC
Created attachment 190812 [details] [review]
Bug 27734: Add tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 26 David Nind 2026-01-01 20:22:09 UTC
Created attachment 190813 [details] [review]
Bug 27734: Add C4::Output::redirect_if_opac_hidden()

To test:
1. prove t/db_dependent/Output.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 27 David Nind 2026-01-01 20:22:13 UTC
Created attachment 190814 [details] [review]
Bug 27734: Use redirect_if_opac_hidden() in opac-*detail.pages

Signed-off-by: David Nind <david@davidnind.com>
Comment 28 David Nind 2026-01-01 20:22:17 UTC
Created attachment 190815 [details] [review]
Bug 27734: Add OpacHiddenRecordRedirect system preference

This patch adds option analogous to OpacSuppressionRedirect for OpacHiddtenItems

Test plan:
1. Apply patch
2. Create a bibiliographic record and add an item to it. Note down its biblionumber.

It will be later reffered to as <biblionumber>.

3. Set system preference OpacHiddenRecordRedirect on
4. Create HTML customization OpacHiddenRecordMessage. Add some placeholder
   content in it.
5. Enable OpacHiddenItemsHidesRecord and hide a biblio using OpacHiddenItems:

biblionumber: [<biblionumber>]

e.g.

biblionumber: [123]

6. Test OpacHiddenRecordRedirect and OpacHiddenRecordMessage to ensure they work
   as expected
7. Hit these links:

http://koha/cgi-bin/koha/opac-detail.pl?biblionumber=<biblionumber>
http://koha/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=<biblionumber>
http://koha/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=<biblionumber>

8. When OpacHiddenRecordRedirect set to 404 you get a 404
9. When OpacHiddenRecordRedirect set to 'an explanatory page' you get 'opac-blocked.pl'
10. If you set a message in OpacHiddenRecordMessage it displays on opac-blocked.pl
11. Using preference OpacHiddenItemsExceptions, exclude your patron category
12. Observe the record is no longer blocked when your patron category is excluded

Signed-off-by: David Nind <david@davidnind.com>
Comment 29 David Nind 2026-01-01 20:31:01 UTC
I've signed off, but have some feedback:

1. System preference description:
   - "..opac.." should be OPAC
   - for the option "an explanatory page ('This record is blocked').", 
     the title of the page generated is "Record blocked", so the option for 
     the system preference should match.
   - for  "..redirect page for suppressed bibliographic records...", I think
     that should be "..hidden..", as this is what we use in the sentence 
     above and in the system preference name

2. With the "explanatory page" option, I found it odd that the title and message 
   are hard-coded. That is, we are adding a way to add a custom page for the 404,
   but then are hard-coding a 

   What is displayed if the "explanatory page" option is selected:

     Record blocked

     You are not authorized to view this record.

     [Then anything in OpacHiddenRecordMessage] 

Testing notes (using KTD):

1. To reproduce the issue:
   1.1 Place a hold on a record for a patron (I used Programming Perl, 262)
   1.2 Log in to the OPAC as that patron and go to Your account > Your summary > Holds
   1.3 Click on the link to the item on hold. Success! You can see the record.
   1.4 For OpacHiddenItems, add: biblionumber: [262]
   1.5 Repeat step 1.3
   1.6 You get the "An error has occurred page":
         Sorry, the requested page is not available
         Error 404
2. Apply the patch
3. Update the database: updatedatabase
4. Restart everything: restart_all
5. For step 2 of the test plab: I used an existing record, CGI programming with Perl (11), which has one item
6. For step 3: OpacHiddenRecordRedirect is on by default (set to 'Hide')
7. For step 5: OpacHiddenItems - biblionumber: [11]