fix: Dispatch favorite event with an actual path

The `$path` argument was added in #48612, but was never actually used by the callers. The path was therefore missing in the favorite/unfavorite events, which lead to a broken activity information.

I also added a fallback to handle `addToFavorites` and `removeFromFavorites`, which are part of a public API, and are calling `tagAs` and `untag` without `$path`.

Fix nextcloud/activity#2134

Signed-off-by: Louis Chemineau <louis@chmn.me>

Author: artonge

apps/dav/lib/Connector/Sabre/TagsPlugin.php

@@ -176,8 +176,9 @@ private function prefetchTagsForFileIds(array $fileIds) {
 	 *
 	 * @param int $fileId
 	 * @param array $tags array of tag strings
+	 * @param string $path path of the file
 	 */
-	private function updateTags($fileId, $tags) {
+	private function updateTags($fileId, $tags, string $path) {
 		$tagger = $this->getTagger();
 		$currentTags = $this->getTags($fileId);
 
@@ -186,14 +187,14 @@ private function updateTags($fileId, $tags) {
 			if ($tag === self::TAG_FAVORITE) {
 				continue;
 			}
-			$tagger->tagAs($fileId, $tag);
+			$tagger->tagAs($fileId, $tag, $path);
 		}
 		$deletedTags = array_diff($currentTags, $tags);
 		foreach ($deletedTags as $tag) {
 			if ($tag === self::TAG_FAVORITE) {
 				continue;
 			}
-			$tagger->unTag($fileId, $tag);
+			$tagger->unTag($fileId, $tag, $path);
 		}
 	}
 
@@ -269,16 +270,16 @@ public function handleUpdateProperties($path, PropPatch $propPatch) {
 			return;
 		}
 
-		$propPatch->handle(self::TAGS_PROPERTYNAME, function ($tagList) use ($node) {
-			$this->updateTags($node->getId(), $tagList->getTags());
+		$propPatch->handle(self::TAGS_PROPERTYNAME, function ($tagList) use ($node, $path) {
+			$this->updateTags($node->getId(), $tagList->getTags(), $path);
 			return true;
 		});
 
 		$propPatch->handle(self::FAVORITE_PROPERTYNAME, function ($favState) use ($node, $path) {
 			if ((int)$favState === 1 || $favState === 'true') {
-				$this->getTagger()->tagAs($node->getId(), self::TAG_FAVORITE);
+				$this->getTagger()->tagAs($node->getId(), self::TAG_FAVORITE, $path);
 			} else {
-				$this->getTagger()->unTag($node->getId(), self::TAG_FAVORITE);
+				$this->getTagger()->unTag($node->getId(), self::TAG_FAVORITE, $path);
 			}
 
 			if (is_null($favState)) {

apps/dav/tests/unit/Connector/Sabre/TagsPluginTest.php

@@ -262,8 +262,8 @@ public function testUpdateTags(): void {
 
 		// then tag as tag1 and tag2
 		$calls = [
-			[123, 'tag1'],
-			[123, 'tag2'],
+			[123, 'tag1', '/dummypath'],
+			[123, 'tag2', '/dummypath'],
 		];
 		$this->tagger->expects($this->exactly(count($calls)))
 			->method('tagAs')
@@ -315,8 +315,8 @@ public function testUpdateTagsFromScratch(): void {
 
 		// then tag as tag1 and tag2
 		$calls = [
-			[123, 'tag1'],
-			[123, 'tag2'],
+			[123, 'tag1', '/dummypath'],
+			[123, 'tag2', '/dummypath'],
 		];
 		$this->tagger->expects($this->exactly(count($calls)))
 			->method('tagAs')

apps/files/lib/Service/TagService.php

@@ -54,11 +54,11 @@ public function updateFileTags($path, $tags) {
 
 		$newTags = array_diff($tags, $currentTags);
 		foreach ($newTags as $tag) {
-			$this->tagger->tagAs($fileId, $tag);
+			$this->tagger->tagAs($fileId, $tag, $path);
 		}
 		$deletedTags = array_diff($currentTags, $tags);
 		foreach ($deletedTags as $tag) {
-			$this->tagger->unTag($fileId, $tag);
+			$this->tagger->unTag($fileId, $tag, $path);
 		}
 
 		// TODO: re-read from tagger to make sure the

lib/private/TagManager.php

@@ -13,6 +13,7 @@
 use OCP\EventDispatcher\Event;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\EventDispatcher\IEventListener;
+use OCP\Files\IRootFolder;
 use OCP\IDBConnection;
 use OCP\ITagManager;
 use OCP\ITags;
@@ -31,6 +32,7 @@ public function __construct(
 		private IDBConnection $connection,
 		private LoggerInterface $logger,
 		private IEventDispatcher $dispatcher,
+		private IRootFolder $rootFolder,
 	) {
 	}
 
@@ -56,7 +58,8 @@ public function load($type, $defaultTags = [], $includeShared = false, $userId =
 			}
 			$userId = $this->userSession->getUser()->getUId();
 		}
-		return new Tags($this->mapper, $userId, $type, $this->logger, $this->connection, $this->dispatcher, $this->userSession, $defaultTags);
+		$userFolder = $this->rootFolder->getUserFolder($userId);
+		return new Tags($this->mapper, $userId, $type, $this->logger, $this->connection, $this->dispatcher, $this->userSession, $userFolder, $defaultTags);
 	}
 
 	/**

lib/private/Tags.php

@@ -14,6 +14,7 @@
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Events\NodeAddedToFavorite;
 use OCP\Files\Events\NodeRemovedFromFavorite;
+use OCP\Files\Folder;
 use OCP\IDBConnection;
 use OCP\ITags;
 use OCP\IUserSession;
@@ -65,6 +66,7 @@ public function __construct(
 		private IDBConnection $db,
 		private IEventDispatcher $dispatcher,
 		private IUserSession $userSession,
+		private Folder $userFolder,
 		array $defaultTags = [],
 	) {
 		$this->owners = [$this->user];
@@ -495,12 +497,8 @@ public function removeFromFavorites($objid) {
 
 	/**
 	 * Creates a tag/object relation.
-	 *
-	 * @param int $objid The id of the object
-	 * @param string $tag The id or name of the tag
-	 * @return boolean Returns false on error.
 	 */
-	public function tagAs($objid, $tag, string $path = '') {
+	public function tagAs($objid, $tag, ?string $path = null) {
 		if (is_string($tag) && !is_numeric($tag)) {
 			$tag = trim($tag);
 			if ($tag === '') {
@@ -531,19 +529,24 @@ public function tagAs($objid, $tag, string $path = '') {
 			return false;
 		}
 		if ($tag === ITags::TAG_FAVORITE) {
+			if ($path === null) {
+				$node = $this->userFolder->getFirstNodeById($objid);
+				if ($node !== null) {
+					$path = $node->getPath();
+				} else {
+					throw new Exception('Failed to favorite: node with id ' . $objid . ' not found');
+				}
+			}
+
 			$this->dispatcher->dispatchTyped(new NodeAddedToFavorite($this->userSession->getUser(), $objid, $path));
 		}
 		return true;
 	}
 
 	/**
 	 * Delete single tag/object relation from the db
-	 *
-	 * @param int $objid The id of the object
-	 * @param string $tag The id or name of the tag
-	 * @return boolean
 	 */
-	public function unTag($objid, $tag, string $path = '') {
+	public function unTag($objid, $tag, ?string $path = null) {
 		if (is_string($tag) && !is_numeric($tag)) {
 			$tag = trim($tag);
 			if ($tag === '') {
@@ -571,6 +574,15 @@ public function unTag($objid, $tag, string $path = '') {
 			return false;
 		}
 		if ($tag === ITags::TAG_FAVORITE) {
+			if ($path === null) {
+				$node = $this->userFolder->getFirstNodeById($objid);
+				if ($node !== null) {
+					$path = $node->getPath();
+				} else {
+					throw new Exception('Failed to unfavorite: node with id ' . $objid . ' not found');
+				}
+			}
+
 			$this->dispatcher->dispatchTyped(new NodeRemovedFromFavorite($this->userSession->getUser(), $objid, $path));
 		}
 		return true;

lib/public/ITags.php

@@ -184,20 +184,26 @@ public function removeFromFavorites($objid);
 	 *
 	 * @param int $objid The id of the object
 	 * @param string $tag The id or name of the tag
+	 * @param string $path The optional path of the node. Used when dispatching the favorite change event.
 	 * @return boolean Returns false on database error.
 	 * @since 6.0.0
+	 * @since 31.0.0 Added the $path argument.
+	 * @since 33.0.0 Change $path default value from '' to null.
 	 */
-	public function tagAs($objid, $tag);
+	public function tagAs($objid, $tag, ?string $path = null);
 
 	/**
 	 * Delete single tag/object relation from the db
 	 *
 	 * @param int $objid The id of the object
 	 * @param string $tag The id or name of the tag
+	 * @param string $path The optional path of the node. Used when dispatching the favorite change event.
 	 * @return boolean
 	 * @since 6.0.0
+	 * @since 31.0.0 Added the $path argument.
+	 * @since 33.0.0 Change $path default value from '' to null.
 	 */
-	public function unTag($objid, $tag);
+	public function unTag($objid, $tag, ?string $path = null);
 
 	/**
 	 * Delete tags from the database

tests/lib/TagsTest.php

@@ -11,6 +11,9 @@
 use OC\Tagging\TagMapper;
 use OC\TagManager;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Files\Folder;
+use OCP\Files\IRootFolder;
+use OCP\Files\Node;
 use OCP\IDBConnection;
 use OCP\ITagManager;
 use OCP\IUser;
@@ -52,10 +55,22 @@ protected function setUp(): void {
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($this->user);
+		$userFolder = $this->createMock(Folder::class);
+		$node = $this->createMock(Node::class);
+		$this->rootFolder = $this->createMock(IRootFolder::class);
+		$this->rootFolder
+			->method('getUserFolder')
+			->willReturnCallback(fn () => $userFolder);
+		$userFolder
+			->method('getFirstNodeById')
+			->willReturnCallback(fn () => $node);
+		$node
+			->method('getPath')
+			->willReturnCallback(fn () => 'file.txt');
 
 		$this->objectType = $this->getUniqueID('type_');
 		$this->tagMapper = new TagMapper(Server::get(IDBConnection::class));
-		$this->tagMgr = new TagManager($this->tagMapper, $this->userSession, Server::get(IDBConnection::class), Server::get(LoggerInterface::class), Server::get(IEventDispatcher::class));
+		$this->tagMgr = new TagManager($this->tagMapper, $this->userSession, Server::get(IDBConnection::class), Server::get(LoggerInterface::class), Server::get(IEventDispatcher::class), $this->rootFolder);
 	}
 
 	protected function tearDown(): void {
@@ -72,7 +87,7 @@ public function testTagManagerWithoutUserReturnsNull(): void {
 			->expects($this->any())
 			->method('getUser')
 			->willReturn(null);
-		$this->tagMgr = new TagManager($this->tagMapper, $this->userSession, Server::get(IDBConnection::class), Server::get(LoggerInterface::class), Server::get(IEventDispatcher::class));
+		$this->tagMgr = new TagManager($this->tagMapper, $this->userSession, Server::get(IDBConnection::class), Server::get(LoggerInterface::class), Server::get(IEventDispatcher::class), $this->rootFolder);
 		$this->assertNull($this->tagMgr->load($this->objectType));
 	}