Allow uploading custom files for bookmarks (#713)

bookmarks/frontend/behaviors/form.js

@@ -40,5 +40,25 @@ class AutoSubmitBehavior extends Behavior {
   }
 }
 
+class UploadButton extends Behavior {
+  constructor(element) {
+    super(element);
+
+    const fileInput = element.nextElementSibling;
+
+    element.addEventListener("click", () => {
+      fileInput.click();
+    });
+
+    fileInput.addEventListener("change", () => {
+      const form = fileInput.closest("form");
+      const event = new Event("submit", { cancelable: true });
+      event.submitter = element;
+      form.dispatchEvent(event);
+    });
+  }
+}
+
 registerBehavior("ld-form", FormBehavior);
 registerBehavior("ld-auto-submit", AutoSubmitBehavior);
+registerBehavior("ld-upload-button", UploadButton);

bookmarks/models.py

@@ -91,6 +91,7 @@ class Bookmark(models.Model):
 
 class BookmarkAsset(models.Model):
     TYPE_SNAPSHOT = "snapshot"
+    TYPE_UPLOAD = "upload"
 
     CONTENT_TYPE_HTML = "text/html"
 

bookmarks/services/bookmarks.py

@@ -1,12 +1,18 @@
+import logging
+import os
 from typing import Union
 
+from django.conf import settings
 from django.contrib.auth.models import User
+from django.core.files.uploadedfile import UploadedFile
 from django.utils import timezone
 
-from bookmarks.models import Bookmark, parse_tag_string
-from bookmarks.services.tags import get_or_create_tags
-from bookmarks.services import website_loader
+from bookmarks.models import Bookmark, BookmarkAsset, parse_tag_string
 from bookmarks.services import tasks
+from bookmarks.services import website_loader
+from bookmarks.services.tags import get_or_create_tags
+
+logger = logging.getLogger(__name__)
 
 
 def create_bookmark(bookmark: Bookmark, tag_string: str, current_user: User):
@@ -176,6 +182,46 @@ def unshare_bookmarks(bookmark_ids: [Union[int, str]], current_user: User):
     )
 
 
+def _generate_upload_asset_filename(asset: BookmarkAsset, filename: str):
+    formatted_datetime = asset.date_created.strftime("%Y-%m-%d_%H%M%S")
+    return f"{asset.asset_type}_{formatted_datetime}_{filename}"
+
+
+def upload_asset(bookmark: Bookmark, upload_file: UploadedFile) -> BookmarkAsset:
+    asset = BookmarkAsset(
+        bookmark=bookmark,
+        asset_type=BookmarkAsset.TYPE_UPLOAD,
+        content_type=upload_file.content_type,
+        display_name=upload_file.name,
+        status=BookmarkAsset.STATUS_PENDING,
+        gzip=False,
+    )
+    asset.save()
+
+    try:
+        filename = _generate_upload_asset_filename(asset, upload_file.name)
+        filepath = os.path.join(settings.LD_ASSET_FOLDER, filename)
+        with open(filepath, "wb") as f:
+            for chunk in upload_file.chunks():
+                f.write(chunk)
+        asset.status = BookmarkAsset.STATUS_COMPLETE
+        asset.file = filename
+        asset.file_size = upload_file.size
+        logger.info(
+            f"Successfully uploaded asset file. bookmark={bookmark} file={upload_file.name}"
+        )
+    except Exception as e:
+        logger.error(
+            f"Failed to upload asset file. bookmark={bookmark} file={upload_file.name}",
+            exc_info=e,
+        )
+        asset.status = BookmarkAsset.STATUS_FAILURE
+
+    asset.save()
+
+    return asset
+
+
 def _merge_bookmark_data(from_bookmark: Bookmark, to_bookmark: Bookmark):
     to_bookmark.title = from_bookmark.title
     to_bookmark.description = from_bookmark.description

bookmarks/styles/bookmark-details.scss

@@ -61,16 +61,22 @@
 
   .assets .asset-text {
     flex: 1 1 0;
+    gap: $unit-2;
+    min-width: 0;
+    display: flex;
+  }
+
+  .assets .asset-text .truncate {
+    flex-shrink: 1;
   }
 
   .assets .asset-text .filesize {
     color: $gray-color;
-    margin-left: $unit-2;
   }
 
   .assets .asset-actions, .assets-actions {
     display: flex;
-    gap: $unit-3;
+    gap: $unit-4;
     align-items: center;
   }
 

bookmarks/templates/bookmarks/details/assets.html

@@ -9,12 +9,12 @@
           <div class="asset-icon {{ asset.icon_classes }}">
             {% include 'bookmarks/details/asset_icon.html' %}
           </div>
-          <div class="asset-text truncate {{ asset.text_classes }}">
-        <span>
-          {{ asset.display_name }}
-          {% if asset.status == 'pending' %}(queued){% endif %}
-          {% if asset.status == 'failure' %}(failed){% endif %}
-        </span>
+          <div class="asset-text {{ asset.text_classes }}">
+          <span class="truncate">
+            {{ asset.display_name }}
+            {% if asset.status == 'pending' %}(queued){% endif %}
+            {% if asset.status == 'failure' %}(failed){% endif %}
+          </span>
             {% if asset.file_size %}
               <span class="filesize">{{ asset.file_size|filesizeformat }}</span>
             {% endif %}
@@ -39,6 +39,10 @@
       <button type="submit" name="create_snapshot" class="btn btn-link"
               {% if details.has_pending_assets %}disabled{% endif %}>Create HTML snapshot
       </button>
+      <button ld-upload-button id="upload-asset" name="upload_asset" value="{{ details.bookmark.id }}" type="button"
+              class="btn btn-link">Upload file
+      </button>
+      <input id="upload-asset-file" name="upload_asset_file" type="file" class="d-hide">
     </div>
   {% endif %}
 </div>

bookmarks/tests/test_bookmark_details_modal.py

@@ -1,12 +1,13 @@
 import re
 from unittest.mock import patch
 
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase, override_settings
 from django.urls import reverse
 from django.utils import formats
 
 from bookmarks.models import BookmarkAsset, UserProfile
-from bookmarks.services import tasks
+from bookmarks.services import bookmarks, tasks
 from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin
 
 
@@ -862,3 +863,34 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin
             "button", string=re.compile("Create HTML snapshot")
         )
         self.assertTrue(create_button.has_attr("disabled"))
+
+    def test_upload_file(self):
+        bookmark = self.setup_bookmark()
+        file_content = b"file content"
+        upload_file = SimpleUploadedFile("test.txt", file_content)
+
+        with patch.object(bookmarks, "upload_asset") as mock_upload_asset:
+            response = self.client.post(
+                self.get_base_url(bookmark),
+                {"upload_asset": "", "upload_asset_file": upload_file},
+            )
+            self.assertEqual(response.status_code, 302)
+
+            mock_upload_asset.assert_called_once()
+
+            args, kwargs = mock_upload_asset.call_args
+            self.assertEqual(args[0], bookmark)
+
+            upload_file = args[1]
+            self.assertEqual(upload_file.name, "test.txt")
+
+    def test_upload_file_without_file(self):
+        bookmark = self.setup_bookmark()
+
+        with patch.object(bookmarks, "upload_asset") as mock_upload_asset:
+            response = self.client.post(
+                self.get_base_url(bookmark),
+                {"upload_asset": ""},
+            )
+            self.assertEqual(response.status_code, 400)
+            mock_upload_asset.assert_not_called()

bookmarks/tests/test_bookmarks_service.py

@@ -1,10 +1,13 @@
+import os
+import tempfile
 from unittest.mock import patch
 
 from django.contrib.auth import get_user_model
-from django.test import TestCase
+from django.core.files.uploadedfile import SimpleUploadedFile
+from django.test import TestCase, override_settings
 from django.utils import timezone
 
-from bookmarks.models import Bookmark, Tag
+from bookmarks.models import Bookmark, BookmarkAsset, Tag
 from bookmarks.services import tasks
 from bookmarks.services import website_loader
 from bookmarks.services.bookmarks import (
@@ -21,6 +24,7 @@ from bookmarks.services.bookmarks import (
     mark_bookmarks_as_unread,
     share_bookmarks,
     unshare_bookmarks,
+    upload_asset,
 )
 from bookmarks.services.website_loader import WebsiteMetadata
 from bookmarks.tests.helpers import BookmarkFactoryMixin
@@ -835,3 +839,50 @@ class BookmarkServiceTestCase(TestCase, BookmarkFactoryMixin):
         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_upload_asset_should_save_file(self):
+        bookmark = self.setup_bookmark()
+        with tempfile.TemporaryDirectory() as temp_assets:
+            with override_settings(LD_ASSET_FOLDER=temp_assets):
+                file_content = b"file content"
+                upload_file = SimpleUploadedFile(
+                    "test_file.txt", file_content, content_type="text/plain"
+                )
+                upload_asset(bookmark, upload_file)
+
+                assets = bookmark.bookmarkasset_set.all()
+                self.assertEqual(1, len(assets))
+
+                asset = assets[0]
+                self.assertEqual("test_file.txt", asset.display_name)
+                self.assertEqual("text/plain", asset.content_type)
+                self.assertEqual(upload_file.size, asset.file_size)
+                self.assertEqual(BookmarkAsset.STATUS_COMPLETE, asset.status)
+                self.assertTrue(asset.file.startswith("upload_"))
+                self.assertTrue(asset.file.endswith(upload_file.name))
+
+                # check file exists
+                filepath = os.path.join(temp_assets, asset.file)
+                self.assertTrue(os.path.exists(filepath))
+                with open(filepath, "rb") as f:
+                    self.assertEqual(file_content, f.read())
+
+    def test_upload_asset_should_be_failed_if_saving_file_fails(self):
+        bookmark = self.setup_bookmark()
+        # Use an invalid path to force an error
+        with override_settings(LD_ASSET_FOLDER="/non/existing/folder"):
+            file_content = b"file content"
+            upload_file = SimpleUploadedFile(
+                "test_file.txt", file_content, content_type="text/plain"
+            )
+            upload_asset(bookmark, upload_file)
+
+            assets = bookmark.bookmarkasset_set.all()
+            self.assertEqual(1, len(assets))
+
+            asset = assets[0]
+            self.assertEqual("test_file.txt", asset.display_name)
+            self.assertEqual("text/plain", asset.content_type)
+            self.assertIsNone(asset.file_size)
+            self.assertEqual(BookmarkAsset.STATUS_FAILURE, asset.status)
+            self.assertEqual("", asset.file)

bookmarks/views/bookmarks.py

@@ -34,7 +34,7 @@ from bookmarks.services.bookmarks import (
     share_bookmarks,
     unshare_bookmarks,
 )
-from bookmarks.services import tasks
+from bookmarks.services import bookmarks as bookmark_actions, tasks
 from bookmarks.utils import get_safe_return_url
 from bookmarks.views.partials import contexts
 
@@ -145,6 +145,11 @@ def _details(request, bookmark_id: int, template: str):
             asset.delete()
         if "create_snapshot" in request.POST:
             tasks.create_html_snapshot(bookmark)
+        if "upload_asset" in request.POST:
+            file = request.FILES.get("upload_asset_file")
+            if not file:
+                return HttpResponseBadRequest("No file uploaded")
+            bookmark_actions.upload_asset(bookmark, file)
         else:
             bookmark.is_archived = request.POST.get("is_archived") == "on"
             bookmark.unread = request.POST.get("unread") == "on"

web-types.json

@@ -61,6 +61,13 @@
             "required": false
           }
         },
+        {
+          "name": "ld-upload-button",
+          "description": "Opens the related file input when clicked, and submits the form when a file is selected",
+          "value": {
+            "required": false
+          }
+        },
         {
           "name": "ld-modal",
           "description": "Adds Javascript behavior to a modal HTML component",