From 7f14dde3622fc73c86d4efb25250a8055d2721bf Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 20:51:09 +0200 Subject: [PATCH] fix(tours): tidy the day times of a tour after review The get_tour_plan description named the estimate of a tour "such a field" right after the values of a recording, so it could be read as one of them. It now follows the sentence on derived fields and says outright that it is one of them, worked out from distance and heights or the pace of a run. The arrow between a tour's start and end points from the start to the end in a right-to-left language as well. A tour just added to a day, which carries no tour_place_id until the next refresh, now says its overrun like the row above it. A phone row of an import with a total the travellers entered shows that one time, the estimate joining its words as on the desktop. The German overrun line reads naturally without an item name. --- .../Planner/PlaceFormModal.test.tsx | 34 ++++++++++++++++- .../components/Tours/TourDayRowFacts.test.tsx | 9 +++++ .../src/components/Tours/TourDayRowFacts.tsx | 17 ++++----- .../Tours/TourDayTimeParts.test.tsx | 37 ++++++++++++++++++- .../src/components/Tours/TourDayTimeParts.tsx | 10 +++-- .../src/components/Tours/tourDayTime.test.ts | 7 ++++ client/src/components/Tours/tourDayTime.ts | 3 +- server/src/nest/tours/tours.mcp.ts | 2 +- server/tests/unit/nest/tours.mcp.test.ts | 5 +++ shared/src/i18n/de/tours.ts | 4 +- 10 files changed, 109 insertions(+), 19 deletions(-) diff --git a/client/src/components/Planner/PlaceFormModal.test.tsx b/client/src/components/Planner/PlaceFormModal.test.tsx index 8ed801975..3a26ba0e5 100644 --- a/client/src/components/Planner/PlaceFormModal.test.tsx +++ b/client/src/components/Planner/PlaceFormModal.test.tsx @@ -1,5 +1,6 @@ -// FE-COMP-PLACEFORM-001 to FE-COMP-PLACEFORM-036, FE-PLANNER-PLACEFORM-016 to FE-PLANNER-PLACEFORM-067, plus FE-PLANNER-PLACEFORM-068 to -124 +// FE-COMP-PLACEFORM-001 to FE-COMP-PLACEFORM-036, FE-PLANNER-PLACEFORM-016 to FE-PLANNER-PLACEFORM-067, plus FE-PLANNER-PLACEFORM-068 to -125 import { render, screen, waitFor, fireEvent, within, act } from '../../../tests/helpers/render'; +import type { TourListItem } from '@trek/shared'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; @@ -878,6 +879,37 @@ describe('PlaceFormModal', () => { expect(screen.getByDisplayValue('20:34')).toBeInTheDocument(); }); + it('FE-PLANNER-PLACEFORM-125: a tour with a start and no end says below its times the end it works out to (TIME-02)', () => { + const place = buildPlace({ id: 7, name: 'Ridge', place_time: '09:00', end_time: null }); + const assignment = buildAssignment({ id: 42, day_id: 3, place }); + const tour: TourListItem = { + place_id: 7, + name: 'Ridge', + tour_type: 'hike', + distance: 8, + elevation_gain: 300, + elevation_loss: 300, + duration: 150, + planned_duration_minutes: null, + break_additional_minutes: 30, + difficulty: null, + wanderer_ref: null, + match_confidence: 1, + max_hiking_difficulty: 2, + planned: true, + caution: false, + }; + const { unmount } = render( + + ); + expect(screen.getByTestId('tour-day-end-hint')).toHaveTextContent( + 'Left empty, the end is worked out: 12:00, the start plus the tour time of 3 h.' + ); + unmount(); + render(); + expect(screen.queryByTestId('tour-day-end-hint')).not.toBeInTheDocument(); + }); + it('FE-PLANNER-PLACEFORM-086: on a road trip the End says the drive leaves at it', () => { const place = buildPlace({ name: 'Test', place_time: '09:00', end_time: '14:00' }); const assignment = buildAssignment({ id: 10, day_id: 5, place }); diff --git a/client/src/components/Tours/TourDayRowFacts.test.tsx b/client/src/components/Tours/TourDayRowFacts.test.tsx index 72e5bc4a3..0db9ba770 100644 --- a/client/src/components/Tours/TourDayRowFacts.test.tsx +++ b/client/src/components/Tours/TourDayRowFacts.test.tsx @@ -265,6 +265,15 @@ describe('TIME-01: the estimated time of an import on a row', () => { expect(screen.queryByTestId('trip-plan-tour-estimate')).not.toBeInTheDocument(); }); + it('TIME-01-FE-006: with a total the travellers entered a phone row shows that one time, the estimate in its words', () => { + render(); + expect(screen.queryByTestId('trip-plan-tour-estimate')).not.toBeInTheDocument(); + const total = screen.getByTestId('trip-plan-tour-planned-duration'); + expect(total).toHaveTextContent('5 h'); + expect(total).not.toHaveTextContent('≈'); + expect(total.getAttribute('aria-label')).toMatch(/: 5 h\. Estimated time: 4 h\. Worked out/); + }); + it('TIME-01-FE-004: a tour the planner timed shows its own time and no estimate', () => { render(); expect(screen.queryByTestId('trip-plan-tour-estimate')).not.toBeInTheDocument(); diff --git a/client/src/components/Tours/TourDayRowFacts.tsx b/client/src/components/Tours/TourDayRowFacts.tsx index 9e6610eb0..e2cc6fd41 100644 --- a/client/src/components/Tours/TourDayRowFacts.tsx +++ b/client/src/components/Tours/TourDayRowFacts.tsx @@ -298,9 +298,10 @@ export default function TourDayRowFacts({ movingTimeLabel(tour, t('tours.planner.inspector.duration'), t), 'trip-plan-tour-walking-time' ); - // TIME-01: an import's estimate, said in full where it is opened. + // TIME-01: an import's estimate, said in full where it is opened. A total + // the travellers entered is the time shown; the estimate then only joins its words. const estimate = estimateFact(times, t); - if (estimate.value && estimate.label) + if (estimate.value && estimate.label && times.plannedTotalMinutes == null) add( tourDayRowIcon(movingIconType(tour.tour_type)), estimate.value, @@ -318,13 +319,11 @@ export default function TourDayRowFacts({ 'trip-plan-tour-breaks' ); const total = plannedTotalText(times, t, true); - if (total) - add( - Clock, - total, - `${t('tours.planner.plannedTotalDuration')}${times.manuallyOverridden ? `, ${t('tours.planner.plannedTotalManual')}` : ''}`, - 'trip-plan-tour-planned-duration' - ); + if (total) { + const label = `${t('tours.planner.plannedTotalDuration')}${times.manuallyOverridden ? `, ${t('tours.planner.plannedTotalManual')}` : ''}`; + const said = estimate.label ? `${label}: ${total}. ${estimate.label}` : undefined; + add(Clock, total, label, 'trip-plan-tour-planned-duration', said); + } const check = paddleCheckOf(tour, t); if (check) add(check.icon, check.value, t('tours.paddle.check.title'), 'plan-tour-paddle-check'); return ( diff --git a/client/src/components/Tours/TourDayTimeParts.test.tsx b/client/src/components/Tours/TourDayTimeParts.test.tsx index a1c385c06..c21dac725 100644 --- a/client/src/components/Tours/TourDayTimeParts.test.tsx +++ b/client/src/components/Tours/TourDayTimeParts.test.tsx @@ -1,4 +1,4 @@ -// TIME-02-011 to TIME-02-016: how the day plan says a tour's time, its overrun and the hint below its time fields. +// TIME-02-011 to TIME-02-016, TIME-02-021 and TIME-02-022: how the day plan says a tour's time, its overrun and the hint below its time fields. import type { TourListItem } from '@trek/shared'; import { beforeEach, describe, expect, it } from 'vitest'; import { render, screen } from '../../../tests/helpers/render'; @@ -53,6 +53,9 @@ describe('TourDayTimeText', () => { unmount(); render(); expect(screen.getByTestId('tour-day-end-worked-out')).toHaveTextContent('01:30 +1'); + expect(screen.getByTestId('tour-day-end-worked-out')).toHaveAccessibleName( + 'Ends at 01:30 on the next day, worked out from the start and the tour time of 3 h' + ); }); it('TIME-02-013: an end typed by hand shows as typed, and without an end only the start shows', () => { @@ -64,6 +67,33 @@ describe('TourDayTimeText', () => { const plain = render(); expect(plain.container.textContent).toBe('09:00'); }); + + it('TIME-02-021: a city walk with a stay nobody entered ends then at the earliest, marked with ≥', () => { + // 2 h 15 min on foot and 1 h of stays entered, one station of four without one. + const walk: TourListItem = { + ...tour, + tour_type: 'walk', + duration: 135, + max_hiking_difficulty: null, + station_count: 4, + station_stay_minutes: 60, + station_stays_unknown: 1, + }; + render(); + const end = screen.getByTestId('tour-day-end-worked-out'); + expect(end).toHaveTextContent('≥ 12:15'); + expect(end).toHaveAccessibleName('Ends at 12:15, worked out from the start and the tour time of ≥ 3 h 15 min'); + }); + + it('TIME-02-022: in a right-to-left language the arrow points from the start on the right to the end', () => { + seedStore(useSettingsStore, { settings: { time_format: '24h', language: 'ar' } }); + const { container, unmount } = render(); + expect(container.textContent).toBe('09:00 ← 15:00'); + unmount(); + const worked = render(); + expect(worked.container).toHaveTextContent('09:00 ← 12:00'); + expect(worked.container.textContent).not.toContain('→'); + }); }); describe('TourOverrunLine', () => { @@ -74,6 +104,11 @@ describe('TourOverrunLine', () => { render(); expect(screen.getByTestId('tour-day-overrun')).toHaveTextContent('Ends at 13:00, after Lunch starts at 12:00'); }); + + it('TIME-02-014b: an item without a name is called the next item', () => { + render(); + expect(screen.getByTestId('tour-day-overrun')).toHaveTextContent('Ends at 13:00, after the next item starts at 12:00'); + }); }); describe('workedOutEndHint', () => { diff --git a/client/src/components/Tours/TourDayTimeParts.tsx b/client/src/components/Tours/TourDayTimeParts.tsx index aefd55e3b..f9829eda5 100644 --- a/client/src/components/Tours/TourDayTimeParts.tsx +++ b/client/src/components/Tours/TourDayTimeParts.tsx @@ -1,6 +1,6 @@ import type { TourListItem } from '@trek/shared'; import { Clock } from 'lucide-react'; -import { useTranslation } from '../../i18n'; +import { isRtlLanguage, useTranslation } from '../../i18n'; import { useSettingsStore } from '../../store/settingsStore'; import type { Assignment } from '../../types'; import { formatTime } from '../../utils/formatters'; @@ -46,18 +46,20 @@ export function TourDayTimeText({ tour: TourListItem; mobile?: boolean; }) { - const { t, locale } = useTranslation(); + const { t, locale, language } = useTranslation(); const timeFormat = useSettingsStore((state) => state.settings.time_format) ?? '24h'; const time = tourDayTimeOf(entry, tour); if (!time) return null; const clock = (value: string) => formatTime(value, locale, timeFormat); + // The arrow is not mirrored by the bidi rules: in a right-to-left row the start sits on the right. + const arrow = isRtlLanguage(language) ? '←' : '→'; if (!time.end) return <>{clock(time.start)}; - if (!time.workedOut) return <>{`${clock(time.start)} → ${clock(time.end)}`}; + if (!time.workedOut) return <>{`${clock(time.start)} ${arrow} ${clock(time.end)}`}; const label = workedOutLabel(time, t, clock); const mark = time.estimated ? '≈ ' : time.atLeast ? '≥ ' : ''; return ( <> - {`${clock(time.start)} → `} + {`${clock(time.start)} ${arrow} `} { expect(dayItemOverrun(items, 1, 3, tourOf)).toBeNull(); expect(dayItemOverrun(items, 0, 3, () => undefined)).toBeNull(); expect(dayItemOverrun([note('09:00', 'Wake'), ...items], 0, 3, tourOf)).toBeNull(); + // A tour just added to the day, online or offline, carries no tour_place_id yet and still runs over. + const fresh = placeItem(7, 'Ridge', '10:00', { assignment_time: '10:00' }); + expect(dayItemOverrun([fresh, placeItem(2, 'Lunch', '12:00')], 0, 3, tourOf)).toEqual({ + name: 'Lunch', + time: '12:00', + end: '13:00', + }); }); }); diff --git a/client/src/components/Tours/tourDayTime.ts b/client/src/components/Tours/tourDayTime.ts index 7506fc1e5..a46bbec70 100644 --- a/client/src/components/Tours/tourDayTime.ts +++ b/client/src/components/Tours/tourDayTime.ts @@ -135,7 +135,8 @@ export function dayItemOverrun( const item = items[index]; if (item?.type !== 'place') return null; const assignment: Assignment = item.data; - const tour = assignment.tour_place_id === assignment.place_id ? tourOf(assignment.place_id) : undefined; + // By the trip's tours alone, as the rows are: an entry added a moment ago carries no tour_place_id yet. + const tour = tourOf(assignment.place_id); if (!tour) return null; return tourOverrunOf(items.slice(index + 1), dayId, tourDayTimeOf(tourDayEntryOf(assignment), tour)); } diff --git a/server/src/nest/tours/tours.mcp.ts b/server/src/nest/tours/tours.mcp.ts index e6a914171..4d64f583c 100644 --- a/server/src/nest/tours/tours.mcp.ts +++ b/server/src/nest/tours/tours.mcp.ts @@ -117,7 +117,7 @@ export class ToursMcp { @Tool({ name: 'get_tour_plan', description: - 'Read one saved tour of a trip as the travellers saved it: its facts (tour: place_id, name, tour_type, distance, heights and walking time as stored, null when not known; the routing options as list_tours describes them; revision, raised by every saved change; the days it is on), its ordered control points (start, via and end, each with its sequence and its key, the same across saves; an import without its own shows the two ends of its track), where it came from (provenance) and its legs. Drafts in a browser are never visible: only saved tours exist here, and reading changes nothing, not the tour, its revision nor any visit. A control point with linked_place_id stands for that place of the trip (linked_place_name names it); it keeps its own coordinates. A control point with stage_end true ends a stage of a multi-day tour, and the next stage begins there; the stages run in the order of the route, the first from the start, the last to the end, and a tour without such a point is a tour of one day. legs lists every step from one control point to the next (from_sequence, to_sequence, from_key, to_key) with its kind: routed (planned by the router), track (part of a recorded line, an import without control points of its own, which no router planned), ferrata (follows a via ferrata, name gives it), free (drawn by hand and checked by nothing; every leg of a planned ski tour is free, a straight line between its points), portage (a paddle tour carried over land) or horse_route (a ride only: a section of a signposted horse route from OpenStreetMap in the direction the rider chose; the fixed_leg of its control point says signposted, with or against the direction the route is signposted in, null where it is not signposted one way, and access, the metres of each horse=* value its ways carried when it was taken, unknown where untagged; being on a horse route is no permission to ride it); length_km is the length of the line a fixed leg keeps (for the leg of a ski tour the straight line) and null for a routed or track leg, whose length is not stored apart. line gives the saved line in brief: point_count, has_heights and a preview thinned to 200 points; null while the tour waits for its way to be planned. A paddle tour also carries its obstacle checklist (obstacle_check): the obstacles of the OpenStreetMap data along its line, the decisions the travellers made about them and their own portages and locks; a planning aid, never a safety or legal clearance. A city walk (tour_type walk) has stations for control points, shaping points and the way back of a loop aside, each with every field spelled out: key (the visits of a day are kept by it), name, stay_minutes (the stay in minutes; 0 for a point only passed through, null when not known), importance (optional or must; null for a normal station), protected (true for a station any change you propose has to keep where it is: neither removed, replaced nor moved; the travellers can still edit it themselves in the app), opening_hours (as OpenStreetMap writes them; a hint to check, never a promise; null when none is known) and visit_status. Visits belong to one day of the walk: name that day with dayId (one of its days) to read the status of each station that day (planned, visited or skipped) and run (the day, its assignment and what is still open); without dayId visit_status and run are null. A station visited or skipped that day is done: do not ask about it again or move it to another day. dayId is for a city walk only. The recorded tracks assigned to the tour (activities, ACT-01) each name their execution (assignment_id and day_id, null for none), the recording (track: its tour in the trip, origin, file name, when it came in) and what the recording measures (metrics: distance_km, elevation_gain_m and elevation_loss_m, null without heights; started_at, ended_at and elapsed_minutes from the first to the last time on its points, pauses included, null when the file carried no times, never estimated); these are the values of the recording, the planned values of the tour stay apart and unchanged. tour.time_estimate, where there is one, is such a field: an estimate, never a time the travellers planned or recorded. derived names the fields worked out on reading rather than stored with the tour: never present them as saved facts.', + 'Read one saved tour of a trip as the travellers saved it: its facts (tour: place_id, name, tour_type, distance, heights and walking time as stored, null when not known; the routing options as list_tours describes them; revision, raised by every saved change; the days it is on), its ordered control points (start, via and end, each with its sequence and its key, the same across saves; an import without its own shows the two ends of its track), where it came from (provenance) and its legs. Drafts in a browser are never visible: only saved tours exist here, and reading changes nothing, not the tour, its revision nor any visit. A control point with linked_place_id stands for that place of the trip (linked_place_name names it); it keeps its own coordinates. A control point with stage_end true ends a stage of a multi-day tour, and the next stage begins there; the stages run in the order of the route, the first from the start, the last to the end, and a tour without such a point is a tour of one day. legs lists every step from one control point to the next (from_sequence, to_sequence, from_key, to_key) with its kind: routed (planned by the router), track (part of a recorded line, an import without control points of its own, which no router planned), ferrata (follows a via ferrata, name gives it), free (drawn by hand and checked by nothing; every leg of a planned ski tour is free, a straight line between its points), portage (a paddle tour carried over land) or horse_route (a ride only: a section of a signposted horse route from OpenStreetMap in the direction the rider chose; the fixed_leg of its control point says signposted, with or against the direction the route is signposted in, null where it is not signposted one way, and access, the metres of each horse=* value its ways carried when it was taken, unknown where untagged; being on a horse route is no permission to ride it); length_km is the length of the line a fixed leg keeps (for the leg of a ski tour the straight line) and null for a routed or track leg, whose length is not stored apart. line gives the saved line in brief: point_count, has_heights and a preview thinned to 200 points; null while the tour waits for its way to be planned. A paddle tour also carries its obstacle checklist (obstacle_check): the obstacles of the OpenStreetMap data along its line, the decisions the travellers made about them and their own portages and locks; a planning aid, never a safety or legal clearance. A city walk (tour_type walk) has stations for control points, shaping points and the way back of a loop aside, each with every field spelled out: key (the visits of a day are kept by it), name, stay_minutes (the stay in minutes; 0 for a point only passed through, null when not known), importance (optional or must; null for a normal station), protected (true for a station any change you propose has to keep where it is: neither removed, replaced nor moved; the travellers can still edit it themselves in the app), opening_hours (as OpenStreetMap writes them; a hint to check, never a promise; null when none is known) and visit_status. Visits belong to one day of the walk: name that day with dayId (one of its days) to read the status of each station that day (planned, visited or skipped) and run (the day, its assignment and what is still open); without dayId visit_status and run are null. A station visited or skipped that day is done: do not ask about it again or move it to another day. dayId is for a city walk only. The recorded tracks assigned to the tour (activities, ACT-01) each name their execution (assignment_id and day_id, null for none), the recording (track: its tour in the trip, origin, file name, when it came in) and what the recording measures (metrics: distance_km, elevation_gain_m and elevation_loss_m, null without heights; started_at, ended_at and elapsed_minutes from the first to the last time on its points, pauses included, null when the file carried no times, never estimated); these are the values of the recording, the planned values of the tour stay apart and unchanged. derived names the fields worked out on reading rather than stored with the tour: never present them as saved facts. tour.time_estimate is one of them where present: an estimate from distance and heights (or the pace of a run), never a time the travellers planned or recorded.', inputSchema: { tripId: idSchema, placeId: idSchema.describe('The place id of the tour, as list_tours gives it'), diff --git a/server/tests/unit/nest/tours.mcp.test.ts b/server/tests/unit/nest/tours.mcp.test.ts index 5bee1e266..684f5f197 100644 --- a/server/tests/unit/nest/tours.mcp.test.ts +++ b/server/tests/unit/nest/tours.mcp.test.ts @@ -696,5 +696,10 @@ describe('ToursMcp', () => { const description = entry?.kind === 'tool' ? (entry.options.description ?? '') : ''; expect(description).toContain('time_estimate'); expect(description).toContain('never a recorded or planned time'); + const planEntry = getEntry(ToursMcp as unknown as ClassRef, 'getTourPlan'); + const planDescription = planEntry?.kind === 'tool' ? (planEntry.options.description ?? '') : ''; + expect(planDescription).toContain( + 'never present them as saved facts. tour.time_estimate is one of them where present' + ); }); }); diff --git a/shared/src/i18n/de/tours.ts b/shared/src/i18n/de/tours.ts index 8f275aa96..400fa6b85 100644 --- a/shared/src/i18n/de/tours.ts +++ b/shared/src/i18n/de/tours.ts @@ -1345,8 +1345,8 @@ const tours: TranslationStrings = { 'tours.dayTime.workedOut': 'Endet um {time}, berechnet aus dem Start und der Tourzeit von {total}', 'tours.dayTime.workedOutNextDay': 'Endet am nächsten Tag um {time}, berechnet aus dem Start und der Tourzeit von {total}', - 'tours.dayTime.overrun': 'Endet um {end}, nach dem Start von {name} um {time}', - 'tours.dayTime.nextItem': 'dem nächsten Eintrag', + 'tours.dayTime.overrun': 'Endet um {end}, nachdem {name} um {time} beginnt', + 'tours.dayTime.nextItem': 'der nächste Eintrag', 'tours.dayTime.formHint': 'Leer gelassen, wird das Ende berechnet: {time}, der Start plus die Tourzeit von {total}.', 'tours.dayTime.formNoStart': 'Mit einer Startzeit wird das Ende aus der Tourzeit von {total} berechnet.', };