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

add commands to list shares and set share owner #38489

Closed
wants to merge 1 commit into from

Conversation

icewind1991
Copy link
Member

  • Add command to list shares with filter option
  • Add command to set share owner

Use case is for setups where multiple users have access to the same set of files (group folders/external storage/etc) and the admins wants to transfer the shares from a user that is being deleted to another user that still has access.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label May 26, 2023
@icewind1991 icewind1991 added this to the Nextcloud 28 milestone May 26, 2023
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, blizzz and nfebe and removed request for a team May 26, 2023 15:28
@icewind1991 icewind1991 marked this pull request as ready for review May 26, 2023 15:32

$allShares = $this->shareManager->getAllShares();

$filteredShares = new \CallbackFilterIterator($allShares, function(IShare $share) use ($ownerInput, $shareType, $sharedByInput, $sharedWithInput, $fileInput) {

Check failure

Code scanning / Psalm

InvalidArgument

Argument 1 of CallbackFilterIterator::__construct expects Iterator<mixed, mixed>, but iterable<mixed, mixed> provided
return 1;
}

if (!$targetHasNonSharedAccess && $targetHasSharedAccess) {

Check failure

Code scanning / Psalm

RedundantCondition

Operand of type true is always truthy

$allShares = $this->shareManager->getAllShares();

$filteredShares = new \CallbackFilterIterator($allShares, function(IShare $share) use ($ownerInput, $shareType, $sharedByInput, $sharedWithInput, $fileInput) {

Check notice

Code scanning / Psalm

PossiblyUndefinedVariable

Possibly undefined variable $shareType, first seen on line 65
}

$share = $this->shareManager->getShareById($shareId);
if (!$share) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Operand of type false is always falsy
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace by try/catch or remove

}

$share = $this->shareManager->getShareById($shareId);
if (!$share) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Docblock-defined type OCP\Share\IShare for $share is never falsy
return 1;
}

$share->setNode($targetNode);

Check notice

Code scanning / Psalm

PossiblyNullArgument

Argument 1 of OCP\Share\IShare::setNode cannot be null, possibly null value provided
return 1;
}
} else {
$shareTypeInput = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$shareTypeInput = null;
$shareType = null;

});

$shareData = [];
foreach ($filteredShares as $share) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the easiest to avoid type problems with the iterable is to just call filterShare in an if there and continue.

Comment on lines +112 to +116
string $ownerInput = null,
string $sharedByInput = null,
string $sharedWithInput = null,
int $shareType = null,
int $fileInput = null
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the default values?
If it’s just to get nullable types prefix type with ? instead.

}

$share = $this->shareManager->getShareById($shareId);
if (!$share) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace by try/catch or remove


$targetHasNonSharedAccess = false;
$targetHasSharedAccess = false;
$fileOrFolder = ($sourceFile instanceof File) ? "file" : "folder";;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$fileOrFolder = ($sourceFile instanceof File) ? "file" : "folder";;
$fileOrFolder = ($sourceFile instanceof File) ? "file" : "folder";

return 1;
}

if (!$targetHasNonSharedAccess && $targetHasSharedAccess) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!$targetHasNonSharedAccess && $targetHasSharedAccess) {
if (!$targetHasNonSharedAccess) {

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv skjnldsv added 2. developing Work in progress feature: sharing and removed 3. to review Waiting for reviews labels Feb 23, 2024
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Mar 28, 2024
@skjnldsv skjnldsv added this to the Nextcloud 30 milestone Mar 28, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@skjnldsv skjnldsv deleted the share-list-set-owner branch December 19, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants