Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ia): skip cache on google oauth flow #3743

Merged
merged 4 commits into from
Feb 12, 2025
Merged

Conversation

miguelpeixe
Copy link
Member

All Submissions:

Changes proposed in this Pull Request:

1205919985867982-as-1209366560752423

The caching strategy behind useWizardFetch is preventing the Google OAuth flow to start. This PR removes cache from that request.

How to test the changes in this Pull Request:

  1. Confirm you are connected to the staging manager and without Google OAuth configured (disconnect if you are connected)
  2. While on the epic/ia branch confirm you are unable to start the OAuth in the "Newspack -> Settings -> Connections" tab
  3. Checkout this branch and confirm you are able to start and complete the flow

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@miguelpeixe miguelpeixe added the [Status] Needs Review The issue or pull request needs to be reviewed label Feb 11, 2025
@miguelpeixe miguelpeixe self-assigned this Feb 11, 2025
@miguelpeixe miguelpeixe requested a review from a team as a code owner February 11, 2025 15:15
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

I'm connected to the staging manager instance, but on this branch I get the following error message when attempting to connect:

Screenshot 2025-02-12 at 10 44 46 AM

Stack trace:

Uncaught (in promise) WizardApiError: cURL error 35: error:0A000126:SSL routines::unexpected eof while reading
    WizardError class-wizard-error.ts:9
    WizardApiError class-wizard-api-error.ts:13
    parseApiError use-wizard-api-fetch.ts:61
    catchCallback use-wizard-api-fetch.ts:225
    promise callback*./src/wizards/hooks/use-wizard-api-fetch.ts/useWizardApiFetch/apiFetch< use-wizard-api-fetch.ts:268
    openAuth google-oauth.tsx:91
    key components.js:2104
    React 23
    tsx index.tsx:120
    Webpack 5

Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

Works! I ran into one issue that's probably not related to these changes, but let's fix it while we're here.

  1. Disconnect Google OAuth if already connected
  2. Refresh Connections page so you see the "Invalid or missing Google credentials" error message
  3. Connect Google OAuth
  4. After completing the connection, the error message lingers until a page refresh:
Screenshot 2025-02-12 at 11 01 16 AM

@miguelpeixe
Copy link
Member Author

Hmm... I thought the change in line 72 fixed this 🤔

Taking a look

@miguelpeixe
Copy link
Member Author

4341b4e should take care of it

@miguelpeixe miguelpeixe requested a review from dkoo February 12, 2025 18:11
Copy link
Contributor

@dkoo dkoo left a comment

Choose a reason for hiding this comment

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

Yep, that fixed it!

@github-actions github-actions bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Feb 12, 2025
@miguelpeixe miguelpeixe merged commit baad1b2 into epic/ia Feb 12, 2025
8 checks passed
@miguelpeixe miguelpeixe deleted the fix/ia-google-oauth-flow branch February 12, 2025 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Information Architecture [Status] Approved The pull request has been reviewed and is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants