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

Implement missing share settings #5622

Merged
merged 1 commit into from
Apr 27, 2023
Merged

Implement missing share settings #5622

merged 1 commit into from
Apr 27, 2023

Conversation

allexzander
Copy link
Contributor

@allexzander allexzander commented Apr 21, 2023 •
edited
Loading

BEFORE

share_settings_before

AFTER

share_settings_after

@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from 49bedc7 to dae5917 Compare April 21, 2023 14:55
claucambra
claucambra requested changes Apr 22, 2023
View reviewed changes
src/gui/filedetails/NCRadioButton.qml Outdated Show resolved Hide resolved
src/gui/filedetails/NCRadioButton.qml Outdated Show resolved Hide resolved
src/gui/filedetails/ShareDetailsPage.qml Outdated Show resolved Hide resolved
src/gui/filedetails/ShareDetailsPage.qml Outdated Show resolved Hide resolved
src/gui/filedetails/ShareDetailsPage.qml Outdated Show resolved Hide resolved
src/gui/sharemanager.cpp Outdated Show resolved Hide resolved
src/gui/filedetails/sharemodel.h Outdated Show resolved Hide resolved
src/gui/sharemanager.cpp Outdated Show resolved Hide resolved
src/gui/sharemanager.h Outdated Show resolved Hide resolved
src/gui/sharemanager.h Outdated Show resolved Hide resolved
@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from 53eec17 to 263f170 Compare April 24, 2023 12:44
@allexzander allexzander requested a review from claucambra April 24, 2023 12:46
@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from c2df823 to 69fd533 Compare April 25, 2023 11:33
@allexzander
Copy link
Contributor Author

/backport to stable-3.8

claucambra
claucambra reviewed Apr 26, 2023
View reviewed changes
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.

A couple of small comments left from my previous review, though they are nitpicks

src/gui/filedetails/NCRadioButton.qml Outdated Show resolved Hide resolved
@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from 69fd533 to 6cd3d4a Compare April 26, 2023 08:49
@allexzander allexzander requested a review from claucambra April 26, 2023 08:53
mgallien
mgallien reviewed Apr 26, 2023
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

mostly questions
sometime the code is a bit hard to follow between what comes from the network jobs and what comes from user actions

src/gui/sharemanager.cpp Show resolved Hide resolved
src/gui/filedetails/sharemodel.cpp Outdated Show resolved Hide resolved
src/gui/filedetails/sharemodel.cpp Outdated Show resolved Hide resolved
src/gui/filedetails/sharemodel.cpp Outdated Show resolved Hide resolved
src/gui/filedetails/sharemodel.cpp Outdated Show resolved Hide resolved
src/gui/filedetails/sharemodel.h Outdated Show resolved Hide resolved
src/gui/filedetails/sharemodel.h Outdated Show resolved Hide resolved
@allexzander allexzander requested a review from mgallien April 26, 2023 10:30
@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from 37cd5dc to d2290c7 Compare April 26, 2023 10:30
mgallien
mgallien reviewed Apr 26, 2023
View reviewed changes
src/gui/filedetails/sharemodel.cpp Outdated
return _isSecureFileDropSupportedFolder && share->getPermissions().testFlag(OCC::SharePermission::SharePermissionCreate);
case CurrentPermissionModeRole: {
if (share->getPermissions() == OCC::SharePermission::SharePermissionCreate) {
return static_cast<int>(SharePermissionsMode::ModeFileDropOnly);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I had missed that in my first review
should be

return QVariand::fromValue(SharePermissionsMode::ModeFileDropOnly);

mgallien
mgallien requested changes Apr 26, 2023
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

see inline comments

src/gui/filedetails/sharemodel.cpp Outdated
if (share->getPermissions() == OCC::SharePermission::SharePermissionCreate) {
return static_cast<int>(SharePermissionsMode::ModeFileDropOnly);
} else if ((share->getPermissions() & SharePermissionRead) && (share->getPermissions() & SharePermissionUpdate)) {
return static_cast<int>(SharePermissionsMode::ModeUploadAndEditing);
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

src/gui/filedetails/sharemodel.cpp Outdated
} else if ((share->getPermissions() & SharePermissionRead) && (share->getPermissions() & SharePermissionUpdate)) {
return static_cast<int>(SharePermissionsMode::ModeUploadAndEditing);
} else {
return static_cast<int>(SharePermissionsMode::ModeViewOnly);
Copy link
Collaborator

Choose a reason for hiding this comment

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

same

@allexzander allexzander requested a review from mgallien April 27, 2023 10:10
mgallien
mgallien approved these changes Apr 27, 2023
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

sounds good
did not do any local tests during review

@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from 6730c44 to 40d26fd Compare April 27, 2023 10:35
@allexzander
Implement missing share settings
f39a090 
Signed-off-by: alex-z <blackslayer4@gmail.com>
@allexzander allexzander force-pushed the feature/bring-back-missing-share-options branch from 40d26fd to f39a090 Compare April 27, 2023 10:55
@sonarcloud
Copy link

sonarcloud bot commented Apr 27, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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

21.8% 21.8% Coverage
0.0% 0.0% Duplication

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5622-f39a0903c5ba88c8df09262349aad38e9dc8244d-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.

@allexzander allexzander merged commit 6eb248b into master Apr 27, 2023
@allexzander allexzander deleted the feature/bring-back-missing-share-options branch April 27, 2023 12:46
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Apr 27, 2023
Merged
@mgallien mgallien added this to the 3.9.0 milestone May 10, 2023
This was referenced Nov 22, 2023
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien approved these changes

@camilasan camilasan Awaiting requested review from camilasan

@claucambra claucambra Awaiting requested review from claucambra

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@allexzander @nextcloud-desktop-bot @mgallien @claucambra