fix(collections): ask the same question at all three call sites

savePlace was not the only caller of findDuplicateCollectionPlace. The bulk copy
already carries google_place_id/google_ftid/osm_id into the row it writes but
asked without them, and the import picker never selected them at all, so a
renamed place was recognised on one path and not on the other two. The picker
answering differently from the import is the drift the method exists to prevent:
a row shown as new would come back refused.

COLLECTIONS-SVC-102 and -103 cover the bulk copy and the picker.
This commit is contained in:
Maurice
2026-08-29 16:04:00 +02:00
committed by Maurice
parent 8a0932df72
commit 78548ca2ba
2 changed files with 47 additions and 2 deletions
@@ -610,8 +610,10 @@ export class CollectionsService {
const rows = this.db.all<{
place_id: number; name: string; address: string | null; lat: number | null; lng: number | null;
category_id: number | null; image_url: string | null; day_number: number | null; date: string | null;
google_place_id: string | null; google_ftid: string | null; osm_id: string | null;
}>(`
SELECT p.id AS place_id, p.name, p.address, p.lat, p.lng, p.category_id, p.image_url,
p.google_place_id, p.google_ftid, p.osm_id,
(SELECT MIN(d.day_number) FROM day_assignments da
JOIN days d ON d.id = da.day_id
WHERE da.place_id = p.id AND d.trip_id = p.trip_id) AS day_number,
@@ -627,7 +629,9 @@ export class CollectionsService {
return {
places: rows.map(r => ({
...r,
already_in_list: this.findDuplicateCollectionPlace(collectionId, { name: r.name, lat: r.lat, lng: r.lng }) != null,
// Asked with the same candidate the save will use, or the picker marks a
// place as new and the save then refuses it as a duplicate.
already_in_list: this.findDuplicateCollectionPlace(collectionId, r) != null,
scheduled: r.day_number != null,
})),
};
@@ -660,7 +664,16 @@ export class CollectionsService {
const name = p.name as string;
const lat = (p.lat as number | null) ?? null;
const lng = (p.lng as number | null) ?? null;
if (!force && this.findDuplicateCollectionPlace(collectionId, { name, lat, lng })) {
// The provider ids go with it: they are already carried into the insert
// below, so leaving them out here would recognise less than the row that
// gets written knows about.
const candidate = {
name, lat, lng,
google_place_id: (p.google_place_id as string | null) ?? null,
google_ftid: (p.google_ftid as string | null) ?? null,
osm_id: (p.osm_id as string | null) ?? null,
};
if (!force && this.findDuplicateCollectionPlace(collectionId, candidate)) {
skipped.push({ id: placeId, name });
continue;
}
@@ -220,6 +220,38 @@ describe('saved places + dedup', () => {
expect(result.duplicate).toBe(true);
expect(result.duplicateOf?.name).toBe('Original Name');
});
it('COLLECTIONS-SVC-102: the bulk import recognises a renamed place by its provider id too', () => {
// savePlace was not the only caller. The bulk copy carries the provider ids
// into the row it writes, so asking without them would recognise less than
// the row it just wrote already knows.
const u = createUser(testDb).user;
const col = svc.createCollection(u.id, { name: 'Rome' });
const trip = createTrip(testDb, u.id);
const place = createPlace(testDb, trip.id, { name: 'Trattoria da Enzo' });
testDb.prepare('UPDATE places SET google_ftid = ? WHERE id = ?').run('0x1:0x2', place.id);
svc.savePlace(u.id, { collection_id: col.id, name: 'Dinner Tuesday', lat: 41.88, lng: 12.47, google_ftid: '0x1:0x2' });
const out = svc.saveFromTripPlaces(u.id, col.id, trip.id, [place.id]);
expect(out.copied).toBe(0);
expect(out.skipped.map(s => s.name)).toEqual(['Trattoria da Enzo']);
});
it('COLLECTIONS-SVC-103: the import picker marks that same place as already saved', () => {
// The dialog and the import have to agree: a row shown as new that the import
// then refuses is the drift this method exists to prevent.
const u = createUser(testDb).user;
const col = svc.createCollection(u.id, { name: 'Rome' });
const trip = createTrip(testDb, u.id);
const place = createPlace(testDb, trip.id, { name: 'Trattoria da Enzo' });
testDb.prepare('UPDATE places SET google_ftid = ? WHERE id = ?').run('0x1:0x2', place.id);
svc.savePlace(u.id, { collection_id: col.id, name: 'Dinner Tuesday', lat: 41.88, lng: 12.47, google_ftid: '0x1:0x2' });
const listed = svc.importablePlaces(u.id, col.id, trip.id).places.find(p => p.place_id === place.id);
expect(listed?.already_in_list).toBe(true);
});
});
// ── save-from-trip provenance + IDOR ─────────────────────────────────────────