diff --git a/bookmarks/services/tasks.py b/bookmarks/services/tasks.py index 6d24ba2..e97f55c 100644 --- a/bookmarks/services/tasks.py +++ b/bookmarks/services/tasks.py @@ -1,6 +1,7 @@ import functools import logging import os +from typing import List import waybackpy from django.conf import settings @@ -228,6 +229,26 @@ def create_html_snapshot(bookmark: Bookmark): if not is_html_snapshot_feature_active(): return + asset = _create_snapshot_asset(bookmark) + asset.save() + + +def create_html_snapshots(bookmark_list: List[Bookmark]): + if not is_html_snapshot_feature_active(): + return + + assets_to_create = [] + for bookmark in bookmark_list: + asset = _create_snapshot_asset(bookmark) + assets_to_create.append(asset) + + BookmarkAsset.objects.bulk_create(assets_to_create) + + +MAX_SNAPSHOT_FILENAME_LENGTH = 192 + + +def _create_snapshot_asset(bookmark: Bookmark) -> BookmarkAsset: timestamp = formats.date_format(timezone.now(), "SHORT_DATE_FORMAT") asset = BookmarkAsset( bookmark=bookmark, @@ -236,10 +257,7 @@ def create_html_snapshot(bookmark: Bookmark): display_name=f"HTML snapshot from {timestamp}", status=BookmarkAsset.STATUS_PENDING, ) - asset.save() - - -MAX_SNAPSHOT_FILENAME_LENGTH = 192 + return asset def _generate_snapshot_filename(asset: BookmarkAsset) -> str: @@ -305,3 +323,23 @@ def _create_html_snapshot_task(asset_id: int): ) asset.status = BookmarkAsset.STATUS_FAILURE asset.save() + + +def create_missing_html_snapshots(user: User) -> int: + if not is_html_snapshot_feature_active(): + return 0 + + bookmarks_without_snapshots = Bookmark.objects.filter(owner=user).exclude( + bookmarkasset__asset_type=BookmarkAsset.TYPE_SNAPSHOT, + bookmarkasset__status__in=[ + BookmarkAsset.STATUS_PENDING, + BookmarkAsset.STATUS_COMPLETE, + ], + ) + bookmarks_without_snapshots |= Bookmark.objects.filter(owner=user).exclude( + bookmarkasset__asset_type=BookmarkAsset.TYPE_SNAPSHOT + ) + + create_html_snapshots(list(bookmarks_without_snapshots)) + + return bookmarks_without_snapshots.count() diff --git a/bookmarks/templates/settings/general.html b/bookmarks/templates/settings/general.html index f495869..acec66d 100644 --- a/bookmarks/templates/settings/general.html +++ b/bookmarks/templates/settings/general.html @@ -8,6 +8,12 @@ {# Profile section #}
+ {% if success_message %} +
{{ success_message }}
+ {% endif %} + {% if error_message %} +
{{ error_message }}
+ {% endif %}

Profile

Change password @@ -120,13 +126,6 @@ {% if request.user_profile.enable_favicons and enable_refresh_favicons %} {% endif %} - {% if refresh_favicons_success_message %} -

-

- {{ refresh_favicons_success_message }} -

-
- {% endif %}
+ {% endif %}
@@ -189,13 +189,6 @@
- {% if update_profile_success_message %} -
-

- {{ update_profile_success_message }} -

-
- {% endif %}
@@ -224,20 +217,6 @@ - {% if import_success_message %} -
-

- {{ import_success_message }} -

-
- {% endif %} - {% if import_errors_message %} -
-

- {{ import_errors_message }} -

-
- {% endif %} diff --git a/bookmarks/tests/test_bookmarks_tasks.py b/bookmarks/tests/test_bookmarks_tasks.py index 5040caa..3c087d7 100644 --- a/bookmarks/tests/test_bookmarks_tasks.py +++ b/bookmarks/tests/test_bookmarks_tasks.py @@ -611,3 +611,89 @@ class BookmarkTasksTestCase(TestCase, BookmarkFactoryMixin): tasks.create_html_snapshot(bookmark) self.assertEqual(BookmarkAsset.objects.count(), 0) + + @override_settings(LD_ENABLE_SNAPSHOTS=True) + def test_create_missing_html_snapshots(self): + bookmarks_with_snapshots = [] + bookmarks_without_snapshots = [] + + # setup bookmarks with snapshots + bookmark = self.setup_bookmark() + self.setup_asset( + bookmark=bookmark, + asset_type=BookmarkAsset.TYPE_SNAPSHOT, + status=BookmarkAsset.STATUS_COMPLETE, + ) + bookmarks_with_snapshots.append(bookmark) + + bookmark = self.setup_bookmark() + self.setup_asset( + bookmark=bookmark, + asset_type=BookmarkAsset.TYPE_SNAPSHOT, + status=BookmarkAsset.STATUS_PENDING, + ) + bookmarks_with_snapshots.append(bookmark) + + # setup bookmarks without snapshots + bookmark = self.setup_bookmark() + bookmarks_without_snapshots.append(bookmark) + + bookmark = self.setup_bookmark() + self.setup_asset( + bookmark=bookmark, + asset_type=BookmarkAsset.TYPE_SNAPSHOT, + status=BookmarkAsset.STATUS_FAILURE, + ) + bookmarks_without_snapshots.append(bookmark) + + bookmark = self.setup_bookmark() + self.setup_asset( + bookmark=bookmark, + asset_type="some_other_type", + status=BookmarkAsset.STATUS_PENDING, + ) + bookmarks_without_snapshots.append(bookmark) + + bookmark = self.setup_bookmark() + self.setup_asset( + bookmark=bookmark, + asset_type="some_other_type", + status=BookmarkAsset.STATUS_COMPLETE, + ) + bookmarks_without_snapshots.append(bookmark) + + initial_assets = list(BookmarkAsset.objects.all()) + initial_assets_count = len(initial_assets) + initial_asset_ids = [asset.id for asset in initial_assets] + count = tasks.create_missing_html_snapshots(self.get_or_create_test_user()) + + self.assertEqual(count, 4) + self.assertEqual(BookmarkAsset.objects.count(), initial_assets_count + count) + + for bookmark in bookmarks_without_snapshots: + new_assets = BookmarkAsset.objects.filter(bookmark=bookmark).exclude( + id__in=initial_asset_ids + ) + self.assertEqual(new_assets.count(), 1) + + for bookmark in bookmarks_with_snapshots: + new_assets = BookmarkAsset.objects.filter(bookmark=bookmark).exclude( + id__in=initial_asset_ids + ) + self.assertEqual(new_assets.count(), 0) + + @override_settings(LD_ENABLE_SNAPSHOTS=True) + def test_create_missing_html_snapshots_respects_current_user(self): + self.setup_bookmark() + self.setup_bookmark() + self.setup_bookmark() + + other_user = self.setup_user() + self.setup_bookmark(user=other_user) + self.setup_bookmark(user=other_user) + self.setup_bookmark(user=other_user) + + count = tasks.create_missing_html_snapshots(self.get_or_create_test_user()) + + self.assertEqual(count, 3) + self.assertEqual(BookmarkAsset.objects.count(), count) diff --git a/bookmarks/tests/test_settings_general_view.py b/bookmarks/tests/test_settings_general_view.py index f2768ad..3f6c4c7 100644 --- a/bookmarks/tests/test_settings_general_view.py +++ b/bookmarks/tests/test_settings_general_view.py @@ -44,6 +44,24 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): return {**form_data, **overrides} + def assertSuccessMessage(self, html, message: str, count=1): + self.assertInHTML( + f""" +
{ message }
+ """, + html, + count=count, + ) + + def assertErrorMessage(self, html, message: str, count=1): + self.assertInHTML( + f""" +
{ message }
+ """, + html, + count=count, + ) + def test_should_render_successfully(self): response = self.client.get(reverse("bookmarks:settings.general")) @@ -138,12 +156,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): self.user.profile.permanent_notes, form_data["permanent_notes"] ) self.assertEqual(self.user.profile.custom_css, form_data["custom_css"]) - self.assertInHTML( - """ -

Profile updated

- """, - html, - ) + self.assertSuccessMessage(html, "Profile updated") def test_update_profile_should_not_be_called_without_respective_form_action(self): form_data = { @@ -156,13 +169,7 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): self.assertEqual(response.status_code, 200) self.assertEqual(self.user.profile.theme, UserProfile.THEME_AUTO) - self.assertInHTML( - """ -

Profile updated

- """, - html, - count=0, - ) + self.assertSuccessMessage(html, "Profile updated", count=0) def test_enable_favicons_should_schedule_icon_update(self): with patch.object( @@ -210,13 +217,8 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): html = response.content.decode() mock_schedule_refresh_favicons.assert_called_once() - self.assertInHTML( - """ -

- Scheduled favicon update. This may take a while... -

- """, - html, + self.assertSuccessMessage( + html, "Scheduled favicon update. This may take a while..." ) def test_refresh_favicons_should_not_be_called_without_respective_form_action(self): @@ -230,14 +232,8 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): html = response.content.decode() mock_schedule_refresh_favicons.assert_not_called() - self.assertInHTML( - """ -

- Scheduled favicon update. This may take a while... -

- """, - html, - count=0, + self.assertSuccessMessage( + html, "Scheduled favicon update. This may take a while...", count=0 ) def test_refresh_favicons_should_be_visible_when_favicons_enabled_in_profile(self): @@ -365,3 +361,57 @@ class SettingsGeneralViewTestCase(TestCase, BookmarkFactoryMixin): with patch.object(requests, "get", return_value=latest_version_response_mock): version_info = get_version_info(random.random()) self.assertEqual(version_info, app_version) + + @override_settings(LD_ENABLE_SNAPSHOTS=True) + def test_create_missing_html_snapshots(self): + with patch.object( + tasks, "create_missing_html_snapshots" + ) as mock_create_missing_html_snapshots: + mock_create_missing_html_snapshots.return_value = 5 + form_data = { + "create_missing_html_snapshots": "", + } + response = self.client.post( + reverse("bookmarks:settings.general"), form_data + ) + html = response.content.decode() + + mock_create_missing_html_snapshots.assert_called_once() + self.assertSuccessMessage( + html, "Queued 5 missing snapshots. This may take a while..." + ) + + @override_settings(LD_ENABLE_SNAPSHOTS=True) + def test_create_missing_html_snapshots_no_missing_snapshots(self): + with patch.object( + tasks, "create_missing_html_snapshots" + ) as mock_create_missing_html_snapshots: + mock_create_missing_html_snapshots.return_value = 0 + form_data = { + "create_missing_html_snapshots": "", + } + response = self.client.post( + reverse("bookmarks:settings.general"), form_data + ) + html = response.content.decode() + + mock_create_missing_html_snapshots.assert_called_once() + self.assertSuccessMessage(html, "No missing snapshots found.") + + def test_create_missing_html_snapshots_should_not_be_called_without_respective_form_action( + self, + ): + with patch.object( + tasks, "create_missing_html_snapshots" + ) as mock_create_missing_html_snapshots: + mock_create_missing_html_snapshots.return_value = 5 + form_data = {} + response = self.client.post( + reverse("bookmarks:settings.general"), form_data + ) + html = response.content.decode() + + mock_create_missing_html_snapshots.assert_not_called() + self.assertSuccessMessage( + html, "Queued 5 missing snapshots. This may take a while...", count=0 + ) diff --git a/bookmarks/tests/test_settings_import_view.py b/bookmarks/tests/test_settings_import_view.py index 06b6f02..eee9ad4 100644 --- a/bookmarks/tests/test_settings_import_view.py +++ b/bookmarks/tests/test_settings_import_view.py @@ -11,19 +11,27 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin): user = self.get_or_create_test_user() self.client.force_login(user) - def assertFormSuccessHint(self, response, text: str): - self.assertContains(response, '
') - self.assertContains(response, text) + def assertSuccessMessage(self, response, message: str): + self.assertInHTML( + f""" +
{ message }
+ """, + response.content.decode("utf-8"), + ) - def assertNoFormSuccessHint(self, response): - self.assertNotContains(response, '
') + def assertNoSuccessMessage(self, response): + self.assertNotContains(response, '
') - def assertFormErrorHint(self, response, text: str): - self.assertContains(response, '
') - self.assertContains(response, text) + def assertErrorMessage(self, response, message: str): + self.assertInHTML( + f""" +
{ message }
+ """, + response.content.decode("utf-8"), + ) - def assertNoFormErrorHint(self, response): - self.assertNotContains(response, '
') + def assertNoErrorMessage(self, response): + self.assertNotContains(response, '
') def test_should_import_successfully(self): with open( @@ -36,10 +44,10 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin): ) self.assertRedirects(response, reverse("bookmarks:settings.general")) - self.assertFormSuccessHint( - response, "3 bookmarks were successfully imported" + self.assertSuccessMessage( + response, "3 bookmarks were successfully imported." ) - self.assertNoFormErrorHint(response) + self.assertNoErrorMessage(response) def test_should_check_authentication(self): self.client.logout() @@ -53,8 +61,8 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin): response = self.client.post(reverse("bookmarks:settings.import"), follow=True) self.assertRedirects(response, reverse("bookmarks:settings.general")) - self.assertNoFormSuccessHint(response) - self.assertFormErrorHint(response, "Please select a file to import.") + self.assertNoSuccessMessage(response) + self.assertErrorMessage(response, "Please select a file to import.") @disable_logging def test_should_show_hint_if_import_raises_exception(self): @@ -68,8 +76,8 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin): ) self.assertRedirects(response, reverse("bookmarks:settings.general")) - self.assertNoFormSuccessHint(response) - self.assertFormErrorHint( + self.assertNoSuccessMessage(response) + self.assertErrorMessage( response, "An error occurred during bookmark import." ) @@ -87,10 +95,13 @@ class SettingsImportViewTestCase(TestCase, BookmarkFactoryMixin): ) self.assertRedirects(response, reverse("bookmarks:settings.general")) - self.assertFormSuccessHint( - response, "2 bookmarks were successfully imported" + self.assertSuccessMessage( + response, "2 bookmarks were successfully imported." + ) + self.assertErrorMessage( + response, + "1 bookmarks could not be imported. Please check the logs for more details.", ) - self.assertFormErrorHint(response, "1 bookmarks could not be imported") def test_should_respect_map_private_flag_option(self): with open( diff --git a/bookmarks/views/settings.py b/bookmarks/views/settings.py index a75615a..9e46112 100644 --- a/bookmarks/views/settings.py +++ b/bookmarks/views/settings.py @@ -25,12 +25,10 @@ def general(request): profile_form = None enable_refresh_favicons = django_settings.LD_ENABLE_REFRESH_FAVICONS has_snapshot_support = django_settings.LD_ENABLE_SNAPSHOTS - update_profile_success_message = None - refresh_favicons_success_message = None - import_success_message = _find_message_with_tag( + success_message = _find_message_with_tag( messages.get_messages(request), "bookmark_import_success" ) - import_errors_message = _find_message_with_tag( + error_message = _find_message_with_tag( messages.get_messages(request), "bookmark_import_errors" ) version_info = get_version_info(get_ttl_hash()) @@ -38,12 +36,18 @@ def general(request): if request.method == "POST": if "update_profile" in request.POST: profile_form = update_profile(request) - update_profile_success_message = "Profile updated" + success_message = "Profile updated" if "refresh_favicons" in request.POST: tasks.schedule_refresh_favicons(request.user) - refresh_favicons_success_message = ( - "Scheduled favicon update. This may take a while..." - ) + success_message = "Scheduled favicon update. This may take a while..." + if "create_missing_html_snapshots" in request.POST: + count = tasks.create_missing_html_snapshots(request.user) + if count > 0: + success_message = ( + f"Queued {count} missing snapshots. This may take a while..." + ) + else: + success_message = "No missing snapshots found." if not profile_form: profile_form = UserProfileForm(instance=request.user_profile) @@ -55,10 +59,8 @@ def general(request): "form": profile_form, "enable_refresh_favicons": enable_refresh_favicons, "has_snapshot_support": has_snapshot_support, - "update_profile_success_message": update_profile_success_message, - "refresh_favicons_success_message": refresh_favicons_success_message, - "import_success_message": import_success_message, - "import_errors_message": import_errors_message, + "success_message": success_message, + "error_message": error_message, "version_info": version_info, }, )