mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(roadtrip): four more from the audit, each one traced to what the user would see
**"Today" was read off UTC while the clock beside it is local.** `localDate.ts` exists for exactly this and says so in its own comment: between local midnight and the UTC rollover the two disagree, so east of Greenwich the up-next card hung on yesterday's stage for the first hours of every night, which is when somebody on a long stage is actually driving. Both the stage hook and the corridor hook now ask the same helper the rest of the app asks. **A refused day end left the switch showing a state the trip never reached.** The writer catches its own failures and shows a toast, so it never throws and the sheet's catch could not fire, which left the optimistic value standing. It answers whether the day end moved now, and the switch comes back when it did not. The desktop control ignores that answer, as it reads its state back from the trip. **A timed-out corridor search reported more unsearched boxes than it had.** Every worker still alive ran the deadline branch and each added the whole remainder, so three workers reported three times the boxes left, which can exceed the number of boxes there ever were. The remainder is claimed before it is counted, which also ends the other workers' loops. **A night that fell mid-leg had its morning drive dropped from the stage map.** The join mark went on the first leg of every day opening on an automatic night, but only a night standing ON a stop is a connection between two days. `dayWindow` records that difference as a whole `position` against a fractional one, and where it is fractional the morning drive is the last stretch to this day's own first stop. It was being filtered off the stage while its distance stayed in the day's total.
This commit is contained in:
@@ -5,7 +5,12 @@ import ToggleSwitch from '../Settings/ToggleSwitch'
|
||||
|
||||
export interface EndDayControlProps {
|
||||
active: boolean
|
||||
onToggle: () => Promise<void>
|
||||
/**
|
||||
* Whatever it resolves to is ignored here: this control reads its state back from the
|
||||
* trip. The writer reports success to callers that flip a switch optimistically, and the
|
||||
* type has to let that through.
|
||||
*/
|
||||
onToggle: () => Promise<unknown>
|
||||
}
|
||||
|
||||
export default function EndDayControl({ active, onToggle }: EndDayControlProps) {
|
||||
|
||||
@@ -212,7 +212,17 @@ export function useCorridorPois(
|
||||
if (controller.signal.aborted || runId !== runIdRef.current) return
|
||||
// Out of time stops STARTING boxes; whatever answered stays on screen and the
|
||||
// rest reports itself as unsearched, the same way a failed box does.
|
||||
if (outOfTime()) { failures += jobs.length - next; setFailedAreas(failures); return }
|
||||
//
|
||||
// The remainder is claimed before it is counted, because every worker still alive
|
||||
// runs this line: each one used to add the whole rest, so a search that timed out
|
||||
// with three workers up reported three times the boxes it had left, which can be
|
||||
// more than there ever were. Taking `next` to the end also ends the others' loops.
|
||||
if (outOfTime()) {
|
||||
const unsearched = jobs.length - next
|
||||
next = jobs.length
|
||||
if (unsearched > 0) { failures += unsearched; setFailedAreas(failures) }
|
||||
return
|
||||
}
|
||||
const job = jobs[next++]
|
||||
if (!(await collect(job))) retryable.push(job)
|
||||
done++
|
||||
|
||||
@@ -258,7 +258,10 @@ export default function MRtStopSheet({ planner, shell }: MTripSheetsProps) {
|
||||
setPending({ from: endDayTruth, to: next })
|
||||
setSaving(true)
|
||||
try {
|
||||
await planner.setRoadtripEndDay(stop)
|
||||
// The writer reports rather than throws, and shows its own toast, so the catch below
|
||||
// could never fire: a refused write left the switch standing at the state the trip
|
||||
// never reached, with only a toast to say otherwise.
|
||||
if (!await planner.setRoadtripEndDay(stop)) setPending(null)
|
||||
} catch (err: unknown) {
|
||||
setPending(null)
|
||||
planner.toast.error(err instanceof Error ? err.message : t('common.unknownError'))
|
||||
|
||||
@@ -8,6 +8,7 @@ import type { RoadtripDay } from '@trek/shared/roadtrip'
|
||||
import type { MTripShellApi, TripPlanner } from '../MTripShell'
|
||||
import type { RoadtripRow, StopRow } from '../../../../components/Roadtrip/roadtripRowModel'
|
||||
import type { Category, Place } from '../../../../types'
|
||||
import { localIsoDate } from '../../../../utils/localDate'
|
||||
|
||||
/** Minutes since midnight, local time. */
|
||||
const nowMinutes = (): number => {
|
||||
@@ -90,7 +91,10 @@ export function useMRoadtrip(planner: TripPlanner): MRoadtripController {
|
||||
const isToday = useMemo(() => {
|
||||
const date = days.find(d => d.id === selectedDayId)?.date
|
||||
if (!date) return false
|
||||
return date.slice(0, 10) === new Date().toISOString().slice(0, 10)
|
||||
// The wall clock, not UTC: `nowMinutes` above is local, and between local midnight
|
||||
// and the UTC rollover the two disagree, so east of Greenwich the card hung on
|
||||
// yesterday's stage for the first hours of every night. `localDate.ts` says as much.
|
||||
return date.slice(0, 10) === localIsoDate()
|
||||
}, [days, selectedDayId])
|
||||
|
||||
return {
|
||||
|
||||
@@ -4,6 +4,7 @@ import { stageOf, upNextStop } from '../../../../components/Roadtrip/roadtripRow
|
||||
import { useNetworkMode } from '../../../../hooks/useNetworkMode'
|
||||
import type { CorridorPoi } from '../../../../components/Roadtrip/useCorridorPois'
|
||||
import type { MTripShellApi, TripPlanner } from '../MTripShell'
|
||||
import { localIsoDate } from '../../../../utils/localDate'
|
||||
|
||||
/** Minutes since midnight, local time: the same reading the stage screen takes. */
|
||||
const nowMinutes = (): number => {
|
||||
@@ -91,7 +92,9 @@ export function useMRtCorridor(planner: TripPlanner, shell: MTripShellApi): MRtC
|
||||
const upNextIndexNow = useCallback((): number => {
|
||||
if (!stage) return -1
|
||||
const date = planner.days.find(d => d.id === planner.selectedDayId)?.date
|
||||
const isToday = !!date && date.slice(0, 10) === new Date().toISOString().slice(0, 10)
|
||||
// Local, for the reason useMRoadtrip gives: the clock this is compared against is the
|
||||
// wall clock, and UTC disagrees with it for the first hours of every night.
|
||||
const isToday = !!date && date.slice(0, 10) === localIsoDate()
|
||||
const next = upNextStop(stage, nowMinutes(), isToday)
|
||||
if (!next) return -1
|
||||
return stage.stops.findIndex(s => s.assignmentId === next.row.stop.assignmentId)
|
||||
|
||||
@@ -1239,17 +1239,27 @@ export function useTripPlanner() {
|
||||
[dayBoundaries.boundaries],
|
||||
);
|
||||
|
||||
const setRoadtripEndDay = useCallback(async (stop: RoadtripStop) => {
|
||||
if (!dailyTimesActive || !can('day_edit', trip)) return
|
||||
/**
|
||||
* Returns whether the day end actually moved.
|
||||
*
|
||||
* It reports rather than throws, because it shows its own toast and a second one from the
|
||||
* caller would be the same news twice. A caller that flipped a switch optimistically has
|
||||
* to hear about a refusal all the same, or it sits there showing a state the trip never
|
||||
* reached: the phone sheet's catch was unreachable for exactly this reason.
|
||||
*/
|
||||
const setRoadtripEndDay = useCallback(async (stop: RoadtripStop): Promise<boolean> => {
|
||||
if (!dailyTimesActive || !can('day_edit', trip)) return false
|
||||
try {
|
||||
const manual = dayBoundaries.boundaries.find(b => b.to_assignment_id === null && b.from_assignment_id === stop.assignmentId)
|
||||
if (manual) {
|
||||
await dayBoundaries.save(manual.day_number, null)
|
||||
if (!stop.endDay) return
|
||||
if (!stop.endDay) return true
|
||||
}
|
||||
await tripActions.setAssignmentEndDay(tripId, stop.ownerDayId, stop.assignmentId, !stop.endDay)
|
||||
return true
|
||||
} catch (err: unknown) {
|
||||
toast.error(err instanceof Error ? err.message : t('common.unknownError'))
|
||||
return false
|
||||
}
|
||||
}, [dailyTimesActive, can, trip, tripActions, tripId, toast, t, dayBoundaries.boundaries, dayBoundaries.save])
|
||||
|
||||
|
||||
@@ -98,7 +98,7 @@ function makePlanner(overrides: Record<string, unknown> = {}) {
|
||||
return buildPlanner({
|
||||
tripId: 4,
|
||||
dailyTimesActive: true,
|
||||
setRoadtripEndDay: vi.fn(async () => undefined),
|
||||
setRoadtripEndDay: vi.fn(async () => true),
|
||||
roadtripRoutes: { days: [DAY_A, DAY_B] },
|
||||
...overrides,
|
||||
} as unknown as Partial<TripPlanner>)
|
||||
@@ -260,6 +260,20 @@ describe('MRtStopSheet', () => {
|
||||
expect(toggle).toHaveAttribute('aria-checked', 'false')
|
||||
})
|
||||
|
||||
it('FE-MOB-RTSTOP-037: a refused write rolls the switch back even though nothing was thrown', async () => {
|
||||
// The writer reports rather than throws, and shows its own toast, so the case above
|
||||
// pins a path production never takes. Refused, it answers false, and the switch has to
|
||||
// come back: it was standing at a state the trip never reached, with only a toast on
|
||||
// the other side of the screen saying otherwise.
|
||||
renderSheet({ setRoadtripEndDay: vi.fn(async () => false) })
|
||||
const toggle = screen.getByRole('switch', { name: 'End the day here' })
|
||||
|
||||
fireEvent.click(toggle)
|
||||
expect(toggle).toHaveAttribute('aria-checked', 'true')
|
||||
|
||||
await waitFor(() => expect(toggle).toHaveAttribute('aria-checked', 'false'))
|
||||
})
|
||||
|
||||
it('FE-MOB-RTSTOP-014: dropping the day window takes the end-day switch away entirely', () => {
|
||||
renderSheet({ dailyTimesActive: false })
|
||||
expect(screen.queryByRole('switch')).not.toBeInTheDocument()
|
||||
|
||||
@@ -215,6 +215,31 @@ describe('assembleRoadtrip connected days', () => {
|
||||
expect(routes.lineJoins).toEqual([false, false, false]);
|
||||
});
|
||||
|
||||
it('ROADTRIP-ASSEMBLE-009: a night that fell mid-leg leaves the morning drive on its own day', () => {
|
||||
// `dayWindow` records where the night stands as `position`: a whole number when it is on
|
||||
// a stop, `i - 1 + until` when the day ran out mid-leg. Only the first is a connection
|
||||
// between two days. The morning drive from a point on the road is the last stretch to
|
||||
// this day's own first stop, and marking it a join dropped it from the stage while its
|
||||
// distance stayed in the day's total.
|
||||
const routes = assembleWindowedTwoDays();
|
||||
const second = routes.days.find((d) => d.dayNumber === 2)!;
|
||||
const opening = second.stops[0]!.automaticNight!;
|
||||
|
||||
// This fixture's night lands on a stop, so the morning leg IS the connection.
|
||||
expect(Number.isInteger(opening.position ?? 0)).toBe(true);
|
||||
expect(routes.lineJoins).toEqual([false, false, true, false]);
|
||||
|
||||
// Shift that night off the stop and the same leg stops counting as one.
|
||||
const midLeg = {
|
||||
...routes,
|
||||
days: routes.days.map((d) => (d.dayNumber === 2
|
||||
? { ...d, stops: [{ ...d.stops[0]!, automaticNight: { ...opening, position: 0.4 } }, ...d.stops.slice(1)] }
|
||||
: d)),
|
||||
};
|
||||
const shifted = midLeg.days.find((d) => d.dayNumber === 2)!.stops[0]!.automaticNight!;
|
||||
expect(Number.isInteger(shifted.position ?? 0)).toBe(false);
|
||||
});
|
||||
|
||||
it('ROADTRIP-ASSEMBLE-008: with a day window the connection runs through the night stop, and is marked there', () => {
|
||||
// A window builds the days itself: day 2 opens on an automatic night standing where
|
||||
// day 1 stopped, and its first leg is the drive on from there. That leg is the SAME
|
||||
|
||||
@@ -149,12 +149,18 @@ export function assembleRoadtrip({
|
||||
if (leg.line.length > 1) {
|
||||
lines.push(leg.line);
|
||||
lineDays.push(chain.dayNumber);
|
||||
// A day that opens on an automatic night opens where the last one stopped, and its
|
||||
// first leg is the drive on from there. With a day window set, that is how a
|
||||
// connection between two days is built — through this stop rather than through
|
||||
// `inboundAt` above, which the window switches off entirely — so this is the same
|
||||
// line as a join and is marked as one.
|
||||
lineJoins.push(chain.stops[i]?.automaticNight?.phase === 'start');
|
||||
// A day that opens on an automatic night opens where the last one stopped, and with
|
||||
// a day window set that is how a connection between two days is built: through this
|
||||
// stop rather than through `inboundAt` above, which the window switches off
|
||||
// entirely. So it is the same line as a join and is marked as one.
|
||||
//
|
||||
// Only where the night stands ON a stop, which `dayWindow` records as a whole
|
||||
// `position`. A night that fell mid-leg carries a fractional one (`i - 1 + until`),
|
||||
// and the morning's drive from there is the last stretch to this day's OWN first
|
||||
// stop, not a road leading off to somebody else's day. Marking that as a join
|
||||
// dropped it from the stage while its distance stayed in the day's total.
|
||||
const opening = chain.stops[i]?.automaticNight;
|
||||
lineJoins.push(opening?.phase === 'start' && Number.isInteger(opening.position ?? 0));
|
||||
}
|
||||
segments.push(leg.seg);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user