mirror of
https://github.com/nextcloud/server.git
synced 2026-04-15 22:11:17 -04:00
fix: Dispatch favorite event with an actual path
The `$path` argument was added in https://github.com/nextcloud/server/pull/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 https://github.com/nextcloud/activity/issues/2134 Signed-off-by: Louis Chemineau <louis@chmn.me>
This commit is contained in:
parent
be23649d4e
commit
431f6d7c87
7 changed files with 65 additions and 28 deletions
|
|
@ -176,8 +176,9 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
|
|||
*
|
||||
* @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 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
|
|||
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 @@ class TagsPlugin extends \Sabre\DAV\ServerPlugin {
|
|||
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)) {
|
||||
|
|
|
|||
|
|
@ -262,8 +262,8 @@ class TagsPluginTest extends \Test\TestCase {
|
|||
|
||||
// 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 @@ class TagsPluginTest extends \Test\TestCase {
|
|||
|
||||
// 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')
|
||||
|
|
|
|||
|
|
@ -54,11 +54,11 @@ class TagService {
|
|||
|
||||
$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
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
|
|||
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 @@ class TagManager implements ITagManager, IEventListener {
|
|||
private IDBConnection $connection,
|
||||
private LoggerInterface $logger,
|
||||
private IEventDispatcher $dispatcher,
|
||||
private IRootFolder $rootFolder,
|
||||
) {
|
||||
}
|
||||
|
||||
|
|
@ -56,7 +58,8 @@ class TagManager implements ITagManager, IEventListener {
|
|||
}
|
||||
$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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder;
|
|||
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 @@ class Tags implements ITags {
|
|||
private IDBConnection $db,
|
||||
private IEventDispatcher $dispatcher,
|
||||
private IUserSession $userSession,
|
||||
private Folder $userFolder,
|
||||
array $defaultTags = [],
|
||||
) {
|
||||
$this->owners = [$this->user];
|
||||
|
|
@ -495,12 +497,8 @@ class Tags implements ITags {
|
|||
|
||||
/**
|
||||
* 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,6 +529,15 @@ class Tags implements ITags {
|
|||
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;
|
||||
|
|
@ -538,12 +545,8 @@ class Tags implements ITags {
|
|||
|
||||
/**
|
||||
* 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 @@ class Tags implements ITags {
|
|||
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;
|
||||
|
|
|
|||
|
|
@ -184,20 +184,26 @@ interface ITags {
|
|||
*
|
||||
* @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
|
||||
|
|
|
|||
|
|
@ -11,6 +11,9 @@ namespace Test;
|
|||
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 @@ class TagsTest extends \Test\TestCase {
|
|||
->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 @@ class TagsTest extends \Test\TestCase {
|
|||
->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));
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue