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: Make sure that rollback hook is triggered on all version backends #36690

Merged
merged 4 commits into from
Apr 14, 2023

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Feb 13, 2023

Summary

Alternative to nextcloud/groupfolders#2260 to apply to any version backend.

Checklist

@juliusknorr
Copy link
Member Author

Will have a look into the failing tests...

@juliusknorr juliusknorr force-pushed the bugfix/noid/rollback-hook branch from 6f7fe63 to e6317dd Compare February 14, 2023 11:40
$userFolder = $this->rootFolder->getUserFolder($user->getUID());
$nodes = $userFolder->getById($file->getId());
$file2 = array_pop($nodes);
$userFolder = $this->rootFolder->getUserFolder($user->getUID());

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getUID on possibly null value
$versionEntity->setTimestamp($file2->getMTime());
$versionEntity->setSize($file2->getSize());
$versionEntity->setMimetype($this->mimeTypeLoader->getId($file2->getMimetype()));
$versionEntity->setFileId($file->getId());

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of setFileId cannot be null, possibly null value provided
$versionEntity->setMetadata([]);
$this->versionsMapper->insert($versionEntity);

// Insert entries in the DB for existing versions.
$versionsOnFS = Storage::getVersions($user->getUID(), $userFolder->getRelativePath($file2->getPath()));
$versionsOnFS = Storage::getVersions($user->getUID(), $userFolder->getRelativePath($file->getPath()));

Check notice

Code scanning / Psalm

PossiblyUndefinedVariable

Possibly undefined variable $userFolder, first seen on line 73
foreach ($versionsOnFS as $version) {
$versionEntity = new VersionEntity();
$versionEntity->setFileId($file2->getId());
$versionEntity->setFileId($file->getId());

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of setFileId cannot be null, possibly null value provided
@icewind1991 icewind1991 force-pushed the bugfix/noid/rollback-hook branch from 771cd47 to 5f478a9 Compare March 10, 2023 18:52
$nodes = $userFolder->getById($file->getId());
$file2 = array_pop($nodes);
$userFolder = $this->rootFolder->getUserFolder($user->getUID());
$nodes = $userFolder->getById($file->getId());

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of OCP\Files\Folder::getById cannot be null, possibly null value provided
@juliusknorr juliusknorr requested a review from artonge March 13, 2023 09:35
juliusknorr and others added 4 commits March 13, 2023 10:52
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr force-pushed the bugfix/noid/rollback-hook branch from 5f478a9 to f00f424 Compare March 13, 2023 09:59
@juliusknorr
Copy link
Member Author

Thanks a lot @icewind1991 💙

@juliusknorr juliusknorr requested a review from mejo- March 15, 2023 09:02
@juliusknorr juliusknorr added this to the Nextcloud 27 milestone Mar 15, 2023
@juliusknorr
Copy link
Member Author

@icewind1991 Kindly also asking for a review :)

@ghost
Copy link

ghost commented Mar 28, 2023

Hello, Is there any updates on this. We created a ticket with onlyoffice regarding this since we're enterprise user with them. Looks like there was a sugestion fix via nextcloud/groupfolders#2260 Can Nextcloud Devs look into this as our users with onlyofice are effected by this. Thank you!

@juliusknorr
Copy link
Member Author

@AndyXheli Please make sure to reach out through support if this is affecting enterprise customers ;)

@juliusknorr juliusknorr requested review from artonge and come-nc and removed request for blizzz and mejo- April 3, 2023 14:38
@juliusknorr
Copy link
Member Author

Sorry accidentally cleared the review @artonge Nothing changed since back then.

@juliusknorr juliusknorr merged commit 2e1a560 into master Apr 14, 2023
@juliusknorr juliusknorr deleted the bugfix/noid/rollback-hook branch April 14, 2023 15:09
@juliusknorr
Copy link
Member Author

/backport to stable26

@juliusknorr
Copy link
Member Author

/backport to stable25

@juliusknorr
Copy link
Member Author

/backport to stable24

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin/stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable25
git pull origin/stable25

# Create the new backport branch
git checkout -b fix/foo-stable25

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable24
git pull origin/stable24

# Create the new backport branch
git checkout -b fix/foo-stable24

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable24

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@solracsf
Copy link
Member

/backport to stable26

@solracsf
Copy link
Member

/backport to stable25

artonge added a commit that referenced this pull request May 25, 2023
Broken after #36690

Signed-off-by: Louis Chemineau <louis@chmn.me>
artonge added a commit that referenced this pull request May 25, 2023
Broken after #36690

Signed-off-by: Louis Chemineau <louis@chmn.me>
artonge added a commit that referenced this pull request May 30, 2023
Broken after #36690

Signed-off-by: Louis Chemineau <louis@chmn.me>
artonge added a commit that referenced this pull request May 30, 2023
Broken after #36690

Signed-off-by: Louis Chemineau <louis@chmn.me>
nextcloud-command pushed a commit that referenced this pull request May 31, 2023
Broken after #36690

Signed-off-by: Louis Chemineau <louis@chmn.me>
nextcloud-command pushed a commit that referenced this pull request Jun 5, 2023
Broken after #36690

Signed-off-by: Louis Chemineau <louis@chmn.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants