Gh 148 onconnectionchange oauth fix#149
Merged
Merged
Conversation
…ker handling Design/root-cause analysis for GH-148. Two defects: - Post-OAuth onConnectionChange (callable) is emitted from AuthorizeButton, which unmounts the moment the connection becomes callable, cancelling the emit. Fix: emit from the stable ConnectionsProvider on the transition into callable, deduped; remove the fragile AuthorizeButton emits. - autoStartAuthorization calls window.open with no user gesture; a blocked popup leaves the button stuck loading. Fix: detect null handle, toast + reset so a manual click (a real gesture) can retry. Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
Move the post-OAuth "now usable" emission out of AuthorizeButton (which unmounts the moment a connection becomes callable, cancelling the emit) into ConnectionsProvider, which never unmounts on the view switch. Emit once per transition into `callable`, deduped via a ref against updateConnection. Remove the now-redundant AuthorizeButton emits + prop. Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
autoStartAuthorization (and any authorize/re-authorize) opens the OAuth window via window.open; a popup blocker returns null and left the button stuck in its loading state forever. Detect the null handle, show a toast, and reset state so a manual click (a real user gesture) can retry. Adds "Popup blocked" copy to all 5 locales. Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
Addresses the two confirmed bugs from the code review of the callable-emit change: 1. Missed emit on reconnect: the set-only lastEmittedRef never reset when a connection left `callable`, so callable -> invalid -> callable (a revoked token that is re-authorized) emitted nothing. Detect the transition from prevConnection.state instead of a persistent flag, so a re-entry emits. 2. Double emit on re-authorize: connectionActions.handleRedirect emitted inline AND the provider effect fired for an authorized -> callable re-auth. Replace the fragile ref with a disjoint-responsibility model: the provider effect owns "entry into callable"; updateConnection and handleRedirect emit everything EXCEPT an entry into callable. No shared dedup ref, race-free. Regression tests: callable->invalid->callable emits once; authorized->callable re-auth emits once (isolated SWR cache); clear SWR cache between tests. Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
…low-up) The disjoint-emit model had one hole found in review: in multi-connection list mode, clicking an `available` connection that the backend enables straight to `callable` emitted nothing. updateConnection based its skip on `selectedConnection?.state` (the useState var — null/stub when nothing is selected), so it read `undefined` and skipped, while the provider effect also couldn't fire (no prior non-callable render for an unselected connection). Base the skip on the merged `connection` (the actually-loaded connection) instead: only skip when the connection was already selected at a non-callable state — exactly when the effect will cover the transition. Regression test: enabling an unselected connection straight to callable emits onConnectionChange exactly once. Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
size-limit report 📦
|
The blocked-popup toast was misleading: with autoStartAuthorization the popup is blocked because window.open runs without a user gesture — the user doesn't need to change any browser setting, they just click the button (a gesture), which opens it. No customer asked for an "allow pop-ups" prompt. Keep the load-bearing fix (reset loading state so the button is clickable again) and remove the toast + its now-dead i18n strings. Ref #148 Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.