feat(collections): import a file into a list that already exists

The import dialog always made a new list. It now asks where the places go:
a new list, as before and still the default, or a list that is already there.
The picker offers the lists the person may edit, which is their own plus a
shared one where they are an editor or an admin, and starts on the list they
have open.

Adding to a list only ever adds. A place the list already holds is left
exactly as it is, with its status, notes, rating and labels, and counted as
already there in what the import reports; it is recognised the way a single
save recognises one, by provider id, then name, then position. The list keeps
its own name, colour, icon and description whatever the file says, a label
the file brings is created only when the list has none of that name, and new
places are appended after the ones already there so a manual order survives.

The route is POST /api/addons/collections/:id/import, 200 because it adds to
a list rather than making one, and both imports now write their places through
the same helper.
This commit is contained in:
Maurice
2026-09-19 23:12:25 +02:00
parent 7cfec6452d
commit d803efd39d
36 changed files with 978 additions and 81 deletions
@@ -52,6 +52,7 @@ import {
CollectionLabelUpdateDto,
CollectionLabelAssignDto,
CollectionImportDto,
CollectionImportIntoDto,
CollectionGpxReadDto,
} from './collections.dto';
import { PlaceRatingDto } from '../places/places.dto';
@@ -423,6 +424,24 @@ export class CollectionsController {
return this.collections.exportCollectionGpx(user.id, Number(id));
}
/**
* The same file read into a list that already exists (#2301 follow-up).
*
* 200, unlike POST /import above: this one adds to a list rather than making
* one. Anyone who may add a place to the list may do it, and the socket id
* rides along so the browser that asked does not echo its own change.
*/
@Post(':id/import')
@HttpCode(200)
importIntoCollection(
@CurrentUser() user: User,
@Param('id') id: string,
@Body() body: CollectionImportIntoDto,
@Headers('x-socket-id') socketId?: string,
) {
return this.collections.importIntoCollection(user.id, Number(id), body, socketId);
}
@Get(':id/importable/:tripId')
importable(@CurrentUser() user: User, @Param('id') id: string, @Param('tripId') tripId: string) {
return this.collections.importablePlaces(user.id, Number(id), Number(tripId));
@@ -11,6 +11,7 @@ import {
collectionSetStatusRequestSchema,
collectionSetStatusManyRequestSchema,
collectionSetStatusFromTripRequestSchema,
collectionImportIntoRequestSchema,
collectionCopyToTripRequestSchema,
collectionInviteRequestSchema,
collectionInviteActionRequestSchema,
@@ -42,6 +43,7 @@ export class CollectionPlaceUpdateDto extends createZodDto(collectionPlaceUpdate
export class CollectionSetStatusDto extends createZodDto(collectionSetStatusRequestSchema) {}
export class CollectionSetStatusManyDto extends createZodDto(collectionSetStatusManyRequestSchema) {}
export class CollectionSetStatusFromTripDto extends createZodDto(collectionSetStatusFromTripRequestSchema) {}
export class CollectionImportIntoDto extends createZodDto(collectionImportIntoRequestSchema) {}
export class CollectionCopyToTripDto extends createZodDto(collectionCopyToTripRequestSchema) {}
export class CollectionInviteDto extends createZodDto(collectionInviteRequestSchema) {}
export class CollectionInviteActionDto extends createZodDto(collectionInviteActionRequestSchema) {}
@@ -21,7 +21,9 @@ import {
type PlaceMatchCandidate,
type CollectionFileLabel,
type CollectionFilePlace,
type CollectionFile,
type CollectionImportRequest,
type CollectionImportIntoRequest,
type CollectionImportResult,
type CollectionGpxExport,
type CollectionGpxReadRequest,
@@ -444,28 +446,20 @@ export class CollectionsService {
/**
* Read a file back as a new list of the caller's own.
*
* Always a new list. Importing into an existing one would mean deciding what
* happens to a place that is already there, and every answer to that is a
* merge somebody did not ask for; a fresh list is a thing they can look at
* and then move places out of, which the list UI already does well.
*
* The places go in through the same INSERT the rest of the service uses,
* inside one transaction, so a file that fails halfway leaves nothing
* behind. Each place is re-validated against the file contract on the way
* in — the body was validated once at the pipe, and this is the second pass
* that lets one bad row be dropped instead of failing the whole import.
*
* A new list is empty, so every place in the file is written and the answer
* carries no duplicate count. Adding to a list that already exists is
* importIntoCollection below.
*/
importCollection(userId: number, body: CollectionImportRequest): CollectionImportResult {
const file = body.file;
const name = (body.name ?? file.name).trim().slice(0, 120) || file.name;
// The palette is instance-wide and read-only here: a file names a category,
// it does not get to create one.
const categoryIdByName = new Map<string, number>();
for (const c of this.db.all<{ id: number; name: string }>('SELECT id, name FROM categories')) {
categoryIdByName.set(c.name.trim().toLowerCase(), c.id);
}
const result = this.db.transaction(() => {
const collection = this.createCollection(userId, {
name,
@@ -473,46 +467,9 @@ export class CollectionsService {
color: file.color ?? undefined,
icon: file.icon ?? undefined,
});
const labelIdByName = new Map<string, number>();
for (const label of (file.labels ?? []).slice(0, MAX_COLLECTION_FILE_LABELS)) {
const key = label.name.trim().toLowerCase();
if (!key || labelIdByName.has(key)) continue;
labelIdByName.set(key, this.insertImportedLabel(collection.id, label, labelIdByName.size));
}
const insertPlace = this.db.prepare(`
INSERT INTO collection_places (
collection_id, owner_id, saved_by, name, description, lat, lng, address,
category_id, price, currency, notes, image_url, google_place_id, google_ftid,
osm_id, website, phone, status, links, sort_order
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`);
const assignLabel = this.db.prepare('INSERT OR IGNORE INTO collection_place_labels (collection_place_id, label_id) VALUES (?, ?)');
let imported = 0;
let skipped = 0;
for (const raw of file.places) {
const parsed = collectionFilePlaceSchema.safeParse(raw);
if (!parsed.success) { skipped += 1; continue; }
const place = parsed.data;
const res = insertPlace.run(
collection.id, userId, userId,
place.name, place.description ?? null, place.lat ?? null, place.lng ?? null, place.address ?? null,
place.category ? (categoryIdByName.get(place.category.trim().toLowerCase()) ?? null) : null,
place.price ?? null, place.currency ?? null, place.notes ?? null,
place.image_url ?? null, place.google_place_id ?? null, place.google_ftid ?? null,
place.osm_id ?? null, place.website ?? null, place.phone ?? null,
place.status ?? 'idea', serializeLinks(place.links), imported,
);
const placeId = Number(res.lastInsertRowid);
for (const labelName of place.labels ?? []) {
const labelId = labelIdByName.get(labelName.trim().toLowerCase());
if (labelId) assignLabel.run(placeId, labelId);
}
imported += 1;
}
return { collectionId: collection.id, imported, skipped };
const labels = this.labelIdsForFile(collection.id, file.labels);
const counts = this.writeFilePlaces(collection.id, userId, file, labels.byName, { skipDuplicates: false });
return { collectionId: collection.id, ...counts };
});
const collection = this.getCollectionRow(result.collectionId);
@@ -523,11 +480,155 @@ export class CollectionsService {
};
}
/**
* Read the same file into a list that already exists (#2301 follow-up).
*
* Only ever adds. A place the list already holds, by the rule a single save
* uses (provider id, then name, then position), is counted and left exactly
* as it is: its status, notes, rating and labels belong to the people on the
* list, and a file is not a reason to overwrite them. The list keeps its own
* name, colour, icon and description too; what the file says about those is
* about the list it came from.
*
* Editing rights, not ownership: whoever may add a place here may add a file
* of them, and everyone on the list sees the result at once.
*/
importIntoCollection(
userId: number, id: number, body: CollectionImportIntoRequest, socketId?: string,
): CollectionImportResult {
this.assertCanEdit(userId, id);
const file = body.file;
const result = this.db.transaction(() => {
const labels = this.labelIdsForFile(id, file.labels);
const counts = this.writeFilePlaces(id, userId, file, labels.byName, { skipDuplicates: true });
return { ...counts, labelsCreated: labels.created };
});
// A file whose places were all already there can still have brought a label
// with it, and that is a change the other members should see.
if (result.imported > 0 || result.labelsCreated > 0) {
this.notifyCollectionUsers(id, socketId, 'collections:updated');
}
const collection = this.getCollectionRow(id);
return {
collection: { ...collection, is_owner: collection.owner_id === userId, labels: this.loadLabelsByCollection(id) },
imported: result.imported,
skipped: result.skipped,
duplicates: result.duplicates,
};
}
/**
* The file's labels as ids in this list: the ones it already has, matched by
* name, plus the ones it does not, created in the file's order. A file never
* renames or recolours a label that is already there.
*/
private labelIdsForFile(
collectionId: number, labels: CollectionFileLabel[] | undefined,
): { byName: Map<string, number>; created: number } {
const byName = new Map<string, number>();
for (const row of this.db.all<{ id: number; name: string }>('SELECT id, name FROM collection_labels WHERE collection_id = ?', collectionId)) {
byName.set(row.name.trim().toLowerCase(), row.id);
}
let sortOrder = this.db.get<{ m: number }>(
'SELECT COALESCE(MAX(sort_order), -1) AS m FROM collection_labels WHERE collection_id = ?', collectionId,
)!.m + 1;
let created = 0;
for (const label of (labels ?? []).slice(0, MAX_COLLECTION_FILE_LABELS)) {
const key = label.name.trim().toLowerCase();
if (!key || byName.has(key)) continue;
byName.set(key, this.insertImportedLabel(collectionId, label, sortOrder));
sortOrder += 1;
created += 1;
}
return { byName, created };
}
/**
* The places of a file, written into a list that is already there to take
* them. Both imports go through here, so a file behaves the same way
* whichever one read it.
*
* `skipDuplicates` is the whole difference. A list that was just created out
* of this file is empty and takes every place it carries, duplicates within
* the file included, exactly as it always has; a list people have been
* working in keeps what it has.
*
* New places are appended after the ones already there rather than renumbered
* from zero, so a manual order survives an import.
*/
private writeFilePlaces(
collectionId: number,
savedBy: number,
file: CollectionFile,
labelIdByName: Map<string, number>,
opts: { skipDuplicates: boolean },
): { imported: number; skipped: number; duplicates: number } {
// The palette is instance-wide and read-only here: a file names a category,
// it does not get to create one.
const categoryIdByName = new Map<string, number>();
for (const c of this.db.all<{ id: number; name: string }>('SELECT id, name FROM categories')) {
categoryIdByName.set(c.name.trim().toLowerCase(), c.id);
}
const ownerId = this.ownerOf(collectionId);
const firstOrder = this.db.get<{ m: number }>(
'SELECT COALESCE(MAX(sort_order), -1) AS m FROM collection_places WHERE collection_id = ?', collectionId,
)!.m + 1;
const insertPlace = this.db.prepare(`
INSERT INTO collection_places (
collection_id, owner_id, saved_by, name, description, lat, lng, address,
category_id, price, currency, notes, image_url, google_place_id, google_ftid,
osm_id, website, phone, status, links, sort_order
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`);
const assignLabel = this.db.prepare('INSERT OR IGNORE INTO collection_place_labels (collection_place_id, label_id) VALUES (?, ?)');
let imported = 0;
let skipped = 0;
let duplicates = 0;
for (const raw of file.places) {
const parsed = collectionFilePlaceSchema.safeParse(raw);
if (!parsed.success) { skipped += 1; continue; }
const place = parsed.data;
// Rows written earlier in this same run count as already there, so a file
// that lists a place twice adds it once.
if (opts.skipDuplicates && this.findDuplicateCollectionPlace(collectionId, {
name: place.name,
lat: place.lat ?? null,
lng: place.lng ?? null,
google_place_id: place.google_place_id ?? null,
google_ftid: place.google_ftid ?? null,
osm_id: place.osm_id ?? null,
})) {
duplicates += 1;
continue;
}
const res = insertPlace.run(
collectionId, ownerId, savedBy,
place.name, place.description ?? null, place.lat ?? null, place.lng ?? null, place.address ?? null,
place.category ? (categoryIdByName.get(place.category.trim().toLowerCase()) ?? null) : null,
place.price ?? null, place.currency ?? null, place.notes ?? null,
place.image_url ?? null, place.google_place_id ?? null, place.google_ftid ?? null,
place.osm_id ?? null, place.website ?? null, place.phone ?? null,
place.status ?? 'idea', serializeLinks(place.links), firstOrder + imported,
);
const placeId = Number(res.lastInsertRowid);
for (const labelName of place.labels ?? []) {
const labelId = labelIdByName.get(labelName.trim().toLowerCase());
if (labelId) assignLabel.run(placeId, labelId);
}
imported += 1;
}
return { imported, skipped, duplicates };
}
/**
* A label straight from a file, without createLabel's duplicate check.
*
* The caller already de-duplicates by name, the list is empty, and
* createLabel would notify the (nonexistent) members once per label.
* The caller de-duplicates by name, and one notification for the whole
* import is sent by the caller rather than one per label.
*/
private insertImportedLabel(collectionId: number, label: CollectionFileLabel, sortOrder: number): number {
const res = this.db.run(
+29
View File
@@ -403,6 +403,35 @@ describe('Collections e2e (real auth guard + real service + temp SQLite)', () =>
expect((await request(server).get(`/api/addons/collections/${col.id}/export/gpx`).set('Cookie', sessionCookie(otherId))).status).toBe(404);
});
it('COLLECTIONS-E2E-083: a file goes into a list that already exists, without touching what is in it', async () => {
const as = (userId: number) => ({ Cookie: sessionCookie(userId) });
const col = (await request(server).post('/api/addons/collections').set(as(ownerId)).send({ name: 'Keep me' })).body;
await request(server).post('/api/addons/collections/places').set(as(ownerId))
.send({ collection_id: col.id, name: 'Pinned', lat: 41.9, lng: 12.48, status: 'want' });
const file = {
format: 'trek.collection', version: 1, name: 'From a friend', color: '#ef4444',
places: [{ name: 'Pinned', lat: 41.9, lng: 12.48 }, { name: 'New one' }],
};
const added = await request(server).post(`/api/addons/collections/${col.id}/import`).set(as(ownerId)).send({ file });
expect(added.status).toBe(200);
expect(added.body).toMatchObject({ imported: 1, skipped: 0, duplicates: 1 });
expect(added.body.collection).toMatchObject({ id: col.id, name: 'Keep me' });
const detail = await request(server).get(`/api/addons/collections/${col.id}`).set(as(ownerId));
expect(detail.body.places.map((p: { name: string }) => p.name)).toEqual(['Pinned', 'New one']);
expect(detail.body.places[0]).toMatchObject({ status: 'want' });
});
it('COLLECTIONS-E2E-084: a list the caller may only read, or cannot see at all, takes no file', async () => {
const col = (await request(server).post('/api/addons/collections').set('Cookie', sessionCookie(ownerId)).send({ name: 'Mine' })).body;
const file = { format: 'trek.collection', version: 1, name: 'Theirs', places: [{ name: 'Belém' }] };
expect((await request(server).post(`/api/addons/collections/${col.id}/import`).set('Cookie', sessionCookie(otherId)).send({ file })).status).toBe(404);
expect((await request(server).post(`/api/addons/collections/${col.id}/import`).send({ file })).status).toBe(401);
expect((await request(server).post(`/api/addons/collections/${col.id}/import`).set('Cookie', sessionCookie(ownerId)).send({ file: { name: 'no format' } })).status).toBe(400);
});
// ── delete ───────────────────────────────────────────────────────────────
it('COLLECTIONS-E2E-050: owner deletes; non-owner member cannot (403)', async () => {
const col = (await request(server).post('/api/addons/collections').set('Cookie', sessionCookie(ownerId)).send({ name: 'Doomed' })).body;
@@ -1284,6 +1284,156 @@ describe('exportCollection / importCollection (#2198)', () => {
});
});
// ── The same file into a list that is already there (#2301 follow-up) ────────
describe('importIntoCollection', () => {
const file = (places: unknown[], labels?: { name: string; color?: string }[]) => ({
format: 'trek.collection' as const, version: 1, name: 'From a friend',
description: 'Their notes', color: '#ef4444', ...(labels ? { labels } : {}), places,
});
it('COLLECTIONS-SVC-120: adds the file to the list and leaves the list itself alone', () => {
const owner = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Lisbon', description: 'Mine', color: '#111827' });
svc.savePlace(owner.id, { collection_id: col.id, name: 'Time Out Market' });
const result = svc.importIntoCollection(owner.id, col.id, { file: file([{ name: 'Belém' }, { name: 'Alfama' }]) });
expect(result).toMatchObject({ imported: 2, skipped: 0, duplicates: 0 });
const after = svc.getCollection(owner.id, col.id);
expect(after.collection).toMatchObject({ name: 'Lisbon', description: 'Mine', color: '#111827' });
expect(after.places.map(p => p.name)).toEqual(['Time Out Market', 'Belém', 'Alfama']);
// Appended after what was there, so a manual order survives.
const orders = testDb.prepare('SELECT name, sort_order FROM collection_places WHERE collection_id = ? ORDER BY sort_order').all(col.id);
expect(orders).toEqual([
{ name: 'Time Out Market', sort_order: 0 },
{ name: 'Belém', sort_order: 1 },
{ name: 'Alfama', sort_order: 2 },
]);
});
it('COLLECTIONS-SVC-121: a place the list already has is counted and left exactly as it was', () => {
const owner = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Lisbon' });
const mine = svc.savePlace(owner.id, {
collection_id: col.id, name: 'Time Out Market', notes: 'Before noon', status: 'visited', osm_id: 'node/1',
}).place!;
const result = svc.importIntoCollection(owner.id, col.id, { file: file([
{ name: 'Time Out Market', notes: 'Overrated', status: 'idea' },
{ name: 'Renamed at the source', osm_id: 'node/1' },
{ name: 'Belém' },
]) });
expect(result).toMatchObject({ imported: 1, duplicates: 2, skipped: 0 });
const places = svc.getCollection(owner.id, col.id).places;
expect(places.map(p => p.name)).toEqual(['Time Out Market', 'Belém']);
expect(places[0]).toMatchObject({ id: mine.id, notes: 'Before noon', status: 'visited' });
});
it('COLLECTIONS-SVC-122: a file that lists the same place twice adds it once', () => {
const owner = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Lisbon' });
const result = svc.importIntoCollection(owner.id, col.id, { file: file([{ name: 'Belém' }, { name: 'belém' }]) });
expect(result).toMatchObject({ imported: 1, duplicates: 1 });
});
it('COLLECTIONS-SVC-123: labels are matched by name, and only the missing ones are created', () => {
const owner = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Lisbon' });
const must = svc.createLabel(owner.id, col.id, 'Must see', '#ff0000');
svc.importIntoCollection(owner.id, col.id, { file: file(
[{ name: 'Belém', labels: ['Must see', 'Rainy day'] }],
[{ name: 'must see', color: '#00ff00' }, { name: 'Rainy day', color: '#0000ff' }],
) });
const labels = svc.getCollection(owner.id, col.id).collection.labels ?? [];
expect(labels.map(l => ({ name: l.name, color: l.color }))).toEqual([
// The list's own label keeps its name and its colour.
{ name: 'Must see', color: '#ff0000' },
{ name: 'Rainy day', color: '#0000ff' },
]);
const added = svc.getCollection(owner.id, col.id).places.find(p => p.name === 'Belém')!;
expect([...(added.label_ids ?? [])].sort()).toEqual([must.id, labels[1].id].sort());
});
it('COLLECTIONS-SVC-124: an editor may add a file, a viewer may not, a stranger does not see the list', () => {
const owner = createUser(testDb).user;
const editor = createUser(testDb).user;
const viewer = createUser(testDb).user;
const stranger = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Shared' });
addMember(col.id, editor.id, 'editor');
addMember(col.id, viewer.id, 'viewer');
expect(svc.importIntoCollection(editor.id, col.id, { file: file([{ name: 'Belém' }]) })).toMatchObject({ imported: 1 });
expect(() => svc.importIntoCollection(viewer.id, col.id, { file: file([{ name: 'Alfama' }]) })).toThrow();
try {
svc.importIntoCollection(stranger.id, col.id, { file: file([{ name: 'Alfama' }]) });
throw new Error('should have thrown');
} catch (e) {
expect((e as { status: number }).status).toBe(404);
}
expect(svc.getCollection(owner.id, col.id).places.map(p => p.name)).toEqual(['Belém']);
});
it('COLLECTIONS-SVC-125: the places stay the list owner\'s, saved by whoever brought the file', () => {
const owner = createUser(testDb).user;
const editor = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Shared' });
addMember(col.id, editor.id, 'editor');
svc.importIntoCollection(editor.id, col.id, { file: file([{ name: 'Belém' }]) });
const row = testDb.prepare('SELECT owner_id, saved_by FROM collection_places WHERE collection_id = ?').get(col.id);
expect(row).toEqual({ owner_id: owner.id, saved_by: editor.id });
});
it('COLLECTIONS-SVC-126: everyone on the list is told once, and nothing is said when nothing changed', () => {
const owner = createUser(testDb).user;
const member = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Shared' });
addMember(col.id, member.id, 'editor');
svc.savePlace(owner.id, { collection_id: col.id, name: 'Belém' });
broadcastToUser.mockClear();
svc.importIntoCollection(owner.id, col.id, { file: file([{ name: 'Alfama' }]) });
// Once per person on the list, whole import, not once per place.
expect(broadcastToUser).toHaveBeenCalledTimes(2);
expect(broadcastToUser).toHaveBeenCalledWith(member.id, expect.objectContaining({ type: 'collections:updated' }), undefined);
broadcastToUser.mockClear();
const again = svc.importIntoCollection(owner.id, col.id, { file: file([{ name: 'Alfama' }]) });
expect(again).toMatchObject({ imported: 0, duplicates: 1 });
expect(broadcastToUser).not.toHaveBeenCalled();
});
it('COLLECTIONS-SVC-127: a file that fails halfway leaves the list as it was', () => {
const owner = createUser(testDb).user;
const col = svc.createCollection(owner.id, { name: 'Lisbon' });
svc.savePlace(owner.id, { collection_id: col.id, name: 'Time Out Market' });
const real = testDb.prepare.bind(testDb);
const spy = vi.spyOn(testDb, 'prepare').mockImplementation((sql: string) => {
if (sql.includes('INSERT INTO collection_places')) throw new Error('disk is full');
return real(sql);
});
try {
expect(() => svc.importIntoCollection(owner.id, col.id, {
file: file([{ name: 'Belém' }], [{ name: 'Rainy day' }]),
})).toThrow('disk is full');
} finally {
spy.mockRestore();
}
expect(svc.getCollection(owner.id, col.id).places.map(p => p.name)).toEqual(['Time Out Market']);
expect(svc.getCollection(owner.id, col.id).collection.labels ?? []).toEqual([]);
});
});
// ── The same list as GPX (#2301) ─────────────────────────────────────────────
describe('exportCollectionGpx / readCollectionGpx (#2301)', () => {