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 populating user drive and drives #5426

Merged
merged 3 commits into from
Jan 26, 2023
Merged

fix populating user drive and drives #5426

merged 3 commits into from
Jan 26, 2023

Conversation

butonic
Copy link
Member

@butonic butonic commented Jan 19, 2023

continuation of #5421

The personal drive should not be part of the drives list.

And when decomposedfs supports the space owner filter we should use that instead of a user + space type "personal" filter, but not part of this PR.

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@update-docs
Copy link

update-docs bot commented Jan 19, 2023

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.

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic self-assigned this Jan 19, 2023
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic marked this pull request as ready for review January 19, 2023 09:47
@ownclouders
Copy link
Contributor

ownclouders commented Jan 19, 2023

💥 Acceptance test Core-API-Tests-ocis-storage-3 failed. Further test are cancelled...

@micbar
Copy link
Contributor

micbar commented Jan 19, 2023

@butonic please double check this with the client platforms.

@sonarcloud
Copy link

sonarcloud bot commented Jan 19, 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 1 Code Smell

95.2% 95.2% Coverage
0.0% 0.0% Duplication

@butonic butonic requested review from micbar and rhafer and removed request for rhafer January 19, 2023 14:30
@butonic
Copy link
Member Author

butonic commented Jan 25, 2023

clients did not report to expect problems with this

Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

Just clarification needed. Rest looks good 👍

Comment on lines +340 to +347
if d.GetDriveType() == "personal" && sp.GetOwner().GetId().GetOpaqueId() == user.GetId() {
if listDrive {
user.Drive = d
}
} else {
drives = append(drives, *d)
user.Drives = drives
if listDrives {
user.Drives = append(user.Drives, *d)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for clarification: This means that personal spaces do not appear in the drives list. I can only get them as single drive. Is that the intended behaviour?

Copy link
Member Author

@butonic butonic Jan 26, 2023

Choose a reason for hiding this comment

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

Yes, there is always only one personal drive you are the owner of: /me/drive

As a secretary you might get access to other personal drives but they will only show up in /me/drives.

The admin might create another drive that is owned by you. I would still use type 'project' for that. We should reserve type 'personal' only for the users personal drive.

@butonic butonic merged commit aa12a60 into master Jan 26, 2023
@delete-merged-branch delete-merged-branch bot deleted the populate-drive branch January 26, 2023 10:54
ownclouders pushed a commit that referenced this pull request Jan 26, 2023
Author: Jörn Friedrich Dreyer <[email protected]>
Date:   Thu Jan 26 11:54:32 2023 +0100

    fix populating user drive and drives (#5426)

    * fix populating user drive and drives

    Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

    * update changelog

    Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

    * fix test condition

    Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

    Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@micbar micbar mentioned this pull request May 3, 2023
89 tasks
@ScharfViktor ScharfViktor mentioned this pull request May 4, 2023
86 tasks
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.

4 participants