rusackas commented on code in PR #40292:
URL: https://github.com/apache/superset/pull/40292#discussion_r4161665784
##########
superset/templates/superset/oauth2.html:
##########
@@ -25,13 +25,23 @@
<body>
<script nonce="{{ macros.get_nonce() }}">
const message = { tabId: '{{ tab_id }}' };
+ // Emit on both channels: postMessage success doesn't guarantee the
original
+ // tab's listener was attached. The receiver dedupes via a `handled`
flag.
if (typeof BroadcastChannel !== 'undefined') {
- const channel = new BroadcastChannel('oauth');
- channel.postMessage(message);
- channel.close();
+ try {
+ const channel = new BroadcastChannel('oauth');
+ channel.postMessage(message);
+ channel.close();
+ } catch (e) {
+ // ignore; storage path below still fires
+ }
+ }
+ try {
+ localStorage.setItem('oauth2_auth_complete', JSON.stringify(message));
+ localStorage.removeItem('oauth2_auth_complete');
+ } catch (e) {
Review Comment:
Good catch, fixed in b0cc385: `window.close()` now only runs once at least
one of the two sends actually succeeded, otherwise the window stays open with
the re-run instructions visible. Added `oauth2CallbackScript.test.ts` covering
it, it runs the actual inline script from the template rather than a copy of it.
##########
superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.test.tsx:
##########
@@ -304,4 +312,82 @@ describe('OAuth2RedirectMessage Component', () => {
});
expect(api.util.invalidateTags).not.toHaveBeenCalled();
});
+
+ test('dispatches only once when both BroadcastChannel and storage signals
arrive', async () => {
+ render(setup());
+
+ simulateBroadcastMessage({ tabId: 'tabId' });
+ simulateStorageMessage({ tabId: 'tabId' });
+
+ await waitFor(() => {
+ expect(reRunQuery).toHaveBeenCalledTimes(1);
+ });
+ });
+
+ test('falls back to storage events when BroadcastChannel construction
throws', async () => {
+ globalWithBroadcastChannel.BroadcastChannel = jest
+ .fn()
+ .mockImplementation(() => {
+ throw new Error('blocked');
+ });
+
+ render(setup());
+
+ simulateStorageMessage({ tabId: 'tabId' });
+
+ await waitFor(() => {
+ expect(reRunQuery).toHaveBeenCalledWith({ sql: 'SELECT * FROM table' });
+ });
+ });
+
+ test('re-processes a later signal when the first arrived before state was
ready', async () => {
+ const initialState = {
+ sqlLab: {
+ queries: {},
+ queryEditors: [{ id: 'editor-id' }],
+ tabHistory: ['editor-id'],
+ },
+ explore: { slice: { slice_id: 123 } },
+ charts: { '1': {}, '2': {} },
+ dashboardInfo: { id: 'dashboard-id' },
+ };
+ type DynamicStoreState = typeof initialState;
+ type DynamicStoreAction = { type: string };
+ const dynamicStore = createStore(
+ (state: DynamicStoreState = initialState, action: DynamicStoreAction) =>
{
+ if (action.type === 'SET_READY') {
+ return {
+ ...state,
+ sqlLab: {
+ ...state.sqlLab,
+ queries: { 'query-id': { sql: 'SELECT * FROM table' } },
+ queryEditors: [{ id: 'editor-id', latestQueryId: 'query-id' }],
+ },
+ };
+ }
+ return state;
+ },
+ );
+
+ render(
+ <Provider store={dynamicStore}>
+ <OAuth2RedirectMessage {...defaultProps} />
+ </Provider>,
+ );
+
+ // First signal arrives before the SQL Lab query state is populated;
+ // nothing dispatches and `handled` must NOT be flipped.
+ simulateBroadcastMessage({ tabId: 'tabId' });
+ expect(reRunQuery).not.toHaveBeenCalled();
+
+ // Query state becomes available, then the storage fallback signal fires.
+ act(() => {
+ dynamicStore.dispatch({ type: 'SET_READY' });
Review Comment:
Fair, fixed in b0cc385: the test now asserts the BroadcastChannel mock and
the storage listener spy both stay at a call count of 1 across the Redux
dispatch, so a regression back to the old state-dependent deps would actually
fail this test instead of sliding through on the final outcome alone.
--
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]
