Bug 34836 - OPAC ISBD or MARC view blows up with error 500
Summary: OPAC ISBD or MARC view blows up with error 500
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Pedro Amorim
QA Contact: Marcel de Rooy
URL:
Keywords: rel_22_11_candidate
: 24574 (view as bug list)
Depends on: 30846 34694
Blocks: 34886
  Show dependency treegraph
 
Reported: 2023-09-20 07:50 UTC by Fridolin Somers
Modified: 2023-10-18 14:48 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes an error that occurs when viewing the MARC and ISBD views of a record in the OPAC (when not logged in) - the detail pages cannot be viewed and there is an error trace displayed.
Version(s) released in:
23.11.00,23.05.05,22.11.11


Attachments
Bug 34836: Add patron check to isbd and marc detail pages (1.92 KB, patch)
2023-09-25 10:22 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34836: Add patron check to isbd and marc detail pages (1.97 KB, patch)
2023-09-27 17:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 34836: Add patron check to isbd and marc detail pages (2.06 KB, patch)
2023-09-29 06:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2023-09-20 07:50:53 UTC
Bug 34694 added check on $patron var for opac-detail.pl

Since Bug 30846 also impacts ISBD and MARC views, same check should be made.
Comment 1 Fridolin Somers 2023-09-20 07:59:00 UTC
I confirm in KTD with test plan of Bug 34694 => Kaboum
Comment 2 Pedro Amorim 2023-09-25 10:22:57 UTC
Created attachment 156146 [details] [review]
Bug 34836: Add patron check to isbd and marc detail pages

Test plan:
- Go to circulation rules and set On shelf holds allowed to If all unavailable
- Log out
- Visit a MARC detail or ISBD detail bib record on OPAC:
http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=76
http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=76
- Notice it blows up with error 500, on both occasions
- Apply patch. Repeat.
Comment 3 Pedro Amorim 2023-09-25 10:27:03 UTC
Only now catching up on bug 34886.
Comment 4 Pedro Amorim 2023-09-25 12:52:39 UTC
If what I submitted in bug 34886 makes sense and gets pushed (move the patron check onto C4/Reserves.pm), I think there's nothing to be done here as the issue should be fixed.
Comment 5 Pedro Amorim 2023-09-27 14:02:25 UTC
I've restored my patch since this is critical and bug 34886 is revealing itself to be a bigger can of worms than originally anticipated.

I'm setting bug 34886 as dependent on this one.

This is to keep things seperate. This current bug 34836 is to prevent error 500 on the opac marc-detail and isbd-detail when On shelf holds allowed = "If all unavailable".

Bug 34886 is to restore "Place hold" link for unauthenticated users.
Comment 6 Pedro Amorim 2023-09-27 14:04:32 UTC
I think this should at least be considered for the releases this month.
Comment 7 David Nind 2023-09-27 17:19:41 UTC
Created attachment 156303 [details] [review]
Bug 34836: Add patron check to isbd and marc detail pages

Test plan:
- Go to circulation rules and set On shelf holds allowed to If all unavailable
- Log out
- Visit a MARC detail or ISBD detail bib record on OPAC:
http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=76
http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=76
- Notice it blows up with error 500, on both occasions
- Apply patch. Repeat.

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Jonathan Druart 2023-09-28 06:32:24 UTC
Shouldn't this be fixed exactly the same as bug 34886?
Comment 9 Pedro Amorim 2023-09-28 08:22:12 UTC
(In reply to Jonathan Druart from comment #8)
> Shouldn't this be fixed exactly the same as bug 34886?

This was my original thinking, hence submitting the patches there originally.

But rethinking it, in my current opinion, no. The regression mentioned in bug 34886 was actually caused by bug 30846. It's just that bug 30846 caused the 500 error on the record page (normal, marc and isbd) for the unauthenticated user before the "place hold" link could be even be assessed.

Bug 34694 fixed the 500 error for the normal record page (forgetting about marc+isbd), but didn't address the "place hold" link issue, since the "place hold" issue was not detectable before - the page blew up.

The fixing of the "place hold" link should be a follow-up to the fixing of the 500 error, hence making the "place hold" bug 34886 now depend on this one.

Does this make sense?
Comment 10 Marcel de Rooy 2023-09-29 06:12:16 UTC
Created attachment 156361 [details] [review]
Bug 34836: Add patron check to isbd and marc detail pages

Test plan:
- Go to circulation rules and set On shelf holds allowed to If all unavailable
- Log out
- Visit a MARC detail or ISBD detail bib record on OPAC:
http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=76
http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=76
- Notice it blows up with error 500, on both occasions
- Apply patch. Repeat.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Tomás Cohen Arazi 2023-10-03 12:55:23 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Fridolin Somers 2023-10-05 18:41:54 UTC
Pushed to 23.05.x for 23.05.05
Comment 13 Laura O'Neil 2023-10-10 13:34:27 UTC
Could this also be pushed to 22.11.x?
Comment 14 Claudio Costales 2023-10-13 18:00:26 UTC
(In reply to Laura O'Neil from comment #13)
> Could this also be pushed to 22.11.x?

+1
Comment 15 Jacob O'Mara 2023-10-17 13:37:49 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x
Comment 16 Liz Rea 2023-10-18 14:48:30 UTC
*** Bug 24574 has been marked as a duplicate of this bug. ***