Repository: sissbruecker/linkding
Pull request #860 (2024-09-27 01:40:23)

Add LAST_MODIFIED attribute when exporting

sissbruecker: What would be the benefit of adding this?
sissbruecker: Preserving information is not really a benefit in itself, unless it supports some other feature. The modified date isn't really used anywhere in the app, which is why I'm asking.
sissbruecker: I saw that `LAST_MODIFIED` is something that's also supported by browsers, with that I'm fine with adding it.

One thing that's missing from this PR is tests, would be good if you could add those.

Patch:
diff --git a/bookmarks/services/exporter.py b/bookmarks/services/exporter.py
index ebc3632..fe41bf4 100644
--- a/bookmarks/services/exporter.py
+++ b/bookmarks/services/exporter.py
@@ -40,9 +40,10 @@ def append_bookmark(doc: BookmarkDocument, bookmark: Bookmark):
     toread = "1" if bookmark.unread else "0"
     private = "0" if bookmark.shared else "1"
     added = int(bookmark.date_added.timestamp())
+    modified = int(bookmark.date_modified.timestamp())
 
     doc.append(
-        f'<DT><A HREF="{url}" ADD_DATE="{added}" PRIVATE="{private}" TOREAD="{toread}" TAGS="{tags}">{title}</A>'
+        f'<DT><A HREF="{url}" ADD_DATE="{added}" LAST_MODIFIED="{modified}" PRIVATE="{private}" TOREAD="{toread}" TAGS="{tags}">{title}</A>'
     )
 
     if desc:
diff --git a/bookmarks/services/importer.py b/bookmarks/services/importer.py
index d4bbd5a..41fda08 100644
--- a/bookmarks/services/importer.py
+++ b/bookmarks/services/importer.py
@@ -231,7 +231,10 @@ def _copy_bookmark_data(
         bookmark.date_added = parse_timestamp(netscape_bookmark.date_added)
     else:
         bookmark.date_added = timezone.now()
-    bookmark.date_modified = bookmark.date_added
+    if netscape_bookmark.date_modified:
+        bookmark.date_modified = parse_timestamp(netscape_bookmark.date_modified)
+    else:
+        bookmark.date_modified = bookmark.date_added
     bookmark.unread = netscape_bookmark.to_read
     if netscape_bookmark.title:
         bookmark.title = netscape_bookmark.title
diff --git a/bookmarks/services/parser.py b/bookmarks/services/parser.py
index 81c4a41..4eb7c1c 100644
--- a/bookmarks/services/parser.py
+++ b/bookmarks/services/parser.py
@@ -12,6 +12,7 @@ class NetscapeBookmark:
     description: str
     notes: str
     date_added: str
+    date_modified: str
     tag_names: List[str]
     to_read: bool
     private: bool
@@ -27,6 +28,7 @@ class BookmarkParser(HTMLParser):
         self.bookmark = None
         self.href = ""
         self.add_date = ""
+        self.last_modified = ""
         self.tags = ""
         self.title = ""
         self.description = ""
@@ -72,6 +74,7 @@ class BookmarkParser(HTMLParser):
             description="",
             notes="",
             date_added=self.add_date,
+            date_modified=self.last_modified,
             tag_names=tag_names,
             to_read=self.toread == "1",
             # Mark as private by default, also when attribute is not specified
@@ -97,6 +100,7 @@ class BookmarkParser(HTMLParser):
         self.bookmark = None
         self.href = ""
         self.add_date = ""
+        self.last_modified = ""
         self.tags = ""
         self.title = ""
         self.description = ""
diff --git a/docker/alpine.Dockerfile b/docker/alpine.Dockerfile
index 74e2aa8..4f7bb3e 100644
--- a/docker/alpine.Dockerfile
+++ b/docker/alpine.Dockerfile
@@ -111,7 +111,9 @@ RUN apk update && apk add nodejs npm chromium
 RUN npm install -g https://github.com/sissbruecker/single-file-cli/tarball/4c54b3bc704cfb3e96cec2d24854caca3df0b3b6
 # copy uBlock
 COPY --from=ublock-build /etc/linkding/uBOLite.chromium.mv3 uBOLite.chromium.mv3/
-# create chromium profile folder for user running background tasks
-RUN mkdir -p chromium-profile && chown -R www-data:www-data chromium-profile
+# create chromium profile folder for user running background tasks and set permissions
+RUN mkdir -p chromium-profile &&  \
+    chown -R www-data:www-data chromium-profile &&  \
+    chown -R www-data:www-data uBOLite.chromium.mv3
 # enable snapshot support
 ENV LD_ENABLE_SNAPSHOTS=True
diff --git a/docker/default.Dockerfile b/docker/default.Dockerfile
index 629bbdd..242567c 100644
--- a/docker/default.Dockerfile
+++ b/docker/default.Dockerfile
@@ -115,7 +115,9 @@ RUN apt-get install -y gnupg2 apt-transport-https ca-certificates && \
 RUN npm install -g https://github.com/sissbruecker/single-file-cli/tarball/4c54b3bc704cfb3e96cec2d24854caca3df0b3b6
 # copy uBlock
 COPY --from=ublock-build /etc/linkding/uBOLite.chromium.mv3 uBOLite.chromium.mv3/
-# create chromium profile folder for user running background tasks
-RUN mkdir -p chromium-profile && chown -R www-data:www-data chromium-profile
+# create chromium profile folder for user running background tasks and set permissions
+RUN mkdir -p chromium-profile &&  \
+    chown -R www-data:www-data chromium-profile &&  \
+    chown -R www-data:www-data uBOLite.chromium.mv3
 # enable snapshot support
 ENV LD_ENABLE_SNAPSHOTS=True


Test patch:
diff --git a/bookmarks/tests/helpers.py b/bookmarks/tests/helpers.py
index ca3ea64..bc6539e 100644
--- a/bookmarks/tests/helpers.py
+++ b/bookmarks/tests/helpers.py
@@ -45,6 +45,7 @@ class BookmarkFactoryMixin:
         favicon_file: str = "",
         preview_image_file: str = "",
         added: datetime = None,
+        modified: datetime = None,
     ):
         if title is None:
             title = get_random_string(length=32)
@@ -57,13 +58,15 @@ class BookmarkFactoryMixin:
             url = "https://example.com/" + unique_id
         if added is None:
             added = timezone.now()
+        if modified is None:
+            modified = timezone.now()
         bookmark = Bookmark(
             url=url,
             title=title,
             description=description,
             notes=notes,
             date_added=added,
-            date_modified=timezone.now(),
+            date_modified=modified,
             owner=user,
             is_archived=is_archived,
             unread=unread,
@@ -320,6 +323,7 @@ class BookmarkHtmlTag:
         title: str = "",
         description: str = "",
         add_date: str = "",
+        last_modified: str = "",
         tags: str = "",
         to_read: bool = False,
         private: bool = True,
@@ -328,6 +332,7 @@ class BookmarkHtmlTag:
         self.title = title
         self.description = description
         self.add_date = add_date
+        self.last_modified = last_modified
         self.tags = tags
         self.to_read = to_read
         self.private = private
@@ -339,6 +344,7 @@ class ImportTestMixin:
         <DT>
         <A {f'HREF="{tag.href}"' if tag.href else ''}
            {f'ADD_DATE="{tag.add_date}"' if tag.add_date else ''}
+           {f'LAST_MODIFIED="{tag.last_modified}"' if tag.last_modified else ''}
            {f'TAGS="{tag.tags}"' if tag.tags else ''}
            TOREAD="{1 if tag.to_read else 0}"
            PRIVATE="{1 if tag.private else 0}">
diff --git a/bookmarks/tests/test_exporter.py b/bookmarks/tests/test_exporter.py
index 2457866..2056bc7 100644
--- a/bookmarks/tests/test_exporter.py
+++ b/bookmarks/tests/test_exporter.py
@@ -1,5 +1,6 @@
+from datetime import datetime, timezone
+
 from django.test import TestCase
-from django.utils import timezone
 
 from bookmarks.services import exporter
 from bookmarks.tests.helpers import BookmarkFactoryMixin
@@ -7,20 +8,19 @@ from bookmarks.tests.helpers import BookmarkFactoryMixin
 
 class ExporterTestCase(TestCase, BookmarkFactoryMixin):
     def test_export_bookmarks(self):
-        added = timezone.now()
-        timestamp = int(added.timestamp())
-
         bookmarks = [
             self.setup_bookmark(
                 url="https://example.com/1",
                 title="Title 1",
-                added=added,
+                added=datetime.fromtimestamp(1, timezone.utc),
+                modified=datetime.fromtimestamp(11, timezone.utc),
                 description="Example description",
             ),
             self.setup_bookmark(
                 url="https://example.com/2",
                 title="Title 2",
-                added=added,
+                added=datetime.fromtimestamp(2, timezone.utc),
+                modified=datetime.fromtimestamp(22, timezone.utc),
                 tags=[
                     self.setup_tag(name="tag1"),
                     self.setup_tag(name="tag2"),
@@ -28,15 +28,24 @@ class ExporterTestCase(TestCase, BookmarkFactoryMixin):
                 ],
             ),
             self.setup_bookmark(
-                url="https://example.com/3", title="Title 3", added=added, unread=True
+                url="https://example.com/3",
+                title="Title 3",
+                added=datetime.fromtimestamp(3, timezone.utc),
+                modified=datetime.fromtimestamp(33, timezone.utc),
+                unread=True,
             ),
             self.setup_bookmark(
-                url="https://example.com/4", title="Title 4", added=added, shared=True
+                url="https://example.com/4",
+                title="Title 4",
+                added=datetime.fromtimestamp(4, timezone.utc),
+                modified=datetime.fromtimestamp(44, timezone.utc),
+                shared=True,
             ),
             self.setup_bookmark(
                 url="https://example.com/5",
                 title="Title 5",
-                added=added,
+                added=datetime.fromtimestamp(5, timezone.utc),
+                modified=datetime.fromtimestamp(55, timezone.utc),
                 shared=True,
                 description="Example description",
                 notes="Example notes",
@@ -44,20 +53,23 @@ class ExporterTestCase(TestCase, BookmarkFactoryMixin):
             self.setup_bookmark(
                 url="https://example.com/6",
                 title="Title 6",
-                added=added,
+                added=datetime.fromtimestamp(6, timezone.utc),
+                modified=datetime.fromtimestamp(66, timezone.utc),
                 shared=True,
                 notes="Example notes",
             ),
             self.setup_bookmark(
                 url="https://example.com/7",
                 title="Title 7",
-                added=added,
+                added=datetime.fromtimestamp(7, timezone.utc),
+                modified=datetime.fromtimestamp(77, timezone.utc),
                 is_archived=True,
             ),
             self.setup_bookmark(
                 url="https://example.com/8",
                 title="Title 8",
-                added=added,
+                added=datetime.fromtimestamp(8, timezone.utc),
+                modified=datetime.fromtimestamp(88, timezone.utc),
                 tags=[self.setup_tag(name="tag4"), self.setup_tag(name="tag5")],
                 is_archived=True,
             ),
@@ -65,17 +77,17 @@ class ExporterTestCase(TestCase, BookmarkFactoryMixin):
         html = exporter.export_netscape_html(bookmarks)
 
         lines = [
-            f'<DT><A HREF="https://example.com/1" ADD_DATE="{timestamp}" PRIVATE="1" TOREAD="0" TAGS="">Title 1</A>',
+            '<DT><A HREF="https://example.com/1" ADD_DATE="1" LAST_MODIFIED="11" PRIVATE="1" TOREAD="0" TAGS="">Title 1</A>',
             "<DD>Example description",
-            f'<DT><A HREF="https://example.com/2" ADD_DATE="{timestamp}" PRIVATE="1" TOREAD="0" TAGS="tag1,tag2,tag3">Title 2</A>',
-            f'<DT><A HREF="https://example.com/3" ADD_DATE="{timestamp}" PRIVATE="1" TOREAD="1" TAGS="">Title 3</A>',
-            f'<DT><A HREF="https://example.com/4" ADD_DATE="{timestamp}" PRIVATE="0" TOREAD="0" TAGS="">Title 4</A>',
-            f'<DT><A HREF="https://example.com/5" ADD_DATE="{timestamp}" PRIVATE="0" TOREAD="0" TAGS="">Title 5</A>',
+            '<DT><A HREF="https://example.com/2" ADD_DATE="2" LAST_MODIFIED="22" PRIVATE="1" TOREAD="0" TAGS="tag1,tag2,tag3">Title 2</A>',
+            '<DT><A HREF="https://example.com/3" ADD_DATE="3" LAST_MODIFIED="33" PRIVATE="1" TOREAD="1" TAGS="">Title 3</A>',
+            '<DT><A HREF="https://example.com/4" ADD_DATE="4" LAST_MODIFIED="44" PRIVATE="0" TOREAD="0" TAGS="">Title 4</A>',
+            '<DT><A HREF="https://example.com/5" ADD_DATE="5" LAST_MODIFIED="55" PRIVATE="0" TOREAD="0" TAGS="">Title 5</A>',
             "<DD>Example description[linkding-notes]Example notes[/linkding-notes]",
-            f'<DT><A HREF="https://example.com/6" ADD_DATE="{timestamp}" PRIVATE="0" TOREAD="0" TAGS="">Title 6</A>',
+            '<DT><A HREF="https://example.com/6" ADD_DATE="6" LAST_MODIFIED="66" PRIVATE="0" TOREAD="0" TAGS="">Title 6</A>',
             "<DD>[linkding-notes]Example notes[/linkding-notes]",
-            f'<DT><A HREF="https://example.com/7" ADD_DATE="{timestamp}" PRIVATE="1" TOREAD="0" TAGS="linkding:archived">Title 7</A>',
-            f'<DT><A HREF="https://example.com/8" ADD_DATE="{timestamp}" PRIVATE="1" TOREAD="0" TAGS="tag4,tag5,linkding:archived">Title 8</A>',
+            '<DT><A HREF="https://example.com/7" ADD_DATE="7" LAST_MODIFIED="77" PRIVATE="1" TOREAD="0" TAGS="linkding:archived">Title 7</A>',
+            '<DT><A HREF="https://example.com/8" ADD_DATE="8" LAST_MODIFIED="88" PRIVATE="1" TOREAD="0" TAGS="tag4,tag5,linkding:archived">Title 8</A>',
         ]
         self.assertIn("\n\r".join(lines), html)
 
diff --git a/bookmarks/tests/test_importer.py b/bookmarks/tests/test_importer.py
index 56b97ce..b632d62 100644
--- a/bookmarks/tests/test_importer.py
+++ b/bookmarks/tests/test_importer.py
@@ -26,6 +26,9 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
             self.assertEqual(bookmark.title, html_tag.title)
             self.assertEqual(bookmark.description, html_tag.description)
             self.assertEqual(bookmark.date_added, parse_timestamp(html_tag.add_date))
+            self.assertEqual(
+                bookmark.date_modified, parse_timestamp(html_tag.last_modified)
+            )
             self.assertEqual(bookmark.unread, html_tag.to_read)
             self.assertEqual(bookmark.shared, not html_tag.private)
 
@@ -45,6 +48,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Example title",
                 description="Example description",
                 add_date="1",
+                last_modified="11",
                 tags="example-tag",
             ),
             BookmarkHtmlTag(
@@ -52,6 +56,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Foo title",
                 description="",
                 add_date="2",
+                last_modified="22",
                 tags="",
             ),
             BookmarkHtmlTag(
@@ -59,6 +64,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Bar title",
                 description="Bar description",
                 add_date="3",
+                last_modified="33",
                 tags="bar-tag, other-tag",
             ),
             BookmarkHtmlTag(
@@ -66,6 +72,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Baz title",
                 description="Baz description",
                 add_date="4",
+                last_modified="44",
                 to_read=True,
             ),
         ]
@@ -90,6 +97,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Example title",
                 description="Example description",
                 add_date="1",
+                last_modified="11",
                 tags="example-tag",
             ),
             BookmarkHtmlTag(
@@ -97,6 +105,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Foo title",
                 description="",
                 add_date="2",
+                last_modified="22",
                 tags="",
             ),
             BookmarkHtmlTag(
@@ -104,20 +113,23 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Bar title",
                 description="Bar description",
                 add_date="3",
+                last_modified="33",
                 tags="bar-tag, other-tag",
             ),
             BookmarkHtmlTag(
                 href="https://example.com/unread",
                 title="Unread title",
                 description="Unread description",
-                add_date="3",
+                add_date="4",
+                last_modified="44",
                 to_read=True,
             ),
             BookmarkHtmlTag(
                 href="https://example.com/private",
                 title="Private title",
                 description="Private description",
-                add_date="4",
+                add_date="5",
+                last_modified="55",
                 private=True,
             ),
         ]
@@ -136,6 +148,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Updated Example title",
                 description="Updated Example description",
                 add_date="111",
+                last_modified="1111",
                 tags="updated-example-tag",
             ),
             BookmarkHtmlTag(
@@ -143,6 +156,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Updated Foo title",
                 description="Updated Foo description",
                 add_date="222",
+                last_modified="2222",
                 tags="new-tag",
             ),
             BookmarkHtmlTag(
@@ -150,6 +164,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Updated Bar title",
                 description="Updated Bar description",
                 add_date="333",
+                last_modified="3333",
                 tags="updated-bar-tag, updated-other-tag",
             ),
             BookmarkHtmlTag(
@@ -157,6 +172,7 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Unread title",
                 description="Unread description",
                 add_date="3",
+                last_modified="3",
                 to_read=False,
             ),
             BookmarkHtmlTag(
@@ -164,9 +180,15 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 title="Private title",
                 description="Private description",
                 add_date="4",
+                last_modified="4",
                 private=False,
             ),
-            BookmarkHtmlTag(href="https://baz.com", add_date="444", tags="baz-tag"),
+            BookmarkHtmlTag(
+                href="https://baz.com",
+                add_date="444",
+                last_modified="4444",
+                tags="baz-tag",
+            ),
         ]
 
         # Import updated data
@@ -291,6 +313,19 @@ class ImporterTestCase(TestCase, BookmarkFactoryMixin, ImportTestMixin):
                 Bookmark.objects.all()[0].date_added, timezone.datetime(2021, 1, 1)
             )
 
+    def test_use_add_date_when_no_last_modified(self):
+        test_html = self.render_html(
+            tags_html=f"""
+            <DT><A HREF="https://example.com" ADD_DATE="1">Example.com</A>
+            <DD>Example.com
+        """
+        )
+
+        import_netscape_html(test_html, self.get_or_create_test_user())
+
+        self.assertEqual(Bookmark.objects.count(), 1)
+        self.assertEqual(Bookmark.objects.all()[0].date_modified, parse_timestamp("1"))
+
     def test_keep_title_if_imported_bookmark_has_empty_title(self):
         test_html = self.render_html(
             tags=[BookmarkHtmlTag(href="https://example.com", title="Example.com")]
diff --git a/bookmarks/tests/test_parser.py b/bookmarks/tests/test_parser.py
index 1607217..3e82980 100644
--- a/bookmarks/tests/test_parser.py
+++ b/bookmarks/tests/test_parser.py
@@ -18,6 +18,7 @@ class ParserTestCase(TestCase, ImportTestMixin):
             self.assertEqual(bookmark.href, html_tag.href)
             self.assertEqual(bookmark.title, html_tag.title)
             self.assertEqual(bookmark.date_added, html_tag.add_date)
+            self.assertEqual(bookmark.date_modified, html_tag.last_modified)
             self.assertEqual(bookmark.description, html_tag.description)
             self.assertEqual(bookmark.tag_names, parse_tag_string(html_tag.tags))
             self.assertEqual(bookmark.to_read, html_tag.to_read)
@@ -30,6 +31,7 @@ class ParserTestCase(TestCase, ImportTestMixin):
                 title="Example title",
                 description="Example description",
                 add_date="1",
+                last_modified="11",
                 tags="example-tag",
             ),
             BookmarkHtmlTag(
@@ -37,6 +39,7 @@ class ParserTestCase(TestCase, ImportTestMixin):
                 title="Foo title",
                 description="",
                 add_date="2",
+                last_modified="22",
                 tags="",
             ),
             BookmarkHtmlTag(
@@ -44,13 +47,14 @@ class ParserTestCase(TestCase, ImportTestMixin):
                 title="Bar title",
                 description="Bar description",
                 add_date="3",
+                last_modified="33",
                 tags="bar-tag, other-tag",
             ),
             BookmarkHtmlTag(
                 href="https://example.com/baz",
                 title="Baz title",
                 description="Baz description",
-                add_date="3",
+                add_date="4",
                 to_read=True,
             ),
         ]
@@ -72,9 +76,17 @@ class ParserTestCase(TestCase, ImportTestMixin):
                 title="Example title",
                 description="Example description",
                 add_date="1",
+                last_modified="1",
                 tags="example-tag",
             ),
-            BookmarkHtmlTag(href="", title="", description="", add_date="", tags=""),
+            BookmarkHtmlTag(
+                href="",
+                title="",
+                description="",
+                add_date="",
+                last_modified="",
+                tags="",
+            ),
         ]
         html = self.render_html(html_tags)
         bookmarks = parse(html)