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.

Sign up for GitHub

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

Jump to bottom

[full-ci] Restore files: Implement conflict dialog #7635

Merged
merged 39 commits into from
Oct 26, 2022
Merged

[full-ci] Restore files: Implement conflict dialog #7635

merged 39 commits into from
Oct 26, 2022

Conversation

lookacat
Copy link
Contributor

@lookacat lookacat commented Sep 12, 2022 •
edited
Loading

Description

See #1753

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added

@update-docs
Copy link

update-docs bot commented Sep 12, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@lookacat lookacat force-pushed the restore-conflict-dialog branch 2 times, most recently from 37025eb to 4e8b057 Compare September 15, 2022 14:01
@lookacat lookacat marked this pull request as ready for review September 15, 2022 14:31
@ownclouders
Copy link
Contributor

ownclouders commented Sep 16, 2022 •
edited
Loading

Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/28935/31/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUITrashbinRestore-trashbinRestore_feature-L138.png

webUITrashbinRestore-trashbinRestore_feature-L138.png

webUITrashbinRestore-trashbinRestore_feature-L173.png

webUITrashbinRestore-trashbinRestore_feature-L173.png

@lookacat lookacat changed the title Restore files: Implement conflict dialog [full-ci] Restore files: Implement conflict dialog Sep 16, 2022
@lookacat lookacat force-pushed the restore-conflict-dialog branch from 387863d to 753c521 Compare September 16, 2022 14:15
@kulmann kulmann requested a review from JammingBen September 20, 2022 08:09
JammingBen
JammingBen reviewed Sep 20, 2022
View reviewed changes
Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

I'd like to try it, but I always get an error on restoring files: TypeError: Cannot read properties of undefined (reading 'includes') in this line: const overwrite = filesToOverwrite.includes(resource).

@lookacat
Copy link
Contributor Author

sorry that it didnt worked, im confused too as it did work before :O will look into it

@lookacat lookacat requested a review from JammingBen September 20, 2022 10:06
@lookacat lookacat force-pushed the restore-conflict-dialog branch from 2cf8658 to f66b469 Compare October 6, 2022 07:42
@lookacat lookacat requested a review from dschmidt October 10, 2022 21:14
@JammingBen JammingBen mentioned this pull request Oct 11, 2022
Closed
9 tasks
@JammingBen
Copy link
Contributor

The Replace strategy does not work for me. The file disappears in the trashbin after clicking "Replace", but it re-appears after a page reload. Also, no new version gets created.

#1753 is not fixed by this, right? It still gives me an error.

@lookacat
Copy link
Contributor Author

@JammingBen thats weird, i will look into it. But yes the bug ticket will be fixed by this PR.

@lookacat lookacat force-pushed the restore-conflict-dialog branch from 94e256a to 0219618 Compare October 11, 2022 11:13
dschmidt
dschmidt reviewed Oct 12, 2022
View reviewed changes
Copy link
Member

@dschmidt dschmidt left a comment

Choose a reason for hiding this comment

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

Just a few more or less nitpicky comments

packages/web-app-files/tests/unit/mixins/actions/restore.spec.js
return []
})
}
},
Copy link
Member

Choose a reason for hiding this comment

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

I understand if you don't want to port everything to the defaultComponentMocks, but you could mock the whole clientService like this:

web/tests/unit/mocks/defaultComponentMocks.ts

Line 20 in 3808ee0

$clientService: mockDeep<ClientService>(),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doesnt work 😬

Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt that it just doesn't work 👀 any specific reason why it doesn't work?

kulmann
kulmann suggested changes Oct 12, 2022
View reviewed changes
@lookacat lookacat force-pushed the restore-conflict-dialog branch 3 times, most recently from 247697f to 97465d0 Compare October 20, 2022 09:15
@lookacat lookacat requested review from kulmann and dschmidt October 20, 2022 09:35
@lookacat lookacat force-pushed the restore-conflict-dialog branch 2 times, most recently from 100b8b8 to b754085 Compare October 20, 2022 11:59
lookacat and others added 21 commits October 25, 2022 11:43
@lookacat @kulmann
Fix restore overwrite / keep both
5bc0670
@lookacat @kulmann
Recreate folder structure in case parent folder doesnt exist
22670a7
@lookacat @kulmann
Fix unittests
f0c826d
@lookacat @kulmann
Update expected failures
2e9a3a6
@lookacat @kulmann
Use clientService webdav
555c6f6
@lookacat @kulmann
Removed bug demonstration, linting
d5d65dd
@lookacat @kulmann
Fix unittests
a5d5474
@lookacat @kulmann
Remove bug/fixed expected failure
2c5aebf
@lookacat @kulmann
Fix listFiles parameter
295efdb
@lookacat @kulmann
Fix unittest
19b09b4
@lookacat @kulmann
Fix linting
3f896f4
@lookacat @kulmann
Address PR issues
4ad3ee8
@lookacat @kulmann
Fix linting, unittests
233b1f8
@lookacat @kulmann
Refactor into RestoreFileFactory
9df9954
@lookacat @kulmann
Fix space restore file, linting, unittests
cb81225
@lookacat @kulmann
Fix imports
3bb47af
@lookacat @kulmann
Refactor into functions, fix failing imports in unittests
de16129
@kulmann
fix: utilize space in restoreFile
d660f2d
@kulmann
fix imports, delete unused code
f2d4b96
@kulmann
fix: restore files in sub folders
1f4b21d 
Path was built without a slash between parent path and file name
@lookacat @kulmann
Sort by path length, check further if reconstruction is needd
ceaa124
@kulmann kulmann force-pushed the restore-conflict-dialog branch from 95049ca to ceaa124 Compare October 25, 2022 09:48
@kulmann kulmann force-pushed the restore-conflict-dialog branch from faaeccc to 27b90dc Compare October 26, 2022 12:23
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

44.2% 44.2% Coverage
0.0% 0.0% Duplication

@kulmann kulmann merged commit 6d0d57d into master Oct 26, 2022
@delete-merged-branch delete-merged-branch bot deleted the restore-conflict-dialog branch October 26, 2022 13:22
@micbar micbar mentioned this pull request Nov 25, 2022
Closed
73 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann approved these changes

@AlexAndBear AlexAndBear Awaiting requested review from AlexAndBear

@JammingBen JammingBen Awaiting requested review from JammingBen

@dschmidt dschmidt Awaiting requested review from dschmidt

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@lookacat @ownclouders @JammingBen @dschmidt @kulmann