From 5d2acca1229b36b2f4f0c97c9b60f7c976723b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Sat, 20 Apr 2024 12:14:11 +0200 Subject: [PATCH] Allow uploading custom files for bookmarks (#713) --- bookmarks/frontend/behaviors/form.js | 20 +++++++ bookmarks/models.py | 1 + bookmarks/services/bookmarks.py | 52 +++++++++++++++++- bookmarks/styles/bookmark-details.scss | 10 +++- .../templates/bookmarks/details/assets.html | 16 ++++-- .../tests/test_bookmark_details_modal.py | 34 +++++++++++- bookmarks/tests/test_bookmarks_service.py | 55 ++++++++++++++++++- bookmarks/views/bookmarks.py | 7 ++- web-types.json | 7 +++ 9 files changed, 187 insertions(+), 15 deletions(-) diff --git a/bookmarks/frontend/behaviors/form.js b/bookmarks/frontend/behaviors/form.js index ae5520dc..3b6dec23 100644 --- a/bookmarks/frontend/behaviors/form.js +++ b/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); diff --git a/bookmarks/models.py b/bookmarks/models.py index 96bbb1f2..b922bd24 100644 --- a/bookmarks/models.py +++ b/bookmarks/models.py @@ -91,6 +91,7 @@ def __str__(self): class BookmarkAsset(models.Model): TYPE_SNAPSHOT = "snapshot" + TYPE_UPLOAD = "upload" CONTENT_TYPE_HTML = "text/html" diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 32976937..72ec16d1 100644 --- a/bookmarks/services/bookmarks.py +++ b/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 diff --git a/bookmarks/styles/bookmark-details.scss b/bookmarks/styles/bookmark-details.scss index b51a59ce..d19ed1af 100644 --- a/bookmarks/styles/bookmark-details.scss +++ b/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; } diff --git a/bookmarks/templates/bookmarks/details/assets.html b/bookmarks/templates/bookmarks/details/assets.html index d8576572..ab08e278 100644 --- a/bookmarks/templates/bookmarks/details/assets.html +++ b/bookmarks/templates/bookmarks/details/assets.html @@ -9,12 +9,12 @@
{% include 'bookmarks/details/asset_icon.html' %}
-
- - {{ asset.display_name }} - {% if asset.status == 'pending' %}(queued){% endif %} - {% if asset.status == 'failure' %}(failed){% endif %} - +
+ + {{ asset.display_name }} + {% if asset.status == 'pending' %}(queued){% endif %} + {% if asset.status == 'failure' %}(failed){% endif %} + {% if asset.file_size %} {{ asset.file_size|filesizeformat }} {% endif %} @@ -39,6 +39,10 @@ + +
{% endif %}
\ No newline at end of file diff --git a/bookmarks/tests/test_bookmark_details_modal.py b/bookmarks/tests/test_bookmark_details_modal.py index bf4a36a3..abe529b4 100644 --- a/bookmarks/tests/test_bookmark_details_modal.py +++ b/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 @@ def test_create_snapshot_is_disabled_when_having_pending_asset(self): "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() diff --git a/bookmarks/tests/test_bookmarks_service.py b/bookmarks/tests/test_bookmarks_service.py index 55abc60b..5326d9cd 100644 --- a/bookmarks/tests/test_bookmarks_service.py +++ b/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 @@ 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 @@ def test_unshare_bookmarks_should_accept_mix_of_int_and_string_ids(self): 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) diff --git a/bookmarks/views/bookmarks.py b/bookmarks/views/bookmarks.py index 513f7bc1..d2146524 100644 --- a/bookmarks/views/bookmarks.py +++ b/bookmarks/views/bookmarks.py @@ -34,7 +34,7 @@ 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" diff --git a/web-types.json b/web-types.json index f0c7ec1c..1f25ae24 100644 --- a/web-types.json +++ b/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",