summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--apps/dav/lib/Command/ListCalendars.php4
-rw-r--r--apps/dav/lib/Command/MoveCalendar.php57
-rw-r--r--apps/dav/tests/unit/Command/ListCalendarsTest.php10
-rw-r--r--apps/dav/tests/unit/Command/MoveCalendarTest.php114
4 files changed, 134 insertions, 51 deletions
diff --git a/apps/dav/lib/Command/ListCalendars.php b/apps/dav/lib/Command/ListCalendars.php
index 9080e9f8e29..6c2f5bdb506 100644
--- a/apps/dav/lib/Command/ListCalendars.php
+++ b/apps/dav/lib/Command/ListCalendars.php
@@ -59,13 +59,13 @@ class ListCalendars extends Command {
$this
->setName('dav:list-calendars')
->setDescription('List all calendars of a user')
- ->addArgument('user',
+ ->addArgument('uid',
InputArgument::REQUIRED,
'User for whom all calendars will be listed');
}
protected function execute(InputInterface $input, OutputInterface $output) {
- $user = $input->getArgument('user');
+ $user = $input->getArgument('uid');
if (!$this->userManager->userExists($user)) {
throw new \InvalidArgumentException("User <$user> is unknown.");
}
diff --git a/apps/dav/lib/Command/MoveCalendar.php b/apps/dav/lib/Command/MoveCalendar.php
index 23aee0f2d86..a2c7ca8c4d8 100644
--- a/apps/dav/lib/Command/MoveCalendar.php
+++ b/apps/dav/lib/Command/MoveCalendar.php
@@ -93,18 +93,18 @@ class MoveCalendar extends Command {
->addArgument('name',
InputArgument::REQUIRED,
'Name of the calendar to move')
- ->addArgument('userorigin',
+ ->addArgument('sourceuid',
InputArgument::REQUIRED,
'User who currently owns the calendar')
- ->addArgument('userdestination',
+ ->addArgument('destinationuid',
InputArgument::REQUIRED,
'User who will receive the calendar')
- ->addOption('force', 'f', InputOption::VALUE_NONE, "Force the migration by removing shares with groups that the destination user is not in");
+ ->addOption('force', 'f', InputOption::VALUE_NONE, "Force the migration by removing existing shares");
}
protected function execute(InputInterface $input, OutputInterface $output) {
- $userOrigin = $input->getArgument('userorigin');
- $userDestination = $input->getArgument('userdestination');
+ $userOrigin = $input->getArgument('sourceuid');
+ $userDestination = $input->getArgument('destinationuid');
$this->io = new SymfonyStyle($input, $output);
@@ -128,9 +128,7 @@ class MoveCalendar extends Command {
throw new \InvalidArgumentException("User <$userDestination> already has a calendar named <$name>.");
}
- if ($this->shareManager->shareWithGroupMembersOnly() === true) {
- $this->checkShares($calendar, $userDestination, $input->getOption('force'));
- }
+ $this->checkShares($calendar, $userOrigin, $userDestination, $input->getOption('force'));
$this->calDav->moveCalendar($name, self::URI_USERS . $userOrigin, self::URI_USERS . $userDestination);
@@ -138,25 +136,50 @@ class MoveCalendar extends Command {
}
/**
- * Check that user destination is member of the groups which whom the calendar was shared
- * If we ask to force the migration, the share with the group is dropped
+ * Check that moving the calendar won't break shares
*
- * @param $calendar
- * @param $userDestination
+ * @param array $calendar
+ * @param string $userOrigin
+ * @param string $userDestination
* @param bool $force
*/
- private function checkShares($calendar, $userDestination, $force = false)
+ private function checkShares(array $calendar, string $userOrigin, string $userDestination, bool $force = false)
{
$shares = $this->calDav->getShares($calendar['id']);
foreach ($shares as $share) {
- list(, $prefix, $group) = explode('/', $share['href'], 3);
- if ('groups' === $prefix && !$this->groupManager->isInGroup($userDestination, $group)) {
+ list(, $prefix, $userOrGroup) = explode('/', $share['href'], 3);
+
+ /**
+ * Check that user destination is member of the groups which whom the calendar was shared
+ * If we ask to force the migration, the share with the group is dropped
+ */
+ if ($this->shareManager->shareWithGroupMembersOnly() === true && 'groups' === $prefix && !$this->groupManager->isInGroup($userDestination, $userOrGroup)) {
+ if ($force) {
+ $this->calDav->updateShares(new Calendar($this->calDav, $calendar, $this->l10n, $this->config), [], ['href' => 'principal:principals/groups/' . $userOrGroup]);
+ } else {
+ throw new \InvalidArgumentException("User <$userDestination> is not part of the group <$userOrGroup> with whom the calendar <" . $calendar['uri'] . "> was shared. You may use -f to move the calendar while deleting this share.");
+ }
+ }
+
+ /**
+ * Check that calendar isn't already shared with user destination
+ */
+ if ($userOrGroup === $userDestination) {
if ($force) {
- $this->calDav->updateShares(new Calendar($this->calDav, $calendar, $this->l10n, $this->config), [], ['href' => 'principal:principals/groups/' . $group]);
+ $this->calDav->updateShares(new Calendar($this->calDav, $calendar, $this->l10n, $this->config), [], ['href' => 'principal:principals/users/' . $userOrGroup]);
} else {
- throw new \InvalidArgumentException("User <$userDestination> is not part of the group <$group> with whom the calendar <" . $calendar['uri'] . "> was shared. You may use -f to move the calendar while deleting this share.");
+ throw new \InvalidArgumentException("The calendar <" . $calendar['uri'] . "> is already shared to user <$userDestination>.You may use -f to move the calendar while deleting this share.");
}
}
}
+ /**
+ * Warn that share links have changed if there are shares
+ */
+ if (count($shares) > 0) {
+ $this->io->note([
+ "Please note that moving calendar " . $calendar['uri'] . " from user <$userOrigin> to <$userDestination> has caused share links to change.",
+ "Sharees will need to change \"example.com/remote.php/dav/calendars/uid/" . $calendar['uri'] . "_shared_by_$userOrigin\" to \"example.com/remote.php/dav/calendars/uid/" . $calendar['uri'] . "_shared_by_$userDestination\""
+ ]);
+ }
}
}
diff --git a/apps/dav/tests/unit/Command/ListCalendarsTest.php b/apps/dav/tests/unit/Command/ListCalendarsTest.php
index 8d19c724917..1a0ca8c7954 100644
--- a/apps/dav/tests/unit/Command/ListCalendarsTest.php
+++ b/apps/dav/tests/unit/Command/ListCalendarsTest.php
@@ -36,10 +36,10 @@ use Test\TestCase;
*/
class ListCalendarsTest extends TestCase {
- /** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject $userManager */
+ /** @var \OCP\IUserManager|\PHPUnit\Framework\MockObject\MockObject $userManager */
private $userManager;
- /** @var CalDavBackend|\PHPUnit_Framework_MockObject_MockObject $l10n */
+ /** @var CalDavBackend|\PHPUnit\Framework\MockObject\MockObject $l10n */
private $calDav;
/** @var ListCalendars */
@@ -71,7 +71,7 @@ class ListCalendarsTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
- 'user' => self::USERNAME,
+ 'uid' => self::USERNAME,
]);
$this->assertContains("User <" . self::USERNAME . "> in unknown", $commandTester->getDisplay());
}
@@ -90,7 +90,7 @@ class ListCalendarsTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
- 'user' => self::USERNAME,
+ 'uid' => self::USERNAME,
]);
$this->assertContains("User <" . self::USERNAME . "> has no calendars\n", $commandTester->getDisplay());
}
@@ -131,7 +131,7 @@ class ListCalendarsTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
- 'user' => self::USERNAME,
+ 'uid' => self::USERNAME,
]);
$this->assertContains($output, $commandTester->getDisplay());
$this->assertNotContains(BirthdayService::BIRTHDAY_CALENDAR_URI, $commandTester->getDisplay());
diff --git a/apps/dav/tests/unit/Command/MoveCalendarTest.php b/apps/dav/tests/unit/Command/MoveCalendarTest.php
index f7e7840322a..07d789dbfb2 100644
--- a/apps/dav/tests/unit/Command/MoveCalendarTest.php
+++ b/apps/dav/tests/unit/Command/MoveCalendarTest.php
@@ -39,10 +39,10 @@ use Test\TestCase;
*/
class MoveCalendarTest extends TestCase {
- /** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject $userManager */
+ /** @var \OCP\IUserManager|\PHPUnit\Framework\MockObject\MockObject $userManager */
private $userManager;
- /** @var \OCP\IGroupManager|\PHPUnit_Framework_MockObject_MockObject $groupManager */
+ /** @var \OCP\IGroupManager|\PHPUnit\Framework\MockObject\MockObject $groupManager */
private $groupManager;
/** @var \OCP\Share\IManager|\PHPUnit_Framework_MockObject_MockObject $shareManager */
@@ -111,8 +111,8 @@ class MoveCalendarTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => $this->command->getName(),
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
]);
}
@@ -139,8 +139,8 @@ class MoveCalendarTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => 'personal',
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
]);
}
@@ -175,8 +175,8 @@ class MoveCalendarTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => 'personal',
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
]);
}
@@ -206,14 +206,11 @@ class MoveCalendarTest extends TestCase {
->with(1234)
->willReturn([]);
- $this->shareManager->expects($this->once())->method('shareWithGroupMembersOnly')
- ->willReturn(true);
-
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => 'personal',
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
]);
$this->assertContains("[OK] Calendar <personal> was moved from user <user> to <user2>", $commandTester->getDisplay());
@@ -256,13 +253,12 @@ class MoveCalendarTest extends TestCase {
$this->shareManager->expects($this->once())->method('shareWithGroupMembersOnly')
->willReturn($shareWithGroupMembersOnly);
+ $this->calDav->expects($this->once())->method('getShares')
+ ->with(1234)
+ ->willReturn([
+ ['href' => 'principal:principals/groups/nextclouders']
+ ]);
if ($shareWithGroupMembersOnly === true) {
- $this->calDav->expects($this->once())->method('getShares')
- ->with(1234)
- ->willReturn([
- ['href' => 'principal:principals/groups/nextclouders']
- ]);
-
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage("User <user2> is not part of the group <nextclouders> with whom the calendar <personal> was shared. You may use -f to move the calendar while deleting this share.");
}
@@ -270,8 +266,8 @@ class MoveCalendarTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => 'personal',
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
]);
}
@@ -314,8 +310,8 @@ class MoveCalendarTest extends TestCase {
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => 'personal',
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
]);
$this->assertContains("[OK] Calendar <personal> was moved from user <user> to <user2>", $commandTester->getDisplay());
@@ -338,7 +334,7 @@ class MoveCalendarTest extends TestCase {
->willReturn([
'id' => 1234,
'uri' => 'personal',
- '{DAV:}displayname' => 'personal'
+ '{DAV:}displayname' => 'Personal'
]);
$this->calDav->expects($this->at(1))->method('getCalendarByUri')
@@ -351,17 +347,81 @@ class MoveCalendarTest extends TestCase {
$this->calDav->expects($this->once())->method('getShares')
->with(1234)
->willReturn([
- ['href' => 'principal:principals/groups/nextclouders']
+ [
+ 'href' => 'principal:principals/groups/nextclouders',
+ '{DAV:}displayname' => 'Personal'
+ ]
]);
+ $this->calDav->expects($this->once())->method('updateShares');
$commandTester = new CommandTester($this->command);
$commandTester->execute([
'name' => 'personal',
- 'userorigin' => 'user',
- 'userdestination' => 'user2',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
'--force' => true
]);
$this->assertContains("[OK] Calendar <personal> was moved from user <user> to <user2>", $commandTester->getDisplay());
}
+
+ public function dataTestMoveWithCalendarAlreadySharedToDestination(): array
+ {
+ return [
+ [true],
+ [false]
+ ];
+ }
+
+ /**
+ * @dataProvider dataTestMoveWithCalendarAlreadySharedToDestination
+ */
+ public function testMoveWithCalendarAlreadySharedToDestination(bool $force)
+ {
+ $this->userManager->expects($this->at(0))
+ ->method('userExists')
+ ->with('user')
+ ->willReturn(true);
+
+ $this->userManager->expects($this->at(1))
+ ->method('userExists')
+ ->with('user2')
+ ->willReturn(true);
+
+ $this->calDav->expects($this->at(0))->method('getCalendarByUri')
+ ->with('principals/users/user', 'personal')
+ ->willReturn([
+ 'id' => 1234,
+ 'uri' => 'personal',
+ '{DAV:}displayname' => 'Personal',
+ ]);
+
+ $this->calDav->expects($this->at(1))->method('getCalendarByUri')
+ ->with('principals/users/user2', 'personal')
+ ->willReturn(null);
+
+ $this->calDav->expects($this->once())->method('getShares')
+ ->with(1234)
+ ->willReturn([
+ [
+ 'href' => 'principal:principals/users/user2',
+ '{DAV:}displayname' => 'Personal'
+ ]
+ ]);
+
+ if ($force === false) {
+ $this->expectException(InvalidArgumentException::class);
+ $this->expectExceptionMessage("The calendar <personal> is already shared to user <user2>.You may use -f to move the calendar while deleting this share.");
+ } else {
+ $this->calDav->expects($this->once())->method('updateShares');
+ }
+
+ $commandTester = new CommandTester($this->command);
+ $commandTester->execute([
+ 'name' => 'personal',
+ 'sourceuid' => 'user',
+ 'destinationuid' => 'user2',
+ '--force' => $force,
+ ]);
+ }
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-01-24 11:26:40 +0000