mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 23:16:46 +00:00
feat: six community requests — import dedup, accommodations in the feed, OAuth refresh race, visited-from-trip, journal jumps, phone packing rows (#1915)
* fix(places): match imports on the provider id so renamed places are not duplicated Re-importing a Google Maps list duplicated every place the user had renamed. buildDedupSet only ever collected names and, for unnamed places, coordinates, and isPlaceDuplicate returns on the name before it reaches the coordinate fallback, so a renamed place was findable by neither. The ids were stored all along, and even backfilled onto matched places, they were simply never read. Match on google_place_id / google_ftid / osm_id first, keeping name and coordinates in their old roles below it. Widening the coordinate check instead would have merged the restaurant and the bar in the same building. The collection copy carried a hand-mirrored port of the same predicate; it now uses the shared helpers and gets the fix with them. Closes #1550 * feat(calendar): put accommodations into the subscribed feed as all-day stays A hotel keeps its dates on the linked stay, not on the reservation: the booking form writes reservation_time = NULL for type 'hotel', and day_accommodations carries start day, end day and the check-in/out clock. The feed only ever read the reservation, so a stay showed up once on the arrival day at best, and not at all when it came from the booking form. Join the stay in and emit the reservation as an all-day range over every night, which also keeps it correct when days are reordered later — the dates now come from the day rows rather than from the copy stamped at creation time. Check-in and check-out become their own timed events when the stay records a clock, because an all-day event cannot carry the time you actually need reminding of. The check-in window, when set, becomes that event's DTEND. Closes #1586 * fix(oauth): treat a concurrent refresh as a race, not as a stolen token Refresh rotation is a race by construction. Several MCP clients share one refresh token, both post it within the same second, and the loser was read as a replay: chain revoked, MCP sessions torn down, browser window asking to log in again. That is the daily re-authentication people were seeing, not a short lifetime — access tokens last an hour and refresh tokens roll for thirty days. A revoked refresh token now only counts as a replay once it is outside a 30-second grace window, and only when the rotation that revoked it left a successor still alive. Inside the window the caller gets its own pair branched off the same parent, so both clients walk away with a working token. Explicit revocation leaves no live successor and a chain killed by a real replay has none either, so neither can slip through the window. Closes #1007 * feat(collections): mark a place visited from the trip it belongs to A place saved in several lists had to be opened in each of them to say you had been there, which is the wrong way round: you find out you have been somewhere while looking at the trip, not while browsing the library. The save picker now shows each list's own status and cycles it in place, with one action for "visited, wherever this is saved". The planner's selection bar and the mobile bulk toolbar carry the same thing for a whole selection — select all, one tap, done. Matching happens on the server with the signals findMembership already uses plus the source link a place saved from this trip carries, so a copy renamed inside a list is still found. Lists you may only read are skipped rather than refused; a shared list you cannot edit should not stop you marking your own. Closes #1469 * feat(journey): jump to the top or back to the last entry in a long journal Everything that adds to a journal sits at the top — the entry button, the gallery upload — and where you left off sits at the bottom, so adding one photo to a long journey meant scrolling the whole way twice. Two buttons float over the middle of the feed, clear of the Add Entry column on one side and the reorder arrows on the other, each appearing only when there is more than a screenful to travel in that direction. They live in a zero-height sticky box so they take no layout space and never push the timeline around. While in the header, the status pill also picks up the height of the round buttons beside it. Closes #1088 * fix(packing): give the item name the row back on phones An item name had roughly a third of the row to itself: a spelled-out sharing sentence, an empty weight column and two round action buttons took the rest, so anything longer than a word was cut off. Sharing is now the icon it always was next to a label rather than a sentence in the layout, an unset weight prints nothing instead of a dash, and edit/delete sit behind one overflow button that opens the same inline menu the bag picker already uses. The to-do list needed none of this — it wraps its chips onto a second line already. Closes #1525 * docs(wiki): cover the six community changes - Calendar feeds: accommodations as an all-day range plus check-in/check-out - Collections: setting the visited status from a trip, and how places are matched - Journey: the jump buttons on a long journal - Packing: what the phone row keeps and what moved behind the overflow button - Places: re-importing a list no longer duplicates renamed places - MCP setup: the rotation race no longer ends an OAuth session * test(collections): assert the stored status instead of a private method tests/ sits outside the build tsconfig, so reaching into getPlaceById only showed up in the dedicated typecheck step. Reading the row is the better assertion anyway — it pins what was persisted, not what a getter returns.
This commit is contained in:
@@ -147,11 +147,19 @@ export class CalendarService {
|
||||
const trip = this.db.prepare('SELECT * FROM trips WHERE id = ?').get(tripId) as any;
|
||||
if (!trip) throw new NotFoundError('Trip not found');
|
||||
|
||||
// A hotel keeps its dates on the linked stay, not on the reservation: the
|
||||
// booking form writes reservation_time = NULL for type 'hotel' and lets
|
||||
// day_accommodations carry start day, end day and the check-in/out clock.
|
||||
// Joining them here is what lets a stay span its whole range (#1586).
|
||||
const reservations = this.db
|
||||
.prepare(
|
||||
`SELECT r.*, pl.lat AS place_lat, pl.lng AS place_lng
|
||||
`SELECT r.*, pl.lat AS place_lat, pl.lng AS place_lng,
|
||||
sd.date AS stay_start_date, ed.date AS stay_end_date
|
||||
FROM reservations r
|
||||
LEFT JOIN places pl ON r.place_id = pl.id
|
||||
LEFT JOIN day_accommodations a ON r.accommodation_id = a.id
|
||||
LEFT JOIN days sd ON a.start_day_id = sd.id
|
||||
LEFT JOIN days ed ON a.end_day_id = ed.id
|
||||
WHERE r.trip_id = ?`,
|
||||
)
|
||||
.all(tripId) as any[];
|
||||
@@ -302,6 +310,18 @@ export class CalendarService {
|
||||
// both ends in the subscribed feed (#1453). Hotels/restaurants have no
|
||||
// endpoints and fall through to reservation_time.
|
||||
const buildReservationTimeLines = (r: any): string | null => {
|
||||
// A stay wins over everything below: it is the only source that knows how
|
||||
// long the booking lasts. Emitted as an all-day range over every night, so
|
||||
// the hotel sits above the day in a subscribed calendar instead of
|
||||
// appearing once on the arrival day (#1586). DTEND is exclusive, hence +1.
|
||||
if (isDate(r.stay_start_date)) {
|
||||
const lastDay = isDate(r.stay_end_date) && r.stay_end_date >= r.stay_start_date
|
||||
? r.stay_end_date
|
||||
: r.stay_start_date;
|
||||
return `DTSTART;VALUE=DATE:${fmtDate(r.stay_start_date)}\r\n` +
|
||||
`DTEND;VALUE=DATE:${fmtDate(addDays(lastDay, 1))}\r\n`;
|
||||
}
|
||||
|
||||
const eps = endpointsMap.get(r.id);
|
||||
const ordered = eps && eps.length > 0 ? [...eps].sort((a, b) => a.sequence - b.sequence) : null;
|
||||
const first = ordered?.[0];
|
||||
@@ -389,6 +409,56 @@ export class CalendarService {
|
||||
events.push(ev);
|
||||
}
|
||||
|
||||
// Check-in and check-out as their own timed events, when the stay records the
|
||||
// clock (#1586). They are separate from the all-day stay above on purpose: an
|
||||
// all-day event cannot carry a time, and "be there at 15:00" is the part a
|
||||
// subscriber actually wants a reminder for. Emitted per stay rather than per
|
||||
// reservation so a stay whose reservation was deleted still shows them.
|
||||
const stays = this.db.prepare(`
|
||||
SELECT a.id, a.check_in, a.check_in_end, a.check_out,
|
||||
sd.date AS start_date, ed.date AS end_date,
|
||||
p.name AS place_name, p.address AS place_address, p.lat AS place_lat, p.lng AS place_lng,
|
||||
r.title AS reservation_title
|
||||
FROM day_accommodations a
|
||||
LEFT JOIN days sd ON a.start_day_id = sd.id
|
||||
LEFT JOIN days ed ON a.end_day_id = ed.id
|
||||
LEFT JOIN places p ON a.place_id = p.id
|
||||
LEFT JOIN reservations r ON r.accommodation_id = a.id
|
||||
WHERE a.trip_id = ?
|
||||
ORDER BY a.id ASC
|
||||
`).all(tripId) as any[];
|
||||
|
||||
for (const stay of stays) {
|
||||
const name = stay.reservation_title || stay.place_name || 'Accommodation';
|
||||
const zone = resolveTimeZone(stay.place_lat, stay.place_lng);
|
||||
// No end day (the day row was removed) → check out on the arrival day.
|
||||
const checkOutDate = isDate(stay.end_date) ? stay.end_date : stay.start_date;
|
||||
|
||||
const marker = (
|
||||
kind: 'checkin' | 'checkout',
|
||||
date: string,
|
||||
time: string,
|
||||
summary: string,
|
||||
endTime?: string | null,
|
||||
) => {
|
||||
const ref = `${date}T00:00`;
|
||||
let ev = `BEGIN:VEVENT\r\nUID:${uid(stay.id, kind)}\r\nDTSTAMP:${now}\r\n`;
|
||||
ev += dtLine('DTSTART', time, zone, ref);
|
||||
if (isTime(endTime)) ev += dtLine('DTEND', endTime!, zone, ref);
|
||||
ev += `SUMMARY:${esc(summary)}\r\n`;
|
||||
if (stay.place_address) ev += `LOCATION:${esc(stay.place_address)}\r\n`;
|
||||
ev += `END:VEVENT\r\n`;
|
||||
events.push(ev);
|
||||
};
|
||||
|
||||
if (isDate(stay.start_date) && isTime(stay.check_in)) {
|
||||
marker('checkin', stay.start_date, stay.check_in, `Check-in: ${name}`, stay.check_in_end);
|
||||
}
|
||||
if (isDate(checkOutDate) && isTime(stay.check_out)) {
|
||||
marker('checkout', checkOutDate, stay.check_out, `Check-out: ${name}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Every referenced zone gets a VTIMEZONE. They are emitted before the first
|
||||
// event so the TZID references resolve; keyed by TZID so a merged calendar
|
||||
// can define each one once.
|
||||
|
||||
@@ -42,6 +42,8 @@ import {
|
||||
CollectionSaveFromTripManyDto,
|
||||
CollectionPlaceUpdateDto,
|
||||
CollectionSetStatusDto,
|
||||
CollectionSetStatusManyDto,
|
||||
CollectionSetStatusFromTripDto,
|
||||
CollectionCopyToTripDto,
|
||||
CollectionInviteDto,
|
||||
CollectionInviteActionDto,
|
||||
@@ -132,6 +134,18 @@ export class CollectionsController {
|
||||
return { deleted: this.collections.deletePlacesMany(user.id, body.ids, socketId) };
|
||||
}
|
||||
|
||||
@Post('places/status-many')
|
||||
@HttpCode(200)
|
||||
setStatusMany(@CurrentUser() user: User, @Body() body: CollectionSetStatusManyDto, @Headers('x-socket-id') socketId?: string) {
|
||||
return this.collections.setStatusMany(user.id, body.ids, body.status, socketId);
|
||||
}
|
||||
|
||||
@Post('places/status-from-trip')
|
||||
@HttpCode(200)
|
||||
setStatusFromTrip(@CurrentUser() user: User, @Body() body: CollectionSetStatusFromTripDto, @Headers('x-socket-id') socketId?: string) {
|
||||
return this.collections.setStatusFromTrip(user.id, body.trip_id, body.place_ids, body.status, socketId);
|
||||
}
|
||||
|
||||
@Patch('places/:pid')
|
||||
updatePlace(
|
||||
@CurrentUser() user: User,
|
||||
|
||||
@@ -9,6 +9,8 @@ import {
|
||||
collectionSaveFromTripManyRequestSchema,
|
||||
collectionPlaceUpdateRequestSchema,
|
||||
collectionSetStatusRequestSchema,
|
||||
collectionSetStatusManyRequestSchema,
|
||||
collectionSetStatusFromTripRequestSchema,
|
||||
collectionCopyToTripRequestSchema,
|
||||
collectionInviteRequestSchema,
|
||||
collectionInviteActionRequestSchema,
|
||||
@@ -36,6 +38,8 @@ export class CollectionSaveFromTripDto extends createZodDto(collectionSaveFromTr
|
||||
export class CollectionSaveFromTripManyDto extends createZodDto(collectionSaveFromTripManyRequestSchema) {}
|
||||
export class CollectionPlaceUpdateDto extends createZodDto(collectionPlaceUpdateRequestSchema) {}
|
||||
export class CollectionSetStatusDto extends createZodDto(collectionSetStatusRequestSchema) {}
|
||||
export class CollectionSetStatusManyDto extends createZodDto(collectionSetStatusManyRequestSchema) {}
|
||||
export class CollectionSetStatusFromTripDto extends createZodDto(collectionSetStatusFromTripRequestSchema) {}
|
||||
export class CollectionCopyToTripDto extends createZodDto(collectionCopyToTripRequestSchema) {}
|
||||
export class CollectionInviteDto extends createZodDto(collectionInviteRequestSchema) {}
|
||||
export class CollectionInviteActionDto extends createZodDto(collectionInviteActionRequestSchema) {}
|
||||
|
||||
@@ -5,6 +5,13 @@ import { DatabaseService } from '../database/database.service';
|
||||
import { PermissionsService } from '../permissions/permissions.service';
|
||||
import { RealtimeService } from '../realtime/realtime.service';
|
||||
import { reclaimPlaceImage } from '../places/place-image';
|
||||
import {
|
||||
COORD_DEDUP_TOLERANCE,
|
||||
externalIdsOf,
|
||||
isPlaceDuplicate,
|
||||
trackInsertedInDedupSet,
|
||||
type DedupSet,
|
||||
} from '../places/places.helpers';
|
||||
import type {
|
||||
Collection,
|
||||
CollectionDetailResponse,
|
||||
@@ -71,32 +78,6 @@ interface PlaceRow extends CollectionPlace {
|
||||
category_icon?: string | null;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Dedup (collection-scoped ports of placeService helpers)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const COORD_DEDUP_TOLERANCE = 0.0001; // ≈ 11 m
|
||||
|
||||
interface DedupSet {
|
||||
names: Set<string>;
|
||||
coords: Array<{ lat: number; lng: number }>;
|
||||
}
|
||||
|
||||
function isCollectionPlaceDuplicate(
|
||||
candidate: { name: string | null | undefined; lat: number | null | undefined; lng: number | null | undefined },
|
||||
dedup: DedupSet,
|
||||
): boolean {
|
||||
const normalizedName = candidate.name?.trim().toLowerCase();
|
||||
if (normalizedName) return dedup.names.has(normalizedName);
|
||||
if (candidate.lat != null && candidate.lng != null) {
|
||||
return dedup.coords.some(c =>
|
||||
Math.abs(c.lat - candidate.lat!) <= COORD_DEDUP_TOLERANCE &&
|
||||
Math.abs(c.lng - candidate.lng!) <= COORD_DEDUP_TOLERANCE,
|
||||
);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
const MAX_LABELS_PER_COLLECTION = 50;
|
||||
|
||||
/**
|
||||
@@ -776,6 +757,109 @@ export class CollectionsService {
|
||||
return deleted;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the same status on several saved places at once (#1469).
|
||||
*
|
||||
* Same all-or-nothing shape as deletePlacesMany: every id is resolved and
|
||||
* permission-checked before the first write, so a list the caller may not edit
|
||||
* cannot leave half the batch applied. Rows that already carry the status are
|
||||
* counted as untouched rather than rewritten, which keeps updated_at honest.
|
||||
*/
|
||||
setStatusMany(userId: number, ids: number[], status: CollectionStatus, socketId?: string): { updated: number } {
|
||||
const touched = new Set<number>();
|
||||
for (const id of ids) {
|
||||
const collectionId = this.collectionIdOfPlace(id);
|
||||
this.assertCanEdit(userId, collectionId);
|
||||
touched.add(collectionId);
|
||||
}
|
||||
let updated = 0;
|
||||
this.db.transaction(() => {
|
||||
for (const id of ids) {
|
||||
const res = this.db.run(
|
||||
"UPDATE collection_places SET status = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ? AND status IS NOT ?",
|
||||
status, id, status,
|
||||
);
|
||||
updated += res.changes;
|
||||
}
|
||||
});
|
||||
if (updated > 0) touched.forEach(cid => this.notifyCollectionUsers(cid, socketId, 'collections:updated'));
|
||||
return { updated };
|
||||
}
|
||||
|
||||
/**
|
||||
* Set a status on every saved copy of the given trip places (#1469): "I have
|
||||
* been here now" said once from the trip, instead of hunting the place down in
|
||||
* each list it was saved to.
|
||||
*
|
||||
* The match is findMembership's, so what gets updated is exactly what the
|
||||
* place dialog listed as "saved in". Lists the caller may only read are left
|
||||
* alone rather than refused — a shared list you cannot edit should not stop
|
||||
* you marking your own.
|
||||
*/
|
||||
setStatusFromTrip(
|
||||
userId: number,
|
||||
tripId: number,
|
||||
placeIds: number[],
|
||||
status: CollectionStatus,
|
||||
socketId?: string,
|
||||
): { updated: number; places: number } {
|
||||
if (!this.db.canAccessTrip(tripId, userId)) httpError(404, 'Trip not found');
|
||||
|
||||
const sources = placeIds.length
|
||||
? this.db.all<{ id: number; name: string; lat: number | null; lng: number | null; google_place_id: string | null; google_ftid: string | null; osm_id: string | null }>(`
|
||||
SELECT id, name, lat, lng, google_place_id, google_ftid, osm_id
|
||||
FROM places WHERE trip_id = ? AND id IN (${placeIds.map(() => '?').join(',')})
|
||||
`, tripId, ...placeIds)
|
||||
: [];
|
||||
if (sources.length === 0) return { updated: 0, places: 0 };
|
||||
|
||||
const editable = this.accessibleCollectionIds(userId).filter(cid => {
|
||||
const role = this.roleOf(userId, cid);
|
||||
return role !== null && role !== 'viewer';
|
||||
});
|
||||
if (editable.length === 0) return { updated: 0, places: 0 };
|
||||
|
||||
const matched = new Set<number>();
|
||||
const placesWithMatch = new Set<number>();
|
||||
for (const src of sources) {
|
||||
for (const row of this.matchingCollectionPlaces(editable, tripId, src)) {
|
||||
matched.add(row.id);
|
||||
placesWithMatch.add(src.id);
|
||||
}
|
||||
}
|
||||
if (matched.size === 0) return { updated: 0, places: 0 };
|
||||
|
||||
const { updated } = this.setStatusMany(userId, [...matched], status, socketId);
|
||||
return { updated, places: placesWithMatch.size };
|
||||
}
|
||||
|
||||
/**
|
||||
* The saved copies of one trip place. Same signals findMembership uses — a
|
||||
* provider id, or the same spot within the dedup tolerance — plus the
|
||||
* source link a place saved out of this very trip already carries, which beats
|
||||
* inferring anything. A bare name match is left out here for the same reason
|
||||
* as there: every "Starbucks" in the library would answer to it.
|
||||
*/
|
||||
private matchingCollectionPlaces(
|
||||
collectionIds: number[],
|
||||
tripId: number,
|
||||
place: { id: number; lat: number | null; lng: number | null; google_place_id: string | null; google_ftid: string | null; osm_id: string | null },
|
||||
): Array<{ id: number }> {
|
||||
const conditions: string[] = ['(cp.source_trip_id = ? AND cp.source_place_id = ?)'];
|
||||
const params: (string | number)[] = [...collectionIds, tripId, place.id];
|
||||
if (place.google_place_id) { conditions.push('cp.google_place_id = ?'); params.push(place.google_place_id); }
|
||||
if (place.google_ftid) { conditions.push('cp.google_ftid = ?'); params.push(place.google_ftid); }
|
||||
if (place.osm_id) { conditions.push('cp.osm_id = ?'); params.push(place.osm_id); }
|
||||
if (place.lat != null && place.lng != null) {
|
||||
conditions.push('(cp.lat IS NOT NULL AND cp.lng IS NOT NULL AND abs(cp.lat - ?) <= ? AND abs(cp.lng - ?) <= ?)');
|
||||
params.push(place.lat, COORD_DEDUP_TOLERANCE, place.lng, COORD_DEDUP_TOLERANCE);
|
||||
}
|
||||
return this.db.all<{ id: number }>(`
|
||||
SELECT cp.id FROM collection_places cp
|
||||
WHERE cp.collection_id IN (${collectionIds.map(() => '?').join(',')}) AND (${conditions.join(' OR ')})
|
||||
`, ...params);
|
||||
}
|
||||
|
||||
/** Set (or clear) a saved place's custom thumbnail, reclaiming the previous upload. */
|
||||
setPlaceImage(userId: number, placeId: number, imageUrl: string | null, socketId?: string): CollectionPlace {
|
||||
const collectionId = this.collectionIdOfPlace(placeId);
|
||||
@@ -823,10 +907,15 @@ export class CollectionsService {
|
||||
sources.push(row);
|
||||
}
|
||||
|
||||
// Trip dedup set (mirrors placeService.buildDedupSet).
|
||||
const existing = this.db.all<{ name: string | null; lat: number | null; lng: number | null }>('SELECT name, lat, lng FROM places WHERE trip_id = ?', body.trip_id);
|
||||
const dedup: DedupSet = { names: new Set(), coords: [] };
|
||||
// Trip dedup set — same helpers the importers use, so a place renamed in the
|
||||
// trip is still recognised by its provider id when it is copied again (#1550).
|
||||
const existing = this.db.all<{
|
||||
name: string | null; lat: number | null; lng: number | null;
|
||||
google_place_id: string | null; google_ftid: string | null; osm_id: string | null;
|
||||
}>('SELECT name, lat, lng, google_place_id, google_ftid, osm_id FROM places WHERE trip_id = ?', body.trip_id);
|
||||
const dedup: DedupSet = { names: new Set(), coords: [], externalIds: new Set() };
|
||||
for (const r of existing) {
|
||||
for (const id of externalIdsOf(r)) dedup.externalIds.add(id);
|
||||
if (r.name) dedup.names.add(r.name.trim().toLowerCase());
|
||||
else if (r.lat != null && r.lng != null) dedup.coords.push({ lat: r.lat, lng: r.lng });
|
||||
}
|
||||
@@ -844,7 +933,10 @@ export class CollectionsService {
|
||||
// The whole copy is one logical write — atomic since the post-fold quirk pass.
|
||||
this.db.transaction(() => {
|
||||
for (const s of sources) {
|
||||
if (!body.force && isCollectionPlaceDuplicate({ name: s.name, lat: s.lat, lng: s.lng }, dedup)) {
|
||||
if (!body.force && isPlaceDuplicate({
|
||||
name: s.name, lat: s.lat, lng: s.lng,
|
||||
google_place_id: s.google_place_id, google_ftid: s.google_ftid, osm_id: s.osm_id,
|
||||
}, dedup)) {
|
||||
skipped.push({ id: s.id, name: s.name });
|
||||
continue;
|
||||
}
|
||||
@@ -861,8 +953,7 @@ export class CollectionsService {
|
||||
const votes = this.db.all<{ user_id: number; rating: number }>('SELECT user_id, rating FROM collection_place_ratings WHERE collection_place_id = ?', s.id);
|
||||
for (const v of votes) if (tripMemberIds.has(v.user_id)) insertRating.run(newPlaceId, v.user_id, v.rating);
|
||||
|
||||
if (s.name) dedup.names.add(s.name.trim().toLowerCase());
|
||||
else if (s.lat != null && s.lng != null) dedup.coords.push({ lat: s.lat, lng: s.lng });
|
||||
trackInsertedInDedupSet(s, dedup);
|
||||
copied++;
|
||||
}
|
||||
});
|
||||
@@ -896,14 +987,26 @@ export class CollectionsService {
|
||||
}
|
||||
if (conditions.length === 0) return { saved: false, lists: [] };
|
||||
|
||||
const rows = this.db.all<{ place_id: number; collection_id: number; name: string }>(`
|
||||
SELECT cp.id AS place_id, cp.collection_id, c.name
|
||||
const rows = this.db.all<{ place_id: number; collection_id: number; name: string; status: CollectionStatus }>(`
|
||||
SELECT cp.id AS place_id, cp.collection_id, c.name, cp.status
|
||||
FROM collection_places cp
|
||||
JOIN collections c ON c.id = cp.collection_id
|
||||
WHERE cp.collection_id IN (${placeholders}) AND (${conditions.join(' OR ')})
|
||||
`, ...params);
|
||||
|
||||
return { saved: rows.length > 0, lists: rows.map(r => ({ collection_id: r.collection_id, name: r.name, place_id: r.place_id })) };
|
||||
return {
|
||||
saved: rows.length > 0,
|
||||
lists: rows.map(r => {
|
||||
const role = this.roleOf(userId, r.collection_id);
|
||||
return {
|
||||
collection_id: r.collection_id,
|
||||
name: r.name,
|
||||
place_id: r.place_id,
|
||||
status: r.status ?? 'idea',
|
||||
can_edit: role !== null && role !== 'viewer',
|
||||
};
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
@@ -9,6 +9,29 @@ import crypto, { createHash, randomBytes } from 'crypto';
|
||||
export const ACCESS_TOKEN_TTL_S = 60 * 60; // 1 hour
|
||||
export const REFRESH_TOKEN_TTL_MS = 30 * 24 * 60 * 60 * 1000; // 30 days rolling
|
||||
|
||||
/**
|
||||
* How long a just-rotated refresh token still counts as "in flight" rather than
|
||||
* as a replay (#1007).
|
||||
*
|
||||
* Rotation is a race by construction: two clients sharing one token — several
|
||||
* MCP sessions, a client that retried after a timeout — can present it within
|
||||
* the same second. Without leeway the second one is read as theft and the whole
|
||||
* chain is revoked, which is why a handful of MCP tabs would all pop a login
|
||||
* window every day. RFC 9700 §4.14.2 names exactly this and asks for a short
|
||||
* grace period. Short is the point: it is measured from the *first* rotation and
|
||||
* does not slide, so a stolen token still gets caught as soon as it is used
|
||||
* outside the window.
|
||||
*/
|
||||
export const REFRESH_ROTATION_GRACE_MS = 30 * 1000;
|
||||
|
||||
/** SQLite writes CURRENT_TIMESTAMP as UTC without a zone; JS would read it as local. */
|
||||
export function parseSqliteUtc(ts: string | null | undefined): Date | null {
|
||||
if (!ts) return null;
|
||||
const iso = ts.endsWith('Z') ? ts : ts.replace(' ', 'T') + 'Z';
|
||||
const d = new Date(iso);
|
||||
return Number.isNaN(d.getTime()) ? null : d;
|
||||
}
|
||||
|
||||
// PKCE format (RFC 7636)
|
||||
export const CODE_CHALLENGE_RE = /^[A-Za-z0-9_-]{43}$/;
|
||||
export const CODE_VERIFIER_RE = /^[A-Za-z0-9\-._~]{43,128}$/;
|
||||
|
||||
@@ -18,10 +18,12 @@ import {
|
||||
ACCESS_TOKEN_TTL_S,
|
||||
CODE_CHALLENGE_RE,
|
||||
CODE_VERIFIER_RE,
|
||||
REFRESH_ROTATION_GRACE_MS,
|
||||
REFRESH_TOKEN_TTL_MS,
|
||||
generateAccessToken,
|
||||
generateRefreshToken,
|
||||
hashToken,
|
||||
parseSqliteUtc,
|
||||
timingSafeEqualHex,
|
||||
type OAuthClientRow,
|
||||
type OAuthTokenRow,
|
||||
@@ -422,6 +424,27 @@ export class OauthService {
|
||||
return ids;
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a revoked refresh token is the loser of a concurrent rotation
|
||||
* rather than a replayed one.
|
||||
*
|
||||
* Two conditions, and both matter. The revocation has to be recent, and it has
|
||||
* to have produced a successor that is still alive. The second one is what
|
||||
* keeps the window from re-opening a session someone deliberately closed: an
|
||||
* explicit revoke leaves no live child, and a chain revoked after a real replay
|
||||
* has every child revoked with it, so neither can slip through here.
|
||||
*/
|
||||
private isConcurrentRotation(row: OAuthTokenRow): boolean {
|
||||
const revokedAt = parseSqliteUtc(row.revoked_at);
|
||||
if (!revokedAt) return false;
|
||||
if (Date.now() - revokedAt.getTime() > REFRESH_ROTATION_GRACE_MS) return false;
|
||||
const successor = this.db.get<{ id: number }>(
|
||||
'SELECT id FROM oauth_tokens WHERE parent_token_id = ? AND revoked_at IS NULL LIMIT 1',
|
||||
row.id,
|
||||
);
|
||||
return !!successor;
|
||||
}
|
||||
|
||||
refreshTokens(
|
||||
rawRefreshToken: string,
|
||||
clientId: string,
|
||||
@@ -447,6 +470,22 @@ export class OauthService {
|
||||
|
||||
// ---- Replay detection (C3) ----
|
||||
if (row.revoked_at) {
|
||||
// …unless the rotation that revoked it happened seconds ago and produced a
|
||||
// successor that is still alive. That is two clients refreshing at once,
|
||||
// not theft (#1007): they share one token, both post it, and the loser used
|
||||
// to take the whole chain down with it. Issue a sibling pair off the same
|
||||
// parent so each client walks away with its own token.
|
||||
if (this.isConcurrentRotation(row)) {
|
||||
const tokens = this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null);
|
||||
this.audit.writeAudit({
|
||||
userId: row.user_id,
|
||||
action: 'oauth.token.refresh',
|
||||
details: { client_id: clientId, concurrent: true },
|
||||
ip,
|
||||
});
|
||||
return { tokens };
|
||||
}
|
||||
|
||||
// A revoked refresh token was replayed — assume token theft. Cascade-revoke the chain.
|
||||
const rootId = this.findChainRoot(row.id);
|
||||
this.revokeChain(rootId);
|
||||
|
||||
@@ -118,17 +118,48 @@ export function reclaimPhotoCache(cache: PlacePhotoCacheService, googlePlaceId:
|
||||
export interface DedupSet {
|
||||
names: Set<string>;
|
||||
coords: Array<{ lat: number; lng: number }>;
|
||||
/** Provider ids (google_place_id, google_ftid, osm_id) of the places already in the trip. */
|
||||
externalIds: Set<string>;
|
||||
}
|
||||
|
||||
/** The provider ids a candidate can be recognised by, ignoring blanks. */
|
||||
export function externalIdsOf(candidate: {
|
||||
google_place_id?: string | null;
|
||||
google_ftid?: string | null;
|
||||
osm_id?: string | null;
|
||||
}): string[] {
|
||||
return [candidate.google_place_id, candidate.google_ftid, candidate.osm_id]
|
||||
.filter((id): id is string => typeof id === 'string' && id.trim() !== '')
|
||||
.map(id => id.trim());
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if a candidate place is already represented in the dedup set.
|
||||
* Named places match by case-insensitive name; unnamed places fall back to
|
||||
* coordinate proximity.
|
||||
*
|
||||
* The provider id comes first, because it is the only part of a place that
|
||||
* survives the user editing it. Re-importing a Google Maps list used to duplicate
|
||||
* every place someone had renamed (#1550): the name no longer matched, and the
|
||||
* coordinate fallback never ran for a named candidate. The ids were already being
|
||||
* stored, and even backfilled onto matched places, they were simply never read.
|
||||
*
|
||||
* Name and coordinates keep their previous roles below it, deliberately unchanged:
|
||||
* widening the coordinate check to named places would merge the restaurant and the
|
||||
* bar in the same building, which is a worse failure than the one being fixed.
|
||||
*/
|
||||
export function isPlaceDuplicate(
|
||||
candidate: { name: string | null | undefined; lat: number | null; lng: number | null },
|
||||
candidate: {
|
||||
name: string | null | undefined;
|
||||
lat: number | null;
|
||||
lng: number | null;
|
||||
google_place_id?: string | null;
|
||||
google_ftid?: string | null;
|
||||
osm_id?: string | null;
|
||||
},
|
||||
dedup: DedupSet,
|
||||
): boolean {
|
||||
for (const id of externalIdsOf(candidate)) {
|
||||
if (dedup.externalIds.has(id)) return true;
|
||||
}
|
||||
const normalizedName = candidate.name?.trim().toLowerCase();
|
||||
if (normalizedName) return dedup.names.has(normalizedName);
|
||||
if (candidate.lat != null && candidate.lng != null) {
|
||||
@@ -143,9 +174,17 @@ export function isPlaceDuplicate(
|
||||
|
||||
/** Record a newly inserted place so subsequent candidates in the same batch are checked against it. */
|
||||
export function trackInsertedInDedupSet(
|
||||
place: { name: string | null | undefined; lat: number | null; lng: number | null },
|
||||
place: {
|
||||
name: string | null | undefined;
|
||||
lat: number | null;
|
||||
lng: number | null;
|
||||
google_place_id?: string | null;
|
||||
google_ftid?: string | null;
|
||||
osm_id?: string | null;
|
||||
},
|
||||
dedup: DedupSet,
|
||||
): void {
|
||||
for (const id of externalIdsOf(place)) dedup.externalIds.add(id);
|
||||
const normalizedName = place.name?.trim().toLowerCase();
|
||||
if (normalizedName) {
|
||||
dedup.names.add(normalizedName);
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
MAX_LIST_RESPONSE_BYTES,
|
||||
googleMapsFeatureIdFromItem,
|
||||
gpxParser,
|
||||
externalIdsOf,
|
||||
isPlaceDuplicate,
|
||||
kmlParser,
|
||||
mapWithConcurrency,
|
||||
@@ -422,25 +423,46 @@ export class PlacesService {
|
||||
|
||||
/** Build a lookup of names/coords for places already in a trip. */
|
||||
private buildDedupSet(tripId: string): DedupSet {
|
||||
const rows = this.dbs.all<{ name: string | null; lat: number | null; lng: number | null }>(
|
||||
'SELECT name, lat, lng FROM places WHERE trip_id = ?', tripId,
|
||||
const rows = this.dbs.all<{
|
||||
name: string | null; lat: number | null; lng: number | null;
|
||||
google_place_id: string | null; google_ftid: string | null; osm_id: string | null;
|
||||
}>(
|
||||
'SELECT name, lat, lng, google_place_id, google_ftid, osm_id FROM places WHERE trip_id = ?', tripId,
|
||||
);
|
||||
const names = new Set<string>();
|
||||
const coords: Array<{ lat: number; lng: number }> = [];
|
||||
// Provider ids are collected for every place, named or not: they are what lets a
|
||||
// renamed place still be recognised on a re-import (#1550).
|
||||
const externalIds = new Set<string>();
|
||||
for (const row of rows) {
|
||||
for (const id of externalIdsOf(row)) externalIds.add(id);
|
||||
if (row.name) {
|
||||
names.add(row.name.trim().toLowerCase());
|
||||
} else if (row.lat != null && row.lng != null) {
|
||||
coords.push({ lat: row.lat, lng: row.lng });
|
||||
}
|
||||
}
|
||||
return { names, coords };
|
||||
return { names, coords, externalIds };
|
||||
}
|
||||
|
||||
private findDuplicatePlace(
|
||||
tripId: string,
|
||||
place: { name: string | null | undefined; lat: number | null; lng: number | null },
|
||||
place: {
|
||||
name: string | null | undefined; lat: number | null; lng: number | null;
|
||||
google_place_id?: string | null; google_ftid?: string | null; osm_id?: string | null;
|
||||
},
|
||||
): { id: number; google_ftid: string | null } | null {
|
||||
// Same order as isPlaceDuplicate: the provider id wins, because it is what
|
||||
// survives a rename (#1550).
|
||||
for (const id of externalIdsOf(place)) {
|
||||
const byId = 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 = ?)
|
||||
ORDER BY id ASC
|
||||
LIMIT 1
|
||||
`, tripId, id, id, id);
|
||||
if (byId) return byId;
|
||||
}
|
||||
const normalizedName = place.name?.trim().toLowerCase();
|
||||
if (normalizedName) {
|
||||
const duplicate = this.dbs.get<{ id: number; google_ftid: string | null }>(`
|
||||
@@ -901,7 +923,7 @@ export class PlacesService {
|
||||
let skipped = 0;
|
||||
this.dbs.transaction(() => {
|
||||
for (const p of places) {
|
||||
if (isPlaceDuplicate({ name: p.name, lat: p.lat, lng: p.lng }, dedup)) {
|
||||
if (isPlaceDuplicate({ name: p.name, lat: p.lat, lng: p.lng, google_ftid: p.googleFtid }, dedup)) {
|
||||
const duplicate = this.findDuplicatePlace(tripId, p);
|
||||
if (duplicate && !duplicate.google_ftid && p.googleFtid) {
|
||||
updateGoogleFtidStmt.run(p.googleFtid, duplicate.id);
|
||||
@@ -912,7 +934,7 @@ export class PlacesService {
|
||||
const result = insertStmt.run(tripId, p.name, p.lat, p.lng, p.notes, p.googleFtid);
|
||||
const place = this.dbs.getPlaceWithTags(Number(result.lastInsertRowid))!;
|
||||
created.push(place);
|
||||
trackInsertedInDedupSet({ name: p.name, lat: p.lat, lng: p.lng }, dedup);
|
||||
trackInsertedInDedupSet({ name: p.name, lat: p.lat, lng: p.lng, google_ftid: p.googleFtid }, dedup);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user