From 3031bb844f8f4c6ccbac4e61d052dc99f71c0a62 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 4 Jun 2024 10:12:04 -0400 Subject: [PATCH] delete tags when remove mark --- journal/api.py | 8 +++----- journal/importers/douban.py | 4 +--- journal/importers/letterboxd.py | 5 ++--- journal/models/mark.py | 10 ++++++---- journal/models/tag.py | 16 ++++++++-------- journal/tests.py | 24 ++++++++++++++---------- journal/views/mark.py | 2 +- 7 files changed, 35 insertions(+), 34 deletions(-) diff --git a/journal/api.py b/journal/api.py index 0529d7af..d6a8d340 100644 --- a/journal/api.py +++ b/journal/api.py @@ -103,11 +103,11 @@ def mark_item(request, item_uuid: str, mark: MarkInSchema): if mark.created_time and mark.created_time >= timezone.now(): mark.created_time = None m = Mark(request.user.identity, item) - TagManager.tag_item(item, request.user.identity, mark.tags, mark.visibility) m.update( mark.shelf_type, mark.comment_text, mark.rating_grade, + mark.tags, mark.visibility, created_time=mark.created_time, share_to_mastodon=mark.post_to_fediverse, @@ -122,15 +122,13 @@ def mark_item(request, item_uuid: str, mark: MarkInSchema): ) def delete_mark(request, item_uuid: str): """ - Remove a holding mark about an item for current user. + Remove a holding mark about an item for current user, unlike the web behavior, this does not clean up tags. """ item = Item.get_by_url(item_uuid) if not item: return 404, {"message": "Item not found"} m = Mark(request.user.identity, item) - m.delete() - # skip tag deletion for now to be consistent with web behavior - # TagManager.tag_item(item, request.user, [], 0) + m.delete(keep_tags=True) return 200, {"message": "OK"} diff --git a/journal/importers/douban.py b/journal/importers/douban.py index 62ad783f..5feab816 100644 --- a/journal/importers/douban.py +++ b/journal/importers/douban.py @@ -268,11 +268,9 @@ class DoubanImporter: print("-", end="", flush=True) return 2 mark.update( - shelf_type, comment, rating_grade, self.visibility, created_time=time + shelf_type, comment, rating_grade, tags, self.visibility, created_time=time ) print("+", end="", flush=True) - if tags: - TagManager.tag_item(item, self.user.identity, tags) return 1 def import_review_sheet(self, worksheet, sheet_name): diff --git a/journal/importers/letterboxd.py b/journal/importers/letterboxd.py index 677f3570..c925a576 100644 --- a/journal/importers/letterboxd.py +++ b/journal/importers/letterboxd.py @@ -107,16 +107,15 @@ class LetterboxdImporter(Task): else: title = _("a review of {item_title}").format(item_title=item.title) Review.update_item_review(item, owner, title, text, visibility, dt) + tag_titles = [s.strip() for s in tags.split(",")] if tags else None mark.update( shelf_type, comment_text=comment, rating_grade=rating_grade, + tags=tag_titles, visibility=visibility, created_time=dt, ) - if tags: - tag_titles = [s.strip() for s in tags.split(",")] - TagManager.tag_item(item, owner, tag_titles, visibility) self.progress(1) def progress(self, mark_state: int, url=None): diff --git a/journal/models/mark.py b/journal/models/mark.py index 79f4235f..d3992051 100644 --- a/journal/models/mark.py +++ b/journal/models/mark.py @@ -195,6 +195,7 @@ class Mark: shelf_type: ShelfType | None, comment_text: str | None = None, rating_grade: int | None = None, + tags: list[str] | None = None, visibility: int | None = None, metadata: dict[str, Any] | None = None, created_time: datetime | None = None, @@ -207,7 +208,9 @@ class Mark: 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 + if tags is not None: + self.owner.tag_manager.tag_item(self.item, tags, visibility) + if shelf_type is None: # take item off shelf if self.shelfmember: Takahe.delete_posts(self.shelfmember.all_post_ids) @@ -285,9 +288,8 @@ class Mark: boost_toot_later(self.owner.user, 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) + def delete(self, keep_tags=False): + self.update(None, tags=None if keep_tags else []) def delete_log(self, log_id: int): ShelfLogEntry.objects.filter( diff --git a/journal/models/tag.py b/journal/models/tag.py index 27bdbad8..4d481eca 100644 --- a/journal/models/tag.py +++ b/journal/models/tag.py @@ -91,9 +91,9 @@ class TagManager: return tags @staticmethod - def tag_item( - item: Item, + def tag_item_for_owner( owner: APIdentity, + item: Item, tag_titles: list[str], default_visibility: int = 0, ): @@ -113,6 +113,9 @@ class TagManager: if tag: tag.remove_item(item) + def tag_item(self, item: Item, tag_titles: list[str], default_visibility: int = 0): + TagManager.tag_item_for_owner(self.owner, item, tag_titles, default_visibility) + @staticmethod def get_manager_for_user(owner): return TagManager(owner) @@ -144,10 +147,7 @@ class TagManager: def get_item_tags(self, item: Item): return sorted( - [ - m["parent__title"] - for m in TagMember.objects.filter( - parent__owner=self.owner, item=item - ).values("parent__title") - ] + TagMember.objects.filter(parent__owner=self.owner, item=item).values_list( + "parent__title", flat=True + ) ) diff --git a/journal/tests.py b/journal/tests.py index 4b3f0d24..c436eb3f 100644 --- a/journal/tests.py +++ b/journal/tests.py @@ -119,7 +119,7 @@ class ShelfTest(TestCase): 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} + ShelfType.COMPLETE, metadata={"progress": 100}, tags=["best"] ) self.assertEqual(Mark(user.identity, book1).visibility, 1) self.assertEqual(shelf_manager.get_log_for_item(book1).count(), 3) @@ -128,6 +128,9 @@ class ShelfTest(TestCase): # test delete mark -> one more log Mark(user.identity, book1).delete() self.assertEqual(log.count(), 4) + deleted_mark = Mark(user.identity, book1) + self.assertEqual(deleted_mark.shelf_type, None) + self.assertEqual(deleted_mark.tags, []) class TagTest(TestCase): @@ -140,16 +143,17 @@ class TagTest(TestCase): self.user1 = User.register(email="a@b.com", username="user") self.user2 = User.register(email="x@b.com", username="user2") self.user3 = User.register(email="y@b.com", username="user3") - pass def test_user_tag(self): t1 = "tag 1" t2 = "tag 2" t3 = "tag 3" - TagManager.tag_item(self.book1, self.user2.identity, [t1, t3]) + TagManager.tag_item_for_owner(self.user2.identity, self.book1, [t1, t3]) self.assertEqual(self.book1.tags, [t1, t3]) - TagManager.tag_item(self.book1, self.user2.identity, [t2, t3]) + TagManager.tag_item_for_owner(self.user2.identity, self.book1, [t2, t3]) self.assertEqual(self.book1.tags, [t2, t3]) + m = Mark(self.user2.identity, self.book1) + self.assertEqual(m.tags, [t2, t3]) class MarkTest(TestCase): @@ -171,7 +175,7 @@ class MarkTest(TestCase): self.assertEqual(mark.visibility, 2) self.assertEqual(mark.review, None) self.assertEqual(mark.tags, []) - mark.update(ShelfType.WISHLIST, "a gentle comment", 9, 1) + mark.update(ShelfType.WISHLIST, "a gentle comment", 9, None, 1) mark = Mark(self.user1.identity, self.book1) self.assertEqual(mark.shelf_type, ShelfType.WISHLIST) @@ -193,7 +197,9 @@ class MarkTest(TestCase): self.assertIsNone(mark.review) def test_tag(self): - TagManager.tag_item(self.book1, self.user1.identity, [" Sci-Fi ", " fic "]) + TagManager.tag_item_for_owner( + self.user1.identity, self.book1, [" Sci-Fi ", " fic "] + ) mark = Mark(self.user1.identity, self.book1) self.assertEqual(mark.tags, ["Sci-Fi", "fic"]) @@ -208,9 +214,8 @@ class DebrisTest(TestCase): self.user1 = User.register(email="test@test", username="test") def test_journal_migration(self): - TagManager.tag_item(self.book1, self.user1.identity, ["Sci-Fi", "fic"]) mark = Mark(self.user1.identity, self.book1) - mark.update(ShelfType.WISHLIST, "a gentle comment", 9, 1) + mark.update(ShelfType.WISHLIST, "a gentle comment", 9, ["Sci-Fi", "fic"], 1) Review.update_item_review(self.book1, self.user1.identity, "Critic", "Review") collection = Collection.objects.create(title="test", owner=self.user1.identity) collection.append_item(self.book1) @@ -219,9 +224,8 @@ class DebrisTest(TestCase): cnt = Debris.objects.all().count() self.assertEqual(cnt, 0) - TagManager.tag_item(self.book3, self.user1.identity, ["Sci-Fi", "fic"]) mark = Mark(self.user1.identity, self.book3) - mark.update(ShelfType.WISHLIST, "a gentle comment", 9, 1) + mark.update(ShelfType.WISHLIST, "a gentle comment", 9, ["Sci-Fi", "fic"], 1) Review.update_item_review(self.book3, self.user1.identity, "Critic", "Review") collection.append_item(self.book3) self.book3.merge_to(self.book2) diff --git a/journal/views/mark.py b/journal/views/mark.py index 0d19c3cf..c9e2cfac 100644 --- a/journal/views/mark.py +++ b/journal/views/mark.py @@ -91,12 +91,12 @@ def mark(request: AuthedHttpRequest, item_uuid): ) if mark_date and mark_date >= timezone.now(): mark_date = None - TagManager.tag_item(item, request.user.identity, tags, visibility) try: mark.update( status, text, rating_grade, + tags, visibility, share_to_mastodon=share_to_mastodon, created_time=mark_date,