diff --git a/src/integrations/imports/anilist.py b/src/integrations/imports/anilist.py index 8eba369a7..96bca2903 100644 --- a/src/integrations/imports/anilist.py +++ b/src/integrations/imports/anilist.py @@ -5,6 +5,7 @@ import requests from django.apps import apps from django.conf import settings +from django.db import transaction from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext as _ @@ -103,7 +104,7 @@ def __init__(self, token, user, mode, username): username (str): AniList username to import from token (str): Encrypted access token for private imports (optional) user: Django user object to import data for - mode (str): Import mode ("new" or "overwrite") + mode (str): Import mode ("new", "update" or "overwrite") """ self.username = username self.token = token @@ -114,19 +115,19 @@ def __init__(self, token, user, mode, username): if self.token is not None: self.token = helpers.decrypt(self.token) - # Track existing media for "new" mode + # Track existing media for "new" and "update" mode self.existing_media = helpers.get_existing_media(user) # Track every entry of each media to sync the current one and add the - # missing repeats in "new" mode + # missing repeats in "new" and "update" mode self.existing_entries = {} - if mode == "new": + if mode != "overwrite": self.existing_entries = helpers.get_existing_entries( user, [MediaTypes.ANIME.value, MediaTypes.MANGA.value], ) - # Track existing media changed in "new" mode, by primary key + # Track existing media changed in "new" and "update" mode, by primary key self.to_update = defaultdict(dict) # Track media IDs to delete in overwrite mode @@ -250,13 +251,16 @@ def import_data(self): self._process_media_data(response["data"]["anime"], MediaTypes.ANIME.value) self._process_media_data(response["data"]["manga"], MediaTypes.MANGA.value) - helpers.cleanup_existing_media(self.to_delete, self.user) - helpers.bulk_create_media(self.bulk_media, self.user) - helpers.bulk_update_media( - self.to_update, - helpers.EXISTING_MEDIA_UPDATE_FIELDS, - self.user, - ) + # A finished entry and the repeats added after it are saved together, + # or a retry would find the repeats and leave the entry unfinished + with transaction.atomic(): + helpers.cleanup_existing_media(self.to_delete, self.user) + helpers.bulk_create_media(self.bulk_media, self.user) + helpers.bulk_update_media( + self.to_update, + helpers.EXISTING_MEDIA_UPDATE_FIELDS, + self.user, + ) imported_counts = { media_type: len(media_list) @@ -319,6 +323,12 @@ def _process_entry(self, content, media_type): "chapters", ) + updated_at = ( + timezone.now() + if content["updatedAt"] == 0 + else timezone.datetime.fromtimestamp(content["updatedAt"], tz=UTC) + ) + repeats_count = content["repeat"] if content["status"] == "REPEATING" and repeats_count == 0: repeats_count = 1 @@ -331,6 +341,7 @@ def _process_entry(self, content, media_type): current, max_progress, repeats_count + 1, + updated_at, ) if not missing: return @@ -346,11 +357,6 @@ def _process_entry(self, content, media_type): }, ) model = apps.get_model(app_label="app", model_name=media_type) - updated_at = ( - timezone.now() - if content["updatedAt"] == 0 - else timezone.datetime.fromtimestamp(content["updatedAt"], tz=UTC) - ) # Add completed repeats for _count in range(repeats_count): @@ -373,28 +379,43 @@ def _process_entry(self, content, media_type): self.bulk_media[media_type].append(instance) def _get_existing(self, media_type, source, media_id): - """Get the current entry of media already in Yamtrack, only in "new" mode.""" - if self.mode != "new": + """Get the current entry of media already in Yamtrack, except in overwrite.""" + if self.mode == "overwrite": return None entries = self.existing_entries.get((media_type, source, media_id)) return helpers.get_current_entry(entries) if entries else None - def _sync_existing(self, media_type, media, current, max_progress, entries): + def _sync_existing( + self, + media_type, + media, + current, + max_progress, + entries, + updated_at, + ): """Sync existing media and return how many of its entries are missing.""" - changed, missing = helpers.sync_existing_entries( - media, - current, - max_progress, - entries, - len( - self.existing_entries[ - media_type, - media.item.source, - media.item.media_id, - ], - ), + existing_count = len( + self.existing_entries[media_type, media.item.source, media.item.media_id], ) + if self.mode == "update": + changed, missing = helpers.update_existing_entries( + media, + current, + max_progress, + entries, + existing_count, + updated_at, + ) + else: + changed, missing = helpers.sync_existing_entries( + media, + current, + max_progress, + entries, + existing_count, + ) if changed: self.to_update[media_type][media.pk] = media diff --git a/src/integrations/imports/helpers.py b/src/integrations/imports/helpers.py index 01b330999..94be49252 100644 --- a/src/integrations/imports/helpers.py +++ b/src/integrations/imports/helpers.py @@ -4,6 +4,7 @@ import json import logging from collections import defaultdict +from decimal import Decimal from cryptography.fernet import Fernet from django.apps import apps @@ -300,6 +301,72 @@ def sync_existing_entries(media, current, max_progress, entries, existing_count) return changed, missing +def is_same_value(current, value): + """Return whether a field of existing media already has an imported value.""" + if isinstance(current, Decimal) and value is not None: + return current == Decimal(str(value)) + return current == value + + +def update_existing_media(media, values, updated_at): + """Update existing media to the values of an import, return whether it changed. + + The history record is dated when the source changed the media. When Yamtrack + has a record as new or newer, the update is dated now, as the history shows + each record as a change from the one before it. + """ + changed = { + field: value + for field, value in values.items() + if not is_same_value(getattr(media, field), value) + } + if not changed: + return False + + latest = media.history.first() + if latest and latest.history_date >= updated_at: + updated_at = timezone.now() + + for field, value in changed.items(): + setattr(media, field, value) + if "progress" in changed: + media.progressed_at = updated_at + media._history_date = updated_at + return True + + +def update_existing_entries( + media, + current, + max_progress, + entries, + existing_count, + updated_at, +): + """Update the current entry of existing media from a list import. + + When the source has entries Yamtrack is missing, the current entry becomes a + finished repeat. It keeps its dates, as the source only has the dates of its + newest entry. + + Returns whether the media changed and how many entries are missing. + """ + missing = max(entries - existing_count, 0) + + if not missing: + return update_existing_media(media, current, updated_at), missing + + values = { + "score": current["score"], + "notes": current["notes"], + "status": Status.COMPLETED.value, + } + if max_progress: + values["progress"] = max_progress + + return update_existing_media(media, values, updated_at), missing + + def should_process_media(existing_media, to_delete, media_type, source, media_id, mode): """Determine if a media item should be processed based on mode.""" exists = media_id in existing_media[media_type][source] diff --git a/src/integrations/tasks.py b/src/integrations/tasks.py index 60f977bed..5a6cdbb69 100644 --- a/src/integrations/tasks.py +++ b/src/integrations/tasks.py @@ -25,6 +25,7 @@ logger = logging.getLogger(__name__) ERROR_TITLE = "\n\n\n Couldn't import the following media: \n\n" +UPDATE_MODE_IMPORTERS = {anilist.importer} def format_media_type_display(count, media_type): @@ -70,6 +71,10 @@ def import_media( # Celery tasks run outside a request, so build the warnings, errors and # summary in the user's language instead of LANGUAGE_CODE with translation.override(user.language): + if mode == "update" and importer_func not in UPDATE_MODE_IMPORTERS: + msg = _("Update mode is only available for AniList imports.") + raise helpers.MediaImportError(msg) + with disable_fetch_releases(): if oauth_username is None: imported_counts, warnings = importer_func( diff --git a/src/integrations/tests/imports/test_anilist.py b/src/integrations/tests/imports/test_anilist.py index 27b23b92f..78601abc2 100644 --- a/src/integrations/tests/imports/test_anilist.py +++ b/src/integrations/tests/imports/test_anilist.py @@ -1,5 +1,6 @@ import json from datetime import UTC, date, datetime +from decimal import Decimal from pathlib import Path from unittest.mock import patch @@ -105,8 +106,8 @@ def test_user_not_found(self): ) -class ImportAniListNewMode(TestCase): - """Test that "new" mode only imports what is missing from Yamtrack.""" +class AniListEntryMixin: + """Build AniList list entries and import them.""" def setUp(self): """Create user for the tests.""" @@ -139,16 +140,20 @@ def anime(self, status, progress, score=0, repeat=0, mal_id=1): "notes": None, } - def run_import(self, anime): - """Run an AniList import in "new" mode.""" + def run_import(self, *anime, mode="new"): + """Run an AniList import of the given anime entries.""" response = { "data": { - "anime": {"lists": [{"isCustomList": False, "entries": [anime]}]}, + "anime": {"lists": [{"isCustomList": False, "entries": anime}]}, "manga": {"lists": []}, }, } with patch("app.providers.services.api_request", return_value=response): - return anilist.importer(None, self.user, "new", "test") + return anilist.importer(None, self.user, mode, "test") + + +class ImportAniListNewMode(AniListEntryMixin, TestCase): + """Test that "new" mode only imports what is missing from Yamtrack.""" def test_existing_anime_is_synced(self): """An existing anime gets its progress, status, score and end date.""" @@ -237,3 +242,104 @@ def test_anime_without_mal_id_uses_anilist(self): anime = Anime.objects.get(user=self.user) self.assertEqual(anime.progress, 26) self.assertEqual(anime.status, Status.COMPLETED.value) + + +class ImportAniListUpdateMode(AniListEntryMixin, TestCase): + """Test that "update" mode updates existing media to match AniList.""" + + def test_existing_anime_is_updated_in_place(self): + """AniList values replace the tracked ones, dated by AniList.""" + self.run_import(self.anime("CURRENT", 10, score=8), mode="update") + pk = Anime.objects.get(user=self.user).pk + + paused = self.anime("PAUSED", 4, score=7.3) + paused["notes"] = "On hold" + paused["updatedAt"] = 1700000000 + self.run_import(paused, mode="update") + + anime = Anime.objects.get(user=self.user) + updated_at = datetime(2023, 11, 14, 22, 13, 20, tzinfo=UTC) + self.assertEqual(anime.pk, pk) + self.assertEqual(anime.status, Status.PAUSED.value) + self.assertEqual(anime.progress, 4) + self.assertEqual(anime.score, Decimal("7.3")) + self.assertEqual(anime.notes, "On hold") + self.assertEqual(anime.progressed_at, updated_at) + self.assertEqual(anime.history.first().history_date, updated_at) + + def test_change_older_than_history_is_dated_now(self): + """An AniList change older than the newest history record is dated now.""" + newer = self.anime("CURRENT", 10) + newer["updatedAt"] = 1700000000 + self.run_import(newer, mode="update") + + now = datetime(2024, 1, 1, tzinfo=UTC) + with patch("django.utils.timezone.now", return_value=now): + self.run_import(self.anime("PAUSED", 4), mode="update") + + anime = Anime.objects.get(user=self.user) + self.assertEqual(anime.status, Status.PAUSED.value) + self.assertEqual(anime.progressed_at, now) + self.assertEqual(anime.history.first().history_date, now) + + def test_unchanged_anime_writes_no_history(self): + """Importing the same values again writes no history record.""" + anime = self.anime("COMPLETED", 26, score=7.3) + self.run_import(anime, mode="update") + self.run_import(anime, mode="update") + + history = Anime.objects.get(user=self.user).history + self.assertEqual(history.count(), 1) + + self.run_import(self.anime("COMPLETED", 26, score=9), mode="update") + self.assertEqual(history.count(), 2) + + def test_missing_anime_is_added_without_deleting(self): + """Media missing from Yamtrack is added and media not on AniList is kept.""" + self.run_import(self.anime("CURRENT", 3, mal_id=1), mode="update") + self.run_import(self.anime("PLANNING", 0, mal_id=2), mode="update") + + self.assertEqual( + sorted( + Anime.objects.filter(user=self.user).values_list( + "item__media_id", + "status", + ), + ), + [("1", Status.IN_PROGRESS.value), ("2", Status.PLANNING.value)], + ) + + def test_rewatch_is_added(self): + """A rewatch finishes the existing entry and adds the current one once.""" + self.run_import(self.anime("COMPLETED", 26, score=8), mode="update") + + rewatch = self.anime("REPEATING", 3, score=9) + self.run_import(rewatch, mode="update") + self.run_import(rewatch, mode="update") + + entries = Anime.objects.filter(user=self.user).order_by("created_at") + self.assertEqual( + [(anime.status, anime.progress, anime.score) for anime in entries], + [(Status.COMPLETED.value, 26, 9), (Status.IN_PROGRESS.value, 3, 9)], + ) + self.assertEqual( + [anime.history.count() for anime in entries], + [2, 1], + ) + + def test_failed_rewatch_saves_nothing(self): + """A rewatch is not added when finishing the existing entry fails.""" + self.run_import(self.anime("CURRENT", 10), mode="update") + + with ( + patch.object(helpers, "bulk_update_media", side_effect=RuntimeError), + self.assertRaises(RuntimeError), + ): + self.run_import(self.anime("REPEATING", 3, repeat=1), mode="update") + + self.assertEqual( + list( + Anime.objects.filter(user=self.user).values_list("status", "progress") + ), + [(Status.IN_PROGRESS.value, 10)], + ) diff --git a/src/integrations/tests/test_tasks.py b/src/integrations/tests/test_tasks.py index 52e2c4d41..72524306a 100644 --- a/src/integrations/tests/test_tasks.py +++ b/src/integrations/tests/test_tasks.py @@ -4,6 +4,7 @@ from django.test import TestCase from django.utils import translation +from integrations.imports.helpers import MediaImportError from integrations.tasks import import_media @@ -46,3 +47,32 @@ def importer(_identifier, _user, _mode): self.assertEqual(active_languages, ["it-it"]) self.assertEqual(translation.get_language(), language_before) + + +class ImportMediaModeTests(TestCase): + """Test import tasks only run "update" mode for importers that support it.""" + + def setUp(self): + """Create a user.""" + self.user = get_user_model().objects.create_user( + username="test", + password="12345", # noqa: S106 + ) + + @patch("integrations.tasks.events.tasks.reload_calendar.delay") + def test_update_mode_unsupported(self, _mock_reload_calendar): + """Test an importer without "update" mode runs only in other modes.""" + modes = [] + + def importer(_identifier, _user, mode): + modes.append(mode) + return {}, None + + with self.assertRaisesMessage( + MediaImportError, + "Update mode is only available for AniList imports.", + ): + import_media(importer, "identifier", self.user.id, "update") + import_media(importer, "identifier", self.user.id, "new") + + self.assertEqual(modes, ["new"]) diff --git a/src/templates/users/import_data.html b/src/templates/users/import_data.html index 7c5ed17a3..53bcde738 100644 --- a/src/templates/users/import_data.html +++ b/src/templates/users/import_data.html @@ -54,6 +54,7 @@

{% trans "Import Settings" %}

{% include "app/icons/chevron-down.svg" with classes="absolute right-2 top-1/2 -translate-y-1/2 w-4 h-4 text-gray-400 pointer-events-none" %} diff --git a/src/users/helpers.py b/src/users/helpers.py index 51d99ea3b..e5a0aa076 100644 --- a/src/users/helpers.py +++ b/src/users/helpers.py @@ -54,7 +54,10 @@ def get_next_run_info(periodic_task): except json.JSONDecodeError: mode = "new" - mode = _("Only New Items") if mode == "new" else _("Overwrite Existing") + mode = { + "new": _("Only New Items"), + "update": _("Update Existing"), + }.get(mode, _("Overwrite Existing")) cron = periodic_task.crontab tz = zoneinfo.ZoneInfo(str(cron.timezone))