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 renaming of folders with a deep hierarchy inside them #5182

Merged
merged 7 commits into from
Nov 18, 2022

Conversation

mgallien
Copy link
Collaborator

currently broken test as we miss to rename recursively

Signed-off-by: Matthieu Gallien matthieu.gallien@nextcloud.com

@mgallien mgallien changed the title test for proper renaming of folder parent of a deep hierarchy fix renaming of folders with a deep hierarchy inside them Nov 16, 2022
@mgallien mgallien force-pushed the bugfix/fixRenameDeepHierarchy branch from 30badcc to 7a1c47d Compare November 17, 2022 08:43
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
currently broken test as we miss to rename recursively

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
propagate renaming of parent folders to child recursively

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
use QString::arg with multiple parameters because that is faster

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien mgallien force-pushed the bugfix/fixRenameDeepHierarchy branch from 7a1c47d to 2742c6a Compare November 17, 2022 13:56
@mgallien mgallien marked this pull request as ready for review November 17, 2022 13:56
@mgallien
Copy link
Collaborator Author

@claucambra thanks for initial review, can you have another look at it ?

Copy link
Collaborator

@claucambra claucambra left a comment

Choose a reason for hiding this comment

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

Looks good, I provided some nitpicks below

src/libsync/syncengine.cpp Outdated Show resolved Hide resolved
src/libsync/propagatorjobs.cpp Outdated Show resolved Hide resolved
src/libsync/propagatorjobs.cpp Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Merging #5182 (ad32bc7) into master (01f8003) will increase coverage by 0.11%.
The diff coverage is 85.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5182      +/-   ##
==========================================
+ Coverage   57.59%   57.71%   +0.11%     
==========================================
  Files         138      138              
  Lines       17399    17441      +42     
==========================================
+ Hits        10021    10066      +45     
+ Misses       7378     7375       -3     
Impacted Files Coverage Δ
src/libsync/discovery.h 59.25% <ø> (+2.11%) ⬆️
src/libsync/discoveryphase.h 25.00% <ø> (ø)
src/libsync/propagatorjobs.h 0.00% <ø> (ø)
src/libsync/syncengine.cpp 83.33% <25.00%> (-0.38%) ⬇️
src/libsync/propagatorjobs.cpp 75.00% <85.41%> (+1.76%) ⬆️
src/libsync/discovery.cpp 88.24% <100.00%> (-0.09%) ⬇️
src/common/syncjournaldb.cpp 77.93% <0.00%> (+0.29%) ⬆️
src/libsync/filesystem.cpp 74.19% <0.00%> (+1.07%) ⬆️
... and 2 more

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@sonarcloud
Copy link

sonarcloud bot commented Nov 18, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 13 Code Smells

82.4% 82.4% Coverage
0.0% 0.0% Duplication

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5182-ad32bc7ce55c01e17f7063860a4dcd6f9205b419-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

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

@mgallien mgallien merged commit 4135b75 into master Nov 18, 2022
@mgallien mgallien deleted the bugfix/fixRenameDeepHierarchy branch November 18, 2022 10:43
@mgallien mgallien added this to the 3.7.0 milestone Nov 18, 2022
@mgallien
Copy link
Collaborator Author

/backport to stable-3.6

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

Successfully merging this pull request may close these issues.

3 participants