From 04458bee122f4104f94fd2bb3a40d0b0cdd655a8 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 30 Oct 2026 21:04:15 +0100 Subject: [PATCH] place-contrib: community hours in the details' own shape, and no origin rows for collection copies Community opening hours laid over OSM details now go through the same parser as OSM's own, so they arrive as one line per day with open-now and the periods, and a community description lands in the details' summary. A place copied from a collection no longer records its values as an import: without an origin row a value is never sent anyway, and a later edit marks only the fields it changes. --- server/src/nest/collections/collections.service.ts | 4 ---- server/src/nest/maps/community-overlay.ts | 13 +++++++++++-- server/src/nest/places/places.module.ts | 2 +- server/tests/helpers/plugin-host.ts | 1 - server/tests/unit/nest/collections.service.test.ts | 6 ------ .../tests/unit/nest/maps.community-overlay.test.ts | 5 +++-- server/tests/unit/nest/maps.service.test.ts | 9 +++------ server/tests/unit/nest/osm.photon.test.ts | 2 +- 8 files changed, 19 insertions(+), 23 deletions(-) diff --git a/server/src/nest/collections/collections.service.ts b/server/src/nest/collections/collections.service.ts index 03e078af6..42bc1412b 100644 --- a/server/src/nest/collections/collections.service.ts +++ b/server/src/nest/collections/collections.service.ts @@ -7,7 +7,6 @@ import { PermissionsService } from '../permissions/permissions.service'; import { RealtimeService } from '../realtime/realtime.service'; import { isUploadedPlaceImage } from '../places/place-image'; import { PlacesService } from '../places/places.service'; -import { PlaceOriginsService } from '../places/place-origins.service'; import { StorageService } from '../storage/storage.service'; import { Collections } from '../../db/entities/Collections.entity'; import { CollectionsRepository, type CollectionPlaceRow } from '../../db/repositories/Collections.repository'; @@ -131,7 +130,6 @@ export class CollectionsService { @InjectRepository(Users) private readonly users: UsersRepository, private readonly placeImport: PlaceImportService, private readonly places: PlacesService, // the journey hook of every place copyToTrip adds - private readonly origins: PlaceOriginsService, ) {} /** @@ -1188,8 +1186,6 @@ export class CollectionsService { created.push({ id: newPlaceId, source_id: s.id }); } }); - // A copied value is the collection's, not typed in this write: recorded as an import, never shared. - if (created.length > 0) await this.origins.recordImported(created.map(({ id }) => id)); const rows = await Promise.all(created.map(({ id }) => this.tripPlaces.findWithTagsAndRatings(id))); // reads only for (const [i, place] of rows.entries()) { if (place) events.emit('place:created', { place }); // as the place routes send it diff --git a/server/src/nest/maps/community-overlay.ts b/server/src/nest/maps/community-overlay.ts index d6f3c47d2..cf85df53e 100644 --- a/server/src/nest/maps/community-overlay.ts +++ b/server/src/nest/maps/community-overlay.ts @@ -1,3 +1,4 @@ +import { buildOsmDetails } from './maps.helpers'; import { trekCommunityByKeys, type TrekCommunityValues } from './trek-places.client'; import { normalizePlaceWebsite } from '@trek/shared'; @@ -26,8 +27,16 @@ export function withCommunityValues( if (website) out.website = website; if (values.contact?.phone) out.phone = values.contact.phone; if (values.contact?.email) out.email = values.contact.email; - if (values.hours?.osm) out.opening_hours = values.hours.osm; - if (values.description?.text) out.description = values.description.text; + if (values.hours?.osm) { + // The same shape OSM's own hours take in the details: one line per day, open now, the periods. + const hours = buildOsmDetails({ opening_hours: values.hours.osm }, '', ''); + if (hours.opening_hours) { + out.opening_hours = hours.opening_hours; + out.open_now = hours.open_now; + out.opening_periods = hours.opening_periods; + } + } + if (values.description?.text) out.summary = values.description.text; if (typeof values.community.version === 'number') out.community_version = values.community.version; return out; } diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index db125ccc7..7e485782c 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -106,7 +106,7 @@ import { MulterModule } from '@nestjs/platform-express'; ], controllers: [PlacesController], providers: [PlacesService, PlacesMcp, PlacesRpc, PlaceOriginsService], - // The origin bookkeeping too: the copy from a collection and the booking import write places as well. + // The origin bookkeeping too: the booking import writes places as well. exports: [PlacesService, PlaceOriginsService], }) export class PlacesModule {} diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 2acbac0db..8f3fef3fe 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -491,7 +491,6 @@ export async function createPluginRpcHostParts( usersRepo, buildPlaceImportService(), places, - placeOriginsStub(), ); // After accommodations: a hotel booking writes the stay's day stop through it. const reservations = new ReservationsService( diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index 4de725bf6..34ff57c63 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -120,9 +120,6 @@ const removeIfUnreferenced = (id: string) => photoCache.removeIfUnreferenced(id) // (part B) finished the conversion: `DatabaseService` is GONE from the constructor // entirely (savePlace onward was its last use) — the trip/place/tag/user // repositories below are Task 2's own additions. -// The origin bookkeeping after a copy; its own suite is place-origins.service.test.ts. -const originsForSpy = { recordImported: vi.fn(async () => undefined) }; - beforeAll(async () => { tripsRepoForSpy = await createTestTripsRepo(testDb); permissionsForSpy = new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)); @@ -137,7 +134,6 @@ beforeAll(async () => { await createTestTagsRepo(testDb), await createTestUsersRepo(testDb), buildPlaceImportService(), placesHooks as never, - originsForSpy as never, ); photoCache = new PlacePhotoCacheService( makeStorageFixture('photos/google/').storage, @@ -508,8 +504,6 @@ describe('copyToTrip', () => { const res = await svc.copyToTrip(u.id, { trip_id: trip.id, place_ids: [p1.id, p2.id] }, copyEvents(trip.id)); expect(res.copied).toBe(1); expect(res.skipped.map((s) => s.name)).toEqual(['Pantheon']); - // What was copied is the collection's, so it is recorded as an import, never as typed. - expect(originsForSpy.recordImported).toHaveBeenLastCalledWith(res.created.map((c) => c.id)); const inserted = (await findRow(await orm(), Places, { trip: trip.id, name: 'Colosseum' }))!; expect(inserted.reservation_status).toBe('none'); // itinerary column took the table default diff --git a/server/tests/unit/nest/maps.community-overlay.test.ts b/server/tests/unit/nest/maps.community-overlay.test.ts index 7368c4c17..7c9f7f2f5 100644 --- a/server/tests/unit/nest/maps.community-overlay.test.ts +++ b/server/tests/unit/nest/maps.community-overlay.test.ts @@ -40,11 +40,12 @@ describe('community values on OSM details', () => { expect(got.website).toMatch(/^https:\/\/www\.skybldg\.co\.jp\/?$/); expect(got).toMatchObject({ phone: '+81 6 6440 3855', - opening_hours: 'Mo-Su 09:30-22:30', - description: 'A tower of two linked blocks.', + summary: 'A tower of two linked blocks.', community_version: 3, name: 'Umeda Sky Building', }); + expect(got.opening_hours).toEqual(expect.arrayContaining(['Monday: 09:30-22:30', 'Sunday: 09:30-22:30'])); + expect(got.opening_periods).toBeTruthy(); expect(got.email).toBeUndefined(); }); diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index 1b4adfc70..03c886554 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -1925,13 +1925,10 @@ describe('getPlaceDetails (fetch stubbed)', () => { expect(place.lat).toBeCloseTo(48.85); expect(place.lng).toBeCloseTo(2.29); expect(place.source).toBe('openstreetmap'); - // Overpass first, then Nominatim, then the community values for the element - // from the TREK index, which answer nothing here and leave the details as they are. + // Overpass, then Nominatim, then the element's community values, of which none are published here. expect(fetchMock).toHaveBeenCalledTimes(3); - const overpassUrl = fetchMock.mock.calls[0][0] as string; - const nominatimUrl = fetchMock.mock.calls[1][0] as string; - expect(overpassUrl).toContain('overpass'); - expect(nominatimUrl).toContain('nominatim'); + expect(fetchMock.mock.calls[0][0] as string).toContain('overpass'); + expect(fetchMock.mock.calls[1][0] as string).toContain('nominatim'); expect(String(fetchMock.mock.calls[2][0])).toContain('/v1/community?keys=osm%3Away%2F5'); }); diff --git a/server/tests/unit/nest/osm.photon.test.ts b/server/tests/unit/nest/osm.photon.test.ts index 3564478bb..b6923398c 100644 --- a/server/tests/unit/nest/osm.photon.test.ts +++ b/server/tests/unit/nest/osm.photon.test.ts @@ -320,7 +320,7 @@ it("OSM-PHOTON-021: an element in TREK's own details layer asks no Overpass, and const details = await osm.elementDetails('way:518071791', 'en'); expect(details).toMatchObject({ website: 'https://example.org', - opening_hours: '24/7', + opening_hours: expect.arrayContaining(['Monday: 00:00-24:00']), phone: '+49 30 1234567', community_version: 2, lat: 52.5163,