diff --git a/twitch/management/commands/better_import_drops.py b/twitch/management/commands/better_import_drops.py index 26c408e..0ec3771 100644 --- a/twitch/management/commands/better_import_drops.py +++ b/twitch/management/commands/better_import_drops.py @@ -36,6 +36,7 @@ from twitch.utils import parse_date if TYPE_CHECKING: from django.core.management.base import CommandParser + from django.db.models import Model from json_repair import JSONReturnType from twitch.schemas import ChannelInfoSchema @@ -544,6 +545,32 @@ class Command(BaseCommand): return valid_responses, broken_dir + def _save_if_changed(self, obj: Model, defaults: dict[str, object]) -> bool: + """Save the model instance only when data actually changed. + + This prevents unnecessary updates and avoids touching fields like + `updated_at` when the imported values are identical. + + Args: + obj: The model instance to potentially update. + defaults: Field values to apply. + + Returns: + True if the object was saved, False if no changes were detected. + """ + changed_fields: list[str] = [] + + for field, new_value in defaults.items(): + if getattr(obj, field, None) != new_value: + setattr(obj, field, new_value) + changed_fields.append(field) + + if not changed_fields: + return False + + obj.save(update_fields=changed_fields) + return True + def _get_or_create_organization(self, org_data: OrganizationSchema) -> Organization: """Get or create an organization. @@ -553,11 +580,13 @@ class Command(BaseCommand): Returns: Organization instance. """ - org_obj, created = Organization.objects.update_or_create( + org_obj, created = Organization.objects.get_or_create( twitch_id=org_data.twitch_id, defaults={"name": org_data.name}, ) - if created: + if not created: + self._save_if_changed(org_obj, {"name": org_data.name}) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Created new organization: {org_data.name}", ) @@ -589,19 +618,23 @@ class Command(BaseCommand): if campaign_org_obj: owner_orgs.add(campaign_org_obj) - game_obj, created = Game.objects.update_or_create( + defaults: dict[str, str] = { + "display_name": game_data.display_name or (game_data.name or ""), + "name": game_data.name or "", + "slug": game_data.slug or "", + "box_art": game_data.box_art_url or "", + } + + game_obj, created = Game.objects.get_or_create( twitch_id=game_data.twitch_id, - defaults={ - "display_name": game_data.display_name or (game_data.name or ""), - "name": game_data.name or "", - "slug": game_data.slug or "", - "box_art": game_data.box_art_url or "", - }, + defaults=defaults, ) # Set owners (ManyToMany) if created or owner_orgs: game_obj.owners.add(*owner_orgs) - if created: + if not created: + self._save_if_changed(game_obj, defaults) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Created new game: {game_data.display_name}", ) @@ -648,11 +681,17 @@ class Command(BaseCommand): # Use name as display_name fallback if displayName is None display_name: str = channel_info.display_name or channel_info.name - channel_obj, created = Channel.objects.update_or_create( + defaults: dict[str, str] = { + "name": channel_info.name, + "display_name": display_name, + } + channel_obj, created = Channel.objects.get_or_create( twitch_id=channel_info.twitch_id, - defaults={"name": channel_info.name, "display_name": display_name}, + defaults=defaults, ) - if created: + if not created: + self._save_if_changed(channel_obj, defaults) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Created new channel: {display_name}", ) @@ -750,11 +789,13 @@ class Command(BaseCommand): "account_link_url": drop_campaign.account_link_url, } - campaign_obj, created = DropCampaign.objects.update_or_create( + campaign_obj, created = DropCampaign.objects.get_or_create( twitch_id=drop_campaign.twitch_id, defaults=defaults, ) - if created: + if not created: + self._save_if_changed(campaign_obj, defaults) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Created new campaign: {drop_campaign.name}", ) @@ -829,11 +870,13 @@ class Command(BaseCommand): if end_at_dt is not None: drop_defaults["end_at"] = end_at_dt - drop_obj, created = TimeBasedDrop.objects.update_or_create( + drop_obj, created = TimeBasedDrop.objects.get_or_create( twitch_id=drop_schema.twitch_id, defaults=drop_defaults, ) - if created: + if not created: + self._save_if_changed(drop_obj, drop_defaults) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Created TimeBasedDrop: {drop_schema.name}", ) @@ -859,11 +902,13 @@ class Command(BaseCommand): if created_at_dt: benefit_defaults["created_at"] = created_at_dt - benefit_obj, created = DropBenefit.objects.update_or_create( + benefit_obj, created = DropBenefit.objects.get_or_create( twitch_id=benefit_schema.twitch_id, defaults=benefit_defaults, ) - if created: + if not created: + self._save_if_changed(benefit_obj, benefit_defaults) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Created DropBenefit: {benefit_schema.name}", ) @@ -888,12 +933,15 @@ class Command(BaseCommand): benefit_schema=benefit_schema, ) - _edge_obj, created = DropBenefitEdge.objects.update_or_create( + defaults = {"entitlement_limit": edge_schema.entitlement_limit} + edge_obj, created = DropBenefitEdge.objects.get_or_create( drop=drop_obj, benefit=benefit_obj, - defaults={"entitlement_limit": edge_schema.entitlement_limit}, + defaults=defaults, ) - if created: + if not created: + self._save_if_changed(edge_obj, defaults) + else: tqdm.write( f"{Fore.GREEN}✓{Style.RESET_ALL} Linked benefit: {benefit_schema.name} → {drop_obj.name}", ) @@ -996,22 +1044,27 @@ class Command(BaseCommand): else "", } - _reward_campaign_obj, created = RewardCampaign.objects.update_or_create( + reward_obj, created = RewardCampaign.objects.get_or_create( twitch_id=reward_campaign.twitch_id, defaults=defaults, ) - action: Literal["Imported new", "Updated"] = ( - "Imported new" if created else "Updated" - ) - display_name = ( - f"{reward_campaign.brand}: {reward_campaign.name}" - if reward_campaign.brand - else reward_campaign.name - ) - tqdm.write( - f"{Fore.GREEN}✓{Style.RESET_ALL} {action} reward campaign: {display_name}", - ) + updated: bool = False + if not created: + updated = self._save_if_changed(reward_obj, defaults) + + if created or updated: + action: Literal["Imported new", "Updated"] = ( + "Imported new" if created else "Updated" + ) + display_name = ( + f"{reward_campaign.brand}: {reward_campaign.name}" + if reward_campaign.brand + else reward_campaign.name + ) + tqdm.write( + f"{Fore.GREEN}✓{Style.RESET_ALL} {action} reward campaign: {display_name}", + ) def handle(self, *args, **options) -> None: # noqa: ARG002 """Main entry point for the command. diff --git a/twitch/tests/test_better_import_drops.py b/twitch/tests/test_better_import_drops.py index 8150346..2938625 100644 --- a/twitch/tests/test_better_import_drops.py +++ b/twitch/tests/test_better_import_drops.py @@ -165,6 +165,81 @@ class ExtractCampaignsTests(TestCase): assert campaign.name == "Test Inventory Campaign" assert campaign.operation_names == ["Inventory"] + def test_import_does_not_update_campaign_when_data_unchanged(self) -> None: + """Ensure repeated imports do not modify the campaign updated_at.""" + command = Command() + + payload: dict[str, object] = { + "data": { + "currentUser": { + "id": "17658559", + "inventory": { + "dropCampaignsInProgress": [ + { + "id": "inventory-campaign-1", + "name": "Test Inventory Campaign", + "description": "Campaign from Inventory operation", + "startAt": "2025-01-01T00:00:00Z", + "endAt": "2025-12-31T23:59:59Z", + "accountLinkURL": "https://example.com/link", + "detailsURL": "https://example.com/details", + "imageURL": "https://example.com/campaign.png", + "status": "ACTIVE", + "self": { + "isAccountConnected": True, + "__typename": "DropCampaignSelfEdge", + }, + "game": { + "id": "inventory-game-1", + "displayName": "Inventory Game", + "boxArtURL": "https://example.com/boxart.png", + "slug": "inventory-game", + "name": "Inventory Game", + "__typename": "Game", + }, + "owner": { + "id": "inventory-org-1", + "name": "Inventory Organization", + "__typename": "Organization", + }, + "timeBasedDrops": [], + "eventBasedDrops": None, + "__typename": "DropCampaign", + }, + ], + "gameEventDrops": None, + "__typename": "Inventory", + }, + "__typename": "User", + }, + }, + "extensions": {"operationName": "Inventory"}, + } + + # First import to create the campaign + success, _ = command.process_responses( + responses=[payload], + file_path=Path("test_inventory.json"), + options={}, + ) + assert success is True + + campaign: DropCampaign = DropCampaign.objects.get( + twitch_id="inventory-campaign-1", + ) + updated_at = campaign.updated_at + + # Second import should not change updated_at since data is identical + success, _ = command.process_responses( + responses=[payload], + file_path=Path("test_inventory.json"), + options={}, + ) + assert success is True + + campaign.refresh_from_db() + assert campaign.updated_at == updated_at + def test_handles_inventory_with_null_campaigns(self) -> None: """Ensure Inventory JSON with null dropCampaignsInProgress is handled correctly.""" command = Command() diff --git a/twitch/tests/test_views.py b/twitch/tests/test_views.py index 8c50e74..c88e555 100644 --- a/twitch/tests/test_views.py +++ b/twitch/tests/test_views.py @@ -1,6 +1,7 @@ import datetime import json from datetime import timedelta +from pathlib import Path from typing import TYPE_CHECKING from typing import Any from typing import Literal @@ -8,6 +9,7 @@ from typing import Literal import pytest from django.core.handlers.wsgi import WSGIRequest from django.core.paginator import Paginator +from django.db.models import Max from django.test import RequestFactory from django.urls import reverse from django.utils import timezone @@ -15,6 +17,7 @@ from django.utils import timezone from kick.models import KickCategory from kick.models import KickDropCampaign from kick.models import KickOrganization +from twitch.management.commands.better_import_drops import Command from twitch.models import Channel from twitch.models import ChatBadge from twitch.models import ChatBadgeSet @@ -22,6 +25,7 @@ from twitch.models import DropBenefit from twitch.models import DropCampaign from twitch.models import Game from twitch.models import Organization +from twitch.models import RewardCampaign from twitch.models import TimeBasedDrop from twitch.views import _build_breadcrumb_schema from twitch.views import _build_pagination_info @@ -1421,6 +1425,113 @@ class TestSitemapView: # Ensure at least one entry includes a lastmod (there are entities created by the fixture) assert "" in content + def test_import_does_not_update_lastmod_on_repeated_imports( + self, + client: Client, + sample_entities: dict[str, Any], + ) -> None: + """Ensure repeated imports do not change sitemap lastmod timestamps.""" + command = Command() + + payload: dict[str, object] = { + "data": { + "currentUser": { + "id": "17658559", + "inventory": { + "dropCampaignsInProgress": [ + { + "id": "inventory-campaign-1", + "name": "Test Inventory Campaign", + "description": "Campaign from Inventory operation", + "startAt": "2025-01-01T00:00:00Z", + "endAt": "2025-12-31T23:59:59Z", + "accountLinkURL": "https://example.com/link", + "detailsURL": "https://example.com/details", + "imageURL": "https://example.com/campaign.png", + "status": "ACTIVE", + "self": { + "isAccountConnected": True, + "__typename": "DropCampaignSelfEdge", + }, + "game": { + "id": "inventory-game-1", + "displayName": "Inventory Game", + "boxArtURL": "https://example.com/boxart.png", + "slug": "inventory-game", + "name": "Inventory Game", + "__typename": "Game", + }, + "owner": { + "id": "inventory-org-1", + "name": "Inventory Organization", + "__typename": "Organization", + }, + "timeBasedDrops": [], + "eventBasedDrops": None, + "__typename": "DropCampaign", + }, + ], + "gameEventDrops": None, + "__typename": "Inventory", + }, + "__typename": "User", + }, + }, + "extensions": {"operationName": "Inventory"}, + } + + def _lastmod_values() -> tuple[ + datetime.datetime | None, + datetime.datetime | None, + ]: + twitch_drops_lastmod = max( + [ + dt + for dt in [ + DropCampaign.objects.aggregate(max=Max("updated_at"))["max"], + RewardCampaign.objects.aggregate(max=Max("updated_at"))["max"], + ] + if dt is not None + ], + default=None, + ) + twitch_others_lastmod = max( + [ + dt + for dt in [ + Game.objects.aggregate(max=Max("updated_at"))["max"], + Organization.objects.aggregate(max=Max("updated_at"))["max"], + ChatBadgeSet.objects.aggregate(max=Max("updated_at"))["max"], + ] + if dt is not None + ], + default=None, + ) + return twitch_drops_lastmod, twitch_others_lastmod + + # Initial import + success, _ = command.process_responses( + responses=[payload], + file_path=Path("test_inventory.json"), + options={}, + ) + assert success is True + + first_drops, first_others = _lastmod_values() + + # Second import should not change lastmod values for related models + success, _ = command.process_responses( + responses=[payload], + file_path=Path("test_inventory.json"), + options={}, + ) + assert success is True + + second_drops, second_others = _lastmod_values() + + assert first_drops == second_drops + assert first_others == second_others + def test_sitemap_contains_static_pages( self, client: Client,