fix(planner): show a place photo again once its missing cache file is fetched back

A proxy image_url whose cached file was gone answered 204, and the self-heal
got the same URL back, so the list avatar kept its broken image until a
reload while the inspector showed the photo. The image is now mounted afresh
after the refetch; a second failure still ends on the category icon.
This commit is contained in:
Maurice
2026-10-14 10:45:09 +02:00
committed by Maurice
parent b3ef13616f
commit 2fe8590c0b
2 changed files with 100 additions and 4 deletions
@@ -1,5 +1,5 @@
import { render, screen, fireEvent, act } from '../../../tests/helpers/render';
import { getCached, isLoading, fetchPhoto, onThumbReady } from '../../services/photoService';
import { getCached, isLoading, fetchPhoto, onThumbReady, onPhotoLoaded } from '../../services/photoService';
// Mock photoService — all functions are no-ops / return null
vi.mock('../../services/photoService', () => ({
@@ -7,6 +7,7 @@ vi.mock('../../services/photoService', () => ({
isLoading: vi.fn(() => false),
fetchPhoto: vi.fn(),
onThumbReady: vi.fn(() => () => {}),
onPhotoLoaded: vi.fn(() => () => {}),
}));
// Mock IntersectionObserver as a class constructor
@@ -34,6 +35,7 @@ beforeEach(() => {
vi.mocked(isLoading).mockReturnValue(false);
vi.mocked(fetchPhoto).mockReset();
vi.mocked(onThumbReady).mockReturnValue(() => {});
vi.mocked(onPhotoLoaded).mockReset().mockReturnValue(() => {});
});
afterEach(() => {
@@ -182,4 +184,79 @@ describe('PlaceAvatar', () => {
render(<PlaceAvatar place={basePlaceWithImage} />);
expect(mockObserve).not.toHaveBeenCalled();
});
/** The callback the self-heal handed to fetchPhoto. */
function photoCallback() {
const callback = vi.mocked(fetchPhoto).mock.calls[0]?.[5];
if (!callback) throw new Error('fetchPhoto was called without a callback');
return callback;
}
// A proxy image_url whose cached file is gone answers 204; the refetch then
// hands back the very same URL once the server has the photo again.
const proxyPlace = {
...basePlaceNoImage,
google_place_id: 'G1',
image_url: '/api/maps/place-photo/G1/bytes',
};
it('FE-COMP-AVATAR-017: loads the image again when the self-heal returns the same proxy URL', () => {
const { container } = render(<PlaceAvatar place={proxyPlace} />);
const before = container.querySelector('img') as HTMLImageElement;
fireEvent.error(before);
expect(vi.mocked(fetchPhoto)).toHaveBeenCalledWith('refetch:G1', 'G1', proxyPlace.lat, proxyPlace.lng, proxyPlace.name, expect.any(Function));
const callback = photoCallback();
act(() => callback({ photoUrl: proxyPlace.image_url, thumbDataUrl: null }));
const after = container.querySelector('img') as HTMLImageElement;
expect(after).not.toBe(before);
expect(after.getAttribute('src')).toBe(proxyPlace.image_url);
});
it('FE-COMP-AVATAR-018: ends on the category icon when the reloaded image fails too', () => {
const { container } = render(<PlaceAvatar place={proxyPlace} />);
fireEvent.error(container.querySelector('img') as HTMLImageElement);
const callback = photoCallback();
act(() => callback({ photoUrl: proxyPlace.image_url, thumbDataUrl: null }));
fireEvent.error(container.querySelector('img') as HTMLImageElement);
expect(container.querySelector('img')).toBeNull();
expect(container.querySelector('svg')).toBeTruthy();
expect(vi.mocked(fetchPhoto)).toHaveBeenCalledTimes(1);
});
it('FE-COMP-AVATAR-019: ends on the category icon when the self-heal finds no photo', () => {
const { container } = render(<PlaceAvatar place={proxyPlace} />);
fireEvent.error(container.querySelector('img') as HTMLImageElement);
const callback = photoCallback();
act(() => callback({ photoUrl: null, thumbDataUrl: null }));
expect(container.querySelector('img')).toBeNull();
});
it('FE-COMP-AVATAR-020: a reused avatar gets the self-heal again for the next place', () => {
const { container, rerender } = render(<PlaceAvatar place={proxyPlace} />);
fireEvent.error(container.querySelector('img') as HTMLImageElement);
const next = { ...proxyPlace, id: 2, google_place_id: 'G2', image_url: '/api/maps/place-photo/G2/bytes' };
rerender(<PlaceAvatar place={next} />);
fireEvent.error(container.querySelector('img') as HTMLImageElement);
expect(vi.mocked(fetchPhoto)).toHaveBeenLastCalledWith('refetch:G2', 'G2', next.lat, next.lng, next.name, expect.any(Function));
});
it('FE-COMP-AVATAR-021: takes the photo of a fetch another view started, even without a thumb', () => {
vi.mocked(isLoading).mockReturnValue(true);
const { container } = render(<PlaceAvatar place={{ ...basePlaceNoImage, google_place_id: 'G3' }} />);
act(() => {
observerInstance?.callback([{ isIntersecting: true }]);
});
expect(vi.mocked(onPhotoLoaded)).toHaveBeenCalledWith('G3', expect.any(Function));
const listener = vi.mocked(onPhotoLoaded).mock.calls[0][1];
act(() => listener({ photoUrl: '/photo/G3.jpg', thumbDataUrl: null }));
expect((container.querySelector('img') as HTMLImageElement).getAttribute('src')).toBe('/photo/G3.jpg');
});
});
+22 -3
View File
@@ -1,6 +1,6 @@
import React, { useState, useEffect, useRef } from 'react'
import { getCategoryIcon } from './categoryIcons'
import { getCached, isLoading, fetchPhoto, onThumbReady } from '../../services/photoService'
import { getCached, isLoading, fetchPhoto, onThumbReady, onPhotoLoaded } from '../../services/photoService'
import { useAuthStore } from '../../store/authStore'
import type { Place } from '../../types'
@@ -17,6 +17,9 @@ interface PlaceAvatarProps {
export default React.memo(function PlaceAvatar({ place, size = 32, category }: PlaceAvatarProps) {
const [photoSrc, setPhotoSrc] = useState<string | null>(place.image_url || null)
// Bumped to remount the <img> when the photo should be loaded again from the
// same URL, which a src change alone cannot do.
const [loadAttempt, setLoadAttempt] = useState(0)
const [visible, setVisible] = useState(false)
const imageUrlFailed = useRef(false)
const ref = useRef<HTMLDivElement>(null)
@@ -39,6 +42,8 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P
}, [place.id])
useEffect(() => {
// A new place or a new picture gets its own self-heal below.
imageUrlFailed.current = false
if (!visible) return
if (place.image_url) { setPhotoSrc(place.image_url); return }
if (!placesPhotosEnabled) return
@@ -57,7 +62,12 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P
}
if (isLoading(cacheKey)) {
return onThumbReady(cacheKey, thumb => setPhotoSrc(thumb))
// Another view started this fetch. Take its photo as soon as it lands,
// and the thumb when that follows; a failed thumb must not leave the
// category icon standing in for a photo that loaded.
const offPhoto = onPhotoLoaded(cacheKey, entry => setPhotoSrc(entry.thumbDataUrl || entry.photoUrl))
const offThumb = onThumbReady(cacheKey, thumb => setPhotoSrc(thumb))
return () => { offPhoto(); offThumb() }
}
fetchPhoto(cacheKey, photoId || `coords:${place.lat}:${place.lng}`, place.lat, place.lng, place.name,
@@ -83,6 +93,7 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P
return (
<div ref={ref} style={containerStyle}>
<img
key={loadAttempt}
src={photoSrc}
alt={place.name}
decoding="async"
@@ -92,8 +103,16 @@ export default React.memo(function PlaceAvatar({ place, size = 32, category }: P
imageUrlFailed.current = true
const photoId = place.google_place_id || place.osm_id!
const cacheKey = `refetch:${photoId}`
// The server answers a missing cache file with an empty 204 and
// fetches the photo again on this call. It hands back the same
// proxy URL, so the <img> has to be remounted to ask once more;
// setting an unchanged src would leave it broken (5.0.0-pre.1
// feedback). A second failure ends on the category icon.
fetchPhoto(cacheKey, photoId, place.lat ?? undefined, place.lng ?? undefined, place.name,
entry => { setPhotoSrc(entry.thumbDataUrl || entry.photoUrl) }
entry => {
setPhotoSrc(entry.thumbDataUrl || entry.photoUrl)
setLoadAttempt(n => n + 1)
}
)
} else {
setPhotoSrc(null)