-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(files): handle empty view with error #48625
Conversation
4ebe5a3
to
6dec370
Compare
apps/files/src/views/FilesList.vue
Outdated
if (error instanceof Error) { | ||
const status = ('status' in error && error.status as number) || ('response' in error && (error.response as AxiosError).status) || 0 | ||
if ([400, 404, 405].includes(status)) { | ||
this.error = t('files', 'Folder not found') | ||
} else if (status === 403) { | ||
this.error = t('files', 'This operation is forbidden') | ||
} else if (status === 500) { | ||
this.error = t('files', 'This directory is unavailable, please check the logs or contact the administrator') | ||
} else if (status === 503) { | ||
this.error = t('files', 'Storage is temporarily not available') | ||
} else { | ||
this.error = t('files', 'Unexpected error: {error}', { error: error.message }) | ||
} | ||
} else { | ||
this.error = t('files', 'Unknown error') | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen this code snippet before in our vue migration.
Maybe it should be a composable or mixin instead of a copy ? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a direct copy, but addopted simular code (linked in the PR description).
Moving to an util (not a mixin/composable as this is not a part of Vue component).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to utils
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make sense to move to nextcloud-files
6dec370
to
60cf5a2
Compare
apps/files/src/utils/davUtils.ts
Outdated
/** | ||
* Get the sharing token | ||
* @return {string|null} The sharing token | ||
*/ | ||
export function getToken() { | ||
const tokenElement = document.getElementById('sharingToken') as (HTMLInputElement | null) | ||
return tokenElement?.value | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here also available in @nextcloud/sharing
, also version here only works with legacy ui.
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
9779bd6
to
18d5232
Compare
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Rebased onto master, squashed |
/backport to stable30 |
/backport to stable29 |
The backport to # Switch to the target branch and update it
git checkout stable29
git pull origin stable29
# Create the new backport branch
git checkout -b backport/48625/stable29
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 2696715d 080a8c7f 18d52323 ed9fdbad
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/48625/stable29 Error: No changes found in backport branch Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
Summary
Handles error during content fetching.
In Nextcloud 27 in case of an error it redirected to the
/
root and (sometimes) showed a toast message with an error.server/apps/files/js/filelist.js
Lines 2256 to 2296 in ba452f9
After migrating to Vue, there is no error handling. It just shows a folder as an empty folder.
It is especially confusing when it was supposed to be a valid link:
It looks like the folder is available, just empty.
I found redirecting to the root less convenient to a user in flavor of explicitly showing an error and a button to reload it.
But I'm not sure about the text/design.
Screenshots
Checklist