Bug 38936

Summary: Move suppressed record redirection into a sub
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Patch doesn't apply --- QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P5 - low CC: dcook, didier.gautheron, fridolin.somers, jesse, jonathan.druart, katrin.fischer, laurent.ducos, lisette, lucas, m.de.rooy, martin.renvoize, nick, philippe.blouin, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27734
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 28478    
Bug Blocks:    
Attachments: Bug 38936: Unit tests
Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed
Bug 38936: Use redirect_if_opac_suppressed() in opac-*detail.pl pages
[24.05.x] Bug 38936: Unit tests
[24.05.x] Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed
[24.05.x] Bug 38936: Use redirect_if_opac_suppressed() in opac-*detail.pl pages

Description Tomás Cohen Arazi (tcohen) 2025-01-21 11:05:51 UTC
Bug 28478 solved a long standing issue, but there's room for improvement: the controllers use a fairly big code block to handle redirection for suppressed records, with checks for things like `OpacSuppressionByIPRange`.

This should be put into a sub. Probably `C4::Output::opac_suppression_redirect`
Comment 1 Tomás Cohen Arazi (tcohen) 2025-01-21 11:28:16 UTC
Moving to the security component as the diff would expose unpublished code from a `Koha security` bug.
Comment 2 Tomás Cohen Arazi (tcohen) 2025-01-21 12:43:37 UTC
Created attachment 176855 [details] [review]
Bug 38936: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi (tcohen) 2025-01-21 12:43:40 UTC
Created attachment 176856 [details] [review]
Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi (tcohen) 2025-01-21 12:43:43 UTC
Created attachment 176857 [details] [review]
Bug 38936: Use redirect_if_opac_suppressed() in opac-*detail.pl pages

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Martin Renvoize (ashimema) 2025-01-21 13:23:20 UTC
Looking good to me :)
Comment 6 Tomás Cohen Arazi (tcohen) 2025-02-24 13:59:01 UTC
Created attachment 178576 [details] [review]
[24.05.x] Bug 38936: Unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi (tcohen) 2025-02-24 13:59:04 UTC
Created attachment 178577 [details] [review]
[24.05.x] Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi (tcohen) 2025-02-24 13:59:07 UTC
Created attachment 178578 [details] [review]
[24.05.x] Bug 38936: Use redirect_if_opac_suppressed() in opac-*detail.pl pages

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 David Cook 2025-02-24 22:26:33 UTC
From a separation of concerns perspective, I don't like the $biblio object being in this function, but having this code in a function is a great step forward.
Comment 10 Jonathan Druart 2025-03-11 14:39:03 UTC
I am ready to QA this as soon as it's signed off.
Comment 11 Jesse Maseto 2025-04-03 09:31:14 UTC
Patch doesn't apply