From fd6357708940e68eb01c3761d404053366213bee Mon Sep 17 00:00:00 2001 From: Jan-Christoph Borchardt Date: Wed, 17 May 2017 15:35:10 +0200 Subject: [PATCH] Improve wording of various error messages Signed-off-by: Jan-Christoph Borchardt --- apps/files_sharing/tests/ApiTest.php | 2 +- core/css/guest.css | 2 +- lib/private/Share20/Manager.php | 40 ++++++++--------- .../Exceptions/GenericShareException.php | 2 +- tests/lib/Share20/ManagerTest.php | 44 +++++++++---------- 5 files changed, 45 insertions(+), 45 deletions(-) diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php index 046ede1f83..67e5055469 100644 --- a/apps/files_sharing/tests/ApiTest.php +++ b/apps/files_sharing/tests/ApiTest.php @@ -1382,7 +1382,7 @@ class ApiTest extends TestCase { $this->fail(); } catch (OCSException $e) { $this->assertEquals(404, $e->getCode()); - $this->assertEquals('Cannot set expiration date more than 7 days in the future', $e->getMessage()); + $this->assertEquals('Can’t set expiration date more than 7 days in the future', $e->getMessage()); } $ocs->cleanup(); diff --git a/core/css/guest.css b/core/css/guest.css index d55d8785b4..32af45e088 100644 --- a/core/css/guest.css +++ b/core/css/guest.css @@ -391,7 +391,7 @@ form .warning input[type='checkbox']+label { /* Database selector on install page */ form #selectDbType { text-align:center; - white-space: nowrap; + white-space: nowrap; margin: 0; } form #selectDbType .info { diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 57ba39d41f..7c39733ce2 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -210,7 +210,7 @@ class Manager implements IManager { // Cannot share with yourself if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && $share->getSharedWith() === $share->getSharedBy()) { - throw new \InvalidArgumentException('Can\'t share with yourself'); + throw new \InvalidArgumentException('Can’t share with yourself'); } // The path should be set @@ -231,7 +231,7 @@ class Manager implements IManager { $sharedPath = $this->rootFolder->getUserFolder($share->getShareOwner())->getPath(); } if ($sharedPath === $share->getNode()->getPath()) { - throw new \InvalidArgumentException('You can\'t share your root folder'); + throw new \InvalidArgumentException('You can’t share your root folder'); } // Check if we actually have share permissions @@ -258,7 +258,7 @@ class Manager implements IManager { // Check that we do not share with more permissions than we have if ($share->getPermissions() & ~$permissions) { - $message_t = $this->l->t('Cannot increase permissions of %s', [$share->getNode()->getPath()]); + $message_t = $this->l->t('Can’t increase permissions of %s', [$share->getNode()->getPath()]); throw new GenericShareException($message_t, $message_t, 404); } @@ -274,11 +274,11 @@ class Manager implements IManager { if ($share->getNode() instanceof \OCP\Files\File) { if ($share->getPermissions() & \OCP\Constants::PERMISSION_DELETE) { - $message_t = $this->l->t('Files can\'t be shared with delete permissions'); + $message_t = $this->l->t('Files can’t be shared with delete permissions'); throw new GenericShareException($message_t); } if ($share->getPermissions() & \OCP\Constants::PERMISSION_CREATE) { - $message_t = $this->l->t('Files can\'t be shared with create permissions'); + $message_t = $this->l->t('Files can’t be shared with create permissions'); throw new GenericShareException($message_t); } } @@ -333,7 +333,7 @@ class Manager implements IManager { $date->setTime(0, 0, 0); $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); if ($date < $expirationDate) { - $message = $this->l->t('Cannot set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]); + $message = $this->l->t('Can’t set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]); throw new GenericShareException($message, $message, 404); } } @@ -373,7 +373,7 @@ class Manager implements IManager { $this->groupManager->getUserGroupIds($sharedWith) ); if (empty($groups)) { - throw new \Exception('Only sharing with group members is allowed'); + throw new \Exception('Sharing is only allowed with group members'); } } @@ -396,7 +396,7 @@ class Manager implements IManager { // Identical share already existst if ($existingShare->getSharedWith() === $share->getSharedWith()) { - throw new \Exception('Path already shared with this user'); + throw new \Exception('Path is already shared with this user'); } // The share is already shared with this user via a group share @@ -406,7 +406,7 @@ class Manager implements IManager { $user = $this->userManager->get($share->getSharedWith()); if ($group->inGroup($user) && $existingShare->getShareOwner() !== $share->getShareOwner()) { - throw new \Exception('Path already shared with this user'); + throw new \Exception('Path is already shared with this user'); } } } @@ -430,7 +430,7 @@ class Manager implements IManager { $sharedBy = $this->userManager->get($share->getSharedBy()); $sharedWith = $this->groupManager->get($share->getSharedWith()); if (is_null($sharedWith) || !$sharedWith->inGroup($sharedBy)) { - throw new \Exception('Only sharing within your own groups is allowed'); + throw new \Exception('Sharing is only allowed within your own groups'); } } @@ -451,7 +451,7 @@ class Manager implements IManager { } if ($existingShare->getSharedWith() === $share->getSharedWith()) { - throw new \Exception('Path already shared with this group'); + throw new \Exception('Path is already shared with this group'); } } } @@ -465,18 +465,18 @@ class Manager implements IManager { protected function linkCreateChecks(\OCP\Share\IShare $share) { // Are link shares allowed? if (!$this->shareApiAllowLinks()) { - throw new \Exception('Link sharing not allowed'); + throw new \Exception('Link sharing is not allowed'); } // Link shares by definition can't have share permissions if ($share->getPermissions() & \OCP\Constants::PERMISSION_SHARE) { - throw new \InvalidArgumentException('Link shares can\'t have reshare permissions'); + throw new \InvalidArgumentException('Link shares can’t have reshare permissions'); } // Check if public upload is allowed if (!$this->shareApiLinkAllowPublicUpload() && ($share->getPermissions() & (\OCP\Constants::PERMISSION_CREATE | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_DELETE))) { - throw new \InvalidArgumentException('Public upload not allowed'); + throw new \InvalidArgumentException('Public upload is not allowed'); } } @@ -526,11 +526,11 @@ class Manager implements IManager { */ protected function canShare(\OCP\Share\IShare $share) { if (!$this->shareApiEnabled()) { - throw new \Exception('The share API is disabled'); + throw new \Exception('Sharing is disabled'); } if ($this->sharingDisabledForUser($share->getSharedBy())) { - throw new \Exception('You are not allowed to share'); + throw new \Exception('Sharing is disabled for you'); } } @@ -611,7 +611,7 @@ class Manager implements IManager { // Cannot share with the owner if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && $share->getSharedWith() === $share->getShareOwner()) { - throw new \InvalidArgumentException('Can\'t share with the share owner'); + throw new \InvalidArgumentException('Can’t share with the share owner'); } // Generate the target @@ -689,7 +689,7 @@ class Manager implements IManager { // We can't change the share type! if ($share->getShareType() !== $originalShare->getShareType()) { - throw new \InvalidArgumentException('Can\'t change share type'); + throw new \InvalidArgumentException('Can’t change share type'); } // We can only change the recipient on user shares @@ -701,7 +701,7 @@ class Manager implements IManager { // Cannot share with the owner if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && $share->getSharedWith() === $share->getShareOwner()) { - throw new \InvalidArgumentException('Can\'t share with the share owner'); + throw new \InvalidArgumentException('Can’t share with the share owner'); } $this->generalCreateChecks($share); @@ -880,7 +880,7 @@ class Manager implements IManager { */ public function moveShare(\OCP\Share\IShare $share, $recipientId) { if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK) { - throw new \InvalidArgumentException('Can\'t change target of link share'); + throw new \InvalidArgumentException('Can’t change target of link share'); } if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER && $share->getSharedWith() !== $recipientId) { diff --git a/lib/public/Share/Exceptions/GenericShareException.php b/lib/public/Share/Exceptions/GenericShareException.php index 4a05d7a034..8410a2d003 100644 --- a/lib/public/Share/Exceptions/GenericShareException.php +++ b/lib/public/Share/Exceptions/GenericShareException.php @@ -40,7 +40,7 @@ class GenericShareException extends HintException { */ public function __construct($message = '', $hint = '', $code = 0, \Exception $previous = null) { if (empty($message)) { - $message = 'Unspecified share exception'; + $message = 'There was an error retrieving the share. Maybe the link is wrong, it was unshared, or it was deleted.'; } parent::__construct($message, $hint, $code, $previous); } diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 79af2f60ce..1cc165106d 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -508,7 +508,7 @@ class ManagerTest extends \Test\TestCase { [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $file, $group0, null, $user0, 31, null, null), 'SharedBy should be set', true], [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $file, null, null, $user0, 31, null, null), 'SharedBy should be set', true], - [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $file, $user0, $user0, $user0, 31, null, null), 'Can\'t share with yourself', true], + [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $file, $user0, $user0, $user0, 31, null, null), 'Can’t share with yourself', true], [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, null, $user2, $user0, $user0, 31, null, null), 'Path should be set', true], [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, null, $group0, $user0, $user0, 31, null, null), 'Path should be set', true], @@ -539,26 +539,26 @@ class ManagerTest extends \Test\TestCase { $mount = $this->createMock(MoveableMount::class); $limitedPermssions->method('getMountPoint')->willReturn($mount); - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 31, null, null), 'Cannot increase permissions of path', true]; - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, 17, null, null), 'Cannot increase permissions of path', true]; - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $limitedPermssions, null, $user0, $user0, 3, null, null), 'Cannot increase permissions of path', true]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $limitedPermssions, $user2, $user0, $user0, 31, null, null), 'Can’t increase permissions of path', true]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $limitedPermssions, $group0, $user0, $user0, 17, null, null), 'Can’t increase permissions of path', true]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $limitedPermssions, null, $user0, $user0, 3, null, null), 'Can’t increase permissions of path', true]; $nonMoveableMountPermssions = $this->createMock(Folder::class); $nonMoveableMountPermssions->method('isShareable')->willReturn(true); $nonMoveableMountPermssions->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_READ); $nonMoveableMountPermssions->method('getPath')->willReturn('path'); - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $nonMoveableMountPermssions, $user2, $user0, $user0, 11, null, null), 'Cannot increase permissions of path', false]; - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $nonMoveableMountPermssions, $group0, $user0, $user0, 11, null, null), 'Cannot increase permissions of path', false]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $nonMoveableMountPermssions, $user2, $user0, $user0, 11, null, null), 'Can’t increase permissions of path', false]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $nonMoveableMountPermssions, $group0, $user0, $user0, 11, null, null), 'Can’t increase permissions of path', false]; $rootFolder = $this->createMock(Folder::class); $rootFolder->method('isShareable')->willReturn(true); $rootFolder->method('getPermissions')->willReturn(\OCP\Constants::PERMISSION_ALL); $rootFolder->method('getPath')->willReturn('myrootfolder'); - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $rootFolder, $user2, $user0, $user0, 30, null, null), 'You can\'t share your root folder', true]; - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $rootFolder, $group0, $user0, $user0, 2, null, null), 'You can\'t share your root folder', true]; - $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $rootFolder, null, $user0, $user0, 16, null, null), 'You can\'t share your root folder', true]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_USER, $rootFolder, $user2, $user0, $user0, 30, null, null), 'You can’t share your root folder', true]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_GROUP, $rootFolder, $group0, $user0, $user0, 2, null, null), 'You can’t share your root folder', true]; + $data[] = [$this->createShare(null, \OCP\Share::SHARE_TYPE_LINK, $rootFolder, null, $user0, $user0, 16, null, null), 'You can’t share your root folder', true]; $allPermssions = $this->createMock(Folder::class); $allPermssions->method('isShareable')->willReturn(true); @@ -614,7 +614,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage You can't share your root folder + * @expectedExceptionMessage You can’t share your root folder */ public function testGeneralCheckShareRoot() { $thrown = null; @@ -882,7 +882,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Only sharing with group members is allowed + * @expectedExceptionMessage Sharing is only allowed with group members */ public function testUserCreateChecksShareWithGroupMembersOnlyDifferentGroups() { $share = $this->manager->newShare(); @@ -954,7 +954,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Path already shared with this user + * @expectedExceptionMessage Path is already shared with this user */ public function testUserCreateChecksIdenticalShareExists() { $share = $this->manager->newShare(); @@ -979,7 +979,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Path already shared with this user + * @expectedExceptionMessage Path is already shared with this user */ public function testUserCreateChecksIdenticalPathSharedViaGroup() { $share = $this->manager->newShare(); @@ -1105,7 +1105,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Only sharing within your own groups is allowed + * @expectedExceptionMessage Sharing is only allowed within your own groups */ public function testGroupCreateChecksShareWithGroupMembersOnlyNotInGroup() { $share = $this->manager->newShare(); @@ -1131,7 +1131,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Only sharing within your own groups is allowed + * @expectedExceptionMessage Sharing is only allowed within your own groups */ public function testGroupCreateChecksShareWithGroupMembersOnlyNullGroup() { $share = $this->manager->newShare(); @@ -1183,7 +1183,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Path already shared with this group + * @expectedExceptionMessage Path is already shared with this group */ public function testGroupCreateChecksPathAlreadySharedWithSameGroup() { $share = $this->manager->newShare(); @@ -1238,7 +1238,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Link sharing not allowed + * @expectedExceptionMessage Link sharing is not allowed */ public function testLinkCreateChecksNoLinkSharesAllowed() { $share = $this->manager->newShare(); @@ -1254,7 +1254,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Link shares can't have reshare permissions + * @expectedExceptionMessage Link shares can’t have reshare permissions */ public function testLinkCreateChecksSharePermissions() { $share = $this->manager->newShare(); @@ -1272,7 +1272,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Public upload not allowed + * @expectedExceptionMessage Public upload is not allowed */ public function testLinkCreateChecksNoPublicUpload() { $share = $this->manager->newShare(); @@ -2297,7 +2297,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Can't change share type + * @expectedExceptionMessage Can’t change share type */ public function testUpdateShareCantChangeShareType() { $manager = $this->createManagerMock() @@ -2351,7 +2351,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException Exception - * @expectedExceptionMessage Can't share with the share owner + * @expectedExceptionMessage Can’t share with the share owner */ public function testUpdateShareCantShareWithOwner() { $manager = $this->createManagerMock() @@ -2642,7 +2642,7 @@ class ManagerTest extends \Test\TestCase { /** * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Can't change target of link share + * @expectedExceptionMessage Can’t change target of link share */ public function testMoveShareLink() { $share = $this->manager->newShare();