mirror of
https://github.com/apache/superset.git
synced 2026-07-27 09:02:29 +00:00
fix(chatbot): track view recency so last-loaded chatbot resolves correctly
The singleton chatbot resolver selects the most-recently-registered view, but the views registry backed each location with a Set and `add` is a no-op for an existing id — so re-registering a chatbot (e.g. on extension reload) did not move it to the end, and the resolver could keep selecting an older chatbot. - registerView now deletes-then-adds the id so re-registration reflects true recency; getRegisteredViewIds therefore returns ids in recency order. - Update the chatbot resolver comment to match (recency, not first-registration). - Add a registry regression test for re-register ordering. Also drop SIP section cross-references (§3.2, §10/§11) from the chatbot mount and dashboard context comments; the surrounding explanations stay. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -59,10 +59,11 @@ export const getActiveChatbot = (): ActiveChatbot | undefined => {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// `getRegisteredViewIds` returns ids in registration order, so the last entry
|
||||
// is the most-recently-loaded chatbot. `getViewProvider` reads the same
|
||||
// synchronous registry maps, so the id always has a live provider; the final
|
||||
// guard is cheap defensiveness, not a fallback path.
|
||||
// `getRegisteredViewIds` returns ids in recency order — the registry moves an
|
||||
// id to the end when it re-registers — so the last entry is the
|
||||
// most-recently-loaded chatbot. `getViewProvider` reads the same synchronous
|
||||
// registry maps, so the id always has a live provider; the final guard is
|
||||
// cheap defensiveness, not a fallback path.
|
||||
const selectedId = registeredIds[registeredIds.length - 1];
|
||||
|
||||
const provider = getViewProvider(CHATBOT_LOCATION, selectedId);
|
||||
|
||||
@@ -56,8 +56,8 @@ function buildChartSummaries(state: RootState): ChartSummary[] {
|
||||
vizType: slice?.viz_type ?? '',
|
||||
datasourceId: slice?.datasource_id ?? null,
|
||||
datasourceName: slice?.datasource_name ?? null,
|
||||
// Tab-accurate visibility is a deferred phase (SIP §10/§11); every chart
|
||||
// on the dashboard is reported visible for now.
|
||||
// Tab-accurate visibility is a deferred phase; every chart on the
|
||||
// dashboard is reported visible for now.
|
||||
isVisible: true,
|
||||
};
|
||||
});
|
||||
|
||||
@@ -168,6 +168,36 @@ test('getRegisteredViewIds returns ids in registration order', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
test('re-registering an existing id moves it to the end (recency order)', () => {
|
||||
const provider = () => React.createElement('div', null, 'Test');
|
||||
disposables.push(
|
||||
views.registerView(
|
||||
{ id: 'first.chatbot', name: 'First' },
|
||||
'core.chatbot',
|
||||
provider,
|
||||
),
|
||||
views.registerView(
|
||||
{ id: 'second.chatbot', name: 'Second' },
|
||||
'core.chatbot',
|
||||
provider,
|
||||
),
|
||||
);
|
||||
|
||||
// Re-register the first id; it must become the most recent, not stay first.
|
||||
disposables.push(
|
||||
views.registerView(
|
||||
{ id: 'first.chatbot', name: 'First' },
|
||||
'core.chatbot',
|
||||
provider,
|
||||
),
|
||||
);
|
||||
|
||||
expect(getRegisteredViewIds('core.chatbot')).toEqual([
|
||||
'second.chatbot',
|
||||
'first.chatbot',
|
||||
]);
|
||||
});
|
||||
|
||||
test('getRegisteredViewIds returns an empty array for an unused location', () => {
|
||||
expect(getRegisteredViewIds('core.chatbot')).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -75,6 +75,11 @@ const registerView: typeof viewsApi.registerView = (
|
||||
viewRegistry.set(id, { view, location, provider });
|
||||
|
||||
const ids = locationIndex.get(location) ?? new Set();
|
||||
// Re-registering an existing id must reflect its new recency. A Set preserves
|
||||
// first-insertion order and `add` is a no-op for an existing member, so delete
|
||||
// first to move the id to the end. Consumers that select the most-recently
|
||||
// registered view (e.g. the chatbot resolver) depend on this ordering.
|
||||
ids.delete(id);
|
||||
ids.add(id);
|
||||
locationIndex.set(location, ids);
|
||||
|
||||
|
||||
@@ -118,7 +118,7 @@ const App = () => (
|
||||
The singleton chatbot bubble. Rendered as a sibling of the route
|
||||
Switch — inside ExtensionsStartup so chatbot extensions have been
|
||||
loaded and registered, but outside the Switch so the bubble persists
|
||||
across route changes (SIP §3.2).
|
||||
across route changes.
|
||||
*/}
|
||||
{isFeatureEnabled(FeatureFlag.EnableExtensions) && <ChatbotMount />}
|
||||
</ExtensionsStartup>
|
||||
|
||||
Reference in New Issue
Block a user