diff --git a/bookmarks/services/bookmarks.py b/bookmarks/services/bookmarks.py index 2f052e87..0ba54440 100644 --- a/bookmarks/services/bookmarks.py +++ b/bookmarks/services/bookmarks.py @@ -67,9 +67,9 @@ def archive_bookmark(bookmark: Bookmark): def archive_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.update(is_archived=True, date_modified=timezone.now()) + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).update(is_archived=True, + date_modified=timezone.now()) def unarchive_bookmark(bookmark: Bookmark): @@ -81,70 +81,76 @@ def unarchive_bookmark(bookmark: Bookmark): def unarchive_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.update(is_archived=False, date_modified=timezone.now()) + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).update(is_archived=False, + date_modified=timezone.now()) def delete_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.delete() + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).delete() def tag_bookmarks(bookmark_ids: [Union[int, str]], tag_string: str, current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) + owned_bookmark_ids = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).values_list('id', + flat=True) tag_names = parse_tag_string(tag_string) tags = get_or_create_tags(tag_names, current_user) - for bookmark in bookmarks: - bookmark.tags.add(*tags) - bookmark.date_modified = timezone.now() + BookmarkToTagRelationShip = Bookmark.tags.through + relationships = [] + for tag in tags: + for bookmark_id in owned_bookmark_ids: + relationships.append(BookmarkToTagRelationShip(bookmark_id=bookmark_id, tag=tag)) - Bookmark.objects.bulk_update(bookmarks, ['date_modified']) + # Insert all bookmark -> tag associations at once, should ignore errors if association already exists + BookmarkToTagRelationShip.objects.bulk_create(relationships, ignore_conflicts=True) + Bookmark.objects.filter(id__in=owned_bookmark_ids).update(date_modified=timezone.now()) def untag_bookmarks(bookmark_ids: [Union[int, str]], tag_string: str, current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) + owned_bookmark_ids = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).values_list('id', + flat=True) tag_names = parse_tag_string(tag_string) tags = get_or_create_tags(tag_names, current_user) - for bookmark in bookmarks: - bookmark.tags.remove(*tags) - bookmark.date_modified = timezone.now() + BookmarkToTagRelationShip = Bookmark.tags.through + for tag in tags: + # Remove all bookmark -> tag associations for the owned bookmarks and the current tag + BookmarkToTagRelationShip.objects.filter(bookmark_id__in=owned_bookmark_ids, tag=tag).delete() - Bookmark.objects.bulk_update(bookmarks, ['date_modified']) + Bookmark.objects.filter(id__in=owned_bookmark_ids).update(date_modified=timezone.now()) def mark_bookmarks_as_read(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.update(unread=False, date_modified=timezone.now()) + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).update(unread=False, + date_modified=timezone.now()) def mark_bookmarks_as_unread(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.update(unread=True, date_modified=timezone.now()) + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).update(unread=True, + date_modified=timezone.now()) def share_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.update(shared=True, date_modified=timezone.now()) + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).update(shared=True, + date_modified=timezone.now()) def unshare_bookmarks(bookmark_ids: [Union[int, str]], current_user: User): sanitized_bookmark_ids = _sanitize_id_list(bookmark_ids) - bookmarks = Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids) - bookmarks.update(shared=False, date_modified=timezone.now()) + Bookmark.objects.filter(owner=current_user, id__in=sanitized_bookmark_ids).update(shared=False, + date_modified=timezone.now()) def _merge_bookmark_data(from_bookmark: Bookmark, to_bookmark: Bookmark): diff --git a/bookmarks/tests/test_bookmarks_service.py b/bookmarks/tests/test_bookmarks_service.py index 2ce5b3dd..96b3675a 100644 --- a/bookmarks/tests/test_bookmarks_service.py +++ b/bookmarks/tests/test_bookmarks_service.py @@ -336,6 +336,28 @@ def test_tag_bookmarks_should_create_tags(self): self.assertCountEqual(bookmark2.tags.all(), [tag1, tag2]) self.assertCountEqual(bookmark3.tags.all(), [tag1, tag2]) + def test_tag_bookmarks_should_handle_existing_relationships(self): + tag1 = self.setup_tag() + tag2 = self.setup_tag() + bookmark1 = self.setup_bookmark(tags=[tag1]) + bookmark2 = self.setup_bookmark(tags=[tag1]) + bookmark3 = self.setup_bookmark(tags=[tag1]) + + BookmarkToTagRelationShip = Bookmark.tags.through + self.assertEqual(3, BookmarkToTagRelationShip.objects.count()) + + tag_bookmarks([bookmark1.id, bookmark2.id, bookmark3.id], f'{tag1.name},{tag2.name}', + self.get_or_create_test_user()) + + bookmark1.refresh_from_db() + bookmark2.refresh_from_db() + bookmark3.refresh_from_db() + + self.assertCountEqual(bookmark1.tags.all(), [tag1, tag2]) + self.assertCountEqual(bookmark2.tags.all(), [tag1, tag2]) + self.assertCountEqual(bookmark3.tags.all(), [tag1, tag2]) + self.assertEqual(6, BookmarkToTagRelationShip.objects.count()) + def test_tag_bookmarks_should_only_tag_specified_bookmarks(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark()