Skip to content

fix(ENG-11669): emit on success for oauth flows#86

Merged
adefreitas merged 2 commits intoStackOneHQ:mainfrom
adefreitas:ENG-11669/emit-success-for-all-oauth-flows
Dec 17, 2025
Merged

fix(ENG-11669): emit on success for oauth flows#86
adefreitas merged 2 commits intoStackOneHQ:mainfrom
adefreitas:ENG-11669/emit-success-for-all-oauth-flows

Conversation

@adefreitas
Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings December 17, 2025 15:54
@adefreitas adefreitas enabled auto-merge (squash) December 17, 2025 15:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the OAuth success handling to ensure the onSuccess callback is emitted consistently across all OAuth flow paths. The main change introduces a centralized handleSuccess callback that both sets the connection state and triggers the onSuccess callback, replacing multiple instances of direct state setting.

Key Changes:

  • Introduced a handleSuccess callback that centralizes success handling logic
  • Changed the timing of onSuccess emission in the non-OAuth flow from delayed (2 seconds) to immediate
  • Updated dependency arrays for affected useCallback hooks to include handleSuccess

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/modules/integration-picker/hooks/useIntegrationPicker.ts Outdated
@adefreitas adefreitas disabled auto-merge December 17, 2025 16:02
@adefreitas adefreitas enabled auto-merge (squash) December 17, 2025 16:08
Comment thread dev/main.tsx
token={token}
baseUrl={apiUrl}
theme={theme}
onSuccess={() => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional we're showing an alert instead of previously showing a log?

Copy link
Copy Markdown
Contributor

@joeStackOne joeStackOne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adefreitas adefreitas merged commit b62990b into StackOneHQ:main Dec 17, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants