diff --git a/server/src/db/repositories/PlaceRegions.repository.ts b/server/src/db/repositories/PlaceRegions.repository.ts index 2cfa6e34e..147eeb83b 100644 --- a/server/src/db/repositories/PlaceRegions.repository.ts +++ b/server/src/db/repositories/PlaceRegions.repository.ts @@ -19,6 +19,11 @@ interface PlaceCountryRow { country_code: string | null; } +/** A place of a started trip, with the trip's last day (its end, or its start when it has none) for ordering by recency. */ +export interface StartedTripPlaceCountryRow extends PlaceCountryRow { + trip_last_date: string | null; +} + type PlaceRegionsKyselyDB = Pick; /** @@ -192,22 +197,32 @@ export class PlaceRegionsRepository extends TrekRepository { /** * 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 + * pr.country_code, COALESCE(t.end_date, t.start_date) AS trip_last_date + * 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) <= ? 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. + * (`todayUtc()`) and bound here. A place sits on one trip, so the trip's + * last day adds no row to the DISTINCT; the dashboard orders the countries + * by it (#2648). */ - async listStartedTripPlaceCountriesForUser(userId: number, today: string): Promise { + async listStartedTripPlaceCountriesForUser(userId: number, today: string): Promise { 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') .leftJoin('place_regions as pr', 'pr.place_id', 'p.id') - .select(['p.id', 'p.lat', 'p.lng', 'p.address', 'pr.country_code']) + .select((eb) => [ + 'p.id', + 'p.lat', + 'p.lng', + 'p.address', + 'pr.country_code', + eb.fn.coalesce('t.end_date', 't.start_date').as('trip_last_date'), + ]) .distinct() .where((eb) => eb.or([eb('t.user_id', '=', userId), eb('tm.user_id', '=', userId)])) .where((eb) => eb(eb.fn.coalesce('t.start_date', 't.end_date'), 'is not', null)) diff --git a/server/src/db/repositories/ReservationEndpoints.repository.ts b/server/src/db/repositories/ReservationEndpoints.repository.ts index b75e2cb2b..be99803d2 100644 --- a/server/src/db/repositories/ReservationEndpoints.repository.ts +++ b/server/src/db/repositories/ReservationEndpoints.repository.ts @@ -201,9 +201,12 @@ export class ReservationEndpointsRepository extends TrekRepository { + async listOwnedEndpointsForUser(userId: number, today: string): Promise { const rows = await this.kysely() .selectFrom('reservation_endpoints as e') .innerJoin('reservations as r', 'r.id', 'e.reservation_id') @@ -228,6 +231,7 @@ export class ReservationEndpointsRepository extends TrekRepository eb.or([eb('t.user_id', '=', userId), eb('tm.user_id', '=', userId)])) @@ -236,7 +240,7 @@ export class ReservationEndpointsRepository extends TrekRepository eb(eb.fn.coalesce('t.start_date', 't.end_date'), '<=', today)) .where((eb) => travelerOwnsExpr(eb, userId)) .execute(); - return rows as TravelerOwnedEndpointRow[]; + return rows as TravelerOwnedEndpointWithTripDateRow[]; } /** @@ -318,6 +322,11 @@ export interface TravelerOwnedEndpointRow { fallback_time: string | null; } +/** AT45's projection: AT6's, plus the last day of the trip the endpoint is on. */ +export interface TravelerOwnedEndpointWithTripDateRow extends TravelerOwnedEndpointRow { + trip_last_date: string | null; +} + /** {@link ReservationEndpointsRepository.listOwnedEndpointsForTrips}'s `reservation_endpoints`/`reservations`/`reservation_travelers` tables (`r` aliased for `travelerOwnsExpr`, R7). */ type TravelerOwnedEndpointsKyselyDB = ReservationTravelersOwnsKyselyDB & Pick; diff --git a/server/src/nest/atlas/atlas.service.ts b/server/src/nest/atlas/atlas.service.ts index fffbf0830..42dafc56b 100644 --- a/server/src/nest/atlas/atlas.service.ts +++ b/server/src/nest/atlas/atlas.service.ts @@ -70,6 +70,19 @@ function daysBetween(today: string, isoDate: string): number { */ type EndpointRow = TravelerOwnedEndpointRow; +/** + * The visited countries in the order the dashboard shows them (#2648): most + * recently visited first, by the last day of the latest started trip that + * reached each one, so the few flags the tile has room for are the ones of the + * last journeys rather than the start of the alphabet. A country only marked + * by hand carries no date and follows, in the order it was marked. + */ +export function countriesByLastVisit(lastVisit: ReadonlyMap, manual: Iterable): string[] { + // Array.prototype.sort is stable, so countries of the same trip keep the order they were found in. + const dated = [...lastVisit].sort(([, a], [, b]) => (a < b ? 1 : a > b ? -1 : 0)).map(([code]) => code); + return [...new Set([...dated, ...manual])]; +} + export type CreateBucketData = { name: string; lat?: number | null; @@ -1069,18 +1082,26 @@ export class AtlasService { }); // 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(); - const manualCountries = await this.visitedCountries.listCodesForUser(userId); // AT43 - manualCountries.forEach(code => { if (code) countryCodes.add(code.toUpperCase()); }); + // their places, resolved the way the Atlas map resolves them (#2875). Each + // reached country keeps the last day of its latest trip, for the order. + const manualCountries = (await this.visitedCountries.listCodesForUser(userId)) // AT43 + .filter(Boolean) + .map(code => code.toUpperCase()); + const lastVisit = new Map(); + const noteVisit = (code: string | null | undefined, tripLastDate: string | null) => { + if (!code) return; + const key = code.toUpperCase(); + const when = tripLastDate ?? ''; + const known = lastVisit.get(key); + if (known === undefined || when > known) lastVisit.set(key, when); + }; // 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). `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()); + noteVisit(countryOf(place, place.country_code), place.trip_last_date); } // Transport bookings don't create a place row, so their geocoded endpoints never @@ -1091,29 +1112,33 @@ export class AtlasService { // connection/layover (e.g. a plane change) the traveler never really visited (#1486), // and the same layover split over two bookings is recognised from its ground time // instead, because there it is stored as a legitimate 'to'/'from' pair (#1535). - const endpointRows: EndpointRow[] = await this.reservationEndpoints.listOwnedEndpointsForUser(userId, today); // AT45, TRAVELER_OWNS (#1966) + const endpointRows = await this.reservationEndpoints.listOwnedEndpointsForUser(userId, today); // AT45, TRAVELER_OWNS (#1966) const transfers = transferEndpointIds( endpointRows.filter(e => e.reservation_type === 'flight' && e.reservation_status !== 'cancelled') ); // The DISTINCT no longer collapses per coordinate now that the endpoint id has to be // in the projection, so collapse here instead: getCountryFromCoords is a - // point-in-polygon scan and must stay at one run per coordinate. - const endpointCoords = new Map(); + // point-in-polygon scan and must stay at one run per coordinate. The latest + // trip through a coordinate is the one its country is dated by. + const endpointCoords = new Map(); for (const e of endpointRows) { if (transfers.has(e.id)) continue; - endpointCoords.set(`${e.lat},${e.lng}`, { lat: e.lat, lng: e.lng }); + const key = `${e.lat},${e.lng}`; + const known = endpointCoords.get(key); + if (!known || (e.trip_last_date ?? '') > (known.tripLastDate ?? '')) { + endpointCoords.set(key, { lat: e.lat, lng: e.lng, tripLastDate: e.trip_last_date }); + } } for (const e of endpointCoords.values()) { - const code = getCountryFromCoords(e.lat, e.lng); - if (code) countryCodes.add(code.toUpperCase()); + noteVisit(getCountryFromCoords(e.lat, e.lng), e.tripLastDate); } // Countries the user removed in Atlas stay removed on the dashboard too, so the // passport card and the Atlas map agree (#1490). - for (const code of await this.getHiddenCountries(userId)) countryCodes.delete(code.toUpperCase()); + const hidden = new Set([...(await this.getHiddenCountries(userId))].map(code => code.toUpperCase())); return { - countries: [...countryCodes], + countries: countriesByLastVisit(lastVisit, manualCountries).filter(code => !hidden.has(code)), cities: [...cities], coords, totalTrips: tripStats.trips || 0, diff --git a/server/tests/unit/db/repositories/PlaceRegions.repository.test.ts b/server/tests/unit/db/repositories/PlaceRegions.repository.test.ts index 4926c7aa2..ba87805db 100644 --- a/server/tests/unit/db/repositories/PlaceRegions.repository.test.ts +++ b/server/tests/unit/db/repositories/PlaceRegions.repository.test.ts @@ -166,11 +166,26 @@ describe('PlaceRegionsRepository.listStartedTripPlaceCountriesForUser (AT44)', ( const today = '2026-01-01'; const expected = [ - { id: cached.id, lat: 48.85, lng: 2.35, address: null, country_code: 'FR' }, - { id: uncached.id, lat: 41.9022, lng: 12.4539, address: null, country_code: null }, + { id: cached.id, lat: 48.85, lng: 2.35, address: null, country_code: 'FR', trip_last_date: '2020-01-10' }, + { id: uncached.id, lat: 41.9022, lng: 12.4539, address: null, country_code: null, trip_last_date: '2020-01-10' }, ]; expect(await repo.listStartedTripPlaceCountriesForUser(owner.id, today)).toEqual(expected); expect(await repo.listStartedTripPlaceCountriesForUser(member.id, today)).toEqual(expected); expect(await repo.listStartedTripPlaceCountriesForUser(stranger.id, today)).toEqual([]); }); + + it('PLACEREGREPO-010: dates each place by its own trip, the start standing in for a missing end (#2648)', async () => { + const { user } = createUser(testDb); + const ended = createTrip(testDb, user.id, { start_date: '2020-01-01', end_date: '2020-01-10' }); + const openEnded = createTrip(testDb, user.id, { start_date: '2021-07-01' }); + const first = createPlace(testDb, ended.id); + const second = createPlace(testDb, openEnded.id); + + const rows = await repo.listStartedTripPlaceCountriesForUser(user.id, '2026-01-01'); + + expect(rows.map((row) => [row.id, row.trip_last_date])).toEqual([ + [first.id, '2020-01-10'], + [second.id, '2021-07-01'], + ]); + }); }); diff --git a/server/tests/unit/db/repositories/_shared/atlas-travowns-consumers.parity.test.ts b/server/tests/unit/db/repositories/_shared/atlas-travowns-consumers.parity.test.ts index 37a84725a..b17f9a4d3 100644 --- a/server/tests/unit/db/repositories/_shared/atlas-travowns-consumers.parity.test.ts +++ b/server/tests/unit/db/repositories/_shared/atlas-travowns-consumers.parity.test.ts @@ -41,13 +41,15 @@ const AT6 = (n: number) => ` WHERE r.trip_id IN (${Array(n).fill('?').join(',')}) AND e.role IN ('from', 'to') AND ${TRAVELER_OWNS}`; +// AT45 also names the trip's last day since #2648, the dashboard's order of countries. const AT45 = ` SELECT DISTINCT e.id, e.reservation_id, r.trip_id, e.role, e.code, e.lat, e.lng, e.local_date, e.local_time, r.type AS reservation_type, r.status AS reservation_status, CASE e.role WHEN 'to' THEN COALESCE(r.reservation_end_time, r.reservation_time) ELSE COALESCE(r.reservation_time, r.reservation_end_time) - END AS fallback_time + END AS fallback_time, + COALESCE(t.end_date, t.start_date) AS trip_last_date FROM reservation_endpoints e JOIN reservations r ON e.reservation_id = r.id JOIN trips t ON r.trip_id = t.id diff --git a/server/tests/unit/nest/travel-stats.service.test.ts b/server/tests/unit/nest/travel-stats.service.test.ts index d06d3653e..38678ede6 100644 --- a/server/tests/unit/nest/travel-stats.service.test.ts +++ b/server/tests/unit/nest/travel-stats.service.test.ts @@ -509,3 +509,71 @@ describe('personal figures on a shared trip (#1966)', () => { expect(a.totalDistanceKm).toBeGreaterThan(0); }); }); + +// ── The order of the passport card's countries (#2648) ────────────────────── + +describe('getTravelStats country order (#2648)', () => { + async function placeIn(tripId: number, lat: number, lng: number) { + await insertRow(await sharedTestOrm(testDb), Places, { trip: tripId, name: 'Stop' }, { lat, lng }); + } + + async function markByHand(userId: number, code: string) { + await insertRow(await sharedTestOrm(testDb), VisitedCountries, { user: userId, country_code: code }); + } + + it('lists the countries of the latest trip first, whatever the alphabet says', async () => { + const { user } = createUser(testDb); + const austria = createTrip(testDb, user.id, { title: 'Vienna', start_date: '2022-03-01', end_date: '2022-03-05' }); + const japan = createTrip(testDb, user.id, { title: 'Tokyo', start_date: '2024-10-01', end_date: '2024-10-12' }); + const france = createTrip(testDb, user.id, { title: 'Paris', start_date: '2023-06-01', end_date: '2023-06-04' }); + await placeIn(austria.id, 48.2082, 16.3738); + await placeIn(japan.id, 35.6762, 139.6503); + await placeIn(france.id, 48.8606, 2.3376); + + expect((await atlas.getTravelStats(user.id)).countries).toEqual(['JP', 'FR', 'AT']); + }); + + it('dates a country by its most recent visit, places and bookings alike', async () => { + const { user } = createUser(testDb); + const early = createTrip(testDb, user.id, { title: 'Paris', start_date: '2021-05-01', end_date: '2021-05-03' }); + const middle = createTrip(testDb, user.id, { title: 'Rome', start_date: '2022-05-01', end_date: '2022-05-03' }); + const late = createTrip(testDb, user.id, { + title: 'Back to France', + start_date: '2023-05-01', + end_date: '2023-05-03', + }); + await placeIn(early.id, 48.8606, 2.3376); + await placeIn(middle.id, 41.8902, 12.4922); + // The return to France is a booking only: its arrival dates FR again, after Rome. + const flight = createReservation(testDb, late.id, { type: 'flight' }); + await makeReservationEndpoint(await sharedTestOrm(testDb), flight.id, { + role: 'to', + sequence: 1, + name: 'Paris', + lat: 49.0097, + lng: 2.5479, + }); + + expect((await atlas.getTravelStats(user.id)).countries).toEqual(['FR', 'IT']); + }); + + it('puts a country only marked by hand after every country a trip reached', async () => { + const { user } = createUser(testDb); + await markByHand(user.id, 'NZ'); + const trip = createTrip(testDb, user.id, { title: 'Paris', start_date: '2023-06-01', end_date: '2023-06-04' }); + await placeIn(trip.id, 48.8606, 2.3376); + + expect((await atlas.getTravelStats(user.id)).countries).toEqual(['FR', 'NZ']); + }); + + it('lists a country both marked by hand and reached by a trip once, by the trip', async () => { + const { user } = createUser(testDb); + await markByHand(user.id, 'AT'); + const austria = createTrip(testDb, user.id, { title: 'Vienna', start_date: '2024-03-01', end_date: '2024-03-05' }); + const france = createTrip(testDb, user.id, { title: 'Paris', start_date: '2023-06-01', end_date: '2023-06-04' }); + await placeIn(austria.id, 48.2082, 16.3738); + await placeIn(france.id, 48.8606, 2.3376); + + expect((await atlas.getTravelStats(user.id)).countries).toEqual(['AT', 'FR']); + }); +}); diff --git a/wiki/My-Trips-Dashboard.md b/wiki/My-Trips-Dashboard.md index 36d378deb..c60b6be4b 100644 --- a/wiki/My-Trips-Dashboard.md +++ b/wiki/My-Trips-Dashboard.md @@ -48,7 +48,7 @@ Under the spotlight card on desktop, a row of four tiles sums up all your trips: | Tile | What it shows | |---|---| -| **Atlas · Countries visited** | The number of countries found in the places and transports of your trips, of 195, with the first flags. A country you removed in [Atlas](Atlas) stays removed here. | +| **Atlas · Countries visited** | The number of countries found in the places and transports of your trips, of 195, with the flags of the countries you visited most recently first; a country you only marked by hand in Atlas comes after those. A country you removed in [Atlas](Atlas) stays removed here. | | **Trips total** | How many trips you have, and how many places are mapped on them. | | **Days traveled** | The days across all trips. | | **Distance flown** | The distance of your flights, and what that comes to in trips around the equator. |