Add button for creating missing HTML snapshots (#696)

* add button for creating missing HTML snapshots

* refactor messages in settings view

* show alternative text when there are no missing snapshots

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()
+    return asset
-
-
-MAX_SNAPSHOT_FILENAME_LENGTH = 192
 
 
 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()

bookmarks/templates/settings/general.html

@@ -8,6 +8,12 @@
 
     {# Profile section #}
     <section class="content-area">
+      {% if success_message %}
+        <div class="toast toast-success mb-4">{{ success_message }}</div>
+      {% endif %}
+      {% if error_message %}
+        <div class="toast toast-error mb-4">{{ error_message }}</div>
+      {% endif %}
       <h2>Profile</h2>
       <p>
         <a href="{% url 'change_password' %}">Change password</a>
@@ -120,13 +126,6 @@
           {% if request.user_profile.enable_favicons and enable_refresh_favicons %}
             <button class="btn mt-2" name="refresh_favicons">Refresh Favicons</button>
           {% endif %}
-          {% if refresh_favicons_success_message %}
-            <div class="has-success">
-              <p class="form-input-hint">
-                {{ refresh_favicons_success_message }}
-              </p>
-            </div>
-          {% endif %}
         </div>
         <div class="form-group">
           <label for="{{ form.web_archive_integration.id_for_label }}" class="form-label">Internet Archive
@@ -173,6 +172,7 @@
               Automatically creates HTML snapshots when adding bookmarks. Alternatively, when disabled, snapshots can be
               created manually in the details view of a bookmark.
             </div>
+            <button class="btn mt-2" name="create_missing_html_snapshots">Create missing HTML snapshots</button>
           </div>
         {% endif %}
         <div class="form-group">
@@ -189,13 +189,6 @@
         </div>
         <div class="form-group">
           <input type="submit" name="update_profile" value="Save" class="btn btn-primary mt-2">
-          {% if update_profile_success_message %}
-            <div class="has-success">
-              <p class="form-input-hint">
-                {{ update_profile_success_message }}
-              </p>
-            </div>
-          {% endif %}
         </div>
       </form>
     </section>
@@ -224,20 +217,6 @@
             <input class="form-input" type="file" name="import_file">
             <input type="submit" class="input-group-btn btn btn-primary" value="Upload">
           </div>
-          {% if import_success_message %}
-            <div class="has-success">
-              <p class="form-input-hint">
-                {{ import_success_message }}
-              </p>
-            </div>
-          {% endif %}
-          {% if import_errors_message %}
-            <div class="has-error">
-              <p class="form-input-hint">
-                {{ import_errors_message }}
-              </p>
-            </div>
-          {% endif %}
         </div>
       </form>
     </section>

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)

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"""
+            <div class="toast toast-success mb-4">{ message }</div>
+        """,
+            html,
+            count=count,
+        )
+
+    def assertErrorMessage(self, html, message: str, count=1):
+        self.assertInHTML(
+            f"""
+            <div class="toast toast-error mb-4">{ message }</div>
+        """,
+            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(
+        self.assertSuccessMessage(html, "Profile updated")
-            """
-                <p class="form-input-hint">Profile updated</p>
-            """,
-            html,
-        )
 
     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(
+        self.assertSuccessMessage(html, "Profile updated", count=0)
-            """
-                <p class="form-input-hint">Profile updated</p>
-            """,
-            html,
-            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(
+            self.assertSuccessMessage(
-                """
+                html, "Scheduled favicon update. This may take a while..."
-                <p class="form-input-hint">
-                    Scheduled favicon update. This may take a while...
-                </p>
-            """,
-                html,
             )
 
     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(
+            self.assertSuccessMessage(
-                """
+                html, "Scheduled favicon update. This may take a while...", count=0
-                <p class="form-input-hint">
-                    Scheduled favicon update. This may take a while...
-                </p>
-            """,
-                html,
-                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
+            )

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):
+    def assertSuccessMessage(self, response, message: str):
-        self.assertContains(response, '<div class="has-success">')
+        self.assertInHTML(
-        self.assertContains(response, text)
+            f"""
+            <div class="toast toast-success mb-4">{ message }</div>
+        """,
+            response.content.decode("utf-8"),
+        )
 
-    def assertNoFormSuccessHint(self, response):
+    def assertNoSuccessMessage(self, response):
-        self.assertNotContains(response, '<div class="has-success">')
+        self.assertNotContains(response, '<div class="toast toast-success mb-4">')
 
-    def assertFormErrorHint(self, response, text: str):
+    def assertErrorMessage(self, response, message: str):
-        self.assertContains(response, '<div class="has-error">')
+        self.assertInHTML(
-        self.assertContains(response, text)
+            f"""
+            <div class="toast toast-error mb-4">{ message }</div>
+        """,
+            response.content.decode("utf-8"),
+        )
 
-    def assertNoFormErrorHint(self, response):
+    def assertNoErrorMessage(self, response):
-        self.assertNotContains(response, '<div class="has-error">')
+        self.assertNotContains(response, '<div class="toast toast-error mb-4">')
 
     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(
+            self.assertSuccessMessage(
-                response, "3 bookmarks were successfully imported"
+                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.assertNoSuccessMessage(response)
-        self.assertFormErrorHint(response, "Please select a file to import.")
+        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.assertNoSuccessMessage(response)
-            self.assertFormErrorHint(
+            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(
+            self.assertSuccessMessage(
-                response, "2 bookmarks were successfully imported"
+                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(

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
+    success_message = _find_message_with_tag(
-    refresh_favicons_success_message = None
-    import_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 = (
+            success_message = "Scheduled favicon update. This may take a while..."
-                "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,
+            "success_message": success_message,
-            "refresh_favicons_success_message": refresh_favicons_success_message,
+            "error_message": error_message,
-            "import_success_message": import_success_message,
-            "import_errors_message": import_errors_message,
             "version_info": version_info,
         },
     )