Skip to content

Commit a809f75

Browse files
atomanticatomantic
committed
Improve ancestry refresh and reduce log noise
1 parent 375c0d3 commit a809f75

8 files changed

Lines changed: 239 additions & 107 deletions

File tree

‎client/src/components/person/PersonDetail.tsx‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -405,17 +405,7 @@ export function PersonDetail() {
405405

406406
if (data) {
407407
setAugmentation(data);
408-
// Refresh photo existence checks
409-
const [wikiExists, ancestryExists, wikiTreeExists, linkedInExists] = await Promise.all([
410-
api.hasWikiPhoto(personId).catch(() => ({ exists: false })),
411-
api.hasAncestryPhoto(personId).catch(() => ({ exists: false })),
412-
api.hasWikiTreePhoto(personId).catch(() => ({ exists: false })),
413-
api.hasLinkedInPhoto(personId).catch(() => ({ exists: false })),
414-
]);
415-
setHasWikiPhoto(wikiExists?.exists ?? false);
416-
setHasAncestryPhoto(ancestryExists?.exists ?? false);
417-
setHasWikiTreePhoto(wikiTreeExists?.exists ?? false);
418-
setHasLinkedInPhoto(linkedInExists?.exists ?? false);
408+
await refreshPhotoState();
419409
toast.success(`Photo fetched from ${platform}`);
420410
}
421411

@@ -515,7 +505,7 @@ export function PersonDetail() {
515505
setHasWikiTreePhoto(wikiTreePhotoCheck?.exists ?? false);
516506
setHasLinkedInPhoto(linkedInPhotoCheck?.exists ?? false);
517507
// Increment photo version to bust browser cache
518-
setPhotoVersion(v => v + 1);
508+
setPhotoVersion(Date.now());
519509
}, [personId]);
520510

521511
const handleSavePersonField = useCallback(async (fieldName: string, value: string, originalValue: string | null) => {
@@ -642,21 +632,21 @@ export function PersonDetail() {
642632
const generations = lineage ? lineage.path.length - 1 : 0;
643633
const relationship = isRoot ? 'Root Person (You)' : lineage ? getRelationshipLabel(generations) : null;
644634
// Photo priority: Primary (user-selected) > Ancestry > WikiTree > LinkedIn > Wiki > FamilySearch
645-
// Add cache-busting version parameter to force refresh after changing primary photo
646-
const cacheBust = photoVersion > 0 ? `?v=${photoVersion}` : '';
635+
// Add cache-busting timestamp to force refresh after changing photos
636+
const cacheBuster = photoVersion > 0 ? photoVersion : undefined;
647637
// Primary photo is at api.getPhotoUrl() - check if it exists by seeing if hasPhoto is true
648638
// but we need to distinguish between "has primary" vs "has any photo"
649639
// For now, always show the primary photo URL if hasPhoto is true (getPhotoPath returns primary first)
650640
const photoUrl = hasPhoto
651-
? api.getPhotoUrl(personId!) + cacheBust
641+
? api.getPhotoUrl(personId!, cacheBuster)
652642
: hasAncestryPhoto
653-
? api.getAncestryPhotoUrl(personId!) + cacheBust
643+
? api.getAncestryPhotoUrl(personId!, cacheBuster)
654644
: hasWikiTreePhoto
655-
? api.getWikiTreePhotoUrl(personId!) + cacheBust
645+
? api.getWikiTreePhotoUrl(personId!, cacheBuster)
656646
: hasLinkedInPhoto
657-
? api.getLinkedInPhotoUrl(personId!) + cacheBust
647+
? api.getLinkedInPhotoUrl(personId!, cacheBuster)
658648
: hasWikiPhoto
659-
? api.getWikiPhotoUrl(personId!) + cacheBust
649+
? api.getWikiPhotoUrl(personId!, cacheBuster)
660650
: null;
661651

662652
// Get primary description from augmentation
@@ -1128,6 +1118,7 @@ export function PersonDetail() {
11281118
hasAncestryPhoto={hasAncestryPhoto}
11291119
hasWikiTreePhoto={hasWikiTreePhoto}
11301120
hasLinkedInPhoto={hasLinkedInPhoto}
1121+
photoCacheBuster={cacheBuster}
11311122
onSyncFromFamilySearch={handleSyncFromFamilySearch}
11321123
onScrapePhoto={handleScrape}
11331124
onFetchPhoto={handleFetchPhotoFromPlatform}

‎client/src/components/person/ProviderDataTable.tsx‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ interface ProviderDataTableProps {
2828
hasAncestryPhoto: boolean;
2929
hasWikiTreePhoto: boolean;
3030
hasLinkedInPhoto: boolean;
31+
photoCacheBuster?: number;
3132
onSyncFromFamilySearch: () => Promise<void>;
3233
onScrapePhoto: () => Promise<void>;
3334
onFetchPhoto: (platform: string) => Promise<void>;
@@ -620,6 +621,7 @@ export function ProviderDataTable({
620621
hasAncestryPhoto,
621622
hasWikiTreePhoto,
622623
hasLinkedInPhoto,
624+
photoCacheBuster,
623625
onSyncFromFamilySearch,
624626
onScrapePhoto,
625627
onFetchPhoto,
@@ -843,15 +845,18 @@ export function ProviderDataTable({
843845
return comparison.fields.filter(f => f.providerValues[provider]?.status === 'different').length;
844846
};
845847

846-
// Provider photo URLs
848+
// Provider photo URLs with cache buster to force refresh after downloads
849+
// Use comparison's generatedAt timestamp as the cache buster
850+
const cacheBuster = photoCacheBuster ?? (comparison?.generatedAt ? new Date(comparison.generatedAt).getTime() : undefined);
851+
847852
// SparseTree row always shows the primary (user-selected) photo - this is our source of truth
848-
const sparseTreePhotoUrl = hasPhoto ? api.getPhotoUrl(personId) : null;
853+
const sparseTreePhotoUrl = hasPhoto ? api.getPhotoUrl(personId, cacheBuster) : null;
849854

850-
const fsPhotoUrl = hasFsPhoto ? api.getFsPhotoUrl(personId) : null;
851-
const ancestryPhotoUrl = hasAncestryPhoto ? api.getAncestryPhotoUrl(personId) : null;
852-
const wikiTreePhotoUrl = hasWikiTreePhoto ? api.getWikiTreePhotoUrl(personId) : null;
853-
const wikiPhotoUrl = hasWikiPhoto ? api.getWikiPhotoUrl(personId) : null;
854-
const linkedInPhotoUrl = hasLinkedInPhoto ? api.getLinkedInPhotoUrl(personId) : null;
855+
const fsPhotoUrl = hasFsPhoto ? api.getFsPhotoUrl(personId, cacheBuster) : null;
856+
const ancestryPhotoUrl = hasAncestryPhoto ? api.getAncestryPhotoUrl(personId, cacheBuster) : null;
857+
const wikiTreePhotoUrl = hasWikiTreePhoto ? api.getWikiTreePhotoUrl(personId, cacheBuster) : null;
858+
const wikiPhotoUrl = hasWikiPhoto ? api.getWikiPhotoUrl(personId, cacheBuster) : null;
859+
const linkedInPhotoUrl = hasLinkedInPhoto ? api.getLinkedInPhotoUrl(personId, cacheBuster) : null;
855860

856861
// Check if a provider needs parent discovery
857862
const needsDiscovery = (provider: string): boolean => {

‎client/src/services/api.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,8 @@ export const api = {
280280
hasPhoto: (personId: string) =>
281281
fetchJson<{ exists: boolean }>(`/browser/photos/${personId}/exists`),
282282

283-
getPhotoUrl: (personId: string) => `${BASE_URL}/browser/photos/${personId}`,
283+
getPhotoUrl: (personId: string, cacheBuster?: string | number) =>
284+
`${BASE_URL}/browser/photos/${personId}${cacheBuster ? `?t=${cacheBuster}` : ''}`,
284285

285286
// Augmentation (Wikipedia, custom data)
286287
getAugmentation: (personId: string) =>
@@ -301,7 +302,8 @@ export const api = {
301302
hasWikiPhoto: (personId: string) =>
302303
fetchJson<{ exists: boolean }>(`/augment/${personId}/wiki-photo/exists`),
303304

304-
getWikiPhotoUrl: (personId: string) => `${BASE_URL}/augment/${personId}/wiki-photo`,
305+
getWikiPhotoUrl: (personId: string, cacheBuster?: string | number) =>
306+
`${BASE_URL}/augment/${personId}/wiki-photo${cacheBuster ? `?t=${cacheBuster}` : ''}`,
305307

306308
// Ancestry linking
307309
linkAncestry: (personId: string, url: string) =>
@@ -313,7 +315,8 @@ export const api = {
313315
hasAncestryPhoto: (personId: string) =>
314316
fetchJson<{ exists: boolean }>(`/augment/${personId}/ancestry-photo/exists`),
315317

316-
getAncestryPhotoUrl: (personId: string) => `${BASE_URL}/augment/${personId}/ancestry-photo`,
318+
getAncestryPhotoUrl: (personId: string, cacheBuster?: string | number) =>
319+
`${BASE_URL}/augment/${personId}/ancestry-photo${cacheBuster ? `?t=${cacheBuster}` : ''}`,
317320

318321
// WikiTree linking
319322
linkWikiTree: (personId: string, url: string) =>
@@ -325,13 +328,15 @@ export const api = {
325328
hasWikiTreePhoto: (personId: string) =>
326329
fetchJson<{ exists: boolean }>(`/augment/${personId}/wikitree-photo/exists`),
327330

328-
getWikiTreePhotoUrl: (personId: string) => `${BASE_URL}/augment/${personId}/wikitree-photo`,
331+
getWikiTreePhotoUrl: (personId: string, cacheBuster?: string | number) =>
332+
`${BASE_URL}/augment/${personId}/wikitree-photo${cacheBuster ? `?t=${cacheBuster}` : ''}`,
329333

330334
// FamilySearch photo (provider-specific, not primary)
331335
hasFsPhotoProvider: (personId: string) =>
332336
fetchJson<{ exists: boolean }>(`/augment/${personId}/familysearch-photo/exists`),
333337

334-
getFsPhotoUrl: (personId: string) => `${BASE_URL}/augment/${personId}/familysearch-photo`,
338+
getFsPhotoUrl: (personId: string, cacheBuster?: string | number) =>
339+
`${BASE_URL}/augment/${personId}/familysearch-photo${cacheBuster ? `?t=${cacheBuster}` : ''}`,
335340

336341
// LinkedIn linking
337342
linkLinkedIn: (personId: string, url: string) =>
@@ -343,7 +348,8 @@ export const api = {
343348
hasLinkedInPhoto: (personId: string) =>
344349
fetchJson<{ exists: boolean }>(`/augment/${personId}/linkedin-photo/exists`),
345350

346-
getLinkedInPhotoUrl: (personId: string) => `${BASE_URL}/augment/${personId}/linkedin-photo`,
351+
getLinkedInPhotoUrl: (personId: string, cacheBuster?: string | number) =>
352+
`${BASE_URL}/augment/${personId}/linkedin-photo${cacheBuster ? `?t=${cacheBuster}` : ''}`,
347353

348354
// Fetch photo from linked platform
349355
fetchPhotoFromPlatform: (personId: string, platform: string) =>

‎server/src/middleware/requestLogger.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,21 @@ import { logger } from '../lib/logger.js';
33

44
export const requestLogger = (req: Request, res: Response, next: NextFunction) => {
55
const start = performance.now();
6-
logger.api('http', `${req.method} ${req.path} started`);
6+
const isGet = req.method === 'GET';
7+
if (!isGet) {
8+
logger.api('http', `${req.method} ${req.path} started`);
9+
}
710

811
res.on('finish', () => {
912
const elapsed = performance.now() - start;
1013
const formatted = elapsed >= 1000
1114
? `${(elapsed / 1000).toFixed(1)}s`
1215
: `${Math.round(elapsed)}ms`;
16+
const isSlow = elapsed >= 500;
17+
const shouldLog = !isGet || res.statusCode >= 400 || isSlow;
18+
19+
if (!shouldLog) return;
20+
1321
if (res.statusCode < 400) {
1422
logger.done('http', `${req.method} ${req.path} ${res.statusCode} (${formatted})`);
1523
} else {

‎server/src/services/ancestry-upload.service.ts‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,30 @@ function getAncestryIds(canonicalId: string): { treeId: string; ancestryPersonId
2929
}
3030

3131
/**
32-
* Find the best local photo for a person (same priority as FS upload)
32+
* Find the best local photo for uploading TO Ancestry.
33+
* Prioritizes non-Ancestry photos since we want to upload new content.
34+
* Falls back to the generic photo, then ancestry as last resort (for re-upload).
3335
*/
34-
function findLocalPhoto(canonicalId: string): { path: string; url: string } | null {
36+
function findLocalPhoto(canonicalId: string): { path: string; url: string; isFromAncestry: boolean } | null {
3537
const photosDir = join(DATA_DIR, 'photos');
38+
// Priority: familysearch > wikitree > wiki > generic > ancestry (last resort)
3639
const photoChecks = [
37-
{ suffix: '-ancestry', path: join(photosDir, `${canonicalId}-ancestry.jpg`), url: `/augment/${canonicalId}/ancestry-photo` },
38-
{ suffix: '-ancestry', path: join(photosDir, `${canonicalId}-ancestry.png`), url: `/augment/${canonicalId}/ancestry-photo` },
39-
{ suffix: '-wikitree', path: join(photosDir, `${canonicalId}-wikitree.jpg`), url: `/augment/${canonicalId}/wikitree-photo` },
40-
{ suffix: '-wikitree', path: join(photosDir, `${canonicalId}-wikitree.png`), url: `/augment/${canonicalId}/wikitree-photo` },
41-
{ suffix: '-wiki', path: join(photosDir, `${canonicalId}-wiki.jpg`), url: `/augment/${canonicalId}/wiki-photo` },
42-
{ suffix: '-wiki', path: join(photosDir, `${canonicalId}-wiki.png`), url: `/augment/${canonicalId}/wiki-photo` },
43-
{ suffix: '', path: join(photosDir, `${canonicalId}.jpg`), url: `/browser/photos/${canonicalId}` },
44-
{ suffix: '', path: join(photosDir, `${canonicalId}.png`), url: `/browser/photos/${canonicalId}` },
40+
{ path: join(photosDir, `${canonicalId}-familysearch.jpg`), url: `/augment/${canonicalId}/familysearch-photo`, isFromAncestry: false },
41+
{ path: join(photosDir, `${canonicalId}-familysearch.png`), url: `/augment/${canonicalId}/familysearch-photo`, isFromAncestry: false },
42+
{ path: join(photosDir, `${canonicalId}-wikitree.jpg`), url: `/augment/${canonicalId}/wikitree-photo`, isFromAncestry: false },
43+
{ path: join(photosDir, `${canonicalId}-wikitree.png`), url: `/augment/${canonicalId}/wikitree-photo`, isFromAncestry: false },
44+
{ path: join(photosDir, `${canonicalId}-wiki.jpg`), url: `/augment/${canonicalId}/wiki-photo`, isFromAncestry: false },
45+
{ path: join(photosDir, `${canonicalId}-wiki.png`), url: `/augment/${canonicalId}/wiki-photo`, isFromAncestry: false },
46+
{ path: join(photosDir, `${canonicalId}.jpg`), url: `/browser/photos/${canonicalId}`, isFromAncestry: false },
47+
{ path: join(photosDir, `${canonicalId}.png`), url: `/browser/photos/${canonicalId}`, isFromAncestry: false },
48+
// Ancestry photos last - only for re-upload scenarios
49+
{ path: join(photosDir, `${canonicalId}-ancestry.jpg`), url: `/augment/${canonicalId}/ancestry-photo`, isFromAncestry: true },
50+
{ path: join(photosDir, `${canonicalId}-ancestry.png`), url: `/augment/${canonicalId}/ancestry-photo`, isFromAncestry: true },
4551
];
4652

4753
for (const check of photoChecks) {
4854
if (existsSync(check.path)) {
49-
return { path: check.path, url: check.url };
55+
return { path: check.path, url: check.url, isFromAncestry: check.isFromAncestry };
5056
}
5157
}
5258
return null;
@@ -71,11 +77,11 @@ export const ancestryUploadService = {
7177
const ancestryPngPath = join(DATA_DIR, 'photos', `${canonical}-ancestry.png`);
7278
const ancestryHasPhoto = existsSync(ancestryPhotoPath) || existsSync(ancestryPngPath);
7379

74-
// Photo differs if we have a local photo from a non-Ancestry source
75-
const photoDiffers = localPhoto !== null && (
76-
!ancestryHasPhoto ||
77-
!localPhoto.path.includes('-ancestry')
78-
);
80+
// Photo differs (i.e., we should offer upload) if:
81+
// 1. We have a local photo to upload, AND
82+
// 2. The best photo is NOT from ancestry (meaning we have a better source)
83+
// If the only photo is from ancestry, photoDiffers=false (nothing new to upload)
84+
const photoDiffers = localPhoto !== null && !localPhoto.isFromAncestry;
7985

8086
return {
8187
photo: {

‎server/src/services/augmentation.service.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,8 +1290,6 @@ export const augmentationService = {
12901290
* Fetch and download photo from a linked platform, making it the primary photo
12911291
*/
12921292
async fetchPhotoFromPlatform(personId: string, platform: PlatformType): Promise<PersonAugmentation> {
1293-
logger.photo('augment', `Fetching photo from ${platform} for ${personId}`);
1294-
12951293
const existing = this.getAugmentation(personId);
12961294
if (!existing) {
12971295
throw new Error('No augmentation data found for this person');
@@ -1310,7 +1308,6 @@ export const augmentationService = {
13101308
const existingLocalPath = fs.existsSync(jpgPath) ? jpgPath : fs.existsSync(pngPath) ? pngPath : null;
13111309

13121310
if (existingLocalPath) {
1313-
logger.photo('augment', `Photo from ${platform} already exists locally: ${existingLocalPath}`);
13141311
// Just ensure it's marked as primary and return
13151312
existing.photos.forEach(p => p.isPrimary = false);
13161313
const existingPhoto = existing.photos.find(p => p.source === platform);
@@ -1381,7 +1378,6 @@ export const augmentationService = {
13811378
const cache = JSON.parse(fs.readFileSync(cachePath, 'utf-8'));
13821379
if (cache.scrapedData?.photoUrl) {
13831380
photoUrl = cache.scrapedData.photoUrl;
1384-
logger.photo('augment', `Found Ancestry photo URL in cache`);
13851381
}
13861382
}
13871383
}
@@ -1435,15 +1431,12 @@ export const augmentationService = {
14351431
const photoPath = path.join(PHOTOS_DIR, `${personId}-${suffix}.${ext}`);
14361432

14371433
// Download the photo
1438-
logger.download('augment', `Downloading photo from ${normalizedPhotoUrl}`);
14391434
await downloadImage(normalizedPhotoUrl, photoPath);
14401435

14411436
if (!fs.existsSync(photoPath)) {
14421437
throw new Error(`Failed to download photo from ${platform}`);
14431438
}
14441439

1445-
logger.ok('augment', `Downloaded ${platform} photo to ${photoPath}`);
1446-
14471440
// Update or create photo entry and set as primary
14481441
// First, unset any existing primary
14491442
existing.photos.forEach(p => p.isPrimary = false);

0 commit comments

Comments
 (0)