From 4a81eab3c6a73ad00dc1e21c7bb5bdc1840845a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Wed, 25 Jul 2018 09:23:56 +0200 Subject: [PATCH] Move "no-change" result below all the checks --- apps/files_sharing/lib/API/Share20OCS.php | 17 +++++++++-------- apps/files_sharing/tests/API/Share20OCSTest.php | 6 +----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php index f68b231f2dfd..4fc6739fff49 100644 --- a/apps/files_sharing/lib/API/Share20OCS.php +++ b/apps/files_sharing/lib/API/Share20OCS.php @@ -888,14 +888,6 @@ private function updateShareState($id, $state) { return new \OC\OCS\Result(null, 404, $this->l->t('Wrong share ID, share doesn\'t exist')); } - if ($share->getState() === $state) { - if ($eventName !== '') { - $this->eventDispatcher->dispatch('share.after' . $eventName, new GenericEvent(null, ['share' => $share])); - } - // if there are no changes in the state, just return the share as if the change was successful - return new \OC\OCS\Result([$this->formatShare($share, true)]); - } - $node = $share->getNode(); $node->lock(\OCP\Lock\ILockingProvider::LOCK_SHARED); @@ -912,6 +904,15 @@ private function updateShareState($id, $state) { return new \OC\OCS\Result(null, 403, $this->l->t('Only recipient can change accepted state')); } + if ($share->getState() === $state) { + if ($eventName !== '') { + $this->eventDispatcher->dispatch('share.after' . $eventName, new GenericEvent(null, ['share' => $share])); + } + // if there are no changes in the state, just return the share as if the change was successful + $node->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED); + return new \OC\OCS\Result([$this->formatShare($share, true)]); + } + // we actually want to update all shares related to the node in case there are multiple // incoming shares for the same node (ex: receiving simultaneously through group share and user share) $allShares = $this->shareManager->getSharedWith($this->currentUser->getUID(), \OCP\Share::SHARE_TYPE_USER, $node, -1, 0); diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php index 6bb73ebdb409..ab872abe53d6 100644 --- a/apps/files_sharing/tests/API/Share20OCSTest.php +++ b/apps/files_sharing/tests/API/Share20OCSTest.php @@ -3215,11 +3215,7 @@ public function providesAcceptRejectShareSameState() { * @dataProvider providesAcceptRejectShareSameState */ public function testAcceptRejectShareSameState($method, $target) { - $node = $this->createMock(Node::class); - - $userShare = $this->newShare(); - $userShare->setId(123); - $userShare->setNode($node); + $userShare = $this->makeReceivedUserShareForOperation($target); $this->shareManager->expects($this->exactly(1)) ->method('getShareById')