diff --git a/bookmarks/e2e/e2e_test_bookmark_page_bulk_edit.py b/bookmarks/e2e/e2e_test_bookmark_page_bulk_edit.py new file mode 100644 index 0000000..d9ca896 --- /dev/null +++ b/bookmarks/e2e/e2e_test_bookmark_page_bulk_edit.py @@ -0,0 +1,232 @@ +from django.urls import reverse +from playwright.sync_api import sync_playwright, expect + +from bookmarks.e2e.helpers import LinkdingE2ETestCase +from bookmarks.models import Bookmark + + +class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): + def setup_test_data(self): + self.setup_numbered_bookmarks(50) + self.setup_numbered_bookmarks(50, archived=True) + self.setup_numbered_bookmarks(50, prefix='foo') + self.setup_numbered_bookmarks(50, archived=True, prefix='foo') + + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='Archived Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='foo').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='foo').count()) + + def test_active_bookmarks_bulk_select_across(self): + self.setup_test_data() + + with sync_playwright() as p: + self.open(reverse('bookmarks:index'), p) + + self.locate_bulk_edit_toggle().click() + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() + self.locate_bulk_edit_bar().get_by_text('Confirm').click() + + self.assertEqual(0, Bookmark.objects.filter(is_archived=False, title__startswith='Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='Archived Bookmark').count()) + self.assertEqual(0, Bookmark.objects.filter(is_archived=False, title__startswith='foo').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='foo').count()) + + def test_archived_bookmarks_bulk_select_across(self): + self.setup_test_data() + + with sync_playwright() as p: + self.open(reverse('bookmarks:archived'), p) + + self.locate_bulk_edit_toggle().click() + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() + self.locate_bulk_edit_bar().get_by_text('Confirm').click() + + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='Bookmark').count()) + self.assertEqual(0, Bookmark.objects.filter(is_archived=True, title__startswith='Archived Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='foo').count()) + self.assertEqual(0, Bookmark.objects.filter(is_archived=True, title__startswith='foo').count()) + + def test_active_bookmarks_bulk_select_across_respects_query(self): + self.setup_test_data() + + with sync_playwright() as p: + self.open(reverse('bookmarks:index') + '?q=foo', p) + + self.locate_bulk_edit_toggle().click() + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() + self.locate_bulk_edit_bar().get_by_text('Confirm').click() + + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='Archived Bookmark').count()) + self.assertEqual(0, Bookmark.objects.filter(is_archived=False, title__startswith='foo').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='foo').count()) + + def test_archived_bookmarks_bulk_select_across_respects_query(self): + self.setup_test_data() + + with sync_playwright() as p: + self.open(reverse('bookmarks:archived') + '?q=foo', p) + + self.locate_bulk_edit_toggle().click() + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() + self.locate_bulk_edit_bar().get_by_text('Confirm').click() + + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=True, title__startswith='Archived Bookmark').count()) + self.assertEqual(50, Bookmark.objects.filter(is_archived=False, title__startswith='foo').count()) + self.assertEqual(0, Bookmark.objects.filter(is_archived=True, title__startswith='foo').count()) + + def test_select_all_toggles_all_checkboxes(self): + self.setup_numbered_bookmarks(5) + + with sync_playwright() as p: + url = reverse('bookmarks:index') + page = self.open(url, p) + + self.locate_bulk_edit_toggle().click() + + checkboxes = page.locator('label[ld-bulk-edit-checkbox] input') + self.assertEqual(6, checkboxes.count()) + for i in range(checkboxes.count()): + expect(checkboxes.nth(i)).not_to_be_checked() + + self.locate_bulk_edit_select_all().click() + + for i in range(checkboxes.count()): + expect(checkboxes.nth(i)).to_be_checked() + + self.locate_bulk_edit_select_all().click() + + for i in range(checkboxes.count()): + expect(checkboxes.nth(i)).not_to_be_checked() + + def test_select_all_shows_select_across(self): + self.setup_numbered_bookmarks(5) + + with sync_playwright() as p: + url = reverse('bookmarks:index') + self.open(url, p) + + self.locate_bulk_edit_toggle().click() + + expect(self.locate_bulk_edit_select_across()).not_to_be_visible() + + self.locate_bulk_edit_select_all().click() + expect(self.locate_bulk_edit_select_across()).to_be_visible() + + self.locate_bulk_edit_select_all().click() + expect(self.locate_bulk_edit_select_across()).not_to_be_visible() + + def test_select_across_is_unchecked_when_toggling_all(self): + self.setup_numbered_bookmarks(5) + + with sync_playwright() as p: + url = reverse('bookmarks:index') + self.open(url, p) + + self.locate_bulk_edit_toggle().click() + + # Show select across, check it + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + expect(self.locate_bulk_edit_select_across()).to_be_checked() + + # Hide select across by toggling select all + self.locate_bulk_edit_select_all().click() + expect(self.locate_bulk_edit_select_across()).not_to_be_visible() + + # Show select across again, verify it is unchecked + self.locate_bulk_edit_select_all().click() + expect(self.locate_bulk_edit_select_across()).not_to_be_checked() + + def test_select_across_is_unchecked_when_toggling_bookmark(self): + self.setup_numbered_bookmarks(5) + + with sync_playwright() as p: + url = reverse('bookmarks:index') + self.open(url, p) + + self.locate_bulk_edit_toggle().click() + + # Show select across, check it + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + expect(self.locate_bulk_edit_select_across()).to_be_checked() + + # Hide select across by toggling a single bookmark + self.locate_bookmark('Bookmark 1').locator('label[ld-bulk-edit-checkbox]').click() + expect(self.locate_bulk_edit_select_across()).not_to_be_visible() + + # Show select across again, verify it is unchecked + self.locate_bookmark('Bookmark 1').locator('label[ld-bulk-edit-checkbox]').click() + expect(self.locate_bulk_edit_select_across()).not_to_be_checked() + + def test_execute_resets_all_checkboxes(self): + self.setup_numbered_bookmarks(100) + + with sync_playwright() as p: + url = reverse('bookmarks:index') + page = self.open(url, p) + + # Select all bookmarks, enable select across + self.locate_bulk_edit_toggle().click() + self.locate_bulk_edit_select_all().click() + self.locate_bulk_edit_select_across().click() + + # Get reference for bookmark list + bookmark_list = page.locator('ul[ld-bookmark-list]') + + # Execute bulk action + self.select_bulk_action('Mark as unread') + self.locate_bulk_edit_bar().get_by_text('Execute').click() + self.locate_bulk_edit_bar().get_by_text('Confirm').click() + + # Wait until bookmark list is updated (old reference becomes invisible) + expect(bookmark_list).not_to_be_visible() + + # Verify bulk edit checkboxes are reset + checkboxes = page.locator('label[ld-bulk-edit-checkbox] input') + self.assertEqual(31, checkboxes.count()) + for i in range(checkboxes.count()): + expect(checkboxes.nth(i)).not_to_be_checked() + + # Toggle select all and verify select across is reset + self.locate_bulk_edit_select_all().click() + expect(self.locate_bulk_edit_select_across()).not_to_be_checked() + + def test_update_select_across_bookmark_count(self): + self.setup_numbered_bookmarks(100) + + with sync_playwright() as p: + url = reverse('bookmarks:index') + self.open(url, p) + + self.locate_bulk_edit_toggle().click() + self.locate_bulk_edit_select_all().click() + + expect(self.locate_bulk_edit_bar().get_by_text('All pages (100 bookmarks)')).to_be_visible() + + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() + self.locate_bulk_edit_bar().get_by_text('Confirm').click() + + self.locate_bulk_edit_select_all().click() + + expect(self.locate_bulk_edit_bar().get_by_text('All pages (70 bookmarks)')).to_be_visible() diff --git a/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py b/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py index 47ec11a..a883c56 100644 --- a/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py +++ b/bookmarks/e2e/e2e_test_bookmark_page_partial_updates.py @@ -150,7 +150,8 @@ class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): self.locate_bulk_edit_toggle().click() self.locate_bookmark('Bookmark 2').locator('label[ld-bulk-edit-checkbox]').click() - self.locate_bulk_edit_bar().get_by_text('Archive').click() + self.select_bulk_action('Archive') + self.locate_bulk_edit_bar().get_by_text('Execute').click() self.locate_bulk_edit_bar().get_by_text('Confirm').click() self.assertVisibleBookmarks(['Bookmark 1', 'Bookmark 3']) @@ -165,7 +166,8 @@ class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): self.locate_bulk_edit_toggle().click() self.locate_bookmark('Bookmark 2').locator('label[ld-bulk-edit-checkbox]').click() - self.locate_bulk_edit_bar().get_by_text('Delete').click() + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() self.locate_bulk_edit_bar().get_by_text('Confirm').click() self.assertVisibleBookmarks(['Bookmark 1', 'Bookmark 3']) @@ -197,7 +199,7 @@ class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): self.assertVisibleTags(['Archived Tag 1', 'Archived Tag 3']) self.assertReloads(0) - def test_archived_bookmarks_partial_update_on_bulk_archive(self): + def test_archived_bookmarks_partial_update_on_bulk_unarchive(self): self.setup_fixture() with sync_playwright() as p: @@ -205,7 +207,8 @@ class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): self.locate_bulk_edit_toggle().click() self.locate_bookmark('Archived Bookmark 2').locator('label[ld-bulk-edit-checkbox]').click() - self.locate_bulk_edit_bar().get_by_text('Archive').click() + self.select_bulk_action('Unarchive') + self.locate_bulk_edit_bar().get_by_text('Execute').click() self.locate_bulk_edit_bar().get_by_text('Confirm').click() self.assertVisibleBookmarks(['Archived Bookmark 1', 'Archived Bookmark 3']) @@ -220,7 +223,8 @@ class BookmarkPagePartialUpdatesE2ETestCase(LinkdingE2ETestCase): self.locate_bulk_edit_toggle().click() self.locate_bookmark('Archived Bookmark 2').locator('label[ld-bulk-edit-checkbox]').click() - self.locate_bulk_edit_bar().get_by_text('Delete').click() + self.select_bulk_action('Delete') + self.locate_bulk_edit_bar().get_by_text('Execute').click() self.locate_bulk_edit_bar().get_by_text('Confirm').click() self.assertVisibleBookmarks(['Archived Bookmark 1', 'Archived Bookmark 3']) diff --git a/bookmarks/e2e/helpers.py b/bookmarks/e2e/helpers.py index ef80625..f3931a2 100644 --- a/bookmarks/e2e/helpers.py +++ b/bookmarks/e2e/helpers.py @@ -41,5 +41,14 @@ class LinkdingE2ETestCase(LiveServerTestCase, BookmarkFactoryMixin): def locate_bulk_edit_bar(self): return self.page.locator('.bulk-edit-bar') + def locate_bulk_edit_select_all(self): + return self.locate_bulk_edit_bar().locator('label[ld-bulk-edit-checkbox][all]') + + def locate_bulk_edit_select_across(self): + return self.locate_bulk_edit_bar().locator('label.select-across') + def locate_bulk_edit_toggle(self): return self.page.get_by_title('Bulk edit') + + def select_bulk_action(self, value: str): + return self.locate_bulk_edit_bar().locator('select[name="bulk_action"]').select_option(value) diff --git a/bookmarks/frontend/behaviors/bookmark-page.js b/bookmarks/frontend/behaviors/bookmark-page.js index f535f90..eb160b2 100644 --- a/bookmarks/frontend/behaviors/bookmark-page.js +++ b/bookmarks/frontend/behaviors/bookmark-page.js @@ -36,8 +36,18 @@ class BookmarkPage { swap(this.bookmarkList, bookmarkListHtml); swap(this.tagCloud, tagCloudHtml); + // Dispatch list updated event + const listElement = this.bookmarkList.querySelector( + "ul[data-bookmarks-total]", + ); + const bookmarksTotal = + (listElement && listElement.dataset.bookmarksTotal) || 0; + this.bookmarkList.dispatchEvent( - new CustomEvent("bookmark-list-updated", { bubbles: true }), + new CustomEvent("bookmark-list-updated", { + bubbles: true, + detail: { bookmarksTotal }, + }), ); }); } diff --git a/bookmarks/frontend/behaviors/bulk-edit.js b/bookmarks/frontend/behaviors/bulk-edit.js index fd9092d..f0927c6 100644 --- a/bookmarks/frontend/behaviors/bulk-edit.js +++ b/bookmarks/frontend/behaviors/bulk-edit.js @@ -4,6 +4,9 @@ class BulkEdit { constructor(element) { this.element = element; this.active = false; + this.actionSelect = element.querySelector("select[name='bulk_action']"); + this.tagAutoComplete = element.querySelector(".tag-autocomplete"); + this.selectAcross = element.querySelector("label.select-across"); element.addEventListener( "bulk-edit-toggle-active", @@ -21,6 +24,11 @@ class BulkEdit { "bookmark-list-updated", this.onListUpdated.bind(this), ); + + this.actionSelect.addEventListener( + "change", + this.onActionSelected.bind(this), + ); } get allCheckbox() { @@ -48,23 +56,56 @@ class BulkEdit { } onToggleBookmark() { - this.allCheckbox.checked = this.bookmarkCheckboxes.every((checkbox) => { + const allChecked = this.bookmarkCheckboxes.every((checkbox) => { return checkbox.checked; }); + this.allCheckbox.checked = allChecked; + this.updateSelectAcross(allChecked); } onToggleAll() { - const checked = this.allCheckbox.checked; + const allChecked = this.allCheckbox.checked; this.bookmarkCheckboxes.forEach((checkbox) => { - checkbox.checked = checked; + checkbox.checked = allChecked; }); + this.updateSelectAcross(allChecked); } - onListUpdated() { + onActionSelected() { + const action = this.actionSelect.value; + + if (action === "bulk_tag" || action === "bulk_untag") { + this.tagAutoComplete.classList.remove("d-none"); + } else { + this.tagAutoComplete.classList.add("d-none"); + } + } + + onListUpdated(event) { + // Reset checkbox states + this.reset(); + + // Update total number of bookmarks + const total = event.detail.bookmarksTotal; + const totalSpan = this.selectAcross.querySelector("span.total"); + totalSpan.textContent = total; + } + + updateSelectAcross(allChecked) { + if (allChecked) { + this.selectAcross.classList.remove("d-none"); + } else { + this.selectAcross.classList.add("d-none"); + this.selectAcross.querySelector("input").checked = false; + } + } + + reset() { this.allCheckbox.checked = false; this.bookmarkCheckboxes.forEach((checkbox) => { checkbox.checked = false; }); + this.updateSelectAcross(false); } } diff --git a/bookmarks/frontend/behaviors/tag-autocomplete.js b/bookmarks/frontend/behaviors/tag-autocomplete.js index a7644ca..24e81a0 100644 --- a/bookmarks/frontend/behaviors/tag-autocomplete.js +++ b/bookmarks/frontend/behaviors/tag-autocomplete.js @@ -14,12 +14,13 @@ class TagAutocomplete { id: element.id, name: element.name, value: element.value, + placeholder: element.getAttribute("placeholder") || "", apiClient: apiClient, variant: element.getAttribute("variant"), }, }); - element.replaceWith(wrapper); + element.replaceWith(wrapper.firstElementChild); } } diff --git a/bookmarks/frontend/components/TagAutocomplete.svelte b/bookmarks/frontend/components/TagAutocomplete.svelte index 52270ab..779fcc9 100644 --- a/bookmarks/frontend/components/TagAutocomplete.svelte +++ b/bookmarks/frontend/components/TagAutocomplete.svelte @@ -4,6 +4,7 @@ export let id; export let name; export let value; + export let placeholder; export let apiClient; export let variant = 'default'; @@ -118,7 +119,7 @@
- @@ -152,6 +153,7 @@ .form-autocomplete.small .form-autocomplete-input { height: 1.4rem; min-height: 1.4rem; + padding: 0.05rem 0.3rem; } .form-autocomplete.small .form-autocomplete-input input { diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 49039fc..2f052e8 100644 --- a/bookmarks/services/bookmarks.py +++ b/bookmarks/services/bookmarks.py @@ -119,6 +119,34 @@ def untag_bookmarks(bookmark_ids: [Union[int, str]], tag_string: str, current_us Bookmark.objects.bulk_update(bookmarks, ['date_modified']) +def mark_bookmarks_as_read(bookmark_ids: [Union[int, str]], current_user: User): + sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) + bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) + + bookmarks.update(unread=False, date_modified=timezone.now()) + + +def mark_bookmarks_as_unread(bookmark_ids: [Union[int, str]], current_user: User): + sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) + bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) + + bookmarks.update(unread=True, date_modified=timezone.now()) + + +def share_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): + sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) + bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) + + bookmarks.update(shared=True, date_modified=timezone.now()) + + +def unshare_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): + sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) + bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) + + bookmarks.update(shared=False, date_modified=timezone.now()) + + def _merge_bookmark_data(from_bookmark: Bookmark, to_bookmark: Bookmark): to_bookmark.title = from_bookmark.title to_bookmark.description = from_bookmark.description diff --git a/bookmarks/styles/base.scss b/bookmarks/styles/base.scss index db31527..f39de6e 100644 --- a/bookmarks/styles/base.scss +++ b/bookmarks/styles/base.scss @@ -118,6 +118,10 @@ span.confirmation { margin-right: auto; } +.ml-auto { + margin-left: auto; +} + .btn.btn-wide { padding-left: $unit-6; padding-right: $unit-6; diff --git a/bookmarks/styles/bookmark-page.scss b/bookmarks/styles/bookmark-page.scss index afa88a8..cdba178 100644 --- a/bookmarks/styles/bookmark-page.scss +++ b/bookmarks/styles/bookmark-page.scss @@ -292,10 +292,15 @@ $bulk-edit-transition-duration: 400ms; text-decoration: underline; } - > input, .form-autocomplete { + > input, .form-autocomplete, select { width: auto; - max-width: 200px; + max-width: 140px; -webkit-appearance: none; } + + .select-across { + margin: 0 0 0 auto; + font-size: $font-size-sm; + } } } diff --git a/bookmarks/templates/bookmarks/archive.html b/bookmarks/templates/bookmarks/archive.html index b3cc1fb..80eaed4 100644 --- a/bookmarks/templates/bookmarks/archive.html +++ b/bookmarks/templates/bookmarks/archive.html @@ -20,10 +20,10 @@
-
{% csrf_token %} - {% include 'bookmarks/bulk_edit/bar.html' with mode='archive' %} + {% include 'bookmarks/bulk_edit/bar.html' with disable_actions='bulk_archive' %}
{% include 'bookmarks/bookmark_list.html' %} diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/bookmarks/templates/bookmarks/bookmark_list.html index 1176f84..55ad673 100644 --- a/bookmarks/templates/bookmarks/bookmark_list.html +++ b/bookmarks/templates/bookmarks/bookmark_list.html @@ -5,7 +5,8 @@ {% if bookmark_list.is_empty %} {% include 'bookmarks/empty_bookmarks.html' %} {% else %} -
- + {% csrf_token %} - {% include 'bookmarks/bulk_edit/bar.html' with mode='default' %} + {% include 'bookmarks/bulk_edit/bar.html' with disable_actions='bulk_unarchive' %}
{% include 'bookmarks/bookmark_list.html' %} diff --git a/bookmarks/templates/bookmarks/shared.html b/bookmarks/templates/bookmarks/shared.html index e9b1de3..7332aaa 100644 --- a/bookmarks/templates/bookmarks/shared.html +++ b/bookmarks/templates/bookmarks/shared.html @@ -16,7 +16,7 @@ {% bookmark_search bookmark_list.filters tag_cloud.tags mode='shared' %}
- {% csrf_token %} diff --git a/bookmarks/tests/helpers.py b/bookmarks/tests/helpers.py index eb8a6b6..d672546 100644 --- a/bookmarks/tests/helpers.py +++ b/bookmarks/tests/helpers.py @@ -100,11 +100,12 @@ class BookmarkFactoryMixin: for i in range(1, count + 1): title = f'{prefix} {i}{suffix}' + url = f'https://example.com/{prefix}/{i}' tags = [] if with_tags: tag_name = f'{tag_prefix} {i}{suffix}' tags = [self.setup_tag(name=tag_name)] - self.setup_bookmark(title=title, is_archived=archived, shared=shared, tags=tags, user=user) + self.setup_bookmark(url=url, title=title, is_archived=archived, shared=shared, tags=tags, user=user) def get_numbered_bookmark(self, title: str): return Bookmark.objects.get(title=title) @@ -251,3 +252,8 @@ def disable_logging(f): return result return wrapper + + +def collapse_whitespace(text: str): + text = text.replace('\n', '').replace('\r', '') + return ' '.join(text.split()) diff --git a/bookmarks/tests/test_bookmark_action_view.py b/bookmarks/tests/test_bookmark_action_view.py index 58d0ba4..1d2469f 100644 --- a/bookmarks/tests/test_bookmark_action_view.py +++ b/bookmarks/tests/test_bookmark_action_view.py @@ -22,7 +22,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): def test_archive_should_archive_bookmark(self): bookmark = self.setup_bookmark() - self.client.post(reverse('bookmarks:action'), { + self.client.post(reverse('bookmarks:index.action'), { 'archive': [bookmark.id], }) @@ -34,7 +34,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') bookmark = self.setup_bookmark(user=other_user) - response = self.client.post(reverse('bookmarks:action'), { + response = self.client.post(reverse('bookmarks:index.action'), { 'archive': [bookmark.id], }) @@ -46,7 +46,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): def test_unarchive_should_unarchive_bookmark(self): bookmark = self.setup_bookmark(is_archived=True) - self.client.post(reverse('bookmarks:action'), { + self.client.post(reverse('bookmarks:index.action'), { 'unarchive': [bookmark.id], }) bookmark.refresh_from_db() @@ -57,7 +57,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') bookmark = self.setup_bookmark(is_archived=True, user=other_user) - response = self.client.post(reverse('bookmarks:action'), { + response = self.client.post(reverse('bookmarks:index.action'), { 'unarchive': [bookmark.id], }) bookmark.refresh_from_db() @@ -68,7 +68,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): def test_delete_should_delete_bookmark(self): bookmark = self.setup_bookmark() - self.client.post(reverse('bookmarks:action'), { + self.client.post(reverse('bookmarks:index.action'), { 'remove': [bookmark.id], }) @@ -78,7 +78,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') bookmark = self.setup_bookmark(user=other_user) - response = self.client.post(reverse('bookmarks:action'), { + response = self.client.post(reverse('bookmarks:index.action'), { 'remove': [bookmark.id], }) self.assertEqual(response.status_code, 404) @@ -87,7 +87,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): def test_mark_as_read(self): bookmark = self.setup_bookmark(unread=True) - self.client.post(reverse('bookmarks:action'), { + self.client.post(reverse('bookmarks:index.action'), { 'mark_as_read': [bookmark.id], }) bookmark.refresh_from_db() @@ -97,7 +97,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): def test_unshare_should_unshare_bookmark(self): bookmark = self.setup_bookmark(shared=True) - self.client.post(reverse('bookmarks:action'), { + self.client.post(reverse('bookmarks:index.action'), { 'unshare': [bookmark.id], }) @@ -109,7 +109,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') bookmark = self.setup_bookmark(user=other_user, shared=True) - response = self.client.post(reverse('bookmarks:action'), { + response = self.client.post(reverse('bookmarks:index.action'), { 'unshare': [bookmark.id], }) @@ -123,8 +123,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - self.client.post(reverse('bookmarks:action'), { - 'bulk_archive': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -138,8 +139,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark(user=other_user) bookmark3 = self.setup_bookmark(user=other_user) - self.client.post(reverse('bookmarks:action'), { - 'bulk_archive': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -152,8 +154,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark(is_archived=True) bookmark3 = self.setup_bookmark(is_archived=True) - self.client.post(reverse('bookmarks:action'), { - 'bulk_unarchive': [''], + self.client.post(reverse('bookmarks:archived.action'), { + 'bulk_action': ['bulk_unarchive'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -167,8 +170,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark(is_archived=True, user=other_user) bookmark3 = self.setup_bookmark(is_archived=True, user=other_user) - self.client.post(reverse('bookmarks:action'), { - 'bulk_unarchive': [''], + self.client.post(reverse('bookmarks:archived.action'), { + 'bulk_action': ['bulk_unarchive'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -181,8 +185,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - self.client.post(reverse('bookmarks:action'), { - 'bulk_delete': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -196,8 +201,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark(user=other_user) bookmark3 = self.setup_bookmark(user=other_user) - self.client.post(reverse('bookmarks:action'), { - 'bulk_delete': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -212,8 +218,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): tag1 = self.setup_tag() tag2 = self.setup_tag() - self.client.post(reverse('bookmarks:action'), { - 'bulk_tag': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_tag'], + 'bulk_execute': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -234,8 +241,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): tag1 = self.setup_tag() tag2 = self.setup_tag() - self.client.post(reverse('bookmarks:action'), { - 'bulk_tag': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_tag'], + 'bulk_execute': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -255,8 +263,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark(tags=[tag1, tag2]) bookmark3 = self.setup_bookmark(tags=[tag1, tag2]) - self.client.post(reverse('bookmarks:action'), { - 'bulk_untag': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_untag'], + 'bulk_execute': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -277,8 +286,9 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark(tags=[tag1, tag2], user=other_user) bookmark3 = self.setup_bookmark(tags=[tag1, tag2], user=other_user) - self.client.post(reverse('bookmarks:action'), { - 'bulk_untag': [''], + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_untag'], + 'bulk_execute': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -291,18 +301,240 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): self.assertCountEqual(bookmark2.tags.all(), [tag1, tag2]) self.assertCountEqual(bookmark3.tags.all(), [tag1, tag2]) + def test_bulk_mark_as_read(self): + bookmark1 = self.setup_bookmark(unread=True) + bookmark2 = self.setup_bookmark(unread=True) + bookmark3 = self.setup_bookmark(unread=True) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_read'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertFalse(Bookmark.objects.get(id=bookmark1.id).unread) + self.assertFalse(Bookmark.objects.get(id=bookmark2.id).unread) + self.assertFalse(Bookmark.objects.get(id=bookmark3.id).unread) + + def test_can_only_bulk_mark_as_read_own_bookmarks(self): + other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') + bookmark1 = self.setup_bookmark(unread=True, user=other_user) + bookmark2 = self.setup_bookmark(unread=True, user=other_user) + bookmark3 = self.setup_bookmark(unread=True, user=other_user) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_read'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertTrue(Bookmark.objects.get(id=bookmark1.id).unread) + self.assertTrue(Bookmark.objects.get(id=bookmark2.id).unread) + self.assertTrue(Bookmark.objects.get(id=bookmark3.id).unread) + + def test_bulk_mark_as_unread(self): + bookmark1 = self.setup_bookmark(unread=False) + bookmark2 = self.setup_bookmark(unread=False) + bookmark3 = self.setup_bookmark(unread=False) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_unread'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertTrue(Bookmark.objects.get(id=bookmark1.id).unread) + self.assertTrue(Bookmark.objects.get(id=bookmark2.id).unread) + self.assertTrue(Bookmark.objects.get(id=bookmark3.id).unread) + + def test_can_only_bulk_mark_as_unread_own_bookmarks(self): + other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') + bookmark1 = self.setup_bookmark(unread=False, user=other_user) + bookmark2 = self.setup_bookmark(unread=False, user=other_user) + bookmark3 = self.setup_bookmark(unread=False, user=other_user) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_unread'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertFalse(Bookmark.objects.get(id=bookmark1.id).unread) + self.assertFalse(Bookmark.objects.get(id=bookmark2.id).unread) + self.assertFalse(Bookmark.objects.get(id=bookmark3.id).unread) + + def test_bulk_share(self): + bookmark1 = self.setup_bookmark(shared=False) + bookmark2 = self.setup_bookmark(shared=False) + bookmark3 = self.setup_bookmark(shared=False) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_share'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertTrue(Bookmark.objects.get(id=bookmark1.id).shared) + self.assertTrue(Bookmark.objects.get(id=bookmark2.id).shared) + self.assertTrue(Bookmark.objects.get(id=bookmark3.id).shared) + + def test_can_only_bulk_share_own_bookmarks(self): + other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') + bookmark1 = self.setup_bookmark(shared=False, user=other_user) + bookmark2 = self.setup_bookmark(shared=False, user=other_user) + bookmark3 = self.setup_bookmark(shared=False, user=other_user) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_share'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertFalse(Bookmark.objects.get(id=bookmark1.id).shared) + self.assertFalse(Bookmark.objects.get(id=bookmark2.id).shared) + self.assertFalse(Bookmark.objects.get(id=bookmark3.id).shared) + + def test_bulk_unshare(self): + bookmark1 = self.setup_bookmark(shared=True) + bookmark2 = self.setup_bookmark(shared=True) + bookmark3 = self.setup_bookmark(shared=True) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_unshare'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertFalse(Bookmark.objects.get(id=bookmark1.id).shared) + self.assertFalse(Bookmark.objects.get(id=bookmark2.id).shared) + self.assertFalse(Bookmark.objects.get(id=bookmark3.id).shared) + + def test_can_only_bulk_unshare_own_bookmarks(self): + other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') + bookmark1 = self.setup_bookmark(shared=True, user=other_user) + bookmark2 = self.setup_bookmark(shared=True, user=other_user) + bookmark3 = self.setup_bookmark(shared=True, user=other_user) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_unshare'], + 'bulk_execute': [''], + 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], + }) + + self.assertTrue(Bookmark.objects.get(id=bookmark1.id).shared) + self.assertTrue(Bookmark.objects.get(id=bookmark2.id).shared) + self.assertTrue(Bookmark.objects.get(id=bookmark3.id).shared) + + def test_bulk_select_across(self): + bookmark1 = self.setup_bookmark() + bookmark2 = self.setup_bookmark() + bookmark3 = self.setup_bookmark() + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], + 'bulk_select_across': ['on'], + }) + + self.assertTrue(Bookmark.objects.get(id=bookmark1.id).is_archived) + self.assertTrue(Bookmark.objects.get(id=bookmark2.id).is_archived) + self.assertTrue(Bookmark.objects.get(id=bookmark3.id).is_archived) + + def test_bulk_select_across_respects_query(self): + self.setup_numbered_bookmarks(3, prefix='foo') + self.setup_numbered_bookmarks(3, prefix='bar') + + self.assertEqual(3, Bookmark.objects.filter(title__startswith='foo').count()) + + self.client.post(reverse('bookmarks:index.action') + '?q=foo', { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], + 'bulk_select_across': ['on'], + }) + + self.assertEqual(0, Bookmark.objects.filter(title__startswith='foo').count()) + self.assertEqual(3, Bookmark.objects.filter(title__startswith='bar').count()) + + def test_bulk_select_across_ignores_page(self): + self.setup_numbered_bookmarks(100) + + self.client.post(reverse('bookmarks:index.action') + '?page=2', { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], + 'bulk_select_across': ['on'], + }) + + self.assertEqual(0, Bookmark.objects.count()) + + def setup_bulk_edit_scope_test_data(self): + # create a number of bookmarks with different states / visibility + self.setup_numbered_bookmarks(3, with_tags=True) + self.setup_numbered_bookmarks(3, with_tags=True, archived=True) + self.setup_numbered_bookmarks(3, + shared=True, + prefix="Joe's Bookmark", + user=self.setup_user(enable_sharing=True)) + + def test_index_action_bulk_select_across_only_affects_active_bookmarks(self): + self.setup_bulk_edit_scope_test_data() + + self.assertIsNotNone(Bookmark.objects.filter(title='Bookmark 1').first()) + self.assertIsNotNone(Bookmark.objects.filter(title='Bookmark 2').first()) + self.assertIsNotNone(Bookmark.objects.filter(title='Bookmark 3').first()) + + self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], + 'bulk_select_across': ['on'], + }) + + self.assertEqual(6, Bookmark.objects.count()) + self.assertIsNone(Bookmark.objects.filter(title='Bookmark 1').first()) + self.assertIsNone(Bookmark.objects.filter(title='Bookmark 2').first()) + self.assertIsNone(Bookmark.objects.filter(title='Bookmark 3').first()) + + def test_archived_action_bulk_select_across_only_affects_archived_bookmarks(self): + self.setup_bulk_edit_scope_test_data() + + self.assertIsNotNone(Bookmark.objects.filter(title='Archived Bookmark 1').first()) + self.assertIsNotNone(Bookmark.objects.filter(title='Archived Bookmark 2').first()) + self.assertIsNotNone(Bookmark.objects.filter(title='Archived Bookmark 3').first()) + + self.client.post(reverse('bookmarks:archived.action'), { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], + 'bulk_select_across': ['on'], + }) + + self.assertEqual(6, Bookmark.objects.count()) + self.assertIsNone(Bookmark.objects.filter(title='Archived Bookmark 1').first()) + self.assertIsNone(Bookmark.objects.filter(title='Archived Bookmark 2').first()) + self.assertIsNone(Bookmark.objects.filter(title='Archived Bookmark 3').first()) + + def test_shared_action_bulk_select_across_not_supported(self): + self.setup_bulk_edit_scope_test_data() + + response = self.client.post(reverse('bookmarks:shared.action'), { + 'bulk_action': ['bulk_delete'], + 'bulk_execute': [''], + 'bulk_select_across': ['on'], + }) + self.assertEqual(response.status_code, 400) + def test_handles_empty_bookmark_id(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - response = self.client.post(reverse('bookmarks:action'), { - 'bulk_archive': [''], + response = self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], }) self.assertEqual(response.status_code, 302) - response = self.client.post(reverse('bookmarks:action'), { - 'bulk_archive': [''], + response = self.client.post(reverse('bookmarks:index.action'), { + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], 'bookmark_id': [], }) self.assertEqual(response.status_code, 302) @@ -314,7 +546,7 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - self.client.post(reverse('bookmarks:action'), { + self.client.post(reverse('bookmarks:index.action'), { 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -325,9 +557,10 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - url = reverse('bookmarks:action') + '?return_url=' + reverse('bookmarks:settings.index') + url = reverse('bookmarks:index.action') + '?return_url=' + reverse('bookmarks:settings.index') response = self.client.post(url, { - 'bulk_archive': [''], + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -339,9 +572,10 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): bookmark3 = self.setup_bookmark() def post_with(return_url, follow=None): - url = reverse('bookmarks:action') + f'?return_url={return_url}' + url = reverse('bookmarks:index.action') + f'?return_url={return_url}' return self.client.post(url, { - 'bulk_archive': [''], + 'bulk_action': ['bulk_archive'], + 'bulk_execute': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }, follow=follow) diff --git a/bookmarks/tests/test_bookmark_archived_view.py b/bookmarks/tests/test_bookmark_archived_view.py index c272f70..1ba10d7 100644 --- a/bookmarks/tests/test_bookmark_archived_view.py +++ b/bookmarks/tests/test_bookmark_archived_view.py @@ -5,7 +5,7 @@ from django.test import TestCase from django.urls import reverse from bookmarks.models import Bookmark, Tag, UserProfile -from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin +from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin, collapse_whitespace class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): @@ -68,8 +68,10 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin ] response = self.client.get(reverse('bookmarks:archived')) + html = collapse_whitespace(response.content.decode()) - self.assertContains(response, '