Feature/sync with case clash names #5232

mgallien · 2022-12-01T14:30:38Z

#5099

claucambra

Just some small nitpicks, please clean up history when done :)

src/libsync/discovery.cpp

src/libsync/owncloudpropagator.cpp

src/libsync/propagatedownload.cpp

claucambra · 2022-12-01T15:39:49Z

The sync engine test has crashed in the Windows pipeline, unsure if this is related to the changes

tobiasKaminsky · 2022-12-02T15:40:19Z

--> there should be a button "resolve conflict", so that it is better visible to the user how to fix this.

codecov · 2022-12-15T12:00:28Z

Codecov Report

Merging #5232 (94605d2) into master (5c42da4) will increase coverage by 0.24%.
The diff coverage is 73.49%.

❗ Current head 94605d2 differs from pull request most recent head fbb0a33. Consider uploading reports for the commit fbb0a33 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5232      +/-   ##
==========================================
+ Coverage   58.06%   58.31%   +0.24%     
==========================================
  Files         139      141       +2     
  Lines       17710    17990     +280     
==========================================
+ Hits        10283    10490     +207     
- Misses       7427     7500      +73

Impacted Files	Coverage Δ
src/common/preparedsqlquerymanager.h	`0.00% <ø> (ø)`
src/common/syncjournaldb.h	`66.66% <ø> (ø)`
src/common/utility.h	`0.00% <ø> (ø)`
src/csync/csync.h	`75.00% <ø> (ø)`
src/csync/csync_exclude.h	`0.00% <ø> (ø)`
src/libsync/discoveryphase.h	`25.00% <ø> (ø)`
src/libsync/owncloudpropagator.h	`66.12% <ø> (-2.53%)`	⬇️
src/libsync/progressdispatcher.cpp	`53.80% <0.00%> (-0.32%)`	⬇️
src/libsync/syncengine.h	`50.00% <ø> (ø)`
src/libsync/caseclashconflictsolver.cpp	`42.73% <42.73%> (ø)`
... and 14 more

CMakeLists.txt

allexzander · 2023-01-05T15:11:51Z

@matthieugallien Could you add a short video of how this feature works?

src/common/filesystembase.cpp

src/csync/csync_exclude.cpp

src/common/syncjournaldb.h

src/common/utility.cpp

src/libsync/caseclashconflictsolver.cpp

src/libsync/caseclashconflictsolver.h

claucambra · 2023-01-11T16:40:36Z

src/libsync/owncloudpropagator.cpp

@@ -913,6 +913,63 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item,
    return true;
 }

+OCC::Optional<QString> OwncloudPropagator::createCaseClashConflict(const SyncFileItemPtr &item, const QString &temporaryDownloadedFile)
+{
+    auto filename = QString{};


I don't think the auto is particularly helpful here

Suggested change

auto filename = QString{};

QString filename;

I disagree
I do not see a good reason not to use it
I have always find that almost always auto is a good rule
we should have auto everywhere unless there is a reason it cannot be used

claucambra · 2023-01-11T16:41:29Z

src/libsync/owncloudpropagator.cpp

+            status = SyncFileItem::NormalError;
+        } else {
+            status = SyncFileItem::FileIgnored;
+            ASSERT(_item->_instruction == CSYNC_INSTRUCTION_IGNORE);


Why not a Q_ASSERT?

the ASSERT macro will also print a message in release mode if it fails

src/libsync/propagatedownload.cpp

claucambra

Merge commit probably needs to be removed before merging this PR, otherwise looks good

introduce a new type of conflict for case clash filename conflicts add proper handling including a new utility class to solve them and a new dialog for the user to pick a fix Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

…ution dialog Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

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

mgallien · 2023-01-25T15:38:21Z

Merge commit probably needs to be removed before merging this PR, otherwise looks good

done
thanks for the notice

nextcloud-desktop-bot · 2023-01-25T15:44:35Z

AppImage file: nextcloud-PR-5232-fbb0a33eb702f7518ceb3fd79699488843ce3c05-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.

sonarcloud · 2023-01-25T16:08:07Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
74 Code Smells

13.2% Coverage
2.7% Duplication

mgallien requested review from camilasan, allexzander and claucambra December 1, 2022 14:30

mgallien force-pushed the feature/syncWithCaseClashNames branch from 40fd992 to 0c4d898 Compare December 1, 2022 14:44

claucambra requested changes Dec 1, 2022

View reviewed changes

mgallien force-pushed the feature/syncWithCaseClashNames branch 2 times, most recently from 0351873 to fe7e992 Compare December 2, 2022 08:27

mgallien force-pushed the feature/syncWithCaseClashNames branch from f27a5b1 to 38daad6 Compare December 9, 2022 11:15

mgallien added this to the 3.6.4 milestone Dec 12, 2022

mgallien linked an issue Dec 14, 2022 that may be closed by this pull request

Allow syncing file with case clash names #5099

Closed

claucambra force-pushed the feature/syncWithCaseClashNames branch from 9640e87 to 42884f5 Compare December 14, 2022 19:27

mgallien removed this from the 3.6.4 milestone Dec 15, 2022

mgallien force-pushed the feature/syncWithCaseClashNames branch from 42884f5 to 6939441 Compare December 15, 2022 11:29

mgallien force-pushed the feature/syncWithCaseClashNames branch 8 times, most recently from db8928d to f4f2987 Compare December 21, 2022 00:01

mgallien force-pushed the feature/syncWithCaseClashNames branch 2 times, most recently from f333549 to 5321f9a Compare January 3, 2023 08:27

allexzander reviewed Jan 5, 2023

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

allexzander reviewed Jan 5, 2023

View reviewed changes

src/common/filesystembase.cpp Outdated Show resolved Hide resolved

allexzander reviewed Jan 5, 2023

View reviewed changes

src/csync/csync_exclude.cpp Outdated Show resolved Hide resolved

mgallien force-pushed the feature/syncWithCaseClashNames branch 2 times, most recently from 3172ff1 to 4d60f75 Compare January 11, 2023 15:41

claucambra reviewed Jan 11, 2023

View reviewed changes

mgallien force-pushed the feature/syncWithCaseClashNames branch from 4d60f75 to 7cebb6a Compare January 24, 2023 21:36

mgallien requested review from claucambra and allexzander January 24, 2023 21:39

allexzander approved these changes Jan 25, 2023

View reviewed changes

claucambra approved these changes Jan 25, 2023

View reviewed changes

mgallien and others added 14 commits January 25, 2023 16:37

Display action button for conflict files in activity

5908ac4

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

Clarify letter casing message in case clash conflict dialog

cf6794c

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

Fix case clash dialog sizing issues

2359c80

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

Display both files and some metadata in the case clash conflict resol…

64e585e

…ution dialog Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

Improve wording in the case clash resolution dialog

4c4e870

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>

move some void returning methods to be slots

9a9e73c

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

simplify code

8dee66f

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

remove some not needed debug logs

7091b7f

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

simplify code

b028024

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

fix wrong code style

3b6e555

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

fix copyright notice of new files

07c292e

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

use #pragma once instead of include guards

eb9a496

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

use auto

fbb0a33

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

mgallien force-pushed the feature/syncWithCaseClashNames branch from 94605d2 to fbb0a33 Compare January 25, 2023 15:37

mgallien merged commit aa74448 into master Jan 26, 2023

mgallien deleted the feature/syncWithCaseClashNames branch January 26, 2023 08:05

mgallien added this to the 3.7.0 milestone Jan 30, 2023

mgallien mentioned this pull request Jun 15, 2023

Issues with special chars on Windows and other operating systems #464

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/sync with case clash names #5232

Feature/sync with case clash names #5232

mgallien commented Dec 1, 2022 •

edited

Loading

claucambra left a comment •

edited

Loading

claucambra commented Dec 1, 2022

tobiasKaminsky commented Dec 2, 2022

codecov bot commented Dec 15, 2022 •

edited

Loading

allexzander commented Jan 5, 2023

claucambra Jan 11, 2023

mgallien Jan 24, 2023

claucambra Jan 11, 2023

mgallien Jan 24, 2023

claucambra left a comment •

edited

Loading

mgallien commented Jan 25, 2023

nextcloud-desktop-bot commented Jan 25, 2023

sonarcloud bot commented Jan 25, 2023

Feature/sync with case clash names #5232

Feature/sync with case clash names #5232

Conversation

mgallien commented Dec 1, 2022 • edited Loading

claucambra left a comment • edited Loading

Choose a reason for hiding this comment

claucambra commented Dec 1, 2022

tobiasKaminsky commented Dec 2, 2022

codecov bot commented Dec 15, 2022 • edited Loading

Codecov Report

allexzander commented Jan 5, 2023

claucambra Jan 11, 2023

Choose a reason for hiding this comment

mgallien Jan 24, 2023

Choose a reason for hiding this comment

claucambra Jan 11, 2023

Choose a reason for hiding this comment

mgallien Jan 24, 2023

Choose a reason for hiding this comment

claucambra left a comment • edited Loading

Choose a reason for hiding this comment

mgallien commented Jan 25, 2023

nextcloud-desktop-bot commented Jan 25, 2023

sonarcloud bot commented Jan 25, 2023

mgallien commented Dec 1, 2022 •

edited

Loading

claucambra left a comment •

edited

Loading

codecov bot commented Dec 15, 2022 •

edited

Loading

claucambra left a comment •

edited

Loading