diff --git a/bookmarks/static/shared.js b/bookmarks/static/shared.js index b613920c..f4ab1f16 100644 --- a/bookmarks/static/shared.js +++ b/bookmarks/static/shared.js @@ -19,6 +19,7 @@ if (buttonEl.nodeName === 'BUTTON') { confirmEl.type = buttonEl.type; confirmEl.name = buttonEl.name; + confirmEl.value = buttonEl.value; } if (buttonEl.nodeName === 'A') { confirmEl.href = buttonEl.href; diff --git a/bookmarks/styles/bookmarks.scss b/bookmarks/styles/bookmarks.scss index 93d03fd5..252125ea 100644 --- a/bookmarks/styles/bookmarks.scss +++ b/bookmarks/styles/bookmarks.scss @@ -153,13 +153,13 @@ ul.bookmark-list { } } -/* Bulk edit */ +/* Bookmark actions / bulk edit */ $bulk-edit-toggle-width: 16px; $bulk-edit-toggle-offset: 8px; $bulk-edit-bar-offset: $bulk-edit-toggle-width + (2 * $bulk-edit-toggle-offset); $bulk-edit-transition-duration: 400ms; -.bulk-edit-form { +.bookmarks-page form.bookmark-actions { .bulk-edit-bar { margin-top: -17px; diff --git a/bookmarks/templates/bookmarks/archive.html b/bookmarks/templates/bookmarks/archive.html index 5fadac45..0c0c5cda 100644 --- a/bookmarks/templates/bookmarks/archive.html +++ b/bookmarks/templates/bookmarks/archive.html @@ -18,7 +18,7 @@

Archived bookmarks

{% include 'bookmarks/bulk_edit/toggle.html' %} -
{% csrf_token %} {% include 'bookmarks/bulk_edit/bar.html' with mode='archive' %} diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/bookmarks/templates/bookmarks/bookmark_list.html index e819a094..829aa78c 100644 --- a/bookmarks/templates/bookmarks/bookmark_list.html +++ b/bookmarks/templates/bookmarks/bookmark_list.html @@ -57,14 +57,14 @@ Edit {% if bookmark.is_archived %} - Unarchive + {% else %} - Archive + {% endif %} - Remove + {% endfor %} diff --git a/bookmarks/templates/bookmarks/index.html b/bookmarks/templates/bookmarks/index.html index 96877da8..80ddde4e 100644 --- a/bookmarks/templates/bookmarks/index.html +++ b/bookmarks/templates/bookmarks/index.html @@ -18,7 +18,7 @@

Bookmarks

{% include 'bookmarks/bulk_edit/toggle.html' %} - {% csrf_token %} {% include 'bookmarks/bulk_edit/bar.html' with mode='default' %} diff --git a/bookmarks/tests/test_bookmark_bulk_edit_view.py b/bookmarks/tests/test_bookmark_action_view.py similarity index 73% rename from bookmarks/tests/test_bookmark_bulk_edit_view.py rename to bookmarks/tests/test_bookmark_action_view.py index 8c6cc961..0737f8b3 100644 --- a/bookmarks/tests/test_bookmark_bulk_edit_view.py +++ b/bookmarks/tests/test_bookmark_action_view.py @@ -7,7 +7,7 @@ from bookmarks.tests.helpers import BookmarkFactoryMixin -class BookmarkBulkEditViewTestCase(TestCase, BookmarkFactoryMixin): +class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): def setUp(self) -> None: user = self.get_or_create_test_user() @@ -19,12 +19,78 @@ def assertBookmarksAreUnmodified(self, bookmarks: [Bookmark]): for bookmark in bookmarks: self.assertEqual(model_to_dict(bookmark), model_to_dict(Bookmark.objects.get(id=bookmark.id))) + def test_archive_should_archive_bookmark(self): + bookmark = self.setup_bookmark() + + self.client.post(reverse('bookmarks:action'), { + 'archive': [bookmark.id], + }) + + bookmark.refresh_from_db() + + self.assertTrue(bookmark.is_archived) + + def test_can_only_archive_own_bookmarks(self): + 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'), { + 'archive': [bookmark.id], + }) + + bookmark.refresh_from_db() + + self.assertEqual(response.status_code, 404) + self.assertFalse(bookmark.is_archived) + + def test_unarchive_should_unarchive_bookmark(self): + bookmark = self.setup_bookmark(is_archived=True) + + self.client.post(reverse('bookmarks:action'), { + 'unarchive': [bookmark.id], + }) + bookmark.refresh_from_db() + + self.assertFalse(bookmark.is_archived) + + def test_unarchive_can_only_archive_own_bookmarks(self): + 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'), { + 'unarchive': [bookmark.id], + }) + bookmark.refresh_from_db() + + self.assertEqual(response.status_code, 404) + self.assertTrue(bookmark.is_archived) + + def test_delete_should_delete_bookmark(self): + bookmark = self.setup_bookmark() + + self.client.post(reverse('bookmarks:action'), { + 'remove': [bookmark.id], + }) + + self.assertEqual(Bookmark.objects.count(), 0) + + + def test_delete_can_only_delete_own_bookmarks(self): + 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'), { + 'remove': [bookmark.id], + }) + self.assertEqual(response.status_code, 404) + self.assertTrue(Bookmark.objects.filter(id=bookmark.id).exists()) + def test_bulk_archive(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_archive': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -39,7 +105,7 @@ def test_can_only_bulk_archive_own_bookmarks(self): bookmark2 = self.setup_bookmark(user=other_user) bookmark3 = self.setup_bookmark(user=other_user) - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_archive': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -53,7 +119,7 @@ def test_bulk_unarchive(self): bookmark2 = self.setup_bookmark(is_archived=True) bookmark3 = self.setup_bookmark(is_archived=True) - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_unarchive': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -68,7 +134,7 @@ def test_can_only_bulk_unarchive_own_bookmarks(self): 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:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_unarchive': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -82,7 +148,7 @@ def test_bulk_delete(self): bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_delete': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -97,7 +163,7 @@ def test_can_only_bulk_delete_own_bookmarks(self): bookmark2 = self.setup_bookmark(user=other_user) bookmark3 = self.setup_bookmark(user=other_user) - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_delete': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) @@ -113,7 +179,7 @@ def test_bulk_tag(self): tag1 = self.setup_tag() tag2 = self.setup_tag() - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_tag': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], @@ -135,7 +201,7 @@ def test_can_only_bulk_tag_own_bookmarks(self): tag1 = self.setup_tag() tag2 = self.setup_tag() - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_tag': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], @@ -156,7 +222,7 @@ def test_bulk_untag(self): bookmark2 = self.setup_bookmark(tags=[tag1, tag2]) bookmark3 = self.setup_bookmark(tags=[tag1, tag2]) - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_untag': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], @@ -178,7 +244,7 @@ def test_can_only_bulk_untag_own_bookmarks(self): 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:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bulk_untag': [''], 'bulk_tag_string': [f'{tag1.name} {tag2.name}'], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], @@ -192,17 +258,17 @@ def test_can_only_bulk_untag_own_bookmarks(self): self.assertCountEqual(bookmark2.tags.all(), [tag1, tag2]) self.assertCountEqual(bookmark3.tags.all(), [tag1, tag2]) - def test_bulk_edit_handles_empty_bookmark_id(self): + 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:bulk_edit'), { + response = self.client.post(reverse('bookmarks:action'), { 'bulk_archive': [''], }) self.assertEqual(response.status_code, 302) - response = self.client.post(reverse('bookmarks:bulk_edit'), { + response = self.client.post(reverse('bookmarks:action'), { 'bulk_archive': [''], 'bookmark_id': [], }) @@ -215,18 +281,18 @@ def test_empty_action_does_not_modify_bookmarks(self): bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - self.client.post(reverse('bookmarks:bulk_edit'), { + self.client.post(reverse('bookmarks:action'), { 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], }) self.assertBookmarksAreUnmodified([bookmark1, bookmark2, bookmark3]) - def test_bulk_edit_should_redirect_to_return_url(self): + def test_should_redirect_to_return_url(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - url = reverse('bookmarks:bulk_edit') + '?return_url=' + reverse('bookmarks:settings.index') + url = reverse('bookmarks:action') + '?return_url=' + reverse('bookmarks:settings.index') response = self.client.post(url, { 'bulk_archive': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], @@ -234,12 +300,12 @@ def test_bulk_edit_should_redirect_to_return_url(self): self.assertRedirects(response, reverse('bookmarks:settings.index')) - def test_bulk_edit_should_not_redirect_to_external_url(self): + def test_should_not_redirect_to_external_url(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark() bookmark3 = self.setup_bookmark() - url = reverse('bookmarks:bulk_edit') + '?return_url=https://example.com' + url = reverse('bookmarks:action') + '?return_url=https://example.com' response = self.client.post(url, { 'bulk_archive': [''], 'bookmark_id': [str(bookmark1.id), str(bookmark2.id), str(bookmark3.id)], diff --git a/bookmarks/tests/test_bookmark_archive_view.py b/bookmarks/tests/test_bookmark_archive_view.py deleted file mode 100644 index b3bbd56a..00000000 --- a/bookmarks/tests/test_bookmark_archive_view.py +++ /dev/null @@ -1,55 +0,0 @@ -from django.contrib.auth.models import User -from django.test import TestCase -from django.urls import reverse - -from bookmarks.tests.helpers import BookmarkFactoryMixin - - -class BookmarkArchiveViewTestCase(TestCase, BookmarkFactoryMixin): - - def setUp(self) -> None: - user = self.get_or_create_test_user() - self.client.force_login(user) - - def test_should_archive_bookmark(self): - bookmark = self.setup_bookmark() - - self.client.get(reverse('bookmarks:archive', args=[bookmark.id])) - bookmark.refresh_from_db() - - self.assertTrue(bookmark.is_archived) - - def test_should_redirect_to_index(self): - bookmark = self.setup_bookmark() - - response = self.client.get(reverse('bookmarks:archive', args=[bookmark.id])) - - self.assertRedirects(response, reverse('bookmarks:index')) - - def test_should_redirect_to_return_url_when_specified(self): - bookmark = self.setup_bookmark() - - response = self.client.get( - reverse('bookmarks:archive', args=[bookmark.id]) + '?return_url=' + reverse('bookmarks:close') - ) - - self.assertRedirects(response, reverse('bookmarks:close')) - - def test_can_only_archive_own_bookmarks(self): - other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') - bookmark = self.setup_bookmark(user=other_user) - - response = self.client.get(reverse('bookmarks:archive', args=[bookmark.id])) - bookmark.refresh_from_db() - - self.assertEqual(response.status_code, 404) - self.assertFalse(bookmark.is_archived) - - def test_should_not_redirect_to_external_url(self): - bookmark = self.setup_bookmark() - - response = self.client.get( - reverse('bookmarks:archive', args=[bookmark.id]) + '?return_url=https://example.com' - ) - - self.assertRedirects(response, reverse('bookmarks:index')) diff --git a/bookmarks/tests/test_bookmark_remove_view.py b/bookmarks/tests/test_bookmark_remove_view.py deleted file mode 100644 index 1a284d8d..00000000 --- a/bookmarks/tests/test_bookmark_remove_view.py +++ /dev/null @@ -1,54 +0,0 @@ -from django.contrib.auth.models import User -from django.test import TestCase -from django.urls import reverse - -from bookmarks.models import Bookmark -from bookmarks.tests.helpers import BookmarkFactoryMixin - - -class BookmarkRemoveViewTestCase(TestCase, BookmarkFactoryMixin): - - def setUp(self) -> None: - user = self.get_or_create_test_user() - self.client.force_login(user) - - def test_should_delete_bookmark(self): - bookmark = self.setup_bookmark() - - self.client.get(reverse('bookmarks:remove', args=[bookmark.id])) - - self.assertEqual(Bookmark.objects.count(), 0) - - def test_should_redirect_to_index(self): - bookmark = self.setup_bookmark() - - response = self.client.get(reverse('bookmarks:remove', args=[bookmark.id])) - - self.assertRedirects(response, reverse('bookmarks:index')) - - def test_should_redirect_to_return_url_when_specified(self): - bookmark = self.setup_bookmark() - - response = self.client.get( - reverse('bookmarks:remove', args=[bookmark.id]) + '?return_url=' + reverse('bookmarks:close') - ) - - self.assertRedirects(response, reverse('bookmarks:close')) - - def test_should_not_redirect_to_external_url(self): - bookmark = self.setup_bookmark() - - response = self.client.get( - reverse('bookmarks:remove', args=[bookmark.id]) + '?return_url=https://example.com' - ) - - self.assertRedirects(response, reverse('bookmarks:index')) - - def test_can_only_edit_own_bookmarks(self): - other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') - bookmark = self.setup_bookmark(user=other_user) - - response = self.client.get(reverse('bookmarks:remove', args=[bookmark.id])) - - self.assertEqual(response.status_code, 404) - self.assertTrue(Bookmark.objects.filter(id=bookmark.id).exists()) diff --git a/bookmarks/tests/test_bookmark_unarchive_view.py b/bookmarks/tests/test_bookmark_unarchive_view.py deleted file mode 100644 index cdfcc518..00000000 --- a/bookmarks/tests/test_bookmark_unarchive_view.py +++ /dev/null @@ -1,55 +0,0 @@ -from django.contrib.auth.models import User -from django.test import TestCase -from django.urls import reverse - -from bookmarks.tests.helpers import BookmarkFactoryMixin - - -class BookmarkUnarchiveViewTestCase(TestCase, BookmarkFactoryMixin): - - def setUp(self) -> None: - user = self.get_or_create_test_user() - self.client.force_login(user) - - def test_should_unarchive_bookmark(self): - bookmark = self.setup_bookmark(is_archived=True) - - self.client.get(reverse('bookmarks:unarchive', args=[bookmark.id])) - bookmark.refresh_from_db() - - self.assertFalse(bookmark.is_archived) - - def test_should_redirect_to_archive(self): - bookmark = self.setup_bookmark() - - response = self.client.get(reverse('bookmarks:unarchive', args=[bookmark.id])) - - self.assertRedirects(response, reverse('bookmarks:archived')) - - def test_should_redirect_to_return_url_when_specified(self): - bookmark = self.setup_bookmark() - - response = self.client.get( - reverse('bookmarks:unarchive', args=[bookmark.id]) + '?return_url=' + reverse('bookmarks:close') - ) - - self.assertRedirects(response, reverse('bookmarks:close')) - - def test_should_not_redirect_to_external_url(self): - bookmark = self.setup_bookmark() - - response = self.client.get( - reverse('bookmarks:unarchive', args=[bookmark.id]) + '?return_url=https://example.com' - ) - - self.assertRedirects(response, reverse('bookmarks:archived')) - - def test_can_only_archive_own_bookmarks(self): - other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') - bookmark = self.setup_bookmark(is_archived=True, user=other_user) - - response = self.client.get(reverse('bookmarks:unarchive', args=[bookmark.id])) - bookmark.refresh_from_db() - - self.assertEqual(response.status_code, 404) - self.assertTrue(bookmark.is_archived) diff --git a/bookmarks/urls.py b/bookmarks/urls.py index 1f8330ed..b37a4dd2 100644 --- a/bookmarks/urls.py +++ b/bookmarks/urls.py @@ -15,10 +15,7 @@ path('bookmarks/new', views.bookmarks.new, name='new'), path('bookmarks/close', views.bookmarks.close, name='close'), path('bookmarks//edit', views.bookmarks.edit, name='edit'), - path('bookmarks//remove', views.bookmarks.remove, name='remove'), - path('bookmarks//archive', views.bookmarks.archive, name='archive'), - path('bookmarks//unarchive', views.bookmarks.unarchive, name='unarchive'), - path('bookmarks/bulkedit', views.bookmarks.bulk_edit, name='bulk_edit'), + path('bookmarks/action', views.bookmarks.action, name='action'), # Settings path('settings', views.settings.general, name='settings.index'), path('settings/general', views.settings.general, name='settings.general'), diff --git a/bookmarks/views/bookmarks.py b/bookmarks/views/bookmarks.py index 44d82bda..ef90dcea 100644 --- a/bookmarks/views/bookmarks.py +++ b/bookmarks/views/bookmarks.py @@ -135,7 +135,6 @@ def edit(request, bookmark_id: int): return render(request, 'bookmarks/edit.html', context) -@login_required def remove(request, bookmark_id: int): try: bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) @@ -143,11 +142,8 @@ def remove(request, bookmark_id: int): raise Http404('Bookmark does not exist') bookmark.delete() - return_url = get_safe_return_url(request.GET.get('return_url'), reverse('bookmarks:index')) - return HttpResponseRedirect(return_url) -@login_required def archive(request, bookmark_id: int): try: bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) @@ -155,11 +151,8 @@ def archive(request, bookmark_id: int): raise Http404('Bookmark does not exist') archive_bookmark(bookmark) - return_url = get_safe_return_url(request.GET.get('return_url'), reverse('bookmarks:index')) - return HttpResponseRedirect(return_url) -@login_required def unarchive(request, bookmark_id: int): try: bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) @@ -167,25 +160,32 @@ def unarchive(request, bookmark_id: int): raise Http404('Bookmark does not exist') unarchive_bookmark(bookmark) - return_url = get_safe_return_url(request.GET.get('return_url'), reverse('bookmarks:archived')) - return HttpResponseRedirect(return_url) @login_required -def bulk_edit(request): - bookmark_ids = request.POST.getlist('bookmark_id') - +def action(request): # Determine action + if 'archive' in request.POST: + archive(request, request.POST['archive']) + if 'unarchive' in request.POST: + unarchive(request, request.POST['unarchive']) + if 'remove' in request.POST: + remove(request, request.POST['remove']) if 'bulk_archive' in request.POST: + bookmark_ids = request.POST.getlist('bookmark_id') archive_bookmarks(bookmark_ids, request.user) if 'bulk_unarchive' in request.POST: + bookmark_ids = request.POST.getlist('bookmark_id') unarchive_bookmarks(bookmark_ids, request.user) if 'bulk_delete' in request.POST: + bookmark_ids = request.POST.getlist('bookmark_id') delete_bookmarks(bookmark_ids, request.user) if 'bulk_tag' in request.POST: + bookmark_ids = request.POST.getlist('bookmark_id') tag_string = convert_tag_string(request.POST['bulk_tag_string']) tag_bookmarks(bookmark_ids, tag_string, request.user) if 'bulk_untag' in request.POST: + bookmark_ids = request.POST.getlist('bookmark_id') tag_string = convert_tag_string(request.POST['bulk_tag_string']) untag_bookmarks(bookmark_ids, tag_string, request.user)