Bug 24424 - Advanced editor - interface hangs as "Loading" when given an invalid bib number
Summary: Advanced editor - interface hangs as "Loading" when given an invalid bib number
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Janusz Kaczmarek
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on: 16424 29697
Blocks: 37212
  Show dependency treegraph
 
Reported: 2020-01-14 23:31 UTC by Cori Lynn Arnold
Modified: 2024-06-27 18:30 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06


Attachments
Bug 24424: Advanced editor - interface hangs as "Loading" when given an invalid bib number (2.31 KB, patch)
2024-05-14 11:49 UTC, Janusz Kaczmarek
Details | Diff | Splinter Review
Bug 24424: Advanced editor - interface hangs as "Loading" when given an invalid bib number (2.37 KB, patch)
2024-05-14 14:00 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Cori Lynn Arnold 2020-01-14 23:31:57 UTC
The Advanced Editor hangs with "Loading, please wait" message when given an invalid bib number /cgi-bin/koha/cataloguing/editor.pl#catalog/55555

https://paste.pics/e7c7e4956dfa888b13b54332472df066
Comment 1 Phil Ringnalda 2024-05-03 21:38:21 UTC
We could have (but didn't) handled the returned 404 Not found from https://git.koha-community.org/Koha-community/Koha/src/commit/ccd4738e26ef3c2e40bfa1dc1e74dfdb94a34239/svc/bib#L84 before bug 29697 accidentally turned that into dead code. Now svc/bib calls Koha::Biblios->find( $biblionumber ) which returns undef if the biblio doesn't exist, but rather than returning 404 if we can't find the biblio, we forge ahead, calling $biblio->metadata->record() on undef and getting a 500 thrown instead.
Comment 2 Janusz Kaczmarek 2024-05-14 11:49:46 UTC
Created attachment 166683 [details] [review]
Bug 24424: Advanced editor - interface hangs as "Loading" when given an invalid bib number

The Advanced Editor hangs with "Loading, please wait" message when given an
invalid bib number, e.g. /cgi-bin/koha/cataloguing/editor.pl#catalog/55555

This is because in the bug 16424, when changes had been made to
koha-tmpl/intranet-tmpl/lib/koha/cateditor/koha-backend.js
(Bug 16424: Add framework support to advanced MARC editor),
the .fail callback method of the outer .get (in the KohaBackend.GetRecord
function called from cateditor-ui.inc) had been (perhaps by mistake?) removed.
So, in case of failure, the situation is not handled properly.

The proposal is to restore the outer .fail.  As a result, the
openRecord( 'new/', editor, finishCb ); will be called (a standard way
of resolving failed openRecord action (cf. the bottom of cateditor-ui.inc).

Test plan:
==========
0. Be sure your browser does not cache JavaScript for this test.
1. Enable EnableAdvancedCatalogingEditor in the system preferences.
2. Try to open in the advanced editor an unexisting record, e.g.
   http://your_ktd:8081/cgi-bin/koha/cataloguing/editor.pl#catalog/55555
   Koha should hang with a message "Loading, please wait".
3. Apply the patch, restart_all.
4. In a new browser window, repeat p. 2 (remember not to cache JS).
5. The editor should open with a new empty record.

Sponsored-by: Ignatianum University in Cracow
Comment 3 Nick Clemens (kidclamp) 2024-05-14 14:00:54 UTC
Created attachment 166690 [details] [review]
Bug 24424: Advanced editor - interface hangs as "Loading" when given an invalid bib number

The Advanced Editor hangs with "Loading, please wait" message when given an
invalid bib number, e.g. /cgi-bin/koha/cataloguing/editor.pl#catalog/55555

This is because in the bug 16424, when changes had been made to
koha-tmpl/intranet-tmpl/lib/koha/cateditor/koha-backend.js
(Bug 16424: Add framework support to advanced MARC editor),
the .fail callback method of the outer .get (in the KohaBackend.GetRecord
function called from cateditor-ui.inc) had been (perhaps by mistake?) removed.
So, in case of failure, the situation is not handled properly.

The proposal is to restore the outer .fail.  As a result, the
openRecord( 'new/', editor, finishCb ); will be called (a standard way
of resolving failed openRecord action (cf. the bottom of cateditor-ui.inc).

Test plan:
==========
0. Be sure your browser does not cache JavaScript for this test.
1. Enable EnableAdvancedCatalogingEditor in the system preferences.
2. Try to open in the advanced editor an unexisting record, e.g.
   http://your_ktd:8081/cgi-bin/koha/cataloguing/editor.pl#catalog/55555
   Koha should hang with a message "Loading, please wait".
3. Apply the patch, restart_all.
4. In a new browser window, repeat p. 2 (remember not to cache JS).
5. The editor should open with a new empty record.

Sponsored-by: Ignatianum University in Cracow
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Nick Clemens (kidclamp) 2024-05-14 14:04:28 UTC
It would be nice if we had a warning about not finding the bib, but I think opening instead of hanging is a great improvement. Trvial, passing QA
Comment 5 Phil Ringnalda 2024-05-17 00:26:10 UTC
First step toward a warning is bug 36891.
Comment 6 Katrin Fischer 2024-05-17 08:17:24 UTC
(In reply to Phil Ringnalda from comment #5)
> First step toward a warning is bug 36891.

Is that one enough or should we file another bug here for a proper user message?
Comment 7 Katrin Fischer 2024-05-17 08:37:04 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 8 Phil Ringnalda 2024-05-17 22:51:04 UTC
(In reply to Katrin Fischer from comment #6)
> (In reply to Phil Ringnalda from comment #5)
> > First step toward a warning is bug 36891.
> 
> Is that one enough or should we file another bug here for a proper user
> message?

I'll file one more, once I know I can land that one.
Comment 9 Katrin Fischer 2024-05-20 08:51:28 UTC
(In reply to Phil Ringnalda from comment #8)
> (In reply to Katrin Fischer from comment #6)
> > (In reply to Phil Ringnalda from comment #5)
> > > First step toward a warning is bug 36891.
> > 
> > Is that one enough or should we file another bug here for a proper user
> > message?
> 
> I'll file one more, once I know I can land that one.

Can you explain what you mean? I am not sure about that phrase.
Comment 10 Phil Ringnalda 2024-05-20 14:34:41 UTC
(In reply to Katrin Fischer from comment #9)
> Can you explain what you mean? I am not sure about that phrase.

Sorry. Once bug 36891 reaches Passed QA and I know that it will be possible to have the advanced editor report a invalid bib number to the user, I'll file another bug to have the advanced editor make use of the 404 return created by bug 36891.
Comment 11 Katrin Fischer 2024-05-20 19:05:54 UTC
(In reply to Phil Ringnalda from comment #10)
> (In reply to Katrin Fischer from comment #9)
> > Can you explain what you mean? I am not sure about that phrase.
> 
> Sorry. Once bug 36891 reaches Passed QA and I know that it will be possible
> to have the advanced editor report a invalid bib number to the user, I'll
> file another bug to have the advanced editor make use of the 404 return
> created by bug 36891.

Thx!
Comment 12 Fridolin Somers 2024-05-29 13:14:54 UTC
Pushed to 23.11.x for 23.11.06
Comment 13 Phil Ringnalda 2024-06-27 18:30:54 UTC
(In reply to Phil Ringnalda from comment #10)
> Sorry. Once bug 36891 reaches Passed QA and I know that it will be possible
> to have the advanced editor report a invalid bib number to the user, I'll
> file another bug to have the advanced editor make use of the 404 return
> created by bug 36891.

Bug 37212