Merge pull request #3060 from hughrun/user-migrate

Fix texts & linting plus minor bugfixes

bookwyrm/models/bookwyrm_export_job.py

@@ -42,13 +42,12 @@ def start_export_task(**kwargs):
         job.export_data = ContentFile(b"", str(uuid4()))
         json_data = json_export(job.user)
         tar_export(json_data, job.user, job.export_data)
+        job.save(update_fields=["export_data"])
     except Exception as err:  # pylint: disable=broad-except
         logger.exception("User Export Job %s Failed with error: %s", job.id, err)
         job.set_status("failed")
-    job.set_status(
+
-        "complete"
+    job.set_status("complete")
-    )  # need to explicitly set this here to trigger notifications
-    job.save(update_fields=["export_data"])
 
 
 def tar_export(json_data: str, user, file):
@@ -61,7 +60,7 @@ def tar_export(json_data: str, user, file):
         if getattr(user, "avatar", False):
             tar.add_image(user.avatar, filename="avatar")
 
-        editions = get_books_for_user(user)
+        editions, books = get_books_for_user(user)  # pylint: disable=unused-argument
         for book in editions:
             if getattr(book, "cover", False):
                 tar.add_image(book.cover)

bookwyrm/models/bookwyrm_import_job.py

@@ -61,8 +61,7 @@ def start_import_task(**kwargs):
 
             process_books(job, tar)
 
-            job.set_status("complete")  # set here to trigger notifications
+            job.set_status("complete")
-            job.save()
         archive_file.close()
 
     except Exception as err:  # pylint: disable=broad-except

bookwyrm/models/notification.py

@@ -255,7 +255,7 @@ def notify_user_on_user_import_complete(
 def notify_user_on_user_export_complete(
     sender, instance, *args, update_fields=None, **kwargs
 ):
-    """we imported your user details! aren't you proud of us"""
+    """we exported your user details! aren't you proud of us"""
     update_fields = update_fields or []
     if not instance.complete or "complete" not in update_fields:
         print("RETURNING", instance.status)

bookwyrm/tests/models/test_bookwyrm_import_job.py

@@ -62,13 +62,12 @@ class BookwyrmImport(TestCase):
                 parent_work=self.work,
             )
 
-        archive_file = pathlib.Path(__file__).parent.joinpath(
+        self.archive_file = pathlib.Path(__file__).parent.joinpath(
             "../data/bookwyrm_account_export.tar.gz"
         )
-        self.tarfile = BookwyrmTarFile.open(
+        with open(self.archive_file, "rb") as fileobj:
-            mode="r:gz", fileobj=open(archive_file, "rb")
+            tarfile = BookwyrmTarFile.open(mode="r:gz", fileobj=fileobj)
-        )
+            self.import_data = json.loads(tarfile.read("archive.json").decode("utf-8"))
-        self.import_data = json.loads(self.tarfile.read("archive.json").decode("utf-8"))
 
     def test_update_user_profile(self):
         """Test update the user's profile from import data"""
@@ -77,19 +76,22 @@ class BookwyrmImport(TestCase):
             "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
         ):
 
-            models.bookwyrm_import_job.update_user_profile(
+            with open(self.archive_file, "rb") as fileobj:
-                self.local_user, self.tarfile, self.import_data.get("user")
+                tarfile = BookwyrmTarFile.open(mode="r:gz", fileobj=fileobj)
-            )
-            self.local_user.refresh_from_db()
 
-            self.assertEqual(
+                models.bookwyrm_import_job.update_user_profile(
-                self.local_user.username, "mouse"
+                    self.local_user, tarfile, self.import_data.get("user")
-            )  # username should not change
+                )
-            self.assertEqual(self.local_user.name, "Rat")
+                self.local_user.refresh_from_db()
-            self.assertEqual(
+
-                self.local_user.summary,
+                self.assertEqual(
-                "I love to make soup in Paris and eat pizza in New York",
+                    self.local_user.username, "mouse"
-            )
+                )  # username should not change
+                self.assertEqual(self.local_user.name, "Rat")
+                self.assertEqual(
+                    self.local_user.summary,
+                    "I love to make soup in Paris and eat pizza in New York",
+                )
 
     def test_update_user_settings(self):
         """Test updating the user's settings from import data"""
@@ -248,14 +250,16 @@ class BookwyrmImport(TestCase):
         find or create the editions in the database and
         return a list of edition instances"""
 
-        self.assertEqual(models.Edition.objects.count(), 1)
         self.assertEqual(models.Edition.objects.count(), 1)
 
-        bookwyrm_import_job.get_or_create_edition(
+        with open(self.archive_file, "rb") as fileobj:
-            self.import_data["books"][1], self.tarfile
+            tarfile = BookwyrmTarFile.open(mode="r:gz", fileobj=fileobj)
-        )  # Sand Talk
 
-        self.assertEqual(models.Edition.objects.count(), 1)
+            bookwyrm_import_job.get_or_create_edition(
+                self.import_data["books"][1], tarfile
+            )  # Sand Talk
+
+            self.assertEqual(models.Edition.objects.count(), 1)
 
     def test_get_or_create_edition_not_existing(self):
         """Test take a JSON string of books and editions,
@@ -264,12 +268,16 @@ class BookwyrmImport(TestCase):
 
         self.assertEqual(models.Edition.objects.count(), 1)
 
-        bookwyrm_import_job.get_or_create_edition(
+        with open(self.archive_file, "rb") as fileobj:
-            self.import_data["books"][0], self.tarfile
+            tarfile = BookwyrmTarFile.open(mode="r:gz", fileobj=fileobj)
-        )  # Seeing like a state
+            bookwyrm_import_job.get_or_create_edition(
+                self.import_data["books"][0], tarfile
+            )  # Seeing like a state
 
-        self.assertTrue(models.Edition.objects.filter(isbn_13="9780300070163").exists())
+            self.assertTrue(
-        self.assertEqual(models.Edition.objects.count(), 2)
+                models.Edition.objects.filter(isbn_13="9780300070163").exists()
+            )
+            self.assertEqual(models.Edition.objects.count(), 2)
 
     def test_clean_values(self):
         """test clean values we don't want when creating new instances"""
@@ -373,7 +381,7 @@ class BookwyrmImport(TestCase):
         self.assertEqual(
             models.Review.objects.filter(book=self.book).first().name, "great book"
         )
-        self.assertEqual(
+        self.assertAlmostEqual(
             models.Review.objects.filter(book=self.book).first().rating, 5.00
         )
 

bookwyrm/tests/utils/test_tar.py

@@ -1,5 +1,6 @@
-from bookwyrm.utils.tar import BookwyrmTarFile
+import os
 import pytest
+from bookwyrm.utils.tar import BookwyrmTarFile
 
 
 @pytest.fixture
@@ -15,10 +16,10 @@ def write_tar():
     archive_path = "/tmp/test.tar.gz"
     with open(archive_path, "wb") as archive_file:
         with BookwyrmTarFile.open(mode="w:gz", fileobj=archive_file) as tar:
-            return tar
+            yield tar
 
     os.remove(archive_path)
 
 
 def test_write_bytes(write_tar):
-    write_tar.write_bytes(b"ABCDEF", filename="example.txt")
+    write_tar.write_bytes(b"ABCDEF")

bookwyrm/utils/tar.py

@@ -1,7 +1,7 @@
 """manage tar files for user exports"""
 import io
 import tarfile
-from typing import Any
+from typing import Any, Optional
 from uuid import uuid4
 from django.core.files import File
 
@@ -16,7 +16,9 @@ class BookwyrmTarFile(tarfile.TarFile):
         info.size = len(data)
         self.addfile(info, fileobj=buffer)
 
-    def add_image(self, image: Any, filename: str = None, directory: Any = "") -> None:
+    def add_image(
+        self, image: Any, filename: Optional[str] = None, directory: Any = ""
+    ) -> None:
         """
         Add an image to the tar archive
         :param str filename: overrides the file name set by image
@@ -35,12 +37,12 @@ class BookwyrmTarFile(tarfile.TarFile):
 
     def read(self, filename: str) -> Any:
         """read data from the tar"""
-        with self.extractfile(filename) as reader:
+        if reader := self.extractfile(filename):
             return reader.read()
 
     def write_image_to_file(self, filename: str, file_field: Any) -> None:
         """add an image to the tar"""
         extension = filename.rsplit(".")[-1]
-        with self.extractfile(filename) as reader:
+        if buf := self.extractfile(filename):
             filename = f"{str(uuid4())}.{extension}"
-            file_field.save(filename, File(reader))
+            file_field.save(filename, File(buf))