Skip to content

Commit fb7da39

Browse files
committed
Remove createdShares and mention warning in readme for groups as approvers
Signed-off-by: Lukas Schaefer <lukas@lschaefer.xyz>
1 parent e6d4c2b commit fb7da39

2 files changed

Lines changed: 12 additions & 17 deletions

File tree

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ Workflows can be chained. For example, if the approved tag of a workflow A is us
3434
then once a file is approved by the workflow A, it becomes pending for the B one. For an example of a chain involving a leave
3535
request that needs to be approved by a manager and then the department head, there is a screenshot below.
3636

37+
**Notice**: If the approvers are a group the file may be shared with the individual users of the group instead of the group.
38+
3739
![Workflow chain](https://github.com/nextcloud/approval/raw/main/img/screenshot_chained.jpg)
3840

3941
## Tag assignment

lib/Service/ApprovalService.php

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -529,17 +529,16 @@ public function requestViaTagAssignment(int $fileId, int $ruleId, string $reques
529529
* @param int $fileId
530530
* @param Rule $rule
531531
* @param string $userId
532-
* @return array list of created shares
532+
* @return void
533533
* @throws \OCP\Files\NotPermittedException
534534
* @throws \OC\User\NoUserException
535535
*/
536-
private function shareWithApprovers(int $fileId, array $rule, string $userId): array {
537-
$createdShares = [];
536+
private function shareWithApprovers(int $fileId, array $rule, string $userId): void {
538537
// get node
539538
$userFolder = $this->root->getUserFolder($userId);
540539
$node = $userFolder->getFirstNodeById($fileId);
541540
if ($node === null) {
542-
return [];
541+
return;
543542
}
544543
// get the node again from the owner's storage to avoid sharing permission issues
545544
$ownerId = $node->getOwner()->getUID();
@@ -558,8 +557,8 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a
558557
foreach ($rule['approvers'] as $approver) {
559558
if ($approver['type'] === 'user' && !in_array($approver['entityId'], $userIdsWithAccess, true)) {
560559
// create user share
561-
if ($this->utilsService->createShare($node, IShare::TYPE_USER, $approver['entityId'], $fileOwner, $label)) {
562-
$createdShares[] = $approver;
560+
if (!$this->utilsService->createShare($node, IShare::TYPE_USER, $approver['entityId'], $fileOwner, $label)) {
561+
$this->logger->warning('Failed to create user share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $approver['entityId']]);
563562
}
564563
}
565564
}
@@ -571,20 +570,16 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a
571570
$groupMembersThatNeedAccess = array_diff($groupMemberIds, $userIdsWithAccess);
572571
// Create group share if everyone in the group needs access
573572
if (count($groupMembersThatNeedAccess) === count($groupMemberIds)) {
574-
if ($this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) {
575-
$createdShares[] = $approver;
573+
if (!$this->utilsService->createShare($node, IShare::TYPE_GROUP, $approver['entityId'], $fileOwner, $label)) {
574+
$this->logger->warning('Failed to create group share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $approver['entityId']]);
576575
}
577576
} elseif (count($groupMembersThatNeedAccess) > 0) {
578577
// Create user shares for each member that needs access
579-
$success = true;
580578
foreach ($groupMembersThatNeedAccess as $groupMemberId) {
581579
if (!$this->utilsService->createShare($node, IShare::TYPE_USER, $groupMemberId, $fileOwner, $label)) {
582-
$success = false;
580+
$this->logger->warning('Failed to create user share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $groupMemberId]);
583581
}
584582
}
585-
if ($success) {
586-
$createdShares[] = $approver;
587-
}
588583
}
589584
}
590585
}
@@ -594,14 +589,12 @@ private function shareWithApprovers(int $fileId, array $rule, string $userId): a
594589
if ($circlesEnabled) {
595590
foreach ($rule['approvers'] as $approver) {
596591
if ($approver['type'] === 'circle') {
597-
if ($this->utilsService->createShare($node, IShare::TYPE_CIRCLE, $approver['entityId'], $fileOwner, $label)) {
598-
$createdShares[] = $approver;
592+
if (!$this->utilsService->createShare($node, IShare::TYPE_CIRCLE, $approver['entityId'], $fileOwner, $label)) {
593+
$this->logger->warning('Failed to create circle share for file {fileId} with approver {approverId}', ['fileId' => $fileId, 'approverId' => $approver['entityId']]);
599594
}
600595
}
601596
}
602597
}
603-
604-
return $createdShares;
605598
}
606599

607600
/**

0 commit comments

Comments
 (0)