diff --git a/db/updates/2026-03-14-user-bookmarks.sql b/db/updates/2026-03-14-user-bookmarks.sql index 48688b7..f95c6b6 100644 --- a/db/updates/2026-03-14-user-bookmarks.sql +++ b/db/updates/2026-03-14-user-bookmarks.sql @@ -30,6 +30,7 @@ CREATE TABLE IF NOT EXISTS `user_bookmarks` ( PRIMARY KEY (`id`), UNIQUE KEY `uniq_ub_user_url` (`user_id`, `url`), KEY `idx_ub_user_sort` (`user_id`, `sort_order`), + KEY `idx_ub_user_group_sort` (`user_id`, `group_id`, `sort_order`), KEY `idx_ub_group` (`group_id`), CONSTRAINT `fk_ub_user` FOREIGN KEY (`user_id`) REFERENCES `users` (`id`) ON DELETE CASCADE, CONSTRAINT `fk_ub_group` FOREIGN KEY (`group_id`) REFERENCES `user_bookmark_groups` (`id`) ON DELETE SET NULL diff --git a/lib/Repository/User/BookmarkGroupRepository.php b/lib/Repository/User/BookmarkGroupRepository.php index e0f7f06..ed6fd5c 100644 --- a/lib/Repository/User/BookmarkGroupRepository.php +++ b/lib/Repository/User/BookmarkGroupRepository.php @@ -3,50 +3,31 @@ namespace MintyPHP\Repository\User; use MintyPHP\DB; +use MintyPHP\Repository\Support\RepoQuery; +use MintyPHP\Repository\Support\RepositoryArrayHelper; class BookmarkGroupRepository implements BookmarkGroupRepositoryInterface { private function unwrapList(mixed $rows): array { - if (!is_array($rows)) { - return []; - } - $list = []; - foreach ($rows as $row) { - $data = $row['user_bookmark_groups'] ?? $row; - if (is_array($data)) { - $list[] = $data; - } - } - return $list; + return RepositoryArrayHelper::unwrapList($rows, 'user_bookmark_groups'); } - public function listByUser(int $userId): array + public function listByUser(int $userId, array $options = []): array { if ($userId <= 0) { return []; } + [$limit, $offset] = RepoQuery::sanitizeLimitOffset($options, 100, 1, 100); $rows = DB::select( - 'select id, user_id, name, icon, sort_order, created, modified from user_bookmark_groups where user_id = ? order by sort_order, id', - (string) $userId + 'select id, user_id, name, icon, sort_order, created, modified from user_bookmark_groups where user_id = ? order by sort_order, id limit ? offset ?', + (string) $userId, + (string) $limit, + (string) $offset ); return $this->unwrapList($rows); } - public function nextGroupSortOrder(int $userId): int - { - if ($userId <= 0) { - return 1; - } - - $max = DB::selectValue( - 'select COALESCE(max(sort_order), 0) from user_bookmark_groups where user_id = ?', - (string) $userId - ); - - return max(1, ((int) $max) + 1); - } - public function countByUser(int $userId): int { if ($userId <= 0) { diff --git a/lib/Repository/User/BookmarkGroupRepositoryInterface.php b/lib/Repository/User/BookmarkGroupRepositoryInterface.php index 1f3af4f..8d86b95 100644 --- a/lib/Repository/User/BookmarkGroupRepositoryInterface.php +++ b/lib/Repository/User/BookmarkGroupRepositoryInterface.php @@ -4,10 +4,11 @@ namespace MintyPHP\Repository\User; interface BookmarkGroupRepositoryInterface { - /** @return list> */ - public function listByUser(int $userId): array; - - public function nextGroupSortOrder(int $userId): int; + /** + * @param array{limit?: int, offset?: int} $options + * @return list> + */ + public function listByUser(int $userId, array $options = []): array; public function countByUser(int $userId): int; diff --git a/lib/Repository/User/BookmarkRepositoryInterface.php b/lib/Repository/User/BookmarkRepositoryInterface.php index cfb56f4..0bba73b 100644 --- a/lib/Repository/User/BookmarkRepositoryInterface.php +++ b/lib/Repository/User/BookmarkRepositoryInterface.php @@ -4,8 +4,11 @@ namespace MintyPHP\Repository\User; interface BookmarkRepositoryInterface { - /** @return list> */ - public function listByUser(int $userId): array; + /** + * @param array{limit?: int, offset?: int} $options + * @return list> + */ + public function listByUser(int $userId, array $options = []): array; public function nextBookmarkSortOrder(int $userId, ?int $groupId): int; @@ -26,5 +29,8 @@ interface BookmarkRepositoryInterface /** @param list $idOrderPairs */ public function updateSortOrders(int $userId, array $idOrderPairs): bool; - public function clearGroupId(int $groupId, int $userId): bool; + /** @return list> */ + public function listByGroup(int $userId, int $groupId): array; + + public function unsetGroupId(int $bookmarkId, int $userId, int $sortOrder): bool; } diff --git a/lib/Support/BookmarkUrlNormalizer.php b/lib/Support/BookmarkUrlNormalizer.php index 3929590..4c5d500 100644 --- a/lib/Support/BookmarkUrlNormalizer.php +++ b/lib/Support/BookmarkUrlNormalizer.php @@ -7,10 +7,12 @@ final class BookmarkUrlNormalizer public static function canonicalizeRelative(string $url): string { $url = trim($url); - if ($url === '') { + if ($url === '' || str_contains($url, "\x00")) { return ''; } + // Strip fragments: they are client-side only and should not + // differentiate bookmarks during deduplication. $hashPos = strpos($url, '#'); if ($hashPos !== false) { $url = substr($url, 0, $hashPos); @@ -20,7 +22,7 @@ final class BookmarkUrlNormalizer return ''; } - if (preg_match('#^[a-z][a-z0-9+.-]*://#i', $url) || str_starts_with($url, '//')) { + if (preg_match('#^[a-z][a-z0-9+.-]*:#i', $url) || str_starts_with($url, '//')) { return ''; } @@ -77,6 +79,7 @@ final class BookmarkUrlNormalizer $segments = explode('/', $collapsed); $normalized = []; + $depth = 0; foreach ($segments as $segment) { $segment = trim($segment); @@ -85,11 +88,16 @@ final class BookmarkUrlNormalizer } if ($segment === '..') { + if ($depth <= 0) { + return ''; + } array_pop($normalized); + $depth--; continue; } $normalized[] = $segment; + $depth++; } return implode('/', $normalized); diff --git a/pages/bookmarks/update-data().php b/pages/bookmarks/update-data().php index ffb8fd3..3fd859c 100644 --- a/pages/bookmarks/update-data().php +++ b/pages/bookmarks/update-data().php @@ -22,7 +22,18 @@ if (!Session::checkCsrfToken()) { $session = app(SessionStoreInterface::class)->all(); $userId = (int) ($session['user']['id'] ?? 0); +if ($userId <= 0) { + http_response_code(401); + Router::json(['ok' => false, 'error' => 'unauthorized']); + return; +} + $bookmarkId = (int) requestInput()->body('id'); +if ($bookmarkId <= 0) { + http_response_code(400); + Router::json(['ok' => false, 'error' => 'invalid_bookmark_id']); + return; +} $data = []; if (requestInput()->body('name') !== null) { @@ -37,6 +48,8 @@ $result = $service->updateBookmark($userId, $bookmarkId, $data); if ($result['ok']) { app(SessionStoreInterface::class)->set('user_bookmarks', $service->listGroupedForUser($userId)); +} else { + http_response_code(400); } Router::json($result); diff --git a/templates/partials/app-bookmark-dialog.phtml b/templates/partials/app-bookmark-dialog.phtml index 604f409..c05c4bf 100644 --- a/templates/partials/app-bookmark-dialog.phtml +++ b/templates/partials/app-bookmark-dialog.phtml @@ -1,13 +1,15 @@ get('user_bookmarks')) ? app(SessionStoreInterface::class)->get('user_bookmarks') : []; $bmSessionGroups = is_array($bmSessionData['groups'] ?? null) ? $bmSessionData['groups'] : []; ?> assertSame('updated', $result['mode']); } - public function testSaveBookmarkUpsertsLegacyBookmarkByCanonicalUrl(): void + public function testSaveBookmarkCreatesWhenNoExactUrlMatch(): void { $this->bookmarkRepo->expects($this->once())->method('findByUserAndUrl')->with(1, 'admin/users?a=1&b=2')->willReturn(false); - $this->bookmarkRepo->expects($this->once())->method('listByUser')->with(1)->willReturn([ - ['id' => 7, 'url' => 'admin/users?b=2&a=1'], - ]); - $this->bookmarkRepo->expects($this->once())->method('update')->with( - 7, - 1, - [ - 'name' => 'Users', - 'group_id' => null, - 'url' => 'admin/users?a=1&b=2', - ] - )->willReturn(true); - $this->bookmarkRepo->expects($this->never())->method('create'); + $this->bookmarkRepo->method('countByUser')->willReturn(0); + $this->navigationRepo->method('nextRootSortOrder')->willReturn(1); + $this->bookmarkRepo->expects($this->once())->method('create')->willReturn(7); $result = $this->service->saveBookmark(1, 'Users', 'admin/users?a=1&b=2', null); $this->assertTrue($result['ok']); - $this->assertSame('updated', $result['mode']); + $this->assertSame('created', $result['mode']); $this->assertSame(7, $result['bookmark']['id']); } @@ -338,22 +328,46 @@ class BookmarkServiceTest extends TestCase $this->assertSame('bi-heart', $result['group']['icon']); } - public function testDeleteGroupClearsBookmarks(): void + public function testDeleteGroupUnsetsGroupAndDeletes(): void { - $this->bookmarkRepo->expects($this->once())->method('clearGroupId')->with(5, 1)->willReturn(true); - $this->groupRepo->expects($this->any())->method('delete')->with(5, 1)->willReturn(true); + $this->bookmarkRepo->expects($this->once())->method('listByGroup')->with(1, 5)->willReturn([ + ['id' => 10, 'name' => 'A', 'url' => 'admin', 'group_id' => 5, 'sort_order' => 1], + ['id' => 11, 'name' => 'B', 'url' => 'admin/users', 'group_id' => 5, 'sort_order' => 2], + ]); + $this->navigationRepo->expects($this->once())->method('nextRootSortOrder')->with(1)->willReturn(20); + $this->bookmarkRepo->expects($this->exactly(2))->method('unsetGroupId') + ->willReturnCallback(fn (int $bmId, int $userId, int $sort) => match ([$bmId, $userId]) { + [10, 1] => $sort === 20, + [11, 1] => $sort === 21, + default => false, + }); + $this->groupRepo->expects($this->once())->method('delete')->with(5, 1)->willReturn(true); $this->assertTrue($this->service->deleteGroup(1, 5)); } - public function testDeleteGroupReturnsFalseWhenClearFails(): void + public function testDeleteGroupReturnsFalseWhenUnsetFails(): void { - $this->bookmarkRepo->expects($this->once())->method('clearGroupId')->with(5, 1)->willReturn(false); + $this->bookmarkRepo->expects($this->once())->method('listByGroup')->with(1, 5)->willReturn([ + ['id' => 10, 'name' => 'A', 'url' => 'admin', 'group_id' => 5, 'sort_order' => 1], + ]); + $this->navigationRepo->expects($this->once())->method('nextRootSortOrder')->with(1)->willReturn(20); + $this->bookmarkRepo->expects($this->once())->method('unsetGroupId')->with(10, 1, 20)->willReturn(false); $this->groupRepo->expects($this->never())->method('delete'); $this->assertFalse($this->service->deleteGroup(1, 5)); } + public function testDeleteGroupWithNoBookmarksDeletesDirectly(): void + { + $this->bookmarkRepo->expects($this->once())->method('listByGroup')->with(1, 5)->willReturn([]); + $this->navigationRepo->expects($this->once())->method('nextRootSortOrder')->with(1)->willReturn(1); + $this->bookmarkRepo->expects($this->never())->method('unsetGroupId'); + $this->groupRepo->expects($this->once())->method('delete')->with(5, 1)->willReturn(true); + + $this->assertTrue($this->service->deleteGroup(1, 5)); + } + public function testReorderGroupsDelegatesToRepository(): void { $pairs = [ @@ -413,6 +427,60 @@ class BookmarkServiceTest extends TestCase $this->assertArrayNotHasKey('icon', $result['ungrouped'][0]); } + public function testReorderFromItemsRejectsInvalidType(): void + { + $result = $this->service->reorderFromItems(1, 'invalid', []); + + $this->assertFalse($result['ok']); + $this->assertSame('invalid_type', $result['error']); + } + + public function testReorderFromItemsRejectsEmptyItems(): void + { + $result = $this->service->reorderFromItems(1, 'bookmarks', []); + + $this->assertFalse($result['ok']); + $this->assertSame('invalid_items', $result['error']); + } + + public function testReorderFromItemsBookmarksHappyPath(): void + { + $this->bookmarkRepo->expects($this->once())->method('updateSortOrders')->with(1, [ + ['id' => 3, 'sort_order' => 1], + ['id' => 7, 'sort_order' => 2], + ])->willReturn(true); + + $result = $this->service->reorderFromItems(1, 'bookmarks', [ + ['id' => 3, 'sort_order' => 1], + ['id' => 7, 'sort_order' => 2], + ]); + + $this->assertTrue($result['ok']); + } + + public function testReorderRootFromItemsHappyPath(): void + { + $this->navigationRepo->expects($this->once())->method('updateRootSortOrders')->with(1, [ + ['kind' => 'group', 'id' => 2, 'sort_order' => 1], + ['kind' => 'bookmark', 'id' => 5, 'sort_order' => 2], + ])->willReturn(true); + + $result = $this->service->reorderRootFromItems(1, [ + ['kind' => 'group', 'id' => 2, 'sort_order' => 1], + ['kind' => 'bookmark', 'id' => 5, 'sort_order' => 2], + ]); + + $this->assertTrue($result['ok']); + } + + public function testReorderRootFromItemsRejectsEmptyItems(): void + { + $result = $this->service->reorderRootFromItems(1, []); + + $this->assertFalse($result['ok']); + $this->assertSame('invalid_items', $result['error']); + } + public function testAllowedGroupIconsReturnsNonEmptyList(): void { $icons = BookmarkService::allowedGroupIcons(); diff --git a/web/css/components/app-bookmark-form.css b/web/css/components/app-bookmark-form.css index 67f7d58..7697fad 100644 --- a/web/css/components/app-bookmark-form.css +++ b/web/css/components/app-bookmark-form.css @@ -108,7 +108,7 @@ color: var(--app-primary); } - .app-bookmark-group-icon-option.active { + .app-bookmark-group-icon-option.app-bookmark-icon-active { border-color: var(--app-primary); background: var(--app-primary); color: #fff; diff --git a/web/js/components/app-bookmark-save.js b/web/js/components/app-bookmark-save.js index e465e77..5f97cb1 100644 --- a/web/js/components/app-bookmark-save.js +++ b/web/js/components/app-bookmark-save.js @@ -5,29 +5,43 @@ import { showAsyncFlash } from './app-async-flash.js'; import { warnOnce } from '../core/app-dom.js'; +import { getAppBase } from '../pages/app-list-utils.js'; -export function init() { - const openBtn = document.querySelector('[data-bookmark-open-dialog]'); - const bookmarkDialog = document.querySelector('[data-app-bookmark-dialog]'); - if (!(bookmarkDialog instanceof HTMLDialogElement) || !(openBtn instanceof HTMLButtonElement)) return; +export function initBookmarkSave(root = document) { + const host = root && typeof root.querySelector === 'function' ? root : document; + const openBtn = host.querySelector('[data-bookmark-open-dialog]') || document.querySelector('[data-bookmark-open-dialog]'); + const bookmarkDialog = host.matches?.('[data-app-bookmark-dialog]') + ? host + : (host.querySelector('[data-app-bookmark-dialog]') || document.querySelector('[data-app-bookmark-dialog]')); + if (!(bookmarkDialog instanceof HTMLDialogElement) || !(openBtn instanceof HTMLButtonElement)) { + return { destroy: () => {} }; + } + if (bookmarkDialog.dataset.bookmarkSaveBound === '1' && bookmarkDialog._bookmarkSaveApi) { + return bookmarkDialog._bookmarkSaveApi; + } const bookmarkForm = bookmarkDialog.querySelector('[data-bookmark-save-form]'); - if (!(bookmarkForm instanceof HTMLFormElement)) return; + if (!(bookmarkForm instanceof HTMLFormElement)) { + return { destroy: () => {} }; + } const groupDialog = document.querySelector('[data-app-bookmark-group-dialog]'); const groupForm = groupDialog instanceof HTMLDialogElement ? groupDialog.querySelector('[data-bookmark-group-save-form]') : null; const hasGroupDialog = groupDialog instanceof HTMLDialogElement && groupForm instanceof HTMLFormElement; + const abortController = new AbortController(); + const { signal } = abortController; + const timerIds = []; - const root = document.documentElement; - const csrfKey = String(root.dataset.csrfKey || '').trim(); - const csrfToken = String(root.dataset.csrfToken || '').trim(); + const documentRoot = document.documentElement; + const csrfKey = String(documentRoot.dataset.csrfKey || '').trim(); + const csrfToken = String(documentRoot.dataset.csrfToken || '').trim(); const moduleContext = { module: 'bookmark-save' }; if (!csrfKey || !csrfToken) { warnOnce('UI_DATA_MISSING', 'Missing CSRF metadata for bookmark dialog', moduleContext); - return; + return { destroy: () => {} }; } const nameInput = bookmarkForm.querySelector('input[name="name"]'); @@ -83,7 +97,8 @@ export function init() { const runReloadWithSuccess = (message) => { showAsyncFlash('success', message, 2500); - window.setTimeout(() => window.location.reload(), 450); + const timerId = window.setTimeout(() => window.location.reload(), 450); + timerIds.push(timerId); }; const closeBookmarkDialog = () => { @@ -210,33 +225,33 @@ export function init() { if (openBookmarkDialog() && nameInput instanceof HTMLInputElement) { nameInput.select(); } - }); + }, { signal }); document.addEventListener('app:bookmark-edit', (event) => { const detail = event instanceof CustomEvent ? event.detail : null; openBookmarkEditFromSidebar(detail); - }); + }, { signal }); closeBookmarkBtns.forEach((btn) => { btn.addEventListener('click', (event) => { event.preventDefault(); closeBookmarkDialog(); - }); + }, { signal }); }); bookmarkDialog.addEventListener('click', (event) => { if (event.target === bookmarkDialog) { closeBookmarkDialog(); } - }); + }, { signal }); bookmarkDialog.addEventListener('close', () => { bookmarkForm.removeAttribute('aria-busy'); syncModalBodyClass(); - }); + }, { signal }); bookmarkDialog.addEventListener('cancel', () => { bookmarkForm.removeAttribute('aria-busy'); syncModalBodyClass(); - }); + }, { signal }); // Save bookmark bookmarkForm.addEventListener('submit', async (event) => { @@ -310,10 +325,22 @@ export function init() { } finally { setBookmarkSubmitPending(false); } - }); + }, { signal }); + + const destroy = () => { + abortController.abort(); + timerIds.forEach((timerId) => window.clearTimeout(timerId)); + timerIds.length = 0; + bookmarkForm.removeAttribute('aria-busy'); + delete bookmarkDialog.dataset.bookmarkSaveBound; + delete bookmarkDialog._bookmarkSaveApi; + }; + const api = { destroy }; + bookmarkDialog.dataset.bookmarkSaveBound = '1'; + bookmarkDialog._bookmarkSaveApi = api; if (!hasGroupDialog) { - return; + return api; } const groupNameInput = groupForm.querySelector('input[name="name"]'); @@ -381,10 +408,10 @@ export function init() { groupIconInput.value = value; } if (groupIconPicker instanceof HTMLElement) { - groupIconPicker.querySelectorAll('.active').forEach((item) => item.classList.remove('active')); + groupIconPicker.querySelectorAll('.app-bookmark-icon-active').forEach((item) => item.classList.remove('app-bookmark-icon-active')); const selected = groupIconPicker.querySelector(`[data-icon="${value}"]`); if (selected instanceof HTMLElement) { - selected.classList.add('active'); + selected.classList.add('app-bookmark-icon-active'); } } }; @@ -459,7 +486,7 @@ export function init() { newGroupBtn.addEventListener('click', (event) => { event.preventDefault(); openCreateGroupFromBookmarkDialog(); - }); + }, { signal }); } if (groupIconPicker instanceof HTMLElement) { @@ -468,29 +495,29 @@ export function init() { if (!(target instanceof HTMLElement)) return; event.preventDefault(); setGroupIcon(target.dataset.icon || 'bi-folder'); - }); + }, { signal }); } groupCloseBtns.forEach((btn) => { btn.addEventListener('click', (event) => { event.preventDefault(); handleGroupCancel(); - }); + }, { signal }); }); groupDialog.addEventListener('click', (event) => { if (event.target === groupDialog) { handleGroupCancel(); } - }); + }, { signal }); groupDialog.addEventListener('cancel', (event) => { event.preventDefault(); handleGroupCancel(); - }); + }, { signal }); groupDialog.addEventListener('close', () => { groupForm.removeAttribute('aria-busy'); syncModalBodyClass(); - }); + }, { signal }); document.addEventListener('app:bookmark-group-edit', (event) => { const detail = event instanceof CustomEvent ? event.detail : null; @@ -513,7 +540,7 @@ export function init() { groupNameInput.focus(); groupNameInput.select(); } - }); + }, { signal }); groupForm.addEventListener('submit', async (event) => { event.preventDefault(); @@ -574,12 +601,9 @@ export function init() { } finally { setGroupSubmitPending(false); } - }); -} + }, { signal }); -function getAppBase() { - const base = document.querySelector('base'); - return base ? base.href : '/'; + return api; } function currentRelativeUrl() {