From 74159af5adcc8dd16f3b194826e2030bdb873ea0 Mon Sep 17 00:00:00 2001 From: Jamie Holding Date: Tue, 28 Jul 2026 14:32:09 +0000 Subject: [PATCH] Fix event-variant filter false-positive: require the share target to exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit isEventVariantAlias dropped any is_event place whose social_sharing_link id differed from its own place_id. That wrongly flagged legit entities whose share link points at a non-existent id — UOR's Epic Universe meets (uor.ueu.show.meet_donkey_kong) link to a phantom uor.ueu.entertainment.meet_donkey_kong the feed never emits, so they were being dropped and marked for deletion on the live site. Now a place is a variant only when the share target is a DIFFERENT place that actually EXISTS in the feed (the canonical to defer to). The check takes the set of real place_ids; it moved from placeToEntity into buildEntityList, which has the full place list. Self-links and links to phantom ids are kept. Verified against the live feed: USH unchanged (Studio Tour language/last-tram variants still dropped, 41 entities resolve); UOR keeps Meet Donkey Kong / Meet Toad / The Cosmos Fountain (were wrongly deleted), still drops Hogwarts Express first/last train. Tests updated to exercise the existence check directly. Co-Authored-By: Claude Opus 4.8 --- src/parks/universal/__tests__/places.test.ts | 61 ++++++++++---------- src/parks/universal/universal.ts | 37 +++++++----- 2 files changed, 54 insertions(+), 44 deletions(-) diff --git a/src/parks/universal/__tests__/places.test.ts b/src/parks/universal/__tests__/places.test.ts index 9c9e485d..756c0a79 100644 --- a/src/parks/universal/__tests__/places.test.ts +++ b/src/parks/universal/__tests__/places.test.ts @@ -2,7 +2,7 @@ * Unit tests for the pure helpers backing the /places migration. */ import {describe, test, expect} from 'vitest'; -import {placeToEntity, parseShowTimes, type UniversalPlace, type UniversalShowListEntry} from '../universal.js'; +import {placeToEntity, isEventVariantAlias, parseShowTimes, type UniversalPlace, type UniversalShowListEntry} from '../universal.js'; const DESTINATION = 'universalresort_orlando'; const TZ = 'America/New_York'; @@ -123,51 +123,50 @@ describe('placeToEntity', () => { expect(placeToEntity(ridePlace, DESTINATION, TZ)?.parentId).toBe('uor.usf'); }); - // Event-flagged variants that alias ANOTHER canonical POI (language / last-tram - // duplicates) are dropped; genuine event entities that link to themselves stay. - const evented = ( - placeId: string, - name: string, - type: 'Ride' | 'Show', - shareTargetId: string, - isEvent = 'true', - ): UniversalPlace => ({ + // isEventVariantAlias: an event-flagged place is a variant ONLY when its share + // link targets a DIFFERENT place that EXISTS in the feed. Self-links and links + // to a non-existent id (sloppy feed data) are kept. + const evented = (placeId: string, shareTargetId: string, isEvent = 'true'): UniversalPlace => ({ place_id: placeId, - name, + name: placeId, resort_area_code: 'ush', - venue_id: 'ush.upper_lot', place_type: { - type, + type: 'Show', attributes: [ {name: 'is_event', value: isEvent}, - { - name: 'social_sharing_link', - value: `https://www.universalstudioshollywood.com/applinks/poi?id=${shareTargetId}`, - }, + {name: 'social_sharing_link', value: `https://x/applinks/poi?id=${shareTargetId}`}, ], }, }); + const feed = (...ids: string[]) => new Set(ids); - test('Studio Tour - Mandarin (event, shares WaterWorld id) → null', () => { - const p = evented('ush.upper_lot.shows.studio_tour_mandarin', 'Studio Tour - Mandarin', 'Show', 'ush.upper_lot.shows.waterworld'); - expect(placeToEntity(p, DESTINATION, TZ)).toBeNull(); + test('variant → true: share target is a different EXISTING place (Studio Tour - Mandarin → WaterWorld)', () => { + const p = evented('ush.upper_lot.shows.studio_tour_mandarin', 'ush.upper_lot.shows.waterworld'); + expect(isEventVariantAlias(p, feed(p.place_id, 'ush.upper_lot.shows.waterworld'))).toBe(true); }); - test('Studio Tour Last Tram (event, shares canonical Studio Tour id) → null', () => { - const p = evented('ush.upper_lot.rides.studio_tour_last_tram', 'Studio Tour Last Tram', 'Ride', 'ush.upper_lot.rides.studio_tour'); - expect(placeToEntity(p, DESTINATION, TZ)).toBeNull(); + test('variant → true: Studio Tour Last Tram and Hogwarts Express First Train (share → canonical that exists)', () => { + const tram = evented('ush.upper_lot.rides.studio_tour_last_tram', 'ush.upper_lot.rides.studio_tour'); + expect(isEventVariantAlias(tram, feed(tram.place_id, 'ush.upper_lot.rides.studio_tour'))).toBe(true); + const train = evented('uor.usf.rides.hogwarts_express_first_train', 'uor.usf.rides.hogwarts_express'); + expect(isEventVariantAlias(train, feed(train.place_id, 'uor.usf.rides.hogwarts_express'))).toBe(true); }); - test('Event entity whose share link points at itself → kept (Bowser Jr. Challenge)', () => { - const p = evented('ush.lower_lot.rides.bowser.jr.challenge', 'Bowser Jr. Challenge', 'Ride', 'ush.lower_lot.rides.bowser.jr.challenge'); - const e = placeToEntity(p, DESTINATION, TZ); - expect(e).not.toBeNull(); - expect(e?.parentId).toBe('ush.ush'); // and reparented off upper_lot + test('kept: event place whose share link points at ITSELF (Bowser Jr. Challenge)', () => { + const p = evented('ush.lower_lot.rides.bowser.jr.challenge', 'ush.lower_lot.rides.bowser.jr.challenge'); + expect(isEventVariantAlias(p, feed(p.place_id))).toBe(false); + }); + + test('kept: event place whose share target does NOT exist in the feed (UOR Meet Donkey Kong false-positive)', () => { + // Only uor.ueu.show.meet_donkey_kong exists; its share link points at a + // phantom uor.ueu.entertainment.meet_donkey_kong. Must NOT be dropped. + const p = evented('uor.ueu.show.meet_donkey_kong', 'uor.ueu.entertainment.meet_donkey_kong'); + expect(isEventVariantAlias(p, feed(p.place_id))).toBe(false); }); - test('Canonical Studio Tour (not an event) → kept', () => { - const p = evented('ush.upper_lot.rides.studio_tour', 'Studio Tour', 'Ride', 'ush.upper_lot.rides.studio_tour', 'false'); - expect(placeToEntity(p, DESTINATION, TZ)).not.toBeNull(); + test('kept: non-event place is never a variant', () => { + const p = evented('ush.upper_lot.rides.studio_tour', 'ush.upper_lot.rides.other', 'false'); + expect(isEventVariantAlias(p, feed(p.place_id, 'ush.upper_lot.rides.other'))).toBe(false); }); test('Shop → null (not in PLACE_TYPE_TO_ENTITY)', () => { diff --git a/src/parks/universal/universal.ts b/src/parks/universal/universal.ts index 9a24d8a7..cdfdd1cb 100644 --- a/src/parks/universal/universal.ts +++ b/src/parks/universal/universal.ts @@ -162,13 +162,20 @@ function attr(place: UniversalPlace, name: string): string | undefined { * e.g. USH's "Studio Tour - Mandarin/Spanish" and "Studio Tour Last Tram", * which are alternate-language / last-departure duplicates of the canonical * Studio Tour (their share links even point at WaterWorld). The feed marks each - * `is_event=true` AND aims its `social_sharing_link?id=` at a DIFFERENT - * canonical place_id. A genuine standalone entity's share link points at itself - * — Bowser Jr. Challenge is also `is_event=true` but links to its own id, so it - * is kept. Dropping these variants keeps the park from filling with per-language - * / operational duplicates of a ride that's already listed. + * `is_event=true` AND aims its `social_sharing_link?id=` at a DIFFERENT place + * that ACTUALLY EXISTS in the feed (the canonical to defer to). + * + * Two things are NOT variants and must be kept: + * - a share link that points at the place's own id (Bowser Jr. Challenge is + * `is_event=true` but links to itself), and + * - a share link that points at an id which does NOT exist in the feed — sloppy + * feed data, not a real alias. UOR's Epic Universe meets (`uor.ueu.show.meet_ + * donkey_kong`) link to a phantom `uor.ueu.entertainment.meet_donkey_kong` + * that the feed never emits; they are legit standalone entities. + * + * `knownPlaceIds` is the set of sanitized place_ids present in the feed. */ -function isEventVariantAlias(place: UniversalPlace): boolean { +export function isEventVariantAlias(place: UniversalPlace, knownPlaceIds: Set): boolean { if (attr(place, 'is_event') !== 'true') return false; const link = attr(place, 'social_sharing_link'); const match = typeof link === 'string' ? link.match(/[?&]id=([^&]+)/) : null; @@ -179,9 +186,10 @@ function isEventVariantAlias(place: UniversalPlace): boolean { } catch { sharedId = match[1]; } - // Compare sanitized ids — the share-link id and place_id are the same raw - // scheme, but normalise defensively (matches how entity ids are formed). - return sanitizeId(sharedId) !== sanitizeId(place.place_id); + const sharedSan = sanitizeId(sharedId); + // A real variant defers to a DIFFERENT canonical that exists. Same-id (self + // link) or a target absent from the feed → keep the entity. + return sharedSan !== sanitizeId(place.place_id) && knownPlaceIds.has(sharedSan); } /** @@ -197,9 +205,6 @@ export function placeToEntity( const entityType = PLACE_TYPE_TO_ENTITY[place.place_type.type]; if (!entityType) return null; - // Drop per-language / operational variants of another POI (see helper). - if (isEventVariantAlias(place)) return null; - const entity: Entity = { id: sanitizeId(place.place_id), name: place.name, @@ -918,8 +923,14 @@ class Universal extends Destination { out.push(park); } - // Non-park entities (rides, shows, restaurants). + // Non-park entities (rides, shows, restaurants). Drop event-flagged variants + // that alias a DIFFERENT existing place (Studio Tour language/last-tram, + // Hogwarts Express first/last train) — checked against the full place set so + // a share link to a non-existent id (sloppy feed data) is NOT treated as a + // variant. + const knownPlaceIds = new Set(places.map((p) => sanitizeId(p.place_id))); for (const place of places) { + if (isEventVariantAlias(place, knownPlaceIds)) continue; const entity = placeToEntity(place, destinationId, this.timezone); if (entity) out.push(entity); }