rusackas commented on code in PR #41285:
URL: https://github.com/apache/superset/pull/41285#discussion_r4154933681
##########
superset-frontend/src/extensions/ExtensionsStartup.test.tsx:
##########
@@ -260,26 +274,49 @@ test('does not initialize ExtensionsLoader when
EnableExtensions feature flag is
initializeSpy.mockRestore();
});
-test('continues rendering children even when ExtensionsLoader initialization
fails', async () => {
+test('surfaces a warning toast naming the extensions that failed to
initialize', async () => {
+ mockIsFeatureEnabled.mockReturnValue(true);
+
+ // A single extension's remote entry failing does not reject the aggregate;
+ // the loader resolves with the names of the failed extensions instead.
+ ExtensionsLoader.prototype.initializeExtensions = jest
+ .fn()
+ .mockResolvedValue(['Broken Extension']);
+
+ const store = createStore(mockInitialState, reducerIndex);
+
+ render(
+ <ExtensionsStartup>
+ <div data-testid="child" />
+ </ExtensionsStartup>,
+ { store, useRouter: true },
+ );
+
+ await waitFor(() => {
+ expect(
+ getToastTexts(store).some(text =>
+ /Some extensions failed to load: Broken Extension/.test(text),
+ ),
+ ).toBe(true);
+ });
Review Comment:
Good catch, added a `toastType === ToastType.Warning` assertion so these
would actually catch a severity downgrade.
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:
##########
@@ -275,6 +304,56 @@ const SqlEditor: FC<Props> = ({
const logAction = useLogAction({ queryEditorId: queryEditor.id });
const isActive = currentQueryEditorId === queryEditor.id;
+
+ // Re-renders when an extension registers a northPane view after async load.
+ const northPaneViews = useViews(ViewLocations.sqllab.northPane) || [];
+
+ // Resolve the per-tab localStorage key the same way every other SQL Lab
+ // consumer does (`tabViewId ?? id`), so the value written, read back, and
+ // observed via the `storage` event all agree once a tab is
backend-persisted.
+ const northPaneStorageId = queryEditor.tabViewId ?? queryEditor.id;
+
+ // ID of the northPane view active for this tab, or null for the default
+ // SQL editor layout. A tab created through the extension API carries the
+ // requested view on its own query editor state, so it can never be picked
+ // up by another tab. Editors hydrated from the backend on reload don't
+ // carry the field, so fall back to the per-tab localStorage entry that the
+ // effect below keeps in sync.
+ const [northPaneViewId, setNorthPaneViewId] = useState<string | null>(
+ () =>
+ queryEditor.northPaneViewId ??
+ readNorthPaneStorage(NORTH_PANE_VIEW_KEY(northPaneStorageId)),
+ );
+
+ // Tracks the storage id last written so that, when a tab syncs to the
+ // backend and `tabViewId` arrives, the entry under the old id-keyed key is
+ // removed rather than left orphaned in localStorage.
+ const northPaneStorageIdRef = useRef(northPaneStorageId);
+
+ useEffect(() => {
+ if (northPaneStorageIdRef.current !== northPaneStorageId) {
+ writeNorthPaneStorage(
+ NORTH_PANE_VIEW_KEY(northPaneStorageIdRef.current),
+ null,
+ );
+ northPaneStorageIdRef.current = northPaneStorageId;
+ }
+ writeNorthPaneStorage(
+ NORTH_PANE_VIEW_KEY(northPaneStorageId),
+ northPaneViewId,
+ );
+ }, [northPaneStorageId, northPaneViewId]);
+
+ useEffect(() => {
+ const handler = (e: StorageEvent) => {
+ if (e.key === NORTH_PANE_VIEW_KEY(northPaneStorageId)) {
+ setNorthPaneViewId(e.newValue || null);
+ }
+ };
+ window.addEventListener('storage', handler);
+ return () => window.removeEventListener('storage', handler);
+ }, [northPaneStorageId]);
Review Comment:
Pulled this into a `useNorthPaneView` hook, behavior unchanged.
##########
superset-frontend/src/core/sqlLab/index.ts:
##########
@@ -211,12 +252,18 @@ function extractBaseData(action: QueryAction): {
queryLimit,
} = query;
+ // Resolve backendId through the same path as every other Tab construction
+ // so query-event listeners can correlate the tab with its tabstateview row.
+ const queryEditor = findQueryEditorByAnyId(sqlEditorId ?? undefined);
Review Comment:
Query events fire once per lifecycle transition, not per keystroke, so the
scan is over however many tabs someone has open, a handful in practice. Not
worth a memoized map and its own cache-invalidation surface for that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
