mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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:
@@ -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 ─────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user