Skip to content

fix: folder delete/new conflict will be "delete"#8671

Merged
mgallien merged 7 commits intomasterfrom
bugfix/improvedFolderDeleteNewConflictSolving
Oct 7, 2025
Merged

fix: folder delete/new conflict will be "delete"#8671
mgallien merged 7 commits intomasterfrom
bugfix/improvedFolderDeleteNewConflictSolving

Conversation

@mgallien
Copy link
Copy Markdown
Collaborator

@mgallien mgallien commented Aug 29, 2025

should enable someone to delete a folder even if there is a new/delete conflict happening for some other users

@mgallien mgallien added this to the 3.18.0 milestone Aug 29, 2025
@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch from a641b68 to 19cd363 Compare September 1, 2025 09:08
@mgallien mgallien marked this pull request as ready for review September 1, 2025 10:37
@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch from 13eed08 to 63e76b6 Compare September 1, 2025 11:19
@mgallien
Copy link
Copy Markdown
Collaborator Author

mgallien commented Sep 1, 2025

so
putting it back as a draft
current solution will not detect the files inside a deleted folder that are local and should be moved to trash

@mgallien mgallien marked this pull request as draft September 1, 2025 12:08
@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch 4 times, most recently from 990edcc to a992edd Compare September 3, 2025 13:26
@mgallien mgallien marked this pull request as ready for review September 3, 2025 13:31
@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch 4 times, most recently from f64ad7b to b58a862 Compare September 5, 2025 15:08
@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch from b58a862 to 39ab518 Compare September 11, 2025 12:15
nilsding
nilsding reviewed Sep 26, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@nilsding nilsding left a comment

Choose a reason for hiding this comment

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

code seems fine to me now (still needs a rebase), but for some reason Linux CI isn't happy about the tests :/

really strange since the tests run fine on my dev machine (/tmp is a tmpfs on my end -- I wonder if that matters here)

nilsding
nilsding reviewed Sep 26, 2025
View reviewed changes
test/testsyncdelete.cpp
@@ -43,11 +43,11 @@ private slots:

// A/a1 must be gone because the directory was removed on the server, but hello.txt must be there
Copy link
Copy Markdown
Member

@nilsding nilsding Sep 26, 2025

Choose a reason for hiding this comment

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

this comment now doesn't match what the test is verifying

@nilsding
Copy link
Copy Markdown
Member

nilsding commented Sep 26, 2025

for some reason Linux CI isn't happy about the tests :/

maybe Qt fails to make use of the freedesktop trash standard in a container?

@nilsding
Copy link
Copy Markdown
Member

nilsding commented Sep 26, 2025

maybe Qt fails to make use of the freedesktop trash standard in a container?

just debugged locally using the same container used in CI
tests started to pass again after manually creating a /home/test/.qttest/share/Trash directory belonging to the test user... bit odd :/

@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch from 39ab518 to 0649018 Compare October 3, 2025 09:04
@mgallien mgallien requested a review from Aiiaiiio as a code owner October 3, 2025 09:04
@mgallien
Copy link
Copy Markdown
Collaborator Author

mgallien commented Oct 6, 2025

working on the broken CI for Windows

@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch 2 times, most recently from 65ed70e to c5dfa75 Compare October 6, 2025 15:56
@mgallien
chore: fix all occurence of range-loop-detach clazy warning
491e7cd 
see https://github.com/KDE/clazy/blob/master/docs/checks/README-range-loop-detach.md

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
fix: folder delete/new conflict will be "delete"
1ec98ee 
should enable someone to delete a folder even if there is a new/delete
conflict happening for some other users

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
feat(propagation): flexible way to configure special actions on items
6cdbf8a 
will enable to perform optional special actions while execution a
propagation action

will enable to move to trash bin or disable server trash bin when
appropriate

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
chore: adjust logs level and clazy warnings for permanent deletions
9d7f865 
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
feat(conflict): use local device trashbin for delete/new conflicts
c4b84a9 
try to use local device trash bin when deleting local items during
automated handling of a new/delete conflict

we still always choose the delete but will do our best to keep a copy of
the deleted files in local device trash bin

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
feat(conflict): use local device trashbin for delete/new conflicts
e9ee060 
try to use local device trash bin when deleting local items during
automated handling of a new/delete conflict

we still always choose the delete but will do our best to keep a copy of
the deleted files in local device trash bin

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien
chore(autotests): folders on-demand breaks the new test: skip it
f02f6c9 
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien mgallien force-pushed the bugfix/improvedFolderDeleteNewConflictSolving branch from c5dfa75 to f02f6c9 Compare October 7, 2025 06:51
@mgallien
Copy link
Copy Markdown
Collaborator Author

mgallien commented Oct 7, 2025

Windows CI is now good again

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 7, 2025

Artifact containing the AppImage: nextcloud-appimage-pr-8671.zip

Digest: sha256:a1990f1fc566b4015546554446695e668e06dafe0657a34c80ad59b9b12115e1

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien enabled auto-merge October 7, 2025 07:07
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 7, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
Image 9 Security Hotspots
Image 60.3% Coverage on New Code (required ≥ 80%)
Image 1 New Bugs (required ≤ 0)
Image E Maintainability Rating on New Code (required ≥ A)
Image E Reliability Rating on New Code (required ≥ A)
Image D Security Rating on New Code (required ≥ A)
Image 604 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Image Catch issues before they fail your Quality Gate with our IDE extension Image SonarQube for IDE

@mgallien mgallien merged commit 2e619ec into master Oct 7, 2025
21 of 22 checks passed
@mgallien mgallien deleted the bugfix/improvedFolderDeleteNewConflictSolving branch October 7, 2025 07:27
@mgallien
Copy link
Copy Markdown
Collaborator Author

mgallien commented Oct 7, 2025

/backport to stable-4.0

@mgallien mgallien modified the milestones: 4.0.0, 4.1.0 Oct 7, 2025
@backportbot backportbot bot mentioned this pull request Oct 7, 2025
Merged
@sndtypsys sndtypsys mentioned this pull request Jan 17, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nilsding nilsding nilsding approved these changes

@camilasan camilasan Awaiting requested review from camilasan camilasan is a code owner

@i2h3 i2h3 Awaiting requested review from i2h3 i2h3 is a code owner

@Aiiaiiio Aiiaiiio Awaiting requested review from Aiiaiiio Aiiaiiio is a code owner

Assignees

No one assigned

Labels

2. developing

Projects

None yet

Milestone

33.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@mgallien @nilsding