Bug 38936 - Move suppressed record redirection into a sub
Summary: Move suppressed record redirection into a sub
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 28478
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-21 11:05 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2025-04-03 09:31 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 38936: Unit tests (5.23 KB, patch)
2025-01-21 12:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed (2.08 KB, patch)
2025-01-21 12:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38936: Use redirect_if_opac_suppressed() in opac-*detail.pl pages (5.45 KB, patch)
2025-01-21 12:43 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[24.05.x] Bug 38936: Unit tests (5.24 KB, patch)
2025-02-24 13:59 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[24.05.x] Bug 38936: Introduce C4::Output::redirect_if_opac_suppressed (2.09 KB, patch)
2025-02-24 13:59 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[24.05.x] Bug 38936: Use redirect_if_opac_suppressed() in opac-*detail.pl pages (5.47 KB, patch)
2025-02-24 13:59 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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