From cb60e70ae9f5fda066cebe0ae684fe843b84fc11 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 7 Sep 2022 16:53:26 +0200 Subject: [PATCH 1/2] refs #4035 fix attachment creator name: show display name instead of user id Signed-off-by: Julien Veyssier --- lib/Service/AttachmentService.php | 41 +++++++++++++++++++++++++- src/components/card/AttachmentList.vue | 2 +- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/Service/AttachmentService.php b/lib/Service/AttachmentService.php index f8b6afc33..c7c4532a2 100644 --- a/lib/Service/AttachmentService.php +++ b/lib/Service/AttachmentService.php @@ -39,6 +39,7 @@ use OCA\Deck\StatusException; use OCP\AppFramework\Db\IMapperException; use OCP\AppFramework\Http\Response; use OCP\IL10N; +use OCP\IUserManager; class AttachmentService { private $attachmentMapper; @@ -58,8 +59,18 @@ class AttachmentService { private $activityManager; /** @var ChangeHelper */ private $changeHelper; + private IUserManager $userManager; - public function __construct(AttachmentMapper $attachmentMapper, CardMapper $cardMapper, ChangeHelper $changeHelper, PermissionService $permissionService, Application $application, AttachmentCacheHelper $attachmentCacheHelper, $userId, IL10N $l10n, ActivityManager $activityManager) { + public function __construct(AttachmentMapper $attachmentMapper, + CardMapper $cardMapper, + IUserManager $userManager, + ChangeHelper $changeHelper, + PermissionService $permissionService, + Application $application, + AttachmentCacheHelper $attachmentCacheHelper, + $userId, + IL10N $l10n, + ActivityManager $activityManager) { $this->attachmentMapper = $attachmentMapper; $this->cardMapper = $cardMapper; $this->permissionService = $permissionService; @@ -69,6 +80,7 @@ class AttachmentService { $this->l10n = $l10n; $this->activityManager = $activityManager; $this->changeHelper = $changeHelper; + $this->userManager = $userManager; // Register shipped attachment services // TODO: move this to a plugin based approach once we have different types of attachments @@ -127,6 +139,7 @@ class AttachmentService { try { $service = $this->getService($attachment->getType()); $service->extendData($attachment); + $this->addCreator($attachment); } catch (InvalidAttachmentType $e) { // Ingore invalid attachment types when extending the data } @@ -210,6 +223,7 @@ class AttachmentService { } $service->extendData($attachment); + $this->addCreator($attachment); } catch (InvalidAttachmentType $e) { // just store the data } @@ -313,6 +327,7 @@ class AttachmentService { $this->attachmentMapper->update($attachment); // extend data so the frontend can use it properly after creating $service->extendData($attachment); + $this->addCreator($attachment); $this->changeHelper->cardChanged($attachment->getCardId()); $this->activityManager->triggerEvent(ActivityManager::DECK_OBJECT_CARD, $attachment, ActivityManager::SUBJECT_ATTACHMENT_UPDATE); @@ -387,4 +402,28 @@ class AttachmentService { } throw new NoPermissionException('Restore is not allowed.'); } + + /** + * @param Attachment $attachment + * @return Attachment + * @throws \ReflectionException + */ + private function addCreator(Attachment $attachment): Attachment { + $createdBy = $attachment->jsonSerialize()['createdBy'] ?? ''; + $creator = [ + 'displayName' => $createdBy, + 'id' => $createdBy, + 'email' => null, + ]; + if ($this->userManager->userExists($createdBy)) { + $user = $this->userManager->get($createdBy); + $creator['displayName'] = $user->getDisplayName(); + $creator['email'] = $user->getEMailAddress(); + } + $extendedData = $attachment->jsonSerialize()['extendedData'] ?? []; + $extendedData['attachmentCreator'] = $creator; + $attachment->setExtendedData($extendedData); + + return $attachment; + } } diff --git a/src/components/card/AttachmentList.vue b/src/components/card/AttachmentList.vue index b466ad14b..4df418d54 100644 --- a/src/components/card/AttachmentList.vue +++ b/src/components/card/AttachmentList.vue @@ -63,7 +63,7 @@
{{ formattedFileSize(attachment.extendedData.filesize) }} {{ relativeDate(attachment.createdAt*1000) }} - {{ attachment.createdBy }} + {{ attachment.extendedData.attachmentCreator.displayName }}
{{ t('deck', 'Pending share') }} From 82c71451635c8e73c469acd04f4ae3aa74d94dae Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 7 Sep 2022 17:15:48 +0200 Subject: [PATCH 2/2] fix tests Signed-off-by: Julien Veyssier --- tests/unit/Service/AttachmentServiceTest.php | 27 +++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/unit/Service/AttachmentServiceTest.php b/tests/unit/Service/AttachmentServiceTest.php index 47b512386..a78999b31 100644 --- a/tests/unit/Service/AttachmentServiceTest.php +++ b/tests/unit/Service/AttachmentServiceTest.php @@ -37,6 +37,7 @@ use OCA\Deck\NotFoundException; use OCP\AppFramework\Http\Response; use OCP\AppFramework\IAppContainer; use OCP\IL10N; +use OCP\IUserManager; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -60,6 +61,8 @@ class MyAttachmentService implements IAttachmentService { class AttachmentServiceTest extends TestCase { + /** @var IUserManager|MockObject */ + private $userManager; /** @var AttachmentMapper|MockObject */ private $attachmentMapper; /** @var CardMapper|MockObject */ @@ -98,6 +101,7 @@ class AttachmentServiceTest extends TestCase { $this->appContainer = $this->createMock(IAppContainer::class); + $this->userManager = $this->createMock(IUserManager::class); $this->attachmentMapper = $this->createMock(AttachmentMapper::class); $this->cardMapper = $this->createMock(CardMapper::class); $this->permissionService = $this->createMock(PermissionService::class); @@ -126,6 +130,7 @@ class AttachmentServiceTest extends TestCase { $this->attachmentService = new AttachmentService( $this->attachmentMapper, $this->cardMapper, + $this->userManager, $this->changeHelper, $this->permissionService, $this->application, @@ -158,7 +163,7 @@ class AttachmentServiceTest extends TestCase { $application->expects($this->any()) ->method('getContainer') ->willReturn($appContainer); - $attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager); + $attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->userManager, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager); $attachmentService->registerAttachmentService('custom', MyAttachmentService::class); $this->assertEquals($fileServiceMock, $attachmentService->getService('deck_file')); $this->assertEquals(MyAttachmentService::class, get_class($attachmentService->getService('custom'))); @@ -188,7 +193,7 @@ class AttachmentServiceTest extends TestCase { ->method('getContainer') ->willReturn($appContainer); - $attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager); + $attachmentService = new AttachmentService($this->attachmentMapper, $this->cardMapper, $this->userManager, $this->changeHelper, $this->permissionService, $application, $this->attachmentCacheHelper, $this->userId, $this->l10n, $this->activityManager); $attachmentService->registerAttachmentService('custom', MyAttachmentService::class); $attachmentService->getService('deck_file_invalid'); } @@ -296,7 +301,14 @@ class AttachmentServiceTest extends TestCase { $actual = $this->attachmentService->create(123, 'deck_file', 'file_name.jpg'); - $expected->setExtendedData(['mime' => 'image/jpeg']); + $expected->setExtendedData([ + 'mime' => 'image/jpeg', + 'attachmentCreator' => [ + 'displayName' => '', + 'id' => '', + 'email' => null, + ], + ]); $this->assertEquals($expected, $actual); } @@ -353,7 +365,14 @@ class AttachmentServiceTest extends TestCase { $actual = $this->attachmentService->update(1, 1, 'file_name.jpg'); - $expected->setExtendedData(['mime' => 'image/jpeg']); + $expected->setExtendedData([ + 'mime' => 'image/jpeg', + 'attachmentCreator' => [ + 'displayName' => '', + 'id' => '', + 'email' => null, + ], + ]); $expected->setLastModified($attachment->getLastModified()); $this->assertEquals($expected, $actual); }