mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
Give GPX tracks their own colour (#776)
Imported tracks all render in the same blue: the GPX/KML importers never assign a category, so category_color is null for every one of them and several walks in the same area are impossible to tell apart. Adds a nullable route_color on places, kept out of the COALESCE group in updatePlace so an explicit null can reset a track back to inheriting its category colour. The hex is validated in the Nest controller rather than in the shared contract: the update body is an open record on purpose, and tightening it would mean removing PlacesController.update from the ratchet-only body-contract allow-list. The importers now hand each new track a colour of its own, picked from the palette entries the trip is not already using. That happens in the Nest service over the injected DatabaseService — the colour assignment is new logic, and new logic does not belong in the legacy place service (see server/src/nest/README.md). The field itself is only threaded through the existing update/create statements, which the proportionality rule covers. Also fixes duplicateTrip, whose explicit column list has been silently dropping route_geometry all along, so a duplicated trip lost its tracks.
This commit is contained in:
@@ -3822,6 +3822,14 @@ function runMigrations(db: Database.Database): void {
|
||||
);
|
||||
`);
|
||||
},
|
||||
// Manual GPX track colour (#776): imported tracks all render in the same blue
|
||||
// because the importer never assigns a category, so several walks in the same
|
||||
// area are indistinguishable. NULL keeps the old behaviour (category colour,
|
||||
// then the #3b82f6 fallback) for every existing row.
|
||||
() => {
|
||||
const hasRouteColor = db.prepare("SELECT 1 FROM pragma_table_info('places') WHERE name = 'route_color'").get();
|
||||
if (!hasRouteColor) db.exec('ALTER TABLE places ADD COLUMN route_color TEXT');
|
||||
},
|
||||
];
|
||||
|
||||
if (currentVersion < migrations.length) {
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
} from '@nestjs/common';
|
||||
import { FileInterceptor } from '@nestjs/platform-express';
|
||||
import { memoryStorage } from 'multer';
|
||||
import { hexColorSchema } from '@trek/shared';
|
||||
import { readEnv } from '../../app-config';
|
||||
import type { User } from '../../types';
|
||||
import { PlacesService } from './places.service';
|
||||
@@ -39,6 +40,16 @@ function validateLengths(body: Record<string, unknown>): void {
|
||||
}
|
||||
}
|
||||
|
||||
// A bad hex makes MapLibre fail hard when it parses the paint property, and the
|
||||
// update body is an open record that Zod does not police — so check it here.
|
||||
function validateRouteColor(body: Record<string, unknown>): void {
|
||||
const value = body.route_color;
|
||||
if (value === undefined || value === null) return;
|
||||
if (typeof value !== 'string' || !hexColorSchema.safeParse(value).success) {
|
||||
throw new HttpException({ error: 'route_color must be a hex colour like #4f46e5' }, 400);
|
||||
}
|
||||
}
|
||||
|
||||
function parseBool(v: unknown, defaultVal: boolean): boolean {
|
||||
return v === undefined || v === null ? defaultVal : String(v) === 'true';
|
||||
}
|
||||
@@ -93,6 +104,7 @@ export class PlacesController {
|
||||
) {
|
||||
const trip = this.requireTrip(tripId, user);
|
||||
validateLengths(body);
|
||||
validateRouteColor(body);
|
||||
this.requireEdit(trip, user);
|
||||
if (!body.name) {
|
||||
throw new HttpException({ error: 'Place name is required' }, 400);
|
||||
@@ -358,6 +370,7 @@ export class PlacesController {
|
||||
) {
|
||||
const trip = this.requireTrip(tripId, user);
|
||||
validateLengths(body);
|
||||
validateRouteColor(body);
|
||||
this.requireEdit(trip, user);
|
||||
const result = this.places.update(tripId, id, body as never, ifMatch);
|
||||
if (!result) {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { TRACK_COLORS } from '@trek/shared';
|
||||
import { broadcast } from '../../websocket';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
@@ -8,6 +9,8 @@ import { onPlaceCreated, onPlaceUpdated, onPlaceDeleted } from '../../services/j
|
||||
|
||||
type Trip = { user_id: number };
|
||||
|
||||
type ImportedPlace = { id: number; route_geometry?: string | null; route_color?: string | null };
|
||||
|
||||
/**
|
||||
* Thin Nest wrapper around the existing place service. Trip access mirrors the
|
||||
* requireTripAccess middleware (canAccessTrip); mutations use 'place_edit'. The
|
||||
@@ -66,11 +69,53 @@ export class PlacesService {
|
||||
buffer: Buffer,
|
||||
opts: { importWaypoints: boolean; importRoutes: boolean; importTracks: boolean; defaultName?: string },
|
||||
) {
|
||||
return svc.importGpx(tripId, buffer, opts);
|
||||
return this.colorizeImportedTracks(tripId, svc.importGpx(tripId, buffer, opts));
|
||||
}
|
||||
|
||||
importMapFile(tripId: string, buffer: Buffer, filename: string, opts: svc.KmlImportOptions) {
|
||||
return svc.importMapFile(tripId, buffer, filename, opts);
|
||||
async importMapFile(tripId: string, buffer: Buffer, filename: string, opts: svc.KmlImportOptions) {
|
||||
return this.colorizeImportedTracks(tripId, await svc.importMapFile(tripId, buffer, filename, opts));
|
||||
}
|
||||
|
||||
/**
|
||||
* Hand every freshly imported track its own colour (#776).
|
||||
*
|
||||
* The importers never assign a category, so without this every track in a
|
||||
* trip renders in the same #3b82f6 — which is the actual complaint behind
|
||||
* the request: several walks in one area are impossible to tell apart.
|
||||
*
|
||||
* Picks the palette entries the trip is not already using rather than
|
||||
* counting rows: counting collides as soon as somebody recolours a track by
|
||||
* hand or deletes one, which is exactly when distinguishability matters.
|
||||
* Once all ten are taken it wraps around — ten walks in one trip is already
|
||||
* past what a colour alone can separate.
|
||||
*
|
||||
* Only rows that carry geometry are touched, and only ones that have no
|
||||
* colour yet; plain waypoints and existing places stay untouched.
|
||||
*/
|
||||
private colorizeImportedTracks<T extends { places: ImportedPlace[] } | null>(tripId: string, result: T): T {
|
||||
const tracks = result?.places?.filter((p) => p.route_geometry && !p.route_color) ?? [];
|
||||
if (tracks.length === 0) return result;
|
||||
|
||||
// Read and write in one transaction so two concurrent imports cannot both
|
||||
// read the same set of free colours.
|
||||
this.dbs.transaction((conn) => {
|
||||
const taken = new Set(
|
||||
(conn
|
||||
.prepare('SELECT DISTINCT route_color AS c FROM places WHERE trip_id = ? AND route_color IS NOT NULL')
|
||||
.all(tripId) as { c: string }[]).map((r) => r.c),
|
||||
);
|
||||
const free = TRACK_COLORS.filter((c) => !taken.has(c));
|
||||
const stmt = conn.prepare('UPDATE places SET route_color = ? WHERE id = ?');
|
||||
tracks.forEach((track, i) => {
|
||||
// Free ones first, then wrap through the whole palette — never reuse a
|
||||
// free colour twice within the same import.
|
||||
const color = i < free.length ? free[i] : TRACK_COLORS[(i - free.length) % TRACK_COLORS.length];
|
||||
stmt.run(color, track.id);
|
||||
track.route_color = color;
|
||||
});
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
importGoogleList(tripId: string, url: string, opts?: Parameters<typeof svc.importGoogleList>[2]) {
|
||||
|
||||
@@ -125,26 +125,28 @@ export function createPlace(
|
||||
place_time?: string; end_time?: string;
|
||||
duration_minutes?: number; notes?: string; image_url?: string;
|
||||
google_place_id?: string; google_ftid?: string; osm_id?: string; website?: string; phone?: string;
|
||||
transport_mode?: string; tags?: number[];
|
||||
transport_mode?: string; route_geometry?: string; route_color?: string; tags?: number[];
|
||||
},
|
||||
) {
|
||||
const {
|
||||
name, description, lat, lng, address, category_id, price, currency,
|
||||
place_time, end_time,
|
||||
duration_minutes, notes, image_url, google_place_id, google_ftid, osm_id, website, phone,
|
||||
transport_mode, tags = [],
|
||||
transport_mode, route_geometry, route_color, tags = [],
|
||||
} = body;
|
||||
|
||||
const result = db.prepare(`
|
||||
INSERT INTO places (trip_id, name, description, lat, lng, address, category_id, price, currency,
|
||||
place_time, end_time,
|
||||
duration_minutes, notes, image_url, google_place_id, google_ftid, osm_id, website, phone, transport_mode)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
duration_minutes, notes, image_url, google_place_id, google_ftid, osm_id, website, phone, transport_mode,
|
||||
route_geometry, route_color)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`).run(
|
||||
tripId, name, description || null, lat || null, lng || null, address || null,
|
||||
category_id || null, price || null, currency || null,
|
||||
place_time || null, end_time || null, duration_minutes || 60, notes || null, image_url || null,
|
||||
google_place_id || null, google_ftid || null, osm_id || null, website || null, phone || null, transport_mode || 'walking',
|
||||
route_geometry || null, route_color || null,
|
||||
);
|
||||
|
||||
const placeId = result.lastInsertRowid;
|
||||
@@ -182,7 +184,7 @@ export function updatePlace(
|
||||
place_time?: string; end_time?: string;
|
||||
duration_minutes?: number; notes?: string; image_url?: string;
|
||||
google_place_id?: string; google_ftid?: string; osm_id?: string; website?: string; phone?: string;
|
||||
transport_mode?: string; tags?: number[];
|
||||
transport_mode?: string; route_color?: string | null; tags?: number[];
|
||||
},
|
||||
ifMatch?: string,
|
||||
): ReturnType<typeof getPlaceWithTags> | UpdateConflict | null {
|
||||
@@ -200,7 +202,7 @@ export function updatePlace(
|
||||
name, description, lat, lng, address, category_id, price, currency,
|
||||
place_time, end_time,
|
||||
duration_minutes, notes, image_url, google_place_id, google_ftid, osm_id, website, phone,
|
||||
transport_mode, tags,
|
||||
transport_mode, route_color, tags,
|
||||
} = body;
|
||||
|
||||
db.prepare(`
|
||||
@@ -224,6 +226,7 @@ export function updatePlace(
|
||||
website = ?,
|
||||
phone = ?,
|
||||
transport_mode = COALESCE(?, transport_mode),
|
||||
route_color = ?,
|
||||
updated_at = CURRENT_TIMESTAMP
|
||||
WHERE id = ?
|
||||
`).run(
|
||||
@@ -246,6 +249,9 @@ export function updatePlace(
|
||||
website !== undefined ? website : existingPlace.website,
|
||||
phone !== undefined ? phone : existingPlace.phone,
|
||||
transport_mode || null,
|
||||
// Deliberately not COALESCE: an explicit null is how the picker resets a
|
||||
// track back to its category colour (#776).
|
||||
route_color !== undefined ? route_color : existingPlace.route_color,
|
||||
placeId,
|
||||
);
|
||||
|
||||
|
||||
@@ -917,14 +917,15 @@ export function copyTripById(sourceTripId: string | number, newOwnerId: number,
|
||||
const insertPlace = db.prepare(`
|
||||
INSERT INTO places (trip_id, name, description, lat, lng, address, category_id, price, currency,
|
||||
reservation_status, reservation_notes, reservation_datetime, place_time, end_time,
|
||||
duration_minutes, notes, image_url, google_place_id, google_ftid, website, phone, transport_mode, osm_id)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
duration_minutes, notes, image_url, google_place_id, google_ftid, website, phone, transport_mode, osm_id,
|
||||
route_geometry, route_color)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`);
|
||||
for (const p of oldPlaces) {
|
||||
const r = insertPlace.run(newTripId, p.name, p.description, p.lat, p.lng, p.address, p.category_id,
|
||||
p.price, p.currency, p.reservation_status, p.reservation_notes, p.reservation_datetime,
|
||||
p.place_time, p.end_time, p.duration_minutes, p.notes, p.image_url, p.google_place_id,
|
||||
p.google_ftid, p.website, p.phone, p.transport_mode, p.osm_id);
|
||||
p.google_ftid, p.website, p.phone, p.transport_mode, p.osm_id, p.route_geometry, p.route_color);
|
||||
placeMap.set(p.id, r.lastInsertRowid);
|
||||
}
|
||||
|
||||
|
||||
@@ -73,6 +73,8 @@ export interface Place {
|
||||
google_place_id?: string | null;
|
||||
google_ftid?: string | null;
|
||||
osm_id?: string | null;
|
||||
route_geometry?: string | null;
|
||||
route_color?: string | null;
|
||||
website?: string | null;
|
||||
phone?: string | null;
|
||||
transport_mode?: string;
|
||||
|
||||
@@ -114,6 +114,24 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
expect(bad.body).toEqual({ error: 'ids must be an array of numbers' });
|
||||
});
|
||||
|
||||
it('PUT route_color: hex through, null through, garbage rejected (#776)', async () => {
|
||||
pl.updatePlace.mockReturnValue({ id: 9, route_color: '#e11d48' });
|
||||
const ok = await request(server).put('/api/trips/5/places/9').set('Cookie', sessionCookie(1)).send({ route_color: '#e11d48' });
|
||||
expect(ok.status).toBe(200);
|
||||
expect(ok.body).toEqual({ place: { id: 9, route_color: '#e11d48' } });
|
||||
expect(pl.updatePlace).toHaveBeenCalledWith('5', '9', expect.objectContaining({ route_color: '#e11d48' }), undefined);
|
||||
|
||||
// null is the reset back to the inherited category colour.
|
||||
pl.updatePlace.mockReturnValue({ id: 9, route_color: null });
|
||||
const cleared = await request(server).put('/api/trips/5/places/9').set('Cookie', sessionCookie(1)).send({ route_color: null });
|
||||
expect(cleared.status).toBe(200);
|
||||
expect(pl.updatePlace).toHaveBeenLastCalledWith('5', '9', expect.objectContaining({ route_color: null }), undefined);
|
||||
|
||||
const bad = await request(server).put('/api/trips/5/places/9').set('Cookie', sessionCookie(1)).send({ route_color: 'red' });
|
||||
expect(bad.status).toBe(400);
|
||||
expect(bad.body).toEqual({ error: 'route_color must be a hex colour like #4f46e5' });
|
||||
});
|
||||
|
||||
it('404 trip when not accessible', async () => {
|
||||
canAccessTrip.mockReturnValue(undefined);
|
||||
const res = await request(server).get('/api/trips/5/places').set('Cookie', sessionCookie(1));
|
||||
|
||||
@@ -236,6 +236,37 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou
|
||||
expect(onUpdated).toHaveBeenCalledWith(9);
|
||||
});
|
||||
|
||||
describe('route_color (#776)', () => {
|
||||
it('400s on anything that is not a hex colour, before the permission check', () => {
|
||||
const canEdit = vi.fn().mockReturnValue(false); // would 403 if it got that far
|
||||
const err = { status: 400, body: { error: 'route_color must be a hex colour like #4f46e5' } };
|
||||
expect(thrown(() => new PlacesController(svc({ canEdit })).update(user, '5', '9', { route_color: 'red' }))).toEqual(err);
|
||||
expect(thrown(() => new PlacesController(svc({ canEdit })).update(user, '5', '9', { route_color: '#12345' }))).toEqual(err);
|
||||
expect(thrown(() => new PlacesController(svc({ canEdit })).update(user, '5', '9', { route_color: 123 }))).toEqual(err);
|
||||
expect(thrown(() => new PlacesController(svc({ canEdit })).create(user, '5', { name: 'T', route_color: 'red' }))).toEqual(err);
|
||||
expect(canEdit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('passes a valid hex through, and null through as the reset to auto', () => {
|
||||
const update = vi.fn().mockReturnValue({ id: 9, route_color: '#e11d48' });
|
||||
const broadcast = vi.fn();
|
||||
const s = svc({ update, broadcast } as Partial<PlacesService>);
|
||||
expect(new PlacesController(s).update(user, '5', '9', { route_color: '#e11d48' }, 'sock'))
|
||||
.toEqual({ place: { id: 9, route_color: '#e11d48' } });
|
||||
expect(update).toHaveBeenCalledWith('5', '9', expect.objectContaining({ route_color: '#e11d48' }), undefined);
|
||||
// The colour has to reach the other members too, not just the DB.
|
||||
expect(broadcast).toHaveBeenCalledWith('5', 'place:updated', { place: { id: 9, route_color: '#e11d48' } }, 'sock');
|
||||
|
||||
new PlacesController(svc({ update } as Partial<PlacesService>)).update(user, '5', '9', { route_color: null });
|
||||
expect(update).toHaveBeenLastCalledWith('5', '9', expect.objectContaining({ route_color: null }), undefined);
|
||||
});
|
||||
|
||||
it('accepts the short #abc form', () => {
|
||||
const update = vi.fn().mockReturnValue({ id: 9 });
|
||||
expect(new PlacesController(svc({ update } as Partial<PlacesService>)).update(user, '5', '9', { route_color: '#abc' })).toEqual({ place: { id: 9 } });
|
||||
});
|
||||
});
|
||||
|
||||
it('PUT /:id forwards the base-version token and 409s on a conflict (#1135)', () => {
|
||||
const update = vi.fn().mockReturnValue({ conflict: true, server: { id: 9, name: 'Theirs' } });
|
||||
const onUpdated = vi.fn(); const broadcast = vi.fn();
|
||||
|
||||
@@ -0,0 +1,189 @@
|
||||
/**
|
||||
* Unit tests for PlacesService — PLACES-SVC-001 through PLACES-SVC-005.
|
||||
*
|
||||
* Covers the one piece of logic the Nest wrapper owns rather than delegates:
|
||||
* handing every freshly imported track its own colour (#776). The importers
|
||||
* never assign a category, so without this each track in a trip renders in the
|
||||
* same blue and several walks in one area are impossible to tell apart.
|
||||
*
|
||||
* Uses a real in-memory SQLite DB so the SQL is exercised faithfully; the
|
||||
* service is constructed directly, no Nest container needed.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest';
|
||||
import { TRACK_COLORS } from '@trek/shared';
|
||||
|
||||
// ── DB setup ──────────────────────────────────────────────────────────────────
|
||||
|
||||
const { testDb, dbMock } = vi.hoisted(() => {
|
||||
const Database = require('better-sqlite3');
|
||||
const db = new Database(':memory:');
|
||||
db.exec('PRAGMA journal_mode = WAL');
|
||||
db.exec('PRAGMA foreign_keys = ON');
|
||||
db.exec('PRAGMA busy_timeout = 5000');
|
||||
const mock = {
|
||||
db,
|
||||
closeDb: () => {},
|
||||
reinitialize: () => {},
|
||||
getPlaceWithTags: (placeId: any) => db.prepare('SELECT * FROM places WHERE id = ?').get(placeId) ?? null,
|
||||
canAccessTrip: () => null,
|
||||
isOwner: () => false,
|
||||
};
|
||||
return { testDb: db, dbMock: mock };
|
||||
});
|
||||
|
||||
vi.mock('../../../src/db/database', () => dbMock);
|
||||
vi.mock('../../../src/config', () => ({
|
||||
JWT_SECRET: 'test-secret',
|
||||
ENCRYPTION_KEY: 'a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2a3b4c5d6a7b8c9d0e1f2',
|
||||
updateJwtSecret: () => {},
|
||||
}));
|
||||
|
||||
import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createUser, createTrip } from '../../helpers/factories';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import { PlacesService } from '../../../src/nest/places/places.service';
|
||||
|
||||
// Three tracks plus a plain waypoint — the shared fixture only has waypoints,
|
||||
// and the whole point here is what happens to geometry.
|
||||
const GPX_WITH_TRACKS = `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<gpx version="1.1" creator="TREK Tests" xmlns="http://www.topografix.com/GPX/1/1">
|
||||
<wpt lat="48.8566" lon="2.3522"><name>Trailhead</name></wpt>
|
||||
<trk><name>Morning walk</name><trkseg>
|
||||
<trkpt lat="48.10" lon="2.10"/><trkpt lat="48.11" lon="2.11"/>
|
||||
</trkseg></trk>
|
||||
<trk><name>Afternoon walk</name><trkseg>
|
||||
<trkpt lat="48.20" lon="2.20"/><trkpt lat="48.21" lon="2.21"/>
|
||||
</trkseg></trk>
|
||||
<trk><name>Evening walk</name><trkseg>
|
||||
<trkpt lat="48.30" lon="2.30"/><trkpt lat="48.31" lon="2.31"/>
|
||||
</trkseg></trk>
|
||||
</gpx>`;
|
||||
|
||||
const svc = new PlacesService(new DatabaseService(testDb));
|
||||
|
||||
function importFixture(tripId: number) {
|
||||
return svc.importGpx(String(tripId), Buffer.from(GPX_WITH_TRACKS), {
|
||||
importWaypoints: true, importRoutes: true, importTracks: true,
|
||||
});
|
||||
}
|
||||
|
||||
function tracksOf(tripId: number) {
|
||||
return testDb.prepare('SELECT id, route_color FROM places WHERE trip_id = ? AND route_geometry IS NOT NULL ORDER BY id')
|
||||
.all(tripId) as { id: number; route_color: string | null }[];
|
||||
}
|
||||
|
||||
beforeAll(() => {
|
||||
createTables(testDb);
|
||||
runMigrations(testDb);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
resetTestDb(testDb);
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
testDb.close();
|
||||
});
|
||||
|
||||
describe('PlacesService — automatic track colours (#776)', () => {
|
||||
it('PLACES-SVC-001 — every imported track gets a colour from the shared palette', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
importFixture(trip.id);
|
||||
|
||||
const tracks = tracksOf(trip.id);
|
||||
expect(tracks.length).toBeGreaterThan(0);
|
||||
for (const track of tracks) {
|
||||
expect(TRACK_COLORS).toContain(track.route_color);
|
||||
}
|
||||
});
|
||||
|
||||
it('PLACES-SVC-002 — the returned places carry the colour, not just the DB rows', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const result = importFixture(trip.id) as { places: any[] } | null;
|
||||
|
||||
const returnedTracks = (result?.places ?? []).filter(p => p.route_geometry);
|
||||
expect(returnedTracks.length).toBeGreaterThan(0);
|
||||
for (const track of returnedTracks) {
|
||||
expect(TRACK_COLORS).toContain(track.route_color);
|
||||
}
|
||||
});
|
||||
|
||||
it('PLACES-SVC-003 — plain waypoints keep no colour at all', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
importFixture(trip.id);
|
||||
|
||||
const waypoints = testDb.prepare(
|
||||
'SELECT route_color FROM places WHERE trip_id = ? AND route_geometry IS NULL',
|
||||
).all(trip.id) as { route_color: string | null }[];
|
||||
for (const wp of waypoints) {
|
||||
expect(wp.route_color).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('PLACES-SVC-004 — a second import continues the palette instead of repeating it', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
importFixture(trip.id);
|
||||
const first = tracksOf(trip.id).map(t => t.route_color);
|
||||
|
||||
// Same fixture again: dedup skips the identical rows, so seed a distinct
|
||||
// track directly and let the service colour the next import round.
|
||||
testDb.prepare(
|
||||
"INSERT INTO places (trip_id, name, lat, lng, route_geometry) VALUES (?, 'Second walk', 1, 1, '[[1,1],[2,2]]')",
|
||||
).run(trip.id);
|
||||
const seeded = testDb.prepare('SELECT id FROM places WHERE name = ?').get('Second walk') as { id: number };
|
||||
(svc as any).colorizeImportedTracks(String(trip.id), {
|
||||
places: [{ id: seeded.id, route_geometry: '[[1,1],[2,2]]', route_color: null }],
|
||||
});
|
||||
|
||||
const seededColor = (testDb.prepare('SELECT route_color FROM places WHERE id = ?').get(seeded.id) as any).route_color;
|
||||
expect(TRACK_COLORS).toContain(seededColor);
|
||||
expect(first).not.toContain(seededColor);
|
||||
});
|
||||
|
||||
it('PLACES-SVC-006 — a colour already in use by hand is not handed out again', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
// Someone recoloured an existing track to the palette's second entry. A
|
||||
// plain row count would hand exactly that colour to the next import.
|
||||
testDb.prepare(
|
||||
"INSERT INTO places (trip_id, name, lat, lng, route_geometry, route_color) VALUES (?, 'Old walk', 1, 1, '[[1,1],[2,2]]', ?)",
|
||||
).run(trip.id, TRACK_COLORS[1]);
|
||||
|
||||
importFixture(trip.id);
|
||||
|
||||
const colors = tracksOf(trip.id).map(t => t.route_color);
|
||||
expect(new Set(colors).size).toBe(colors.length);
|
||||
});
|
||||
|
||||
it('PLACES-SVC-007 — gaps left by deleted tracks are reused, not skipped past', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
importFixture(trip.id);
|
||||
// Drop the first track; its colour becomes free again.
|
||||
const first = tracksOf(trip.id)[0];
|
||||
testDb.prepare('DELETE FROM places WHERE id = ?').run(first.id);
|
||||
|
||||
const seeded = testDb.prepare(
|
||||
"INSERT INTO places (trip_id, name, lat, lng, route_geometry) VALUES (?, 'Later walk', 9, 9, '[[9,9],[8,8]]') RETURNING id",
|
||||
).get(trip.id) as { id: number };
|
||||
(svc as any).colorizeImportedTracks(String(trip.id), {
|
||||
places: [{ id: seeded.id, route_geometry: '[[9,9],[8,8]]', route_color: null }],
|
||||
});
|
||||
|
||||
const colors = tracksOf(trip.id).map(t => t.route_color);
|
||||
expect(new Set(colors).size).toBe(colors.length);
|
||||
});
|
||||
|
||||
it('PLACES-SVC-005 — an import that yields nothing is passed straight through', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
expect((svc as any).colorizeImportedTracks(String(trip.id), null)).toBeNull();
|
||||
expect((svc as any).colorizeImportedTracks(String(trip.id), { places: [] })).toEqual({ places: [] });
|
||||
});
|
||||
});
|
||||
@@ -232,6 +232,49 @@ describe('updatePlace', () => {
|
||||
const updated = updatePlace(String(trip.id), String(place.id), { tags: [] }) as any;
|
||||
expect(updated.tags).toHaveLength(0);
|
||||
});
|
||||
|
||||
// ── Track colour (#776) ─────────────────────────────────────────────────────
|
||||
|
||||
it('PLACE-SVC-052 — stores a picked route_color', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Walk' }) as any;
|
||||
const updated = updatePlace(String(trip.id), String(place.id), { route_color: '#e11d48' }) as any;
|
||||
expect(updated.route_color).toBe('#e11d48');
|
||||
});
|
||||
|
||||
it('PLACE-SVC-053 — an explicit null clears it again (the reset to auto)', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Walk' }) as any;
|
||||
updatePlace(String(trip.id), String(place.id), { route_color: '#e11d48' });
|
||||
// Guards the COALESCE trap: name/currency/transport_mode can never be
|
||||
// emptied, and route_color built that way would be a one-way door.
|
||||
const cleared = updatePlace(String(trip.id), String(place.id), { route_color: null }) as any;
|
||||
expect(cleared.route_color).toBeNull();
|
||||
});
|
||||
|
||||
it('PLACE-SVC-054 — an unrelated update leaves the colour alone', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Walk' }) as any;
|
||||
updatePlace(String(trip.id), String(place.id), { route_color: '#059669' });
|
||||
const renamed = updatePlace(String(trip.id), String(place.id), { name: 'Hike' }) as any;
|
||||
expect(renamed.name).toBe('Hike');
|
||||
expect(renamed.route_color).toBe('#059669');
|
||||
});
|
||||
|
||||
it('PLACE-SVC-055 — createPlace carries geometry and colour instead of dropping them', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const created = svcCreatePlace(String(trip.id), {
|
||||
name: 'Restored track',
|
||||
route_geometry: '[[48.0,2.0],[49.0,3.0]]',
|
||||
route_color: '#7c3aed',
|
||||
}) as any;
|
||||
expect(created.route_geometry).toBe('[[48.0,2.0],[49.0,3.0]]');
|
||||
expect(created.route_color).toBe('#7c3aed');
|
||||
});
|
||||
});
|
||||
|
||||
// ── updatePlacesMany ────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -31,6 +31,7 @@ export * from './airtrail/airtrail.schema';
|
||||
export * from './day/day.schema';
|
||||
export * from './assignment/assignment.schema';
|
||||
export * from './place/place.schema';
|
||||
export * from './place/track-colors';
|
||||
export * from './collection/collection.schema';
|
||||
export * from './trip/trip.schema';
|
||||
export * from './trip-invite/trip-invite.schema';
|
||||
|
||||
@@ -1,7 +1,30 @@
|
||||
import { placeCreateRequestSchema, placeBulkDeleteRequestSchema, placeImportListRequestSchema } from './place.schema';
|
||||
import {
|
||||
placeCreateRequestSchema,
|
||||
placeBulkDeleteRequestSchema,
|
||||
placeImportListRequestSchema,
|
||||
placeSchema,
|
||||
} from './place.schema';
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
|
||||
describe('placeSchema route_color (#776)', () => {
|
||||
const place = { id: 1, trip_id: 1, name: 'Walk' };
|
||||
|
||||
it('takes a hex colour, null, or nothing at all', () => {
|
||||
expect(placeSchema.safeParse({ ...place, route_color: '#e11d48' }).success).toBe(true);
|
||||
expect(placeSchema.safeParse({ ...place, route_color: '#abc' }).success).toBe(true);
|
||||
// null is how a track goes back to inheriting its category colour.
|
||||
expect(placeSchema.safeParse({ ...place, route_color: null }).success).toBe(true);
|
||||
expect(placeSchema.safeParse(place).success).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects anything a map renderer could not parse', () => {
|
||||
expect(placeSchema.safeParse({ ...place, route_color: 'blue' }).success).toBe(false);
|
||||
expect(placeSchema.safeParse({ ...place, route_color: '#12345' }).success).toBe(false);
|
||||
expect(placeSchema.safeParse({ ...place, route_color: 'e11d48' }).success).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('placeCreateRequestSchema', () => {
|
||||
it('requires a name and keeps the other place fields open', () => {
|
||||
expect(
|
||||
|
||||
@@ -16,6 +16,9 @@ import { z } from 'zod';
|
||||
|
||||
const open = z.record(z.string(), z.unknown());
|
||||
|
||||
/** `#rgb` / `#rrggbb`, the form both map renderers and CSS accept. */
|
||||
export const hexColorSchema = z.string().regex(/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/);
|
||||
|
||||
/**
|
||||
* Embedded category as returned on a place — a trimmed projection of the
|
||||
* categories row (id/name/color/icon), built inline by placeService and
|
||||
@@ -70,6 +73,8 @@ export const placeSchema = z.object({
|
||||
google_ftid: z.string().nullable().optional(),
|
||||
osm_id: z.string().nullable().optional(),
|
||||
route_geometry: z.string().nullable().optional(),
|
||||
// Manual track colour (#776). null = inherit the category colour like before.
|
||||
route_color: hexColorSchema.nullable().optional(),
|
||||
website: z.string().nullable().optional(),
|
||||
phone: z.string().nullable().optional(),
|
||||
transport_mode: z.string().nullable().optional(),
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
/**
|
||||
* Track colour palette (#776) — shared so the importer's automatic assignment
|
||||
* and the picker in the UI hand out the exact same set.
|
||||
*
|
||||
* Tuned against map tiles rather than UI surfaces: the 600 tier survives thin
|
||||
* 3.5px lines on light basemaps where 400/500 washes out, and nothing here is
|
||||
* a neutral grey (invisible on CartoDB Dark) or a pale pastel (invisible at
|
||||
* any size). Blue is deliberately blue-700 — darker than both the #3b82f6 GPX
|
||||
* fallback and the #0a84ff day route, so a blue track reads as chosen rather
|
||||
* than defaulted.
|
||||
*
|
||||
* The order matters: the importer assigns these round-robin, so the first five
|
||||
* are the ones most tracks get. They follow the Okabe-Ito idea of staying
|
||||
* separable under red-green colour blindness (blue, orange, green, magenta,
|
||||
* cyan) instead of running through the rainbow and handing out four adjacent
|
||||
* warm tones in a row.
|
||||
*/
|
||||
export const TRACK_COLORS = [
|
||||
'#1d4ed8', // blue-700
|
||||
'#ea580c', // orange-600
|
||||
'#059669', // emerald-600
|
||||
'#c026d3', // fuchsia-600
|
||||
'#0891b2', // cyan-600
|
||||
'#e11d48', // rose-600
|
||||
'#d97706', // amber-600
|
||||
'#65a30d', // lime-600
|
||||
'#7c3aed', // violet-600
|
||||
'#4f46e5', // indigo-600
|
||||
] as const;
|
||||
|
||||
/** Colour a track falls back to when it has neither a manual nor a category colour. */
|
||||
export const TRACK_COLOR_FALLBACK = '#3b82f6';
|
||||
Reference in New Issue
Block a user