fix(assistant): taking a proposal needs place rights, and a card's choices wait while a change runs

Plan this checks place_edit like the tour routes, the include choices of a
run or group stay disabled while the chat applies a change, and a follow-up
can say when it applies, so Confirm the pending bookings shows only when the
card holds a pending booking of this trip.
This commit is contained in:
Maurice
2026-10-22 22:46:35 +00:00
parent cf8cf9058d
commit 3eded5f5be
13 changed files with 195 additions and 39 deletions
@@ -1,9 +1,10 @@
/**
* FE-AI-FOLLOW-001 to FE-AI-FOLLOW-002
* FE-AI-FOLLOW-001 to FE-AI-FOLLOW-003
*
* The follow-ups a card offers under an answer that ends on it: the ones that
* ask for a change only for someone who may make it, and the answer's own
* when none is left for a reader.
* ask for a change only for someone who may make it, the answer's own when
* none is left for a reader, and the ones with a condition only when a card of
* the answer meets it.
*/
import type { AssistantMessage, TrekCard } from '@trek/shared';
import { afterEach, describe, expect, it } from 'vitest';
@@ -41,4 +42,21 @@ describe('card follow-ups', () => {
CARD_LOOKS.vacay.followUps = [{ id: 'planDays', labelKey: 'assistant.followUp.next', edits: true }];
expect(followUpsFor(answer, false).map((f) => f.id)).toEqual(['more', 'next']);
});
it('FE-AI-FOLLOW-003: a follow-up with a condition is offered only when a card of the answer meets it', () => {
const asked: TrekCard[] = [];
CARD_LOOKS.vacay.followUps = [
{ id: 'planDays', labelKey: 'assistant.followUp.next', when: () => false },
{
id: 'holidays',
labelKey: 'assistant.followUp.more',
when: (card) => {
asked.push(card);
return true;
},
},
];
expect(followUpsFor(answer, true).map((f) => f.id)).toEqual(['holidays']);
expect(asked).toEqual([{ kind: 'vacay' }]);
});
});
@@ -1,7 +1,8 @@
import type { AssistantMessage } from '@trek/shared';
import type { AssistantMessage, TrekCardKind } from '@trek/shared';
import { messageBlocks } from './assistantParts';
import { CARD_LOOKS } from './cards/cardLooks';
import type { CardEntry, CardFollowUp, CardLook } from './cards/cardTypes';
/**
* The next questions an answer suggests, as one tap each: read from what the
@@ -40,17 +41,27 @@ const GENERAL: readonly FollowUp[] = [
{ id: 'next', labelKey: 'assistant.followUp.next' },
];
/** What a kind offers to ask next, typed for any kind. */
function offeredBy(kind: TrekCardKind): readonly CardFollowUp<TrekCardKind>[] {
return (CARD_LOOKS[kind] as CardLook<TrekCardKind>).followUps ?? [];
}
/**
* The follow-ups of the cards an answer ends on (past its closing words): what
* the first card's kind offers, laid out as the chat lays it out, without the
* ones that ask for a change when the user may not make one.
* ones that ask for a change when the user may not make one, and without the
* ones none of the answer's cards of that kind calls for.
*/
function cardFollowUps(message: AssistantMessage, canEdit: boolean): readonly FollowUp[] {
const blocks = messageBlocks(message, false).filter((block) => block.kind !== 'text');
const last = blocks[blocks.length - 1];
const kind = last?.kind === 'cards' ? last.entries[0]?.card.kind : undefined;
const offered = kind ? (CARD_LOOKS[kind].followUps ?? []) : [];
return canEdit ? offered : offered.filter((followUp) => !followUp.edits);
const entries: readonly CardEntry[] = last?.kind === 'cards' ? last.entries : [];
const kind = entries[0]?.card.kind;
if (!kind) return [];
const cards = entries.filter((entry) => entry.card.kind === kind).map((entry) => entry.card);
return offeredBy(kind).filter(
(followUp) => (canEdit || !followUp.edits) && (!followUp.when || cards.some((card) => followUp.when?.(card)))
);
}
/** What an answer offers to ask next; changes are offered only to someone who may make them. */
@@ -1,11 +1,12 @@
/**
* FE-AI-SLOTS-001 to FE-AI-SLOTS-006
* FE-AI-SLOTS-001 to FE-AI-SLOTS-007
*
* A change that carries a card is drawn through its kind's view: the slots
* of a waiting row and of its receipt, a kind that draws its own subject, a
* group around a run of rows of one kind, a run of waiting rows decided as
* one, rows that start switched off, and slots that throw drawing what the
* row shows without a card. The views here are test views.
* one, rows that start switched off, slots that throw drawing what the row
* shows without a card, and a run and a group told to wait while the chat
* runs. The views here are test views.
*/
import type { AssistantApplied, AssistantChangePreview, TrekCard } from '@trek/shared';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
@@ -36,21 +37,31 @@ vi.mock('./cardViews', async (importOriginal) => {
},
tourList: {
ownsSubject: true,
Group: function Group({ items, place }: { items: readonly ChangeItem[]; place: string }) {
return createElement('p', null, `${place} of ${items.length}`);
Group: function Group({
items,
place,
disabled,
}: {
items: readonly ChangeItem[];
place: string;
disabled: boolean;
}) {
return createElement('p', null, `${place} of ${items.length}${disabled ? ', waiting' : ''}`);
},
Run: function Run({
items,
excluded,
disabled,
onToggle,
}: {
items: readonly ChangeItem[];
excluded: ReadonlySet<string>;
disabled: boolean;
onToggle: (id: string) => void;
}) {
return createElement(
'button',
{ type: 'button', onClick: () => onToggle(items[0]?.toolCallId ?? '') },
{ type: 'button', disabled, onClick: () => onToggle(items[0]?.toolCallId ?? '') },
`run of ${items.length}, ${excluded.size} off`
);
},
@@ -184,4 +195,13 @@ describe('ChangeCardSlots', () => {
expect(logged).toHaveBeenCalled();
logged.mockRestore();
});
it('FE-AI-SLOTS-007: while the chat runs a run and a group are told to wait, as the rows are', () => {
useAssistantStore.setState({
byTrip: { [TRIP]: { ...chatOf(useAssistantStore.getState(), TRIP), threadId: 1, isRunning: true } },
});
renderCard([pending('c1', 'One', LIST), pending('c2', 'Two', LIST)]);
expect(screen.getByRole('button', { name: /^run of 2/ })).toBeDisabled();
expect(screen.getByText('head of 2, waiting')).toBeInTheDocument();
});
});
@@ -346,7 +346,7 @@ export function ChangeRows({ items, folded, excluded, disabled, undone, undoable
// A run that throws falls back to its rows one by one.
body.push(
<CardBoundary key={`run-${taken[0]?.toolCallId}`} fallback={taken.map(row)}>
<Run items={taken} excluded={excluded} onToggle={onToggle} />
<Run items={taken} excluded={excluded} disabled={disabled} onToggle={onToggle} />
</CardBoundary>
);
}
@@ -357,7 +357,7 @@ export function ChangeRows({ items, folded, excluded, disabled, undone, undoable
if (!Group || !first) return body;
const group = (place: 'head' | 'foot') => (
<CardBoundary key={`${place}-${first.toolCallId}`}>
<Group items={run} excluded={excluded} onToggle={onToggle} place={place} />
<Group items={run} excluded={excluded} disabled={disabled} onToggle={onToggle} place={place} />
</CardBoundary>
);
return [group('head'), ...body, group('foot')];
@@ -5,11 +5,12 @@ import { daysLater, isChanged, itemsOf, layoversOf, stopsOf, toneOfItem } from '
import { BOOKING_LOOK, bookingSpotOf, bookingTarget } from './bookingLook';
/**
* FE-AI-BOOKLOOK-001 to FE-AI-BOOKLOOK-008
* FE-AI-BOOKLOOK-001 to FE-AI-BOOKLOOK-009
*
* The pure side of the booking cards: what an applied booking or stay opens,
* where it sits on the map, which entity a card is about, and the ticket
* arithmetic (stops, stopovers, a later arrival) the card draws from.
* where it sits on the map, which entity a card is about, the ticket
* arithmetic (stops, stopovers, a later arrival) the card draws from, and
* when confirming is offered as a follow-up.
*/
const day = (id: number | null, number: number | null, date: string | null) => ({ id, number, date });
@@ -171,4 +172,18 @@ describe('booking look', () => {
expect(toneOfItem({ type: 'restaurant', status: 'cancelled' })).toBe('pending');
expect(toneOfItem({ type: 'hotel', status: null })).toBeNull();
});
it('FE-AI-BOOKLOOK-009: confirming is offered only beside a booking of this trip that waits for it', () => {
const when = BOOKING_LOOK.cards.booking.followUps.find((followUp) => followUp.id === 'bookingsConfirm')?.when;
expect(when?.(card([FLIGHT]))).toBe(true);
expect(
when?.(
card([
{ ...FLIGHT, status: 'confirmed' },
{ ...STAY, status: null },
])
)
).toBe(false);
expect(when?.(card([{ ...FLIGHT, tripId: 9, tripTitle: 'Vienna' }]))).toBe(false);
});
});
@@ -1,4 +1,10 @@
import { isTransportReservationType, type AssistantApplied, type BookingItem, type TrekCard } from '@trek/shared';
import {
isTransportReservationType,
type AssistantApplied,
type BookingCard,
type BookingItem,
type TrekCard,
} from '@trek/shared';
import { BedDouble, CalendarCog, Hotel, Ticket } from 'lucide-react';
import type { EntityTarget, PackLook } from '../cardTypes';
@@ -63,6 +69,11 @@ function reservationOpen(applied: AssistantApplied, card: TrekCard | null): Enti
return item && id !== null ? bookingTarget(item, id) : null;
}
/** Whether a card shows a booking of this trip still waiting for its confirmation, one the chat can confirm. */
function holdsPendingHere(card: BookingCard): boolean {
return card.items.some((item) => item.tripTitle === null && item.status === 'pending');
}
/** The check-in day of an applied stay, where the planner's day head shows it. */
function stayDayOf(applied: AssistantApplied, card: TrekCard | null): number | null {
const body = bookingItemOf(card)?.body;
@@ -119,7 +130,7 @@ export const BOOKING_LOOK = {
followUps: [
{ id: 'bookingsOpen', labelKey: 'assistant.followUp.bookingsOpen' },
{ id: 'bookingsNights', labelKey: 'assistant.followUp.bookingsNights' },
{ id: 'bookingsConfirm', labelKey: 'assistant.followUp.bookingsConfirm', edits: true },
{ id: 'bookingsConfirm', labelKey: 'assistant.followUp.bookingsConfirm', edits: true, when: holdsPendingHere },
],
},
},
@@ -53,6 +53,11 @@ export interface EntityLook {
phoneOpen?(applied: AssistantApplied, card: TrekCard | null): EntityTarget | null;
}
/** What a kind offers to ask next; `when` keeps it to the answers whose cards of the kind call for it. */
export interface CardFollowUp<K extends TrekCardKind> extends FollowUp {
when?(card: TrekCardOf<K>): boolean;
}
/**
* The pure side of one kind: what it touches, what to ask next, how an answer
* lays it out, and what the map draws for it (see types/assistantMap.ts). Pure,
@@ -60,7 +65,7 @@ export interface EntityLook {
*/
export interface CardLook<K extends TrekCardKind> {
entity?(card: TrekCardOf<K>): AssistantApplied['entity'];
followUps?: readonly FollowUp[];
followUps?: readonly CardFollowUp<K>[];
/** The group an answer card joins; the kind when left out. */
group?(card: TrekCardOf<K>): string;
/** Drops a lookup card next to the change it served. */
@@ -95,6 +100,8 @@ interface CardSlotProps<K extends TrekCardKind> {
interface CardRunProps {
items: readonly ChangeItem[];
excluded: ReadonlySet<string>;
/** The choices wait while the chat runs, as the change card's own do. */
disabled: boolean;
onToggle(id: string): void;
}
@@ -1,10 +1,11 @@
/**
* FE-AI-PACKRUN-001 to FE-AI-PACKRUN-003
* FE-AI-PACKRUN-001 to FE-AI-PACKRUN-004
*
* A run of single-row packing changes reads as one checklist inside the
* change card: each category's head once, a row per change with its own
* include choice and the tile of what it does, and the facts of the change
* with old values struck through. A change switched off fades.
* with old values struck through. A change switched off fades, and the
* choices wait while the chat runs.
*/
import type { PackingCardItem, PackingListCard } from '@trek/shared';
import type { ReactNode } from 'react';
@@ -74,7 +75,7 @@ function inView(ui: ReactNode, view: Partial<AssistantView> = {}) {
describe('PackingChangeRun', () => {
it("FE-AI-PACKRUN-001: each category's head shows once, a row per change with what it does", () => {
inView(<PackingChangeRun items={ITEMS} excluded={new Set()} onToggle={vi.fn()} />);
inView(<PackingChangeRun items={ITEMS} excluded={new Set()} disabled={false} onToggle={vi.fn()} />);
expect(screen.getAllByText('Toiletries')).toHaveLength(1);
expect(screen.getAllByText('Clothes')).toHaveLength(1);
expect(screen.getAllByText('New')).toHaveLength(2);
@@ -87,7 +88,7 @@ describe('PackingChangeRun', () => {
it('FE-AI-PACKRUN-002: every row has its own include choice, and a switched-off one fades', () => {
const onToggle = vi.fn();
inView(<PackingChangeRun items={ITEMS} excluded={new Set(['c5'])} onToggle={onToggle} />);
inView(<PackingChangeRun items={ITEMS} excluded={new Set(['c5'])} disabled={false} onToggle={onToggle} />);
const choices = screen.getAllByRole('button', { name: 'Include this change' });
expect(choices).toHaveLength(5);
expect(choices[4]).toHaveAttribute('aria-pressed', 'false');
@@ -98,11 +99,27 @@ describe('PackingChangeRun', () => {
it('FE-AI-PACKRUN-003: on the phone the choice gets a larger hit area; a change without a card is left out', () => {
const plain: ChangeItem = { toolCallId: 'c9', preview: null, applied: null, state: 'pending', card: null };
inView(<PackingChangeRun items={[...ITEMS.slice(0, 2), plain]} excluded={new Set()} onToggle={vi.fn()} />, {
variant: 'fullscreen',
});
inView(
<PackingChangeRun
items={[...ITEMS.slice(0, 2), plain]}
excluded={new Set()}
disabled={false}
onToggle={vi.fn()}
/>,
{ variant: 'fullscreen' }
);
const choices = screen.getAllByRole('button', { name: 'Include this change' });
expect(choices).toHaveLength(2);
expect(choices[0]).toHaveClass('h-8', 'w-8');
});
it('FE-AI-PACKRUN-004: while the chat runs every include choice waits', () => {
const onToggle = vi.fn();
inView(<PackingChangeRun items={ITEMS} excluded={new Set()} disabled onToggle={onToggle} />);
const choices = screen.getAllByRole('button', { name: 'Include this change' });
expect(choices).toHaveLength(5);
for (const choice of choices) expect(choice).toBeDisabled();
fireEvent.click(choices[0] as HTMLElement);
expect(onToggle).not.toHaveBeenCalled();
});
});
@@ -188,7 +188,7 @@ describe('TodoListCard', () => {
pending('c1', one(task(null, 'Book the train', { change: 'add' }))),
pending('c2', one(task(22, 'Travel insurance', { change: 'check', checked: true }))),
];
inView(<TodoChangeRun items={items} excluded={new Set()} onToggle={onToggle} />);
inView(<TodoChangeRun items={items} excluded={new Set()} disabled={false} onToggle={onToggle} />);
expect(screen.getAllByText('Bookings')).toHaveLength(1);
expect(screen.getByText('New')).toBeInTheDocument();
expect(screen.getByText('Done')).toBeInTheDocument();
@@ -138,8 +138,18 @@ export interface RunEntry {
facts: ReactNode;
}
/** A change in a run: the round include choice, the tile of what it does, and the row. */
function RunRow({ entry, excluded, onToggle }: { entry: RunEntry; excluded: boolean; onToggle: (id: string) => void }) {
/** A change in a run: the round include choice (waiting while the chat runs), the tile of what it does, and the row. */
function RunRow({
entry,
excluded,
disabled,
onToggle,
}: {
entry: RunEntry;
excluded: boolean;
disabled: boolean;
onToggle: (id: string) => void;
}) {
const { t } = useTranslation();
const { variant } = useAssistantView();
const glyph = ROW_GLYPH[entry.change];
@@ -152,9 +162,10 @@ function RunRow({ entry, excluded, onToggle }: { entry: RunEntry; excluded: bool
<button
type="button"
onClick={() => onToggle(entry.item.toolCallId)}
disabled={disabled}
aria-pressed={!excluded}
aria-label={t('assistant.change.include')}
className={`grid flex-none place-items-center rounded-full outline-none focus-visible:ring-2 focus-visible:ring-accent ${
className={`grid flex-none place-items-center rounded-full outline-none focus-visible:ring-2 focus-visible:ring-accent disabled:opacity-50 ${
variant === 'fullscreen' ? 'h-8 w-8' : 'h-[18px] w-[18px]'
}`}
>
@@ -179,6 +190,8 @@ function RunRow({ entry, excluded, onToggle }: { entry: RunEntry; excluded: bool
export interface RunProps {
items: readonly ChangeItem[];
excluded: ReadonlySet<string>;
/** The include choices wait while the chat runs. */
disabled: boolean;
onToggle(id: string): void;
}
@@ -191,6 +204,7 @@ export interface RunProps {
export function ChecklistRun({
items,
excluded,
disabled,
onToggle,
entryOf,
}: RunProps & { entryOf: (item: ChangeItem) => RunEntry | null }) {
@@ -200,7 +214,12 @@ export function ChecklistRun({
{entries.map((entry, i) => (
<div key={entry.item.toolCallId}>
{entries[i - 1]?.headKey !== entry.headKey && entry.head}
<RunRow entry={entry} excluded={excluded.has(entry.item.toolCallId)} onToggle={onToggle} />
<RunRow
entry={entry}
excluded={excluded.has(entry.item.toolCallId)}
disabled={disabled}
onToggle={onToggle}
/>
</div>
))}
</div>