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

Spaces migration #10019

Merged
merged 2 commits into from
Apr 4, 2023
Merged

Spaces migration #10019

merged 2 commits into from
Apr 4, 2023

Conversation

TheOneRing
Copy link
Member

No description provided.

@TheOneRing TheOneRing added this to the 3.0 milestone Aug 11, 2022
@TheOneRing TheOneRing requested review from dragotin, labkode and a team August 11, 2022 09:18
TheOneRing
TheOneRing commented Aug 11, 2022
View reviewed changes
test/testspacesmigration/testspacesmigration.cpp
QCOMPARE(folder3->remotePath(), QStringLiteral("/"));

// The shares folder was not migrated
// TODO: should shares be disabled by the migration?
Copy link
Member Author

Choose a reason for hiding this comment

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

Should in this case the server rather ask to disable the folder instead of sending an error?

TheOneRing
TheOneRing commented Aug 11, 2022
View reviewed changes
src/libsync/capabilities.cpp
@@ -29,6 +29,7 @@ Capabilities::Capabilities(const QVariantMap &capabilities)
, _tusSupport(_capabilities.value(QStringLiteral("files")).toMap().value(QStringLiteral("tus_support")).toMap())
, _spaces(_capabilities.value(QStringLiteral("spaces")).toMap())
, _status(_capabilities.value(QStringLiteral("core")).toMap().value(QStringLiteral("status")).toMap())
, _migration(_capabilities.value(QStringLiteral("migration")).toMap())
Copy link
Member Author

Choose a reason for hiding this comment

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

Is the capability ok?

@TheOneRing TheOneRing linked an issue Aug 11, 2022 that may be closed by this pull request
[Spaces] [Desktop] OCIS Spaces Migration API #9945
Closed
TheOneRing
TheOneRing commented Aug 11, 2022
View reviewed changes
test/testspacesmigration/migrationpayload.json
@@ -0,0 +1,19 @@
{
"folders": {
"/": {
Copy link
Member Author

Choose a reason for hiding this comment

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

In difference to the first proposal in owncloud/ocis#3574 (comment) each folder has an object and not an array.

@TheOneRing TheOneRing force-pushed the work/spacemigration branch 2 times, most recently from bd5fe29 to 5001de2 Compare August 11, 2022 10:28
@fmoc fmoc force-pushed the work/spacemigration branch 2 times, most recently from ef05a4f to 83915c9 Compare September 9, 2022 12:21
@sonarcloud
Copy link

sonarcloud bot commented Sep 9, 2022

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 4 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

ownclouders commented Sep 9, 2022 •
edited
Loading

Results for GUI-tests https://drone.owncloud.com/owncloud/client/13644/6/1

💥 The GUI tests failed.

GUI Logs: https://cache.owncloud.com/public/owncloud/client/13644/guiReportUpload/index.html

Server Logs: https://cache.owncloud.com/public/owncloud/client/13644/guiReportUpload/serverlog.log

Screenshots:

@michaelstingl
Copy link
Contributor

@TheOneRing @fmoc can we merge it "as is" for 3.0 ?

@michaelstingl michaelstingl added the p2-high Escalation, on top of current planning, release blocker label Nov 9, 2022
@TheOneRing TheOneRing changed the base branch from master to 3.0 November 10, 2022 15:50
@owncloud owncloud deleted a comment from CLAassistant Nov 10, 2022
@TheOneRing TheOneRing marked this pull request as ready for review November 10, 2022 16:00
fmoc
fmoc suggested changes Nov 15, 2022
View reviewed changes
Copy link
Contributor

@fmoc fmoc left a comment

Choose a reason for hiding this comment

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

Looks okay to me, although I'm still a bit objected to merging this without being able to test this on a real server... Please see my remarks.

src/gui/accountstate.h Show resolved Hide resolved
src/gui/spaces/CMakeLists.txt Outdated Show resolved Hide resolved
src/gui/spacemigration.cpp Outdated Show resolved Hide resolved
src/gui/spacemigration.h Outdated Show resolved Hide resolved
src/gui/spacemigration.cpp Outdated Show resolved Hide resolved
src/gui/spacemigration.cpp Outdated
if (status == 200 && job->parseError().error == QJsonParseError::NoError) {
migrate(job->data().value(QStringLiteral("folders")).toObject());
} else {
qCInfo(lcMigration) << status << job->data();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not be info I guess. A little explanation what this message is about would help, too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not much to log here, and the logging category should provide enough context.

src/gui/spacemigration.cpp Show resolved Hide resolved
src/gui/spacemigration.cpp Outdated Show resolved Hide resolved
src/gui/spacemigration.cpp Outdated Show resolved Hide resolved
src/gui/spacemigration.cpp Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Nov 16, 2022

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 4 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TheOneRing TheOneRing marked this pull request as draft November 16, 2022 16:14
@TheOneRing TheOneRing modified the milestones: 3.0, 4.0 Nov 16, 2022
@TheOneRing TheOneRing changed the base branch from 3.0 to master March 21, 2023 14:34
@owncloud owncloud deleted a comment from CLAassistant Mar 21, 2023
@ownclouders
Copy link
Contributor

ownclouders commented Mar 21, 2023 •
edited
Loading

Results for GUI-tests-oc10 https://drone.owncloud.com/owncloud/client/14798/6/1

💥 The GUI tests failed.

GUI Logs: https://cache.owncloud.com/public/owncloud/client/14798/oc10/guiReportUpload/index.html

Server Logs: https://cache.owncloud.com/public/owncloud/client/14798/oc10/guiReportUpload/serverlog.log

Screenshots:

@TheOneRing TheOneRing requested a review from fmoc March 21, 2023 15:36
@TheOneRing TheOneRing marked this pull request as ready for review March 21, 2023 15:37
@TheOneRing TheOneRing force-pushed the work/spacemigration branch 2 times, most recently from 1e9a793 to 0e49966 Compare March 22, 2023 09:25
@TheOneRing TheOneRing mentioned this pull request Mar 22, 2023
Merged
fmoc
fmoc suggested changes Apr 4, 2023
View reviewed changes
src/gui/accountstate.cpp Outdated Show resolved Hide resolved
src/gui/spacemigration.h Show resolved Hide resolved
test/testspacesmigration/testspacesmigration.cpp Outdated Show resolved Hide resolved
@TheOneRing TheOneRing requested a review from fmoc April 4, 2023 08:57
@sonarcloud
Copy link

sonarcloud bot commented Apr 4, 2023

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 6 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TheOneRing TheOneRing merged commit 0f36464 into master Apr 4, 2023
@delete-merged-branch delete-merged-branch bot deleted the work/spacemigration branch April 4, 2023 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmoc fmoc fmoc approved these changes

@dragotin dragotin Awaiting requested review from dragotin

@labkode labkode Awaiting requested review from labkode

Assignees
No one assigned
Labels
Early-Adopter:CERN p2-high Escalation, on top of current planning, release blocker
Projects
None yet
Milestone
Desktop Client 4.0
Development

Successfully merging this pull request may close these issues.

[Spaces] [Desktop] OCIS Spaces Migration API
4 participants
@TheOneRing @ownclouders @michaelstingl @fmoc