fix: follow-ups from the review of the price, Immich, moving day and import fixes

Saved place price (#2471)
- A price above zero that would be left without a currency is refused
  with a 400, over REST and through update_collection_place alike. A list
  has no base currency, so such an amount was read in each viewer's own
  default currency and went into a trip as the trip's. A price sent alone
  keeps the stored currency, and a zero price or clearing both fields
  still goes through. The check sits in the service because the MCP tool
  spreads the contract's shape, where a refine would never arrive.
- An older row that holds an amount without a currency shows it as a
  bare number instead of in the viewer's own currency.
- The Hungarian hint uses the informal register of its file, which is
  now prettier clean.

Immich certificate switch (#2475)
- The switch trusts one server. Saving a different URL without sending
  it turns it off; saving the same URL keeps it as before. The wiki says
  so.

Moving day (#2476)
- Google Maps and CoMaps are left out only when there is nothing to hand
  over. A day with a single stop opens it as a pin again, on the desktop
  and on the phone, as it did before.
- Desktop: a moving day whose flight or train has no stations shows no
  route tools, since there is nothing left to draw. With stations they
  stay.
- Phone: the drive between two stays on a day without stops or bookings
  shows once at the top instead of again at the bottom.
- The export helpers take the carrier flags as one object instead of two
  booleans in a row.

Import card (#2477)
- The success and error marks use the theme tokens.
- The e2e test names its upload with a made-up booking number.

Tests: COLLECTIONS-SVC-132 and 133, COLLECTIONS-E2E-088,
update_collection_place with a price and no currency, FE-COMP-COLEXTRAS-013
(011 adjusted), IMMICH-TLS-010, IMMICH-103 extended,
FE-PLANNER-DAYPLAN-222 adjusted and 224, FE-MOB-PLTL-051, FE-MOB-PTLM-047,
FE-W4BGT-023 and 024 check the mark colours.
This commit is contained in:
Maurice
2026-09-23 17:39:25 +02:00
parent 0df82185bd
commit a910c2a837
22 changed files with 269 additions and 84 deletions
@@ -290,7 +290,7 @@ describe('BackgroundTasksWidget: the mark on a finished job (#2477)', () => {
const { baseElement } = render(<BackgroundTasksWidget />)
expect(screen.getByText('No reservations could be extracted from the uploaded files.')).toBeInTheDocument()
expect(baseElement.querySelector('svg.lucide-alert-triangle')).not.toBeNull()
expect(baseElement.querySelector('svg.lucide-alert-triangle')).toHaveAttribute('stroke', 'var(--warning)')
expect(baseElement.querySelector('svg.lucide-check-circle2')).toBeNull()
expect(baseElement.querySelector('.animate-spin')).toBeNull()
})
@@ -299,7 +299,8 @@ describe('BackgroundTasksWidget: the mark on a finished job (#2477)', () => {
useBackgroundTasksStore.setState({ tasks: [task({ items: [{ id: 1 }] as never, warnings: ['could not locate "X"'] })] })
const { baseElement } = render(<BackgroundTasksWidget />)
expect(baseElement.querySelector('svg.lucide-check-circle2')).not.toBeNull()
// The theme's own success colour, so it follows light and dark.
expect(baseElement.querySelector('svg.lucide-check-circle2')).toHaveAttribute('stroke', 'var(--success)')
expect(baseElement.querySelector('svg.lucide-alert-triangle')).toBeNull()
})
})
@@ -141,12 +141,12 @@ export default function BackgroundTasksWidget() {
>
<div style={{ flexShrink: 0, marginTop: 1 }}>
{(task.status === 'running' || (task.status === 'done' && task.items === undefined)) && <Loader2 size={16} className="animate-spin" color="var(--accent)" />}
{task.status === 'done' && task.items !== undefined && task.items.length > 0 && <CheckCircle2 size={16} color="#10b981" />}
{task.status === 'done' && task.items !== undefined && task.items.length > 0 && <CheckCircle2 size={16} color="var(--success)" />}
{/* A parse that found nothing is not a success, whatever the job
status says: the same card used to show a green tick next to
"no reservations could be extracted" (#2477). */}
{task.status === 'done' && task.items?.length === 0 && <AlertTriangle size={16} color="var(--warning)" />}
{task.status === 'error' && <AlertCircle size={16} color="#ef4444" />}
{task.status === 'error' && <AlertCircle size={16} color="var(--danger)" />}
</div>
<div style={{ flex: 1, minWidth: 0 }}>
@@ -1,4 +1,4 @@
// FE-COMP-COLEXTRAS-001 to FE-COMP-COLEXTRAS-012: price, website and phone of a saved place (#2471).
// FE-COMP-COLEXTRAS-001 to FE-COMP-COLEXTRAS-013: price, website and phone of a saved place (#2471).
import { act, renderHook } from '@testing-library/react'
import { useSettingsStore } from '../../store/settingsStore'
import { resetAllStores } from '../../../tests/helpers/store'
@@ -8,6 +8,7 @@ import {
extrasPatch,
parsePriceInput,
phoneHref,
priceLabel,
useCollectionPlaceExtras,
type CollectionPlaceExtrasDraft,
} from './useCollectionPlaceExtras'
@@ -87,6 +88,15 @@ describe('phoneHref', () => {
})
})
describe('priceLabel', () => {
it('FE-COMP-COLEXTRAS-013: names the stored currency, and without one shows the bare amount whatever the viewer uses', () => {
expect(priceLabel(2000, 'JPY', 'en-US')).toBe(formatMoney(2000, 'JPY', 'en-US'))
expect(priceLabel(2000, null, 'en-US')).toBe('2,000')
expect(priceLabel(12.5, '', 'de-DE')).toBe('12,5')
expect(priceLabel(12.5, undefined, 'de-DE')).not.toMatch(/€|\$|EUR|USD/)
})
})
describe('useCollectionPlaceExtras', () => {
it('FE-COMP-COLEXTRAS-011: formats the stored price for read mode, only above zero, and guards the website', () => {
withDefaultCurrency('CHF')
@@ -96,9 +106,9 @@ describe('useCollectionPlaceExtras', () => {
expect(paid.result.current.phone).toBe('+41 44')
expect(paid.result.current.phoneHref).toBe('tel:+4144')
// No currency stored: the user's own default names it.
// No currency stored: a bare amount, not the viewer's own default currency.
const bare = renderHook(() => useCollectionPlaceExtras({ price: 8, currency: null }))
expect(bare.result.current.priceLabel).toBe(formatMoney(8, 'CHF', 'en-US'))
expect(bare.result.current.priceLabel).toBe('8')
const free = renderHook(() => useCollectionPlaceExtras({ price: 0, currency: 'EUR', website: 'javascript:alert(1)', phone: ' ' }))
expect(free.result.current.priceLabel).toBeNull()
@@ -69,6 +69,17 @@ export function extrasPatch(initial: CollectionPlaceExtrasDraft, draft: Collecti
return patch
}
/**
* The read-mode price. An older row can hold an amount without a currency (a
* trip place saved before it had one); it shows as a bare number rather than
* in the viewer's own default currency, which would name a different currency
* for every member of a shared list.
*/
export function priceLabel(price: number, currency: string | null | undefined, locale: string): string {
if (currency) return formatMoney(price, currency, locale)
return new Intl.NumberFormat(locale || undefined, { maximumFractionDigits: 2 }).format(price)
}
/** A tel: link keeps the digits and a leading plus, nothing a dialer would choke on. */
export function phoneHref(phone: string): string {
return `tel:${phone.replace(/[^\d+]/g, '')}`
@@ -115,7 +126,7 @@ export function useCollectionPlaceExtras(place: ExtrasSource | null | undefined)
options: currenciesWith(draft.currency).map(c => ({ value: c, label: SYMBOLS[c] ? `${c} ${SYMBOLS[c]}` : c })),
},
// Read mode: a free place shows no price, the same rule the trip inspector follows.
priceLabel: price > 0 ? formatMoney(price, place?.currency || defaultCurrency || 'EUR', locale) : null,
priceLabel: price > 0 ? priceLabel(price, place?.currency, locale) : null,
websiteHref: safeHttpUrl(place?.website),
phone,
phoneHref: phone ? phoneHref(phone) : null,
@@ -3959,9 +3959,12 @@ describe('DayPlanSidebar', () => {
const { unmount } = render(<DayPlanSidebar {...makeDefaultProps({
days: movingDays, accommodations: stays, reservations: [flight(located)], selectedDayId: 11,
})} />)
// The route itself can still be shown; only the hand-offs, which could only
// ever describe a drive from Munich to Hamburg, are gone.
expect(screen.getByRole('button', { name: 'Route' })).toBeInTheDocument()
// With its airports the route to and from them can still be shown; without
// them there is nothing left to draw, so the tools go as a whole instead of
// standing there dead. The hand-offs, which could only ever describe a drive
// from Munich to Hamburg, are gone either way.
if (located) expect(screen.getByRole('button', { name: 'Route' })).toBeInTheDocument()
else expect(screen.queryByRole('button', { name: 'Route' })).not.toBeInTheDocument()
expect(screen.queryByRole('button', { name: 'Open in Google Maps' })).not.toBeInTheDocument()
expect(screen.queryByRole('button', { name: 'Open in CoMaps' })).not.toBeInTheDocument()
unmount()
@@ -3978,6 +3981,27 @@ describe('DayPlanSidebar', () => {
expect(pairs).toContainEqual([{ lat: 53.63, lng: 9.988 }, { lat: 53.551, lng: 9.993 }])
expect(pairs).not.toContainEqual([{ lat: 48.137, lng: 11.575 }, { lat: 53.551, lng: 9.993 }])
})
it('FE-PLANNER-DAYPLAN-224: a day with a single located stop still hands it over as a pin', async () => {
// Two stops, one of them without coordinates, and no stay: the route tools show,
// and the hand-offs open the one stop the way they always did.
const user = userEvent.setup()
const { generateGoogleMapsUrl } = await import('../Map/RouteCalculator')
const openSpy = vi.spyOn(window, 'open').mockReturnValue(null)
const day = buildDay({ id: 10, date: '2026-11-03', title: 'Day 1' })
const assignments = {
'10': [
buildAssignment({ id: 1, day_id: 10, order_index: 0, place: buildPlace({ id: 1, name: 'Elbphilharmonie', lat: 53.541, lng: 9.984 }) }),
buildAssignment({ id: 2, day_id: 10, order_index: 1, place: buildPlace({ id: 2, name: 'Somewhere nice', lat: null, lng: null }) }),
],
}
render(<DayPlanSidebar {...makeDefaultProps({ days: [day], assignments, selectedDayId: 10 })} />)
expect(screen.getByRole('button', { name: 'Open in CoMaps' })).toBeInTheDocument()
await user.click(screen.getByRole('button', { name: 'Open in Google Maps' }))
expect(vi.mocked(generateGoogleMapsUrl)).toHaveBeenCalledWith([{ lat: 53.541, lng: 9.984, name: 'Elbphilharmonie' }])
openSpy.mockRestore()
})
})
it('FE-PLANNER-DAYPLAN-169: picking a whole-day travel mode persists it and redraws the map', async () => {
@@ -1628,15 +1628,19 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
// distinct bookend hotels you actually slept in / sleep in tonight — so the route
// tools appear when you click the day (#1297). A same-hotel rest day or a plain
// arrival/departure day has morning === evening and stays excluded. With a flight
// or train booked on it the map drops that leg and keeps only the drives to and
// from located stations, and the exports below have nothing to hand over (#2476).
// or train booked on it the map drops that leg (#2476): it keeps only the drives
// to and from the booking's located stations, and a booking without any leaves
// nothing to draw, so the tools stay away instead of sitting there dead.
const transferMorning = routeBookends?.morning
const transferEvening = routeBookends?.evening
const dayCarriers = (mergedItemsMap[day.id] || []).filter(i => i.type === 'transport' && isCarrierTransport(i.data))
const dayHasLocatedCarrier = dayCarriers.some(i => hasCarrierEndpointOnDay(i.data, day.id))
const hasHotelTransfer = !!(
routeBookends?.morningIsSleptHere && routeBookends?.eveningIsOvernight &&
transferMorning?.place_lat != null && transferMorning?.place_lng != null &&
transferEvening?.place_lat != null && transferEvening?.place_lng != null &&
(transferMorning.place_lat !== transferEvening.place_lat || transferMorning.place_lng !== transferEvening.place_lng)
(transferMorning.place_lat !== transferEvening.place_lat || transferMorning.place_lng !== transferEvening.place_lng) &&
(dayCarriers.length === 0 || dayHasLocatedCarrier)
)
const routeToolsRoutable = da.length >= 2 || (loc != null && hasRouteBookend) || hasHotelTransfer
/**
@@ -1652,16 +1656,15 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
// A flight, train, ferry or coach on a day without stops is the move itself,
// located or not: the hotels at either end are joined by it, not by a road
// worth handing to a map app (#2476).
if (dayStops.length === 0 && (mergedItemsMap[day.id] || []).some(i => i.type === 'transport' && isCarrierTransport(i.data))) return []
if (dayStops.length === 0 && dayCarriers.length > 0) return []
const stops = dayStops.map(a => ({ lat: a.place!.lat!, lng: a.place!.lng!, name: a.place!.name }))
const first = dayStops[0] ? { isPlace: true, time: dayStops[0].place?.place_time ?? null, lat: dayStops[0].place!.lat!, lng: dayStops[0].place!.lng! } : undefined
const lastAssignment = dayStops[dayStops.length - 1]
const last = lastAssignment ? { isPlace: true, time: lastAssignment.place?.place_time ?? null, lat: lastAssignment.place!.lat!, lng: lastAssignment.place!.lng! } : undefined
// Same carrier gate as the drawn route (#2157): the exported link must not
// start at a hotel you only reach tonight or lead back to one you left.
const dayHasCarrier = (mergedItemsMap[day.id] || []).some(i => i.type === 'transport' && hasCarrierEndpointOnDay(i.data, day.id))
const drawMorning = !!routeBookends && shouldDrawMorningLeg(routeBookends, day, first, dayHasCarrier)
const drawEvening = !!routeBookends && shouldDrawEveningLeg(routeBookends, day, last, dayHasCarrier)
const drawMorning = !!routeBookends && shouldDrawMorningLeg(routeBookends, day, first, dayHasLocatedCarrier)
const drawEvening = !!routeBookends && shouldDrawEveningLeg(routeBookends, day, last, dayHasLocatedCarrier)
const morning = drawMorning && routeBookends?.morning?.place_lat != null && routeBookends?.morning?.place_lng != null
? { lat: routeBookends.morning.place_lat, lng: routeBookends.morning.place_lng, name: routeBookends.morning.place_name } : null
const evening = drawEvening && routeBookends?.evening?.place_lat != null && routeBookends?.evening?.place_lng != null
@@ -1669,8 +1672,9 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
return [...(morning ? [morning] : []), ...stops, ...(evening ? [evening] : [])]
}
const showRouteTools = (isSelected || (showRouteToolsWhenExpanded && isExpanded)) && routeToolsRoutable
// Built once, for the day the tools show on. A route needs two ends: with
// fewer the hand-offs would open a lone pin or nothing at all (#2476).
// Built once, for the day the tools show on. With no stop to hand over the
// hand-offs would open nothing, so they are left out; a single stop still
// opens as a pin (#2476).
const exportStops = showRouteTools ? dayExportStops() : []
// Is this day's inline route currently on? Mobile toggles it per day (its
// own expandedRouteDayIds entry); desktop uses the global Route toggle on
@@ -2875,7 +2879,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
{!narrowPanel && t('dayplan.route')}
</button>
{/* Open the day's stops as a route in Google Maps (planned order). #1255 */}
{exportStops.length >= 2 && <Tooltip label={t('planner.openGoogleMaps')} placement="top">
{exportStops.length > 0 && <Tooltip label={t('planner.openGoogleMaps')} placement="top">
<button type="button"
onClick={() => {
const url = generateGoogleMapsUrl(exportStops)
@@ -2895,7 +2899,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
{/* The same day, handed to CoMaps for offline navigation (#1904). The
day's own travel mode rides along, so the route it builds walks
when the plan walks. */}
{exportStops.length >= 2 && <Tooltip label={t('planner.openCoMaps')} placement="top">
{exportStops.length > 0 && <Tooltip label={t('planner.openCoMaps')} placement="top">
<button type="button"
onClick={() => {
const url = generateCoMapsUrl(exportStops, day.default_transport_mode ?? routeProfile)
@@ -47,18 +47,18 @@ describe('dayExportStops', () => {
it('FE-MOBILE-DAYROUTE-006: with a carrier on the day, the not-yet-reached check-in hotel is not prepended (#2157)', () => {
// Arrival day: you fly in, so the hotel may end the list but must not start it.
const stops = dayExportStops(days[0], days, [homeNear], [hotel], true, true)
const stops = dayExportStops(days[0], days, [homeNear], [hotel], true, { located: true })
expect(stops.map(s => s.name)).toEqual(['Home', 'Hotel Lutetia'])
})
it('FE-MOBILE-DAYROUTE-007: with a carrier on the day, the already-left check-out hotel is not appended (#2157)', () => {
// Check-out day: the hotel starts the day, nothing routes back to it after the flight home.
const stops = dayExportStops(days[2], days, [homeNear], [hotel], true, true)
const stops = dayExportStops(days[2], days, [homeNear], [hotel], true, { located: true })
expect(stops.map(s => s.name)).toEqual(['Hotel Lutetia', 'Home'])
})
it('FE-MOBILE-DAYROUTE-008: without a carrier the no-time loop still closes (#2009 preserved)', () => {
const stops = dayExportStops(days[0], days, [louvre], [hotel], true, false)
const stops = dayExportStops(days[0], days, [louvre], [hotel], true, { located: false })
expect(stops.map(s => s.name)).toEqual(['Hotel Lutetia', 'Louvre', 'Hotel Lutetia'])
})
@@ -77,16 +77,16 @@ describe('dayExportStops', () => {
it('FE-MOBILE-DAYROUTE-010: a flight between the two stays leaves nothing to export (#2476)', () => {
// Located or not, the flight is how the day gets from one hotel to the other; a
// driving route between them is the stretch nobody drives.
expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true, true, true)).toEqual([])
expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true, false, true)).toEqual([])
expect(dayGoogleMapsUrl(days[1], days, [], [hotelA, hotelB], true, false, true)).toBeNull()
expect(dayCoMapsUrl(days[1], days, [], [hotelA, hotelB], true, 'driving', false, true)).toBeNull()
expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true, { located: true, booked: true })).toEqual([])
expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true, { booked: true })).toEqual([])
expect(dayGoogleMapsUrl(days[1], days, [], [hotelA, hotelB], true, { booked: true })).toBeNull()
expect(dayCoMapsUrl(days[1], days, [], [hotelA, hotelB], true, 'driving', { booked: true })).toBeNull()
})
it('FE-MOBILE-DAYROUTE-011: without a booking the move is still the drive from one stay to the next (#1297)', () => {
expect(dayExportStops(days[1], days, [], [hotelA, hotelB], true).map(s => s.name)).toEqual(['Hotel A', 'Hotel B'])
// A stop of its own keeps the day exportable even with the flight booked.
expect(dayExportStops(days[1], days, [louvre], [hotelA, hotelB], true, true, true).map(s => s.name)).toContain('Louvre')
expect(dayExportStops(days[1], days, [louvre], [hotelA, hotelB], true, { located: true, booked: true }).map(s => s.name)).toContain('Louvre')
})
})
+24 -18
View File
@@ -60,16 +60,25 @@ export function optimizeDayOrder(
return { order, usedHotel: Boolean(anchors.start || anchors.end) }
}
/**
* What the day's carrier bookings (flight, train, ferry, coach) tell the
* exports. `located`: one has an endpoint with coordinates on the day, which
* keeps the no-time bookend default from opening a leg out of a hotel only
* reached tonight or back into one already left (#2157), same as the drawn
* route. `booked`: one is booked on the day at all, located or not, which
* leaves a day without stops with nothing to export: the hotels at either end
* are joined by that booking, not by a road (#2476).
*/
export interface DayCarrier {
located?: boolean
booked?: boolean
}
/**
* The day's located stops in planned order, bookended by the morning/evening
* accommodation exactly like the drawn route. Names ride along for the deep
* links that can label a pin with one. `dayHasCarrier` — a carrier transport
* with a located endpoint on the day — keeps the no-time bookend default from
* opening a leg out of a hotel only reached tonight or back into one already
* left (#2157), same as the drawn route. `dayHasCarrierBooking`, any carrier
* booked on the day with or without coordinates, leaves a day without stops
* with nothing to export: the hotels at either end are joined by that booking,
* not by a road (#2476).
* links that can label a pin with one. See DayCarrier for what the day's
* carrier bookings change.
*/
export function dayExportStops(
day: Day,
@@ -77,11 +86,10 @@ export function dayExportStops(
dayAssignments: Assignment[],
accommodations: Accommodation[],
bookendFromAccommodation: boolean,
dayHasCarrier?: boolean,
dayHasCarrierBooking?: boolean,
carrier: DayCarrier = {},
): NamedWaypoint[] {
const located = dayAssignments.filter(a => a.place?.lat != null && a.place?.lng != null)
if (located.length === 0 && dayHasCarrierBooking) return []
if (located.length === 0 && carrier.booked) return []
const stops = located.map(a => ({ lat: a.place!.lat!, lng: a.place!.lng!, name: a.place!.name }))
const bookends = bookendFromAccommodation ? getDayBookendHotels(day, days, accommodations) : null
const firstStop = located[0]
@@ -91,10 +99,10 @@ export function dayExportStops(
const lastStop = last
? { isPlace: true, time: last.place?.place_time ?? null, lat: last.place!.lat!, lng: last.place!.lng! }
: undefined
const morning = bookends && shouldDrawMorningLeg(bookends, day, firstStop, dayHasCarrier)
const morning = bookends && shouldDrawMorningLeg(bookends, day, firstStop, carrier.located)
&& bookends.morning?.place_lat != null && bookends.morning?.place_lng != null
? { lat: bookends.morning.place_lat, lng: bookends.morning.place_lng, name: bookends.morning.place_name } : null
const evening = bookends && shouldDrawEveningLeg(bookends, day, lastStop, dayHasCarrier)
const evening = bookends && shouldDrawEveningLeg(bookends, day, lastStop, carrier.located)
&& bookends.evening?.place_lat != null && bookends.evening?.place_lng != null
? { lat: bookends.evening.place_lat, lng: bookends.evening.place_lng, name: bookends.evening.place_name } : null
return [...(morning ? [morning] : []), ...stops, ...(evening ? [evening] : [])]
@@ -107,11 +115,10 @@ export function dayGoogleMapsUrl(
dayAssignments: Assignment[],
accommodations: Accommodation[],
bookendFromAccommodation: boolean,
dayHasCarrier?: boolean,
dayHasCarrierBooking?: boolean,
carrier: DayCarrier = {},
): string | null {
return generateGoogleMapsUrl(
dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier, dayHasCarrierBooking),
dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, carrier),
) || null
}
@@ -123,11 +130,10 @@ export function dayCoMapsUrl(
accommodations: Accommodation[],
bookendFromAccommodation: boolean,
profile: RouteProfileKey,
dayHasCarrier?: boolean,
dayHasCarrierBooking?: boolean,
carrier: DayCarrier = {},
): string | null {
return generateCoMapsUrl(
dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, dayHasCarrier, dayHasCarrierBooking),
dayExportStops(day, days, dayAssignments, accommodations, bookendFromAccommodation, carrier),
profile,
) || null
}
@@ -208,7 +208,12 @@ export function hotelLegsForDay(
: [...routeSegments].reverse().find(s => sameCoord(s.to, coord))
return seg ? { seg, name: accommodationName(a) } : null
}
return { top: legAt(morning, 'from'), bottom: legAt(evening, 'to') }
const top = legAt(morning, 'from')
const bottom = legAt(evening, 'to')
// A moving day without stops is one drive from one hotel to the next (#1297): it
// both leaves the morning hotel and reaches the evening one, so it shows once, at
// the top, instead of again at the bottom (#2476).
return { top, bottom: top && bottom && bottom.seg === top.seg ? null : bottom }
}
/** The day headline as city pills — a "Tokyo → Kyoto" title becomes two pills with an arrow. */
@@ -5,7 +5,7 @@ import { assignmentsApi, reservationsApi, weatherApi } from '../../../../api/cli
import { usePluginStore } from '../../../../store/pluginStore'
import { getDayBookendHotels } from '../../../../utils/dayOrder'
import { getDisplayTimeForDay, getMergedItems, getTransportForDay, hasCarrierEndpointOnDay, isCarrierTransport } from '../../../../utils/dayMerge'
import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder } from '../lib/dayRoute'
import { dayCoMapsUrl, dayExportStops, dayGoogleMapsUrl, optimizeDayOrder, type DayCarrier } from '../lib/dayRoute'
import { buildTransitLeg, buildTransitNameIndex, type TransitLeg } from '../../../../components/Planner/transitLeg'
import {
buildPlanRows, breaksChronology, findUpNext, hotelChipsForDay, hotelLegsForDay, itemHasTime,
@@ -60,11 +60,11 @@ export function useMPlanTimeline(planner: TripPlanner) {
() => !!day && merged.some(it => it.type === 'transport' && hasCarrierEndpointOnDay(it.data, day.id)),
[day, merged],
)
// Any carrier booked today, located or not. On a day without stops it is the move
// itself, so the exports have no road to hand over (#2476).
const dayHasCarrierBooking = useMemo(
() => merged.some(it => it.type === 'transport' && isCarrierTransport(it.data)),
[merged],
// Plus any carrier booked today, located or not. On a day without stops it is the
// move itself, so the exports have no road to hand over (#2476).
const dayCarrier = useMemo<DayCarrier>(
() => ({ located: dayHasCarrier, booked: merged.some(it => it.type === 'transport' && isCarrierTransport(it.data)) }),
[dayHasCarrier, merged],
)
// Travel-time connectors (walk · distance · drive between consecutive places)
@@ -324,14 +324,14 @@ export function useMPlanTimeline(planner: TripPlanner) {
}
}, [day, dayAssignments, days, tripAccommodations, settings, dayHasCarrier, tripActions, tripId, pushUndo, updateRouteForDay, toast, t])
// A route needs two ends. With fewer the map hand-offs would open a lone pin or
// nothing at all, so the plan does not offer them (#2476).
// With no stop left to hand over, such as a moving day that only holds its flight,
// the map hand-offs would open nothing, so the plan does not offer them. A single
// stop still opens as a pin (#2476).
const canExportRoute = useMemo(
() => !!day && dayExportStops(
day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false,
dayHasCarrier, dayHasCarrierBooking,
).length >= 2,
[day, days, dayAssignments, tripAccommodations, settings, dayHasCarrier, dayHasCarrierBooking],
day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, dayCarrier,
).length > 0,
[day, days, dayAssignments, tripAccommodations, settings, dayCarrier],
)
const exportGoogleMaps = useCallback(() => {
@@ -339,20 +339,19 @@ export function useMPlanTimeline(planner: TripPlanner) {
// Bookend the exported route with the day's accommodation the same way the
// drawn route does — only when the leg is real (#1372, #1465).
const url = dayGoogleMapsUrl(
day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false,
dayHasCarrier, dayHasCarrierBooking,
day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false, dayCarrier,
)
if (url) window.open(url, '_blank', 'noopener,noreferrer')
}, [day, dayAssignments, days, tripAccommodations, settings, dayHasCarrier, dayHasCarrierBooking])
}, [day, dayAssignments, days, tripAccommodations, settings, dayCarrier])
const exportCoMaps = useCallback(() => {
if (!day) return
const url = dayCoMapsUrl(
day, days, dayAssignments, tripAccommodations, settings.optimize_from_accommodation !== false,
day.default_transport_mode ?? routeProfile, dayHasCarrier, dayHasCarrierBooking,
day.default_transport_mode ?? routeProfile, dayCarrier,
)
if (url) window.open(url, '_blank', 'noopener,noreferrer')
}, [day, dayAssignments, days, tripAccommodations, settings, routeProfile, dayHasCarrier, dayHasCarrierBooking])
}, [day, dayAssignments, days, tripAccommodations, settings, routeProfile, dayCarrier])
const renameDay = useCallback((title: string) => {
if (!day) return
@@ -200,7 +200,7 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) {
if (!day) return
const url = dayGoogleMapsUrl(
day, planner.days, dayAssignments, planner.tripAccommodations, optimizeFromAccommodation !== false,
dayHasCarrier,
{ located: dayHasCarrier },
)
if (url) window.open(url, '_blank', 'noopener,noreferrer')
}
@@ -210,7 +210,7 @@ export default function MDaySheet({ planner, shell }: MTripSheetsProps) {
if (!day) return
const url = dayCoMapsUrl(
day, planner.days, dayAssignments, planner.tripAccommodations, optimizeFromAccommodation !== false,
day.default_transport_mode ?? planner.routeProfile, dayHasCarrier,
day.default_transport_mode ?? planner.routeProfile, { located: dayHasCarrier },
)
if (url) window.open(url, '_blank', 'noopener,noreferrer')
}
@@ -12,7 +12,7 @@ import type {
Accommodation, Assignment, Day, DayNote, Reservation, RouteSegment, TranslationFn,
} from '../../../../src/types'
// FE-MOB-PTLM-001 to FE-MOB-PTLM-046
// FE-MOB-PTLM-001 to FE-MOB-PTLM-047
const DAYS = [
{ id: 1, trip_id: 1, day_number: 1, date: '2026-05-01', title: null },
@@ -364,6 +364,14 @@ describe('planTimelineModel — hotel chips and legs', () => {
expect(located.top).toMatchObject({ name: 'Hotel A', seg: { from: [HOTEL_A.lat, HOTEL_A.lng], to: [MUC.lat, MUC.lng] } })
expect(located.bottom).toMatchObject({ name: 'Hotel B', seg: { from: [HAM.lat, HAM.lng], to: [HOTEL_B.lat, HOTEL_B.lng] } })
})
it('FE-MOB-PTLM-047: without a flight the one drive between the two stays shows once, not above and below', () => {
// No stop and no booking: the day is the drive from Munich to Hamburg (#1297).
// That one segment leaves the morning hotel and reaches the evening one.
const legs = legsOf([])
expect(legs.top).toMatchObject({ name: 'Hotel A', seg: { from: [HOTEL_A.lat, HOTEL_A.lng], to: [HOTEL_B.lat, HOTEL_B.lng] } })
expect(legs.bottom).toBeNull()
})
})
})
@@ -12,7 +12,7 @@ import { buildPlanner, buildTripActions } from '../../../helpers/mobileTrip'
import { resetAllStores, seedStore } from '../../../helpers/store'
import { act, renderHook, waitFor } from '../../../helpers/render'
// FE-MOB-PLTL-001 to FE-MOB-PLTL-050
// FE-MOB-PLTL-001 to FE-MOB-PLTL-051
// The connector calculation is its own hook with real OSRM calls — stubbed here so
// the timeline sees exactly the legs a test wants to match against.
@@ -627,6 +627,24 @@ describe('useMPlanTimeline', () => {
)
})
it('FE-MOB-PLTL-051: a day with a single stop still offers the hand-offs, which open that stop as a pin', async () => {
// No stay in TREK: one sight is the whole export. The #2476 gate only drops a day
// with nothing to hand over, not one that opens a pin to navigate to.
const { result } = await renderTimeline(makePlanner({ assignments: { '2': [A_MUSEUM] } }))
expect(result.current.canExportRoute).toBe(true)
act(() => { result.current.exportGoogleMaps() })
expect(window.open).toHaveBeenCalledWith(
'https://www.google.com/maps/search/?api=1&query=48,16.1', '_blank', 'noopener,noreferrer',
)
const vague = buildAssignment({
id: 15, day_id: 2, order_index: 0, place_id: 105,
place: buildPlace({ id: 105, name: 'Idea', lat: null, lng: null }),
})
const nothing = await renderTimeline(makePlanner({ assignments: { '2': [vague] } }))
expect(nothing.result.current.canExportRoute).toBe(false)
})
it('FE-MOB-PLTL-039: renames the day with a trimmed title', async () => {
const planner = makePlanner()
const { result } = await renderTimeline(planner)
@@ -995,6 +995,7 @@ export class CollectionsService {
async updatePlace(userId: number, placeId: number, body: CollectionPlaceUpdateRequest, socketId?: string): Promise<CollectionPlace> {
const currentCollection = this.collectionIdOfPlace(placeId);
this.assertCanEdit(userId, currentCollection);
if (body.price !== undefined || body.currency !== undefined) this.assertPriceHasCurrency(placeId, body);
// Capture the previous thumbnail so a replaced/cleared custom upload (#1136)
// can be reclaimed once nothing references it any more.
@@ -1056,6 +1057,23 @@ export class CollectionsService {
return this.getPlaceById(placeId);
}
/**
* A list has no base currency, so a price that names none would be read in
* each viewer's own default currency and carried into a trip as the trip's
* (#2471). Checked here rather than in the contract so REST and the MCP tool,
* which spreads the contract's shape, refuse it alike. Only the row as it
* would look after this update counts, so a zero price or clearing both
* fields still goes through.
*/
private assertPriceHasCurrency(placeId: number, body: Pick<CollectionPlaceUpdateRequest, 'price' | 'currency'>): void {
const stored = this.db.get<{ price: number | null; currency: string | null }>(
'SELECT price, currency FROM collection_places WHERE id = ?', placeId,
);
const price = body.price === undefined ? stored?.price ?? null : body.price;
const currency = body.currency === undefined ? stored?.currency ?? null : body.currency;
if (price != null && price > 0 && !currency) httpError(400, 'A price needs a currency: send its three-letter code along');
}
setStatus(userId: number, placeId: number, status: CollectionStatus, socketId?: string): CollectionPlace {
const collectionId = this.collectionIdOfPlace(placeId);
this.assertCanEdit(userId, collectionId);
+12 -10
View File
@@ -115,9 +115,10 @@ export class ImmichService {
}
/**
* `allowInsecureTls` left undefined keeps the stored choice, so a client that
* does not know the switch cannot clear it by saving. Disconnecting (no URL)
* always turns it off again.
* `allowInsecureTls` left undefined keeps the stored choice while the URL
* stays the same, so a client that does not know the switch cannot clear it
* by saving. The switch trusts one server, so a new URL without it starts
* off, and disconnecting (no URL) always turns it off again.
*/
async saveImmichSettings(
userId: number,
@@ -134,16 +135,17 @@ export class ImmichService {
if (!ssrf.allowed) {
return { success: false, error: `Invalid Immich URL: ${ssrf.error}` };
}
const url = immichUrl.trim();
const insecure = allowInsecureTls === undefined ? null : Number(allowInsecureTls);
// SET expressions read the row as it was, so `immich_url IS ?` compares the
// stored URL with the new one.
this.db
.prepare(
'UPDATE users SET immich_url = ?, immich_api_key = ?, immich_allow_insecure_tls = COALESCE(?, immich_allow_insecure_tls) WHERE id = ?',
`UPDATE users SET immich_url = ?, immich_api_key = ?,
immich_allow_insecure_tls = CASE WHEN immich_url IS ? THEN COALESCE(?, immich_allow_insecure_tls) ELSE COALESCE(?, 0) END
WHERE id = ?`,
)
.run(
immichUrl.trim(),
maybe_encrypt_api_key(immichApiKey),
allowInsecureTls === undefined ? null : Number(allowInsecureTls),
userId,
);
.run(url, maybe_encrypt_api_key(immichApiKey), url, insecure, insecure, userId);
if (ssrf.isPrivate) {
this.audit.writeAudit({
userId,
+11
View File
@@ -214,6 +214,17 @@ describe('Collections e2e (real auth guard + real service + temp SQLite)', () =>
expect(db.prepare('SELECT price, currency, website FROM collection_places WHERE id = ?').get(placeId)).toEqual({ price: 10, currency: 'EUR', website: null });
});
it('COLLECTIONS-E2E-088: a price without a currency on a place that has none is a 400', async () => {
const col = (await request(server).post('/api/addons/collections').set('Cookie', sessionCookie(ownerId)).send({ name: 'Bare prices' })).body;
const placeId = (await request(server).post('/api/addons/collections/places')
.set('Cookie', sessionCookie(ownerId)).send({ collection_id: col.id, name: 'Kunsthaus' })).body.place.id;
const res = await request(server).patch(`/api/addons/collections/places/${placeId}`).set('Cookie', sessionCookie(ownerId)).send({ price: 14 });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/currency/);
expect(db.prepare('SELECT price, currency FROM collection_places WHERE id = ?').get(placeId)).toEqual({ price: null, currency: null });
});
it('COLLECTIONS-E2E-087: a member who may only read cannot set a price (403)', async () => {
const col = (await request(server).post('/api/addons/collections').set('Cookie', sessionCookie(ownerId)).send({ name: 'Read only prices' })).body;
await request(server).post('/api/addons/collections/invite').set('Cookie', sessionCookie(ownerId)).send({ collection_id: col.id, user_id: otherId, role: 'viewer' });
@@ -1164,7 +1164,7 @@ describe('Immich self-signed certificate switch (#2475)', () => {
expect(get.body.allow_insecure_tls).toBe(true);
});
it('IMMICH-103: a save without the switch keeps it, and disconnecting clears it', async () => {
it('IMMICH-103: a save without the switch keeps it for the same server, a new server or disconnecting clears it', async () => {
const { user } = createUser(testDb);
const put = (body: Record<string, unknown>) =>
request(app).put(`${IMMICH}/settings`).set('Cookie', authCookie(user.id)).send(body);
@@ -1176,6 +1176,12 @@ describe('Immich self-signed certificate switch (#2475)', () => {
await put({ immich_url: 'https://immich.example.com', immich_api_key: 'k' });
expect(allowInsecureTls(user.id)).toBe(1);
// A different server is a different trust decision: without the switch it starts off.
await put({ immich_url: 'https://photos.example.com', immich_api_key: 'k' });
expect(allowInsecureTls(user.id)).toBe(0);
await put({ immich_url: 'https://photos.example.com', immich_api_key: 'k', allow_insecure_tls: true });
expect(allowInsecureTls(user.id)).toBe(1);
await put({ immich_url: '', immich_api_key: 'k', allow_insecure_tls: true });
expect(allowInsecureTls(user.id)).toBe(0);
});
@@ -406,6 +406,25 @@ describe('Tool: update_collection_place / set_collection_place_status / rate_col
});
});
// The contract takes price and currency as separate fields; the service holds
// them together, so an agent cannot leave a bare amount on a place without one.
it('refuses a price without a currency on a place that has none', async () => {
const { user } = createUser(testDb);
const col = seedCollection(user.id, 'P');
const pid = seedPlace(col, user.id, 'Pergamon');
await withHarness(user.id, async (h) => {
const bare = await h.client.callTool({ name: 'update_collection_place', arguments: { placeId: pid, price: 14 } });
expect(bare.isError).toBe(true);
expect(errorText(bare)).toMatch(/currency/);
expect(testDb.prepare('SELECT price, currency FROM collection_places WHERE id = ?').get(pid)).toEqual({ price: null, currency: null });
const paired = parseToolResult(await h.client.callTool({
name: 'update_collection_place', arguments: { placeId: pid, price: 14, currency: 'EUR' },
})) as { place: { price: number; currency: string } };
expect(paired.place).toMatchObject({ price: 14, currency: 'EUR' });
});
});
it('describes the price as a rough cost that needs its currency', async () => {
const { user } = createUser(testDb);
const h = await createMcpHarness({ userId: user.id, withResources: false });
@@ -379,6 +379,31 @@ describe('status + updatePlace move', () => {
expect(renamed).toMatchObject({ name: 'Kunsthaus Zürich', price: 24.5, currency: 'CHF', website: 'https://kunsthaus.example', phone: '+41 44' });
});
// A list has no base currency, so a bare amount would be read in every
// viewer's own default currency. The service refuses it for REST and MCP alike.
it('COLLECTIONS-SVC-132: a price without a currency on a place that has none is a 400 and changes nothing', async () => {
const u = createUser(testDb).user;
const col = svc.createCollection(u.id, { name: 'Zurich' });
const p = svc.savePlace(u.id, { collection_id: col.id, name: 'Kunsthaus' }).place!;
await expect(svc.updatePlace(u.id, p.id, { price: 14 })).rejects.toMatchObject({ status: 400, message: expect.stringMatching(/currency/) });
await expect(svc.updatePlace(u.id, p.id, { price: 14, currency: null })).rejects.toMatchObject({ status: 400 });
expect(testDb.prepare('SELECT price, currency FROM collection_places WHERE id = ?').get(p.id)).toEqual({ price: null, currency: null });
});
it('COLLECTIONS-SVC-133: a price alone keeps the stored currency, a zero price needs none, and a priced place cannot lose its currency', async () => {
const u = createUser(testDb).user;
const col = svc.createCollection(u.id, { name: 'Zurich' });
const priced = svc.savePlace(u.id, { collection_id: col.id, name: 'Kunsthaus', price: 10, currency: 'CHF' }).place!;
const free = svc.savePlace(u.id, { collection_id: col.id, name: 'Lake shore' }).place!;
expect(await svc.updatePlace(u.id, priced.id, { price: 24.5 })).toMatchObject({ price: 24.5, currency: 'CHF' });
expect(await svc.updatePlace(u.id, free.id, { price: 0 })).toMatchObject({ price: 0, currency: null });
await expect(svc.updatePlace(u.id, priced.id, { currency: null })).rejects.toMatchObject({ status: 400 });
expect(await svc.updatePlace(u.id, priced.id, { price: null, currency: null })).toMatchObject({ price: null, currency: null });
});
it('COLLECTIONS-SVC-131: copyToTrip carries the edited price, currency, website and phone into the trip place', async () => {
const u = createUser(testDb).user;
const trip = createTrip(testDb, u.id);
@@ -844,6 +844,23 @@ describe('self-signed certificates', () => {
expect(row).toEqual({ immich_url: null, immich_allow_insecure_tls: 0 });
});
it('IMMICH-TLS-010: the switch trusts one server, so a new URL saved without it starts off', async () => {
seedUser(USER, 'https://nas.local', 'key-1', 0, 1);
// Same server with a trailing slash: still the same connection, the switch stays.
await svc.saveImmichSettings(USER, 'https://nas.local/', 'key-1', null);
expect(svc.getConnectionSettings(USER).allow_insecure_tls).toBe(true);
await svc.saveImmichSettings(USER, 'https://photos.example.com', 'key-2', null);
expect(testDb.prepare('SELECT immich_url, immich_allow_insecure_tls FROM users WHERE id = ?').get(USER)).toEqual({
immich_url: 'https://photos.example.com', immich_allow_insecure_tls: 0,
});
// Sent along with the new URL, it holds for that server.
await svc.saveImmichSettings(USER, 'https://other.example.com', 'key-3', null, true);
expect(svc.getConnectionSettings(USER).allow_insecure_tls).toBe(true);
});
it('IMMICH-TLS-006: a URL the guard refuses leaves the stored switch alone', async () => {
seedUser(USER, 'https://immich.test', 'key-1', 0, 1);
checkSsrf.mockResolvedValue({ allowed: false, error: 'blocked host' });
+3 -2
View File
@@ -61,7 +61,7 @@ const collection: TranslationStrings = {
'collections.coordinates': 'Koordináták',
'collections.price': 'Ár',
'collections.priceHint':
'Hozzávetőleges költség, például belépődíj. A hellyel együtt átkerül, amikor utazásba másolja.',
'Hozzávetőleges költség, például belépődíj. A hellyel együtt átkerül, amikor utazásba másolod.',
'collections.phone': 'Telefon',
'collections.showList': 'Lista megjelenítése',
'collections.expandMap': 'Térkép nagyítása',
@@ -161,7 +161,8 @@ const collection: TranslationStrings = {
'collections.file.targetExisting': 'Hozzáadás listához',
'collections.file.targetExistingHint': 'Egy meglévő listához',
'collections.file.searchLists': 'Listák keresése',
'collections.file.intoHint': 'A listában már meglévő helyek változatlanok maradnak, ahogy a neve és a színe is. A fájl címkéi bekerülnek.',
'collections.file.intoHint':
'A listában már meglévő helyek változatlanok maradnak, ahogy a neve és a színe is. A fájl címkéi bekerülnek.',
'collections.file.confirmInto': 'Hozzáadás a listához',
'collections.file.doneInto': '{count} hely hozzáadva ehhez: {name}',
'collections.file.doneIntoDuplicates': '{count} hozzáadva ehhez: {name}, {duplicates} már benne volt',
+1 -1
View File
@@ -38,7 +38,7 @@ Enter the full URL of your Immich instance and an Immich API key. The API key is
If your Immich server uses a self-signed certificate (for example behind a reverse proxy on your home network), the connection test fails with `fetch failed (self-signed certificate)`. Turn on **Skip SSL certificate verification**, enter the API key again and test or save.
- The setting is per user and only applies to your own Immich connection. Disconnecting Immich turns it off again.
- The setting is per user and only applies to your own Immich connection. Disconnecting Immich, or saving a different server URL without turning it on again, turns it off.
- Only the certificate check is skipped. The connection is still encrypted, but TREK can no longer tell your server from an impostor on the same network, and your API key travels over that connection. Prefer a certificate from a trusted CA where you can.
- Photos you share on a trip are loaded through your connection, so your setting applies when other trip members view them.
- A server on a local or private address still needs internal network access on the TREK server. See [Internal-Network-Access](Internal-Network-Access).