mirror of
https://github.com/FuzzyGrim/Yamtrack.git
synced 2026-06-27 22:35:55 +00:00
Refresh Item.image on details page when missing (#1432)
* refresh Item.image on details page when missing The home page reads Item.image from the database, which is only written when the Item is first created. The details page fetches metadata live from the provider, so the two can drift out of sync — most visibly when a TV season has no thumbnail at creation time and never updates after. Add a helper that opportunistically refreshes Item.image whenever a details view has freshly-fetched metadata and the stored image is empty or IMG_NONE. Apply it to the primary item in media_details and season_details, and to tracked items surfaced through related sections (e.g. the seasons listed on a TV details page). Fixes #1365. Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> * address PR review: bulk_update + select_related - enrich_items_with_user_data now collects items needing an image refresh and issues a single Item.objects.bulk_update at the end instead of one save per item. Lookup building was extracted to _build_user_media_lookup to keep complexity in check. - filter_media_prefetch now select_relateds 'item' so accessing current_instance.item in the details views doesn't trigger an extra query. Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> --------- Co-authored-by: David Davis <86290+daviddavis@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -12,7 +12,7 @@ from django.utils import timezone
|
||||
from django.utils.encoding import iri_to_uri
|
||||
from django.utils.http import url_has_allowed_host_and_scheme
|
||||
|
||||
from app.models import BasicMedia, MediaTypes, Status
|
||||
from app.models import BasicMedia, Item, MediaTypes, Status
|
||||
|
||||
YEAR_ONLY_PARTS = 1
|
||||
YEAR_MONTH_PARTS = 2
|
||||
@@ -151,6 +151,21 @@ def is_released_date(air_date, current_date=None):
|
||||
return normalized_air_date <= current_date
|
||||
|
||||
|
||||
def _needs_image_refresh(item, new_image):
|
||||
"""Return True when ``item.image`` should be replaced with ``new_image``."""
|
||||
if item is None or not new_image or new_image == settings.IMG_NONE:
|
||||
return False
|
||||
return not item.image or item.image == settings.IMG_NONE
|
||||
|
||||
|
||||
def refresh_item_image_if_missing(item, new_image):
|
||||
"""Update an Item's stored image when it's missing and a real one is available."""
|
||||
if not _needs_image_refresh(item, new_image):
|
||||
return
|
||||
item.image = new_image
|
||||
item.save(update_fields=["image"])
|
||||
|
||||
|
||||
def enrich_items_with_user_data(request, items, section_name):
|
||||
"""Enrich a list of items with user tracking data."""
|
||||
if not items:
|
||||
@@ -158,9 +173,41 @@ def enrich_items_with_user_data(request, items, section_name):
|
||||
|
||||
# All items are the same media type
|
||||
media_type = items[0]["media_type"]
|
||||
media_lookup = _build_user_media_lookup(request.user, items, media_type)
|
||||
|
||||
# Enrich items with matched media
|
||||
enriched_items = []
|
||||
items_to_refresh = []
|
||||
for item in items:
|
||||
if media_type == MediaTypes.SEASON.value:
|
||||
key = (str(item["media_id"]), item["source"], item.get("season_number"))
|
||||
else:
|
||||
key = (str(item["media_id"]), item["source"])
|
||||
|
||||
media_item = media_lookup.get(key)
|
||||
if _should_skip_completed_recommendation(
|
||||
request.user, section_name, media_item
|
||||
):
|
||||
continue
|
||||
|
||||
if media_item is not None and _needs_image_refresh(
|
||||
media_item.item, item.get("image")
|
||||
):
|
||||
media_item.item.image = item["image"]
|
||||
items_to_refresh.append(media_item.item)
|
||||
|
||||
enriched_items.append({"item": item, "media": media_item})
|
||||
|
||||
if items_to_refresh:
|
||||
Item.objects.bulk_update(items_to_refresh, ["image"])
|
||||
|
||||
return enriched_items
|
||||
|
||||
|
||||
def _build_user_media_lookup(user, items, media_type):
|
||||
"""Fetch the user's media for ``items`` and return a {key: media} lookup."""
|
||||
source = items[0]["source"]
|
||||
|
||||
# Build Q objects for all items
|
||||
q_objects = Q()
|
||||
for item in items:
|
||||
filter_params = {
|
||||
@@ -168,15 +215,12 @@ def enrich_items_with_user_data(request, items, section_name):
|
||||
"item__media_type": media_type,
|
||||
"item__source": source,
|
||||
}
|
||||
|
||||
if media_type == MediaTypes.SEASON.value:
|
||||
filter_params["item__season_number"] = item.get("season_number")
|
||||
|
||||
q_objects |= Q(**filter_params)
|
||||
|
||||
q_objects &= Q(user=request.user)
|
||||
q_objects &= Q(user=user)
|
||||
|
||||
# Bulk fetch all media with prefetch
|
||||
model = apps.get_model(app_label="app", model_name=media_type)
|
||||
media_queryset = model.objects.filter(q_objects).select_related("item")
|
||||
media_queryset = BasicMedia.objects._apply_prefetch_related(
|
||||
@@ -185,37 +229,22 @@ def enrich_items_with_user_data(request, items, section_name):
|
||||
)
|
||||
BasicMedia.objects.annotate_max_progress(media_queryset, media_type)
|
||||
|
||||
# Create a lookup dictionary for fast matching
|
||||
media_lookup = {}
|
||||
for media in media_queryset:
|
||||
if media_type == MediaTypes.SEASON.value:
|
||||
key = (media.item.media_id, media.item.source, media.item.season_number)
|
||||
else:
|
||||
key = (media.item.media_id, media.item.source)
|
||||
|
||||
media_lookup[key] = media
|
||||
|
||||
# Enrich items with matched media
|
||||
enriched_items = []
|
||||
for item in items:
|
||||
if media_type == MediaTypes.SEASON.value:
|
||||
key = (str(item["media_id"]), item["source"], item.get("season_number"))
|
||||
else:
|
||||
key = (str(item["media_id"]), item["source"])
|
||||
return media_lookup
|
||||
|
||||
media_item = media_lookup.get(key)
|
||||
if (
|
||||
request.user.hide_completed_recommendations
|
||||
and section_name == "recommendations"
|
||||
and media_item
|
||||
and media_item.status == Status.COMPLETED.value
|
||||
):
|
||||
continue
|
||||
|
||||
enriched_item = {
|
||||
"item": item,
|
||||
"media": media_item,
|
||||
}
|
||||
enriched_items.append(enriched_item)
|
||||
|
||||
return enriched_items
|
||||
def _should_skip_completed_recommendation(user, section_name, media_item):
|
||||
"""Return True when a completed recommendation should be hidden."""
|
||||
return (
|
||||
user.hide_completed_recommendations
|
||||
and section_name == "recommendations"
|
||||
and media_item is not None
|
||||
and media_item.status == Status.COMPLETED.value
|
||||
)
|
||||
|
||||
@@ -727,7 +727,7 @@ class MediaManager(models.Manager):
|
||||
source,
|
||||
season_number,
|
||||
episode_number,
|
||||
)
|
||||
).select_related("item")
|
||||
queryset = self._apply_prefetch_related(queryset, media_type)
|
||||
self.annotate_max_progress(queryset, media_type)
|
||||
|
||||
|
||||
@@ -1,12 +1,16 @@
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse
|
||||
|
||||
from app.models import (
|
||||
Item,
|
||||
MediaTypes,
|
||||
Movie,
|
||||
Sources,
|
||||
Status,
|
||||
)
|
||||
|
||||
|
||||
@@ -114,3 +118,91 @@ class MediaDetailsViewTests(TestCase):
|
||||
Sources.TMDB.value,
|
||||
[1],
|
||||
)
|
||||
|
||||
@patch("app.providers.services.get_media_metadata")
|
||||
def test_media_details_refreshes_missing_item_image(self, mock_get_metadata):
|
||||
"""Item.image is updated when missing and live metadata has one."""
|
||||
live_image = "http://example.com/fresh.jpg"
|
||||
mock_get_metadata.return_value = {
|
||||
"media_id": "238",
|
||||
"title": "Test Movie",
|
||||
"media_type": MediaTypes.MOVIE.value,
|
||||
"source": Sources.TMDB.value,
|
||||
"image": live_image,
|
||||
"max_progress": 1,
|
||||
"overview": "Test overview",
|
||||
"release_date": "2023-01-01",
|
||||
}
|
||||
|
||||
item = Item.objects.create(
|
||||
media_id="238",
|
||||
source=Sources.TMDB.value,
|
||||
media_type=MediaTypes.MOVIE.value,
|
||||
title="Test Movie",
|
||||
image=settings.IMG_NONE,
|
||||
)
|
||||
Movie.objects.create(
|
||||
item=item,
|
||||
user=self.user,
|
||||
status=Status.IN_PROGRESS.value,
|
||||
)
|
||||
|
||||
response = self.client.get(
|
||||
reverse(
|
||||
"media_details",
|
||||
kwargs={
|
||||
"source": Sources.TMDB.value,
|
||||
"media_type": MediaTypes.MOVIE.value,
|
||||
"media_id": "238",
|
||||
"title": "test-movie",
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 200)
|
||||
item.refresh_from_db()
|
||||
self.assertEqual(item.image, live_image)
|
||||
|
||||
@patch("app.providers.services.get_media_metadata")
|
||||
def test_media_details_keeps_existing_item_image(self, mock_get_metadata):
|
||||
"""Item.image is left alone when already set."""
|
||||
existing_image = "http://example.com/stored.jpg"
|
||||
mock_get_metadata.return_value = {
|
||||
"media_id": "238",
|
||||
"title": "Test Movie",
|
||||
"media_type": MediaTypes.MOVIE.value,
|
||||
"source": Sources.TMDB.value,
|
||||
"image": "http://example.com/fresh.jpg",
|
||||
"max_progress": 1,
|
||||
"overview": "Test overview",
|
||||
"release_date": "2023-01-01",
|
||||
}
|
||||
|
||||
item = Item.objects.create(
|
||||
media_id="238",
|
||||
source=Sources.TMDB.value,
|
||||
media_type=MediaTypes.MOVIE.value,
|
||||
title="Test Movie",
|
||||
image=existing_image,
|
||||
)
|
||||
Movie.objects.create(
|
||||
item=item,
|
||||
user=self.user,
|
||||
status=Status.IN_PROGRESS.value,
|
||||
)
|
||||
|
||||
response = self.client.get(
|
||||
reverse(
|
||||
"media_details",
|
||||
kwargs={
|
||||
"source": Sources.TMDB.value,
|
||||
"media_type": MediaTypes.MOVIE.value,
|
||||
"media_id": "238",
|
||||
"title": "test-movie",
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
self.assertEqual(response.status_code, 200)
|
||||
item.refresh_from_db()
|
||||
self.assertEqual(item.image, existing_image)
|
||||
|
||||
@@ -283,6 +283,11 @@ def media_details(request, source, media_type, media_id, title): # noqa: ARG001
|
||||
)
|
||||
current_instance = user_medias[0] if user_medias else None
|
||||
|
||||
if current_instance is not None:
|
||||
helpers.refresh_item_image_if_missing(
|
||||
current_instance.item, media_metadata.get("image")
|
||||
)
|
||||
|
||||
# Enrich related items with user tracking data
|
||||
if media_metadata.get("related"):
|
||||
for section_name, related_items in media_metadata["related"].items():
|
||||
@@ -333,6 +338,11 @@ def season_details(request, source, media_id, title, season_number): # noqa: AR
|
||||
current_instance = user_medias[0] if user_medias else None
|
||||
episodes_in_db = current_instance.episodes.all() if current_instance else []
|
||||
|
||||
if current_instance is not None:
|
||||
helpers.refresh_item_image_if_missing(
|
||||
current_instance.item, season_metadata.get("image")
|
||||
)
|
||||
|
||||
if source == Sources.MANUAL.value:
|
||||
season_metadata["episodes"] = manual.process_episodes(
|
||||
season_metadata,
|
||||
|
||||
Reference in New Issue
Block a user