further streamline mark.update()

journal/models/common.py

@@ -194,9 +194,6 @@ def ap_object(self):
     def link_post_id(self, post_id: int):
         PiecePost.objects.get_or_create(piece=self, post_id=post_id)
 
-    def link_post(self, post: "Post"):
-        return self.link_post_id(post.pk)
-
     def clear_post_ids(self):
         PiecePost.objects.filter(piece=self).delete()
 

journal/models/mark.py

@@ -88,7 +88,7 @@ def visibility(self) -> int:
         if self.shelfmember:
             return self.shelfmember.visibility
         else:
-            # mark not saved yet, return default visibility for editing ui
+            # mark not created/saved yet, use user's default visibility
             return self.owner.preference.default_visibility
 
     @cached_property
@@ -148,48 +148,33 @@ def logs(self):
     @property
     def all_post_ids(self):
         """all post ids for this user and item"""
-        pass
+        return self.logs.values_list("posts", flat=True)
 
     @property
     def current_post_ids(self):
-        """all post ids for this user and item for its current shelf"""
-        pass
+        """all post ids for this user and item for its current status"""
+        return self.shelfmember.all_post_ids if self.shelfmember else []
 
     @property
     def latest_post_id(self):
-        """latest post id for this user and item for its current shelf"""
-        pass
-
-    def wish(self):
-        """add to wishlist if not on shelf"""
-        if self.shelfmember:
-            logger.warning("item already on shelf, cannot wishlist again")
-            return False
-        self.shelfmember = ShelfMember.objects.create(
-            owner=self.owner,
-            item=self.item,
-            parent=Shelf.objects.get(owner=self.owner, shelf_type=ShelfType.WISHLIST),
-            visibility=self.owner.preference.default_visibility,
-        )
-        self.shelfmember.create_log_entry()
-        post = Takahe.post_mark(self, True)
-        if post and not self.owner.preference.default_no_share:
-            boost_toot_later(self.owner, post.url)
-        return True
+        """latest post id for this user and item for its current status"""
+        return self.shelfmember.latest_post_id if self.shelfmember else None
 
     def update(
         self,
         shelf_type: ShelfType | None,
-        comment_text: str | None,
-        rating_grade: int | None,
-        visibility: int,
+        comment_text: str | None = None,
+        rating_grade: int | None = None,
+        visibility: int | None = None,
         metadata=None,
         created_time=None,
         share_to_mastodon=False,
     ):
         """change shelf, comment or rating"""
         if created_time and created_time >= timezone.now():
             created_time = None
+        if visibility is None:
+            visibility = self.visibility
         last_shelf_type = self.shelf_type
         last_visibility = self.visibility if last_shelf_type else None
         if shelf_type is None:  # TODO change this use case to DEFERRED status
@@ -201,14 +186,19 @@ def update(
         # create/update shelf member and shelf log if necessary
         if last_shelf_type == shelf_type:
             shelfmember_changed = False
+            log_entry = self.shelfmember.ensure_log_entry()
+            if metadata is not None and metadata != self.shelfmember.metadata:
+                self.shelfmember.metadata = metadata
+                shelfmember_changed = True
             if last_visibility != visibility:
                 self.shelfmember.visibility = visibility
                 shelfmember_changed = True
                 # retract most recent post about this status when visibility changed
-                Takahe.delete_posts([self.shelfmember.latest_post_id])
+                latest_post = self.shelfmember.latest_post
+                if latest_post:
+                    Takahe.delete_posts([latest_post.pk])
             if created_time and created_time != self.shelfmember.created_time:
                 self.shelfmember.created_time = created_time
-                log_entry = self.shelfmember.ensure_log_entry()
                 log_entry.timestamp = created_time
                 log_entry.save(update_fields=["timestamp"])
                 self.shelfmember.change_timestamp(created_time)
@@ -225,32 +215,36 @@ def update(
             self.shelfmember, _ = ShelfMember.objects.update_or_create(
                 owner=self.owner, item=self.item, defaults=d
             )
-            self.shelfmember.create_log_entry()
+            self.shelfmember.ensure_log_entry()
             self.shelfmember.clear_post_ids()
         # create/update/detele comment if necessary
-        if comment_text != self.comment_text or visibility != last_visibility:
-            self.comment = Comment.comment_item(
-                self.item,
-                self.owner,
-                comment_text,
-                visibility,
-                self.shelfmember.created_time,
-            )
+        if comment_text is not None:
+            if comment_text != self.comment_text or visibility != last_visibility:
+                self.comment = Comment.comment_item(
+                    self.item,
+                    self.owner,
+                    comment_text,
+                    visibility,
+                    self.shelfmember.created_time,
+                )
         # create/update/detele rating if necessary
-        if rating_grade != self.rating_grade or visibility != last_visibility:
-            Rating.update_item_rating(self.item, self.owner, rating_grade, visibility)
-            self.rating_grade = rating_grade
+        if rating_grade is not None:
+            if rating_grade != self.rating_grade or visibility != last_visibility:
+                Rating.update_item_rating(
+                    self.item, self.owner, rating_grade, visibility
+                )
+                self.rating_grade = rating_grade
         # publish a new or updated ActivityPub post
-        post_as_new = shelf_type != self.shelf_type or visibility != self.visibility
-        post = Takahe.post_mark(self, post_as_new)
+        post_as_new = shelf_type != last_shelf_type or visibility != last_visibility
+        post = Takahe.post_mark(self, post_as_new)  # this will update linked post
         # async boost to mastodon
         if post and share_to_mastodon:
             boost_toot_later(self.owner, post.url)
         return True
 
     def delete(self):
         # self.logs.delete()  # When deleting a mark, all logs of the mark are deleted first.
-        self.update(None, None, None, 0)
+        self.update(None)
 
     def delete_log(self, log_id):
         ShelfLogEntry.objects.filter(

journal/models/shelf.py

@@ -128,24 +128,32 @@ def comment_text(self):
     def tags(self):
         return self.mark.tags
 
-    def get_log_entry(self):
-        return ShelfLogEntry.objects.filter(
-            owner=self.owner,
-            item=self.item,
-            timestamp=self.created_time,
-        ).first()
+    # def get_log_entry(self):
+    #     return ShelfLogEntry.objects.filter(
+    #         owner=self.owner,
+    #         item=self.item,
+    #         shelf_type=self.shelf_type,
+    #         timestamp=self.created_time,
+    #     ).first()
+
+    # def create_log_entry(self):
+    #     return ShelfLogEntry.objects.create(
+    #         owner=self.owner,
+    #         shelf_type=self.shelf_type,
+    #         item=self.item,
+    #         metadata=self.metadata,
+    #         timestamp=self.created_time,
+    #     )
 
-    def create_log_entry(self):
-        return ShelfLogEntry.objects.create(
+    def ensure_log_entry(self):
+        log, _ = ShelfLogEntry.objects.get_or_create(
             owner=self.owner,
             shelf_type=self.shelf_type,
             item=self.item,
-            metadata=self.metadata,
             timestamp=self.created_time,
+            defaults={"metadata": self.metadata},
         )
-
-    def ensure_log_entry(self):
-        return self.get_log_entry() or self.create_log_entry()
+        return log
 
     def log_and_delete(self):
         ShelfLogEntry.objects.create(
@@ -241,67 +249,6 @@ def initialize(self):
     def locate_item(self, item: Item) -> ShelfMember | None:
         return ShelfMember.objects.filter(item=item, owner=self.owner).first()
 
-    def move_item(  # TODO remove this method
-        self,
-        item: Item,
-        shelf_type: ShelfType,
-        visibility: int = 0,
-        metadata: dict | None = None,
-    ):
-        # shelf_type=None means remove from current shelf
-        # metadata=None means no change
-        if not item:
-            raise ValueError("empty item")
-        new_shelfmember = None
-        last_shelfmember = self.locate_item(item)
-        last_shelf = last_shelfmember.parent if last_shelfmember else None
-        last_metadata = last_shelfmember.metadata if last_shelfmember else None
-        last_visibility = last_shelfmember.visibility if last_shelfmember else None
-        shelf = self.shelf_list[shelf_type] if shelf_type else None
-        changed = False
-        if last_shelf != shelf:  # change shelf
-            changed = True
-            if last_shelf:
-                last_shelf.remove_item(item)
-            if shelf:
-                new_shelfmember = shelf.append_item(
-                    item, visibility=visibility, metadata=metadata or {}
-                )
-        elif last_shelf is None:
-            raise ValueError("empty shelf")
-        else:
-            new_shelfmember = last_shelfmember
-            if last_shelfmember:
-                if (
-                    metadata is not None and metadata != last_metadata
-                ):  # change metadata
-                    changed = True
-                    last_shelfmember.metadata = metadata
-                    last_shelfmember.visibility = visibility
-                    last_shelfmember.save()
-                elif visibility != last_visibility:  # change visibility
-                    last_shelfmember.visibility = visibility
-                    last_shelfmember.save()
-        if changed:
-            if metadata is None:
-                metadata = last_metadata or {}
-            log_time = (
-                new_shelfmember.created_time
-                if new_shelfmember and new_shelfmember != last_shelfmember
-                else timezone.now()
-            )
-            ShelfLogEntry.objects.create(
-                owner=self.owner,
-                shelf_type=shelf_type,
-                item=item,
-                metadata=metadata,
-                timestamp=log_time,
-            )
-        return new_shelfmember
-
-    def get_log(self):
-        return ShelfLogEntry.objects.filter(owner=self.owner).order_by("timestamp")
-
     def get_log_for_item(self, item: Item):
         return ShelfLogEntry.objects.filter(owner=self.owner, item=item).order_by(
             "timestamp"

journal/tests.py

@@ -56,61 +56,72 @@ def test_shelf(self):
         self.assertIsNotNone(q2)
         self.assertEqual(q1.members.all().count(), 0)
         self.assertEqual(q2.members.all().count(), 0)
-        shelf_manager.move_item(book1, ShelfType.WISHLIST)
-        time.sleep(0.001)
-        shelf_manager.move_item(book2, ShelfType.WISHLIST)
+        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)
         time.sleep(0.001)
         self.assertEqual(q1.members.all().count(), 2)
-        shelf_manager.move_item(book1, ShelfType.PROGRESS)
+        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)
+        self.assertEqual(len(Mark(user.identity, book1).all_post_ids), 2)
         log = shelf_manager.get_log_for_item(book1)
         self.assertEqual(log.count(), 2)
         last_log = log.last()
         self.assertEqual(last_log.metadata if last_log else 42, {})
-        shelf_manager.move_item(book1, ShelfType.PROGRESS, metadata={"progress": 1})
+        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(), 3)
-        last_log = log.last()
-        self.assertEqual(last_log.metadata if last_log else 42, {"progress": 1})
-        shelf_manager.move_item(book1, ShelfType.PROGRESS, metadata={"progress": 1})
-        time.sleep(0.001)
-        log = shelf_manager.get_log_for_item(book1)
-        self.assertEqual(log.count(), 3)
-        last_log = log.last()
-        self.assertEqual(last_log.metadata if last_log else 42, {"progress": 1})
-        shelf_manager.move_item(book1, ShelfType.PROGRESS, metadata={"progress": 10})
-        time.sleep(0.001)
-        log = shelf_manager.get_log_for_item(book1)
-        self.assertEqual(log.count(), 4)
+        self.assertEqual(log.count(), 2)
+        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
+        # last_log = log.last()
+        # self.assertEqual(last_log.metadata if last_log else 42, {"progress": 1})
+        # Mark(user.identity, book1).update(ShelfType.PROGRESS, metadata={"progress": 1})
+        # time.sleep(0.001)
+        # log = shelf_manager.get_log_for_item(book1)
+        # self.assertEqual(log.count(), 3)
+        # last_log = log.last()
+        # self.assertEqual(last_log.metadata if last_log else 42, {"progress": 1})
+        # Mark(user.identity, book1).update(ShelfType.PROGRESS, metadata={"progress": 10})
+        # time.sleep(0.001)
+        # log = shelf_manager.get_log_for_item(book1)
+        # self.assertEqual(log.count(), 4)
+        # last_log = log.last()
+        # self.assertEqual(last_log.metadata if last_log else 42, {"progress": 10})
+        # shelf_manager.move_item(book1, ShelfType.PROGRESS)
+        # time.sleep(0.001)
+        # log = shelf_manager.get_log_for_item(book1)
+        # self.assertEqual(log.count(), 4)
+        # last_log = log.last()
+        # self.assertEqual(last_log.metadata if last_log else 42, {"progress": 10})
+        # shelf_manager.move_item(book1, ShelfType.PROGRESS, metadata={"progress": 90})
+        # time.sleep(0.001)
+        # log = shelf_manager.get_log_for_item(book1)
+        # self.assertEqual(log.count(), 5)
 
-        last_log = log.last()
-        self.assertEqual(last_log.metadata if last_log else 42, {"progress": 10})
-        shelf_manager.move_item(book1, ShelfType.PROGRESS)
-        time.sleep(0.001)
-        log = shelf_manager.get_log_for_item(book1)
-        self.assertEqual(log.count(), 4)
-        last_log = log.last()
-        self.assertEqual(last_log.metadata if last_log else 42, {"progress": 10})
-        shelf_manager.move_item(book1, ShelfType.PROGRESS, metadata={"progress": 90})
-        time.sleep(0.001)
-        log = shelf_manager.get_log_for_item(book1)
-        self.assertEqual(log.count(), 5)
         self.assertEqual(Mark(user.identity, book1).visibility, 0)
-        shelf_manager.move_item(
-            book1, ShelfType.PROGRESS, metadata={"progress": 90}, visibility=1
+        self.assertEqual(len(Mark(user.identity, book1).current_post_ids), 1)
+        Mark(user.identity, book1).update(
+            ShelfType.PROGRESS, metadata={"progress": 90}, visibility=1
         )
+        self.assertEqual(len(Mark(user.identity, book1).current_post_ids), 2)
+        self.assertEqual(len(Mark(user.identity, book1).all_post_ids), 3)
         time.sleep(0.001)
+        Mark(user.identity, book1).update(
+            ShelfType.COMPLETE, metadata={"progress": 100}
+        )
         self.assertEqual(Mark(user.identity, book1).visibility, 1)
-        self.assertEqual(shelf_manager.get_log_for_item(book1).count(), 5)
+        self.assertEqual(shelf_manager.get_log_for_item(book1).count(), 3)
+        self.assertEqual(len(Mark(user.identity, book1).all_post_ids), 4)
 
         # test delete mark ->  one more log
         Mark(user.identity, book1).delete()
-        self.assertEqual(log.count(), 6)
+        self.assertEqual(log.count(), 4)
 
 
 class TagTest(TestCase):

journal/views/mark.py

@@ -32,7 +32,9 @@ def wish(request: AuthedHttpRequest, item_uuid):
     item = get_object_or_404(Item, uid=get_uuid_or_404(item_uuid))
     if not item:
         raise Http404()
-    Mark(request.user.identity, item).wish()
+    mark = Mark(request.user.identity, item)
+    if not mark.shelf_type:
+        mark.update(ShelfType.WISHLIST)
     if request.GET.get("back"):
         return HttpResponseRedirect(request.META.get("HTTP_REFERER", "/"))
     return HttpResponse(_checkmark)