Bug 24424

Summary: Advanced editor - interface hangs as "Loading" when given an invalid bib number
Product: Koha Reporter: Cori Lynn Arnold <carnold>
Component: CatalogingAssignee: Janusz Kaczmarek <januszop>
Status: Pushed to stable --- QA Contact: Nick Clemens (kidclamp) <nick>
Severity: trivial    
Priority: P5 - low CC: ere.maijala, fridolin.somers, januszop, lauren_denny, m.de.rooy, martin.renvoize, nick, phil
Version: Main   
Hardware: All   
OS: All   
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
Bug Depends on: 16424, 29697    
Bug Blocks: 37212    
Attachments: Bug 24424: Advanced editor - interface hangs as "Loading" when given an invalid bib number
Bug 24424: Advanced editor - interface hangs as "Loading" when given an invalid bib number

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