Fix code-review findings: ajax error handling, translated comment, docs
- js/index.js: the occ-command ajax call used the wrong jQuery .fail() signature ((response, code) instead of (jqXHR, textStatus, errorThrown)), so any failure printed the literal string "[object Object]" instead of a useful message. - js/index.js: the initial bootstrap call that builds the terminal had no .fail() handler at all - a failure there (expired session/CSRF token, network error, rejected admin check) left #app-content permanently blank with zero diagnostic information. This is exactly what made today's NC34 debugging session harder than it needed to be; now it shows the actual HTTP status/reason in the page. - OccController.php: translated a leftover Dutch comment (missed by the earlier Russian->English pass since it wasn't Russian), and documented why requireAdmin() is intentional defense-in-depth rather than a misunderstanding of the framework: verified against Nextcloud's own SecurityMiddleware source that any controller method without @NoAdminRequired already rejects non-admins by default, so this check (and the original "security fix" commit that added it) was addressing a non-issue for THIS controller specifically. DbController's equivalent check is still genuinely load-bearing - that controller originally shipped with @PublicPage, which does bypass the default protection - so it gets a note explaining the distinction instead. - README: added compatibility matrix rows for 0.4.0 (app id rename to extended_occweb) and 0.4.1 (these fixes), both verified on NC 34.0.4. Deferred to when a test instance exists (tracked, not fixed here): no documented migration path from the old occweb id to extended_occweb for existing installs, OccController's constructor doing expensive work before the (redundant) admin check runs, and js/jquery.js loading unconditionally on NC30-33 where core still provides jQuery. Version bump to 0.4.1.
This commit is contained in:
+12
-2
@@ -387,8 +387,8 @@
|
||||
data: JSON.stringify(occCommand)
|
||||
}).done(function (response) {
|
||||
term.echo('\n' + response).resume();
|
||||
}).fail(function (response, code) {
|
||||
term.echo('\n' + response).resume();
|
||||
}).fail(function (xhr, status) {
|
||||
term.echo('\n[[;#ff5555;]Request failed: ]' + $.terminal.escape_formatting(xhr.status + ' ' + xhr.statusText)).resume();
|
||||
});
|
||||
}
|
||||
}, {
|
||||
@@ -410,6 +410,16 @@
|
||||
scrollToBottom()
|
||||
}
|
||||
});
|
||||
}).fail(function (xhr, status) {
|
||||
// Without this, a failure here (expired session/CSRF token, network
|
||||
// error, a rejected admin check) leaves #app-content permanently
|
||||
// empty with no visible error and nothing in the console pointing
|
||||
// at the cause - exactly what happened during NC34 testing before
|
||||
// this handler existed.
|
||||
$('#app-content').text(
|
||||
'Failed to load the terminal: ' + xhr.status + ' ' + xhr.statusText +
|
||||
'. Try reloading the page; if that keeps happening, check the server logs.'
|
||||
);
|
||||
});
|
||||
$('html').keypress(function(){
|
||||
scrollToBottom()
|
||||
|
||||
Reference in New Issue
Block a user