sadpandajoe commented on code in PR #40292:
URL: https://github.com/apache/superset/pull/40292#discussion_r4151907526
##########
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:
If BroadcastChannel and localStorage are both blocked, neither completion
signal is sent, but the popup now closes and hides the instruction to re-run
the query manually. Could we leave it open when both sends fail, and cover that
path in a callback-script regression test?
##########
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:
This still passes if the old effect dependencies are restored: the channel
can be closed and recreated on the Redux update, and the later storage event
still succeeds. Could we also assert that an unrelated store update neither
closes nor recreates the channel, and that the original listener handles the
completion?
--
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]
