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
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.
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
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>
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
First step toward a warning is bug 36891.
(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?
Pushed for 24.05! Well done everyone, thank you!
(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.
(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.
(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.
(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!
Pushed to 23.11.x for 23.11.06
(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
Does this need to be backported to 23.05.x?
(In reply to Wainui Witika-Park from comment #14) > Does this need to be backported to 23.05.x? Yes, please. Back to 22.11 should be affected.
(In reply to Phil Ringnalda from comment #15) > (In reply to Wainui Witika-Park from comment #14) > > Does this need to be backported to 23.05.x? > > Yes, please. Back to 22.11 should be affected. Sorry, 23.05 is no longer being maintained https://wiki.koha-community.org/wiki/Roles_for_25.05