Merge pull request #4036 from nextcloud/fix/4035/show-attachment-creator-displayname

Fix attachment creator name: show display name

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;
+	}
 }

src/components/card/AttachmentList.vue

@@ -63,7 +63,7 @@
 						<div v-if="attachment.deletedAt === 0">
 							<span class="filesize">{{ formattedFileSize(attachment.extendedData.filesize) }}</span>
 							<span class="filedate">{{ relativeDate(attachment.createdAt*1000) }}</span>
-							<span class="filedate">{{ attachment.createdBy }}</span>
+							<span class="filedate">{{ attachment.extendedData.attachmentCreator.displayName }}</span>
 						</div>
 						<div v-else>
 							<span class="attachment--info">{{ t('deck', 'Pending share') }}</span>

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);
 	}