From f92c3dd4032767fd1b1e44a6e60eed7f309dc0e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Sat, 14 May 2022 09:46:51 +0200 Subject: [PATCH] Make Internet Archive integration opt-in (#250) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Make web archive integration opt-in * Add toast message about web archive integration opt-in * Improve wording for web archive setting * Add toast admin * Fix toast clear button visited styles * Add test for redirect * Improve wording * Ensure redirects to same domain * Improve wording * Fix snapshot test Co-authored-by: Sascha Ißbrücker --- bookmarks/admin.py | 11 +- bookmarks/context_processors.py | 12 ++ ...011_userprofile_web_archive_integration.py | 18 +++ bookmarks/migrations/0012_toast.py | 26 +++++ .../0013_web_archive_optin_toast.py | 30 +++++ bookmarks/models.py | 17 ++- bookmarks/services/bookmarks.py | 4 +- bookmarks/services/importer.py | 2 +- bookmarks/services/tasks.py | 37 +++--- bookmarks/signals.py | 2 +- bookmarks/styles/base.scss | 12 ++ .../templates/bookmarks/bookmark_list.html | 4 +- bookmarks/templates/bookmarks/layout.html | 36 ++++-- bookmarks/templates/settings/general.html | 21 ++++ bookmarks/tests/test_bookmarks_list_tag.py | 2 +- bookmarks/tests/test_bookmarks_service.py | 4 +- bookmarks/tests/test_bookmarks_tasks.py | 68 +++++++---- bookmarks/tests/test_importer.py | 2 +- bookmarks/tests/test_settings_general_view.py | 2 + bookmarks/tests/test_signals.py | 2 +- bookmarks/tests/test_toasts_view.py | 108 ++++++++++++++++++ bookmarks/urls.py | 2 + bookmarks/views/__init__.py | 1 + bookmarks/views/toasts.py | 19 +++ siteroot/settings/base.py | 1 + 25 files changed, 376 insertions(+), 67 deletions(-) create mode 100644 bookmarks/context_processors.py create mode 100644 bookmarks/migrations/0011_userprofile_web_archive_integration.py create mode 100644 bookmarks/migrations/0012_toast.py create mode 100644 bookmarks/migrations/0013_web_archive_optin_toast.py create mode 100644 bookmarks/tests/test_toasts_view.py create mode 100644 bookmarks/views/toasts.py diff --git a/bookmarks/admin.py b/bookmarks/admin.py index 0133cf7..42d87fa 100644 --- a/bookmarks/admin.py +++ b/bookmarks/admin.py @@ -7,7 +7,7 @@ from django.utils.translation import ngettext, gettext from rest_framework.authtoken.admin import TokenAdmin from rest_framework.authtoken.models import TokenProxy -from bookmarks.models import Bookmark, Tag, UserProfile +from bookmarks.models import Bookmark, Tag, UserProfile, Toast from bookmarks.services.bookmarks import archive_bookmark, unarchive_bookmark @@ -58,7 +58,7 @@ class AdminTag(admin.ModelAdmin): def bookmarks_count(self, obj): return obj.bookmarks_count - + bookmarks_count.admin_order_field = 'bookmarks_count' def delete_unused_tags(self, request, queryset: QuerySet): @@ -95,8 +95,15 @@ class AdminCustomUser(UserAdmin): return super(AdminCustomUser, self).get_inline_instances(request, obj) +class AdminToast(admin.ModelAdmin): + list_display = ('key', 'message', 'owner', 'acknowledged') + search_fields = ('key', 'message') + list_filter = ('owner__username',) + + linkding_admin_site = LinkdingAdminSite() linkding_admin_site.register(Bookmark, AdminBookmark) linkding_admin_site.register(Tag, AdminTag) linkding_admin_site.register(User, AdminCustomUser) linkding_admin_site.register(TokenProxy, TokenAdmin) +linkding_admin_site.register(Toast, AdminToast) diff --git a/bookmarks/context_processors.py b/bookmarks/context_processors.py new file mode 100644 index 0000000..1773b6e --- /dev/null +++ b/bookmarks/context_processors.py @@ -0,0 +1,12 @@ +from bookmarks.models import Toast + + +def toasts(request): + user = request.user if hasattr(request, 'user') else None + toast_messages = Toast.objects.filter(owner=user, acknowledged=False) if user and user.is_authenticated else [] + has_toasts = len(toast_messages) > 0 + + return { + 'has_toasts': has_toasts, + 'toast_messages': toast_messages, + } diff --git a/bookmarks/migrations/0011_userprofile_web_archive_integration.py b/bookmarks/migrations/0011_userprofile_web_archive_integration.py new file mode 100644 index 0000000..309e4b0 --- /dev/null +++ b/bookmarks/migrations/0011_userprofile_web_archive_integration.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.6 on 2022-01-08 12:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('bookmarks', '0010_userprofile_bookmark_link_target'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='web_archive_integration', + field=models.CharField(choices=[('disabled', 'Disabled'), ('enabled', 'Enabled')], default='disabled', max_length=10), + ), + ] diff --git a/bookmarks/migrations/0012_toast.py b/bookmarks/migrations/0012_toast.py new file mode 100644 index 0000000..b4136c0 --- /dev/null +++ b/bookmarks/migrations/0012_toast.py @@ -0,0 +1,26 @@ +# Generated by Django 3.2.6 on 2022-01-08 19:24 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('bookmarks', '0011_userprofile_web_archive_integration'), + ] + + operations = [ + migrations.CreateModel( + name='Toast', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('key', models.CharField(max_length=50)), + ('message', models.TextField()), + ('acknowledged', models.BooleanField(default=False)), + ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + ] diff --git a/bookmarks/migrations/0013_web_archive_optin_toast.py b/bookmarks/migrations/0013_web_archive_optin_toast.py new file mode 100644 index 0000000..c079a19 --- /dev/null +++ b/bookmarks/migrations/0013_web_archive_optin_toast.py @@ -0,0 +1,30 @@ +# Generated by Django 3.2.6 on 2022-01-08 19:27 + +from django.db import migrations +from django.contrib.auth import get_user_model + +from bookmarks.models import Toast + +User = get_user_model() + + +def forwards(apps, schema_editor): + for user in User.objects.all(): + toast = Toast(key='web_archive_opt_in_hint', + message='The Internet Archive Wayback Machine integration has been disabled by default. Check the Settings to re-enable it.', + owner=user) + toast.save() + + +def reverse(apps, schema_editor): + Toast.objects.filter(key='web_archive_opt_in_hint').delete() + + +class Migration(migrations.Migration): + dependencies = [ + ('bookmarks', '0012_toast'), + ] + + operations = [ + migrations.RunPython(forwards, reverse), + ] diff --git a/bookmarks/models.py b/bookmarks/models.py index 764c492..fe64b8b 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -128,18 +128,26 @@ class UserProfile(models.Model): (BOOKMARK_LINK_TARGET_BLANK, 'New page'), (BOOKMARK_LINK_TARGET_SELF, 'Same page'), ] + WEB_ARCHIVE_INTEGRATION_DISABLED = 'disabled' + WEB_ARCHIVE_INTEGRATION_ENABLED = 'enabled' + WEB_ARCHIVE_INTEGRATION_CHOICES = [ + (WEB_ARCHIVE_INTEGRATION_DISABLED, 'Disabled'), + (WEB_ARCHIVE_INTEGRATION_ENABLED, 'Enabled'), + ] user = models.OneToOneField(get_user_model(), related_name='profile', on_delete=models.CASCADE) theme = models.CharField(max_length=10, choices=THEME_CHOICES, blank=False, default=THEME_AUTO) bookmark_date_display = models.CharField(max_length=10, choices=BOOKMARK_DATE_DISPLAY_CHOICES, blank=False, default=BOOKMARK_DATE_DISPLAY_RELATIVE) bookmark_link_target = models.CharField(max_length=10, choices=BOOKMARK_LINK_TARGET_CHOICES, blank=False, default=BOOKMARK_LINK_TARGET_BLANK) + web_archive_integration = models.CharField(max_length=10, choices=WEB_ARCHIVE_INTEGRATION_CHOICES, blank=False, + default=WEB_ARCHIVE_INTEGRATION_DISABLED) class UserProfileForm(forms.ModelForm): class Meta: model = UserProfile - fields = ['theme', 'bookmark_date_display', 'bookmark_link_target'] + fields = ['theme', 'bookmark_date_display', 'bookmark_link_target', 'web_archive_integration'] @receiver(post_save, sender=get_user_model()) @@ -151,3 +159,10 @@ def create_user_profile(sender, instance, created, **kwargs): @receiver(post_save, sender=get_user_model()) def save_user_profile(sender, instance, **kwargs): instance.profile.save() + + +class Toast(models.Model): + key = models.CharField(max_length=50) + message = models.TextField() + acknowledged = models.BooleanField(default=False) + owner = models.ForeignKey(get_user_model(), on_delete=models.CASCADE) diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 3408fd5..ade5eb8 100644 --- a/bookmarks/services/bookmarks.py +++ b/bookmarks/services/bookmarks.py @@ -29,7 +29,7 @@ def create_bookmark(bookmark: Bookmark, tag_string: str, current_user: User): _update_bookmark_tags(bookmark, tag_string, current_user) bookmark.save() # Create snapshot on web archive - tasks.create_web_archive_snapshot(bookmark.id, False) + tasks.create_web_archive_snapshot(current_user, bookmark, False) return bookmark @@ -47,7 +47,7 @@ def update_bookmark(bookmark: Bookmark, tag_string, current_user: User): bookmark.save() # Update web archive snapshot, if URL changed if has_url_changed: - tasks.create_web_archive_snapshot(bookmark.id, True) + tasks.create_web_archive_snapshot(current_user, bookmark, True) return bookmark diff --git a/bookmarks/services/importer.py b/bookmarks/services/importer.py index 7fb0bd9..b72f32c 100644 --- a/bookmarks/services/importer.py +++ b/bookmarks/services/importer.py @@ -40,7 +40,7 @@ def import_netscape_html(html: str, user: User): result.failed = result.failed + 1 # Create snapshots for newly imported bookmarks - tasks.schedule_bookmarks_without_snapshots(user.id) + tasks.schedule_bookmarks_without_snapshots(user) return result diff --git a/bookmarks/services/tasks.py b/bookmarks/services/tasks.py index eb7347d..5c8300d 100644 --- a/bookmarks/services/tasks.py +++ b/bookmarks/services/tasks.py @@ -4,30 +4,29 @@ import waybackpy from background_task import background from django.conf import settings from django.contrib.auth import get_user_model +from django.contrib.auth.models import User from waybackpy.exceptions import WaybackError -from bookmarks.models import Bookmark +from bookmarks.models import Bookmark, UserProfile logger = logging.getLogger(__name__) -def when_background_tasks_enabled(fn): - def wrapper(*args, **kwargs): - if settings.LD_DISABLE_BACKGROUND_TASKS: - return - return fn(*args, **kwargs) +def is_web_archive_integration_active(user: User) -> bool: + background_tasks_enabled = not settings.LD_DISABLE_BACKGROUND_TASKS + web_archive_integration_enabled = \ + user.profile.web_archive_integration == UserProfile.WEB_ARCHIVE_INTEGRATION_ENABLED - # Expose attributes from wrapped TaskProxy function - attrs = vars(fn) - for key, value in attrs.items(): - setattr(wrapper, key, value) - - return wrapper + return background_tasks_enabled and web_archive_integration_enabled + + +def create_web_archive_snapshot(user: User, bookmark: Bookmark, force_update: bool): + if is_web_archive_integration_active(user): + _create_web_archive_snapshot_task(bookmark.id, force_update) -@when_background_tasks_enabled @background() -def create_web_archive_snapshot(bookmark_id: int, force_update: bool): +def _create_web_archive_snapshot_task(bookmark_id: int, force_update: bool): try: bookmark = Bookmark.objects.get(id=bookmark_id) except Bookmark.DoesNotExist: @@ -52,11 +51,15 @@ def create_web_archive_snapshot(bookmark_id: int, force_update: bool): logger.debug(f'Successfully created web archive link for bookmark: {bookmark}...') -@when_background_tasks_enabled +def schedule_bookmarks_without_snapshots(user: User): + if is_web_archive_integration_active(user): + _schedule_bookmarks_without_snapshots_task(user.id) + + @background() -def schedule_bookmarks_without_snapshots(user_id: int): +def _schedule_bookmarks_without_snapshots_task(user_id: int): user = get_user_model().objects.get(id=user_id) bookmarks_without_snapshots = Bookmark.objects.filter(web_archive_snapshot_url__exact='', owner=user) for bookmark in bookmarks_without_snapshots: - create_web_archive_snapshot(bookmark.id, False) + _create_web_archive_snapshot_task(bookmark.id, False) diff --git a/bookmarks/signals.py b/bookmarks/signals.py index 602ef4f..dafb1b9 100644 --- a/bookmarks/signals.py +++ b/bookmarks/signals.py @@ -5,4 +5,4 @@ from bookmarks.services import tasks @receiver(user_logged_in) def user_logged_in(sender, request, user, **kwargs): - tasks.schedule_bookmarks_without_snapshots(user.id) + tasks.schedule_bookmarks_without_snapshots(user) diff --git a/bookmarks/styles/base.scss b/bookmarks/styles/base.scss index f7f1a96..5086c50 100644 --- a/bookmarks/styles/base.scss +++ b/bookmarks/styles/base.scss @@ -11,6 +11,18 @@ header { margin-bottom: 40px; } +header .toasts { + margin-bottom: 20px; + + .toast { + margin-bottom: 0.4rem; + } + + .toast a.btn-clear:visited { + color: currentColor; + } +} + .navbar { .navbar-brand { diff --git a/bookmarks/templates/bookmarks/bookmark_list.html b/bookmarks/templates/bookmarks/bookmark_list.html index 829aa78..7777e38 100644 --- a/bookmarks/templates/bookmarks/bookmark_list.html +++ b/bookmarks/templates/bookmarks/bookmark_list.html @@ -30,7 +30,7 @@ {% if bookmark.web_archive_snapshot_url %} + title="Show snapshot on the Internet Archive Wayback Machine" target="{{ link_target }}" rel="noopener"> {% endif %} {{ bookmark.date_added|humanize_relative_date }} {% if bookmark.web_archive_snapshot_url %} @@ -44,7 +44,7 @@ {% if bookmark.web_archive_snapshot_url %} + title="Show snapshot on the Internet Archive Wayback Machine" target="{{ link_target }}" rel="noopener"> {% endif %} {{ bookmark.date_added|humanize_absolute_date }} {% if bookmark.web_archive_snapshot_url %} diff --git a/bookmarks/templates/bookmarks/layout.html b/bookmarks/templates/bookmarks/layout.html index c0e5950..e6ce7a7 100644 --- a/bookmarks/templates/bookmarks/layout.html +++ b/bookmarks/templates/bookmarks/layout.html @@ -27,19 +27,31 @@ {% endif %} -
-
- - -

linkding

- -
- {# Only show nav items menu when logged in #} - {% if request.user.is_authenticated %} -
- {% include 'bookmarks/nav_menu.html' %} -
+
+ {% if has_toasts %} +
+ {% for toast in toast_messages %} +
+ {{ toast.message }} + +
+ {% endfor %} +
{% endif %} +
+
+ + +

linkding

+ +
+ {# Only show nav items menu when logged in #} + {% if request.user.is_authenticated %} +
+ {% include 'bookmarks/nav_menu.html' %} +
+ {% endif %} +
{% block content %} diff --git a/bookmarks/templates/settings/general.html b/bookmarks/templates/settings/general.html index 32cde04..4cec26b 100644 --- a/bookmarks/templates/settings/general.html +++ b/bookmarks/templates/settings/general.html @@ -17,14 +17,35 @@
{{ form.theme|add_class:"form-select col-2 col-sm-12" }} +
+ Whether to use a light or dark theme, or automatically adjust the theme based on your system's settings. +
{{ form.bookmark_date_display|add_class:"form-select col-2 col-sm-12" }} +
+ Whether to show bookmark dates as relative (how long ago), or as absolute dates. Alternatively the date can be hidden. +
{{ form.bookmark_link_target|add_class:"form-select col-2 col-sm-12" }} +
+ Whether to open bookmarks a new page or in the same page. +
+
+
+ + {{ form.web_archive_integration|add_class:"form-select col-2 col-sm-12" }} +
+ Enabling this feature will automatically create snapshots of bookmarked websites on the Internet Archive Wayback + Machine. This allows + to preserve, and later access, the website as it was at the point in time it was bookmarked, in + case it goes offline or its content is modified. +
diff --git a/bookmarks/tests/test_bookmarks_list_tag.py b/bookmarks/tests/test_bookmarks_list_tag.py index dff1b11..3543748 100644 --- a/bookmarks/tests/test_bookmarks_list_tag.py +++ b/bookmarks/tests/test_bookmarks_list_tag.py @@ -28,7 +28,7 @@ class BookmarkListTagTest(TestCase, BookmarkFactoryMixin): self.assertInHTML(f''' + title="Show snapshot on the Internet Archive Wayback Machine" target="{link_target}" rel="noopener"> {label_content} ∞ diff --git a/bookmarks/tests/test_bookmarks_service.py b/bookmarks/tests/test_bookmarks_service.py index d438491..8a3a5b4 100644 --- a/bookmarks/tests/test_bookmarks_service.py +++ b/bookmarks/tests/test_bookmarks_service.py @@ -23,7 +23,7 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin): bookmark_data = Bookmark(url='https://example.com') bookmark = create_bookmark(bookmark_data, 'tag1,tag2', self.user) - mock_create_web_archive_snapshot.assert_called_once_with(bookmark.id, False) + mock_create_web_archive_snapshot.assert_called_once_with(self.user, bookmark, False) def test_update_should_create_web_archive_snapshot_if_url_did_change(self): with patch.object(tasks, 'create_web_archive_snapshot') as mock_create_web_archive_snapshot: @@ -31,7 +31,7 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin): bookmark.url = 'https://example.com/updated' update_bookmark(bookmark, 'tag1,tag2', self.user) - mock_create_web_archive_snapshot.assert_called_once_with(bookmark.id, True) + mock_create_web_archive_snapshot.assert_called_once_with(self.user, bookmark, True) def test_update_should_not_create_web_archive_snapshot_if_url_did_not_change(self): with patch.object(tasks, 'create_web_archive_snapshot') as mock_create_web_archive_snapshot: diff --git a/bookmarks/tests/test_bookmarks_tasks.py b/bookmarks/tests/test_bookmarks_tasks.py index af585a4..01c0f51 100644 --- a/bookmarks/tests/test_bookmarks_tasks.py +++ b/bookmarks/tests/test_bookmarks_tasks.py @@ -5,8 +5,8 @@ from background_task.models import Task from django.contrib.auth.models import User from django.test import TestCase, override_settings -from bookmarks.models import Bookmark -from bookmarks.services.tasks import create_web_archive_snapshot, schedule_bookmarks_without_snapshots +from bookmarks.models import Bookmark, UserProfile +from bookmarks.services import tasks from bookmarks.tests.helpers import BookmarkFactoryMixin, disable_logging @@ -26,6 +26,11 @@ class MockWaybackUrlWithSaveError: class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): + def setUp(self): + user = self.get_or_create_test_user() + user.profile.web_archive_integration = UserProfile.WEB_ARCHIVE_INTEGRATION_ENABLED + user.profile.save() + @disable_logging def run_pending_task(self, task_function): func = getattr(task_function, 'task_function', None) @@ -48,16 +53,16 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): bookmark = self.setup_bookmark() with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')): - create_web_archive_snapshot(bookmark.id, False) - self.run_pending_task(create_web_archive_snapshot) + tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False) + self.run_pending_task(tasks._create_web_archive_snapshot_task) bookmark.refresh_from_db() self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com') def test_create_web_archive_snapshot_should_handle_missing_bookmark_id(self): with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')) as mock_wayback_url: - create_web_archive_snapshot(123, False) - self.run_pending_task(create_web_archive_snapshot) + tasks._create_web_archive_snapshot_task(123, False) + self.run_pending_task(tasks._create_web_archive_snapshot_task) mock_wayback_url.assert_not_called() @@ -67,15 +72,15 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): with patch.object(waybackpy, 'Url', return_value=MockWaybackUrlWithSaveError()): with self.assertRaises(NotImplementedError): - create_web_archive_snapshot(bookmark.id, False) - self.run_pending_task(create_web_archive_snapshot) + tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False) + self.run_pending_task(tasks._create_web_archive_snapshot_task) def test_create_web_archive_snapshot_should_skip_if_snapshot_exists(self): bookmark = self.setup_bookmark(web_archive_snapshot_url='https://example.com') with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://other.com')): - create_web_archive_snapshot(bookmark.id, False) - self.run_pending_task(create_web_archive_snapshot) + tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False) + self.run_pending_task(tasks._create_web_archive_snapshot_task) bookmark.refresh_from_db() self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com') @@ -84,8 +89,8 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): bookmark = self.setup_bookmark(web_archive_snapshot_url='https://example.com') with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://other.com')): - create_web_archive_snapshot(bookmark.id, True) - self.run_pending_task(create_web_archive_snapshot) + tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, True) + self.run_pending_task(tasks._create_web_archive_snapshot_task) bookmark.refresh_from_db() self.assertEqual(bookmark.web_archive_snapshot_url, 'https://other.com') @@ -93,7 +98,16 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): @override_settings(LD_DISABLE_BACKGROUND_TASKS=True) def test_create_web_archive_snapshot_should_not_run_when_background_tasks_are_disabled(self): bookmark = self.setup_bookmark() - create_web_archive_snapshot(bookmark.id, False) + tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False) + + self.assertEqual(Task.objects.count(), 0) + + def test_create_web_archive_snapshot_should_not_run_when_web_archive_integration_is_disabled(self): + self.user.profile.web_archive_integration = UserProfile.WEB_ARCHIVE_INTEGRATION_DISABLED + self.user.profile.save() + + bookmark = self.setup_bookmark() + tasks.create_web_archive_snapshot(self.get_or_create_test_user(), bookmark, False) self.assertEqual(Task.objects.count(), 0) @@ -104,9 +118,9 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): self.setup_bookmark() with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')): - schedule_bookmarks_without_snapshots(user.id) - self.run_pending_task(schedule_bookmarks_without_snapshots) - self.run_all_pending_tasks(create_web_archive_snapshot) + tasks.schedule_bookmarks_without_snapshots(user) + self.run_pending_task(tasks._schedule_bookmarks_without_snapshots_task) + self.run_all_pending_tasks(tasks._create_web_archive_snapshot_task) for bookmark in Bookmark.objects.all(): self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com') @@ -118,9 +132,9 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): self.setup_bookmark(web_archive_snapshot_url='https://example.com') with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://other.com')): - schedule_bookmarks_without_snapshots(user.id) - self.run_pending_task(schedule_bookmarks_without_snapshots) - self.run_all_pending_tasks(create_web_archive_snapshot) + tasks.schedule_bookmarks_without_snapshots(user) + self.run_pending_task(tasks._schedule_bookmarks_without_snapshots_task) + self.run_all_pending_tasks(tasks._create_web_archive_snapshot_task) for bookmark in Bookmark.objects.all(): self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com') @@ -136,9 +150,9 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): self.setup_bookmark(user=other_user) with patch.object(waybackpy, 'Url', return_value=MockWaybackUrl('https://example.com')): - schedule_bookmarks_without_snapshots(user.id) - self.run_pending_task(schedule_bookmarks_without_snapshots) - self.run_all_pending_tasks(create_web_archive_snapshot) + tasks.schedule_bookmarks_without_snapshots(user) + self.run_pending_task(tasks._schedule_bookmarks_without_snapshots_task) + self.run_all_pending_tasks(tasks._create_web_archive_snapshot_task) for bookmark in Bookmark.objects.all().filter(owner=user): self.assertEqual(bookmark.web_archive_snapshot_url, 'https://example.com') @@ -148,7 +162,13 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): @override_settings(LD_DISABLE_BACKGROUND_TASKS=True) def test_schedule_bookmarks_without_snapshots_should_not_run_when_background_tasks_are_disabled(self): - user = self.get_or_create_test_user() - schedule_bookmarks_without_snapshots(user.id) + tasks.schedule_bookmarks_without_snapshots(self.user) + + self.assertEqual(Task.objects.count(), 0) + + def test_schedule_bookmarks_without_snapshots_should_not_run_when_web_archive_integration_is_disabled(self): + self.user.profile.web_archive_integration = UserProfile.WEB_ARCHIVE_INTEGRATION_DISABLED + self.user.profile.save() + tasks.schedule_bookmarks_without_snapshots(self.user) self.assertEqual(Task.objects.count(), 0) diff --git a/bookmarks/tests/test_importer.py b/bookmarks/tests/test_importer.py index bc6934c..c5113e7 100644 --- a/bookmarks/tests/test_importer.py +++ b/bookmarks/tests/test_importer.py @@ -55,4 +55,4 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin): with patch.object(tasks, 'schedule_bookmarks_without_snapshots') as mock_schedule_bookmarks_without_snapshots: import_netscape_html(test_html, user) - mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user.id) + mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user) diff --git a/bookmarks/tests/test_settings_general_view.py b/bookmarks/tests/test_settings_general_view.py index e6d7299..2278f82 100644 --- a/bookmarks/tests/test_settings_general_view.py +++ b/bookmarks/tests/test_settings_general_view.py @@ -27,6 +27,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): 'theme': UserProfile.THEME_DARK, 'bookmark_date_display': UserProfile.BOOKMARK_DATE_DISPLAY_HIDDEN, 'bookmark_link_target': UserProfile.BOOKMARK_LINK_TARGET_SELF, + 'web_archive_integration': UserProfile.WEB_ARCHIVE_INTEGRATION_ENABLED, } response = self.client.post(reverse('bookmarks:settings.general'), form_data) @@ -36,3 +37,4 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): self.assertEqual(self.user.profile.theme, form_data['theme']) self.assertEqual(self.user.profile.bookmark_date_display, form_data['bookmark_date_display']) self.assertEqual(self.user.profile.bookmark_link_target, form_data['bookmark_link_target']) + self.assertEqual(self.user.profile.web_archive_integration, form_data['web_archive_integration']) diff --git a/bookmarks/tests/test_signals.py b/bookmarks/tests/test_signals.py index fe2c5ac..c855c07 100644 --- a/bookmarks/tests/test_signals.py +++ b/bookmarks/tests/test_signals.py @@ -12,4 +12,4 @@ class SignalsTestCase(TestCase, BookmarkFactoryMixin): with patch.object(tasks, 'schedule_bookmarks_without_snapshots') as mock_schedule_bookmarks_without_snapshots: self.client.force_login(user) - mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user.id) + mock_schedule_bookmarks_without_snapshots.assert_called_once_with(user) diff --git a/bookmarks/tests/test_toasts_view.py b/bookmarks/tests/test_toasts_view.py new file mode 100644 index 0000000..bad3530 --- /dev/null +++ b/bookmarks/tests/test_toasts_view.py @@ -0,0 +1,108 @@ +from django.contrib.auth.models import User +from django.test import TestCase +from django.urls import reverse + +from bookmarks.models import Toast +from bookmarks.tests.helpers import BookmarkFactoryMixin, random_sentence, disable_logging + + +class ToastsViewTestCase(TestCase, BookmarkFactoryMixin): + + def setUp(self) -> None: + user = self.get_or_create_test_user() + self.client.force_login(user) + + def create_toast(self, user: User = None, message: str = None, acknowledged: bool = False): + if not user: + user = self.user + if not message: + message = random_sentence() + + toast = Toast(owner=user, key='test', message=message, acknowledged=acknowledged) + toast.save() + return toast + + def test_should_render_unacknowledged_toasts(self): + self.create_toast() + self.create_toast() + self.create_toast(acknowledged=True) + + response = self.client.get(reverse('bookmarks:index')) + + # Should render toasts container + self.assertContains(response, '
') + # Should render two toasts + self.assertContains(response, '
', count=2) + + def test_should_not_render_acknowledged_toasts(self): + self.create_toast(acknowledged=True) + self.create_toast(acknowledged=True) + self.create_toast(acknowledged=True) + + response = self.client.get(reverse('bookmarks:index')) + + # Should not render toasts container + self.assertContains(response, '
', count=0) + # Should not render toasts + self.assertContains(response, '
', count=0) + + def test_should_not_render_toasts_of_other_users(self): + other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') + + self.create_toast(user=other_user) + self.create_toast(user=other_user) + self.create_toast(user=other_user) + + response = self.client.get(reverse('bookmarks:index')) + + # Should not render toasts container + self.assertContains(response, '
', count=0) + # Should not render toasts + self.assertContains(response, '
', count=0) + + def test_toast_content(self): + toast = self.create_toast() + expected_toast = f''' +
+ {toast.message} + +
+ ''' + + response = self.client.get(reverse('bookmarks:index')) + html = response.content.decode() + + self.assertInHTML(expected_toast, html) + + def test_acknowledge_toast(self): + toast = self.create_toast() + + self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id])) + + toast.refresh_from_db() + self.assertTrue(toast.acknowledged) + + def test_acknowledge_toast_should_redirect_to_return_url(self): + toast = self.create_toast() + return_url = reverse('bookmarks:settings.general') + acknowledge_url = reverse('bookmarks:toasts.acknowledge', args=[toast.id]) + acknowledge_url = acknowledge_url + '?return_url=' + return_url + + response = self.client.get(acknowledge_url) + + self.assertRedirects(response, return_url) + + def test_acknowledge_toast_should_redirect_to_index_by_default(self): + toast = self.create_toast() + + response = self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id])) + + self.assertRedirects(response, reverse('bookmarks:index')) + + @disable_logging + def test_acknowledge_toast_should_not_acknowledge_other_users_toast(self): + other_user = User.objects.create_user('otheruser', 'otheruser@example.com', 'password123') + toast = self.create_toast(user=other_user) + + response = self.client.get(reverse('bookmarks:toasts.acknowledge', args=[toast.id])) + self.assertEqual(response.status_code, 404) diff --git a/bookmarks/urls.py b/bookmarks/urls.py index b37a4dd..241473b 100644 --- a/bookmarks/urls.py +++ b/bookmarks/urls.py @@ -22,6 +22,8 @@ urlpatterns = [ path('settings/integrations', views.settings.integrations, name='settings.integrations'), path('settings/import', views.settings.bookmark_import, name='settings.import'), path('settings/export', views.settings.bookmark_export, name='settings.export'), + # Toasts + path('toasts//acknowledge', views.toasts.acknowledge, name='toasts.acknowledge'), # API path('api/', include(router.urls), name='api') ] diff --git a/bookmarks/views/__init__.py b/bookmarks/views/__init__.py index 5a0f826..84f3f08 100644 --- a/bookmarks/views/__init__.py +++ b/bookmarks/views/__init__.py @@ -1,2 +1,3 @@ from .bookmarks import * from .settings import * +from .toasts import * diff --git a/bookmarks/views/toasts.py b/bookmarks/views/toasts.py new file mode 100644 index 0000000..2af5bd3 --- /dev/null +++ b/bookmarks/views/toasts.py @@ -0,0 +1,19 @@ +from django.contrib.auth.decorators import login_required +from django.http import HttpResponseRedirect, Http404 +from django.urls import reverse + +from bookmarks.models import Toast +from bookmarks.utils import get_safe_return_url + + +@login_required +def acknowledge(request, toast_id: int): + try: + toast = Toast.objects.get(pk=toast_id, owner=request.user) + except Toast.DoesNotExist: + raise Http404('Toast does not exist') + toast.acknowledged = True + toast.save() + + return_url = get_safe_return_url(request.GET.get('return_url'), reverse('bookmarks:index')) + return HttpResponseRedirect(return_url) diff --git a/siteroot/settings/base.py b/siteroot/settings/base.py index ca522e8..a7da61a 100644 --- a/siteroot/settings/base.py +++ b/siteroot/settings/base.py @@ -69,6 +69,7 @@ TEMPLATES = [ 'django.template.context_processors.request', 'django.contrib.auth.context_processors.auth', 'django.contrib.messages.context_processors.messages', + 'bookmarks.context_processors.toasts', ], }, },