From ad6640cdb2cdd44269b9a81c28390833fc2891ea Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 10 Aug 2026 11:33:08 +0200 Subject: [PATCH] Fill the gaps Google leaves from the free sources --- server/src/nest/maps/maps.helpers.ts | 12 ++- .../place-enrichment.service.ts | 23 +++++- server/tests/unit/nest/maps.service.test.ts | 18 +++++ .../nest/place-enrichment.description.test.ts | 74 ++++++++++++++++++- 4 files changed, 120 insertions(+), 7 deletions(-) diff --git a/server/src/nest/maps/maps.helpers.ts b/server/src/nest/maps/maps.helpers.ts index 65adf8c00..3b15c0116 100644 --- a/server/src/nest/maps/maps.helpers.ts +++ b/server/src/nest/maps/maps.helpers.ts @@ -373,8 +373,16 @@ export function parseOpeningHours(ohString: string): { .split('-') .map((d) => DAYS.indexOf(d.trim())); if (parts.length === 2 && parts[0] >= 0 && parts[1] >= 0) { - for (let i = parts[0]; i !== (parts[1] + 1) % 7; i = (i + 1) % 7) dayIndices.add(i); - dayIndices.add(parts[1]); + // do/while, not while: a range that wraps all the way round — "Mo-Su", + // or "Tu-Mo", both of which mean every day — starts already satisfying + // the exit condition, so the loop never ran and only the closing day + // was added. "Mo-Su 11:30-23:00" is how a place open daily is usually + // tagged, and it produced exactly one day of hours. + let day = parts[0]; + do { + dayIndices.add(day); + day = (day + 1) % 7; + } while (day !== (parts[1] + 1) % 7); } else if (parts[0] >= 0) { dayIndices.add(parts[0]); } diff --git a/server/src/nest/place-enrichment/place-enrichment.service.ts b/server/src/nest/place-enrichment/place-enrichment.service.ts index d379f5561..4f916679e 100644 --- a/server/src/nest/place-enrichment/place-enrichment.service.ts +++ b/server/src/nest/place-enrichment/place-enrichment.service.ts @@ -213,6 +213,19 @@ export function collectHours(details: Record | null): PlaceHour }; } +/** + * Facts from the maps provider, topped up from OpenStreetMap. + * + * Provider first where both know a thing: it is the source the user searched + * and the one more likely to be current for a business. OSM contributes + * everything the provider has no concept of, which for Google is most of this + * list — cuisine, wheelchair access, outdoor seating, the menu link. + */ +export function mergeFacts(primary: PlaceFact[], extra: PlaceFact[]): PlaceFact[] { + const seen = new Set(primary.map((fact) => fact.kind)); + return [...primary, ...extra.filter((fact) => !seen.has(fact.kind))]; +} + /** The provider's star rating, with its count when there is one. */ export function collectRating(details: Record | null): PlaceRating | null { const value = typeof details?.rating === 'number' ? details.rating : null; @@ -281,12 +294,18 @@ export class PlaceEnrichmentService { // no transport deadline at all. const osmDetails = identity.osmTags ? buildOsmDetails(identity.osmTags, '', '') : null; + // Where Google is silent, the free sources fill in. Google's payload has no + // cuisine, no wheelchair access, no outdoor seating and no menu link — it + // never had — so a Google place used to show a rating and nothing else, + // while the same building in OpenStreetMap carried all of it. Each field is + // taken from whichever source actually has it rather than from one source + // for everything. const result: CachedEnrichment = { photos, description, - facts: collectFacts(details), + facts: mergeFacts(collectFacts(details), collectFacts(osmDetails)), hours: collectHours(details) ?? collectHours(osmDetails), - rating: collectRating(details), + rating: collectRating(details) ?? collectRating(osmDetails), }; this.writeCache(placeId, lang, result); return result; diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index 40097160c..c8bbb6350 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -188,6 +188,24 @@ describe('parseOpeningHours', () => { expect(result.openNow).toBeNull(); }); + it('MAPS-005b: a weekday range that wraps the whole week covers all seven days', () => { + // "Mo-Su" is how a place open daily is usually tagged, and it produced + // exactly ONE day: the loop's exit condition was already true on entry, so + // the body never ran and only the closing day was added. + const daily = parseOpeningHours('Mo-Su 11:30-23:00'); + expect(daily.weekdayDescriptions.filter((l) => !l.endsWith('?'))).toHaveLength(7); + expect(daily.weekdayDescriptions[0]).toBe('Monday: 11:30-23:00'); + expect(daily.weekdayDescriptions[6]).toBe('Sunday: 11:30-23:00'); + + // Same shape starting anywhere else in the week. + expect(parseOpeningHours('Tu-Mo 08:00-20:00').weekdayDescriptions.filter((l) => !l.endsWith('?'))).toHaveLength(7); + + // Ranges that do not wrap are unchanged. + expect(parseOpeningHours('Mo-Fr 09:00-18:00').weekdayDescriptions.filter((l) => !l.endsWith('?'))).toHaveLength(5); + expect(parseOpeningHours('Sa-Su 10:00-14:00').weekdayDescriptions.filter((l) => !l.endsWith('?'))).toHaveLength(2); + expect(parseOpeningHours('Mo 09:00-12:00').weekdayDescriptions.filter((l) => !l.endsWith('?'))).toHaveLength(1); + }); + it('MAPS-006: handles comma-separated days', () => { const result = parseOpeningHours('Mo,We,Fr 08:00-17:00'); expect(result.weekdayDescriptions[0]).toContain('Monday: 08:00-17:00'); diff --git a/server/tests/unit/nest/place-enrichment.description.test.ts b/server/tests/unit/nest/place-enrichment.description.test.ts index ccf71faff..5a9c753f3 100644 --- a/server/tests/unit/nest/place-enrichment.description.test.ts +++ b/server/tests/unit/nest/place-enrichment.description.test.ts @@ -115,7 +115,7 @@ describe('identity resolution', () => { expect(maps.resolveOsmIdentity).not.toHaveBeenCalled(); }); - it('ENRICH-072: keeps OSM tags out of a Google payload', async () => { + it('ENRICH-072: never writes OSM tags into a Google payload, but still shows them', async () => { // collectFacts and the OSM branch of collectDescription both gate on // details.source. Merging the looked-up tags in would either be dead weight // or would make a Google place claim to be an OpenStreetMap one. @@ -132,9 +132,15 @@ describe('identity resolution', () => { const out = await make(maps).enrich(1, GOOGLE_REQ); + // The point is that the provider's own record is never written into. Its + // `source` is what collectFacts and the OSM branch of collectDescription + // gate on, so a Google payload carrying OSM tags would either ignore them + // or start claiming to be an OpenStreetMap place. expect(details).toEqual({ source: 'google', name: 'Hamburg Airport', summary: null }); - // Cuisine is an OSM fact and this is a Google place, so it stays out. - expect(out.facts.some((f) => f.kind === 'cuisine')).toBe(false); + // The tags still reach the reader — through the second object, not by + // mutating the first. Google has no concept of cuisine, so without this a + // Google place showed a rating and nothing else. + expect(out.facts.some((f) => f.kind === 'cuisine')).toBe(true); }); }); @@ -268,3 +274,65 @@ describe('article language choice', () => { expect(out.description).toMatchObject({ source: 'wikipedia' }); }); }); + +/** + * ENRICH-097..100 — topping a Google place up from OpenStreetMap. + * + * Google's payload has no cuisine, no wheelchair access, no outdoor seating and + * no menu link. It never had. So a Google place showed a rating and nothing + * else, while the same building in OSM carried all of it — and the lookup that + * finds its wiki tags brings those along at no extra cost. + */ +describe('filling Google gaps from the free sources', () => { + const OSM_TAGS = { + wikidata: 'Q1', + cuisine: 'pizza;italian', + wheelchair: 'limited', + outdoor_seating: 'yes', + 'opening_hours': 'Mo-Su 11:30-23:00', + }; + + const googlePlaceAlsoInOsm = (over: Partial> = {}) => + mapsStub({ + getMapsKey: vi.fn(() => 'key'), + details: vi.fn(async () => ({ place: { source: 'google', rating: 4.6, rating_count: 4495 } })), + resolveOsmIdentity: vi.fn(async () => ({ tags: OSM_TAGS, osmUrl: null, matchedName: "L'Osteria" })), + ...over, + }); + + it('ENRICH-097: shows the OSM facts a Google place has no concept of', async () => { + const out = await make(googlePlaceAlsoInOsm()).enrich(1, GOOGLE_REQ); + + const kinds = out.facts.map((f) => f.kind); + expect(kinds).toContain('cuisine'); + expect(kinds).toContain('wheelchair'); + expect(kinds).toContain('outdoorSeating'); + // Semicolons and underscores are OSM syntax, not something to show a reader. + expect(out.facts.find((f) => f.kind === 'cuisine')?.value).toBe('pizza, italian'); + }); + + it('ENRICH-098: keeps the rating Google reported', async () => { + const out = await make(googlePlaceAlsoInOsm()).enrich(1, GOOGLE_REQ); + expect(out.rating).toEqual({ value: 4.6, count: 4495 }); + }); + + it('ENRICH-099: takes opening hours from OSM when Google reported none', async () => { + const out = await make(googlePlaceAlsoInOsm()).enrich(1, GOOGLE_REQ); + expect(out.hours?.weekdayDescriptions[0]).toContain('11:30-23:00'); + }); + + it('ENRICH-100: the provider wins where both know the same thing', async () => { + // Google's hours are the ones the user searched against and the ones more + // likely to be current for a business. + const maps = googlePlaceAlsoInOsm({ + details: vi.fn(async () => ({ + place: { source: 'google', opening_hours: ['Monday: 09:00-17:00'], rating: 4.6, rating_count: 10 }, + })), + }); + + const out = await make(maps).enrich(1, GOOGLE_REQ); + + expect(out.hours?.weekdayDescriptions).toEqual(['Monday: 09:00-17:00']); + }); +}) +