reduce some N+1 query in mark

journal/models/mark.py

@@ -163,7 +163,7 @@ class Mark:
     log entries
     log entry will be created when item is added to shelf
     log entry will be created when item is moved to another shelf
-    log entry will be created when item is removed from shelf (TODO change this to DEFERRED shelf)
+    log entry will be created when item is removed from shelf
     timestamp of log entry will be updated whenever created_time of shelfmember is updated
     any log entry can be deleted by user arbitrarily
 

journal/models/shelf.py

@@ -7,6 +7,7 @@ from django.db import connection, models
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 from loguru import logger
+from polymorphic.models import PolymorphicManager
 
 from catalog.models import Item, ItemCategory
 from takahe.utils import Takahe
@@ -310,6 +311,28 @@ _SHELF_LABELS = [
 # grammatically problematic, for translation only
 
 
+class ShelfMemberManager(PolymorphicManager):
+    def get_queryset(self):
+        from .comment import Comment
+        from .rating import Rating
+
+        rating_subquery = Rating.objects.filter(
+            owner_id=models.OuterRef("owner_id"), item_id=models.OuterRef("item_id")
+        ).values("grade")[:1]
+        comment_subquery = Comment.objects.filter(
+            owner_id=models.OuterRef("owner_id"), item_id=models.OuterRef("item_id")
+        ).values("text")[:1]
+        return (
+            super()
+            .get_queryset()
+            .annotate(
+                _rating_grade=models.Subquery(rating_subquery),
+                _comment_text=models.Subquery(comment_subquery),
+                _shelf_type=models.F("parent__shelf_type"),
+            )
+        )
+
+
 class ShelfMember(ListMember):
     if TYPE_CHECKING:
         parent: models.ForeignKey["ShelfMember", "Shelf"]
@@ -318,6 +341,8 @@ class ShelfMember(ListMember):
         "Shelf", related_name="members", on_delete=models.CASCADE
     )
 
+    objects = ShelfMemberManager()
+
     class Meta:
         unique_together = [["owner", "item"]]
         indexes = [
@@ -448,6 +473,15 @@ class ShelfMember(ListMember):
             "content": content,
         }
 
+    def save(self, *args, **kwargs):
+        try:
+            del self._shelf_type  # type:ignore
+            del self._rating_grade  # type:ignore
+            del self._comment_text  # type:ignore
+        except AttributeError:
+            pass
+        return super().save(*args, **kwargs)
+
     @cached_property
     def sibling_comment(self) -> "Comment | None":
         from .comment import Comment
@@ -470,18 +504,27 @@ class ShelfMember(ListMember):
 
     @property
     def shelf_label(self) -> str | None:
-        return ShelfManager.get_label(self.parent.shelf_type, self.item.category)
+        return ShelfManager.get_label(self.shelf_type, self.item.category)
 
     @property
     def shelf_type(self):
+        try:
+            return getattr(self, "_shelf_type")
+        except AttributeError:
             return self.parent.shelf_type
 
     @property
     def rating_grade(self):
+        try:
+            return getattr(self, "_rating_grade")
+        except AttributeError:
             return self.mark.rating_grade
 
     @property
     def comment_text(self):
+        try:
+            return getattr(self, "_comment_text")
+        except AttributeError:
             return self.mark.comment_text
 
     @property

journal/tests/piece.py

@@ -65,23 +65,34 @@ class ShelfTest(TestCase):
         self.assertEqual(q1.members.all().count(), 0)
         self.assertEqual(q2.members.all().count(), 0)
         Mark(user.identity, book1).update(ShelfType.WISHLIST)
-        time.sleep(0.001)  # add a little delay to make sure the timestamp is different
         Mark(user.identity, book2).update(ShelfType.WISHLIST)
+        log = [ll.shelf_type for ll in shelf_manager.get_log_for_item(book1)]
+        self.assertEqual(log, ["wishlist"])
+        log = [ll.shelf_type for ll in shelf_manager.get_log_for_item(book2)]
+        self.assertEqual(log, ["wishlist"])
+        time.sleep(0.001)  # add a little delay to make sure the timestamp is different
+
+        Mark(user.identity, book1).update(ShelfType.WISHLIST)
+        log = [ll.shelf_type for ll in shelf_manager.get_log_for_item(book1)]
+        self.assertEqual(log, ["wishlist"])
         time.sleep(0.001)
+
         self.assertEqual(q1.members.all().count(), 2)
         Mark(user.identity, book1).update(ShelfType.PROGRESS)
-        time.sleep(0.001)
         self.assertEqual(q1.members.all().count(), 1)
         self.assertEqual(q2.members.all().count(), 1)
+        time.sleep(0.001)
+
         self.assertEqual(len(Mark(user.identity, book1).all_post_ids), 2)
-        log = shelf_manager.get_log_for_item(book1)
-        self.assertEqual(log.count(), 2)
+        log = [ll.shelf_type for ll in shelf_manager.get_log_for_item(book1)]
+
+        self.assertEqual(log, ["wishlist", "progress"])
         Mark(user.identity, book1).update(ShelfType.PROGRESS, metadata={"progress": 1})
         time.sleep(0.001)
         self.assertEqual(q1.members.all().count(), 1)
         self.assertEqual(q2.members.all().count(), 1)
-        log = shelf_manager.get_log_for_item(book1)
-        self.assertEqual(log.count(), 2)
+        log = [ll.shelf_type for ll in shelf_manager.get_log_for_item(book1)]
+        self.assertEqual(log, ["wishlist", "progress"])
         self.assertEqual(len(Mark(user.identity, book1).all_post_ids), 2)
 
         # theses tests are not relevant anymore, bc we don't use log to track metadata changes
@@ -127,7 +138,8 @@ class ShelfTest(TestCase):
 
         # test delete mark ->  one more log
         Mark(user.identity, book1).delete()
-        self.assertEqual(log.count(), 4)
+        log = [ll.shelf_type for ll in shelf_manager.get_log_for_item(book1)]
+        self.assertEqual(log, ["wishlist", "progress", "complete", None])
         deleted_mark = Mark(user.identity, book1)
         self.assertEqual(deleted_mark.shelf_type, None)
         self.assertEqual(deleted_mark.tags, [])