From 00cce1f8c739e57e4c40e42b4c138a9106756a7f Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 26 Nov 2022 14:58:12 +0100 Subject: [PATCH] Bulk action: add or delete tag to multiple bookmarks (#1898) --- application/bookmark/Bookmark.php | 10 + .../admin/ShaareManageController.php | 81 ++++ assets/default/js/base.js | 35 ++ index.php | 1 + .../AddOrDeleteTagTest.php | 380 ++++++++++++++++++ tpl/default/page.header.html | 36 ++ 6 files changed, 543 insertions(+) create mode 100644 tests/front/controller/admin/ShaareManageControllerTest/AddOrDeleteTagTest.php diff --git a/application/bookmark/Bookmark.php b/application/bookmark/Bookmark.php index 316ae693..cea52741 100644 --- a/application/bookmark/Bookmark.php +++ b/application/bookmark/Bookmark.php @@ -517,6 +517,16 @@ class Bookmark } } + /** + * Add a tag in tags list. + * + * @param string $tag + */ + public function addTag(string $tag): self + { + return $this->setTags(array_merge($this->getTags(), [$tag])); + } + /** * Delete a tag from tags list. * diff --git a/application/front/controller/admin/ShaareManageController.php b/application/front/controller/admin/ShaareManageController.php index d29353a4..05b81678 100644 --- a/application/front/controller/admin/ShaareManageController.php +++ b/application/front/controller/admin/ShaareManageController.php @@ -203,4 +203,85 @@ class ShaareManageController extends ShaarliAdminController '/shaare/' . $hash . '?key=' . $bookmark->getAdditionalContentEntry('private_key') ); } + + /** + * POST /admin/shaare/update-tags + * + * Bulk add or delete a tags on one or multiple bookmarks. + */ + public function addOrDeleteTags(Request $request, Response $response): Response + { + $this->checkToken($request); + + $ids = trim(escape($request->getParam('id') ?? '')); + if (empty($ids) || strpos($ids, ' ') !== false) { + // multiple, space-separated ids provided + $ids = array_values(array_filter(preg_split('/\s+/', $ids), 'ctype_digit')); + } else { + // only a single id provided + $ids = [$ids]; + } + + // assert at least one id is given + if (0 === count($ids)) { + $this->saveErrorMessage(t('Invalid bookmark ID provided.')); + + return $this->redirectFromReferer($request, $response, ['/updateTag'], []); + } + + // assert that the action is valid + $action = $request->getParam('action'); + if (!in_array($action, ['add', 'delete'], true)) { + $this->saveErrorMessage(t('Invalid action provided.')); + + return $this->redirectFromReferer($request, $response, ['/updateTag'], []); + } + + // assert that the tag name is valid + $tagString = trim($request->getParam('tag')); + if (empty($tagString)) { + $this->saveErrorMessage(t('Invalid tag name provided.')); + + return $this->redirectFromReferer($request, $response, ['/updateTag'], []); + } + + $tags = tags_str2array($tagString, $this->container->conf->get('general.tags_separator', ' ')); + $formatter = $this->container->formatterFactory->getFormatter('raw'); + $count = 0; + + foreach ($ids as $id) { + try { + $bookmark = $this->container->bookmarkService->get((int) $id); + } catch (BookmarkNotFoundException $e) { + $this->saveErrorMessage(sprintf( + t('Bookmark with identifier %s could not be found.'), + $id + )); + + continue; + } + + foreach ($tags as $tag) { + if ($action === 'add') { + $bookmark->addTag($tag); + } else { + $bookmark->deleteTag($tag); + } + } + + // To preserve backward compatibility with 3rd parties, plugins still use arrays + $data = $formatter->format($bookmark); + $this->executePageHooks('save_link', $data); + $bookmark->fromArray($data, $this->container->conf->get('general.tags_separator', ' ')); + + $this->container->bookmarkService->set($bookmark, false); + ++$count; + } + + if ($count > 0) { + $this->container->bookmarkService->save(); + } + + return $this->redirectFromReferer($request, $response, ['/updateTag'], []); + } } diff --git a/assets/default/js/base.js b/assets/default/js/base.js index 1c603788..48291d52 100644 --- a/assets/default/js/base.js +++ b/assets/default/js/base.js @@ -383,6 +383,10 @@ function init(description) { }); sub.classList.toggle('open'); + const autofocus = sub.querySelector('.autofocus'); + if (autofocus) { + autofocus.focus(); + } } }); }); @@ -507,6 +511,37 @@ function init(description) { }); } + ['add', 'delete'].forEach((action) => { + const subHeader = document.getElementById(`bulk-tag-action-${action}`); + + if (subHeader) { + subHeader.querySelectorAll('a.button').forEach((link) => { + if (!link.classList.contains('action')) { + return; + } + + subHeader.querySelector('input[name="tag"]').addEventListener('keypress', (event) => { + if (event.keyCode === 13) { // enter + link.click(); + } + }); + + link.addEventListener('click', (event) => { + event.preventDefault(); + + const ids = []; + const linkCheckedCheckboxes = document.querySelectorAll('.link-checkbox:checked'); + [...linkCheckedCheckboxes].forEach((checkbox) => { + ids.push(checkbox.value); + }); + + subHeader.querySelector('input[name="id"]').value = ids.join(' '); + subHeader.querySelector('form').submit(); + }); + }); + } + }); + /** * Select all button */ diff --git a/index.php b/index.php index 944c697c..4b71c672 100644 --- a/index.php +++ b/index.php @@ -151,6 +151,7 @@ $app->group('/admin', function () { $this->post('/shaare', '\Shaarli\Front\Controller\Admin\ShaarePublishController:save'); $this->get('/shaare/delete', '\Shaarli\Front\Controller\Admin\ShaareManageController:deleteBookmark'); $this->get('/shaare/visibility', '\Shaarli\Front\Controller\Admin\ShaareManageController:changeVisibility'); + $this->post('/shaare/update-tags', '\Shaarli\Front\Controller\Admin\ShaareManageController:addOrDeleteSingleTag'); $this->get('/shaare/{id:[0-9]+}/pin', '\Shaarli\Front\Controller\Admin\ShaareManageController:pinBookmark'); $this->patch( '/shaare/{id:[0-9]+}/update-thumbnail', diff --git a/tests/front/controller/admin/ShaareManageControllerTest/AddOrDeleteTagTest.php b/tests/front/controller/admin/ShaareManageControllerTest/AddOrDeleteTagTest.php new file mode 100644 index 00000000..cd76491d --- /dev/null +++ b/tests/front/controller/admin/ShaareManageControllerTest/AddOrDeleteTagTest.php @@ -0,0 +1,380 @@ +createContainer(); + + $this->container->httpAccess = $this->createMock(HttpAccess::class); + $this->controller = new ShaareManageController($this->container); + } + + /** + * Add 1 tag to 1 bookmark + */ + public function testAddOneTagOnOneBookmark(): void + { + $parameters = ['id' => '123', 'tag' => 'newtag', 'action' => 'add']; + + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + $bookmark = (new Bookmark()) + ->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123') + ->setTagsString('first second'); + + static::assertSame(['first', 'second'], $bookmark->getTags()); + + $this->container->bookmarkService->expects(static::once())->method('get')->with(123)->willReturn($bookmark); + $this->container->bookmarkService->expects(static::once())->method('set')->with($bookmark, false); + $this->container->bookmarkService->expects(static::once())->method('save'); + $this->container->formatterFactory = $this->createMock(FormatterFactory::class); + $this->container->formatterFactory + ->expects(static::once()) + ->method('getFormatter') + ->with('raw') + ->willReturnCallback(function () use ($bookmark): BookmarkFormatter { + return new BookmarkRawFormatter($this->container->conf, true); + }) + ; + + // Make sure that PluginManager hook is triggered + $this->container->pluginManager + ->expects(static::once()) + ->method('executeHooks') + ->with('save_link') + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(['first', 'second', 'newtag'], $bookmark->getTags()); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Add 2 tags to 2 bookmarks + */ + public function testAddTwoTagsOnTwoBookmarks(): void + { + $parameters = ['id' => '123 456', 'tag' => 'newtag@othertag', 'action' => 'add']; + + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + $bookmark1 = (new Bookmark()) + ->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123') + ->setTagsString('first second'); + $bookmark2 = (new Bookmark()) + ->setId(456)->setUrl('http://domain.tld')->setTitle('Title 123'); + + static::assertSame(['first', 'second'], $bookmark1->getTags()); + static::assertSame([], $bookmark2->getTags()); + + $this->container->bookmarkService->expects(static::exactly(2))->method('get') + ->withConsecutive([123], [456]) + ->willReturnOnConsecutiveCalls($bookmark1, $bookmark2); + $this->container->bookmarkService->expects(static::exactly(2))->method('set') + ->withConsecutive([$bookmark1, false], [$bookmark2, false]); + $this->container->bookmarkService->expects(static::once())->method('save'); + $this->container->formatterFactory = $this->createMock(FormatterFactory::class); + $this->container->formatterFactory + ->expects(static::once()) + ->method('getFormatter') + ->with('raw') + ->willReturnCallback(function (): BookmarkFormatter { + return new BookmarkRawFormatter($this->container->conf, true); + }) + ; + + // Make sure that PluginManager hook is triggered + $this->container->pluginManager + ->expects(static::exactly(2)) + ->method('executeHooks') + ->with('save_link') + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(['first', 'second', 'newtag', 'othertag'], $bookmark1->getTags()); + static::assertSame(['newtag', 'othertag'], $bookmark2->getTags()); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Delete 1 tag to 1 bookmark + */ + public function testDeleteOneTagOnOneBookmark(): void + { + $parameters = ['id' => '123', 'tag' => 'second', 'action' => 'delete']; + + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + $bookmark = (new Bookmark()) + ->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123') + ->setTagsString('first second third'); + + static::assertSame(['first', 'second', 'third'], $bookmark->getTags()); + + $this->container->bookmarkService->expects(static::once())->method('get')->with(123)->willReturn($bookmark); + $this->container->bookmarkService->expects(static::once())->method('set')->with($bookmark, false); + $this->container->bookmarkService->expects(static::once())->method('save'); + $this->container->formatterFactory = $this->createMock(FormatterFactory::class); + $this->container->formatterFactory + ->expects(static::once()) + ->method('getFormatter') + ->with('raw') + ->willReturnCallback(function () use ($bookmark): BookmarkFormatter { + return new BookmarkRawFormatter($this->container->conf, true); + }) + ; + + // Make sure that PluginManager hook is triggered + $this->container->pluginManager + ->expects(static::once()) + ->method('executeHooks') + ->with('save_link') + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(['first', 'third'], $bookmark->getTags()); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Delete 2 tags to 2 bookmarks + */ + public function testDeleteTwoTagOnTwoBookmarks(): void + { + $parameters = ['id' => '123 456', 'tag' => 'second@first', 'action' => 'delete']; + + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + $bookmark1 = (new Bookmark()) + ->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123') + ->setTagsString('first second third other'); + $bookmark2 = (new Bookmark()) + ->setId(456)->setUrl('http://domain.tld')->setTitle('Title 123') + ->setTagsString('first second'); + + static::assertSame(['first', 'second', 'third', 'other'], $bookmark1->getTags()); + static::assertSame(['first', 'second'], $bookmark2->getTags()); + + $this->container->bookmarkService->expects(static::exactly(2))->method('get') + ->withConsecutive([123], [456]) + ->willReturnOnConsecutiveCalls($bookmark1, $bookmark2); + $this->container->bookmarkService->expects(static::exactly(2))->method('set') + ->withConsecutive([$bookmark1, false], [$bookmark2, false]); + $this->container->bookmarkService->expects(static::once())->method('save'); + $this->container->formatterFactory = $this->createMock(FormatterFactory::class); + $this->container->formatterFactory + ->expects(static::once()) + ->method('getFormatter') + ->with('raw') + ->willReturnCallback(function (): BookmarkFormatter { + return new BookmarkRawFormatter($this->container->conf, true); + }) + ; + + // Make sure that PluginManager hook is triggered + $this->container->pluginManager + ->expects(static::exactly(2)) + ->method('executeHooks') + ->with('save_link') + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(['third', 'other'], $bookmark1->getTags()); + static::assertSame([], $bookmark2->getTags()); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Test add a tag without passing an ID. + */ + public function testAddTagWithoutId(): void + { + $parameters = ['tag' => 'newtag', 'action' => 'add']; + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + + $this->container->sessionManager + ->expects(static::once()) + ->method('setSessionParameter') + ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid bookmark ID provided.']) + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Test add a tag without passing an ID. + */ + public function testDeleteTagWithoutId(): void + { + $parameters = ['tag' => 'newtag', 'action' => 'delete']; + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + + $this->container->sessionManager + ->expects(static::once()) + ->method('setSessionParameter') + ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid bookmark ID provided.']) + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Test add a tag without passing an action. + */ + public function testAddTagWithoutAction(): void + { + $parameters = ['id' => '123', 'tag' => 'newtag']; + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + + $this->container->sessionManager + ->expects(static::once()) + ->method('setSessionParameter') + ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid action provided.']) + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Test add a tag without passing a tag string value. + */ + public function testAddTagWithoutValue(): void + { + $parameters = ['id' => '123', 'tag' => '', 'action' => 'add']; + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + + $this->container->sessionManager + ->expects(static::once()) + ->method('setSessionParameter') + ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid tag name provided.']) + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } + + /** + * Test delete a tag without passing a tag string value. + */ + public function testDeleteTagWithoutValue(): void + { + $parameters = ['id' => '123', 'tag' => '', 'action' => 'delete']; + $request = $this->createMock(Request::class); + $request + ->method('getParam') + ->willReturnCallback(function (string $key) use ($parameters): ?string { + return $parameters[$key] ?? null; + }) + ; + $response = new Response(); + + $this->container->sessionManager + ->expects(static::once()) + ->method('setSessionParameter') + ->with(SessionManager::KEY_ERROR_MESSAGES, ['Invalid tag name provided.']) + ; + + $result = $this->controller->addOrDeleteTags($request, $response); + + static::assertSame(302, $result->getStatusCode()); + static::assertSame(['/subfolder/'], $result->getHeader('location')); + } +} diff --git a/tpl/default/page.header.html b/tpl/default/page.header.html index ed5a09e8..bd1397bf 100644 --- a/tpl/default/page.header.html +++ b/tpl/default/page.header.html @@ -131,10 +131,46 @@ {'Set private'|t} +   + + + {'Add tag'|t} +   + + + {'Delete tag'|t} + + {$addDelete=['add', 'delete']} + {loop="$addDelete"} +
+
+
+ + + + + +   + + + {$value === 'add' ? t('Add tag') : t('Delete tag')} +   + {'Cancel'|t} +
+
+
+ {/loop} + {if="!$is_logged_in"}