Skip to content
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_versions): only handle path updates when there is path #51609

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Mar 20, 2025

Summary

getPathForNode can fail with null for various reasons (e.g. no owner), in this cases we need to just skip the event handling.

Checklist

`getPathForNode` can fail with null for various reasons (e.g. no owner),
in this cases we need to just skip the event handling.

Signed-off-by: Ferdinand Thiessen <[email protected]>
@susnux susnux added this to the Nextcloud 32 milestone Mar 20, 2025
@susnux susnux requested a review from a team as a code owner March 20, 2025 17:46
@susnux susnux requested review from ArtificialOwl, nfebe and come-nc and removed request for a team March 20, 2025 17:46
$view = new View(\OC_User::getUser() . '/files');
if ($view->file_exists($newPath)) {
$view = new View($user . '/files');
if ($view->file_exists($newPath) === true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ($view->file_exists($newPath) === true) {
if ($view->file_exists($newPath)) {

Feels dangerous to do a strong comparison here, what’s the reason?

return;
}

$user = \OC_User::getUser();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😿
What’s the reason for getting the current user here?
$node->getPath should return the absolute path already?
Otherwise, use $this->userSession->getUser() like in getPathForNode, as you do not want to skip rename if coming from a public page where incognito mode is active (which is the only added feature of OC_User)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants