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`
Moving to the security component as the diff would expose unpublished code from a `Koha security` bug.
Created attachment 176855 [details] [review] Bug 38936: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 176856 [details] [review] Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
Looking good to me :)
Created attachment 178576 [details] [review] [24.05.x] Bug 38936: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
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>
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.
I am ready to QA this as soon as it's signed off.
Patch doesn't apply