k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-11-14 00:01:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(atlas): count a place's country on the dashboard the way the Atlas map does

Browse Source 
The passport card and the last/next trip countries read place countries
from the place_regions cache alone, while the Atlas map falls back to the
bundled borders for a place without a cached region. A place in Vatican
City never gets a region row (the region bundle has no VA features and
Nominatim names no region there), so the map counted VA and the card did
not. The same gap hid any country whose place was still waiting for its
background lookup, or whose lookup had failed.

Both views now resolve each place through one helper: the cached country
first, the borders otherwise, once per location. The dashboard reads every
place of a started trip with its cached country instead of the distinct
cached codes, and the trip countries count resolved places the same way.
The MCP travel stats tool and the public stats API use the same service
methods and are fixed with it.

Closes #2875
This commit is contained in:
Maurice
2026-10-29 14:52:47 +01:00
parent 3238fbc517
commit 9ab2fc30ff
5 changed files with 229 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/db/repositories/PlaceRegions.repository.ts
+35 -31
View File
@@ -10,6 +10,15 @@ export interface PlaceRegionRow {
region_name: string;
}
/** A place's location next to the country its cached region holds, null while it has no cached region. */
interface PlaceCountryRow {
id: number;
lat: number | null;
lng: number | null;
address: string | null;
country_code: string | null;
}
type PlaceRegionsKyselyDB = Pick<DB, 'place_regions' | 'places' | 'trips' | 'trip_members'>;
/**
@@ -18,7 +27,7 @@ type PlaceRegionsKyselyDB = Pick<DB, 'place_regions' | 'places' | 'trips' | 'tri
* but atlas is its only writer and its only reader beyond that FK — the
* inventory's own "effectively an atlas-owned repository" note — so every
* statement that reads or writes it, including the ones that join out to
* `places`/`trips`/`trip_members` for `getTravelStats`'s country lists
* `places`/`trips`/`trip_members` for the dashboard's country lists
* (AT40/AT44), lives here rather than split across those tables' own
* repositories.
*/
@@ -162,50 +171,45 @@ export class PlaceRegionsRepository extends TrekRepository<PlaceRegions> {
}
/**
* AT40 (`lastTrip`) — `SELECT pr.country_code, COUNT(DISTINCT p.id) AS
* places FROM place_regions pr JOIN places p ON p.id = pr.place_id WHERE
* p.trip_id = ? AND pr.country_code IS NOT NULL GROUP BY pr.country_code
* ORDER BY places DESC, pr.country_code ASC`.
* AT40 (`lastTrip`/`nextTrip`) — `SELECT p.id, p.lat, p.lng, p.address,
* pr.country_code FROM places p LEFT JOIN place_regions pr ON pr.place_id =
* p.id WHERE p.trip_id = ? ORDER BY p.id`. A place without a cached region
* comes back with a null country, for the caller to resolve from its
* location (#2875).
*/
async countPlacesByCountryForTrip(tripId: number): Promise<{ country_code: string; places: number }[]> {
const rows = await this.db()
.selectFrom('place_regions as pr')
.innerJoin('places as p', 'p.id', 'pr.place_id')
.select((eb) => ['pr.country_code', eb.fn.count<number>('p.id').distinct().as('places')])
async listPlaceCountriesForTrip(tripId: number): Promise<PlaceCountryRow[]> {
return await this.db()
.selectFrom('places as p')
.leftJoin('place_regions as pr', 'pr.place_id', 'p.id')
.select(['p.id', 'p.lat', 'p.lng', 'p.address', 'pr.country_code'])
.where('p.trip_id', '=', tripId)
.where('pr.country_code', 'is not', null)
.groupBy('pr.country_code')
.orderBy('places', 'desc')
.orderBy('pr.country_code', 'asc')
.orderBy('p.id')
.execute();
return rows.map((r) => ({ country_code: r.country_code, places: Number(r.places) }));
}
/**
* AT44 (`getTravelStats`) — `SELECT DISTINCT pr.country_code FROM
* place_regions pr JOIN places p ON p.id = pr.place_id JOIN trips t ON
* p.trip_id = t.id LEFT JOIN trip_members tm ON t.id = tm.trip_id WHERE
* (t.user_id = ? OR tm.user_id = ?) AND pr.country_code IS NOT NULL AND
* AT44 (`getTravelStats`) — `SELECT DISTINCT p.id, p.lat, p.lng, p.address,
* pr.country_code FROM places p JOIN trips t ON p.trip_id = t.id LEFT JOIN
* trip_members tm ON t.id = tm.trip_id LEFT JOIN place_regions pr ON
* pr.place_id = p.id WHERE (t.user_id = ? OR tm.user_id = ?) AND
* COALESCE(t.start_date, t.end_date) IS NOT NULL AND COALESCE(t.start_date,
* t.end_date) <= date('now')` — "only started trips count" (#1048).
* `today` (`date('now')`, UTC) is resolved by the caller once
* (`todayUtc()`, the same value `stats()`/`lastTrip` already compute) and
* bound here, the `TripsRepository.activeTrip(user_id, today)` precedent
* for a literal `date('now')` comparison.
* t.end_date) <= ? ORDER BY p.id` — "only started trips count" (#1048).
* Every place comes back, a place without a cached region with a null
* country (#2875). `today` (UTC) is resolved by the caller once
* (`todayUtc()`) and bound here.
*/
async listVisitedCountryCodesForUser(userId: number, today: string): Promise<string[]> {
const rows = await this.db()
.selectFrom('place_regions as pr')
.innerJoin('places as p', 'p.id', 'pr.place_id')
async listStartedTripPlaceCountriesForUser(userId: number, today: string): Promise<PlaceCountryRow[]> {
return await this.db()
.selectFrom('places as p')
.innerJoin('trips as t', 't.id', 'p.trip_id')
.leftJoin('trip_members as tm', 'tm.trip_id', 't.id')
.select('pr.country_code')
.leftJoin('place_regions as pr', 'pr.place_id', 'p.id')
.select(['p.id', 'p.lat', 'p.lng', 'p.address', 'pr.country_code'])
.distinct()
.where((eb) => eb.or([eb('t.user_id', '=', userId), eb('tm.user_id', '=', userId)]))
.where('pr.country_code', 'is not', null)
.where((eb) => eb(eb.fn.coalesce('t.start_date', 't.end_date'), 'is not', null))
.where((eb) => eb(eb.fn.coalesce('t.start_date', 't.end_date'), '<=', today))
.orderBy('p.id')
.execute();
return rows.map((r) => r.country_code);
}
}
server/src/nest/atlas/atlas.service.ts
+26 -26
View File
@@ -39,6 +39,7 @@ import { KNOWN_COUNTRIES } from './known-countries';
import { cityFromAddress } from './city-from-address';
import { transferEndpointIds } from './transfer-endpoints';
import { countryVisitDates } from './visit-dates';
import { placeCountryResolver } from './place-countries';
import { haversineKm } from '../common/geo';
/** The part of a place that its cached region is derived from. */
@@ -213,21 +214,12 @@ export class AtlasService {
const cached = placeIds.length > 0 ? await this.placeRegions.listCountryCodesForPlaceIds(placeIds) : []; // AT3
const cachedMap = new Map(cached.map((r) => [r.place_id, r.country_code]));
const countryOf = placeCountryResolver();
const uncachedForGeocode: PlaceRow[] = [];
for (const p of places) {
const fromDb = cachedMap.get(p.id);
if (fromDb) {
out.set(p.id, fromDb);
continue;
}
const sync = resolveCountryCodeSync(p);
if (sync) {
out.set(p.id, sync);
continue;
}
if (p.lat && p.lng && !geocodingInFlight.has(p.id)) {
uncachedForGeocode.push(p);
}
const code = countryOf(p, cachedMap.get(p.id));
if (code) out.set(p.id, code);
else if (p.lat && p.lng && !geocodingInFlight.has(p.id)) uncachedForGeocode.push(p);
}
this.cacheRegionsInBackground(uncachedForGeocode); // AT4
@@ -1005,11 +997,9 @@ export class AtlasService {
* side of a work trip), and without the second key which one is "last" would be
* whatever the storage engine happened to return first.
*
* Countries come from `place_regions`, Atlas's own cache, most-visited first.
* No fallback to `getCountryFromCoords` here on purpose: this answers a
* dashboard widget, and a point-in-polygon scan over 4MB of boundaries is too
* much to spend on a label. An unresolved trip reports an empty list, which the
* caller renders as "no country" rather than as a wrong one.
* Countries are resolved per place the way the Atlas map resolves them, most
* visited first. A trip none of whose places can be placed reports an empty
* list, which the caller renders as "no country" rather than as a wrong one.
*/
async lastTrip(userId: number): Promise<{ title: string; start_date: string | null; end_date: string | null; countries: string[] } | null> {
const trip = await this.trips.lastStartedTrip(userId, todayUtc()); // AT39
@@ -1047,10 +1037,17 @@ export class AtlasService {
};
}
/** The countries a trip's places resolved to in `place_regions`, most-visited first. */
/** The countries a trip's places sit in, most-visited first, ties by code. */
private async tripCountries(tripId: number): Promise<string[]> {
const rows = await this.placeRegions.countPlacesByCountryForTrip(tripId); // AT40
return rows.map((r) => r.country_code.toUpperCase());
const countryOf = placeCountryResolver();
const placesPerCountry = new Map<string, number>();
for (const place of await this.placeRegions.listPlaceCountriesForTrip(tripId)) { // AT40
const code = countryOf(place, place.country_code)?.toUpperCase();
if (code) placesPerCountry.set(code, (placesPerCountry.get(code) ?? 0) + 1);
}
return [...placesPerCountry]
.sort(([codeA, a], [codeB, b]) => b - a || (codeA < codeB ? -1 : 1))
.map(([code]) => code);
}
async getTravelStats(userId: number) {
@@ -1076,17 +1073,20 @@ export class AtlasService {
if (cityPart) cities.add(cityPart);
});
// Visited countries — same source the Atlas page uses: ISO-2 codes from
// auto-resolved place regions plus countries the user marked manually.
// Visited countries: the ones the user marked by hand, plus the countries of
// their places, resolved the way the Atlas map resolves them (#2875).
const countryCodes = new Set<string>();
const manualCountries = await this.visitedCountries.listCodesForUser(userId); // AT43
manualCountries.forEach(code => { if (code) countryCodes.add(code.toUpperCase()); });
// Only trips that have already started count as visited — a country you have merely
// booked a trip to isn't stamped in the passport yet, and one you jotted down without
// any dates even less so (#1048). date('now') is UTC, matching tripVisitStatus.
const placeRegionCodes = await this.placeRegions.listVisitedCountryCodesForUser(userId, today); // AT44
placeRegionCodes.forEach(code => { if (code) countryCodes.add(code.toUpperCase()); });
// any dates even less so (#1048). `today` is UTC, matching tripVisitStatus.
const countryOf = placeCountryResolver();
for (const place of await this.placeRegions.listStartedTripPlaceCountriesForUser(userId, today)) { // AT44
const code = countryOf(place, place.country_code);
if (code) countryCodes.add(code.toUpperCase());
}
// Transport bookings don't create a place row, so their geocoded endpoints never
// reached place_regions — a country reached only by a flight/train (no lodging or
server/src/nest/atlas/place-countries.ts
+29
View File
@@ -0,0 +1,29 @@
import type { PlaceRow } from '../../db/repositories/Places.repository';
import { resolveCountryCodeSync } from './atlas-geo';
type LocatedPlace = Pick<PlaceRow, 'lat' | 'lng' | 'address'>;
/**
* The one way a place gets its country, for every view that counts countries
* (#2875): the country of its cached region first, the bundled borders for a
* place that has no cached region. place_regions is a region cache, and a place
* in a country the region bundle does not cover (Vatican City) or one whose
* background lookup has not finished holds no row there, so a count that read the
* cache alone lost countries the Atlas map shows.
*
* The border lookup is a point-in-polygon test, so each resolver runs it once per
* location: the places of a city trip mostly share their coordinates or address.
*/
export function placeCountryResolver(): (place: LocatedPlace, cached: string | null | undefined) => string | null {
const byLocation = new Map<string, string | null>();
return (place, cached) => {
if (cached) return cached;
const key = `${place.lat},${place.lng},${place.address ?? ''}`;
let code = byLocation.get(key);
if (code === undefined) {
code = resolveCountryCodeSync(place);
byLocation.set(key, code);
}
return code;
};
}