From 9b62883e64271eea3342d8576dfb67248d8e0f0e Mon Sep 17 00:00:00 2001 From: Her Email Date: Mon, 20 Nov 2023 12:13:43 -0500 Subject: [PATCH] further streamline mark.update() --- journal/models/common.py | 3 -- journal/models/mark.py | 80 ++++++++++++++++------------------ journal/models/shelf.py | 93 +++++++++------------------------------- journal/tests.py | 91 ++++++++++++++++++++++----------------- journal/views/mark.py | 4 +- social/tests.py | 12 ++---- takahe/utils.py | 46 ++++++++++---------- 7 files changed, 137 insertions(+), 192 deletions(-) diff --git a/journal/models/common.py b/journal/models/common.py index 5406daea..53b4b5ed 100644 --- a/journal/models/common.py +++ b/journal/models/common.py @@ -194,9 +194,6 @@ class Piece(PolymorphicModel, UserOwnedObjectMixin): 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() diff --git a/journal/models/mark.py b/journal/models/mark.py index 113b934f..893e68f4 100644 --- a/journal/models/mark.py +++ b/journal/models/mark.py @@ -88,7 +88,7 @@ class Mark: 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,41 +148,24 @@ class Mark: @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, @@ -190,6 +173,8 @@ class Mark: """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 @@ class Mark: # 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,24 +215,28 @@ class Mark: 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) @@ -250,7 +244,7 @@ class Mark: 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( diff --git a/journal/models/shelf.py b/journal/models/shelf.py index a9fd8b56..aae96c24 100644 --- a/journal/models/shelf.py +++ b/journal/models/shelf.py @@ -128,24 +128,32 @@ class ShelfMember(ListMember): 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( + # 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 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 @@ class ShelfManager: 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" diff --git a/journal/tests.py b/journal/tests.py index e505206e..dee0988d 100644 --- a/journal/tests.py +++ b/journal/tests.py @@ -56,61 +56,72 @@ class ShelfTest(TestCase): 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, {}) + 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) - last_log = log.last() - self.assertEqual(last_log.metadata if last_log else 42, {}) - shelf_manager.move_item(book1, 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(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): diff --git a/journal/views/mark.py b/journal/views/mark.py index 8b44ff1d..f8ff53ef 100644 --- a/journal/views/mark.py +++ b/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) diff --git a/social/tests.py b/social/tests.py index b881977e..a9e5d100 100644 --- a/social/tests.py +++ b/social/tests.py @@ -30,17 +30,15 @@ class SocialTest(TestCase): self.assertEqual(len(alice_feed.get_timeline()), 0) # 1 activity after adding first book to shelf - self.alice.identity.shelf_manager.move_item( - self.book1, ShelfType.WISHLIST, visibility=1 - ) + Mark(self.alice.identity, self.book1).update(ShelfType.WISHLIST, visibility=1) self.assertEqual(len(alice_feed.get_timeline()), 1) # 2 activities after adding second book to shelf - self.alice.identity.shelf_manager.move_item(self.book2, ShelfType.WISHLIST) + Mark(self.alice.identity, self.book2).update(ShelfType.WISHLIST) self.assertEqual(len(alice_feed.get_timeline()), 2) # 2 activities after change first mark - self.alice.identity.shelf_manager.move_item(self.book1, ShelfType.PROGRESS) + Mark(self.alice.identity, self.book1).update(ShelfType.PROGRESS) self.assertEqual(len(alice_feed.get_timeline()), 2) # bob see 0 activity in timeline in the beginning @@ -60,9 +58,7 @@ class SocialTest(TestCase): self.assertEqual(len(bob_feed.get_timeline()), 2) # alice:3 bob:2 after alice adding second book to shelf as private - self.alice.identity.shelf_manager.move_item( - self.movie, ShelfType.WISHLIST, visibility=2 - ) + Mark(self.alice.identity, self.movie).update(ShelfType.WISHLIST, visibility=2) self.assertEqual(len(alice_feed.get_timeline()), 3) self.assertEqual(len(bob_feed.get_timeline()), 2) diff --git a/takahe/utils.py b/takahe/utils.py index 7af1bece..499c4d9c 100644 --- a/takahe/utils.py +++ b/takahe/utils.py @@ -396,6 +396,19 @@ class Takahe: @staticmethod def delete_posts(post_pks): Post.objects.filter(pk__in=post_pks).update(state="deleted") + # TimelineEvent.objects.filter(subject_post__in=[post.pk]).delete() + PostInteraction.objects.filter(post__in=post_pks).update(state="undone") + + @staticmethod + def visibility_n2t(visibility: int, default_public) -> Visibilities: + if visibility == 1: + return Takahe.Visibilities.followers + elif visibility == 2: + return Takahe.Visibilities.mentioned + elif default_public: + return Takahe.Visibilities.public + else: + return Takahe.Visibilities.unlisted @staticmethod def post_comment(comment, share_as_new_post: bool) -> Post | None: @@ -418,14 +431,9 @@ class Takahe: "relatedWith": [comment.ap_object], } } - if comment.visibility == 1: - v = Takahe.Visibilities.followers - elif comment.visibility == 2: - v = Takahe.Visibilities.mentioned - elif user.preference.mastodon_publish_public: - v = Takahe.Visibilities.public - else: - v = Takahe.Visibilities.unlisted + v = Takahe.visibility_n2t( + comment.visibility, user.preference.mastodon_publish_public + ) existing_post = None if share_as_new_post else comment.latest_post post = Takahe.post( # TODO post as Article? comment.owner.pk, @@ -465,14 +473,9 @@ class Takahe: "relatedWith": [review.ap_object], } } - if review.visibility == 1: - v = Takahe.Visibilities.followers - elif review.visibility == 2: - v = Takahe.Visibilities.mentioned - elif user.preference.mastodon_publish_public: - v = Takahe.Visibilities.public - else: - v = Takahe.Visibilities.unlisted + v = Takahe.visibility_n2t( + review.visibility, user.preference.mastodon_publish_public + ) existing_post = None if share_as_new_post else review.latest_post post = Takahe.post( # TODO post as Article? review.owner.pk, @@ -518,14 +521,9 @@ class Takahe: data["object"]["relatedWith"].append(mark.comment.ap_object) if mark.rating: data["object"]["relatedWith"].append(mark.rating.ap_object) - if mark.visibility == 1: - v = Takahe.Visibilities.followers - elif mark.visibility == 2: - v = Takahe.Visibilities.mentioned - elif user.preference.mastodon_publish_public: - v = Takahe.Visibilities.public - else: - v = Takahe.Visibilities.unlisted + v = Takahe.visibility_n2t( + mark.visibility, user.preference.mastodon_publish_public + ) existing_post = None if share_as_new_post else mark.shelfmember.latest_post post = Takahe.post( mark.owner.pk,