mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 23:16:46 +00:00
refactor(places): places, tags, ratings and place images through repositories
This commit is contained in:
jubnl
@@ -99,6 +99,27 @@ export class AssignmentParticipantsRepository extends TrekRepository<AssignmentP
|
||||
* for a relation-backed `onConflictFields` list). The caller only ever
|
||||
* passes a non-empty, already-roster-scoped array; the empty-array guard
|
||||
* here is defensive, matching the service's own guard.
|
||||
*
|
||||
* **Two differences from the legacy per-row loop (Task 3 review M1,
|
||||
* absorbed in Task 4), both benign today, documented so they stay that
|
||||
* way on purpose:**
|
||||
* 1. **Narrower failure mode.** `OR IGNORE` swallowed *any* constraint
|
||||
* violation on a row; `ON CONFLICT (assignment_id, user_id) DO
|
||||
* NOTHING` swallows only THAT unique index, and one row that violates
|
||||
* a DIFFERENT constraint aborts the whole multi-row statement rather
|
||||
* than being skipped. Unreachable today — AS28 roster-scopes the ids
|
||||
* before this call, and the caller has already gated the assignment
|
||||
* itself — but a concurrent delete of the assignment mid-request would
|
||||
* now 500 where the legacy loop silently no-opped that one row.
|
||||
* 2. **One extra statement.** `upsertMany` issues a reload `SELECT` after
|
||||
* the insert (`generated SQL, captured: insert … on conflict (…) do
|
||||
* nothing returning id` then `select … from assignment_participants
|
||||
* as a0 where (…)`) that the legacy loop never ran. Harmless — the
|
||||
* reloaded rows are fully hydrated and unchanged, so the enclosing
|
||||
* `em.transactional`'s closing flush emits nothing for them — but it
|
||||
* is a genuine extra round-trip and briefly populates the identity map
|
||||
* inside the write transaction, worth knowing if a future change here
|
||||
* ever needs to reason about statement counts.
|
||||
*/
|
||||
async insertIgnore(assignment_id: number, user_ids: number[]): Promise<void> {
|
||||
if (user_ids.length === 0) return;
|
||||
|
||||
@@ -300,13 +300,20 @@ export class DayAssignmentsRepository extends TrekRepository<DayAssignments> {
|
||||
/**
|
||||
* AS9 (`assignmentExistsInDay`) — `SELECT da.id FROM day_assignments da
|
||||
* JOIN days d ON da.day_id = d.id WHERE da.id = ? AND da.day_id = ? AND
|
||||
* d.trip_id = ?`. Raw-bind (`number | string`, D4's T5 escape hatch,
|
||||
* `TripsRepository.findAccessible`'s precedent): the legacy guard binds
|
||||
* the route's raw params straight into the statement with no `Number()`/
|
||||
* `toRowId` conversion of its own, so this must accept and bind exactly
|
||||
* what it's handed, not coerce it first.
|
||||
* d.trip_id = ?`.
|
||||
*
|
||||
* **`id: number`, not `number | string` (Task 3 review H1, absorbed in
|
||||
* Task 4 — `task-3-review.md` §9.1):** this method's ONLY caller
|
||||
* (`AssignmentsService.assignmentExistsInDay`) now runs `toRowId` on the
|
||||
* route's raw `id` BEFORE calling this — a non-canonical id (`"3 "`,
|
||||
* `"3.0"`) must resolve to "not found" in the SERVICE gate, not pass this
|
||||
* statement's own SQLite text/integer affinity match and then disagree
|
||||
* with `deleteAssignment`'s `toRowId(id)!` downstream (the H1 regression).
|
||||
* `day_id`/`trip_id` keep their `number | string` raw-bind scoping —
|
||||
* `deleteAssignment` never reads either back, so there is no gate-vs-write
|
||||
* id to protect there.
|
||||
*/
|
||||
async existsInDay(id: number | string, day_id: number | string, trip_id: number | string): Promise<boolean> {
|
||||
async existsInDay(id: number, day_id: number | string, trip_id: number | string): Promise<boolean> {
|
||||
const row = await this.qb('da')
|
||||
.join('da.day', 'd')
|
||||
.select(['da.id'])
|
||||
@@ -318,13 +325,21 @@ export class DayAssignmentsRepository extends TrekRepository<DayAssignments> {
|
||||
/**
|
||||
* AS12 (`getAssignmentForTrip`) — `SELECT da.* FROM day_assignments da
|
||||
* JOIN days d ON da.day_id = d.id WHERE da.id = ? AND d.trip_id = ?`.
|
||||
* Same raw-bind seam as `existsInDay` above — every real caller
|
||||
* (`AssignmentOpsController`, `AssignmentsMcp`, `ItineraryRpc`) hands this
|
||||
* the route/tool id verbatim, and `ItineraryRpc.unassign` reads
|
||||
* `existing.day_id` off the result, so this is a genuine re-select, not
|
||||
* only an existence guard.
|
||||
*
|
||||
* **`id: number`, not `number | string` (Task 3 review H1, absorbed in
|
||||
* Task 4 — `task-3-review.md` §9.1):** this method's ONLY caller
|
||||
* (`AssignmentsService.getAssignmentForTrip`) now runs `toRowId` on the
|
||||
* route/tool's raw `id` BEFORE calling this, the same H1 fix
|
||||
* `existsInDay` above applies — every mutation behind this gate
|
||||
* (`updateTime`/`setEndDay`/`updateNotes`/`setLegTransportMode`/
|
||||
* `setIncomingLegTransportMode`/`setParticipants`/`moveAssignment`)
|
||||
* converts `id` with its own `toRowId(id)!`, and `ItineraryRpc.unassign`
|
||||
* reads `existing.day_id` off THIS result (a genuine re-select, not only
|
||||
* an existence guard) — so the id this read resolves must be the same one
|
||||
* every one of those writes uses. `trip_id` keeps its `number | string`
|
||||
* raw-bind scoping, unchanged.
|
||||
*/
|
||||
async findInTrip(id: number | string, trip_id: number | string): Promise<DayAssignmentRow | undefined> {
|
||||
async findInTrip(id: number, trip_id: number | string): Promise<DayAssignmentRow | undefined> {
|
||||
const row = await this.qb('da')
|
||||
.join('da.day', 'd')
|
||||
.select(['da.*'])
|
||||
@@ -509,4 +524,39 @@ export class DayAssignmentsRepository extends TrekRepository<DayAssignments> {
|
||||
async setIncomingLegMode(id: number, mode: string | null): Promise<void> {
|
||||
await this.nativeUpdate({ id }, { incoming_leg_transport_mode: mode });
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 (`PlacesService.exportGpx`) — appended per the task-4
|
||||
// brief's "task-3-report.md" pointer ("DayAssignmentsRepository for the
|
||||
// PL30 GPX itinerary read if it fits there"). Unlike AS16/AS18, this join
|
||||
// (`day_assignments` -> `days`, `day_assignments` -> `places`) is fully
|
||||
// expressible through the entity's own declared relations (`da.day`,
|
||||
// `da.place`) — no `day_accommodations`-shaped gap — so the QueryBuilder is
|
||||
// used directly rather than Kysely.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* PL30 (`places.service.ts::exportGpx`) — `SELECT d.day_number, d.date,
|
||||
* d.title, p.name, p.lat, p.lng FROM days d JOIN day_assignments da ON
|
||||
* da.day_id = d.id JOIN places p ON p.id = da.place_id WHERE d.trip_id = ?
|
||||
* AND p.lat IS NOT NULL AND p.lng IS NOT NULL ORDER BY d.day_number,
|
||||
* da.order_index` — rooted here on `day_assignments` instead of `days`
|
||||
* (the legacy statement's own `FROM` root) because both joins the legacy
|
||||
* statement needs (`day_assignments -> days`, `day_assignments -> places`)
|
||||
* are single hops FROM this entity, while `days` has no direct relation to
|
||||
* `places`. Same result set, same ordering; the root alias choice is not
|
||||
* observable from the row shape.
|
||||
*/
|
||||
async listItineraryForGpx(trip_id: string | number): Promise<{
|
||||
day_number: number; date: string | null; title: string | null;
|
||||
name: string; lat: number; lng: number;
|
||||
}[]> {
|
||||
return this.qb('da')
|
||||
.join('da.day', 'd')
|
||||
.join('da.place', 'p')
|
||||
.select(['d.day_number', 'd.date', 'd.title', 'p.name', 'p.lat', 'p.lng'])
|
||||
.where('d.trip_id = ? AND p.lat IS NOT NULL AND p.lng IS NOT NULL', [trip_id])
|
||||
.orderBy({ 'd.day_number': 'asc', 'da.order_index': 'asc' })
|
||||
.execute<{ day_number: number; date: string | null; title: string | null; name: string; lat: number; lng: number }[]>('all', false);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -40,4 +40,37 @@ export class PlaceRatingsRepository extends TrekRepository<PlaceRatings> {
|
||||
.orderBy({ 'pr.created_at': 'asc' })
|
||||
.execute<PlaceRatingForPlaceRow[]>('all', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* PL51 (`places.service.ts::rate`) — `INSERT INTO place_ratings (place_id,
|
||||
* user_id, rating) VALUES (?, ?, ?) ON CONFLICT(place_id, user_id) DO
|
||||
* UPDATE SET rating = excluded.rating` — the only explicit `ON CONFLICT …
|
||||
* DO UPDATE` in the Plan 3c cluster (#1435). `onConflictFields: ['place',
|
||||
* 'user']` on the entity's own declared composite unique
|
||||
* (`PlaceRatingsSchema`'s `uniques: [{ properties: ['place', 'user'] }]`,
|
||||
* itself generated from the migration's `UNIQUE(place_id, user_id)`
|
||||
* — verified against both, not assumed). `onConflictMergeFields: ['rating']`
|
||||
* is load-bearing, not decorative: without it `em.upsert`'s default merge
|
||||
* set is every non-key column, which on THIS entity is only `rating`
|
||||
* anyway (`created_at` is the sole other column and is excluded because it
|
||||
* is the primary key's sibling default, not a mergeable field) — named
|
||||
* explicitly so a future column added to this entity does not silently
|
||||
* join the merge set and start touching a column the legacy statement
|
||||
* never wrote on conflict. The ruling this method exists to satisfy: a
|
||||
* vote must never touch `places.updated_at` (a separate table this
|
||||
* statement doesn't reference at all) so it can't 409 another member's
|
||||
* `If-Match` — proven by a repository test that reads `places.updated_at`
|
||||
* unchanged across a rating write.
|
||||
*/
|
||||
async upsertRating(place_id: number, user_id: number, rating: number): Promise<void> {
|
||||
await this.upsert(
|
||||
{ place: place_id, user: user_id, rating },
|
||||
{ onConflictFields: ['place', 'user'], onConflictAction: 'merge', onConflictMergeFields: ['rating'] },
|
||||
);
|
||||
}
|
||||
|
||||
/** PL50 — `DELETE FROM place_ratings WHERE place_id = ? AND user_id = ?` (a null rating clears the vote). */
|
||||
async deleteRating(place_id: number, user_id: number): Promise<void> {
|
||||
await this.nativeDelete({ place: place_id, user: user_id });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { Places } from '../entities/Places.entity';
|
||||
import type { Tags } from '../entities/Tags.entity';
|
||||
import { columnRef } from '../dialect/sql-functions';
|
||||
import { absDifference, columnRef, currentTimestamp, lowerTrim } from '../dialect/sql-functions';
|
||||
import { type AssertRowKeys } from './_shared/rows';
|
||||
import { TrekRepository } from './_shared/trek-repository';
|
||||
|
||||
@@ -53,7 +53,8 @@ export interface TagRow {
|
||||
|
||||
const _tagRowKeys: AssertRowKeys<TagRow, Tags> = true;
|
||||
|
||||
interface PlaceWithCategoryRow extends PlaceRow {
|
||||
/** PL3 (`PlacesService.list`) — a `places` row joined with its category's flat columns. */
|
||||
export interface PlaceWithCategoryRow extends PlaceRow {
|
||||
category_name: string | null;
|
||||
category_color: string | null;
|
||||
category_icon: string | null;
|
||||
@@ -181,4 +182,416 @@ export class PlacesRepository extends TrekRepository<Places> {
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 (`PlacesService` core) — every method below is additive;
|
||||
// `findWithTagsAndRatings`/`existsByGoogleIdOrImageUrl` above are Task 0b's
|
||||
// and Task 1's, unchanged.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* PL0/PL7/PL49/AS5 — `SELECT id FROM places WHERE id = ? AND trip_id = ?`.
|
||||
*
|
||||
* **`id: number`, not `number | string` (Task 3 review H1, absorbed
|
||||
* here — `task-3-review.md` §9.1):** the legacy raw-bind seam other
|
||||
* guard-shaped methods in this cluster preserve (`TripsRepository
|
||||
* .findAccessible`'s precedent) is the WRONG shape for a method whose
|
||||
* caller then writes with `toRowId(placeId)!`. A non-canonical id
|
||||
* (`"3 "`, `"3.0"`, `"+3"`) matches this statement via SQLite's own
|
||||
* loose text/integer affinity — `existsInTrip` would answer `true` — while
|
||||
* `toRowId` (`/^\d+$/` + `Number.isSafeInteger`, program rule 15) rejects
|
||||
* the same string and returns `null`, so the write downstream runs
|
||||
* `toRowId(placeId)!` on a value the gate just vouched for and gets
|
||||
* `null` — exactly Task 3's H1 (`AssignmentsService.dayExists`/
|
||||
* `placeExists`/`assignmentExistsInDay`/`getAssignmentForTrip`, all four
|
||||
* kept on this seam, all four reproducing it). This method's only two
|
||||
* callers (`PlacesService.get`/`applyUpdate`'s existence half — via
|
||||
* `applyUpdate` calling `findInTrip` below — and `AssignmentsService
|
||||
* .placeExists`, once Task 4 repoints it) now run `toRowId` FIRST and
|
||||
* pass the validated `number` in, matching `DaysService.getDay`'s shape
|
||||
* (the review's cited correct precedent) — so `id` here is narrowed to
|
||||
* match what every caller actually has after its own gate, and a second
|
||||
* `toRowId(...)!` downstream can never disagree with this one.
|
||||
*/
|
||||
async existsInTrip(id: number, trip_id: number): Promise<boolean> {
|
||||
const row = await this.qb('p')
|
||||
.select(['p.id'])
|
||||
.where('p.id = ? AND p.trip_id = ?', [id, trip_id])
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
}
|
||||
|
||||
/**
|
||||
* PL9/PL48 — `SELECT * FROM places WHERE id = ? AND trip_id = ?`, byte-
|
||||
* identical statements at both legacy sites (`applyUpdate`'s pre-image
|
||||
* read and `searchImage`'s existence-plus-name read) — one method for
|
||||
* both rather than two identically-bodied ones.
|
||||
*
|
||||
* `id: number`, same H1 fix and reasoning as `existsInTrip` above: both
|
||||
* callers write with the SAME already-`toRowId`'d id this read returns
|
||||
* (`applyUpdate` feeds it straight into `updatePlace`/`tagsRepo`;
|
||||
* `searchImage` only reads, but takes the same gate shape for
|
||||
* consistency — one rule for every place-id gate in this service, not
|
||||
* one per call site).
|
||||
*/
|
||||
async findInTrip(id: number, trip_id: number): Promise<PlaceRow | undefined> {
|
||||
return this.qb('p')
|
||||
.select(['p.*'])
|
||||
.where('p.id = ? AND p.trip_id = ?', [id, trip_id])
|
||||
.execute<PlaceRow | undefined>('get', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* PL17/PL20 — `SELECT google_place_id, image_url FROM places WHERE id = ?
|
||||
* AND trip_id = ?`, the reclaim-candidate projection `remove`/`removeMany`
|
||||
* read BEFORE their delete transaction. `id: number`, same H1 fix: both
|
||||
* callers immediately follow a `true` result with `deleteById(id)` on the
|
||||
* SAME numeric id.
|
||||
*/
|
||||
async reclaimInputs(id: number, trip_id: number): Promise<{ google_place_id: string | null; image_url: string | null } | undefined> {
|
||||
return this.qb('p')
|
||||
.select(['p.google_place_id', 'p.image_url'])
|
||||
.where('p.id = ? AND p.trip_id = ?', [id, trip_id])
|
||||
.execute<{ google_place_id: string | null; image_url: string | null } | undefined>('get', false);
|
||||
}
|
||||
|
||||
/** PL19/PL21 — `DELETE FROM places WHERE id = ?`, unscoped: every caller proved trip access first. */
|
||||
async deleteById(id: number): Promise<void> {
|
||||
await this.nativeDelete({ id });
|
||||
}
|
||||
|
||||
/**
|
||||
* PL23 — `` SELECT id FROM places WHERE trip_id = ? AND id IN (${…}) ``,
|
||||
* narrowed to the caller's own INPUT ORDER (not SQL result order — SQLite
|
||||
* makes no ordering promise for an `IN (...)` scan and the legacy code
|
||||
* re-filters the original `ids` array through a `Set` built from the row
|
||||
* set for exactly this reason). `$in` for the dynamic list (rule 17b, "the
|
||||
* good kind").
|
||||
*/
|
||||
async scopedIds(trip_id: string | number, ids: number[]): Promise<number[]> {
|
||||
if (ids.length === 0) return [];
|
||||
const rows = await this.qb('p')
|
||||
.select(['p.id'])
|
||||
.where('p.trip_id = ?', [trip_id])
|
||||
.andWhere({ id: { $in: ids } })
|
||||
.execute<{ id: number }[]>('all', false);
|
||||
const owned = new Set(rows.map((r) => r.id));
|
||||
return ids.filter((id) => owned.has(id));
|
||||
}
|
||||
|
||||
/**
|
||||
* PL4 — the 25-column `INSERT INTO places (...)` the service's `create`
|
||||
* builds. Every value here is already fully coerced by the caller (the
|
||||
* `?? null` vs `|| null` split PL4's own legacy comment documents — 0 is
|
||||
* legitimate for `lat`/`lng`/`price`/`fill_percent`, and
|
||||
* `duration_minutes` defaults to 60 — stays the service's decision, this
|
||||
* writes exactly what it is handed, `DaysRepository.createDay`'s split).
|
||||
* `em.insert()` returns the generated PK (R6's `lastInsertRowid`
|
||||
* replacement).
|
||||
*/
|
||||
async insertPlace(input: {
|
||||
trip_id: number;
|
||||
name: string;
|
||||
description: string | null;
|
||||
lat: number | null;
|
||||
lng: number | null;
|
||||
address: string | null;
|
||||
category_id: number | null;
|
||||
price: number | null;
|
||||
currency: string | null;
|
||||
place_time: string | null;
|
||||
end_time: string | null;
|
||||
duration_minutes: number;
|
||||
notes: string | null;
|
||||
image_url: string | null;
|
||||
google_place_id: string | null;
|
||||
google_ftid: string | null;
|
||||
osm_id: string | null;
|
||||
amap_poi_id: string | null;
|
||||
website: string | null;
|
||||
phone: string | null;
|
||||
transport_mode: string;
|
||||
route_geometry: string | null;
|
||||
route_color: string | null;
|
||||
stop_type: string | null;
|
||||
fill_percent: number | null;
|
||||
}): Promise<number> {
|
||||
return await this.insert({
|
||||
trip: input.trip_id,
|
||||
name: input.name,
|
||||
description: input.description,
|
||||
lat: input.lat,
|
||||
lng: input.lng,
|
||||
address: input.address,
|
||||
category: input.category_id,
|
||||
price: input.price,
|
||||
currency: input.currency,
|
||||
place_time: input.place_time,
|
||||
end_time: input.end_time,
|
||||
duration_minutes: input.duration_minutes,
|
||||
notes: input.notes,
|
||||
image_url: input.image_url,
|
||||
google_place_id: input.google_place_id,
|
||||
google_ftid: input.google_ftid,
|
||||
osm_id: input.osm_id,
|
||||
amap_poi_id: input.amap_poi_id,
|
||||
website: input.website,
|
||||
phone: input.phone,
|
||||
transport_mode: input.transport_mode,
|
||||
route_geometry: input.route_geometry,
|
||||
route_color: input.route_color,
|
||||
stop_type: input.stop_type,
|
||||
fill_percent: input.fill_percent,
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* PL11 — `UPDATE places SET name = COALESCE(?, name), description = ?,
|
||||
* lat = ?, lng = ?, address = ?, category_id = ?, price = ?, currency =
|
||||
* COALESCE(?, currency), place_time = ?, end_time = ?, duration_minutes =
|
||||
* ?, notes = ?, image_url = ?, google_place_id = ?, google_ftid = ?,
|
||||
* osm_id = ?, amap_poi_id = ?, website = ?, phone = ?, transport_mode =
|
||||
* COALESCE(?, transport_mode), route_color = ?, stop_type = ?,
|
||||
* fill_percent = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?` — ONE
|
||||
* `nativeUpdate` with a typed full partial (the ruling): the SQL
|
||||
* `COALESCE(?, col)` keep-if-null semantics (`name`/`currency`/
|
||||
* `transport_mode`) and the twenty `!== undefined ? x : existing.x`
|
||||
* pre-image fallbacks are both already resolved to their FINAL value by
|
||||
* the caller (`PlacesService.applyUpdate`, which reads the pre-image via
|
||||
* `findInTrip` first) — this method writes exactly what it is handed,
|
||||
* plus the timestamp stamp every write gets.
|
||||
*/
|
||||
async updatePlace(id: number, write: {
|
||||
name: string | null;
|
||||
description: string | null;
|
||||
lat: number | null;
|
||||
lng: number | null;
|
||||
address: string | null;
|
||||
category_id: number | null;
|
||||
price: number | null;
|
||||
currency: string | null;
|
||||
place_time: string | null;
|
||||
end_time: string | null;
|
||||
duration_minutes: number | null;
|
||||
notes: string | null;
|
||||
image_url: string | null;
|
||||
google_place_id: string | null;
|
||||
google_ftid: string | null;
|
||||
osm_id: string | null;
|
||||
amap_poi_id: string | null;
|
||||
website: string | null;
|
||||
phone: string | null;
|
||||
transport_mode: string | null;
|
||||
route_color: string | null;
|
||||
stop_type: string | null;
|
||||
fill_percent: number | null;
|
||||
}): Promise<void> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
await this.nativeUpdate({ id }, {
|
||||
name: write.name,
|
||||
description: write.description,
|
||||
lat: write.lat,
|
||||
lng: write.lng,
|
||||
address: write.address,
|
||||
category: write.category_id,
|
||||
price: write.price,
|
||||
currency: write.currency,
|
||||
place_time: write.place_time,
|
||||
end_time: write.end_time,
|
||||
duration_minutes: write.duration_minutes,
|
||||
notes: write.notes,
|
||||
image_url: write.image_url,
|
||||
google_place_id: write.google_place_id,
|
||||
google_ftid: write.google_ftid,
|
||||
osm_id: write.osm_id,
|
||||
amap_poi_id: write.amap_poi_id,
|
||||
website: write.website,
|
||||
phone: write.phone,
|
||||
transport_mode: write.transport_mode,
|
||||
route_color: write.route_color,
|
||||
stop_type: write.stop_type,
|
||||
fill_percent: write.fill_percent,
|
||||
updated_at: currentTimestamp(platform),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* PL25 (`findDuplicatePlace`'s `externalId` strategy) — `SELECT id,
|
||||
* google_ftid FROM places WHERE trip_id = ? AND (google_place_id = ? OR
|
||||
* google_ftid = ? OR osm_id = ? OR amap_poi_id = ?) ORDER BY id ASC LIMIT
|
||||
* 1` — the same candidate id bound in all four positions, exactly as the
|
||||
* legacy statement does.
|
||||
*/
|
||||
async findDuplicateByExternalId(trip_id: string, external_id: string): Promise<{ id: number; google_ftid: string | null } | undefined> {
|
||||
return this.qb('p')
|
||||
.select(['p.id', 'p.google_ftid'])
|
||||
.where('p.trip_id = ?', [trip_id])
|
||||
.andWhere({
|
||||
$or: [
|
||||
{ google_place_id: external_id },
|
||||
{ google_ftid: external_id },
|
||||
{ osm_id: external_id },
|
||||
{ amap_poi_id: external_id },
|
||||
],
|
||||
})
|
||||
.orderBy({ 'p.id': 'asc' })
|
||||
.limit(1)
|
||||
.execute<{ id: number; google_ftid: string | null } | undefined>('get', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* PL26 (`findDuplicatePlace`'s `name` strategy) — `SELECT id, google_ftid
|
||||
* FROM places WHERE trip_id = ? AND lower(trim(name)) = ? ORDER BY id ASC
|
||||
* LIMIT 1`. Rule 18's documented-disagreement exception clause: the legacy
|
||||
* statement's OWN bound value is already JS-lowered+trimmed
|
||||
* (`normalizePlaceName`, `@trek/shared/place/place-match.ts`, Unicode-
|
||||
* aware), so `lowered_trimmed_name` here must be passed in ALREADY
|
||||
* lowered — never re-lowered through `lowerParam` (that would apply
|
||||
* SQLite's ASCII-only `LOWER()` a second time, on a value the caller
|
||||
* already folded fully). `lowerTrim(platform, 'p.name')` renders
|
||||
* `LOWER(TRIM(name))` (SQLite `LOWER()` is ASCII-only, program rule 18 —
|
||||
* the documented divergence PL26's legacy comment (`places.service.ts
|
||||
* :609-619`) already accepts and this repository reproduces literally,
|
||||
* not fixes).
|
||||
*/
|
||||
async findDuplicateByName(trip_id: string, lowered_trimmed_name: string): Promise<{ id: number; google_ftid: string | null } | undefined> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
return this.qb('p')
|
||||
.select(['p.id', 'p.google_ftid'])
|
||||
.where('p.trip_id = ?', [trip_id])
|
||||
.andWhere({ [lowerTrim(platform, 'p.name')]: lowered_trimmed_name })
|
||||
.orderBy({ 'p.id': 'asc' })
|
||||
.limit(1)
|
||||
.execute<{ id: number; google_ftid: string | null } | undefined>('get', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* PL27 (`findDuplicatePlace`'s `coords` strategy) — `SELECT id,
|
||||
* google_ftid FROM places WHERE trip_id = ? AND lat IS NOT NULL AND lng IS
|
||||
* NOT NULL AND abs(lat - ?) <= ? AND abs(lng - ?) <= ? ORDER BY id ASC
|
||||
* LIMIT 1`. `absDifference(platform, ref, value)` renders `ABS(<col> - ?)`
|
||||
* with `value` already bound inside the fragment (`sql-functions.ts`'s own
|
||||
* docstring) — used as a filter KEY paired with `{ $lte: tolerance }`, so
|
||||
* the emitted SQL and bind order match the legacy statement exactly:
|
||||
* `ABS(lat - ?) <= ?` then `ABS(lng - ?) <= ?`, the SAME `tolerance` value
|
||||
* on both.
|
||||
*/
|
||||
async findDuplicateByCoords(trip_id: string, lat: number, lng: number, tolerance: number): Promise<{ id: number; google_ftid: string | null } | undefined> {
|
||||
const platform = this.getEntityManager().getPlatform();
|
||||
return this.qb('p')
|
||||
.select(['p.id', 'p.google_ftid'])
|
||||
.where('p.trip_id = ?', [trip_id])
|
||||
.andWhere({
|
||||
lat: { $ne: null },
|
||||
lng: { $ne: null },
|
||||
[absDifference(platform, 'p.lat', lat)]: { $lte: tolerance },
|
||||
[absDifference(platform, 'p.lng', lng)]: { $lte: tolerance },
|
||||
})
|
||||
.orderBy({ 'p.id': 'asc' })
|
||||
.limit(1)
|
||||
.execute<{ id: number; google_ftid: string | null } | undefined>('get', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* PL29 (`exportGpx`'s waypoint read) — `SELECT p.name, p.description,
|
||||
* p.address, p.lat, p.lng, p.route_geometry, c.name AS category FROM
|
||||
* places p LEFT JOIN categories c ON c.id = p.category_id WHERE p.trip_id
|
||||
* = ? ORDER BY p.id`. PL30 (the `days`/`day_assignments` itinerary half)
|
||||
* is `DayAssignmentsRepository.listItineraryForGpx` — a different table
|
||||
* root, kept there per the task-4 brief's own pointer.
|
||||
*/
|
||||
async listForGpx(trip_id: string): Promise<{
|
||||
name: string; description: string | null; address: string | null;
|
||||
lat: number | null; lng: number | null; route_geometry: string | null;
|
||||
category: string | null;
|
||||
}[]> {
|
||||
return this.qb('p')
|
||||
.leftJoin('p.category', 'c')
|
||||
.select(['p.name', 'p.description', 'p.address', 'p.lat', 'p.lng', 'p.route_geometry', 'c.name as category'])
|
||||
.where('p.trip_id = ?', [trip_id])
|
||||
.orderBy({ 'p.id': 'asc' })
|
||||
.execute<{ name: string; description: string | null; address: string | null; lat: number | null; lng: number | null; route_geometry: string | null; category: string | null }[]>('all', false);
|
||||
}
|
||||
|
||||
/**
|
||||
* PI1 (`place-image.ts:27`) — `SELECT 1 FROM places WHERE image_url = ?
|
||||
* LIMIT 1`. Trip-agnostic on purpose (an uploaded image is ref-counted
|
||||
* across every trip, not just the one it was uploaded from — a place
|
||||
* copied or shared across trips can share the file).
|
||||
*/
|
||||
async existsByImageUrl(imageUrl: string): Promise<boolean> {
|
||||
const row = await this.qb('p')
|
||||
.select(['p.id'])
|
||||
.where({ image_url: imageUrl })
|
||||
.limit(1)
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
}
|
||||
|
||||
/**
|
||||
* PL3 (`PlacesService.list`) — the trip's places, joined with their
|
||||
* category's flat columns, filtered by up to four optional fragments
|
||||
* built as conditional `andWhere`s rather than a hand-assembled SQL
|
||||
* string (§18.3's surprise: the legacy statement's own text is not
|
||||
* knowable from the source without tracing four `if`s — this makes each
|
||||
* fragment a single, separately readable `andWhere` call instead).
|
||||
*
|
||||
* `filters.searchPattern` arrives PRE-BUILT (`%${escapeLikePattern(term)}%`,
|
||||
* the service's job, matching every other "coercions stay in the service"
|
||||
* split in this program) — this binds it verbatim into all three `LIKE …
|
||||
* ESCAPE '\'` fragments, matching the legacy's one-param-three-times bind.
|
||||
* The `ESCAPE '\'` clause is written into the raw fragment text because
|
||||
* MikroORM's `$like` operator does not add one (§18.3).
|
||||
*
|
||||
* `filters.category`/`filters.tag` bind their raw string values with no
|
||||
* `Number()` conversion (matching the legacy statement's own untyped
|
||||
* bind); the two `assignment` fragments reference `place_tags`/
|
||||
* `day_assignments`/`days` by physical table name in a subquery (neither
|
||||
* has, or needs, an entity-relation path for this shape) — the same
|
||||
* raw-SQL-subquery shape `PlacesService.list`'s own legacy string used.
|
||||
*
|
||||
* `DISTINCT` kept via `.select([...], true)` even though a place has at
|
||||
* most one category row (so it changes nothing about THIS join) — parity
|
||||
* with the legacy statement's own `SELECT DISTINCT`, not a functional
|
||||
* requirement.
|
||||
*/
|
||||
async listForTrip(trip_id: string, filters: {
|
||||
searchPattern?: string;
|
||||
category?: string;
|
||||
tag?: string;
|
||||
assignment?: 'all' | 'unassigned' | 'assigned';
|
||||
}): Promise<PlaceWithCategoryRow[]> {
|
||||
const qb = this.qb('p')
|
||||
.leftJoin('p.category', 'c')
|
||||
.select(['p.*', 'c.name as category_name', 'c.color as category_color', 'c.icon as category_icon'], true)
|
||||
.where('p.trip_id = ?', [trip_id]);
|
||||
|
||||
if (filters.searchPattern) {
|
||||
qb.andWhere(
|
||||
"(p.name LIKE ? ESCAPE '\\' OR p.address LIKE ? ESCAPE '\\' OR p.description LIKE ? ESCAPE '\\')",
|
||||
[filters.searchPattern, filters.searchPattern, filters.searchPattern],
|
||||
);
|
||||
}
|
||||
if (filters.category) {
|
||||
qb.andWhere('p.category_id = ?', [filters.category]);
|
||||
}
|
||||
if (filters.tag) {
|
||||
qb.andWhere('p.id IN (SELECT place_id FROM place_tags WHERE tag_id = ?)', [filters.tag]);
|
||||
}
|
||||
if (filters.assignment === 'unassigned') {
|
||||
qb.andWhere(
|
||||
'p.id NOT IN (SELECT da.place_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE d.trip_id = ?)',
|
||||
[trip_id],
|
||||
);
|
||||
} else if (filters.assignment === 'assigned') {
|
||||
qb.andWhere(
|
||||
'p.id IN (SELECT da.place_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE d.trip_id = ?)',
|
||||
[trip_id],
|
||||
);
|
||||
}
|
||||
|
||||
qb.orderBy({ 'p.created_at': 'desc' });
|
||||
|
||||
return qb.execute<PlaceWithCategoryRow[]>('all', false);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,6 +30,22 @@ export interface TagForPlaceRow {
|
||||
created_at: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* `place_tags` (`place_id`, `tag_id`, `PRIMARY KEY (place_id, tag_id)`,
|
||||
* `Migration20200101000000_baseline_schema.ts:195-199` — verified against
|
||||
* the migration, not assumed) as `Places.entity.ts`'s own `place_tags`
|
||||
* many-to-many relation declares it — a PLAIN pivot table
|
||||
* (`p.manyToMany(Tags).pivotTable('place_tags')...`) with no owning entity of
|
||||
* its own (unlike `PackingItems`' `packing_item_contributors`, which has a
|
||||
* `pivotEntity`). Neither `em.upsert` nor `qb()` can target a table with no
|
||||
* entity mapping — both are bound to exactly one mapped entity — so
|
||||
* `TrekRepository.kysely()` (D3's sanctioned next escape hatch) is the only
|
||||
* API that reaches it.
|
||||
*/
|
||||
interface PlaceTagsKyselyDB {
|
||||
place_tags: { place_id: number; tag_id: number };
|
||||
}
|
||||
|
||||
export class TagsRepository extends TrekRepository<Tags> {
|
||||
/**
|
||||
* `SELECT * FROM tags WHERE user_id = ? ORDER BY name ASC`, via the shared
|
||||
@@ -163,4 +179,63 @@ export class TagsRepository extends TrekRepository<Tags> {
|
||||
.where({ 'p.id': { $in: placeIds } })
|
||||
.execute<TagForPlaceRow[]>('all', false);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 (`PlacesService`) — additive methods.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* PL2 (`PlacesService.tagsOnTrip`) — `` SELECT id, user_id FROM tags WHERE
|
||||
* id IN (${…}) ``, the ownership half of tag scoping: the caller
|
||||
* (`tagsOnTrip`) intersects these rows' `user_id` against the trip's
|
||||
* roster, and an id belonging to nobody on the roster drops silently.
|
||||
* `$in` for the dynamic list (rule 17b).
|
||||
*/
|
||||
async findByIds(ids: number[]): Promise<{ id: number; user_id: number }[]> {
|
||||
if (ids.length === 0) return [];
|
||||
// `t.user` (unaliased, not separately joined) resolves to the physical
|
||||
// `user_id` column and comes back keyed as `user_id` on the raw driver
|
||||
// row — `TripsRepository.findAccessible`'s `.select(['t.id', 't.user',
|
||||
// 't.currency'])` establishes the same shape.
|
||||
return this.qb('t')
|
||||
.select(['t.id', 't.user'])
|
||||
.where({ id: { $in: ids } })
|
||||
.execute<{ id: number; user_id: number }[]>('all', false);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 (`PlacesService.create`/`applyUpdate`, PL5/PL12/PL13) —
|
||||
// `place_tags` writes. `PlaceTagsKyselyDB` above documents why Kysely.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* PL5/PL13 — `INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES
|
||||
* (?, ?)`, the legacy statement run once per tag id. One batched Kysely
|
||||
* insert with `onConflict((oc) => oc.columns(['place_id', 'tag_id'])
|
||||
* .doNothing())` on the pivot's real composite `PRIMARY KEY (place_id,
|
||||
* tag_id)` — SQLite's `ON CONFLICT (...) DO NOTHING` (supported since
|
||||
* SQLite 3.24, well within better-sqlite3's bundled version) is the
|
||||
* upsert-API-less equivalent of `INSERT OR IGNORE` for a table with no
|
||||
* entity mapping. Same end state as the legacy per-row loop (every
|
||||
* genuinely new pair inserted, every duplicate silently skipped); the
|
||||
* caller (`PlacesService.tagsOnTrip`) always hands an already
|
||||
* roster-scoped, deduplicated tag id list — the empty-array guard here is
|
||||
* defensive, matching every other batch-write method in this program.
|
||||
*/
|
||||
async insertIgnore(place_id: number, tag_ids: number[]): Promise<void> {
|
||||
if (tag_ids.length === 0) return;
|
||||
await this.kysely<PlaceTagsKyselyDB>()
|
||||
.insertInto('place_tags')
|
||||
.values(tag_ids.map((tag_id) => ({ place_id, tag_id })))
|
||||
.onConflict((oc) => oc.columns(['place_id', 'tag_id']).doNothing())
|
||||
.execute();
|
||||
}
|
||||
|
||||
/** PL12 — `DELETE FROM place_tags WHERE place_id = ?` (the replace-all half of an update). */
|
||||
async deleteForPlace(place_id: number): Promise<void> {
|
||||
await this.kysely<PlaceTagsKyselyDB>()
|
||||
.deleteFrom('place_tags')
|
||||
.where('place_id', '=', place_id)
|
||||
.execute();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,21 +65,26 @@ function sortMinutes(time: string | null): number | null {
|
||||
* `AssignmentParticipantsRepository` (Plan 3c Task 3, consuming Task 2's
|
||||
* `findWithPlaceAndCategory`/`listForDay` projection unchanged for AS1/AS3);
|
||||
* AS20–AS23 (`roadtrip_vias`) stay raw on `DatabaseService`, `// ASn — Plan
|
||||
* 3d` marked — that table belongs to Plan 3d. AS5 (`placeExists`) is the one
|
||||
* exception to "SQL lives in a repository method": `Places.repository.ts`
|
||||
* was another task's exclusive, actively-edited file this session (never
|
||||
* touched here), so the equivalent one-line `qb()` raw-condition read is
|
||||
* inlined below rather than added as a `PlacesRepository` method — flagged
|
||||
* in the task report for a follow-up to move it once that file is free.
|
||||
* 3d` marked — that table belongs to Plan 3d. AS5 (`placeExists`) called
|
||||
* `PlacesRepository.existsInTrip` through Task 4's follow-up, once that file
|
||||
* was free (it moved off an inlined `qb()` read this class carried for one
|
||||
* session while `Places.repository.ts` was another task's exclusive file).
|
||||
*
|
||||
* Assignment ids arriving from a route/tool are an **affinity seam** for the
|
||||
* mutation methods (`toRowId(id)!`, program rule 15) — every real call site
|
||||
* only ever reaches a mutation downstream of its own gate check in the same
|
||||
* request (`assignmentExistsInDay`/`getAssignmentForTrip`, both of which
|
||||
* stay on the legacy raw-bind seam themselves, matching `TripsRepository
|
||||
* .findAccessible`'s documented `number | string` pass-through), so the
|
||||
* non-null assertion is dead-code-safe, the same shape `DaysService` already
|
||||
* uses for its own single `getDay` gate.
|
||||
* **Assignment/day/place ids arriving from a route/tool are `toRowId`'d
|
||||
* inside each existence gate, not passed through on the affinity seam**
|
||||
* (Task 3 review H1, a live regression fixed by Task 4): `dayExists`/
|
||||
* `placeExists`/`assignmentExistsInDay`/`getAssignmentForTrip` used to bind
|
||||
* the caller's raw id straight into their own read, which SQLite's
|
||||
* text/integer affinity matches for a shape (`"3 "`, `"3.0"`, `"+3"`)
|
||||
* `toRowId` rejects — so a gate could answer "found" for an id the mutation
|
||||
* behind it, converting with its own `toRowId(id)!`, then treated as `null`
|
||||
* (a 500 on `move`, a silent no-write "success" on `notes`/`delete`). Fixed
|
||||
* the way `DaysService.getDay` (`days.service.ts:238-242`, the correct
|
||||
* precedent — NOT the raw pass-through this docstring previously,
|
||||
* incorrectly, cited it for) does it: `toRowId` runs INSIDE the gate, which
|
||||
* returns not-found on `null`, so every mutation's own `toRowId(id)!` below
|
||||
* can never disagree with the gate that authorized it — the non-null
|
||||
* assertion is dead-code-safe for real, not just by convention.
|
||||
*/
|
||||
@Injectable()
|
||||
export class AssignmentsService {
|
||||
@@ -173,25 +178,39 @@ export class AssignmentsService {
|
||||
});
|
||||
}
|
||||
|
||||
/** AS4 — `DaysRepository.existsInTrip`. Raw-bind pass-through, unchanged. */
|
||||
/**
|
||||
* AS4 — `DaysRepository.existsInTrip`, `dayId` `toRowId`'d INSIDE this
|
||||
* gate (Task 3 review H1 fix, absorbed in Task 4): `createAssignment`
|
||||
* (the only write behind this gate) converts `dayId` with its own
|
||||
* `toRowId(dayId)!` — a non-canonical id (`"3 "`, `"3.0"`) must answer
|
||||
* "not found" HERE, not pass `DaysRepository.existsInTrip`'s own raw-bind
|
||||
* affinity match and then hit `toRowId(dayId)!` downstream returning
|
||||
* `null` (the live regression: a `day_id: null` INSERT, 500). `tripId`
|
||||
* keeps the trip-scoping seam every gate in this cluster uses (D4's T5,
|
||||
* `TripsRepository.findAccessible`'s precedent) — trip access is already
|
||||
* verified upstream of every route this class serves, so `Number(tripId)`
|
||||
* is the correct, unchanged shape (`DaysService.getDay`'s own precedent
|
||||
* keeps its `tripId` unconverted too).
|
||||
*/
|
||||
async dayExists(dayId: string | number, tripId: string | number) {
|
||||
return await this.daysRepo.existsInTrip(dayId, tripId);
|
||||
const id = toRowId(dayId);
|
||||
if (id === null) return false;
|
||||
return await this.daysRepo.existsInTrip(id, Number(tripId));
|
||||
}
|
||||
|
||||
/**
|
||||
* AS5 — `SELECT id FROM places WHERE id = ? AND trip_id = ?`. `Places
|
||||
* .repository.ts` was another task's exclusive, actively-edited file this
|
||||
* session (see the class docstring) — inlined here as the one exception to
|
||||
* "SQL lives in a repository method" rather than adding a method there.
|
||||
* Same raw-bind seam as `dayExists`/`DayAssignmentsRepository.existsInDay`.
|
||||
* AS5 — `PlacesRepository.existsInTrip`, same H1 fix and reasoning as
|
||||
* `dayExists` above: `createAssignment`'s own `toRowId(placeId)!` must
|
||||
* never disagree with this gate. `PlacesRepository.existsInTrip` itself
|
||||
* narrowed its `id` parameter to `number` for this exact reason
|
||||
* (`Places.repository.ts`'s own docstring) — Plan 3c Task 1 originally
|
||||
* landed the method, Task 4 repointed this call site and applied the H1
|
||||
* fix in the same change.
|
||||
*/
|
||||
async placeExists(placeId: unknown, tripId: string | number) {
|
||||
const row = await this.placesRepo
|
||||
.qb('p')
|
||||
.select(['p.id'])
|
||||
.where('p.id = ? AND p.trip_id = ?', [placeId, tripId])
|
||||
.execute<{ id: number } | undefined>('get', false);
|
||||
return !!row;
|
||||
const id = toRowId(placeId);
|
||||
if (id === null) return false;
|
||||
return await this.placesRepo.existsInTrip(id, Number(tripId));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -233,9 +252,20 @@ export class AssignmentsService {
|
||||
return await this.getAssignmentWithPlace(insertedId);
|
||||
}
|
||||
|
||||
/** AS9 — `DayAssignmentsRepository.existsInDay`. Raw-bind pass-through, unchanged. */
|
||||
/**
|
||||
* AS9 — `DayAssignmentsRepository.existsInDay`, `id` `toRowId`'d INSIDE
|
||||
* this gate (Task 3 review H1 fix, absorbed in Task 4): `deleteAssignment`
|
||||
* (the only write behind this gate, `AssignmentsController.remove`) converts
|
||||
* `id` with its own `toRowId(id)!` — this must reject exactly what that
|
||||
* rejects. `dayId`/`tripId` keep the legacy's own raw-bind scoping
|
||||
* (`DayAssignmentsRepository.existsInDay`'s own `number | string`
|
||||
* parameters, unchanged): `deleteAssignment` never reads either back, so
|
||||
* there is no gate-vs-write id to disagree on for them.
|
||||
*/
|
||||
async assignmentExistsInDay(id: string | number, dayId: string | number, tripId: string | number) {
|
||||
return await this.dayAssignmentsRepo.existsInDay(id, dayId, tripId);
|
||||
const rowId = toRowId(id);
|
||||
if (rowId === null) return false;
|
||||
return await this.dayAssignmentsRepo.existsInDay(rowId, dayId, tripId);
|
||||
}
|
||||
|
||||
/** AS10 — `DayAssignmentsRepository.deleteById`. */
|
||||
@@ -257,9 +287,20 @@ export class AssignmentsService {
|
||||
});
|
||||
}
|
||||
|
||||
/** AS12 — `DayAssignmentsRepository.findInTrip`. Raw-bind pass-through, unchanged. */
|
||||
/**
|
||||
* AS12 — `DayAssignmentsRepository.findInTrip`, `id` `toRowId`'d INSIDE
|
||||
* this gate (Task 3 review H1 fix, absorbed in Task 4): every mutation
|
||||
* behind this gate (`updateTime`, `setEndDay`, `updateNotes`,
|
||||
* `setLegTransportMode`, `setIncomingLegTransportMode`, `setParticipants`,
|
||||
* and `ItineraryRpc.unassign` via `moveAssignment`) converts `id` with its
|
||||
* own `toRowId(id)!` — this must reject exactly what those reject, the
|
||||
* same shape `assignmentExistsInDay` above uses for `deleteAssignment`.
|
||||
* `tripId` keeps the legacy's own raw-bind scoping, unchanged.
|
||||
*/
|
||||
async getAssignmentForTrip(id: string | number, tripId: string | number) {
|
||||
return await this.dayAssignmentsRepo.findInTrip(id, tripId);
|
||||
const rowId = toRowId(id);
|
||||
if (rowId === null) return undefined;
|
||||
return await this.dayAssignmentsRepo.findInTrip(rowId, tripId);
|
||||
}
|
||||
|
||||
async moveAssignment(id: string | number, newDayId: unknown, orderIndex: number | null | undefined) {
|
||||
|
||||
@@ -20,6 +20,23 @@ export function isUploadedPlaceImage(url: string | null | undefined): url is str
|
||||
* 'places' category, mirroring tripService.deleteOldCover. Best-effort: never
|
||||
* throws (central key validation rejects a hostile stored value; the catch
|
||||
* swallows it exactly like the old unlink guard).
|
||||
*
|
||||
* Plan 3c Task 4: `PlacesService` no longer calls this free function — it has
|
||||
* its own converted private `reclaimPlaceImage` method (`places.service.ts`),
|
||||
* which does the same job through `PlacesRepository.existsByImageUrl` for
|
||||
* PI1 and one raw `// Plan 3h` statement for PI2. This copy, and the raw `db`
|
||||
* proxy import it still needs, stay here UNCHANGED because
|
||||
* `collections.service.ts` (`src/nest/collections/`, Plan 3h's file — the
|
||||
* `collection_places` table is Plan 3h's) also imports and calls this exact
|
||||
* function on four of its own call sites, verified by grep
|
||||
* (`grep -rn "from '.*place-image'" server/src`) — contradicting the
|
||||
* inventory's stated "every caller is this service" premise
|
||||
* (`plan3c-sql-inventory.md` §8d). Converting that caller means wiring a
|
||||
* `PlacesRepository` injection into `CollectionsModule`, a change to an
|
||||
* unowned domain's DI graph out of this task's file ownership and test
|
||||
* budget — flagged for whoever next owns `src/nest/collections/**` (Plan 3h)
|
||||
* to finish, the same `// Plan 3h` carve-out pattern PI2/PP6 already use in
|
||||
* this cluster.
|
||||
*/
|
||||
export async function reclaimPlaceImage(storage: StorageService, url: string | null | undefined): Promise<void> {
|
||||
if (!isUploadedPlaceImage(url)) return;
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT,
|
||||
demoDenied, errorResult, ok,
|
||||
} from '../../nest-mcp';
|
||||
import { InjectRepository } from '@mikro-orm/nestjs';
|
||||
import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
|
||||
import {
|
||||
mapsSearchRequestSchema,
|
||||
@@ -20,11 +21,12 @@ import { AuthService } from '../auth/auth.service';
|
||||
import { AssignmentsService } from '../assignments/assignments.service';
|
||||
import { JourneyDomainService } from '../journey/journey-domain.service';
|
||||
import { noAccess, permissionDenied } from '../../mcp/tools/_shared';
|
||||
import { DatabaseService } from '../database/database.service';
|
||||
import { UnitOfWork } from '../database/unit-of-work';
|
||||
import { MapsService } from '../maps/maps.service';
|
||||
import { PlacesService } from './places.service';
|
||||
import { isDirectionsUrl } from './maps-dir.helpers';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import type { TripsRepository } from '../../db/repositories/Trips.repository';
|
||||
|
||||
function parseId(value: string | string[]): number | null {
|
||||
const n = Number(Array.isArray(value) ? value[0] : value);
|
||||
@@ -53,7 +55,7 @@ export class PlacesMcp {
|
||||
constructor(
|
||||
private readonly places: PlacesService,
|
||||
private readonly maps: MapsService,
|
||||
private readonly db: DatabaseService,
|
||||
@InjectRepository(Trips) private readonly tripsRepo: TripsRepository,
|
||||
private readonly auth: AuthService,
|
||||
private readonly journey: JourneyDomainService,
|
||||
private readonly assignments: AssignmentsService,
|
||||
@@ -97,7 +99,7 @@ export class PlacesMcp {
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
const place = await this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, amap_poi_id, notes, website, phone, image_url, price, currency, stop_type });
|
||||
this.guards.safeBroadcast(tripId, 'place:created', { place });
|
||||
@@ -142,7 +144,7 @@ export class PlacesMcp {
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
if (!(await this.assignments.dayExists(dayId, tripId))) return { content: [{ type: 'text' as const, text: 'Day not found.' }], isError: true };
|
||||
try {
|
||||
@@ -205,7 +207,7 @@ export class PlacesMcp {
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
const place = await this.places.update(String(tripId), String(placeId), { name, description, lat, lng, address, category_id, price, currency, place_time, end_time, duration_minutes, notes, website, phone, image_url, transport_mode, osm_id, google_place_id, google_ftid, amap_poi_id, stop_type, fill_percent });
|
||||
if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true };
|
||||
@@ -230,7 +232,7 @@ export class PlacesMcp {
|
||||
) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
// Rating is a personal vote — any trip member may cast one, place_edit not required.
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
const place = await this.places.rate(String(tripId), String(placeId), ctx.userId, rating ?? null);
|
||||
if (!place) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true };
|
||||
this.guards.safeBroadcast(tripId, 'place:updated', { place });
|
||||
@@ -249,7 +251,7 @@ export class PlacesMcp {
|
||||
})
|
||||
async deletePlace({ tripId, placeId }: { tripId: number; placeId: number }, ctx: McpContext) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
// Scope the id to the trip before the hook: onPlaceDeleted keys on the place
|
||||
// id alone, so a foreign id would detach that trip's journey entries even
|
||||
@@ -291,7 +293,7 @@ export class PlacesMcp {
|
||||
},
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
const places = await this.places.list(String(tripId), { search, category, tag, assignment });
|
||||
return ok({ places });
|
||||
}
|
||||
@@ -343,7 +345,7 @@ export class PlacesMcp {
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
|
||||
// Same opts the REST route builds: the enrichment pass is keyed on the calling
|
||||
@@ -374,7 +376,7 @@ export class PlacesMcp {
|
||||
})
|
||||
async importGpx(input: RoadtripGpxImport, ctx: McpContext) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(input.tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(input.tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', input.tripId, ctx.userId))) return permissionDenied();
|
||||
if (!input.importWaypoints && !input.importRoutes && !input.importTracks) return errorResult('No import types selected.');
|
||||
try {
|
||||
@@ -406,7 +408,7 @@ export class PlacesMcp {
|
||||
ctx: McpContext,
|
||||
) {
|
||||
// A read, like the REST route: seeing the trip is enough, no place_edit.
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!waypoints && !tracks && !dayRoutes) return errorResult('No export types selected.');
|
||||
const result = await this.places.exportGpx(String(tripId), { waypoints, tracks, dayRoutes });
|
||||
if (!result) return errorResult('Nothing to export.');
|
||||
@@ -425,7 +427,7 @@ export class PlacesMcp {
|
||||
})
|
||||
async bulkDeletePlaces({ tripId, placeIds }: { tripId: number; placeIds: number[] }, ctx: McpContext) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
|
||||
// Trip-scoped, and ahead of the DELETE: journey_entries.source_place_id is
|
||||
@@ -482,7 +484,7 @@ export class PlacesMcp {
|
||||
ctx: McpContext,
|
||||
) {
|
||||
if (await this.auth.isDemoUser(ctx.userId)) return demoDenied();
|
||||
if (!(await this.db.canAccessTrip(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.tripsRepo.findAccessible(tripId, ctx.userId))) return noAccess();
|
||||
if (!(await this.guards.hasTripPermission('place_edit', tripId, ctx.userId))) return permissionDenied();
|
||||
|
||||
const fields = { category_id, price, currency, transport_mode, place_time, end_time, duration_minutes, notes, website, phone, image_url, description, stop_type, fill_percent };
|
||||
@@ -504,7 +506,7 @@ export class PlacesMcp {
|
||||
})
|
||||
async tripPlacesResource(uri: URL, { tripId }: { tripId: string | string[] }, ctx: McpContext) {
|
||||
const id = parseId(tripId);
|
||||
if (id === null || !(await this.db.canAccessTrip(id, ctx.userId))) {
|
||||
if (id === null || !(await this.tripsRepo.findAccessible(id, ctx.userId))) {
|
||||
return {
|
||||
contents: [{
|
||||
uri: uri.href,
|
||||
|
||||
@@ -1,4 +1,11 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { MikroOrmModule } from '@mikro-orm/nestjs';
|
||||
import { Places } from '../../db/entities/Places.entity';
|
||||
import { Tags } from '../../db/entities/Tags.entity';
|
||||
import { PlaceRatings } from '../../db/entities/PlaceRatings.entity';
|
||||
import { TripMembers } from '../../db/entities/TripMembers.entity';
|
||||
import { DayAssignments } from '../../db/entities/DayAssignments.entity';
|
||||
import { Trips } from '../../db/entities/Trips.entity';
|
||||
import { JourneyDomainModule } from '../journey/journey-domain.module';
|
||||
import { PlacesController } from './places.controller';
|
||||
import { PlacesService } from './places.service';
|
||||
@@ -45,7 +52,17 @@ import { MAX_PLACE_IMAGE_SIZE } from '../common/place-image-upload';
|
||||
StorageModule,
|
||||
// AccommodationsDomainModule: deleting a place takes the nights booked at it with
|
||||
// it, and the cascade behind a stay belongs to the domain that owns it.
|
||||
McpSharedModule, PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule, AssignmentsDomainModule, AccommodationsDomainModule],
|
||||
McpSharedModule, PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule, JourneyDomainModule, RealtimeModule, PluginGuardsModule, AssignmentsDomainModule, AccommodationsDomainModule,
|
||||
// Plan 3c Task 4: `PlacesService`'s and `PlacesMcp`'s own `@InjectRepository`
|
||||
// constructor params (`PlacesRepository`, `TagsRepository`,
|
||||
// `PlaceRatingsRepository`, `TripMembersRepository`,
|
||||
// `DayAssignmentsRepository`, `TripsRepository`) — registered directly on
|
||||
// THIS module (not a sibling domain module's `exports`), matching
|
||||
// `AssignmentsDomainModule`'s own `MikroOrmModule.forFeature(...)`
|
||||
// precedent: a `forFeature` registration only reaches providers declared
|
||||
// in the SAME module.
|
||||
MikroOrmModule.forFeature([Places, Tags, PlaceRatings, TripMembers, DayAssignments, Trips]),
|
||||
],
|
||||
controllers: [PlacesController],
|
||||
providers: [PlacesService, PlacesMcp, PlacesRpc],
|
||||
exports: [PlacesService],
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import path from 'node:path';
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { InjectRepository } from '@mikro-orm/nestjs';
|
||||
import { XMLValidator } from 'fast-xml-parser';
|
||||
import { TRACK_COLORS, placeMatchStrategies, type PlaceMatchCandidate } from '@trek/shared';
|
||||
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
|
||||
@@ -9,7 +11,8 @@ import type { PlaceWithTags } from '../database/database.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
import { MapsService, GOOGLE_SHORT_HOSTS, isGoogleMapsHost } from '../maps/maps.service';
|
||||
import { isDirectionsUrl, parseDirectionsUrl } from './maps-dir.helpers';
|
||||
import type { Place, User } from '../../types';
|
||||
import { toRowId } from '../common/row-id';
|
||||
import type { User } from '../../types';
|
||||
import { QueryHelpersService } from '../query-helpers/query-helpers.service';
|
||||
import { ratingAggregate } from '../common/rowShape';
|
||||
import { checkSsrf, safeFetchFollow, SsrfBlockedError } from '../../utils/ssrfGuard';
|
||||
@@ -26,10 +29,20 @@ import type { GpxExportDay, GpxExportOptions, GpxExportPlace } from './gpx-expor
|
||||
import { UnsplashService } from '../unsplash/unsplash.service';
|
||||
import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service';
|
||||
import { type UpdateConflict, isUpdateConflict } from '../common/conflictResult';
|
||||
import { reclaimPlaceImage } from './place-image';
|
||||
import { isUploadedPlaceImage } from './place-image';
|
||||
import { JourneyDomainService } from '../journey/journey-domain.service';
|
||||
import { StorageService } from '../storage/storage.service';
|
||||
import { AccommodationsService } from '../accommodations/accommodations.service';
|
||||
import { Places } from '../../db/entities/Places.entity';
|
||||
import { Tags } from '../../db/entities/Tags.entity';
|
||||
import { PlaceRatings } from '../../db/entities/PlaceRatings.entity';
|
||||
import { TripMembers } from '../../db/entities/TripMembers.entity';
|
||||
import { DayAssignments } from '../../db/entities/DayAssignments.entity';
|
||||
import type { PlacesRepository, PlaceWithCategoryRow } from '../../db/repositories/Places.repository';
|
||||
import type { TagsRepository } from '../../db/repositories/Tags.repository';
|
||||
import type { PlaceRatingsRepository } from '../../db/repositories/PlaceRatings.repository';
|
||||
import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository';
|
||||
import type { DayAssignmentsRepository } from '../../db/repositories/DayAssignments.repository';
|
||||
|
||||
/** Rows a place delete took down with the nights booked there, for the caller to announce. */
|
||||
export interface CancelledStays {
|
||||
@@ -64,7 +77,6 @@ import {
|
||||
type ListImportOptions,
|
||||
type ListImportResult,
|
||||
type PlaceImportResult,
|
||||
type PlaceWithCategory,
|
||||
} from './places.helpers';
|
||||
|
||||
type Trip = TripAccess;
|
||||
@@ -135,6 +147,11 @@ export class PlacesService {
|
||||
private readonly storage: StorageService,
|
||||
private readonly accommodations: AccommodationsService,
|
||||
private readonly uow: UnitOfWork,
|
||||
@InjectRepository(Places) private readonly placesRepo: PlacesRepository,
|
||||
@InjectRepository(Tags) private readonly tagsRepo: TagsRepository,
|
||||
@InjectRepository(PlaceRatings) private readonly placeRatingsRepo: PlaceRatingsRepository,
|
||||
@InjectRepository(TripMembers) private readonly tripMembersRepo: TripMembersRepository,
|
||||
@InjectRepository(DayAssignments) private readonly dayAssignmentsRepo: DayAssignmentsRepository,
|
||||
) {}
|
||||
|
||||
async verifyTripAccess(tripId: string, userId: number) {
|
||||
@@ -160,11 +177,13 @@ export class PlacesService {
|
||||
private async tagsOnTrip(tripId: string | number, tagIds: number[]): Promise<number[]> {
|
||||
const unique = [...new Set(tagIds)];
|
||||
if (unique.length === 0) return [];
|
||||
const roster = await this.dbs.rosterUserIds(tripId);
|
||||
const owned = this.dbs.all<{ id: number; user_id: number }>(
|
||||
`SELECT id, user_id FROM tags WHERE id IN (${unique.map(() => '?').join(',')})`,
|
||||
...unique,
|
||||
);
|
||||
// PL1 — `TripMembersRepository.rosterUserIds`, injected directly (not
|
||||
// through `DatabaseService`), matching `AssignmentsService`'s AS28
|
||||
// precedent: each domain converts its own `DatabaseService`-delegated
|
||||
// callers as it lands.
|
||||
const roster = await this.tripMembersRepo.rosterUserIds(tripId);
|
||||
// PL2 — `SELECT id, user_id FROM tags WHERE id IN (${…})`.
|
||||
const owned = await this.tagsRepo.findByIds(unique);
|
||||
return owned.filter(t => roster.has(t.user_id)).map(t => t.id);
|
||||
}
|
||||
|
||||
@@ -172,6 +191,37 @@ export class PlacesService {
|
||||
this.realtime.broadcast(tripId, event, payload, socketId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a custom place-image object once nothing references it any more
|
||||
* (moved in from `place-image.ts`'s free function `reclaimPlaceImage`,
|
||||
* option (a) — see `place-image.ts`'s own docstring for why that free
|
||||
* function stays, unconverted, for `collections.service.ts`'s own call
|
||||
* sites). A trip place and a collection saved-place can share the same
|
||||
* uploaded file — save-to-collection and copy-to-trip copy `image_url` by
|
||||
* reference — so this ref-counts across both tables before deleting.
|
||||
*
|
||||
* PI1 (`SELECT 1 FROM places WHERE image_url = ? LIMIT 1`) through
|
||||
* `PlacesRepository.existsByImageUrl`; PI2 (`SELECT 1 FROM collection_places
|
||||
* WHERE image_url = ? LIMIT 1`, Plan 3h's table) stays one raw statement,
|
||||
* evaluated only when PI1 is false — reproducing the legacy `UNION ALL …
|
||||
* LIMIT 1`'s short-circuit by evaluation order, the same pattern
|
||||
* `PlacePhotoCacheService.isReferenced` uses for the same two tables
|
||||
* (Plan 3c Task 1's PP6 ruling). `path.basename()` keeps the storage name
|
||||
* confined to the 'places' category. Best-effort: never throws (central
|
||||
* key validation rejects a hostile stored value; the catch swallows it
|
||||
* exactly like the legacy unlink guard).
|
||||
*/
|
||||
private async reclaimPlaceImage(url: string | null | undefined): Promise<void> {
|
||||
if (!isUploadedPlaceImage(url)) return;
|
||||
if (await this.placesRepo.existsByImageUrl(url)) return;
|
||||
// Plan 3h: collection_places is not yet repository-backed.
|
||||
const referenced = this.dbs.get('SELECT 1 FROM collection_places WHERE image_url = ? LIMIT 1', url);
|
||||
if (referenced) return;
|
||||
await this.storage.delete('places', path.basename(url)).catch(() => {
|
||||
/* best-effort */
|
||||
});
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// List places
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -180,43 +230,17 @@ export class PlacesService {
|
||||
tripId: string,
|
||||
filters: { search?: string; category?: string; tag?: string; assignment?: 'all' | 'unassigned' | 'assigned' },
|
||||
) {
|
||||
let query = `
|
||||
SELECT DISTINCT p.*, c.name as category_name, c.color as category_color, c.icon as category_icon
|
||||
FROM places p
|
||||
LEFT JOIN categories c ON p.category_id = c.id
|
||||
WHERE p.trip_id = ?
|
||||
`;
|
||||
const params: (string | number)[] = [tripId];
|
||||
|
||||
if (filters.search) {
|
||||
// ESCAPE so a `%` or `_` the user typed matches literally instead of
|
||||
// acting as a LIKE wildcard (a bare '%' used to return the whole trip).
|
||||
query += " AND (p.name LIKE ? ESCAPE '\\' OR p.address LIKE ? ESCAPE '\\' OR p.description LIKE ? ESCAPE '\\')";
|
||||
const searchParam = `%${escapeLikePattern(filters.search)}%`;
|
||||
params.push(searchParam, searchParam, searchParam);
|
||||
}
|
||||
|
||||
if (filters.category) {
|
||||
query += ' AND p.category_id = ?';
|
||||
params.push(filters.category);
|
||||
}
|
||||
|
||||
if (filters.tag) {
|
||||
query += ' AND p.id IN (SELECT place_id FROM place_tags WHERE tag_id = ?)';
|
||||
params.push(filters.tag);
|
||||
}
|
||||
|
||||
if (filters.assignment === 'unassigned') {
|
||||
query += ` AND p.id NOT IN (SELECT da.place_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE d.trip_id = ?)`;
|
||||
params.push(tripId);
|
||||
} else if (filters.assignment === 'assigned') {
|
||||
query += ` AND p.id IN (SELECT da.place_id FROM day_assignments da JOIN days d ON da.day_id = d.id WHERE d.trip_id = ?)`;
|
||||
params.push(tripId);
|
||||
}
|
||||
|
||||
query += ' ORDER BY p.created_at DESC';
|
||||
|
||||
const places = this.dbs.prepare(query).all(...params) as PlaceWithCategory[];
|
||||
// ESCAPE so a `%` or `_` the user typed matches literally instead of
|
||||
// acting as a LIKE wildcard (a bare '%' used to return the whole trip) —
|
||||
// the coercion stays here (the service), the repository binds the
|
||||
// already-built pattern verbatim (PL3).
|
||||
const searchPattern = filters.search ? `%${escapeLikePattern(filters.search)}%` : undefined;
|
||||
const places: PlaceWithCategoryRow[] = await this.placesRepo.listForTrip(tripId, {
|
||||
searchPattern,
|
||||
category: filters.category,
|
||||
tag: filters.tag,
|
||||
assignment: filters.assignment,
|
||||
});
|
||||
|
||||
const placeIds = places.map(p => p.id);
|
||||
const tagsByPlaceId = await this.queryHelpers.loadTagsByPlaceIds(placeIds);
|
||||
@@ -248,42 +272,51 @@ export class PlacesService {
|
||||
transport_mode, route_geometry, route_color, stop_type, fill_percent, tags = [],
|
||||
} = body;
|
||||
|
||||
const result = this.dbs.run(`
|
||||
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, amap_poi_id, website, phone, transport_mode,
|
||||
route_geometry, route_color, stop_type, fill_percent)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`,
|
||||
// lat/lng/price/duration_minutes use an explicit undefined check, not `||`:
|
||||
// 0 is a legitimate value for all four (Null Island, a free entry, a
|
||||
// drive-by stop) and the falsy coercion silently threw it away.
|
||||
//
|
||||
// The hour stays the default for a place created without one, which is what it
|
||||
// has always been and what the column itself declares. The road trip rail reads
|
||||
// the value as a stay, and a stop added from the corridor search brings its own
|
||||
// figure — ten minutes for fuel, twenty for a rest area — so the kinds that would
|
||||
// be misread as an hour never take the default in the first place.
|
||||
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, amap_poi_id || null, website || null, phone || null, transport_mode || 'walking',
|
||||
route_geometry || null, route_color || null, stop_type || null,
|
||||
// `?? null` rather than `|| null`, the same reason lat/lng have it: the column is a
|
||||
// percentage and the falsy check would be a silent floor.
|
||||
fill_percent ?? null,
|
||||
);
|
||||
|
||||
const placeId = result.lastInsertRowid;
|
||||
// PL4 — the 25-column INSERT. lat/lng/price/duration_minutes/fill_percent
|
||||
// use an explicit undefined check, not `||`: 0 is a legitimate value for
|
||||
// all five (Null Island, a free entry, a drive-by stop, an empty tank)
|
||||
// and the falsy coercion silently threw it away.
|
||||
//
|
||||
// The hour stays the default for a place created without one, which is what it
|
||||
// has always been and what the column itself declares. The road trip rail reads
|
||||
// the value as a stay, and a stop added from the corridor search brings its own
|
||||
// figure — ten minutes for fuel, twenty for a rest area — so the kinds that would
|
||||
// be misread as an hour never take the default in the first place.
|
||||
const placeId = await this.placesRepo.insertPlace({
|
||||
trip_id: Number(tripId),
|
||||
name,
|
||||
description: description || null,
|
||||
lat: lat ?? null,
|
||||
lng: lng ?? null,
|
||||
address: address || null,
|
||||
category_id: category_id || null,
|
||||
price: price ?? null,
|
||||
currency: currency || null,
|
||||
place_time: place_time || null,
|
||||
end_time: end_time || null,
|
||||
duration_minutes: duration_minutes ?? 60,
|
||||
notes: notes || null,
|
||||
image_url: image_url || null,
|
||||
google_place_id: google_place_id || null,
|
||||
google_ftid: google_ftid || null,
|
||||
osm_id: osm_id || null,
|
||||
amap_poi_id: amap_poi_id || null,
|
||||
website: website || null,
|
||||
phone: phone || null,
|
||||
transport_mode: transport_mode || 'walking',
|
||||
route_geometry: route_geometry || null,
|
||||
route_color: route_color || null,
|
||||
stop_type: stop_type || null,
|
||||
fill_percent: fill_percent ?? null,
|
||||
});
|
||||
|
||||
// PL5 — `INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)`.
|
||||
if (tags && tags.length > 0) {
|
||||
const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)');
|
||||
for (const tagId of await this.tagsOnTrip(tripId, tags)) {
|
||||
insertTag.run(placeId, tagId);
|
||||
}
|
||||
await this.tagsRepo.insertIgnore(placeId, await this.tagsOnTrip(tripId, tags));
|
||||
}
|
||||
|
||||
return (await this.dbs.getPlaceWithTags(Number(placeId)))!;
|
||||
// PL6 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
|
||||
return (await this.placesRepo.findWithTagsAndRatings(placeId))!;
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -291,9 +324,16 @@ export class PlacesService {
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
async get(tripId: string, placeId: string) {
|
||||
const placeCheck = this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId);
|
||||
if (!placeCheck) return null;
|
||||
return await this.dbs.getPlaceWithTags(placeId);
|
||||
// PL7 — the existence gate. `toRowId` first (Task 3 review H1, absorbed
|
||||
// here): a non-canonical id must resolve to "not found" here, the same
|
||||
// answer `findWithTagsAndRatings` below would give it anyway — this
|
||||
// just skips the trip-scoped existence read for an id that can never
|
||||
// exist.
|
||||
const id = toRowId(placeId);
|
||||
if (id === null) return null;
|
||||
if (!(await this.placesRepo.existsInTrip(id, Number(tripId)))) return null;
|
||||
// PL8 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
|
||||
return await this.placesRepo.findWithTagsAndRatings(id);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -307,7 +347,7 @@ export class PlacesService {
|
||||
ifMatch?: string,
|
||||
): Promise<PlaceWithTags | UpdateConflict | null> {
|
||||
const { result, reclaim } = await this.applyUpdate(tripId, placeId, body, ifMatch);
|
||||
if (reclaim !== undefined) await reclaimPlaceImage(this.storage, reclaim);
|
||||
if (reclaim !== undefined) await this.reclaimPlaceImage(reclaim);
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -322,14 +362,24 @@ export class PlacesService {
|
||||
body: PlaceUpdateInput,
|
||||
ifMatch?: string,
|
||||
): Promise<{ result: PlaceWithTags | UpdateConflict | null; reclaim?: string | null }> {
|
||||
const existingPlace = this.dbs.get<Place>('SELECT * FROM places WHERE id = ? AND trip_id = ?', placeId, tripId);
|
||||
// PL9 — the pre-image read every `!== undefined` fallback below needs.
|
||||
// `toRowId` first (Task 3 review H1, absorbed here): every write in
|
||||
// this method (`updatePlace`, `tagsRepo.deleteForPlace`/`insertIgnore`)
|
||||
// uses this SAME validated `id` — a non-canonical `placeId` (`"3 "`,
|
||||
// `"3.0"`) must resolve to "not found" HERE, not pass a loose SQLite
|
||||
// affinity match and then hit a downstream write keyed on a `toRowId(x)!`
|
||||
// that returns `null`.
|
||||
const id = toRowId(placeId);
|
||||
if (id === null) return { result: null };
|
||||
const existingPlace = await this.placesRepo.findInTrip(id, Number(tripId));
|
||||
if (!existingPlace) return { result: null };
|
||||
|
||||
// Optimistic concurrency (#1135): when the caller sent the version it based its
|
||||
// edit on and the row has moved on since, reject instead of clobbering. Absent
|
||||
// token => unconditional update (back-compat — old clients keep last-write-wins).
|
||||
if (ifMatch !== undefined && existingPlace.updated_at != null && String(existingPlace.updated_at) !== ifMatch) {
|
||||
return { result: { conflict: true, server: await this.dbs.getPlaceWithTags(placeId) } };
|
||||
// PL10 — the `{conflict: true, server}` body when `If-Match` loses (#1135).
|
||||
return { result: { conflict: true, server: await this.placesRepo.findWithTagsAndRatings(id) } };
|
||||
}
|
||||
|
||||
const {
|
||||
@@ -339,75 +389,58 @@ export class PlacesService {
|
||||
transport_mode, route_color, stop_type, fill_percent, tags,
|
||||
} = body;
|
||||
|
||||
this.dbs.run(`
|
||||
UPDATE places SET
|
||||
name = COALESCE(?, name),
|
||||
description = ?,
|
||||
lat = ?,
|
||||
lng = ?,
|
||||
address = ?,
|
||||
category_id = ?,
|
||||
price = ?,
|
||||
currency = COALESCE(?, currency),
|
||||
place_time = ?,
|
||||
end_time = ?,
|
||||
duration_minutes = ?,
|
||||
notes = ?,
|
||||
image_url = ?,
|
||||
google_place_id = ?,
|
||||
google_ftid = ?,
|
||||
osm_id = ?,
|
||||
amap_poi_id = ?,
|
||||
website = ?,
|
||||
phone = ?,
|
||||
transport_mode = COALESCE(?, transport_mode),
|
||||
route_color = ?,
|
||||
stop_type = ?,
|
||||
fill_percent = ?,
|
||||
updated_at = CURRENT_TIMESTAMP
|
||||
WHERE id = ?
|
||||
`,
|
||||
name || null,
|
||||
description !== undefined ? description : existingPlace.description,
|
||||
lat !== undefined ? lat : existingPlace.lat,
|
||||
lng !== undefined ? lng : existingPlace.lng,
|
||||
address !== undefined ? address : existingPlace.address,
|
||||
category_id !== undefined ? category_id : existingPlace.category_id,
|
||||
price !== undefined ? price : existingPlace.price,
|
||||
currency || null,
|
||||
place_time !== undefined ? place_time : existingPlace.place_time,
|
||||
end_time !== undefined ? end_time : existingPlace.end_time,
|
||||
// PL11 — ONE `nativeUpdate` with a typed full partial (the ruling): the
|
||||
// SQL `COALESCE(?, col)` keep-if-null semantics (name/currency/
|
||||
// transport_mode, all three bound `x || null`) and the twenty
|
||||
// `!== undefined ? x : existing.x` pre-image fallbacks are resolved to
|
||||
// their final value HERE, in the service — the repository writes
|
||||
// exactly what it is handed.
|
||||
await this.placesRepo.updatePlace(id, {
|
||||
// COALESCE(?, name): the bound value is `name || null` (falsy clears
|
||||
// to null), and `?? existingPlace.name` is the COALESCE half itself —
|
||||
// a `nativeUpdate` writes a literal value, it does not evaluate SQL
|
||||
// COALESCE, so the keep-if-null fold has to be resolved here.
|
||||
name: (name || null) ?? existingPlace.name,
|
||||
description: description !== undefined ? description : existingPlace.description,
|
||||
lat: lat !== undefined ? lat : existingPlace.lat,
|
||||
lng: lng !== undefined ? lng : existingPlace.lng,
|
||||
address: address !== undefined ? address : existingPlace.address,
|
||||
category_id: category_id !== undefined ? category_id : existingPlace.category_id,
|
||||
price: price !== undefined ? price : existingPlace.price,
|
||||
// COALESCE(?, currency) — same fold as `name` above.
|
||||
currency: (currency || null) ?? existingPlace.currency,
|
||||
place_time: place_time !== undefined ? place_time : existingPlace.place_time,
|
||||
end_time: end_time !== undefined ? end_time : existingPlace.end_time,
|
||||
// Not COALESCE, like its neighbours: the contract says an explicit null
|
||||
// clears the planned stay length, and COALESCE made null and absent the
|
||||
// same thing, so the field advertised a reset it never performed. 0 keeps
|
||||
// working, which a `|| null` bind would have swallowed.
|
||||
duration_minutes !== undefined ? duration_minutes : existingPlace.duration_minutes,
|
||||
notes !== undefined ? notes : existingPlace.notes,
|
||||
image_url !== undefined ? image_url : existingPlace.image_url,
|
||||
google_place_id !== undefined ? google_place_id : existingPlace.google_place_id,
|
||||
google_ftid !== undefined ? google_ftid : existingPlace.google_ftid,
|
||||
osm_id !== undefined ? osm_id : existingPlace.osm_id,
|
||||
amap_poi_id !== undefined ? amap_poi_id : existingPlace.amap_poi_id,
|
||||
website !== undefined ? website : existingPlace.website,
|
||||
phone !== undefined ? phone : existingPlace.phone,
|
||||
transport_mode || null,
|
||||
duration_minutes: duration_minutes !== undefined ? duration_minutes : existingPlace.duration_minutes,
|
||||
notes: notes !== undefined ? notes : existingPlace.notes,
|
||||
image_url: image_url !== undefined ? image_url : existingPlace.image_url,
|
||||
google_place_id: google_place_id !== undefined ? google_place_id : existingPlace.google_place_id,
|
||||
google_ftid: google_ftid !== undefined ? google_ftid : existingPlace.google_ftid,
|
||||
osm_id: osm_id !== undefined ? osm_id : existingPlace.osm_id,
|
||||
amap_poi_id: amap_poi_id !== undefined ? amap_poi_id : existingPlace.amap_poi_id,
|
||||
website: website !== undefined ? website : existingPlace.website,
|
||||
phone: phone !== undefined ? phone : existingPlace.phone,
|
||||
// COALESCE(?, transport_mode) — same fold as `name`/`currency` above.
|
||||
transport_mode: (transport_mode || null) ?? existingPlace.transport_mode,
|
||||
// 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,
|
||||
route_color: route_color !== undefined ? route_color : existingPlace.route_color,
|
||||
// Same shape: an explicit null is how a fuel stop becomes an ordinary place again.
|
||||
stop_type !== undefined ? stop_type : existingPlace.stop_type,
|
||||
stop_type: stop_type !== undefined ? stop_type : existingPlace.stop_type,
|
||||
// And how a stop that had its own fill amount goes back to following the setting.
|
||||
fill_percent !== undefined ? fill_percent : existingPlace.fill_percent,
|
||||
placeId,
|
||||
);
|
||||
fill_percent: fill_percent !== undefined ? fill_percent : existingPlace.fill_percent,
|
||||
});
|
||||
|
||||
if (tags !== undefined) {
|
||||
this.dbs.run('DELETE FROM place_tags WHERE place_id = ?', placeId);
|
||||
// PL12 — `DELETE FROM place_tags WHERE place_id = ?`.
|
||||
await this.tagsRepo.deleteForPlace(id);
|
||||
if (tags.length > 0) {
|
||||
const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)');
|
||||
for (const tagId of await this.tagsOnTrip(tripId, tags)) {
|
||||
insertTag.run(placeId, tagId);
|
||||
}
|
||||
// PL13 — `INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)`.
|
||||
await this.tagsRepo.insertIgnore(id, await this.tagsOnTrip(tripId, tags));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -418,7 +451,8 @@ export class PlacesService {
|
||||
? existingPlace.image_url
|
||||
: undefined;
|
||||
|
||||
return { result: await this.dbs.getPlaceWithTags(placeId), reclaim };
|
||||
// PL14 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
|
||||
return { result: await this.placesRepo.findWithTagsAndRatings(id), reclaim };
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -432,6 +466,7 @@ export class PlacesService {
|
||||
*/
|
||||
async linkedExpenseIds(tripId: string | number, placeIds: Array<string | number>): Promise<number[]> {
|
||||
if (placeIds.length === 0) return [];
|
||||
// PL15 — `budget_items` is Plan 3e's table; stays raw.
|
||||
const rows = this.dbs.all<{ id: number }>(
|
||||
`SELECT id FROM budget_items WHERE trip_id = ? AND place_id IN (${placeIds.map(() => '?').join(',')})`,
|
||||
tripId, ...placeIds,
|
||||
@@ -452,6 +487,9 @@ export class PlacesService {
|
||||
* Runs inside the caller's transaction.
|
||||
*/
|
||||
private async cancelStaysAt(tripId: string | number, placeId: string | number, into: CancelledStays): Promise<void> {
|
||||
// PL16 — `day_accommodations` is Plan 3d's table; stays raw. Runs inside
|
||||
// the caller's own transaction (R4 — Plan 3d must not re-open or
|
||||
// re-scope this transaction when it converts `accommodations`).
|
||||
const stays = this.dbs.all<{ id: number }>(
|
||||
'SELECT id FROM day_accommodations WHERE trip_id = ? AND place_id = ?', tripId, placeId,
|
||||
);
|
||||
@@ -467,39 +505,47 @@ export class PlacesService {
|
||||
}
|
||||
|
||||
async remove(tripId: string, placeId: string): Promise<{ deleted: boolean; cancelled: CancelledStays }> {
|
||||
const place = this.dbs.get<{ google_place_id: string | null; image_url: string | null }>(
|
||||
'SELECT google_place_id, image_url FROM places WHERE id = ? AND trip_id = ?', placeId, tripId,
|
||||
);
|
||||
const cancelled = noCancelledStays();
|
||||
// `toRowId` first (Task 3 review H1, absorbed here): the write below
|
||||
// (`deleteById`) must use the SAME id this gate's existence read used.
|
||||
const id = toRowId(placeId);
|
||||
if (id === null) return { deleted: false, cancelled };
|
||||
// PL17 — the reclaim-candidate projection, read before the delete.
|
||||
const place = await this.placesRepo.reclaimInputs(id, Number(tripId));
|
||||
if (!place) return { deleted: false, cancelled };
|
||||
// The linked expense goes with the place, the same way a booking takes its
|
||||
// expense with it (#1298). One transaction, so a place can never survive
|
||||
// half-detached from its money.
|
||||
await this.uow.transactional(async () => {
|
||||
await this.cancelStaysAt(tripId, placeId, cancelled);
|
||||
this.dbs.run('DELETE FROM budget_items WHERE trip_id = ? AND place_id = ?', tripId, placeId);
|
||||
this.dbs.run('DELETE FROM places WHERE id = ?', placeId);
|
||||
await this.cancelStaysAt(tripId, id, cancelled);
|
||||
// PL18 — `budget_items` is Plan 3e's table; stays raw.
|
||||
this.dbs.run('DELETE FROM budget_items WHERE trip_id = ? AND place_id = ?', tripId, id);
|
||||
// PL19 — `DELETE FROM places WHERE id = ?`.
|
||||
await this.placesRepo.deleteById(id);
|
||||
});
|
||||
await reclaimPhotoCache(this.photoCache, place.google_place_id, place.image_url);
|
||||
await reclaimPlaceImage(this.storage, place.image_url);
|
||||
await this.reclaimPlaceImage(place.image_url);
|
||||
return { deleted: true, cancelled };
|
||||
}
|
||||
|
||||
async removeMany(tripId: string, ids: number[]): Promise<{ deleted: number[]; cancelled: CancelledStays }> {
|
||||
const cancelled = noCancelledStays();
|
||||
if (ids.length === 0) return { deleted: [], cancelled };
|
||||
const selectStmt = this.dbs.prepare('SELECT google_place_id, image_url FROM places WHERE id = ? AND trip_id = ?');
|
||||
const deleteStmt = this.dbs.prepare('DELETE FROM places WHERE id = ?');
|
||||
const deleteExpenseStmt = this.dbs.prepare('DELETE FROM budget_items WHERE trip_id = ? AND place_id = ?');
|
||||
const deleted: number[] = [];
|
||||
const reclaimable: { google_place_id: string | null; image_url: string | null }[] = [];
|
||||
await this.uow.transactional(async () => {
|
||||
for (const id of ids) {
|
||||
const row = selectStmt.get(id, tripId) as { google_place_id: string | null; image_url: string | null } | undefined;
|
||||
// PL20 — the reclaim-candidate projection, read before the delete
|
||||
// (same statement as PL17). `id` is already a genuine `number`
|
||||
// here (a body-validated array, not a route string) — no `toRowId`
|
||||
// gate needed, the H1 class of bug is a route-string problem.
|
||||
const row = await this.placesRepo.reclaimInputs(id, Number(tripId));
|
||||
if (!row) continue;
|
||||
await this.cancelStaysAt(tripId, id, cancelled);
|
||||
deleteExpenseStmt.run(tripId, id);
|
||||
deleteStmt.run(id);
|
||||
// PL22 — `budget_items` is Plan 3e's table; stays raw.
|
||||
this.dbs.run('DELETE FROM budget_items WHERE trip_id = ? AND place_id = ?', tripId, id);
|
||||
// PL21 — `DELETE FROM places WHERE id = ?`.
|
||||
await this.placesRepo.deleteById(id);
|
||||
deleted.push(id);
|
||||
reclaimable.push(row);
|
||||
}
|
||||
@@ -507,7 +553,7 @@ export class PlacesService {
|
||||
// Reclaim after the transaction commits so isReferenced() sees the final place set.
|
||||
for (const row of reclaimable) {
|
||||
await reclaimPhotoCache(this.photoCache, row.google_place_id, row.image_url);
|
||||
await reclaimPlaceImage(this.storage, row.image_url);
|
||||
await this.reclaimPlaceImage(row.image_url);
|
||||
}
|
||||
return { deleted, cancelled };
|
||||
}
|
||||
@@ -519,13 +565,8 @@ export class PlacesService {
|
||||
* that trip's journey entries even though the delete itself refuses it.
|
||||
*/
|
||||
async scopedIds(tripId: string, ids: number[]): Promise<number[]> {
|
||||
if (ids.length === 0) return [];
|
||||
const placeholders = ids.map(() => '?').join(',');
|
||||
const rows = this.dbs.all<{ id: number }>(
|
||||
`SELECT id FROM places WHERE trip_id = ? AND id IN (${placeholders})`, tripId, ...ids,
|
||||
);
|
||||
const owned = new Set(rows.map((r) => r.id));
|
||||
return ids.filter((id) => owned.has(id));
|
||||
// PL23 — input-order preservation lives in the repository method now.
|
||||
return this.placesRepo.scopedIds(tripId, ids);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -553,7 +594,7 @@ export class PlacesService {
|
||||
});
|
||||
// Settle reclaims after the transaction commits, so the refcount sees the
|
||||
// final image_url state.
|
||||
for (const reclaim of reclaims) await reclaimPlaceImage(this.storage, reclaim);
|
||||
for (const reclaim of reclaims) await this.reclaimPlaceImage(reclaim);
|
||||
return updated;
|
||||
}
|
||||
|
||||
@@ -630,29 +671,18 @@ export class PlacesService {
|
||||
for (const strategy of placeMatchStrategies(place)) {
|
||||
let hit: { id: number; google_ftid: string | null } | undefined;
|
||||
if (strategy.by === 'externalId') {
|
||||
hit = this.dbs.get<{ id: number; google_ftid: string | null }>(`
|
||||
SELECT id, google_ftid FROM places
|
||||
WHERE trip_id = ? AND (google_place_id = ? OR google_ftid = ? OR osm_id = ? OR amap_poi_id = ?)
|
||||
ORDER BY id ASC
|
||||
LIMIT 1
|
||||
`, tripId, strategy.id, strategy.id, strategy.id, strategy.id);
|
||||
// PL25 — the same candidate id bound in all four positions.
|
||||
hit = await this.placesRepo.findDuplicateByExternalId(tripId, strategy.id);
|
||||
} else if (strategy.by === 'name') {
|
||||
hit = this.dbs.get<{ id: number; google_ftid: string | null }>(`
|
||||
SELECT id, google_ftid FROM places
|
||||
WHERE trip_id = ? AND lower(trim(name)) = ?
|
||||
ORDER BY id ASC
|
||||
LIMIT 1
|
||||
`, tripId, strategy.name);
|
||||
// PL26 — `strategy.name` is already `.trim().toLowerCase()`'d in JS
|
||||
// (`normalizePlaceName`, `@trek/shared/place/place-match.ts`,
|
||||
// Unicode-aware) — the documented legacy disagreement with SQLite's
|
||||
// ASCII-only `lower()` (rule 18's exception clause: the legacy
|
||||
// statement itself received a JS-lowered value).
|
||||
hit = await this.placesRepo.findDuplicateByName(tripId, strategy.name);
|
||||
} else {
|
||||
hit = this.dbs.get<{ id: number; google_ftid: string | null }>(`
|
||||
SELECT id, google_ftid FROM places
|
||||
WHERE trip_id = ?
|
||||
AND lat IS NOT NULL AND lng IS NOT NULL
|
||||
AND abs(lat - ?) <= ?
|
||||
AND abs(lng - ?) <= ?
|
||||
ORDER BY id ASC
|
||||
LIMIT 1
|
||||
`, tripId, strategy.lat, strategy.tolerance, strategy.lng, strategy.tolerance);
|
||||
// PL27 — `abs(lat - ?) <= ? AND abs(lng - ?) <= ?` through the Task 0b `absDifference` helper.
|
||||
hit = await this.placesRepo.findDuplicateByCoords(tripId, strategy.lat, strategy.lng, strategy.tolerance);
|
||||
}
|
||||
if (hit) return hit;
|
||||
}
|
||||
@@ -679,29 +709,21 @@ export class PlacesService {
|
||||
* than handing over a file that imports as nothing on the other end.
|
||||
*/
|
||||
async exportGpx(tripId: string, opts: GpxExportOptions = {}): Promise<{ gpx: string; filename: string } | null> {
|
||||
// PL28 — `SELECT title FROM trips WHERE id = ?`. `trips` is outside this
|
||||
// task's owned repositories (Plan 3c Task 7's domain) and
|
||||
// `Trips.repository.ts` has a live concurrent editor in this tree this
|
||||
// session (Task 6, adding its own byte-identical `getTitle` for a
|
||||
// different caller) — stays raw here rather than risk a duplicate-method
|
||||
// collision; once Task 6 lands, this call can reuse its `getTitle`.
|
||||
const trip = this.dbs.get<{ title: string }>('SELECT title FROM trips WHERE id = ?', tripId);
|
||||
if (!trip) return null;
|
||||
|
||||
const places = this.dbs.all<GpxExportPlace>(`
|
||||
SELECT p.name, p.description, p.address, p.lat, p.lng, p.route_geometry, c.name AS category
|
||||
FROM places p
|
||||
LEFT JOIN categories c ON c.id = p.category_id
|
||||
WHERE p.trip_id = ?
|
||||
ORDER BY p.id
|
||||
`, tripId);
|
||||
// PL29 — the waypoint projection.
|
||||
const places = await this.placesRepo.listForGpx(tripId) as GpxExportPlace[];
|
||||
|
||||
// One row per stop, ordered the way the day plan draws it, then folded into days.
|
||||
const stops = this.dbs.all<{
|
||||
day_number: number; date: string | null; title: string | null;
|
||||
name: string; lat: number; lng: number;
|
||||
}>(`
|
||||
SELECT d.day_number, d.date, d.title, p.name, p.lat, p.lng
|
||||
FROM days d
|
||||
JOIN day_assignments da ON da.day_id = d.id
|
||||
JOIN places p ON p.id = da.place_id
|
||||
WHERE d.trip_id = ? AND p.lat IS NOT NULL AND p.lng IS NOT NULL
|
||||
ORDER BY d.day_number, da.order_index
|
||||
`, tripId);
|
||||
// One row per stop, ordered the way the day plan draws it, then folded
|
||||
// into days. PL30 — `DayAssignmentsRepository.listItineraryForGpx`.
|
||||
const stops = await this.dayAssignmentsRepo.listItineraryForGpx(tripId);
|
||||
|
||||
const days = new Map<number, GpxExportDay>();
|
||||
for (const stop of stops) {
|
||||
@@ -1562,7 +1584,12 @@ export class PlacesService {
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
async searchImage(tripId: string, placeId: string, userId: number) {
|
||||
const place = this.dbs.get<Place>('SELECT * FROM places WHERE id = ? AND trip_id = ?', placeId, tripId);
|
||||
// `toRowId` first (Task 3 review H1, absorbed here) — same gate shape
|
||||
// as every other place-id route in this service.
|
||||
const id = toRowId(placeId);
|
||||
if (id === null) return { error: 'Place not found', status: 404 };
|
||||
// PL48 — same statement as PL9 (`applyUpdate`'s pre-image read).
|
||||
const place = await this.placesRepo.findInTrip(id, Number(tripId));
|
||||
if (!place) return { error: 'Place not found', status: 404 };
|
||||
|
||||
return this.unsplash.searchUnsplashPhotos(place.name + (place.address ? ' ' + place.address : ''), 5, await this.unsplash.getUnsplashKey(userId));
|
||||
@@ -1579,17 +1606,23 @@ export class PlacesService {
|
||||
* place (with the new aggregate) or null when the place isn't in the trip.
|
||||
*/
|
||||
async rate(tripId: string, placeId: string, userId: number, rating: number | null): Promise<PlaceWithTags | null> {
|
||||
const place = this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId);
|
||||
if (!place) return null;
|
||||
// `toRowId` first (Task 3 review H1, absorbed here): PL50/PL51 write
|
||||
// with this SAME id — a non-canonical `placeId` must resolve to "not
|
||||
// found" here rather than pass a loose affinity match and then write a
|
||||
// rating against an id `place_ratings`'s own FK never actually named.
|
||||
const id = toRowId(placeId);
|
||||
if (id === null) return null;
|
||||
// PL49 — the existence gate.
|
||||
if (!(await this.placesRepo.existsInTrip(id, Number(tripId)))) return null;
|
||||
if (rating === null) {
|
||||
this.dbs.run('DELETE FROM place_ratings WHERE place_id = ? AND user_id = ?', placeId, userId);
|
||||
// PL50 — `DELETE FROM place_ratings WHERE place_id = ? AND user_id = ?`.
|
||||
await this.placeRatingsRepo.deleteRating(id, userId);
|
||||
} else {
|
||||
this.dbs.run(`
|
||||
INSERT INTO place_ratings (place_id, user_id, rating) VALUES (?, ?, ?)
|
||||
ON CONFLICT(place_id, user_id) DO UPDATE SET rating = excluded.rating
|
||||
`, placeId, userId, rating);
|
||||
// PL51 — the only explicit `ON CONFLICT … DO UPDATE` in the cluster; never touches `places.updated_at`.
|
||||
await this.placeRatingsRepo.upsertRating(id, userId, rating);
|
||||
}
|
||||
return await this.dbs.getPlaceWithTags(placeId);
|
||||
// PL52 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
|
||||
return await this.placesRepo.findWithTagsAndRatings(id);
|
||||
}
|
||||
|
||||
// Journey hooks — non-fatal, mirroring the route's try/catch wrappers.
|
||||
|
||||
@@ -1,9 +1,18 @@
|
||||
/**
|
||||
* Places module e2e — exercises the migrated /api/trips/:tripId/places endpoints
|
||||
* through the real JwtAuthGuard against a temp SQLite db. PlacesService runs its
|
||||
* real (DI-native) SQL — places/categories/tags/place_tags/place_ratings DDL
|
||||
* below; journeyService, the permission check, canAccessTrip and the WebSocket
|
||||
* broadcast are mocked.
|
||||
* through the real JwtAuthGuard against a real migrated-and-seeded temp SQLite db
|
||||
* (createSnapshotTestDb(), Plan 3c Task 4 — this used to hand-roll a dozen
|
||||
* CREATE TABLEs, a second hand-maintained schema copy that omitted several
|
||||
* `places` columns `PlacesRepository.insertPlace`'s own `em.insert()`
|
||||
* RETURNING read-back names (`reservation_status`, `reservation_notes`,
|
||||
* `reservation_datetime`) and stubbed `getPlaceWithTags`/`canAccessTrip`
|
||||
* directly rather than routing through the real repositories this domain now
|
||||
* uses — the same class of failure `days.e2e.test.ts` (Task 2) and
|
||||
* `assignments.e2e.test.ts` (Task 3) already fixed for their own files.
|
||||
* PlacesService runs its real SQL (DI-injected, no service mock);
|
||||
* journeyService, the permission check and the WebSocket broadcast stay
|
||||
* mocked. Every `it(...)` body below is unchanged from before this
|
||||
* conversion — only the DB bootstrap changed.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type MockInstance } from 'vitest';
|
||||
import request from 'supertest';
|
||||
@@ -12,74 +21,22 @@ import type { Server } from 'http';
|
||||
import { DatabaseModule } from '../../src/nest/database/database.module';
|
||||
import { RealtimeModule } from '../../src/nest/realtime/realtime.module';
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { seedUser, sessionCookie } from './harness';
|
||||
import { sessionCookie } from './harness';
|
||||
|
||||
const { db } = vi.hoisted(() => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const Database = require('better-sqlite3');
|
||||
const tmp = new Database(':memory:');
|
||||
tmp.exec('PRAGMA journal_mode = WAL');
|
||||
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
|
||||
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0,
|
||||
avatar TEXT);`);
|
||||
// PlacesService runs its real SQL since the place fold — the full place column
|
||||
// set the INSERT/UPDATE/SELECT paths touch, plus the joined projections.
|
||||
tmp.exec(`CREATE TABLE places (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, name TEXT,
|
||||
description TEXT, lat REAL, lng REAL, address TEXT, category_id INTEGER, price REAL, currency TEXT,
|
||||
place_time TEXT, end_time TEXT, duration_minutes INTEGER, notes TEXT, image_url TEXT,
|
||||
google_place_id TEXT, google_ftid TEXT, osm_id TEXT, amap_poi_id TEXT, website TEXT, phone TEXT, transport_mode TEXT,
|
||||
route_geometry TEXT, route_color TEXT, stop_type TEXT, fill_percent INTEGER,
|
||||
created_at DATETIME DEFAULT CURRENT_TIMESTAMP, updated_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
|
||||
tmp.exec(`CREATE TABLE categories (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, color TEXT, icon TEXT);`);
|
||||
// Plan 3c Task 1: TagsRepository.listForPlaces (QH1) selects `user_id`
|
||||
// explicitly (the legacy statement's `t.*` tolerated a narrower fixture
|
||||
// schema; a named column list does not) — added here, matching the real
|
||||
// `tags` table (`Migration20200101000000_baseline_schema.ts`).
|
||||
tmp.exec(`CREATE TABLE tags (id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL, name TEXT, color TEXT,
|
||||
created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`);
|
||||
tmp.exec(`CREATE TABLE place_tags (place_id INTEGER NOT NULL, tag_id INTEGER NOT NULL,
|
||||
PRIMARY KEY (place_id, tag_id));`);
|
||||
tmp.exec(`CREATE TABLE place_ratings (place_id INTEGER NOT NULL, user_id INTEGER NOT NULL, rating INTEGER,
|
||||
created_at DATETIME DEFAULT CURRENT_TIMESTAMP, UNIQUE(place_id, user_id));`);
|
||||
// The assignment=unassigned/assigned filters join these.
|
||||
tmp.exec('CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, day_number INTEGER, date TEXT, title TEXT);');
|
||||
// The GPX export reads the trip title for <metadata> and the filename.
|
||||
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT, user_id INTEGER, currency TEXT);');
|
||||
// TripAccessGuard now reads TripsRepository.findAccessible directly
|
||||
// (Plan 3c Task 0b), a real join against trip_members.
|
||||
tmp.exec('CREATE TABLE trip_members (trip_id INTEGER NOT NULL, user_id INTEGER NOT NULL);');
|
||||
tmp.exec(`CREATE TABLE day_assignments (id INTEGER PRIMARY KEY AUTOINCREMENT, day_id INTEGER NOT NULL,
|
||||
place_id INTEGER NOT NULL, order_index INTEGER DEFAULT 0);`);
|
||||
// reclaimPlaceImage ref-counts an uploaded thumbnail across both tables.
|
||||
// Deleting a place cancels the nights booked at it (#2354), so the delete path
|
||||
// reads this table even in a file that never books one.
|
||||
tmp.exec(`CREATE TABLE day_accommodations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER,
|
||||
place_id INTEGER, start_day_id INTEGER, end_day_id INTEGER, check_in TEXT, check_in_end TEXT,
|
||||
check_out TEXT, confirmation TEXT, notes TEXT, created_at TEXT DEFAULT (datetime('now')));`);
|
||||
tmp.exec(`CREATE TABLE collection_places (id INTEGER PRIMARY KEY AUTOINCREMENT, image_url TEXT);`);
|
||||
// reclaimPhotoCache's removeIfUnreferenced sweeps the Google photo cache.
|
||||
tmp.exec(`CREATE TABLE google_place_photo_meta (place_id TEXT PRIMARY KEY, attribution TEXT, error_at DATETIME);`);
|
||||
// Deleting a place takes its linked expense with it (#1298).
|
||||
tmp.exec(`CREATE TABLE budget_items (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL,
|
||||
name TEXT, total_price REAL DEFAULT 0, place_id INTEGER, reservation_id INTEGER);`);
|
||||
// StorageRegistryService (behind StorageModule, now in this module chain) reads
|
||||
// this at onModuleInit.
|
||||
tmp.exec('CREATE TABLE app_settings (key TEXT PRIMARY KEY, value TEXT);');
|
||||
return { db: tmp };
|
||||
vi.mock('../../src/db/database', async () => {
|
||||
const { createSnapshotTestDb, buildDbMock } = await import('../helpers/db-mock');
|
||||
return buildDbMock(createSnapshotTestDb());
|
||||
});
|
||||
const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() }));
|
||||
vi.mock('../../src/websocket', () => ({ broadcast }));
|
||||
|
||||
const { canAccessTrip } = vi.hoisted(() => ({ canAccessTrip: vi.fn() }));
|
||||
const { getPlaceWithTags } = vi.hoisted(() => ({
|
||||
// The one db/database helper PlacesService still calls through (post-write
|
||||
// re-selects). Real implementation over the temp db so the responses carry
|
||||
// the joined category/tags/ratings the client expects.
|
||||
getPlaceWithTags: vi.fn(),
|
||||
import { db } from '../../src/db/database';
|
||||
|
||||
const { onPlaceCreated, onPlaceUpdated, onPlaceDeleted } = vi.hoisted(() => ({
|
||||
onPlaceCreated: vi.fn().mockResolvedValue(undefined),
|
||||
onPlaceUpdated: vi.fn().mockResolvedValue(undefined),
|
||||
onPlaceDeleted: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock('../../src/db/database', () => ({
|
||||
db, canAccessTrip, isOwner: vi.fn(() => true), getPlaceWithTags, closeDb: () => {}, reinitialize: () => {},
|
||||
}));
|
||||
vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() }));
|
||||
import { broadcast } from '../../src/websocket';
|
||||
import { JourneyDomainService } from '../../src/nest/journey/journey-domain.service';
|
||||
|
||||
import { PermissionsService } from '../../src/nest/permissions/permissions.service';
|
||||
@@ -102,11 +59,7 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
async function build() {
|
||||
const moduleRef = await Test.createTestingModule({ imports: [await TestUnitOfWorkModule.forRoot(db), await createTestMikroOrmModule(db), DatabaseModule, RealtimeModule, PlacesModule] })
|
||||
.overrideProvider(JourneyDomainService)
|
||||
.useValue({
|
||||
onPlaceCreated: vi.fn().mockResolvedValue(undefined),
|
||||
onPlaceUpdated: vi.fn().mockResolvedValue(undefined),
|
||||
onPlaceDeleted: vi.fn().mockResolvedValue(undefined),
|
||||
})
|
||||
.useValue({ onPlaceCreated, onPlaceUpdated, onPlaceDeleted })
|
||||
.compile();
|
||||
const nest = moduleRef.createNestApplication();
|
||||
nest.use(cookieParser());
|
||||
@@ -119,28 +72,32 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
seedUser(db as never, { id: 1 });
|
||||
getPlaceWithTags.mockImplementation((placeId: number | string) => {
|
||||
const place = db.prepare(`SELECT p.*, c.name AS category_name, c.color AS category_color, c.icon AS category_icon
|
||||
FROM places p LEFT JOIN categories c ON p.category_id = c.id WHERE p.id = ?`).get(placeId) as Record<string, unknown> | undefined;
|
||||
if (!place) return null;
|
||||
const tags = db.prepare('SELECT t.* FROM tags t JOIN place_tags pt ON t.id = pt.tag_id WHERE pt.place_id = ?').all(placeId);
|
||||
return { ...place, category: place.category_id ? { id: place.category_id, name: place.category_name } : null, tags, ratings: [], rating_avg: null, rating_count: 0 };
|
||||
});
|
||||
// harness.ts's seedUser() omits password_hash, which the real migrated
|
||||
// schema requires NOT NULL (days.e2e.test.ts's/assignments.e2e.test.ts's
|
||||
// own precedent) — raw inserts here instead. `username: 'e2e-user'`
|
||||
// (user 1) matches the harness default so assertion bodies that spell
|
||||
// out the owner's username stay unchanged. User 2 owns the "foreign"
|
||||
// trip (id 6) several tests below reference — `trips.user_id` and
|
||||
// `places.trip_id` both carry a real FK now (`ON DELETE CASCADE`), so a
|
||||
// dangling trip_id the old hand-rolled DDL tolerated would fail here.
|
||||
db.prepare("INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (1, 'e2e-user', 'e2e@example.test', 'x', 'user', 0)").run();
|
||||
db.prepare("INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (2, 'peer', 'peer@example.test', 'x', 'user', 0)").run();
|
||||
db.prepare("INSERT INTO trips (id, title, user_id) VALUES (6, 'Theirs', 2)").run();
|
||||
app = await build();
|
||||
checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission');
|
||||
server = app.getHttpServer();
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
db.exec('DELETE FROM trips; DELETE FROM places; DELETE FROM place_tags; DELETE FROM place_ratings; DELETE FROM day_assignments; DELETE FROM days;');
|
||||
// Plan 3c Task 0b: TripAccessGuard reads TripsRepository.findAccessible
|
||||
// directly now, a real query — `canAccessTrip.mockReturnValue(...)` no
|
||||
// longer intercepts it, so trip 5's real row is (re-)seeded every test
|
||||
// instead, owned by user 1, matching what the mock used to fake. GPX
|
||||
// tests below re-seed their own data-bearing row with `INSERT OR
|
||||
// REPLACE` over this default.
|
||||
// Trip 5 (owned by user 1) is the suite's main trip, re-seeded fresh
|
||||
// every test — deleting it (`ON DELETE CASCADE`) also clears every place/
|
||||
// day/assignment/rating/tag-link/budget-item this suite seeded under it,
|
||||
// the real-FK equivalent of the old blanket `DELETE FROM places; …`.
|
||||
// Trip 6 (the "foreign" trip, owned by user 2) is seeded once in
|
||||
// beforeAll and left alone.
|
||||
db.exec('DELETE FROM trips WHERE id = 5;');
|
||||
db.prepare("INSERT INTO trips (id, title, user_id) VALUES (5, 'Trip', 1)").run();
|
||||
db.exec('DELETE FROM places WHERE trip_id = 6;');
|
||||
checkPermission.mockReturnValue(true);
|
||||
});
|
||||
|
||||
@@ -184,7 +141,8 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('200 (not 201) bulk-delete, 400 on bad ids', async () => {
|
||||
db.prepare("INSERT INTO places (id, trip_id, name) VALUES (1, 5, 'A'), (2, 5, 'B'), (3, 6, 'Foreign')").run();
|
||||
db.prepare("INSERT INTO places (id, trip_id, name) VALUES (1, 5, 'A'), (2, 5, 'B')").run();
|
||||
db.prepare("INSERT INTO places (id, trip_id, name) VALUES (3, 6, 'Foreign')").run();
|
||||
const ok = await request(server).post('/api/trips/5/places/bulk-delete').set('Cookie', sessionCookie(1)).send({ ids: [1, 2, 3] });
|
||||
expect(ok.status).toBe(200);
|
||||
expect(ok.body).toEqual({ deleted: [1, 2], count: 2 });
|
||||
@@ -279,7 +237,8 @@ describe('Places e2e (real auth guard + temp SQLite)', () => {
|
||||
});
|
||||
|
||||
it('DELETE :id removes the row, 404 for a foreign place', async () => {
|
||||
db.prepare("INSERT INTO places (id, trip_id, name) VALUES (9, 5, 'Gone'), (10, 6, 'Foreign')").run();
|
||||
db.prepare("INSERT INTO places (id, trip_id, name) VALUES (9, 5, 'Gone')").run();
|
||||
db.prepare("INSERT INTO places (id, trip_id, name) VALUES (10, 6, 'Foreign')").run();
|
||||
|
||||
const ok = await request(server).delete('/api/trips/5/places/9').set('Cookie', sessionCookie(1));
|
||||
expect(ok.status).toBe(200);
|
||||
|
||||
@@ -291,13 +291,13 @@ describe('DayAssignmentsRepository — AS4/AS9/AS12 existence + trip-scoped read
|
||||
expect(await assignments.existsInDay(a.id, day.id, trip.id + 1)).toBe(false);
|
||||
});
|
||||
|
||||
it('ASSIGNREPO-002 (AS9): raw-bind — a string id binds unconverted, same as a real number', async () => {
|
||||
it('ASSIGNREPO-002 (AS9): raw-bind on day_id/trip_id — a string binds unconverted, same as a real number; `id` itself is `number`-only now (Task 3 review H1, absorbed in Task 4: the method\'s only caller `toRowId`s `id` before calling, so the type narrowed to match)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const a = createDayAssignment(testDb, day.id, place.id);
|
||||
expect(await assignments.existsInDay(String(a.id), String(day.id), String(trip.id))).toBe(true);
|
||||
expect(await assignments.existsInDay(a.id, String(day.id), String(trip.id))).toBe(true);
|
||||
});
|
||||
|
||||
it('ASSIGNREPO-003 (AS12, findInTrip): the raw `da.*` row, scoped to the trip via a two-hop join; undefined cross-trip', async () => {
|
||||
@@ -595,3 +595,49 @@ it('ASSIGNREPO-023 (D-shape): a notes write after an unrelated identity-map read
|
||||
await withRequestContext(t.orm, () => assignments.setNotes(a.id, 'fresh note'));
|
||||
expect(await assignments.findInTrip(a.id, trip.id)).toMatchObject({ notes: 'fresh note' });
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 (`PlacesService.exportGpx`) — the PL30 itinerary read.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('DayAssignmentsRepository.listItineraryForGpx (PL30)', () => {
|
||||
it('ASSIGNREPO-024: joins day + place, only stops with coordinates, ordered by day_number then order_index', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day1 = createDay(testDb, trip.id, { day_number: 1, date: '2026-01-01', title: 'Day One' });
|
||||
const day2 = createDay(testDb, trip.id, { day_number: 2, date: '2026-01-02' });
|
||||
const geo = createPlace(testDb, trip.id, { name: 'Has Coords', lat: 1, lng: 2 });
|
||||
const noGeo = testDb.prepare('INSERT INTO places (trip_id, name, lat, lng) VALUES (?, ?, NULL, NULL)').run(trip.id, 'No Coords').lastInsertRowid as number;
|
||||
const secondStop = createPlace(testDb, trip.id, { name: 'Second Stop', lat: 3, lng: 4 });
|
||||
// day2's stop first (order_index 0), day1's stop second — proves the
|
||||
// ORDER BY is day_number then order_index, not insertion/id order.
|
||||
createDayAssignment(testDb, day2.id, secondStop.id, { order_index: 0 });
|
||||
createDayAssignment(testDb, day1.id, geo.id, { order_index: 0 });
|
||||
createDayAssignment(testDb, day1.id, noGeo, { order_index: 1 });
|
||||
|
||||
const rows = await assignments.listItineraryForGpx(trip.id);
|
||||
expect(rows).toEqual([
|
||||
{ day_number: 1, date: '2026-01-01', title: 'Day One', name: 'Has Coords', lat: 1, lng: 2 },
|
||||
{ day_number: 2, date: '2026-01-02', title: null, name: 'Second Stop', lat: 3, lng: 4 },
|
||||
]);
|
||||
});
|
||||
|
||||
it('ASSIGNREPO-025: a trip with no coordinate-bearing stops returns []', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const noGeo = testDb.prepare('INSERT INTO places (trip_id, name, lat, lng) VALUES (?, ?, NULL, NULL)').run(trip.id, 'No Coords').lastInsertRowid as number;
|
||||
createDayAssignment(testDb, day.id, noGeo);
|
||||
expect(await assignments.listItineraryForGpx(trip.id)).toEqual([]);
|
||||
});
|
||||
|
||||
it('ASSIGNREPO-026: another trip\'s stops never leak in', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const other = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, other.id);
|
||||
const place = createPlace(testDb, other.id, { lat: 5, lng: 6 });
|
||||
createDayAssignment(testDb, day.id, place.id);
|
||||
expect(await assignments.listItineraryForGpx(trip.id)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -89,12 +89,96 @@ describe('PlaceRatingsRepository.listForPlaces', () => {
|
||||
// TAGREPO comments already use for a method that IS at risk.
|
||||
it('PLACERATINGSREPO-005 (D-shape): a rating written after an unrelated identity-map read is visible in the FIRST wider projection', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: seedVoter } = createUser(testDb, { username: 'seed-voter' });
|
||||
const { user: voter } = createUser(testDb, { username: 'fresh-voter' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
await t.repo(PlaceRatings).find({}); // populate the identity map with an unrelated (empty) read
|
||||
rate(place.id, voter.id, 3, '2026-01-01 00:00:00');
|
||||
rate(place.id, seedVoter.id, 1, '2026-01-01 00:00:00');
|
||||
// rule 20: the FIRST, wider setup read passes `disableIdentityMap: false`
|
||||
// explicitly and carries the column the later write targets (`rating`) —
|
||||
// `find({})` with the base default merged in populates nothing.
|
||||
await t.repo(PlaceRatings).find({}, { disableIdentityMap: false });
|
||||
rate(place.id, voter.id, 3, '2026-01-02 00:00:00');
|
||||
const rows = await placeRatings.listForPlaces([place.id]);
|
||||
expect(rows).toEqual([{ place_id: place.id, user_id: voter.id, username: 'fresh-voter', avatar: null, rating: 3 }]);
|
||||
expect(rows).toEqual([
|
||||
{ place_id: place.id, user_id: seedVoter.id, username: 'seed-voter', avatar: null, rating: 1 },
|
||||
{ place_id: place.id, user_id: voter.id, username: 'fresh-voter', avatar: null, rating: 3 },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 — upsertRating (PL51) / deleteRating (PL50).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('PlaceRatingsRepository.upsertRating (PL51)', () => {
|
||||
it('UPSERTRATINGREPO-001: inserts a new vote, then a re-vote replaces the rating via ON CONFLICT DO UPDATE', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: voter } = createUser(testDb, { username: 'voter1' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
|
||||
await placeRatings.upsertRating(place.id, voter.id, 5);
|
||||
let rows = testDb.prepare('SELECT rating FROM place_ratings WHERE place_id = ? AND user_id = ?').all(place.id, voter.id) as { rating: number }[];
|
||||
expect(rows).toEqual([{ rating: 5 }]);
|
||||
|
||||
await placeRatings.upsertRating(place.id, voter.id, 2);
|
||||
rows = testDb.prepare('SELECT rating FROM place_ratings WHERE place_id = ? AND user_id = ?').all(place.id, voter.id) as { rating: number }[];
|
||||
expect(rows).toEqual([{ rating: 2 }]);
|
||||
// Exactly one row for the (place, user) pair — the composite unique key held, not a duplicate insert.
|
||||
const count = testDb.prepare('SELECT COUNT(*) AS n FROM place_ratings WHERE place_id = ? AND user_id = ?').get(place.id, voter.id) as { n: number };
|
||||
expect(count.n).toBe(1);
|
||||
});
|
||||
|
||||
it('UPSERTRATINGREPO-002 (PL51 ruling): a re-vote never touches places.updated_at — a vote must not 409 another member\'s If-Match', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: voter } = createUser(testDb, { username: 'voter2' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
testDb.prepare("UPDATE places SET updated_at = datetime('now', '-1 hour') WHERE id = ?").run(place.id);
|
||||
const before = (testDb.prepare('SELECT updated_at FROM places WHERE id = ?').get(place.id) as { updated_at: string }).updated_at;
|
||||
|
||||
await placeRatings.upsertRating(place.id, voter.id, 4);
|
||||
await placeRatings.upsertRating(place.id, voter.id, 1); // the conflict path
|
||||
|
||||
const after = (testDb.prepare('SELECT updated_at FROM places WHERE id = ?').get(place.id) as { updated_at: string }).updated_at;
|
||||
expect(after).toBe(before);
|
||||
});
|
||||
|
||||
it('UPSERTRATINGREPO-003: two different voters on the same place both keep their own rating', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: voterA } = createUser(testDb, { username: 'va' });
|
||||
const { user: voterB } = createUser(testDb, { username: 'vb' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
await placeRatings.upsertRating(place.id, voterA.id, 5);
|
||||
await placeRatings.upsertRating(place.id, voterB.id, 1);
|
||||
const rows = testDb.prepare('SELECT user_id, rating FROM place_ratings WHERE place_id = ? ORDER BY user_id').all(place.id);
|
||||
expect(rows).toEqual([
|
||||
{ user_id: Math.min(voterA.id, voterB.id), rating: voterA.id < voterB.id ? 5 : 1 },
|
||||
{ user_id: Math.max(voterA.id, voterB.id), rating: voterA.id < voterB.id ? 1 : 5 },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlaceRatingsRepository.deleteRating (PL50)', () => {
|
||||
it('DELETERATINGREPO-001: removes exactly the (place, user) row, leaving another voter\'s row alone', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: voterA } = createUser(testDb, { username: 'da' });
|
||||
const { user: voterB } = createUser(testDb, { username: 'db' });
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
await placeRatings.upsertRating(place.id, voterA.id, 5);
|
||||
await placeRatings.upsertRating(place.id, voterB.id, 3);
|
||||
await placeRatings.deleteRating(place.id, voterA.id);
|
||||
expect(testDb.prepare('SELECT user_id FROM place_ratings WHERE place_id = ?').all(place.id)).toEqual([{ user_id: voterB.id }]);
|
||||
});
|
||||
|
||||
it('DELETERATINGREPO-002: deleting a vote that does not exist is a no-op, not a throw', async () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: voter } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
await expect(placeRatings.deleteRating(place.id, voter.id)).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,7 +9,7 @@ import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { createSnapshotTestDb } from '../../../helpers/db-mock';
|
||||
import { resetTestDb } from '../../../helpers/test-db';
|
||||
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
|
||||
import { createCategory, createPlace, createTag, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { createCategory, createDay, createDayAssignment, createPlace, createTag, createTrip, createUser } from '../../../helpers/factories';
|
||||
import { Places } from '../../../../src/db/entities/Places.entity';
|
||||
import type { PlacesRepository } from '../../../../src/db/repositories/Places.repository';
|
||||
|
||||
@@ -229,3 +229,305 @@ describe('PlacesRepository.existsByGoogleIdOrImageUrl', () => {
|
||||
expect(await places.existsByGoogleIdOrImageUrl('never-seen', '/api/maps/place-photo/never-seen/bytes')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 — the new PlacesRepository methods.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('PlacesRepository.existsInTrip / findInTrip / reclaimInputs / deleteById (PL0/PL7/PL9/PL17/PL19/PL48/PL49/AS5)', () => {
|
||||
it('PLACEREPO-012: existsInTrip is true only for the matching id+trip pair', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const otherTrip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
expect(await places.existsInTrip(place.id, trip.id)).toBe(true);
|
||||
expect(await places.existsInTrip(place.id, otherTrip.id)).toBe(false);
|
||||
expect(await places.existsInTrip(999999, trip.id)).toBe(false);
|
||||
});
|
||||
|
||||
it('PLACEREPO-013 (D-shape): a name write after an unrelated identity-map read is visible in findInTrip\'s FIRST wider read', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Before' });
|
||||
// Populate the identity map with an unrelated, WIDER read first — `{ disableIdentityMap: false }`
|
||||
// explicitly (program rule 20: the base default leaves the identity map empty, making this vacuous).
|
||||
await t.repo(Places).find({ trip: trip.id }, { disableIdentityMap: false });
|
||||
testDb.prepare('UPDATE places SET name = ? WHERE id = ?').run('After', place.id);
|
||||
const row = await places.findInTrip(place.id, trip.id);
|
||||
expect(row?.name).toBe('After');
|
||||
});
|
||||
|
||||
it('PLACEREPO-014: findInTrip returns every scalar column (PL9/PL48\'s SELECT *), undefined cross-trip', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const otherTrip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Full Row' });
|
||||
const row = await places.findInTrip(place.id, trip.id);
|
||||
expect(row).toMatchObject({ id: place.id, trip_id: trip.id, name: 'Full Row' });
|
||||
expect(await places.findInTrip(place.id, otherTrip.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('PLACEREPO-015: reclaimInputs projects only google_place_id/image_url', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
testDb.prepare('UPDATE places SET google_place_id = ?, image_url = ? WHERE id = ?').run('gpid-1', '/uploads/places/x.jpg', place.id);
|
||||
expect(await places.reclaimInputs(place.id, trip.id)).toEqual({ google_place_id: 'gpid-1', image_url: '/uploads/places/x.jpg' });
|
||||
expect(await places.reclaimInputs(999999, trip.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
it('PLACEREPO-016: deleteById removes exactly that row', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const keep = createPlace(testDb, trip.id, { name: 'Keep' });
|
||||
const gone = createPlace(testDb, trip.id, { name: 'Gone' });
|
||||
await places.deleteById(gone.id);
|
||||
expect(testDb.prepare('SELECT id FROM places WHERE id = ?').get(gone.id)).toBeUndefined();
|
||||
expect(testDb.prepare('SELECT id FROM places WHERE id = ?').get(keep.id)).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlacesRepository.scopedIds (PL23) — input-order preservation', () => {
|
||||
it('PLACEREPO-017: returns only the trip\'s own ids, in the CALLER\'s input order, not row/id order', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const other = createTrip(testDb, user.id);
|
||||
const p1 = createPlace(testDb, trip.id);
|
||||
const p2 = createPlace(testDb, trip.id);
|
||||
const p3 = createPlace(testDb, trip.id);
|
||||
const foreign = createPlace(testDb, other.id);
|
||||
// Deliberately out of id order and interleaved with a foreign + missing id.
|
||||
const result = await places.scopedIds(trip.id, [p3.id, 999999, p1.id, foreign.id, p2.id]);
|
||||
expect(result).toEqual([p3.id, p1.id, p2.id]);
|
||||
});
|
||||
|
||||
it('PLACEREPO-018: empty input short-circuits to []', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
expect(await places.scopedIds(trip.id, [])).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlacesRepository.insertPlace (PL4) / updatePlace (PL11)', () => {
|
||||
it('PLACEREPO-019: insertPlace writes every one of the 25 columns and returns the generated id', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const id = await places.insertPlace({
|
||||
trip_id: trip.id, name: 'Full Insert', description: 'd', lat: 0, lng: 0, address: 'a',
|
||||
category_id: null, price: 0, currency: 'USD', place_time: '09:00', end_time: '10:00',
|
||||
duration_minutes: 60, notes: 'n', image_url: '/uploads/places/x.jpg', google_place_id: 'g',
|
||||
google_ftid: 'f', osm_id: 'o', amap_poi_id: 'am', website: 'w', phone: 'p',
|
||||
transport_mode: 'walking', route_geometry: '[[1,2]]', route_color: '#fff', stop_type: 'fuel',
|
||||
fill_percent: 50,
|
||||
});
|
||||
const row = await places.findInTrip(id, trip.id);
|
||||
expect(row).toMatchObject({
|
||||
trip_id: trip.id, name: 'Full Insert', description: 'd', lat: 0, lng: 0, address: 'a',
|
||||
category_id: null, price: 0, currency: 'USD', place_time: '09:00', end_time: '10:00',
|
||||
duration_minutes: 60, notes: 'n', image_url: '/uploads/places/x.jpg', google_place_id: 'g',
|
||||
google_ftid: 'f', osm_id: 'o', amap_poi_id: 'am', website: 'w', phone: 'p',
|
||||
transport_mode: 'walking', route_geometry: '[[1,2]]', route_color: '#fff', stop_type: 'fuel',
|
||||
fill_percent: 50,
|
||||
});
|
||||
});
|
||||
|
||||
it('PLACEREPO-020 (PL11 matrix): a written null vs a written value are byte-distinct on the same column', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Original' });
|
||||
const base = {
|
||||
name: 'Original', description: null, lat: null, lng: null, address: null, category_id: null,
|
||||
price: null, currency: null, place_time: null, end_time: null, duration_minutes: null,
|
||||
notes: null, image_url: null, google_place_id: null, google_ftid: null, osm_id: null,
|
||||
amap_poi_id: null, website: null, phone: null, transport_mode: null, route_color: null,
|
||||
stop_type: null, fill_percent: null,
|
||||
};
|
||||
// A non-COALESCE column (description) written explicitly null clears it.
|
||||
await places.updatePlace(place.id, { ...base, description: 'first' });
|
||||
expect((await places.findInTrip(place.id, trip.id))?.description).toBe('first');
|
||||
await places.updatePlace(place.id, { ...base, description: null });
|
||||
expect((await places.findInTrip(place.id, trip.id))?.description).toBeNull();
|
||||
// duration_minutes: 0 is a real value, distinct from null (the service's
|
||||
// `!== undefined` fallback — this repository test proves the write itself
|
||||
// stores 0 and null distinctly, not that either collapses to the other).
|
||||
await places.updatePlace(place.id, { ...base, duration_minutes: 0 });
|
||||
expect((await places.findInTrip(place.id, trip.id))?.duration_minutes).toBe(0);
|
||||
await places.updatePlace(place.id, { ...base, duration_minutes: null });
|
||||
expect((await places.findInTrip(place.id, trip.id))?.duration_minutes).toBeNull();
|
||||
});
|
||||
|
||||
it('PLACEREPO-021: updatePlace stamps updated_at with the current time', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Stampable' });
|
||||
// Force a stale, second-resolution-distinguishable `updated_at` — this
|
||||
// write and the assertion below both happen within the same real
|
||||
// second, so comparing against `createPlace`'s own `CURRENT_TIMESTAMP`
|
||||
// default would be flaky (Object.is on two identical second-granularity
|
||||
// strings), not a proof of anything.
|
||||
testDb.prepare("UPDATE places SET updated_at = datetime('now', '-1 hour') WHERE id = ?").run(place.id);
|
||||
const before = (await places.findInTrip(place.id, trip.id))?.updated_at;
|
||||
await places.updatePlace(place.id, {
|
||||
name: 'Stamped', description: null, lat: null, lng: null, address: null, category_id: null,
|
||||
price: null, currency: null, place_time: null, end_time: null, duration_minutes: null,
|
||||
notes: null, image_url: null, google_place_id: null, google_ftid: null, osm_id: null,
|
||||
amap_poi_id: null, website: null, phone: null, transport_mode: null, route_color: null,
|
||||
stop_type: null, fill_percent: null,
|
||||
});
|
||||
const after = await places.findInTrip(place.id, trip.id);
|
||||
expect(after?.name).toBe('Stamped');
|
||||
expect(after?.updated_at).not.toBe(before);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlacesRepository.findDuplicateByExternalId / findDuplicateByName / findDuplicateByCoords (PL25/PL26/PL27)', () => {
|
||||
it('PLACEREPO-022 (PL25): matches on any of the four provider-id columns, lowest id wins', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const first = createPlace(testDb, trip.id, { name: 'First' });
|
||||
testDb.prepare('UPDATE places SET osm_id = ? WHERE id = ?').run('node:123', first.id);
|
||||
const second = createPlace(testDb, trip.id, { name: 'Second' });
|
||||
testDb.prepare('UPDATE places SET google_ftid = ? WHERE id = ?').run('node:123', second.id);
|
||||
const hit = await places.findDuplicateByExternalId(String(trip.id), 'node:123');
|
||||
expect(hit?.id).toBe(first.id);
|
||||
});
|
||||
|
||||
it('PLACEREPO-023 (PL26, rule 18 exception): lower(trim(name)) matches ASCII-folded names; a non-ASCII cased letter does NOT match — the documented legacy divergence, pinned not fixed', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: ' Café Central ' });
|
||||
// The service's own JS-lowered+trimmed value, matching `normalizePlaceName`'s
|
||||
// Unicode-aware fold — this side is honest, so it would match the row exactly.
|
||||
const jsLowered = ' café central '.trim().toLowerCase();
|
||||
// SQLite's lower()/trim() is ASCII-only: it lowercases "C" but never touches
|
||||
// "É"/"é" — `Café Central` (stored) trims+lowers via SQL to `café central`.
|
||||
const hit = await places.findDuplicateByName(String(trip.id), jsLowered);
|
||||
expect(hit?.id).toBe(place.id);
|
||||
// A row stored with an upper-case accented letter (`CAFÉ`) does NOT match a
|
||||
// JS-lowered candidate (`café`) — SQLite's LOWER() never folds `É` to `é`,
|
||||
// reproducing the documented disagreement (places.service.ts's own PL26 docstring).
|
||||
const accented = createPlace(testDb, trip.id, { name: 'CAFÉ NORD' });
|
||||
const jsLoweredAccented = 'café nord'.toLowerCase();
|
||||
const miss = await places.findDuplicateByName(String(trip.id), jsLoweredAccented);
|
||||
expect(miss?.id).not.toBe(accented.id);
|
||||
});
|
||||
|
||||
it('PLACEREPO-024 (PL27): matches within the coordinate tolerance box, on both lat and lng', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { lat: 48.8566, lng: 2.3522 });
|
||||
const hit = await places.findDuplicateByCoords(String(trip.id), 48.8566 + 0.00005, 2.3522 - 0.00005, 0.0001);
|
||||
expect(hit?.id).toBe(place.id);
|
||||
const miss = await places.findDuplicateByCoords(String(trip.id), 48.9, 2.5, 0.0001);
|
||||
expect(miss).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlacesRepository.listForGpx (PL29) and existsByImageUrl (PI1)', () => {
|
||||
it('PLACEREPO-025: projects the waypoint columns plus the flat category name, ordered by id', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const category = createCategory(testDb, { name: 'Trails' });
|
||||
const p1 = createPlace(testDb, trip.id, { name: 'A', category_id: category.id });
|
||||
// `createPlace` always falls back to the first available category when
|
||||
// no override is given (its own `?? defaultCat?.id ?? null` chain — an
|
||||
// explicit `category_id: null` override is ALSO nullish, so it falls
|
||||
// through the same way), and deleting the categories table would also
|
||||
// break p1's own join. Insert place B directly instead, to genuinely
|
||||
// pin `category_id = NULL` while `category` (p1's) still exists.
|
||||
const p2Id = testDb.prepare('INSERT INTO places (trip_id, name, category_id) VALUES (?, ?, NULL)').run(trip.id, 'B').lastInsertRowid;
|
||||
const p2 = { id: p2Id as number };
|
||||
const rows = await places.listForGpx(String(trip.id));
|
||||
expect(rows.map((r) => r.name)).toEqual(['A', 'B']);
|
||||
expect(rows[0]).toMatchObject({ name: 'A', category: 'Trails' });
|
||||
expect(rows[1]).toMatchObject({ name: 'B', category: null });
|
||||
void p1; void p2;
|
||||
});
|
||||
|
||||
it('PLACEREPO-026 (PI1): existsByImageUrl is trip-agnostic — true for any place carrying the url', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?').run('/uploads/places/shared.jpg', place.id);
|
||||
expect(await places.existsByImageUrl('/uploads/places/shared.jpg')).toBe(true);
|
||||
expect(await places.existsByImageUrl('/uploads/places/nope.jpg')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('PlacesRepository.listForTrip (PL3) — filter fragments', () => {
|
||||
it('PLACEREPO-027: no filters — every trip place, DISTINCT, newest first', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const older = createPlace(testDb, trip.id, { name: 'Older' });
|
||||
testDb.prepare("UPDATE places SET created_at = datetime('now', '-1 hour') WHERE id = ?").run(older.id);
|
||||
const newer = createPlace(testDb, trip.id, { name: 'Newer' });
|
||||
const rows = await places.listForTrip(String(trip.id), {});
|
||||
expect(rows.map((r) => r.id)).toEqual([newer.id, older.id]);
|
||||
});
|
||||
|
||||
it('PLACEREPO-028: searchPattern matches name/address/description (already-escaped, wrapped by the caller)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const hit = createPlace(testDb, trip.id, { name: 'Eiffel Tower' });
|
||||
const miss = createPlace(testDb, trip.id, { name: 'Louvre' });
|
||||
const rows = await places.listForTrip(String(trip.id), { searchPattern: '%Eiffel%' });
|
||||
expect(rows.map((r) => r.id)).toEqual([hit.id]);
|
||||
void miss;
|
||||
});
|
||||
|
||||
it('PLACEREPO-029: category filters to that category_id only', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const cat = createCategory(testDb, { name: 'Museums' });
|
||||
const inCat = createPlace(testDb, trip.id, { category_id: cat.id });
|
||||
const outCat = createPlace(testDb, trip.id);
|
||||
testDb.prepare('DELETE FROM categories WHERE id != ?').run(cat.id);
|
||||
const rows = await places.listForTrip(String(trip.id), { category: String(cat.id) });
|
||||
expect(rows.map((r) => r.id)).toEqual([inCat.id]);
|
||||
void outCat;
|
||||
});
|
||||
|
||||
it('PLACEREPO-030: tag filters via the place_tags subquery', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const tag = createTag(testDb, user.id, { name: 'Must see' });
|
||||
const tagged = createPlace(testDb, trip.id, { name: 'Tagged' });
|
||||
const untagged = createPlace(testDb, trip.id, { name: 'Untagged' });
|
||||
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(tagged.id, tag.id);
|
||||
const rows = await places.listForTrip(String(trip.id), { tag: String(tag.id) });
|
||||
expect(rows.map((r) => r.id)).toEqual([tagged.id]);
|
||||
void untagged;
|
||||
});
|
||||
|
||||
it('PLACEREPO-031: assignment=unassigned / assigned split the trip\'s places by day_assignments membership', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const assigned = createPlace(testDb, trip.id, { name: 'On a day' });
|
||||
const unassigned = createPlace(testDb, trip.id, { name: 'In the pool' });
|
||||
createDayAssignment(testDb, day.id, assigned.id);
|
||||
const assignedRows = await places.listForTrip(String(trip.id), { assignment: 'assigned' });
|
||||
expect(assignedRows.map((r) => r.id)).toEqual([assigned.id]);
|
||||
const unassignedRows = await places.listForTrip(String(trip.id), { assignment: 'unassigned' });
|
||||
expect(unassignedRows.map((r) => r.id)).toEqual([unassigned.id]);
|
||||
});
|
||||
|
||||
it('PLACEREPO-032: search + category + tag + assignment combine with AND, all four fragments at once', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const cat = createCategory(testDb, { name: 'Parks' });
|
||||
const tag = createTag(testDb, user.id, { name: 'Green' });
|
||||
const day = createDay(testDb, trip.id);
|
||||
const winner = createPlace(testDb, trip.id, { name: 'Central Park', category_id: cat.id });
|
||||
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(winner.id, tag.id);
|
||||
createDayAssignment(testDb, day.id, winner.id);
|
||||
// A near-miss on every axis, to prove the AND actually excludes it.
|
||||
const other = createPlace(testDb, trip.id, { name: 'Central Park Annex', category_id: cat.id });
|
||||
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(other.id, tag.id);
|
||||
// `other` is left unassigned, so `assignment: 'assigned'` alone should exclude it.
|
||||
const rows = await places.listForTrip(String(trip.id), {
|
||||
searchPattern: '%Central Park%', category: String(cat.id), tag: String(tag.id), assignment: 'assigned',
|
||||
});
|
||||
expect(rows.map((r) => r.id)).toEqual([winner.id]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -318,8 +318,11 @@ describe('TagsRepository', () => {
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const tag = createTag(testDb, user.id, { name: 'Fresh' });
|
||||
// Populate the identity map with a narrower read first.
|
||||
await tags.findByIdAndUser(tag.id, user.id);
|
||||
// rule 20: the FIRST, wider setup read passes `disableIdentityMap:
|
||||
// false` explicitly and carries the column the later write targets
|
||||
// (`name`) — `find({})` with the base default merged in populates
|
||||
// nothing.
|
||||
await t.repo(Tags).find({}, { disableIdentityMap: false });
|
||||
attach(tag.id, place.id);
|
||||
testDb.prepare('UPDATE tags SET name = ? WHERE id = ?').run('Renamed', tag.id);
|
||||
const rows = await tags.listForPlaces([place.id]);
|
||||
@@ -327,3 +330,58 @@ describe('TagsRepository', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Plan 3c Task 4 — findByIds (PL2) and the place_tags pivot writes (PL5/PL12/PL13).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('TagsRepository.findByIds (PL2)', () => {
|
||||
it('FINDBYIDSREPO-001: returns id + user_id for the matching ids, empty input short-circuits', async () => {
|
||||
const { user: ownerA } = createUser(testDb);
|
||||
const { user: ownerB } = createUser(testDb, { username: 'owner_b' });
|
||||
const tagA = createTag(testDb, ownerA.id, { name: 'A' });
|
||||
const tagB = createTag(testDb, ownerB.id, { name: 'B' });
|
||||
const rows = await tags.findByIds([tagA.id, tagB.id, 999999]);
|
||||
expect(rows.map((r) => ({ id: r.id, user_id: r.user_id })).sort((a, b) => a.id - b.id)).toEqual(
|
||||
[{ id: tagA.id, user_id: ownerA.id }, { id: tagB.id, user_id: ownerB.id }].sort((a, b) => a.id - b.id),
|
||||
);
|
||||
expect(await tags.findByIds([])).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('TagsRepository.insertIgnore / deleteForPlace (PL5/PL12/PL13 — place_tags pivot)', () => {
|
||||
it('PLACETAGSREPO-001: insertIgnore attaches every listed tag, ignoring a duplicate pair', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const tagA = createTag(testDb, user.id, { name: 'A' });
|
||||
const tagB = createTag(testDb, user.id, { name: 'B' });
|
||||
await tags.insertIgnore(place.id, [tagA.id, tagB.id]);
|
||||
// Re-run with an overlapping id — the pre-existing pair must not error or duplicate.
|
||||
await tags.insertIgnore(place.id, [tagA.id]);
|
||||
const rows = testDb.prepare('SELECT tag_id FROM place_tags WHERE place_id = ? ORDER BY tag_id').all(place.id) as { tag_id: number }[];
|
||||
expect(rows.map((r) => r.tag_id).sort((a, b) => a - b)).toEqual([tagA.id, tagB.id].sort((a, b) => a - b));
|
||||
});
|
||||
|
||||
it('PLACETAGSREPO-002: insertIgnore with an empty array is a no-op', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
await tags.insertIgnore(place.id, []);
|
||||
const rows = testDb.prepare('SELECT tag_id FROM place_tags WHERE place_id = ?').all(place.id);
|
||||
expect(rows).toEqual([]);
|
||||
});
|
||||
|
||||
it('PLACETAGSREPO-003: deleteForPlace removes every row for that place, leaving other places\' tags alone', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const other = createPlace(testDb, trip.id);
|
||||
const tag = createTag(testDb, user.id, { name: 'Shared' });
|
||||
await tags.insertIgnore(place.id, [tag.id]);
|
||||
await tags.insertIgnore(other.id, [tag.id]);
|
||||
await tags.deleteForPlace(place.id);
|
||||
expect(testDb.prepare('SELECT * FROM place_tags WHERE place_id = ?').all(place.id)).toEqual([]);
|
||||
expect(testDb.prepare('SELECT * FROM place_tags WHERE place_id = ?').all(other.id)).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -173,6 +173,60 @@ describe('dayExists / placeExists / assignmentExistsInDay', () => {
|
||||
expect(await svc.assignmentExistsInDay(a.id, otherDay.id, trip.id)).toBe(false);
|
||||
expect(await svc.assignmentExistsInDay(a.id, day.id, trip.id + 1)).toBe(false);
|
||||
});
|
||||
|
||||
// Task 3 review H1 (live regression, fixed and absorbed in Task 4): these
|
||||
// four gates used to bind a non-canonical id (`"<id> "`, `"<id>.0"`,
|
||||
// `"+<id>"`) straight into their own read, which SQLite's text/integer
|
||||
// affinity matches — so the gate answered "found" for an id `toRowId`
|
||||
// (the SAME conversion the write behind each gate applies) rejects as
|
||||
// `null`. Each case below proves the gate itself now says "not found" for
|
||||
// exactly the id shapes `toRowId` rejects, so the mutation behind it can
|
||||
// never be reached with a mismatched id.
|
||||
describe('H1 — non-canonical ids resolve to not-found in the gate itself', () => {
|
||||
it('ASG-SVC-042: dayExists rejects "<id> "/"<id>.0"/"+<id>", not just non-numeric ids', async () => {
|
||||
const { trip, day } = fixture();
|
||||
expect(await svc.dayExists(`${day.id} `, trip.id)).toBe(false);
|
||||
expect(await svc.dayExists(`${day.id}.0`, trip.id)).toBe(false);
|
||||
expect(await svc.dayExists(`+${day.id}`, trip.id)).toBe(false);
|
||||
expect(await svc.dayExists('abc', trip.id)).toBe(false);
|
||||
// The canonical shape still passes, proving the fix didn't just make everything false.
|
||||
expect(await svc.dayExists(String(day.id), trip.id)).toBe(true);
|
||||
});
|
||||
|
||||
it('ASG-SVC-043: placeExists rejects "<id> "/"<id>.0"/"+<id>"', async () => {
|
||||
const { trip, place } = fixture();
|
||||
expect(await svc.placeExists(`${place.id} `, trip.id)).toBe(false);
|
||||
expect(await svc.placeExists(`${place.id}.0`, trip.id)).toBe(false);
|
||||
expect(await svc.placeExists(`+${place.id}`, trip.id)).toBe(false);
|
||||
expect(await svc.placeExists(String(place.id), trip.id)).toBe(true);
|
||||
});
|
||||
|
||||
it('ASG-SVC-044: assignmentExistsInDay rejects a non-canonical assignment id', async () => {
|
||||
const { trip, day, place } = fixture();
|
||||
const a = createDayAssignment(testDb, day.id, place.id);
|
||||
expect(await svc.assignmentExistsInDay(`${a.id} `, day.id, trip.id)).toBe(false);
|
||||
expect(await svc.assignmentExistsInDay(`${a.id}.0`, day.id, trip.id)).toBe(false);
|
||||
expect(await svc.assignmentExistsInDay(String(a.id), day.id, trip.id)).toBe(true);
|
||||
});
|
||||
|
||||
it('ASG-SVC-045: getAssignmentForTrip rejects a non-canonical assignment id (undefined, not the row)', async () => {
|
||||
const { trip, day, place } = fixture();
|
||||
const a = createDayAssignment(testDb, day.id, place.id);
|
||||
expect(await svc.getAssignmentForTrip(`${a.id} `, trip.id)).toBeUndefined();
|
||||
expect(await svc.getAssignmentForTrip(`${a.id}.0`, trip.id)).toBeUndefined();
|
||||
expect(await svc.getAssignmentForTrip(String(a.id), trip.id)).toMatchObject({ id: a.id });
|
||||
});
|
||||
|
||||
it('ASG-SVC-046: deleteAssignment is never reached with a mismatched id — the controller-shape gate-then-write leaves the row intact for a non-canonical id', async () => {
|
||||
const { trip, day, place } = fixture();
|
||||
const a = createDayAssignment(testDb, day.id, place.id);
|
||||
// The controller's own shape: gate, then act only if the gate passed.
|
||||
const gate = await svc.assignmentExistsInDay(`${a.id} `, day.id, trip.id);
|
||||
expect(gate).toBe(false);
|
||||
if (gate) await svc.deleteAssignment(`${a.id} `);
|
||||
expect(await svc.getAssignmentForTrip(String(a.id), trip.id)).toMatchObject({ id: a.id });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ── createAssignment ──────────────────────────────────────────────────────────
|
||||
|
||||
@@ -54,7 +54,7 @@ import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/plac
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
|
||||
import { makeStorageFixture } from '../../helpers/storage-fixture';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestPlacesRepo, createTestTripMembersRepo, createTestDayAssignmentsRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
|
||||
const dbs = new DatabaseService(testDb);
|
||||
const photoCacheStub = { removeIfUnreferenced: vi.fn() } as unknown as PlacePhotoCacheService;
|
||||
@@ -92,6 +92,11 @@ async function svc(searchNominatim: MapsService['searchNominatim']): Promise<Pla
|
||||
new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)),
|
||||
storageFx.storage,
|
||||
await accommodationsOver(dbs), await createTestUnitOfWork(dbs.connection),
|
||||
await createTestPlacesRepo(dbs.connection),
|
||||
await createTestTagsRepo(dbs.connection),
|
||||
await createTestPlaceRatingsRepo(dbs.connection),
|
||||
await createTestTripMembersRepo(dbs.connection),
|
||||
await createTestDayAssignmentsRepo(dbs.connection),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -78,7 +78,7 @@ import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpe
|
||||
import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service';
|
||||
import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository';
|
||||
import { makeStorageFixture } from '../../helpers/storage-fixture';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestUsersRepo, createTestTagsRepo, createTestPlaceRatingsRepo, createTestAssignmentParticipantsRepo, createTestPlacesRepo, createTestTripMembersRepo, createTestDayAssignmentsRepo, sharedTestOrm } from '../../helpers/test-uow';
|
||||
import type { AppSettingsRepository } from '../../../src/db/repositories/AppSettings.repository';
|
||||
import type { UsersRepository } from '../../../src/db/repositories/Users.repository';
|
||||
|
||||
@@ -119,6 +119,11 @@ async function makePlacesService(
|
||||
new JourneyDomainService(dbs, new RealtimeService(), new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)),
|
||||
placesStorageFx.storage,
|
||||
await accommodationsOver(dbs), await createTestUnitOfWork(dbs.connection),
|
||||
await createTestPlacesRepo(dbs.connection),
|
||||
await createTestTagsRepo(dbs.connection),
|
||||
await createTestPlaceRatingsRepo(dbs.connection),
|
||||
await createTestTripMembersRepo(dbs.connection),
|
||||
await createTestDayAssignmentsRepo(dbs.connection),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -295,6 +300,22 @@ describe('get', () => {
|
||||
const trip = createTrip(testDb, user.id);
|
||||
expect(await svc.get(String(trip.id), '99999')).toBeNull();
|
||||
});
|
||||
|
||||
// Task 3 review H1's lesson, applied to every place-id gate this task
|
||||
// converts: a non-canonical id shape (`"<id> "`, `"<id>.0"`, `"+<id>"`)
|
||||
// must resolve to not-found HERE, in the gate — SQLite's own affinity
|
||||
// would otherwise match it, and this service's writes downstream convert
|
||||
// with `toRowId`, which rejects the same shapes.
|
||||
it('PLACE-SVC-012b (H1) — non-canonical id shapes ("<id> ", "<id>.0", "+<id>") also return null, not just non-numeric ids', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
expect(await svc.get(String(trip.id), `${place.id} `)).toBeNull();
|
||||
expect(await svc.get(String(trip.id), `${place.id}.0`)).toBeNull();
|
||||
expect(await svc.get(String(trip.id), `+${place.id}`)).toBeNull();
|
||||
// The canonical shape still finds it, proving the fix didn't just null everything.
|
||||
expect(await svc.get(String(trip.id), String(place.id))).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ── update ────────────────────────────────────────────────────────────────────
|
||||
@@ -316,6 +337,16 @@ describe('update', () => {
|
||||
expect(await svc.update(String(trip.id), '99999', { name: 'Ghost' })).toBeNull();
|
||||
});
|
||||
|
||||
it('PLACE-SVC-014b (H1) — a non-canonical id ("<id> ") is null, not a write: the row is left untouched', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Untouched' });
|
||||
const result = await svc.update(String(trip.id), `${place.id} `, { name: 'Clobbered' });
|
||||
expect(result).toBeNull();
|
||||
const row = testDb.prepare('SELECT name FROM places WHERE id = ?').get(place.id) as { name: string };
|
||||
expect(row.name).toBe('Untouched');
|
||||
});
|
||||
|
||||
it('PLACE-SVC-015 — updates tags (replaces old set)', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
@@ -442,6 +473,15 @@ describe('remove', () => {
|
||||
expect((await svc.remove(String(trip.id), '99999')).deleted).toBe(false);
|
||||
});
|
||||
|
||||
it('PLACE-SVC-018b (H1) — a non-canonical id ("<id> ") is not deleted: deleted:false, row intact', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Survives' });
|
||||
const result = await svc.remove(String(trip.id), `${place.id} `);
|
||||
expect(result.deleted).toBe(false);
|
||||
expect(await svc.get(String(trip.id), String(place.id))).not.toBeNull();
|
||||
});
|
||||
|
||||
it('PLACE-SVC-019 — deleting one place does not remove others', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
@@ -952,6 +992,15 @@ describe('searchImage', () => {
|
||||
expect(result.status).toBe(404);
|
||||
});
|
||||
|
||||
it('PLACE-SVC-030b (H1) — a non-canonical id ("<id> ") also 404s', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id);
|
||||
const result = await svc.searchImage(String(trip.id), `${place.id} `, user.id) as any;
|
||||
expect(result.error).toBeDefined();
|
||||
expect(result.status).toBe(404);
|
||||
});
|
||||
|
||||
it('PLACE-SVC-031 — searches Unsplash without a stored API key', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
@@ -1160,6 +1209,34 @@ describe('custom place image reclaim', () => {
|
||||
const count = testDb.prepare('SELECT COUNT(*) AS n FROM place_ratings WHERE place_id = ?').get(place.id) as { n: number };
|
||||
expect(count.n).toBe(0);
|
||||
});
|
||||
|
||||
it('PLACE-SVC-051b (H1) — a non-canonical id ("<id> ") is null and writes nothing', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id) as { id: number };
|
||||
expect(await svc.rate(String(trip.id), `${place.id} `, user.id, 4)).toBeNull();
|
||||
const count = testDb.prepare('SELECT COUNT(*) AS n FROM place_ratings WHERE place_id = ?').get(place.id) as { n: number };
|
||||
expect(count.n).toBe(0);
|
||||
});
|
||||
|
||||
// PL51's ruling: an ON CONFLICT DO UPDATE limited to `rating` must never
|
||||
// touch `places.updated_at` — a vote must not 409 another member's
|
||||
// offline edit (#1435). Proven directly against the column, not just
|
||||
// against the response shape.
|
||||
it('PLACE-SVC-051c (PL51) — rating a place never bumps places.updated_at', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const place = createPlace(testDb, trip.id, { name: 'Silent Vote' }) as { id: number };
|
||||
const before = (testDb.prepare('SELECT updated_at FROM places WHERE id = ?').get(place.id) as { updated_at: string | null }).updated_at;
|
||||
|
||||
await svc.rate(String(trip.id), String(place.id), user.id, 5);
|
||||
// Re-vote (the ON CONFLICT DO UPDATE path) — the one statement in the
|
||||
// whole cluster that could plausibly touch `updated_at` via a careless
|
||||
// merge-field list.
|
||||
await svc.rate(String(trip.id), String(place.id), user.id, 2);
|
||||
const after = (testDb.prepare('SELECT updated_at FROM places WHERE id = ?').get(place.id) as { updated_at: string | null }).updated_at;
|
||||
expect(after).toBe(before);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Automatic track colours (#776) ────────────────────────────────────────────
|
||||
|
||||
@@ -248,7 +248,7 @@ describe('Roadtrip MCP registration and search', () => {
|
||||
const mcp = new PlacesMcp(
|
||||
places as never,
|
||||
{} as never,
|
||||
{ canAccessTrip: () => true } as never,
|
||||
{ findAccessible: () => ({ id: 1, user_id: 1, currency: null }) } as never,
|
||||
auth as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
|
||||
@@ -63,6 +63,8 @@ import {
|
||||
createTestAssignmentParticipantsRepo,
|
||||
createTestGooglePlacePhotoMetaRepo,
|
||||
createTestPlacesRepo,
|
||||
createTestTripMembersRepo,
|
||||
createTestDayAssignmentsRepo,
|
||||
sharedTestOrm,
|
||||
} from '../../helpers/test-uow';
|
||||
|
||||
@@ -105,6 +107,11 @@ beforeAll(async () => {
|
||||
new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs), await createTestUnitOfWork(dbs.connection)),
|
||||
makeStorageFixture('').storage,
|
||||
await accommodationsOver(dbs), await createTestUnitOfWork(dbs.connection),
|
||||
await createTestPlacesRepo(dbs.connection),
|
||||
await createTestTagsRepo(dbs.connection),
|
||||
await createTestPlaceRatingsRepo(dbs.connection),
|
||||
await createTestTripMembersRepo(dbs.connection),
|
||||
await createTestDayAssignmentsRepo(dbs.connection),
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user