diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index 56751e15d..6a83bef00 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -505,7 +505,7 @@ public function getAdditionalContentEntry(string $key, $default = null) } /** - * Rename a tag in tags list. + * Rename a tag in tags list. If the new tag already exists, merge them * * @param string $fromTag * @param string $toTag @@ -513,7 +513,11 @@ public function getAdditionalContentEntry(string $key, $default = null) public function renameTag(string $fromTag, string $toTag): void { if (($pos = array_search($fromTag, $this->tags ?? [])) !== false) { - $this->tags[$pos] = trim($toTag); + if (in_array($toTag, $this->tags ?? []) !== false) { + $this->deleteTag($fromTag); + } else { + $this->tags[$pos] = trim($toTag); + } } } diff --git a/tests/bookmark/BookmarkTest.php b/tests/bookmark/BookmarkTest.php index 6a34e2ce4..41ed8aa55 100644 --- a/tests/bookmark/BookmarkTest.php +++ b/tests/bookmark/BookmarkTest.php @@ -374,6 +374,9 @@ public function testSetTags() public function testRenameTag() { $bookmark = new Bookmark(); + $bookmark->renameTag('chair', 'table'); + $this->assertEquals([], $bookmark->getTags()); + $bookmark->setTags(['tag1', 'tag2', 'chair']); $bookmark->renameTag('chair', 'table'); $this->assertEquals(['tag1', 'tag2', 'table'], $bookmark->getTags()); @@ -396,6 +399,17 @@ public function testRenameTagNotExists() $this->assertEquals(['tag1', 'tag2', 'chair'], $bookmark->getTags()); } + /** + * Test renameTag() avoid duplicating existing tag + */ + public function testRenameTagNotDuplicated() + { + $bookmark = new Bookmark(); + $bookmark->setTags(['tag1', 'tag2', 'chair']); + $bookmark->renameTag('tag1', 'chair'); + $this->assertEquals(['tag2', 'chair'], $bookmark->getTags()); + } + /** * Test deleteTag() */ diff --git a/tests/front/controller/admin/ManageTagControllerTest.php b/tests/front/controller/admin/ManageTagControllerTest.php index 56a64cbb7..b5c92fc17 100644 --- a/tests/front/controller/admin/ManageTagControllerTest.php +++ b/tests/front/controller/admin/ManageTagControllerTest.php @@ -13,6 +13,12 @@ use Shaarli\TestCase; use Slim\Http\Request; use Slim\Http\Response; +// These are declared for the bookmark service +use malkusch\lock\mutex\NoMutex; +use Shaarli\History; +use Shaarli\Plugin\PluginManager; +use Shaarli\Tests\Utils\FakeBookmarkService; +use Shaarli\Tests\Utils\ReferenceLinkDB; class ManageTagControllerTest extends TestCase { @@ -21,11 +27,46 @@ class ManageTagControllerTest extends TestCase /** @var ManageTagController */ protected $controller; + /** @var string Path of test data store */ + protected static $testDatastore = 'sandbox/datastore.php'; + + /** @var BookmarkServiceInterface instance */ + protected $bookmarkService; + + /** @var BookmarkFilter instance */ + protected $linkFilter; + + /** @var ReferenceLinkDB instance */ + protected static $refDB; + + /** @var PluginManager */ + protected static $pluginManager; + public function setUp(): void { $this->createContainer(); $this->controller = new ManageTagController($this->container); + + $mutex = new NoMutex(); + $conf = new ConfigManager('tests/utils/config/configJson'); + $conf->set('resource.datastore', self::$testDatastore); + static::$pluginManager = new PluginManager($conf); + self::$refDB = new ReferenceLinkDB(); + self::$refDB->write(self::$testDatastore); + $history = new History('sandbox/history.php'); + $this->container->bookmarkService = new FakeBookmarkService( + $conf, + static::$pluginManager, + $history, + $mutex, + true + ); + $this->linkFilter = new BookmarkFilter( + $this->container->bookmarkService->getBookmarks(), + $conf, + static::$pluginManager + ); } /** @@ -80,10 +121,23 @@ public function testSaveRenameTagValid(): void $session = []; $this->assignSessionVars($session); + $this->assertEquals( + 3, + count($this->linkFilter->filter(BookmarkFilter::$FILTER_TAG, 'cartoon')) + ); + $this->assertEquals( + 4, + count($this->linkFilter->filter(BookmarkFilter::$FILTER_TAG, 'web')) + ); + $this->assertEquals( + 2, + count($this->linkFilter->filter(BookmarkFilter::$FILTER_TAG, 'cartoon web')) + ); + $requestParameters = [ 'renametag' => 'rename', - 'fromtag' => 'old-tag', - 'totag' => 'new-tag', + 'fromtag' => 'cartoon', + 'totag' => 'web', ]; $request = $this->createMock(Request::class); $request @@ -94,36 +148,33 @@ public function testSaveRenameTagValid(): void }) ; $response = new Response(); - - $bookmark1 = $this->createMock(Bookmark::class); - $bookmark2 = $this->createMock(Bookmark::class); - $this->container->bookmarkService - ->expects(static::once()) - ->method('search') - ->with(['searchtags' => 'old-tag'], BookmarkFilter::$ALL, true) - ->willReturnCallback(function () use ($bookmark1, $bookmark2): SearchResult { - $bookmark1->expects(static::once())->method('renameTag')->with('old-tag', 'new-tag'); - $bookmark2->expects(static::once())->method('renameTag')->with('old-tag', 'new-tag'); - - return SearchResult::getSearchResult([$bookmark1, $bookmark2]); - }) - ; - $this->container->bookmarkService - ->expects(static::exactly(2)) - ->method('set') - ->withConsecutive([$bookmark1, false], [$bookmark2, false]) - ; - $this->container->bookmarkService->expects(static::once())->method('save'); - $result = $this->controller->save($request, $response); - static::assertSame(302, $result->getStatusCode()); - static::assertSame(['/subfolder/?searchtags=new-tag'], $result->getHeader('location')); + static::assertSame(['/subfolder/?searchtags=web'], $result->getHeader('location')); static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); static::assertArrayHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); - static::assertSame(['The tag was renamed in 2 bookmarks.'], $session[SessionManager::KEY_SUCCESS_MESSAGES]); + static::assertSame(['The tag was renamed in 3 bookmarks.'], $session[SessionManager::KEY_SUCCESS_MESSAGES]); + + $this->assertEquals( + 0, + count($this->linkFilter->filter(BookmarkFilter::$FILTER_TAG, 'cartoon')) + ); + $new = $this->linkFilter->filter(BookmarkFilter::$FILTER_TAG, 'web'); + $this->assertEquals( + 5, + count($new) + ); + + // Make sure there are no duplicate tags + foreach ($new as $bookmark) { + $tags = $bookmark->getTags(); + $this->assertEquals( + count($tags), + count(array_unique($tags)) + ); + } } /** @@ -134,10 +185,16 @@ public function testSaveDeleteTagValid(): void $session = []; $this->assignSessionVars($session); + $this->assertEquals( + 3, + count($this->linkFilter->filter(BookmarkFilter::$FILTER_TAG, 'cartoon')) + ); + $requestParameters = [ 'deletetag' => 'delete', - 'fromtag' => 'old-tag', + 'fromtag' => 'cartoon', ]; + $request = $this->createMock(Request::class); $request ->expects(static::atLeastOnce()) @@ -148,26 +205,6 @@ public function testSaveDeleteTagValid(): void ; $response = new Response(); - $bookmark1 = $this->createMock(Bookmark::class); - $bookmark2 = $this->createMock(Bookmark::class); - $this->container->bookmarkService - ->expects(static::once()) - ->method('search') - ->with(['searchtags' => 'old-tag'], BookmarkFilter::$ALL, true) - ->willReturnCallback(function () use ($bookmark1, $bookmark2): SearchResult { - $bookmark1->expects(static::once())->method('deleteTag')->with('old-tag'); - $bookmark2->expects(static::once())->method('deleteTag')->with('old-tag'); - - return SearchResult::getSearchResult([$bookmark1, $bookmark2]); - }) - ; - $this->container->bookmarkService - ->expects(static::exactly(2)) - ->method('set') - ->withConsecutive([$bookmark1, false], [$bookmark2, false]) - ; - $this->container->bookmarkService->expects(static::once())->method('save'); - $result = $this->controller->save($request, $response); static::assertSame(302, $result->getStatusCode()); @@ -176,7 +213,7 @@ public function testSaveDeleteTagValid(): void static::assertArrayNotHasKey(SessionManager::KEY_ERROR_MESSAGES, $session); static::assertArrayNotHasKey(SessionManager::KEY_WARNING_MESSAGES, $session); static::assertArrayHasKey(SessionManager::KEY_SUCCESS_MESSAGES, $session); - static::assertSame(['The tag was removed from 2 bookmarks.'], $session[SessionManager::KEY_SUCCESS_MESSAGES]); + static::assertSame(['The tag was removed from 3 bookmarks.'], $session[SessionManager::KEY_SUCCESS_MESSAGES]); } /**