mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-11 08:07:15 +00:00
fix(editors): decide chat preservation from route paths, not editor state
navStaysInEditor used the open entity's path as the add->edit promotion signal,
but that path is undefined for a new/draft flow (flowOptions.path = savedFlow?.path),
so cross-flow navigation wrongly preserved the chat ("never reset"). Decide from
the beforeNavigate from/to pathnames instead, which are always concrete.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
00354989a3
commit
acf9ac2b26
@@ -1213,14 +1213,15 @@
|
||||
// false and the chat survives; the fresh onMount then sees mode is still
|
||||
// SCRIPT and skips its clearing saveAndClear.
|
||||
let leaveOnDestroy = $state(false)
|
||||
beforeNavigate(({ to }) => {
|
||||
// Recompute on every navigation (both branches) so a stale decision from
|
||||
// an earlier same-script navigation never lingers into a later
|
||||
// cross-script one.
|
||||
beforeNavigate(({ from, to }) => {
|
||||
// Recompute on every navigation (both branches) so a stale decision never
|
||||
// lingers. Decided from the route pathnames so a new/draft script (whose
|
||||
// scriptEditorOptions.path is undefined) still clears when navigating away.
|
||||
leaveOnDestroy = !navStaysInEditor(
|
||||
from?.url.pathname ?? '',
|
||||
to?.url.pathname ?? '',
|
||||
'/scripts/edit/',
|
||||
aiChatManager.scriptEditorOptions?.path
|
||||
'/scripts/add',
|
||||
'/scripts/edit/'
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -2,37 +2,73 @@ import { describe, it, expect } from 'vitest'
|
||||
import { navStaysInEditor } from './editorNav'
|
||||
|
||||
describe('navStaysInEditor', () => {
|
||||
const FLOW = '/flows/edit/'
|
||||
const ADD = '/flows/add'
|
||||
const EDIT = '/flows/edit/'
|
||||
|
||||
it('preserves on the add → edit promotion (no current path yet)', () => {
|
||||
// On /flows/add the entity has no path; the first save navigates to
|
||||
// /flows/edit/{path}. Both undefined and '' must count as "promotion".
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', FLOW, undefined)).toBe(true)
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', FLOW, '')).toBe(true)
|
||||
it('preserves on the add → edit promotion', () => {
|
||||
expect(navStaysInEditor('/flows/add', '/flows/edit/u/admin/foo', ADD, EDIT)).toBe(true)
|
||||
})
|
||||
|
||||
it('preserves when staying on the same entity', () => {
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', FLOW, 'u/admin/foo')).toBe(true)
|
||||
it('preserves when staying on the same entity (e.g. a query change)', () => {
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', '/flows/edit/u/admin/foo', ADD, EDIT)).toBe(
|
||||
true
|
||||
)
|
||||
})
|
||||
|
||||
it('clears when navigating to a different entity', () => {
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/bar', FLOW, 'u/admin/foo')).toBe(false)
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', '/flows/edit/u/admin/bar', ADD, EDIT)).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('clears cross-entity even when editor state would be undefined (the bug)', () => {
|
||||
// The old signature used the open entity's path as the promotion signal,
|
||||
// which is undefined for a new/draft flow — making this wrongly preserve.
|
||||
// Deciding from `from` (the real current route) fixes it: a draft flow's
|
||||
// edit route is still a concrete `/flows/edit/{path}`.
|
||||
expect(
|
||||
navStaysInEditor('/flows/edit/u/admin/draftflow', '/flows/edit/u/admin/other', ADD, EDIT)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('clears when leaving the editor entirely', () => {
|
||||
expect(navStaysInEditor('/', FLOW, 'u/admin/foo')).toBe(false)
|
||||
expect(navStaysInEditor('/scripts/edit/u/admin/foo', FLOW, 'u/admin/foo')).toBe(false)
|
||||
expect(navStaysInEditor('/flows/get/u/admin/foo', FLOW, 'u/admin/foo')).toBe(false)
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', '/', ADD, EDIT)).toBe(false)
|
||||
expect(
|
||||
navStaysInEditor('/flows/edit/u/admin/foo', '/scripts/edit/u/admin/foo', ADD, EDIT)
|
||||
).toBe(false)
|
||||
expect(navStaysInEditor('/flows/edit/u/admin/foo', '/flows/get/u/admin/foo', ADD, EDIT)).toBe(
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('does not treat a prefixed-but-different route as the editor', () => {
|
||||
// '/flows/edit-history/...' must not be mistaken for '/flows/edit/...'
|
||||
expect(navStaysInEditor('/flows/edit-history/foo', FLOW, undefined)).toBe(false)
|
||||
expect(navStaysInEditor('/flows/add', '/flows/edit-history/foo', ADD, EDIT)).toBe(false)
|
||||
})
|
||||
|
||||
it('works for the raw-app and script prefixes too', () => {
|
||||
expect(navStaysInEditor('/apps_raw/edit/u/admin/a', '/apps_raw/edit/', '')).toBe(true)
|
||||
expect(navStaysInEditor('/apps_raw/edit/u/admin/b', '/apps_raw/edit/', 'u/admin/a')).toBe(false)
|
||||
expect(navStaysInEditor('/scripts/edit/u/admin/s', '/scripts/edit/', 'u/admin/s')).toBe(true)
|
||||
it('works for the raw-app and script add/edit routes too', () => {
|
||||
expect(
|
||||
navStaysInEditor(
|
||||
'/apps_raw/add',
|
||||
'/apps_raw/edit/u/admin/a',
|
||||
'/apps_raw/add',
|
||||
'/apps_raw/edit/'
|
||||
)
|
||||
).toBe(true)
|
||||
expect(
|
||||
navStaysInEditor(
|
||||
'/apps_raw/edit/u/admin/a',
|
||||
'/apps_raw/edit/u/admin/b',
|
||||
'/apps_raw/add',
|
||||
'/apps_raw/edit/'
|
||||
)
|
||||
).toBe(false)
|
||||
expect(
|
||||
navStaysInEditor(
|
||||
'/scripts/edit/u/admin/s',
|
||||
'/scripts/edit/u/admin/s',
|
||||
'/scripts/add',
|
||||
'/scripts/edit/'
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,27 +1,38 @@
|
||||
/**
|
||||
* Whether a navigation destination keeps the user within the same editor
|
||||
* entity, so the AI chat about that entity should be preserved across the
|
||||
* resulting unmount/remount instead of cleared.
|
||||
* Whether a navigation from `fromPathname` to `toPathname` stays within the
|
||||
* same editor entity, so the AI chat about that entity should be preserved
|
||||
* across the resulting unmount/remount instead of cleared.
|
||||
*
|
||||
* Used by FlowEditor / ScriptEditor / RawAppEditor to decide, in a
|
||||
* `beforeNavigate` guard, whether leaving for `destPathname` is a real leave
|
||||
* (clear the chat) or an intra-editor transition (keep it):
|
||||
* Preserves when:
|
||||
* - the destination is the SAME entity currently open
|
||||
* (`{editPrefix}A` → `{editPrefix}A`, e.g. a `?selected=…` query change), or
|
||||
* - the `{addPathname}` → `{editPrefix}{path}` first-save promotion.
|
||||
*
|
||||
* - `editPrefix` is the editor's edit-route prefix, e.g. `'/flows/edit/'`.
|
||||
* - `currentPath` is the entity currently open. It is `undefined`/`''` on the
|
||||
* add page, which is exactly the `add → edit/{path}` first-save promotion —
|
||||
* that case must preserve, hence the `!currentPath` branch.
|
||||
* Clears for a different entity, or any destination outside this editor.
|
||||
*
|
||||
* Note this only covers *navigations*. Non-navigation remounts (e.g. an edit
|
||||
* page wiping then reloading its data) fire no `beforeNavigate`, so callers
|
||||
* default to preserving in that case.
|
||||
* This decides purely from the route pathnames (both reliably available from a
|
||||
* `beforeNavigate` callback's `from`/`to`) rather than from editor state such as
|
||||
* `flowOptions.path`, which is undefined for a new or draft-only entity — using
|
||||
* that as the promotion signal made cross-entity navigation wrongly preserve.
|
||||
*
|
||||
* Used by FlowEditor / ScriptEditor / RawAppEditor. Non-navigation remounts
|
||||
* (e.g. an edit page wiping then reloading its data) fire no `beforeNavigate`,
|
||||
* so callers default to preserving in that case.
|
||||
*/
|
||||
export function navStaysInEditor(
|
||||
destPathname: string,
|
||||
editPrefix: string,
|
||||
currentPath: string | undefined
|
||||
fromPathname: string,
|
||||
toPathname: string,
|
||||
addPathname: string,
|
||||
editPrefix: string
|
||||
): boolean {
|
||||
if (!destPathname.startsWith(editPrefix)) return false
|
||||
const destPath = destPathname.slice(editPrefix.length)
|
||||
return !currentPath || destPath === currentPath
|
||||
// Leaving this editor entirely (home, a different section, the entity's
|
||||
// non-edit pages, …).
|
||||
if (!toPathname.startsWith(editPrefix)) return false
|
||||
// `{addPathname}` → `{editPrefix}{path}`: the first-save promotion.
|
||||
if (fromPathname === addPathname) return true
|
||||
// Same entity (e.g. a `?selected=…` query change on the same edit route).
|
||||
if (fromPathname.startsWith(editPrefix)) {
|
||||
return fromPathname.slice(editPrefix.length) === toPathname.slice(editPrefix.length)
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -147,15 +147,15 @@
|
||||
// so leaveOnDestroy stays false and the chat survives; the fresh onMount then
|
||||
// sees mode is still FLOW and skips its clearing saveAndClear.
|
||||
let leaveOnDestroy = $state(false)
|
||||
beforeNavigate(({ to }) => {
|
||||
// Recompute on every navigation (both branches) so a stale decision from
|
||||
// an earlier same-flow navigation never lingers into a later cross-flow
|
||||
// one. The add page has no flowOptions.path yet, which navStaysInEditor
|
||||
// treats as the add→edit promotion (preserve).
|
||||
beforeNavigate(({ from, to }) => {
|
||||
// Recompute on every navigation (both branches) so a stale decision never
|
||||
// lingers. Decided from the route pathnames so a new/draft flow (whose
|
||||
// flowOptions.path is undefined) still clears when navigating cross-flow.
|
||||
leaveOnDestroy = !navStaysInEditor(
|
||||
from?.url.pathname ?? '',
|
||||
to?.url.pathname ?? '',
|
||||
'/flows/edit/',
|
||||
aiChatManager.flowOptions?.path
|
||||
'/flows/add',
|
||||
'/flows/edit/'
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -628,10 +628,16 @@
|
||||
// beforeNavigate, so leaveOnDestroy stays false and the chat survives; the
|
||||
// fresh onMount then sees mode is still APP and skips its clearing saveAndClear.
|
||||
let leaveOnDestroy = $state(false)
|
||||
beforeNavigate(({ to }) => {
|
||||
// Recompute on every navigation (both branches) so a stale decision from
|
||||
// an earlier same-app navigation never lingers into a later cross-app one.
|
||||
leaveOnDestroy = !navStaysInEditor(to?.url.pathname ?? '', '/apps_raw/edit/', path)
|
||||
beforeNavigate(({ from, to }) => {
|
||||
// Recompute on every navigation (both branches) so a stale decision never
|
||||
// lingers. Decided from the route pathnames (not this instance's `path`,
|
||||
// which is '' on the add page) so cross-app navigation clears reliably.
|
||||
leaveOnDestroy = !navStaysInEditor(
|
||||
from?.url.pathname ?? '',
|
||||
to?.url.pathname ?? '',
|
||||
'/apps_raw/add',
|
||||
'/apps_raw/edit/'
|
||||
)
|
||||
})
|
||||
|
||||
onMount(() => {
|
||||
|
||||
Reference in New Issue
Block a user